Skip to content

Commit eca9b25

Browse files
krickertkrickert
committed
OPENNLP-1930: Show the whitespace mode dependence and the id overflow in the AD readers (red)
The tree and markup scans read whitespace through the mode-dependent check, so a next line character or a file separator parses one way under UNICODE and the other way under LEGACY. A text or paragraph id past the int range throws NumberFormatException instead of being invalid metadata, a contraction lexeme of underscores only throws ArrayIndexOutOfBoundsException, and a fallback line with no text after its last equals sign gives a leaf with an empty lexeme.
1 parent c2a14ed commit eca9b25

4 files changed

Lines changed: 172 additions & 2 deletions

File tree

‎opennlp-core/opennlp-formats/src/test/java/opennlp/tools/formats/ad/ADMetadataTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.stream.Stream;
2121

2222
import org.junit.jupiter.api.Assertions;
23+
import org.junit.jupiter.api.Test;
2324
import org.junit.jupiter.params.ParameterizedTest;
2425
import org.junit.jupiter.params.provider.Arguments;
2526
import org.junit.jupiter.params.provider.CsvSource;
@@ -157,4 +158,17 @@ void testSourceWithLineTerminator(String meta, String source) {
157158
void testSourceRejects(String meta) {
158159
Assertions.assertNull(ADMetadata.source(meta));
159160
}
161+
162+
@ParameterizedTest
163+
@ValueSource(strings = {"2147483648 p=1", "12 p=2147483648", "99999999999 p=1",
164+
"12 p=99999999999", "LIT-2147483648 p=1"})
165+
void testIdsThatDoNotFitAnIntAreInvalid(String meta) {
166+
Assertions.assertNull(ADMetadata.parseTextAndParagraph(meta));
167+
}
168+
169+
@Test
170+
void testLargestIdsAreRead() {
171+
Assertions.assertArrayEquals(new int[] {2147483647, 2147483647},
172+
ADMetadata.parseTextAndParagraph("2147483647 p=2147483647"));
173+
}
160174
}

‎opennlp-core/opennlp-formats/src/test/java/opennlp/tools/formats/ad/ADNameSampleStreamTest.java‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ private static Stream<Arguments> hyphenatedTokens() {
172172
Arguments.of("São-Paulo", new String[] {"São", "Paulo", ""}),
173173
// supplementary-plane letters are letters, a combining mark ends the letter run
174174
Arguments.of("\uD801\uDC12-\uD801\uDC3A", new String[] {"\uD801\uDC12", "\uD801\uDC3A", ""}),
175-
Arguments.of("e\u0301-a", new String[] {null, null, null}));
175+
// a next line character is not a letter, it goes into the rest
176+
Arguments.of("a-b\u0085c", new String[] {"a", "b", "\u0085c"}));
176177
}
177178

178179
@ParameterizedTest
@@ -188,6 +189,8 @@ void testMatchHyphenatedToken(String token, String[] expected) {
188189

189190
@ParameterizedTest
190191
@ValueSource(strings = {"-", "--", "-1", "1-", "a1-b", "a-1", "a--b", "ab", "a -",
192+
// a combining mark ends the letter run
193+
"e\u0301-a",
191194
// only the ASCII hyphen-minus splits; other dashes never do
192195
"guarda\u2011chuva", "guarda\u2013chuva", "guarda\u2014chuva"})
193196
void testMatchHyphenatedTokenRejects(String token) {
@@ -196,6 +199,7 @@ void testMatchHyphenatedTokenRejects(String token) {
196199

197200
@ParameterizedTest
198201
@CsvSource({"<NER:PROP>, PROP", "<PROP>, PROP", "<>, ''", "<NER:>, ''", "<a<b>, a<b",
202+
"<a\u0085b>, a\u0085b",
199203
"<NER:NER:X>, NER:X", "<ner:PROP>, ner:PROP", "<\uD83D\uDE00>, \uD83D\uDE00"})
200204
void testTagContent(String tag, String expected) {
201205
Assertions.assertEquals(expected, ADNameSampleStream.tagContent(tag));
@@ -250,4 +254,40 @@ void testInvalidMetadataIsRejected(String sentenceId, String source) throws IOEx
250254
Assertions.assertTrue(e.getMessage().startsWith("Invalid metadata: " + sentenceId + " p="));
251255
}
252256
}
257+
258+
private static List<String> sentenceLines(String leafLine) {
259+
return List.of("<s>", "SOURCE: ref=\"x\"", "1001 a casa .", "STA:fcl", leafLine,
260+
"=H:n(\"casa\" M S)\tcasa", ".", "</s>");
261+
}
262+
263+
@Test
264+
void testContractionLexemeOfUnderscoresOnlyHasNoLeftPart() throws IOException {
265+
List<String> lines = sentenceLines("==H:prp(\"em\" <sam-> <right>)\t_");
266+
try (ADNameSampleStream stream = new ADNameSampleStream(lineStream(lines), false)) {
267+
NameSample sample = stream.read();
268+
Assertions.assertNotNull(sample);
269+
Assertions.assertArrayEquals(new String[] {"casa", "."}, sample.getSentence());
270+
}
271+
}
272+
273+
@Test
274+
void testLeadingUnderscoreYieldsNoToken() throws IOException {
275+
List<String> lines = sentenceLines("=H:n(\"a\" M S)\t_a");
276+
try (ADNameSampleStream stream = new ADNameSampleStream(lineStream(lines), false)) {
277+
NameSample sample = stream.read();
278+
Assertions.assertNotNull(sample);
279+
Assertions.assertArrayEquals(new String[] {"a", "casa", "."}, sample.getSentence());
280+
}
281+
}
282+
283+
@Test
284+
void testNoBreakSpaceSeparatesTheTagsOfALeaf() throws IOException {
285+
List<String> lines = sentenceLines("=H:prop(\"Lisboa\"\u00A0<NER:civ>\u00A0F S)\tLisboa");
286+
try (ADNameSampleStream stream = new ADNameSampleStream(lineStream(lines), false)) {
287+
NameSample sample = stream.read();
288+
Assertions.assertNotNull(sample);
289+
Assertions.assertArrayEquals(new String[] {"Lisboa", "casa", "."}, sample.getSentence());
290+
Assertions.assertArrayEquals(new Span[] {new Span(0, 1, "place")}, sample.getNames());
291+
}
292+
}
253293
}

‎opennlp-core/opennlp-formats/src/test/java/opennlp/tools/formats/ad/ADSentenceSampleStreamTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,27 @@ public String read() {
8686
}
8787
}
8888

