You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@uima.apache.org by re...@apache.org on 2020/11/03 19:44:13 UTC
[uima-uimaj] 01/01: [UIMA-6289] SelectFS.coveredBy not selecting
annotations with exact same bounds as annotation argument
This is an automated email from the ASF dual-hosted git repository.
rec pushed a commit to branch bugfix/UIMA-6289-SelectFS-coveredBy-not-selecting-annotations-with-exact-same-bounds-as-annotation-argument
in repository https://gitbox.apache.org/repos/asf/uima-uimaj.git
commit fff5c8e7f4a497505394fa31525d1aa9d0b51507
Author: Richard Eckart de Castilho <re...@apache.org>
AuthorDate: Tue Nov 3 20:44:01 2020 +0100
[UIMA-6289] SelectFS.coveredBy not selecting annotations with exact same bounds as annotation argument
- Fixed issue that relevant annotations might be skipped if they occur before the boundary annotation in the index
- Fixed issue that the boundary annotation might be in the result list although it shouldn't be included
- Fixed issue that an unchecked bounds check may trigger a NoSuchElementException because the iterator has become invalid
- Added several test cases derived from the case provided with the original issue report - in particular without using any of uimaFIT
---
.../java/org/apache/uima/cas/impl/Subiterator.java | 59 +++++----
.../SelectFsCoveredByWithTypePrioritiesTest.java | 141 +++++++++++++++++++++
2 files changed, 173 insertions(+), 27 deletions(-)
diff --git a/uimaj-core/src/main/java/org/apache/uima/cas/impl/Subiterator.java b/uimaj-core/src/main/java/org/apache/uima/cas/impl/Subiterator.java
index 6969ed3..f5bd070 100644
--- a/uimaj-core/src/main/java/org/apache/uima/cas/impl/Subiterator.java
+++ b/uimaj-core/src/main/java/org/apache/uima/cas/impl/Subiterator.java
@@ -447,7 +447,13 @@ public class Subiterator<T extends AnnotationFS> implements LowLevelIterator<T>
case coveredBy:
it.moveToNoReinit(boundingAnnot);
- if (!isStrict) {
+ // If the iterator is unambiguous, we must respect the index order. It really depends on the
+ // exact entry point into the index that the bounding annotation gives us which annotations
+ // will be returned and which are skipped. But if the iterator is unambiguous and if we also
+ // ignore the type priorities, then we need to seek backwards in the index as to not skip any
+ // potentially relevant annotations at the same position as the bounding annotation but
+ // (randomly) appearing before the bounding annotation in the index.
+ if (!isUnambiguous && !isUseTypePriority) {
// If the bounding annotation evaluates to being "greater" than any of the annotation in the
// index according to the index order, then the iterator comes back invalid.
if (!it.isValid()) {
@@ -473,14 +479,14 @@ public class Subiterator<T extends AnnotationFS> implements LowLevelIterator<T>
}
}
- // if an annotation is present (found), position is on it, and if not,
- // position is at the next annotation that is higher than (or invalid, if there is none)
- // note that the next found position could be beyond the end.
+ // If an annotation is present (found), position is on it, and if not,
+ // position is at the next annotation that is higher than (or invalid, if there
+ // is none). Note that the next found position could be beyond the end.
if (it.isValid()) {
// skip over bounding annotation
while (equalToBounds(it.getNvc())) {
it.moveToNextNvc();
- if (! it.isValid()) {
+ if (!it.isValid()) {
return;
}
}
@@ -568,7 +574,7 @@ public class Subiterator<T extends AnnotationFS> implements LowLevelIterator<T>
while (equalToBounds(it.getNvc())) {
it.moveToNextNvc();
moved = true;
- if ( ! it.isValid()) {
+ if (!it.isValid()) {
break;
}
}
@@ -577,7 +583,9 @@ public class Subiterator<T extends AnnotationFS> implements LowLevelIterator<T>
adjustForStrictNvc_forward();
}
}
- is_beyond_bounds_chk_coveredByNvc(); // is beyond end iff annot.begin > boundEnd
+ if (it.isValid()) {
+ is_beyond_bounds_chk_coveredByNvc(); // is beyond end iff annot.begin > boundEnd
+ }
return;
} // else is invalid
return;
@@ -1091,30 +1099,27 @@ public class Subiterator<T extends AnnotationFS> implements LowLevelIterator<T>
/**
- *
- * @param forward
* @return true if iterator still valid, false if not valid
*/
- private boolean adjustForStrictNvc_forward() {
- if (isStrict) {
- Annotation item = it.getNvc();
- while (item.getEnd() > this.boundEnd) {
-
- it.moveToNextNvc();
- if (!isValid()) {
- return false;
- }
- item = it.getNvc();
- if (item.getBegin() > this.boundEnd) { // not >= because could of 0 length annot at end
- makeInvalid();
- return false;
- }
-
- }
- return true;
- } else {
+ private boolean adjustForStrictNvc_forward() {
+ if (!isStrict) {
return true;
}
+
+ Annotation item = it.getNvc();
+ while (item.getEnd() > this.boundEnd || equalToBounds(item)) {
+ it.moveToNextNvc();
+ if (!isValid()) {
+ return false;
+ }
+
+ item = it.getNvc();
+ if (item.getBegin() > this.boundEnd) { // not >= because could of 0 length annot at end
+ makeInvalid();
+ return false;
+ }
+ }
+ return true;
}
/**
diff --git a/uimaj-core/src/test/java/org/apache/uima/cas/impl/SelectFsCoveredByWithTypePrioritiesTest.java b/uimaj-core/src/test/java/org/apache/uima/cas/impl/SelectFsCoveredByWithTypePrioritiesTest.java
new file mode 100644
index 0000000..852e6cc
--- /dev/null
+++ b/uimaj-core/src/test/java/org/apache/uima/cas/impl/SelectFsCoveredByWithTypePrioritiesTest.java
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.uima.cas.impl;
+
+import static java.util.Arrays.asList;
+import static org.apache.uima.UIMAFramework.getResourceSpecifierFactory;
+import static org.apache.uima.util.CasCreationUtils.createCas;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.uima.cas.CAS;
+import org.apache.uima.cas.Type;
+import org.apache.uima.jcas.JCas;
+import org.apache.uima.jcas.tcas.Annotation;
+import org.apache.uima.resource.metadata.TypePriorities;
+import org.apache.uima.resource.metadata.TypePriorityList;
+import org.apache.uima.resource.metadata.TypeSystemDescription;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Includes several additional tests for {@code coverdBy} selection which require the CAS to be
+ * prepared differently than in the {@link SelectFsTest}, e.g. with type priorities.
+ */
+public class SelectFsCoveredByWithTypePrioritiesTest {
+
+ private static final String TYPE_NAME_SUBTYPE = "uima.test.selectfs.SubType";
+
+ private static CAS cas;
+ private static Type typeSubType;
+
+ @BeforeClass
+ public static void setupClass() throws Exception
+ {
+ TypeSystemDescription tsd = getResourceSpecifierFactory().createTypeSystemDescription();
+ tsd.addType(TYPE_NAME_SUBTYPE, "", CAS.TYPE_NAME_ANNOTATION);
+
+ TypePriorities prios = getResourceSpecifierFactory().createTypePriorities();
+ TypePriorityList typePrioList = prios.addPriorityList();
+ typePrioList.addType(TYPE_NAME_SUBTYPE);
+ typePrioList.addType(CAS.TYPE_NAME_ANNOTATION);
+
+ cas = createCas(tsd, prios, null, null, null);
+ typeSubType = cas.getTypeSystem().getType(TYPE_NAME_SUBTYPE);
+ }
+
+ @Before
+ public void setup()
+ {
+ cas.reset();
+ }
+
+ @Test
+ public void thatCoveredByFindsTypeUsingSubtype() throws Exception {
+
+ Annotation a1 = (Annotation) cas.createAnnotation(cas.getAnnotationType(), 5, 10);
+ Annotation subType = (Annotation) cas.createAnnotation(typeSubType, 5, 10);
+
+ asList(a1, subType)
+ .forEach(a -> cas.addFsToIndexes(a));
+
+ assertThat(cas.select(Annotation.class).coveredBy(subType).asList())
+ .containsExactly(a1);
+ }
+
+ @Test
+ public void thatCoveredByFindsTypeUsingUnindexedSubtype() throws Exception {
+ Annotation a1 = (Annotation) cas.createAnnotation(cas.getAnnotationType(), 5, 10);
+ Annotation subType = (Annotation) cas.createAnnotation(typeSubType, 5, 10);
+
+ asList(a1)
+ .forEach(a -> cas.addFsToIndexes(a));
+
+ assertThat(cas.select(Annotation.class).coveredBy(subType).asList())
+ .containsExactly(a1);
+ }
+
+ @Test
+ public void thatCoveredByFindsSubtypeUsingType() throws Exception {
+ Annotation a1 = (Annotation) cas.createAnnotation(cas.getAnnotationType(), 5, 10);
+ Annotation subType = (Annotation) cas.createAnnotation(typeSubType, 5, 10);
+
+ asList(a1, subType)
+ .forEach(a -> cas.addFsToIndexes(a));
+
+ assertThat(cas.select(Annotation.class).coveredBy(a1).asList())
+ .containsExactly(subType);
+ }
+
+ @Test
+ public void thatCoveredByWorksWithOffsets() throws Exception {
+ Annotation a1 = (Annotation) cas.createAnnotation(cas.getAnnotationType(), 5, 10);
+
+ asList(a1)
+ .forEach(a -> cas.addFsToIndexes(a));
+
+ assertThat(cas.select(Annotation.class).coveredBy(5, 10).asList())
+ .containsExactly(a1);
+ }
+
+ @Test
+ public void thatCoveredBySkipsIndexedAnchorAnnotation() throws Exception {
+ JCas jCas = cas.getJCas();
+
+ Annotation a1 = new Annotation(jCas, 5, 10);
+ Annotation a2 = new Annotation(jCas, 5, 15);
+ Annotation a3 = new Annotation(jCas, 0, 10);
+ Annotation a4 = new Annotation(jCas, 0, 15);
+ Annotation a5 = new Annotation(jCas, 5, 7);
+ Annotation a6 = new Annotation(jCas, 8, 10);
+ Annotation a7 = new Annotation(jCas, 6, 9);
+ Annotation a8 = new Annotation(jCas, 5, 10);
+ asList(a1, a2, a3, a4, a5, a6, a7, a8).forEach(Annotation::addToIndexes);
+
+ assertThat(jCas.select(Annotation.class).coveredBy(a1).asList())
+ .containsExactly(a8, a5, a7, a6);
+
+ Annotation subType = (Annotation) cas.createAnnotation(typeSubType, 5, 10);
+ subType.addToIndexes();
+
+ assertThat(jCas.select(Annotation.class).coveredBy(subType).asList())
+ .containsExactly(a1, a8, a5, a7, a6);
+ }
+}
+