#4558 - Better verification for feature names

- Factor layer verification code out of the UI layer into the backend

inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/chain/ChainLayerSupport.java

@@ -21,14 +21,15 @@
 
 import java.lang.invoke.MethodHandles;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.function.Supplier;
 
 import org.apache.uima.cas.CAS;
-import org.apache.uima.resource.metadata.TypeDescription;
 import org.apache.uima.resource.metadata.TypeSystemDescription;
 import org.apache.wicket.markup.html.panel.Panel;
 import org.apache.wicket.model.IModel;
+import org.apache.wicket.validation.ValidationError;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.InitializingBean;
@@ -55,6 +56,13 @@ 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";
+    private static final String TYPE_SUFFIX_LINK = "Link";
+    private static final String TYPE_SUFFIX_CHAIN = "Chain";
+
     private final static Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
     @SuppressWarnings("deprecation")
@@ -91,7 +99,7 @@ public void setBeanName(String aBeanName)
     @Override
     public void afterPropertiesSet() throws Exception
     {
-        types = asList(new LayerType(TYPE, "Chain", layerSupportId));
+        types = asList(new LayerType(TYPE, TYPE_SUFFIX_CHAIN, layerSupportId));
     }
 
     @Override
@@ -110,35 +118,33 @@ public boolean accepts(AnnotationLayer aLayer)
     public ChainAdapter createAdapter(AnnotationLayer aLayer,
             Supplier<Collection<AnnotationFeature>> aFeatures)
     {
-        ChainAdapter adapter = new ChainAdapter(getLayerSupportRegistry(), featureSupportRegistry,
-                eventPublisher, aLayer, aFeatures,
+        return new ChainAdapter(getLayerSupportRegistry(), featureSupportRegistry, eventPublisher,
+                aLayer, aFeatures,
                 layerBehaviorsRegistry.getLayerBehaviors(this, SpanLayerBehavior.class));
-
-        return adapter;
     }
 
     @Override
     public void generateTypes(TypeSystemDescription aTsd, AnnotationLayer aLayer,
             List<AnnotationFeature> aAllFeaturesInProject)
     {
-        TypeDescription tdChains = aTsd.addType(aLayer.getName() + "Chain", aLayer.getDescription(),
+        var tdChain = aTsd.addType(aLayer.getName() + TYPE_SUFFIX_CHAIN, aLayer.getDescription(),
                 CAS.TYPE_NAME_ANNOTATION_BASE);
-        tdChains.addFeature("first", "", aLayer.getName() + "Link");
+        tdChain.addFeature(FEATURE_NAME_FIRST, "", aLayer.getName() + TYPE_SUFFIX_LINK);
 
         // Custom features on chain layers are currently not supported
         // generateFeatures(aTsd, tdChains, type);
 
-        TypeDescription tdLink = aTsd.addType(aLayer.getName() + "Link", "",
+        var tdLink = aTsd.addType(aLayer.getName() + TYPE_SUFFIX_LINK, "",
                 CAS.TYPE_NAME_ANNOTATION);
-        tdLink.addFeature("next", "", aLayer.getName() + "Link");
-        tdLink.addFeature("referenceType", "", CAS.TYPE_NAME_STRING);
-        tdLink.addFeature("referenceRelation", "", CAS.TYPE_NAME_STRING);
+        tdLink.addFeature(FEATURE_NAME_NEXT, "", aLayer.getName() + TYPE_SUFFIX_LINK);
+        tdLink.addFeature(FEATURE_NAME_REFERENCE, "", CAS.TYPE_NAME_STRING);
+        tdLink.addFeature(FEATURE_NAME_REFERENCE_RELATION, "", CAS.TYPE_NAME_STRING);
     }
 
     @Override
     public List<String> getGeneratedTypeNames(AnnotationLayer aLayer)
     {
-        return asList(aLayer.getName() + "Chain", aLayer.getName() + "Link");
+        return asList(aLayer.getName() + TYPE_SUFFIX_CHAIN, aLayer.getName() + TYPE_SUFFIX_LINK);
     }
 
     @Override
@@ -153,7 +159,7 @@ public Renderer createRenderer(AnnotationLayer aLayer,
     @Override
     public Panel createTraitsEditor(String aId, IModel<AnnotationLayer> aLayerModel)
     {
-        AnnotationLayer layer = aLayerModel.getObject();
+        var layer = aLayerModel.getObject();
 
         if (!accepts(layer)) {
             throw unsupportedLayerTypeException(layer);
@@ -167,4 +173,18 @@ public ChainLayerTraits createTraits()
     {
         return new ChainLayerTraits();
     }
+
+    @Override
+    public List<ValidationError> validateFeatureName(AnnotationFeature aFeature)
+    {
+        var name = aFeature.getName();
+
+        if (name.equals(ChainLayerSupport.FEATURE_NAME_FIRST)
+                || name.equals(ChainLayerSupport.FEATURE_NAME_NEXT)) {
+            return asList(new ValidationError("[" + name + "] is a reserved feature name on "
+                    + "chain layers. Please use a different name for the feature."));
+        }
+
+        return Collections.emptyList();
+    }
 }

inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/relation/RelationLayerSupport.java

@@ -24,13 +24,15 @@
 import static org.apache.uima.cas.CAS.TYPE_NAME_ANNOTATION;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.function.Supplier;
 
 import org.apache.uima.resource.metadata.TypeDescription;
 import org.apache.uima.resource.metadata.TypeSystemDescription;
 import org.apache.wicket.markup.html.panel.Panel;
 import org.apache.wicket.model.IModel;
+import org.apache.wicket.validation.ValidationError;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationEventPublisher;
@@ -157,4 +159,16 @@ public RelationLayerTraits createTraits()
     {
         return new RelationLayerTraits();
     }
+
+    @Override
+    public List<ValidationError> validateFeatureName(AnnotationFeature aFeature)
+    {
+        var name = aFeature.getName();
+        if (name.equals(FEAT_REL_SOURCE) || name.equals(FEAT_REL_TARGET)) {
+            return asList(new ValidationError("[" + name + "] is a reserved feature name on "
+                    + "relation layers. Please use a different name for the feature."));
+        }
+
+        return Collections.emptyList();
+    }
 }

inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/span/SpanLayerSupport.java

@@ -102,11 +102,9 @@ public boolean accepts(AnnotationLayer aLayer)
     public SpanAdapter createAdapter(AnnotationLayer aLayer,
             Supplier<Collection<AnnotationFeature>> aFeatures)
     {
-        var adapter = new SpanAdapter(getLayerSupportRegistry(), featureSupportRegistry,
-                eventPublisher, aLayer, aFeatures,
+        return new SpanAdapter(getLayerSupportRegistry(), featureSupportRegistry, eventPublisher,
+                aLayer, aFeatures,
                 layerBehaviorsRegistry.getLayerBehaviors(this, SpanLayerBehavior.class));
-
-        return adapter;
     }
 
     @Override
@@ -134,7 +132,7 @@ public SpanRenderer createRenderer(AnnotationLayer aLayer,
     @Override
     public Panel createTraitsEditor(String aId, IModel<AnnotationLayer> aLayerModel)
     {
-        AnnotationLayer layer = aLayerModel.getObject();
+        var layer = aLayerModel.getObject();
 
         if (!accepts(layer)) {
             throw unsupportedLayerTypeException(layer);

inception/inception-layer-docmetadata/src/main/java/de/tudarmstadt/ukp/inception/ui/core/docanno/layer/DocumentMetadataLayerSupport.java

@@ -25,7 +25,6 @@
 import java.util.function.Supplier;
 
 import org.apache.uima.cas.CAS;
-import org.apache.uima.resource.metadata.TypeDescription;
 import org.apache.uima.resource.metadata.TypeSystemDescription;
 import org.apache.wicket.markup.html.panel.Panel;
 import org.apache.wicket.model.IModel;
@@ -107,20 +106,17 @@ public boolean accepts(AnnotationLayer aLayer)
     public DocumentMetadataLayerAdapter createAdapter(AnnotationLayer aLayer,
             Supplier<Collection<AnnotationFeature>> aFeatures)
     {
-        DocumentMetadataLayerAdapter adapter = new DocumentMetadataLayerAdapter(
-                getLayerSupportRegistry(), featureSupportRegistry, eventPublisher, aLayer,
-                aFeatures);
-
-        return adapter;
+        return new DocumentMetadataLayerAdapter(getLayerSupportRegistry(), featureSupportRegistry,
+                eventPublisher, aLayer, aFeatures);
     }
 
     @Override
     public void generateTypes(TypeSystemDescription aTsd, AnnotationLayer aLayer,
             List<AnnotationFeature> aAllFeaturesInProject)
     {
-        TypeDescription td = aTsd.addType(aLayer.getName(), "", CAS.TYPE_NAME_ANNOTATION_BASE);
+        var td = aTsd.addType(aLayer.getName(), "", CAS.TYPE_NAME_ANNOTATION_BASE);
 
-        List<AnnotationFeature> featureForLayer = aAllFeaturesInProject.stream()
+        var featureForLayer = aAllFeaturesInProject.stream()
                 .filter(feature -> aLayer.equals(feature.getLayer())).collect(toList());
         generateFeatures(aTsd, td, featureForLayer);
     }
@@ -136,7 +132,7 @@ public Renderer createRenderer(AnnotationLayer aLayer,
     @Override
     public Panel createTraitsEditor(String aId, IModel<AnnotationLayer> aLayerModel)
     {
-        AnnotationLayer layer = aLayerModel.getObject();
+        var layer = aLayerModel.getObject();
 
         if (!accepts(layer)) {
             throw unsupportedLayerTypeException(layer);

inception/inception-schema-api/src/main/java/de/tudarmstadt/ukp/inception/schema/api/AnnotationSchemaService.java

@@ -27,6 +27,7 @@
 import org.apache.uima.cas.text.AnnotationFS;
 import org.apache.uima.resource.ResourceInitializationException;
 import org.apache.uima.resource.metadata.TypeSystemDescription;
+import org.apache.wicket.validation.ValidationError;
 import org.springframework.security.access.prepost.PreAuthorize;
 
 import de.tudarmstadt.ukp.clarin.webanno.api.casstorage.CasUpgradeMode;
@@ -670,4 +671,8 @@ void importUimaTypeSystem(Project aProject, TypeSystemDescription aTSD)
 
     void createMissingTag(AnnotationFeature aFeature, String aValue)
         throws IllegalFeatureValueException;
+
+    List<ValidationError> validateFeatureName(AnnotationFeature aFeature);
+
+    boolean hasValidFeatureName(AnnotationFeature aFeature);
 }

inception/inception-schema-api/src/main/java/de/tudarmstadt/ukp/inception/schema/api/layer/LayerSupport.java

@@ -28,6 +28,7 @@
 import org.apache.wicket.markup.html.panel.EmptyPanel;
 import org.apache.wicket.markup.html.panel.Panel;
 import org.apache.wicket.model.IModel;
+import org.apache.wicket.validation.ValidationError;
 import org.springframework.beans.factory.BeanNameAware;
 
 import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationFeature;
@@ -174,4 +175,6 @@ default IllegalArgumentException unsupportedLayerTypeException(AnnotationLayer a
     void setLayerSupportRegistry(LayerSupportRegistry aLayerSupportRegistry);
 
     LayerSupportRegistry getLayerSupportRegistry();
+
+    List<ValidationError> validateFeatureName(AnnotationFeature aFeature);
 }

inception/inception-schema-api/src/main/java/de/tudarmstadt/ukp/inception/schema/api/layer/LayerSupport_ImplBase.java

@@ -50,7 +50,7 @@ public LayerSupport_ImplBase(FeatureSupportRegistry aFeatureSupportRegistry)
     public final void generateFeatures(TypeSystemDescription aTSD, TypeDescription aTD,
             List<AnnotationFeature> aFeatures)
     {
-        for (AnnotationFeature feature : aFeatures) {
+        for (var feature : aFeatures) {
             featureSupportRegistry.findExtension(feature)
                     .ifPresent(fs -> fs.generateFeature(aTSD, aTD, feature));
         }

inception/inception-schema/src/main/java/de/tudarmstadt/ukp/inception/schema/service/AnnotationSchemaServiceImpl.java

@@ -24,6 +24,7 @@
 import static de.tudarmstadt.ukp.inception.schema.api.AttachedAnnotation.Direction.LOOP;
 import static de.tudarmstadt.ukp.inception.schema.api.AttachedAnnotation.Direction.OUTGOING;
 import static de.tudarmstadt.ukp.inception.support.WebAnnoConst.RELATION_TYPE;
+import static de.tudarmstadt.ukp.inception.support.WebAnnoConst.RESTRICTED_FEATURE_NAMES;
 import static de.tudarmstadt.ukp.inception.support.uima.ICasUtil.selectByAddr;
 import static de.tudarmstadt.ukp.inception.support.uima.WebAnnoCasUtil.getRealCas;
 import static de.tudarmstadt.ukp.inception.support.uima.WebAnnoCasUtil.isNativeUimaType;
@@ -33,6 +34,9 @@
 import static java.util.Objects.isNull;
 import static java.util.concurrent.TimeUnit.MINUTES;
 import static java.util.stream.Collectors.toList;
+import static org.apache.commons.lang3.StringUtils.isAlphanumeric;
+import static org.apache.commons.lang3.StringUtils.isBlank;
+import static org.apache.commons.lang3.StringUtils.isNumeric;
 import static org.apache.uima.cas.impl.Serialization.deserializeCASComplete;
 import static org.apache.uima.cas.impl.Serialization.serializeCASComplete;
 import static org.apache.uima.cas.impl.Serialization.serializeWithCompression;
@@ -71,6 +75,7 @@
 import org.apache.uima.resource.metadata.impl.TypeSystemDescription_impl;
 import org.apache.uima.util.CasCreationUtils;
 import org.apache.uima.util.CasIOUtils;
+import org.apache.wicket.validation.ValidationError;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -1656,4 +1661,53 @@ public void createMissingTag(AnnotationFeature aFeature, String aValue)
         selectedTag.setTagSet(aFeature.getTagset());
         createTag(selectedTag);
     }
+
+    @Override
+    public boolean hasValidFeatureName(AnnotationFeature aFeature)
+    {
+        return validateFeatureName(aFeature).isEmpty();
+    }
+
+    @Override
+    public List<ValidationError> validateFeatureName(AnnotationFeature aFeature)
+    {
+        String name = aFeature.getName();
+
+        var errors = new ArrayList<ValidationError>();
+
+        if (isBlank(name)) {
+            errors.add(new ValidationError("Feature name cannot be empty."));
+            return errors;
+        }
+
+        // Check if feature name is not from the restricted names list
+        if (RESTRICTED_FEATURE_NAMES.contains(name)) {
+            errors.add(new ValidationError("[" + name + "] is a reserved feature name. Please "
+                    + "use a different name for the feature."));
+            return errors;
+        }
+
+        var layerSupport = layerSupportRegistry.getLayerSupport(aFeature.getLayer());
+        errors.addAll(layerSupport.validateFeatureName(aFeature));
+        if (!errors.isEmpty()) {
+            return errors;
+        }
+
+        // Checking if feature name doesn't start with a number or underscore
+        // And only uses alphanumeric characters
+        if (isNumeric(name.substring(0, 1)) || name.substring(0, 1).equals("_")
+                || !isAlphanumeric(name.replace("_", ""))) {
+            errors.add(new ValidationError("Feature names must start with a letter and consist "
+                    + "only of letters, digits, or underscores."));
+            return errors;
+        }
+
+        if (existsFeature(name, aFeature.getLayer())) {
+            errors.add(new ValidationError(
+                    "A feature with the name [" + name + "] already exists on this layer!"));
+            return errors;
+        }
+
+        return errors;
+    }
 }

inception/inception-ui-kb/src/main/java/de/tudarmstadt/ukp/inception/ui/kb/SubclassCreationDialog.java

@@ -113,10 +113,10 @@ protected void actionCreateSubclass(AjaxRequestTarget aTarget, Form<KBConcept> a
             }
 
             // create the new concept
-            KBConcept newConcept = newSubclassConceptModel.getObject();
+            var newConcept = newSubclassConceptModel.getObject();
             kbService.createConcept(kb, newConcept);
 
-            String parentConceptId = parentConceptHandleModel.getObject().getIdentifier();
+            var parentConceptId = parentConceptHandleModel.getObject().getIdentifier();
 
             // create the subclassof statement and add it to the knowledge base
             ValueFactory vf = SimpleValueFactory.getInstance();