89+
@Test
90+
void testNextLineCharacterInTheSourceIsMetadata() throws IOException {
91+
List<String> lines = List.of(
92+
"<s>",
93+
"SOURCE: ref=\"a\u0085b\"",
94+
"1001 Hello world .",
95+
"STA:fcl",
96+
"=H:n(\"world\" M S)\tworld",
97+
"</s>");
98+
Iterator<String> iterator = lines.iterator();
99+
ObjectStream<String> lineStream = new ObjectStream<>() {
100+
@Override
101+
public String read() {
102+
return iterator.hasNext() ? iterator.next() : null;
103+
}
104+
};
105+
try (ADSentenceSampleStream stream = new ADSentenceSampleStream(lineStream, true)) {
106+
SentenceSample sample = stream.read();
107+
Assertions.assertNotNull(sample);
108+
Assertions.assertEquals("Hello world .", sample.getDocument());
109+
Assertions.assertNull(stream.read());
110+
}
111+
}
89112
}

‎opennlp-core/opennlp-formats/src/test/java/opennlp/tools/formats/ad/ADSentenceStreamTest.java‎

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.List;
2626
import java.util.stream.Stream;
2727

28+
import org.junit.jupiter.api.AfterEach;
2829
import org.junit.jupiter.api.Assertions;
2930
import org.junit.jupiter.api.Test;
3031
import org.junit.jupiter.params.ParameterizedTest;
@@ -40,6 +41,7 @@
4041
import opennlp.tools.formats.ad.ADSentenceStream.SentenceParser.TreeElement;
4142
import opennlp.tools.util.ObjectStream;
4243
import opennlp.tools.util.PlainTextByLineStream;
44+
import opennlp.tools.util.WhitespaceMode;
4345

