#4558 - Better verification for feature names

- Added limited testing
inception-project · Feb 26, 2024 · 1c48595 · 1c48595
1 parent 4bc22f3
commit 1c48595
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 17 deletions.
diff --git a/.../src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/chain/ChainLayerSupport.java b/.../src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/chain/ChainLayerSupport.java
@@ -23,6 +23,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 import java.util.function.Supplier;
 
 import org.apache.uima.cas.CAS;
@@ -56,10 +57,10 @@ public class ChainLayerSupport
     extends LayerSupport_ImplBase<ChainAdapter, ChainLayerTraits>
     implements InitializingBean
 {
-    private static final String FEATURE_NAME_FIRST = "first";
-    private static final String FEATURE_NAME_NEXT = "next";
-    private static final String FEATURE_NAME_REFERENCE_RELATION = "referenceRelation";
-    private static final String FEATURE_NAME_REFERENCE = "referenceType";
+    public static final String FEATURE_NAME_FIRST = "first";
+    public static final String FEATURE_NAME_NEXT = "next";
+    public static final String FEATURE_NAME_REFERENCE_RELATION = "referenceRelation";
+    public static final String FEATURE_NAME_REFERENCE = "referenceType";
     private static final String TYPE_SUFFIX_LINK = "Link";
     private static final String TYPE_SUFFIX_CHAIN = "Chain";
 
@@ -179,8 +180,8 @@ public List<ValidationError> validateFeatureName(AnnotationFeature aFeature)
     {
         var name = aFeature.getName();
 
-        if (name.equals(ChainLayerSupport.FEATURE_NAME_FIRST)
-                || name.equals(ChainLayerSupport.FEATURE_NAME_NEXT)) {
+        if (Set.of(FEATURE_NAME_FIRST, FEATURE_NAME_NEXT, FEATURE_NAME_REFERENCE,
+                FEATURE_NAME_REFERENCE_RELATION).contains(name)) {
             return asList(new ValidationError("[" + name + "] is a reserved feature name on "
                     + "chain layers. Please use a different name for the feature."));
         }

diff --git a/...ain/java/de/tudarmstadt/ukp/inception/annotation/layer/relation/RelationLayerSupport.java b/...ain/java/de/tudarmstadt/ukp/inception/annotation/layer/relation/RelationLayerSupport.java
@@ -17,8 +17,6 @@
  */
 package de.tudarmstadt.ukp.inception.annotation.layer.relation;
 
-import static de.tudarmstadt.ukp.inception.support.WebAnnoConst.FEAT_REL_SOURCE;
-import static de.tudarmstadt.ukp.inception.support.WebAnnoConst.FEAT_REL_TARGET;
 import static java.util.Arrays.asList;
 import static java.util.stream.Collectors.toList;
 import static org.apache.uima.cas.CAS.TYPE_NAME_ANNOTATION;
@@ -56,6 +54,12 @@ public class RelationLayerSupport
     extends LayerSupport_ImplBase<RelationAdapter, RelationLayerTraits>
     implements InitializingBean
 {
+    @SuppressWarnings("deprecation")
+    public static final String FEAT_REL_TARGET = WebAnnoConst.FEAT_REL_TARGET;
+
+    @SuppressWarnings("deprecation")
+    public static final String FEAT_REL_SOURCE = WebAnnoConst.FEAT_REL_SOURCE;
+
     @SuppressWarnings("deprecation")
     public static final String TYPE = WebAnnoConst.RELATION_TYPE;
 

diff --git a/...-schema-api/src/main/java/de/tudarmstadt/ukp/inception/schema/api/layer/LayerSupport.java b/...-schema-api/src/main/java/de/tudarmstadt/ukp/inception/schema/api/layer/LayerSupport.java
@@ -20,6 +20,7 @@
 import static java.util.Arrays.asList;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Supplier;
@@ -176,5 +177,8 @@ default IllegalArgumentException unsupportedLayerTypeException(AnnotationLayer a
 
     LayerSupportRegistry getLayerSupportRegistry();
 
-    List<ValidationError> validateFeatureName(AnnotationFeature aFeature);
+    default List<ValidationError> validateFeatureName(AnnotationFeature aFeature)
+    {
+        return Collections.emptyList();
+    }
 }
diff --git a/inception/inception-schema/pom.xml b/inception/inception-schema/pom.xml
@@ -113,6 +113,11 @@
       <groupId>org.springframework.boot</groupId>
       <artifactId>spring-boot</artifactId>
     </dependency>
+
+    <dependency>
+      <groupId>org.apache.wicket</groupId>
+      <artifactId>wicket-core</artifactId>
+    </dependency>
 
     <!-- Hibernate dependencies -->
     <dependency>

diff --git a/...est/java/de/tudarmstadt/ukp/inception/schema/service/AnnotationSchemaServiceImplTest.java b/...est/java/de/tudarmstadt/ukp/inception/schema/service/AnnotationSchemaServiceImplTest.java
@@ -18,19 +18,19 @@
 package de.tudarmstadt.ukp.inception.schema.service;
 
 import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.ANNOTATOR;
+import static de.tudarmstadt.ukp.inception.annotation.layer.chain.ChainLayerSupport.FEATURE_NAME_FIRST;
+import static de.tudarmstadt.ukp.inception.support.WebAnnoConst.FEAT_REL_SOURCE;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 import java.io.File;
 import java.util.LinkedHashSet;
-import java.util.Set;
 
 import org.apache.uima.UIMAFramework;
 import org.apache.uima.cas.FeatureStructure;
 import org.apache.uima.cas.impl.CASImpl;
-import org.apache.uima.jcas.tcas.Annotation;
-import org.apache.uima.util.AutoCloseableNoException;
 import org.apache.uima.util.CasCreationUtils;
+import org.apache.wicket.validation.ValidationError;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -46,12 +46,16 @@
 
 import de.tudarmstadt.ukp.clarin.webanno.api.export.DocumentImportExportService;
 import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationFeature;
+import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationLayer;
 import de.tudarmstadt.ukp.clarin.webanno.model.Project;
 import de.tudarmstadt.ukp.clarin.webanno.model.TagSet;
 import de.tudarmstadt.ukp.clarin.webanno.project.config.ProjectServiceAutoConfiguration;
 import de.tudarmstadt.ukp.clarin.webanno.security.UserDao;
 import de.tudarmstadt.ukp.clarin.webanno.security.config.SecurityAutoConfiguration;
 import de.tudarmstadt.ukp.clarin.webanno.security.model.User;
+import de.tudarmstadt.ukp.inception.annotation.layer.chain.ChainLayerSupport;
+import de.tudarmstadt.ukp.inception.annotation.layer.relation.RelationLayerSupport;
+import de.tudarmstadt.ukp.inception.annotation.layer.span.SpanLayerSupport;
 import de.tudarmstadt.ukp.inception.documents.api.DocumentService;
 import de.tudarmstadt.ukp.inception.documents.api.RepositoryAutoConfiguration;
 import de.tudarmstadt.ukp.inception.project.api.ProjectService;
@@ -141,13 +145,13 @@ void thatAddingOutOfTagsetTagOnOpenTagsetCreatesTag() throws Exception
     @Test
     void testCasUpgradePerformsGarbageCollection() throws Exception
     {
-        CASImpl cas = (CASImpl) CasCreationUtils.createCas();
-        try (AutoCloseableNoException a = cas.ll_enableV2IdRefs(true)) {
-            Annotation ann = cas.createAnnotation(cas.getAnnotationType(), 0, 1);
+        var cas = (CASImpl) CasCreationUtils.createCas();
+        try (var a = cas.ll_enableV2IdRefs(true)) {
+            var ann = cas.createAnnotation(cas.getAnnotationType(), 0, 1);
             ann.addToIndexes();
             ann.removeFromIndexes();
 
-            Set<FeatureStructure> allFSesBefore = new LinkedHashSet<>();
+            var allFSesBefore = new LinkedHashSet<FeatureStructure>();
             cas.walkReachablePlusFSsSorted(allFSesBefore::add, null, null, null);
 
             assertThat(allFSesBefore) //
@@ -157,7 +161,7 @@ void testCasUpgradePerformsGarbageCollection() throws Exception
             AnnotationSchemaServiceImpl._upgradeCas(cas, cas,
                     UIMAFramework.getResourceSpecifierFactory().createTypeSystemDescription());
 
-            Set<FeatureStructure> allFSesAfter = new LinkedHashSet<>();
+            var allFSesAfter = new LinkedHashSet<FeatureStructure>();
             cas.walkReachablePlusFSsSorted(allFSesAfter::add, null, null, null);
 
             assertThat(allFSesAfter) //
@@ -166,6 +170,38 @@ void testCasUpgradePerformsGarbageCollection() throws Exception
         }
     }
 
+    @Test
+    void testDocumentNameValidationErrorMessages()
+    {
+        var spanLayer = AnnotationLayer.builder().withType(SpanLayerSupport.TYPE).build();
+        var relationLayer = AnnotationLayer.builder().withType(RelationLayerSupport.TYPE).build();
+        var chainLayer = AnnotationLayer.builder().withType(ChainLayerSupport.TYPE).build();
+
+        assertThat(sut.validateFeatureName(
+                AnnotationFeature.builder().withName("").withLayer(spanLayer).build())) //
+                        .hasSize(1) //
+                        .extracting(ValidationError::getMessage).first().asString() //
+                        .contains("empty");
+
+        assertThat(sut.validateFeatureName(
+                AnnotationFeature.builder().withName(" ").withLayer(spanLayer).build())) //
+                        .hasSize(1) //
+                        .extracting(ValidationError::getMessage).first().asString() //
+                        .contains("empty");
+
+        assertThat(sut.validateFeatureName(AnnotationFeature.builder().withName(FEAT_REL_SOURCE)
+                .withLayer(relationLayer).build())) //
+                        .hasSize(1) //
+                        .extracting(ValidationError::getMessage).first().asString() //
+                        .contains("reserved feature name");
+
+        assertThat(sut.validateFeatureName(AnnotationFeature.builder().withName(FEATURE_NAME_FIRST)
+                .withLayer(chainLayer).build())) //
+                        .hasSize(1) //
+                        .extracting(ValidationError::getMessage).first().asString() //
+                        .contains("reserved feature name");
+    }
+
     @SpringBootConfiguration
     public static class TestContext
     {

diff --git a/...on/inception-support/src/main/java/de/tudarmstadt/ukp/inception/support/WebAnnoConst.java b/...on/inception-support/src/main/java/de/tudarmstadt/ukp/inception/support/WebAnnoConst.java
@@ -34,7 +34,16 @@ public class WebAnnoConst
     public static final String COREFRELTYPE = "coreference type";
     public static final String LEMMA = "lemma";
 
+    /**
+     * @deprecated Use {@code RelationLayerSupport.FEAT_REL_TARGET}
+     */
+    @Deprecated
     public static final String FEAT_REL_TARGET = "Dependent";
+
+    /**
+     * @deprecated Use {@code RelationLayerSupport.FEAT_REL_SOURCE}
+     */
+    @Deprecated
     public static final String FEAT_REL_SOURCE = "Governor";
 
     /**