This is an automated email from the ASF dual-hosted git repository. krickert pushed a commit to branch OPENNLP-1898 in repository https://gitbox.apache.org/repos/asf/opennlp.git
commit 8a0f5fb7d28f29115cbc86529a8a154d1973fe70 Author: Kristian Rickert <[email protected]> AuthorDate: Thu Jul 16 04:48:58 2026 -0400 OPENNLP-1898: Address the OPENNLP-1876 review follow-ups These notes were raised in the review of OPENNLP-1876 and addressed before PR #1151 merged, but the commit was never pushed, so the work is absent from main. - Document IllegalArgumentException and IllegalStateException on the public spellcheck normalizer, which throws both but declared neither - Restore the SpellCorrectingTokenStream class javadoc, including the contrast with SpellCorrectingObjectStream in compound mode that explains why the stream is token count preserving - Hoist the former-regex Pattern constants out of the four characterization test bodies, where they were recompiled on every one of the 5000 randomized inputs per case, and name what each pattern represents - Turn the adversarial URL pins into a parameterized test so a regression names the shape that broke instead of only the test --- ...CharSequenceNormalizerCharacterizationTest.java | 6 ++- ...CharSequenceNormalizerCharacterizationTest.java | 11 +++-- ...CharSequenceNormalizerCharacterizationTest.java | 21 +++++---- ...CharSequenceNormalizerCharacterizationTest.java | 54 +++++++++++++--------- .../SpellCheckingCharSequenceNormalizer.java | 9 +++- .../stream/SpellCorrectingTokenStream.java | 8 ++-- 6 files changed, 68 insertions(+), 41 deletions(-) diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NumberCharSequenceNormalizerCharacterizationTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NumberCharSequenceNormalizerCharacterizationTest.java index 16a7ca385..8d38f595b 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NumberCharSequenceNormalizerCharacterizationTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/NumberCharSequenceNormalizerCharacterizationTest.java @@ -39,6 +39,9 @@ public class NumberCharSequenceNormalizerCharacterizationTest { private static final NumberCharSequenceNormalizer NORMALIZER = NumberCharSequenceNormalizer.getInstance(); + /** Former digit-run regex used for differential characterization. */ + private static final Pattern FORMER_DIGIT_REGEX = Pattern.compile("\\d+"); + private static void check(String input, String expected) { assertEquals(expected, NORMALIZER.normalize(input).toString()); } @@ -89,13 +92,12 @@ public class NumberCharSequenceNormalizerCharacterizationTest { @Test void matchesTheFormerRegexOnRandomizedInputs() { - final Pattern formerRegex = Pattern.compile("\\d+"); final String[] pool = {"a", "Z", " ", "0", "1", "23", "007", ".", ",", "-", "+", "\u0661", "\u2460", "\uD835\uDFCF", "\uD83D\uDE00", "\uD83D", "\uDE00"}; final Random random = new Random(42); for (int i = 0; i < 5000; i++) { final String input = CharacterizationInputs.randomInput(random, pool); - assertEquals(formerRegex.matcher(input).replaceAll(" "), + assertEquals(FORMER_DIGIT_REGEX.matcher(input).replaceAll(" "), NORMALIZER.normalize(input).toString(), () -> "Input: " + CharacterizationInputs.escape(input)); } } diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/ShrinkCharSequenceNormalizerCharacterizationTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/ShrinkCharSequenceNormalizerCharacterizationTest.java index 81fb87d59..e4503790f 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/ShrinkCharSequenceNormalizerCharacterizationTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/ShrinkCharSequenceNormalizerCharacterizationTest.java @@ -47,6 +47,11 @@ public class ShrinkCharSequenceNormalizerCharacterizationTest { private static final ShrinkCharSequenceNormalizer NORMALIZER = ShrinkCharSequenceNormalizer.getInstance(); + private static final Pattern FORMER_SPACE_REGEX = + Pattern.compile("\\s{2,}", Pattern.CASE_INSENSITIVE); + private static final Pattern FORMER_REPEATED_CHAR_REGEX = + Pattern.compile("(.)\\1{2,}", Pattern.CASE_INSENSITIVE); + private static void check(String input, String expected) { assertEquals(expected, NORMALIZER.normalize(input).toString()); } @@ -142,16 +147,14 @@ public class ShrinkCharSequenceNormalizerCharacterizationTest { @Test void matchesTheFormerRegexOnRandomizedInputs() { - final Pattern spaceRegex = Pattern.compile("\\s{2,}", Pattern.CASE_INSENSITIVE); - final Pattern repeatedCharRegex = Pattern.compile("(.)\\1{2,}", Pattern.CASE_INSENSITIVE); final String[] pool = {"a", "aa", "aaa", "A", "b", "Z", "!", "!!", ".", "-", " ", " ", "\t", "\n", "\r", "\u0001", "\u0085", "\u2028", "\u00A0", "\u00E9", "\u00C9", "\uD83D\uDE00", "\uD83D", "\uDE00"}; final Random random = new Random(42); for (int i = 0; i < 5000; i++) { final String input = CharacterizationInputs.randomInput(random, pool); - final String expected = repeatedCharRegex - .matcher(spaceRegex.matcher(input).replaceAll(" ")) + final String expected = FORMER_REPEATED_CHAR_REGEX + .matcher(FORMER_SPACE_REGEX.matcher(input).replaceAll(" ")) .replaceAll("$1$1").trim(); assertEquals(expected, NORMALIZER.normalize(input).toString(), () -> "Input: " + CharacterizationInputs.escape(input)); diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/SocialMediaCharSequenceNormalizerCharacterizationTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/SocialMediaCharSequenceNormalizerCharacterizationTest.java index b53be6d26..f1bfe73df 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/SocialMediaCharSequenceNormalizerCharacterizationTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/SocialMediaCharSequenceNormalizerCharacterizationTest.java @@ -52,6 +52,14 @@ public class SocialMediaCharSequenceNormalizerCharacterizationTest { private static final SocialMediaCharSequenceNormalizer NORMALIZER = SocialMediaCharSequenceNormalizer.getInstance(); + private static final Pattern FORMER_HASH_USER_REGEX = Pattern.compile("[#@]\\S+"); + private static final Pattern FORMER_RT_REGEX = + Pattern.compile("\\b(rt[ :])+", Pattern.CASE_INSENSITIVE); + private static final Pattern FORMER_FACE_REGEX = + Pattern.compile("[:;x]-?[()dop]", Pattern.CASE_INSENSITIVE); + private static final Pattern FORMER_LAUGH_REGEX = + Pattern.compile("([hj])+([aieou])+(\\1+\\2+)+", Pattern.CASE_INSENSITIVE); + private static void check(String input, String expected) { assertEquals(expected, NORMALIZER.normalize(input).toString()); } @@ -168,11 +176,6 @@ public class SocialMediaCharSequenceNormalizerCharacterizationTest { @Test void matchesTheFormerRegexesOnRandomizedInputs() { - final Pattern hashUserRegex = Pattern.compile("[#@]\\S+"); - final Pattern rtRegex = Pattern.compile("\\b(rt[ :])+", Pattern.CASE_INSENSITIVE); - final Pattern faceRegex = Pattern.compile("[:;x]-?[()dop]", Pattern.CASE_INSENSITIVE); - final Pattern laughRegex = - Pattern.compile("([hj])+([aieou])+(\\1+\\2+)+", Pattern.CASE_INSENSITIVE); final String[] pool = {"a", "A", "e", "h", "H", "j", "J", "o", "u", "y", "x", "X", "d", "p", "P", ")", "(", "-", ":", ";", " ", " ", "\t", "\n", "#", "@", "_", "1", "rt", "RT", "rt:", "Rt ", "t", "r", "ha", "Ha", "ah", "hh", "aa", "jj", "\u00E9", "\u0301", "\uD800\uDDFD", @@ -180,10 +183,10 @@ public class SocialMediaCharSequenceNormalizerCharacterizationTest { final Random random = new Random(42); for (int i = 0; i < 5000; i++) { final String input = CharacterizationInputs.randomInput(random, pool); - String expected = hashUserRegex.matcher(input).replaceAll(" "); - expected = rtRegex.matcher(expected).replaceAll(" "); - expected = faceRegex.matcher(expected).replaceAll(" "); - expected = laughRegex.matcher(expected).replaceAll("$1$2$1$2"); + String expected = FORMER_HASH_USER_REGEX.matcher(input).replaceAll(" "); + expected = FORMER_RT_REGEX.matcher(expected).replaceAll(" "); + expected = FORMER_FACE_REGEX.matcher(expected).replaceAll(" "); + expected = FORMER_LAUGH_REGEX.matcher(expected).replaceAll("$1$2$1$2"); assertEquals(expected, NORMALIZER.normalize(input).toString(), () -> "Input: " + CharacterizationInputs.escape(input)); } diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/UrlCharSequenceNormalizerCharacterizationTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/UrlCharSequenceNormalizerCharacterizationTest.java index 2898452f6..e043ecff9 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/UrlCharSequenceNormalizerCharacterizationTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/normalizer/UrlCharSequenceNormalizerCharacterizationTest.java @@ -18,9 +18,12 @@ package opennlp.tools.util.normalizer; import java.util.Random; import java.util.regex.Pattern; +import java.util.stream.Stream; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -49,6 +52,14 @@ public class UrlCharSequenceNormalizerCharacterizationTest { private static final UrlCharSequenceNormalizer NORMALIZER = UrlCharSequenceNormalizer.getInstance(); + /** Former URL regex used for differential characterization. */ + private static final Pattern FORMER_URL_REGEX = + Pattern.compile("https?://[-_.?&~;+=/#0-9A-Za-z]+"); + + /** Former mail regex used for differential characterization. */ + private static final Pattern FORMER_MAIL_REGEX = + Pattern.compile("(?<![-+_.0-9A-Za-z])[-+_.0-9A-Za-z]+@[-0-9A-Za-z]+[-.0-9A-Za-z]+"); + private static void check(String input, String expected) { assertEquals(expected, NORMALIZER.normalize(input).toString()); } @@ -138,38 +149,34 @@ public class UrlCharSequenceNormalizerCharacterizationTest { @Test void matchesTheFormerRegexesOnRandomizedInputs() { - final Pattern urlRegex = Pattern.compile("https?://[-_.?&~;+=/#0-9A-Za-z]+"); - final Pattern mailRegex = - Pattern.compile("(?<![-+_.0-9A-Za-z])[-+_.0-9A-Za-z]+@[-0-9A-Za-z]+[-.0-9A-Za-z]+"); final String[] pool = {"a", "b", "Z", "1", " ", ".", ",", "-", "+", "_", "@", ":", "/", "%", "~", ";", "=", "&", "?", "#", "http", "https", "ttp", "://", "http://";, "s", "x.com", "co", ".com", "a@b", "@b.co", "\u00E9", "\uD83D\uDE00", "\uD83D", "\uDE00"}; final Random random = new Random(42); for (int i = 0; i < 5000; i++) { final String input = CharacterizationInputs.randomInput(random, pool); - final String expected = mailRegex - .matcher(urlRegex.matcher(input).replaceAll(" ")) + final String expected = FORMER_MAIL_REGEX + .matcher(FORMER_URL_REGEX.matcher(input).replaceAll(" ")) .replaceAll(" "); assertEquals(expected, NORMALIZER.normalize(input).toString(), () -> "Input: " + CharacterizationInputs.escape(input)); } } + @Test void noMatchInputIsReturnedUncopied() { final String plain = "no links in this sentence at all"; Assertions.assertSame(plain, NORMALIZER.normalize(plain)); } - @Test - void weirdUrlsMatchTheFormerRegexExactly() { - // Adversarial shapes in the spirit of the web-platform-tests URL suite: userinfo, ports, - // percent escapes, IPv6 brackets, backslashes, IDN and punycode hosts, stray schemes and - // separators. Each input runs differentially through the two former patterns, so the - // accept/reject boundary is pinned by construction rather than by hand. - final Pattern urls = Pattern.compile("https?://[-_.?&~;+=/#0-9A-Za-z]+"); - final Pattern mails = - Pattern.compile("(?<![-+_.0-9A-Za-z])[-+_.0-9A-Za-z]+@[-0-9A-Za-z]+[-.0-9A-Za-z]+"); - final String[] inputs = { + /** + * Adversarial shapes in the spirit of the web-platform-tests URL suite: userinfo, ports, + * percent escapes, IPv6 brackets, backslashes, IDN and punycode hosts, stray schemes and + * separators. Each input runs differentially through the two former patterns, so the + * accept/reject boundary is pinned by construction rather than by hand. + */ + static Stream<String> weirdUrls() { + return Stream.of( "http://user:[email protected]/path";, "https://example.com:8080/x";, "http://example.com/foo%20bar";, @@ -193,12 +200,15 @@ public class UrlCharSequenceNormalizerCharacterizationTest { "http://example.com/a b http://second.example/c";, "http:///triple-slash";, "http//missing-colon.example", - "https://.leading.dot";, - }; - for (final String input : inputs) { - final String viaRegexes = - mails.matcher(urls.matcher(input).replaceAll(" ")).replaceAll(" "); - assertEquals(viaRegexes, NORMALIZER.normalize(input).toString(), input); - } + "https://.leading.dot";); + } + + @ParameterizedTest + @MethodSource("weirdUrls") + void weirdUrlsMatchTheFormerRegexExactly(String input) { + final String viaRegexes = FORMER_MAIL_REGEX + .matcher(FORMER_URL_REGEX.matcher(input).replaceAll(" ")) + .replaceAll(" "); + assertEquals(viaRegexes, NORMALIZER.normalize(input).toString(), input); } } diff --git a/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/normalizer/SpellCheckingCharSequenceNormalizer.java b/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/normalizer/SpellCheckingCharSequenceNormalizer.java index d5a1f5d27..51344776a 100644 --- a/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/normalizer/SpellCheckingCharSequenceNormalizer.java +++ b/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/normalizer/SpellCheckingCharSequenceNormalizer.java @@ -179,7 +179,12 @@ public class SpellCheckingCharSequenceNormalizer implements CharSequenceNormaliz .build(); } - /** {@inheritDoc} */ + /** + * {@inheritDoc} + * + * @throws IllegalArgumentException if {@code text} is {@code null} + * @throws IllegalStateException if no {@link SpellChecker} is attached + */ @Override public CharSequence normalize(CharSequence text) { if (spellChecker == null) { @@ -454,6 +459,7 @@ public class SpellCheckingCharSequenceNormalizer implements CharSequenceNormaliz /** * @param value tokens shorter than this are left untouched; must be {@code >= 1} * @return this builder + * @throws IllegalArgumentException if {@code value} is less than {@code 1} */ public Builder minTokenLength(int value) { if (value < 1) { @@ -466,6 +472,7 @@ public class SpellCheckingCharSequenceNormalizer implements CharSequenceNormaliz /** * @param value the maximum edit distance considered per token; must be {@code >= 0} * @return this builder + * @throws IllegalArgumentException if {@code value} is negative */ public Builder maxEditDistance(int value) { if (value < 0) { diff --git a/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/stream/SpellCorrectingTokenStream.java b/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/stream/SpellCorrectingTokenStream.java index 981368c1a..99aa6d4c5 100644 --- a/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/stream/SpellCorrectingTokenStream.java +++ b/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/stream/SpellCorrectingTokenStream.java @@ -31,9 +31,11 @@ import opennlp.tools.util.ObjectStream; * (whitespace by default). Every token is spell-corrected independently and the tokens * are re-joined with the same delimiter. * - * <p>This is the shape produced by OpenNLP tokenizers and token-sample formats, a fixed - * sequence of tokens per element. This stream is token-count preserving: it never splits or - * merges tokens, so the corrected element stays aligned with any parallel annotation.</p> + * <p>This is the shape produced by OpenNLP tokenizers / token-sample formats and is + * what the trainable components consume: a fixed sequence of tokens per element. Unlike + * {@link SpellCorrectingObjectStream} in compound mode, this stream is + * <em>token-count preserving</em> – it never splits or merges tokens, so the + * corrected element stays aligned with any parallel annotation (tags, spans).</p> * * <p>Correction always runs in * {@link SpellCheckingCharSequenceNormalizer.Mode#PER_TOKEN per-token} mode and reuses
