#4942 - Clearing hidden feature does sometimes not work

- Cleaning up code
- Fixed issue
- Added more unit tests

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

@@ -19,6 +19,7 @@
 
 import static de.tudarmstadt.ukp.inception.support.uima.ICasUtil.selectFsByAddr;
 
+import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -32,6 +33,8 @@
 
 import org.apache.uima.cas.CAS;
 import org.apache.uima.cas.FeatureStructure;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.context.ApplicationEventPublisher;
 
@@ -51,6 +54,8 @@
 public abstract class TypeAdapter_ImplBase
     implements TypeAdapter
 {
+    private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
     private final LayerSupportRegistry layerSupportRegistry;
     private final FeatureSupportRegistry featureSupportRegistry;
     private final ConstraintsService constraintsService;
@@ -178,6 +183,8 @@ public final void setFeatureValue(SourceDocument aDocument, String aUsername, CA
     private void clearHiddenFeatures(SourceDocument aDocument, String aUsername,
             FeatureStructure aFS)
     {
+        LOG.trace("begin clear hidden");
+
         var constraints = constraintsService.getMergedConstraints(aDocument.getProject());
         if (constraints == null) {
             return;
@@ -200,6 +207,8 @@ private void clearHiddenFeatures(SourceDocument aDocument, String aUsername,
                 }
             }
         }
+
+        LOG.trace("end clear hidden");
     }
 
     @Override

inception/inception-constraints/src/main/java/de/tudarmstadt/ukp/clarin/webanno/constraints/evaluator/ConstraintsEvaluator.java

@@ -61,15 +61,15 @@ List<PossibleValue> generatePossibleValues(ParsedConstraints aConstraints,
             FeatureStructure aContext, String aFeature)
         throws UIMAException
     {
-        if (!aConstraints.areThereRules(aContext.getType().getName(), aFeature)) {
+        if (!aConstraints.isPathUsedInAnyRestriction(aContext, aFeature)) {
             return Collections.emptyList();
         }
 
         var possibleValues = new ArrayList<PossibleValue>();
 
         var scope = getScope(aConstraints, aContext);
         for (var rule : scope.getRules()) {
-            if (!ruleTriggers(aConstraints, aContext, rule)) {
+            if (!allRuleConditionsMatch(aConstraints, rule, aContext)) {
                 continue;
             }
 
@@ -84,12 +84,12 @@ List<PossibleValue> generatePossibleValues(ParsedConstraints aConstraints,
         return possibleValues;
     }
 
-    public boolean isRestrictionTriggered(ParsedConstraints aConstraints, FeatureStructure aContext,
-            AnnotationFeature aFeature)
+    public boolean anyRuleAffectingFeatureMatchesAllConditions(ParsedConstraints aConstraints,
+            FeatureStructure aContext, AnnotationFeature aFeature)
     {
         var feature = aFeature.getName();
 
-        if (!aConstraints.areThereRules(aContext.getType().getName(), feature)) {
+        if (!aConstraints.isPathUsedInAnyRestriction(aContext, feature)) {
             return false;
         }
 
@@ -99,8 +99,8 @@ public boolean isRestrictionTriggered(ParsedConstraints aConstraints, FeatureStr
         }
 
         for (var rule : scope.getRules()) {
-            if (ruleTriggers(aConstraints, aContext, rule)) {
-                return true;
+            if (allRuleConditionsMatch(aConstraints, rule, aContext)) {
+                return anyRestrictionAffectsFeature(aConstraints, rule, aFeature);
             }
         }
 
@@ -112,7 +112,7 @@ public boolean isValidFeatureValue(ParsedConstraints aConstraints, FeatureStruct
     {
         var feature = aFeature.getName();
 
-        if (!aConstraints.areThereRules(aContext.getType().getName(), feature)) {
+        if (!aConstraints.isPathUsedInAnyRestriction(aContext, feature)) {
             return true;
         }
 
@@ -124,7 +124,7 @@ public boolean isValidFeatureValue(ParsedConstraints aConstraints, FeatureStruct
         var actualFeatureValue = FSUtil.getFeature(aContext, feature, String.class);
 
         for (var rule : scope.getRules()) {
-            if (!ruleTriggers(aConstraints, aContext, rule)) {
+            if (!allRuleConditionsMatch(aConstraints, rule, aContext)) {
                 continue;
             }
 
@@ -150,20 +150,20 @@ public boolean isHiddenConditionalFeature(ParsedConstraints aConstraints,
             return false;
         }
 
-        if (!isAffectedByConstraints(aConstraints, aContext, aFeature)) {
+        if (!isPathUsedInAnyRestriction(aConstraints, aContext, aFeature)) {
             return false;
         }
 
-        return !isRestrictionTriggered(aConstraints, aContext, aFeature);
+        return !anyRuleAffectingFeatureMatchesAllConditions(aConstraints, aContext, aFeature);
     }
 
     @Override
-    public boolean isAffectedByConstraints(ParsedConstraints aConstraints,
+    public boolean isPathUsedInAnyRestriction(ParsedConstraints aConstraints,
             FeatureStructure aContext, AnnotationFeature aFeature)
     {
         var restrictionFeaturePath = getRestrictionPathForFeature(aFeature);
 
-        return aConstraints.areThereRules(aContext.getType().getName(), restrictionFeaturePath);
+        return aConstraints.isPathUsedInAnyRestriction(aContext, restrictionFeaturePath);
     }
 
     private Scope getScope(ParsedConstraints aConstraints, FeatureStructure aContext)
@@ -176,8 +176,17 @@ private Scope getScope(ParsedConstraints aConstraints, FeatureStructure aContext
         return aConstraints.getScopeByName(shortTypeName);
     }
 
-    private boolean ruleTriggers(ParsedConstraints aConstraints, FeatureStructure aContext,
-            Rule aRule)
+    private boolean anyRestrictionAffectsFeature(ParsedConstraints aConstraints, Rule aRule,
+            AnnotationFeature aFeature)
+    {
+        var restrictionFeaturePath = getRestrictionPathForFeature(aFeature);
+
+        return aRule.getRestrictions().stream()
+                .anyMatch(restriction -> restrictionFeaturePath.equals(restriction.getPath()));
+    }
+
+    private boolean allRuleConditionsMatch(ParsedConstraints aConstraints, Rule aRule,
+            FeatureStructure aContext)
     {
         return aRule.getConditions().stream()
                 .allMatch(condition -> conditionMatches(aConstraints, aContext, condition));
@@ -186,13 +195,15 @@ private boolean ruleTriggers(ParsedConstraints aConstraints, FeatureStructure aC
     private boolean conditionMatches(ParsedConstraints aConstraints, FeatureStructure aContext,
             Condition aCondition)
     {
-        var value = getValue(aConstraints, aContext, aCondition.getPath());
+        var values = getValue(aConstraints, aContext, aCondition.getPath());
+        var result = aCondition.matchesAny(values);
 
         if (LOG.isTraceEnabled()) {
-            LOG.trace("comparing [{}] to [{}]", aCondition.getValue(), value);
+            LOG.trace("comparing [{}]/@{}[{} == {}] -> {}", aContext.getType().getName(),
+                    aCondition.getPath(), aCondition.getValue(), values, result);
         }
 
-        return aCondition.matches(value);
+        return result;
     }
 
     private List<String> getValue(ParsedConstraints aConstraints, FeatureStructure aContext,

inception/inception-constraints/src/main/java/de/tudarmstadt/ukp/clarin/webanno/constraints/evaluator/Evaluator.java

@@ -68,7 +68,7 @@ List<PossibleValue> generatePossibleValues(ParsedConstraints aConstraints,
      * 
      * @return true if features can be affected by this execution
      */
-    boolean isAffectedByConstraints(ParsedConstraints aConstraints, FeatureStructure aContext,
+    boolean isPathUsedInAnyRestriction(ParsedConstraints aConstraints, FeatureStructure aContext,
             AnnotationFeature aFeature);
 
     boolean isHiddenConditionalFeature(ParsedConstraints aConstraints, FeatureStructure aContext,

inception/inception-constraints/src/main/java/de/tudarmstadt/ukp/clarin/webanno/constraints/model/Condition.java

@@ -67,7 +67,7 @@ public String toString()
         return "Condition [[" + path + "] = [" + value + "]]";
     }
 
-    public boolean matches(List<String> listOfValues)
+    public boolean matchesAny(List<String> listOfValues)
     {
         return listOfValues.contains(value);
     }

inception/inception-constraints/src/main/java/de/tudarmstadt/ukp/clarin/webanno/constraints/model/ParsedConstraints.java

@@ -29,11 +29,10 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.uima.cas.FeatureStructure;
+
 import de.tudarmstadt.ukp.clarin.webanno.constraints.grammar.ASTConstraintsSet;
 
-/**
- * Serialized Class containing objects after parsing and creating objects based on rules file.
- */
 public class ParsedConstraints
     implements Serializable
 {
@@ -43,7 +42,7 @@ public class ParsedConstraints
     private final List<Scope> scopes = new ArrayList<>();
     private final Map<String, Scope> scopeMap;
     // Contains possible scenarios for which rules are available.
-    private final Set<FSFPair> rulesSet;
+    private final Set<FSFPair> pathsUsedInRestrictions;
 
     public ParsedConstraints(Map<String, String> aAliases, Map<String, List<Rule>> aScopes)
     {
@@ -54,15 +53,15 @@ public ParsedConstraints(Map<String, String> aAliases, Map<String, List<Rule>> a
             scopes.add(scope);
         }
 
-        rulesSet = buildRulesSet();
+        pathsUsedInRestrictions = indexPathsUsedInRestrictions();
         scopeMap = buildScopeMap();
     }
 
     public ParsedConstraints(Map<String, String> aAliases, List<Scope> aScopes)
     {
         imports.putAll(aAliases);
         scopes.addAll(aScopes);
-        rulesSet = buildRulesSet();
+        pathsUsedInRestrictions = indexPathsUsedInRestrictions();
         scopeMap = buildScopeMap();
     }
 
@@ -79,7 +78,7 @@ public ParsedConstraints(ASTConstraintsSet astConstraintsSet)
         }
 
         scopeMap = buildScopeMap();
-        rulesSet = buildRulesSet();
+        pathsUsedInRestrictions = indexPathsUsedInRestrictions();
     }
 
     public Map<String, String> getImports()
@@ -112,22 +111,30 @@ public Scope getScopeByName(String scopeName)
     /**
      * @return if rules exists or not
      */
-    public boolean areThereRules(String aFS, String aFeature)
+    public boolean isPathUsedInAnyRestriction(FeatureStructure aContext, String aPath)
+    {
+        return isPathUsedInAnyRestriction(aContext.getType().getName(), aPath);
+    }
+
+    /**
+     * @return if rules exists or not
+     */
+    public boolean isPathUsedInAnyRestriction(String aContextTypeName, String aPath)
     {
-        if (rulesSet == null) {
-            buildRulesSet();
+        if (pathsUsedInRestrictions == null) {
+            indexPathsUsedInRestrictions();
         }
 
-        var shortName = getShortName(aFS);
-        if (shortName == null) {
+        var shortTypeName = getShortName(aContextTypeName);
+        if (shortTypeName == null) {
             return false;
         }
 
-        if (getScopeByName(shortName) == null) {
+        if (getScopeByName(shortTypeName) == null) {
             return false;
         }
 
-        if (rulesSet.contains(new FSFPair(shortName, aFeature))) {
+        if (pathsUsedInRestrictions.contains(new FSFPair(shortTypeName, aPath))) {
             // If it has rules satisfying with proper input FS and affecting feature
             return true;
         }
@@ -137,17 +144,18 @@ public boolean areThereRules(String aFS, String aFeature)
 
     private Map<String, Scope> buildScopeMap()
     {
-        var scopeMap = new HashMap<String, Scope>();
+        var map = new HashMap<String, Scope>();
         for (var scope : scopes) {
-            scopeMap.put(scope.getScopeName(), scope);
+            map.put(scope.getScopeName(), scope);
         }
-        return unmodifiableMap(scopeMap);
+        return unmodifiableMap(map);
     }
 
     /**
-     * @return Set with values of different conditions for which rules are available.
+     * @return set of pairs where the key of each pair is a scope and the value is a restriction
+     *         path
      */
-    private Set<FSFPair> buildRulesSet()
+    private Set<FSFPair> indexPathsUsedInRestrictions()
     {
         var rs = new HashSet<FSFPair>();
         FSFPair _temp;

inception/inception-constraints/src/main/java/de/tudarmstadt/ukp/clarin/webanno/constraints/model/Restriction.java

@@ -20,6 +20,7 @@
 import static de.tudarmstadt.ukp.clarin.webanno.constraints.grammar.ConstraintsParser.asFlatString;
 
 import java.io.Serializable;
+import java.util.List;
 
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
@@ -66,6 +67,11 @@ public String getValue()
         return value;
     }
 
+    public boolean matchesAny(List<String> listOfValues)
+    {
+        return listOfValues.contains(value);
+    }
+
     public boolean isFlagImportant()
     {
         return flagImportant;