4446
public class ADSentenceStreamTest {
4547

@@ -354,7 +356,15 @@ void testFixPunctuation() {
354356
"<s id=\"<\">|s|true",
355357
"<s<|s|false",
356358
"<<s>>|s|false",
357-
"<s/>|s|false"
359+
// a self-closing tag is not a tag
360+
"<s/>|s|false",
361+
"<p/>|p|false",
362+
"<t/>|t|false",
363+
"<caixa/>|caixa|false",
364+
"<ext/>|ext|false",
365+
// next line is Unicode whitespace, file separator is not
366+
"<s\u0085id=\"1\">|s|true",
367+
"<s\u001Cid=\"1\">|s|false"
358368
})
359369
void testIsOpeningTag(String line, String name, boolean expected) {
360370
Assertions.assertEquals(expected, ADSentenceStream.isOpeningTag(line, name));
@@ -597,4 +607,87 @@ void testUtf8BytesReadAsLatin1() throws IOException {
597607
Assertions.assertEquals(ola + " mundo .", sentences.get(0).text());
598608
Assertions.assertEquals(List.of(ola, "mundo", "."), lexemes(sentences.get(0)));
599609
}
610+
611+
@AfterEach
612+
void resetWhitespaceMode() {
613+
WhitespaceMode.reset();
614+
}
615+
616+
private static Stream<Arguments> whitespaceLinesInEveryMode() {
617+
List<Arguments> rows = new ArrayList<>();
618+
for (WhitespaceMode mode : WhitespaceMode.values()) {
619+
// next line, no-break space, and line separator are Unicode whitespace, so they separate
620+
// the closing parenthesis from the lexeme and end a tag
621+
rows.add(Arguments.of(mode, "=H:n(\"casa\" F S)\u0085casa", true, "H", "casa"));
622+
rows.add(Arguments.of(mode, "=H:n(\"casa\" F S)\u00A0casa", true, "H", "casa"));
623+
rows.add(Arguments.of(mode, "=H:n(\"casa\" F S)\u2028casa", true, "H", "casa"));
624+
rows.add(Arguments.of(mode, "=S:np\u2028", false, "S:np", null));
625+
rows.add(Arguments.of(mode, "=S:np\u00A0", false, "S:np", null));
626+
rows.add(Arguments.of(mode, "==PU:pu(\"\u0085\" PU)\t\u0085", false, "PU:pu", null));
627+
// whitespace inside a tag ends it, and the rest of the line is not a leaf or node tail
628+
rows.add(Arguments.of(mode, "=H:n\u00A0(\"o\" M S)\tx", true, "",
629+
"H:n\u00A0(\"o\" M S)\tx"));
630+
rows.add(Arguments.of(mode, "=S:n\u2028p", true, "", "S:n\u2028p"));
631+
// the file separator is not Unicode whitespace
632+
rows.add(Arguments.of(mode, "=H:n(\"casa\" F S)\u001Ccasa", true, "",
633+
"H:n(\"casa\" F S)\u001Ccasa"));
634+
rows.add(Arguments.of(mode, "=S:np\u001C", true, "", "S:np\u001C"));
635+
}
636+
return rows.stream();
637+
}
638+
639+
@ParameterizedTest(name = "{0}: {1}")
640+
@MethodSource("whitespaceLinesInEveryMode")
641+
void testWhitespaceIsTheUnicodeDefinitionInEveryMode(WhitespaceMode mode, String line,
642+
boolean isLeaf, String syntacticTag, String lexeme) {
643+
WhitespaceMode.setActive(mode);
644+
TreeElement element = new SentenceParser().getElement(line);
645+
Assertions.assertNotNull(element);
646+
Assertions.assertEquals(isLeaf, element.isLeaf());
647+
Assertions.assertEquals(syntacticTag, element.getSyntacticTag());
648+
if (isLeaf) {
649+
Assertions.assertEquals(lexeme, ((Leaf) element).getLexeme());
650+
}
651+
}
652+
653+
@ParameterizedTest
654+
@MethodSource("whitespaceModes")
655+
void testOpeningTagWhitespaceIsTheUnicodeDefinitionInEveryMode(WhitespaceMode mode) {
656+
WhitespaceMode.setActive(mode);
657+
Assertions.assertTrue(ADSentenceStream.isOpeningTag("<s\u0085id=\"1\">", "s"));
658+
Assertions.assertTrue(ADSentenceStream.isOpeningTag("<s\u00A0id=\"1\">", "s"));
659+
Assertions.assertFalse(ADSentenceStream.isOpeningTag("<s\u001Cid=\"1\">", "s"));
660+
}
661+
662+
private static Stream<WhitespaceMode> whitespaceModes() {
663+
return Stream.of(WhitespaceMode.values());
664+
}
665+
666+
@ParameterizedTest
667+
@ValueSource(strings = {"=a=", "=ab=", "==a=", "=x=y(a) ="})
668+
void testFallbackLineWithNoTextAfterTheLastEqualsSignIsSkipped(String line) {
669+
Assertions.assertNull(new SentenceParser().getElement(line));
670+
}
671+
672+
@Test
673+
void testNextLineCharacterInALexemeIsWhitespaceThroughTheStream() throws IOException {
674+
List<String> lines = List.of(
675+
"<s id=\"1\">",
676+
"SOURCE: src",
677+
"CF1001-1 Ol\u00E1 \u0085 .",
678+
"STA:fcl",
679+
"=P:v-fin(\"ol\u00E1\" PR 3S IND VFIN)\tOl\u00E1",
680+
"=PU:pu(\"\u0085\" PU)\t\u0085",
681+
"=.",
682+
"</s>");
683+
List<Sentence> sentences = readSentences(String.join("\n", lines) + "\n");
684+
Assertions.assertEquals(1, sentences.size());
685+
TreeElement[] elements = ((Node) sentences.get(0).root().getElements()[0]).getElements();
686+
Assertions.assertEquals(3, elements.length);
687+
Assertions.assertEquals("Ol\u00E1", ((Leaf) elements[0]).getLexeme());
688+
Assertions.assertFalse(elements[1].isLeaf());
689+
Assertions.assertEquals("PU:pu", elements[1].getSyntacticTag());
690+
Assertions.assertEquals(0, ((Node) elements[1]).getElements().length);
691+
Assertions.assertEquals(".", ((Leaf) elements[2]).getLexeme());
692+
}
600693
}

0 commit comments

Comments
 (0)