Merge branch 'release/33.x'

* release/33.x:
  #4919 - Unable to import project with knowledge-based exported by INCEpTION 33.0 (again)
  #4921 - Constraints do not work properly when rules are distributed over multiple rulesets

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

@@ -28,6 +28,8 @@
 import java.io.InputStream;
 import java.io.StringReader;
 import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Objects;
 
@@ -48,6 +50,7 @@
 import de.tudarmstadt.ukp.clarin.webanno.constraints.grammar.ConstraintsParser;
 import de.tudarmstadt.ukp.clarin.webanno.constraints.grammar.ParseException;
 import de.tudarmstadt.ukp.clarin.webanno.constraints.model.ParsedConstraints;
+import de.tudarmstadt.ukp.clarin.webanno.constraints.model.Rule;
 import de.tudarmstadt.ukp.clarin.webanno.model.ConstraintSet;
 import de.tudarmstadt.ukp.clarin.webanno.model.Project;
 import de.tudarmstadt.ukp.inception.documents.api.RepositoryProperties;
@@ -240,52 +243,56 @@ public ParsedConstraints getMergedConstraints(Project aProject)
     private ParsedConstraints loadConstraints(Project aProject) throws IOException, ParseException
     {
         try (var logCtx = withProjectLogger(aProject)) {
-            ParsedConstraints merged = null;
-
+            var sets = new ArrayList<ParsedConstraints>();
             for (var set : listConstraintSets(aProject)) {
-                var script = readConstrainSet(set);
-                var parser = new ConstraintsParser(new StringReader(script));
-                var astConstraintsSet = parser.constraintsSet();
-                var constraints = new ParsedConstraints(astConstraintsSet);
+                sets.add(loadConstraints(set));
+            }
 
-                if (merged == null) {
-                    merged = constraints;
-                }
-                else {
-                    // Merge imports
-                    for (var e : constraints.getImports().entrySet()) {
-                        // Check if the value already points to some other feature in previous
-                        // constraint file(s).
-                        if (merged.getImports().containsKey(e.getKey()) && !e.getValue()
-                                .equalsIgnoreCase(merged.getImports().get(e.getKey()))) {
-                            // If detected, notify user with proper message and abort merging
-                            var errorMessage = "Conflict detected in imports for key \""
-                                    + e.getKey() + "\", conflicting values are \"" + e.getValue()
-                                    + "\" & \"" + merged.getImports().get(e.getKey())
-                                    + "\". Please contact Project Admin for correcting this."
-                                    + "Constraints feature may not work."
-                                    + "\nAborting Constraint rules merge!";
-                            throw new ParseException(errorMessage);
-                        }
-                    }
-                    merged.getImports().putAll(constraints.getImports());
-
-                    // Merge scopes
-                    for (var scope : constraints.getScopes()) {
-                        var target = merged.getScopeByName(scope.getScopeName());
-                        if (target == null) {
-                            // Scope does not exist yet
-                            merged.getScopes().add(scope);
-                        }
-                        else {
-                            // Scope already exists
-                            target.getRules().addAll(scope.getRules());
-                        }
-                    }
+            return mergeConstraintSets(sets);
+        }
+    }
+
+    static ParsedConstraints mergeConstraintSets(List<ParsedConstraints> sets) throws ParseException
+    {
+        var allImports = new LinkedHashMap<String, String>();
+        var allScopes = new LinkedHashMap<String, List<Rule>>();
+
+        for (var constraints : sets) {
+
+            // Merge imports
+            for (var e : constraints.getImports().entrySet()) {
+                // Check if the value already points to some other feature in previous
+                // constraint file(s).
+                if (allImports.containsKey(e.getKey())
+                        && !e.getValue().equalsIgnoreCase(allImports.get(e.getKey()))) {
+                    // If detected, notify user with proper message and abort merging
+                    var errorMessage = "Conflict detected in imports for key \"" + e.getKey()
+                            + "\", conflicting values are \"" + e.getValue() + "\" & \""
+                            + allImports.get(e.getKey())
+                            + "\". Please contact a project manager to correct this."
+                            + "Constraint may not work." + "\nAborting Constraint rules merge!";
+                    throw new ParseException(errorMessage);
                 }
             }
+            allImports.putAll(constraints.getImports());
 
-            return merged;
+            // Merge scopes
+            for (var scope : constraints.getScopes()) {
+                var target = allScopes.computeIfAbsent(scope.getScopeName(),
+                        $ -> new ArrayList<Rule>());
+                target.addAll(scope.getRules());
+            }
         }
+
+        return new ParsedConstraints(allImports, allScopes);
+    }
+
+    private ParsedConstraints loadConstraints(ConstraintSet set) throws IOException, ParseException
+    {
+        var script = readConstrainSet(set);
+        var parser = new ConstraintsParser(new StringReader(script));
+        var astConstraintsSet = parser.constraintsSet();
+        var constraints = new ParsedConstraints(astConstraintsSet);
+        return constraints;
     }
 }

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

@@ -24,38 +24,51 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 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
 {
     private static final long serialVersionUID = -2401965871743170805L;
 
-    private final Map<String, String> imports;
-    private final List<Scope> scopes;
+    private final Map<String, String> imports = new LinkedHashMap<>();
+    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;
 
+    public ParsedConstraints(Map<String, String> aAliases, Map<String, List<Rule>> aScopes)
+    {
+        imports.putAll(aAliases);
+
+        for (var e : aScopes.entrySet()) {
+            var scope = new Scope(e.getKey(), e.getValue());
+            scopes.add(scope);
+        }
+
+        rulesSet = buildRulesSet();
+        scopeMap = buildScopeMap();
+    }
+
     public ParsedConstraints(Map<String, String> aAliases, List<Scope> aScopes)
     {
-        imports = aAliases;
-        scopes = aScopes;
+        imports.putAll(aAliases);
+        scopes.addAll(aScopes);
         rulesSet = buildRulesSet();
         scopeMap = buildScopeMap();
     }
 
     public ParsedConstraints(ASTConstraintsSet astConstraintsSet)
     {
-        imports = astConstraintsSet.getImports();
-        scopes = new ArrayList<>();
+        imports.putAll(astConstraintsSet.getImports());
 
         for (var ruleGroup : astConstraintsSet.getScopes().entrySet()) {
             var rules = new ArrayList<Rule>();
@@ -105,16 +118,16 @@ public boolean areThereRules(String aFS, String aFeature)
             buildRulesSet();
         }
 
-        if (getShortName(aFS) == null) {
+        var shortName = getShortName(aFS);
+        if (shortName == null) {
             return false;
         }
 
-        if (getScopeByName(getShortName(aFS)) == null) {
+        if (getScopeByName(shortName) == null) {
             return false;
         }
 
-        var _tempFsfPair = new FSFPair(getShortName(aFS), aFeature);
-        if (rulesSet.contains(_tempFsfPair)) {
+        if (rulesSet.contains(new FSFPair(shortName, aFeature))) {
             // If it has rules satisfying with proper input FS and affecting feature
             return true;
         }
@@ -125,7 +138,7 @@ public boolean areThereRules(String aFS, String aFeature)
     private Map<String, Scope> buildScopeMap()
     {
         var scopeMap = new HashMap<String, Scope>();
-        for (Scope scope : scopes) {
+        for (var scope : scopes) {
             scopeMap.put(scope.getScopeName(), scope);
         }
         return unmodifiableMap(scopeMap);
@@ -136,19 +149,19 @@ private Map<String, Scope> buildScopeMap()
      */
     private Set<FSFPair> buildRulesSet()
     {
-        var rulesSet = new HashSet<FSFPair>();
+        var rs = new HashSet<FSFPair>();
         FSFPair _temp;
         for (var scope : scopes) {
             for (var rule : scope.getRules()) {
                 for (var restriction : rule.getRestrictions()) {
                     _temp = new FSFPair(scope.getScopeName(), restriction.getPath());
-                    if (!rulesSet.contains(_temp)) {
-                        rulesSet.add(_temp);
+                    if (!rs.contains(_temp)) {
+                        rs.add(_temp);
                     }
                 }
             }
         }
-        return unmodifiableSet(rulesSet);
+        return unmodifiableSet(rs);
     }
 
     private String printImports()

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

@@ -27,13 +27,14 @@ public class Scope
     implements Serializable
 {
     private static final long serialVersionUID = 226908916809455385L;
+
     private final String scopeName;
     private final List<Rule> rules;
 
-    public Scope(String scopeName, List<Rule> rules)
+    public Scope(String aScopeName, List<Rule> aRules)
     {
-        this.scopeName = scopeName;
-        this.rules = rules;
+        scopeName = aScopeName;
+        rules = aRules;
     }
 
     public String getScopeName()
@@ -51,5 +52,4 @@ public String toString()
     {
         return "Scope [" + scopeName + "]\nRules\n" + rules.toString() + "]";
     }
-
 }

inception/inception-constraints/src/test/java/de/tudarmstadt/ukp/clarin/webanno/constraints/ConstraintsServiceImplTest.java

@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Technische Universität Darmstadt under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The Technische Universität Darmstadt 
+ * 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.
+ *  
+ * 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 de.tudarmstadt.ukp.clarin.webanno.constraints;
+
+import static de.tudarmstadt.ukp.clarin.webanno.constraints.ConstraintsServiceImpl.mergeConstraintSets;
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Map;
+
+import org.junit.jupiter.api.Test;
+
+import de.tudarmstadt.ukp.clarin.webanno.constraints.model.Condition;
+import de.tudarmstadt.ukp.clarin.webanno.constraints.model.ParsedConstraints;
+import de.tudarmstadt.ukp.clarin.webanno.constraints.model.Restriction;
+import de.tudarmstadt.ukp.clarin.webanno.constraints.model.Rule;
+import de.tudarmstadt.ukp.clarin.webanno.constraints.model.Scope;
+
+class ConstraintsServiceImplTest
+{
+    @Test
+    void testMergeConstraintSets() throws Exception
+    {
+        var rule1 = new Rule(new Condition("foo", "lala"), new Restriction("fee", "lili"));
+        var set1 = new ParsedConstraints(Map.of("Foo", "some.Foo"),
+                asList(new Scope("Foo", asList(rule1))));
+
+        var rule2 = new Rule(new Condition("bar", "lolo"), new Restriction("faa", "lulu"));
+        var set2 = new ParsedConstraints(Map.of("Bar", "some.Bar"),
+                asList(new Scope("Bar", asList(rule2))));
+
+        var merged = mergeConstraintSets(asList(set1, set2));
+
+        assertThat(merged.getImports()) //
+                .hasSize(2) //
+                .containsEntry("Foo", "some.Foo") //
+                .containsEntry("Bar", "some.Bar");
+
+        assertThat(merged.getShortName("some.Foo")).isEqualTo("Foo");
+        assertThat(merged.getShortName("some.Bar")).isEqualTo("Bar");
+
+        assertThat(merged.getScopes()) //
+                .extracting(Scope::getScopeName) //
+                .containsExactlyInAnyOrder("Foo", "Bar");
+
+        assertThat(merged.getScopeByName("Foo")) //
+                .extracting(Scope::getScopeName) //
+                .isEqualTo("Foo");
+        assertThat(merged.getScopeByName("Foo").getRules()) //
+                .containsExactly(rule1);
+
+        assertThat(merged.getScopeByName("Bar")) //
+                .extracting(Scope::getScopeName) //
+                .isEqualTo("Bar");
+        assertThat(merged.getScopeByName("Bar").getRules()) //
+                .containsExactly(rule2);
+    }
+}

inception/inception-export-api/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/export/ProjectExporter.java

@@ -18,6 +18,7 @@
 package de.tudarmstadt.ukp.clarin.webanno.api.export;
 
 import static org.apache.commons.io.FilenameUtils.normalize;
+import static org.apache.commons.lang3.StringUtils.isEmpty;
 import static org.apache.commons.lang3.StringUtils.removeStart;
 
 import java.io.IOException;
@@ -56,23 +57,30 @@ void importData(ProjectImportRequest aRequest, Project aProject, ExportedProject
 
     static String normalizeEntryName(ZipEntry aEntry)
     {
-        // Strip leading "/" that we had in ZIP files prior to 2.0.8 (bug #985)
-        var entryName = aEntry.toString();
-        if (entryName.startsWith("/")) {
-            entryName = entryName.substring(1);
+        var name = removeStart(normalize(aEntry.getName(), true), "/");
+
+        if (isEmpty(name)) {
+            return null;
         }
 
-        return entryName;
+        return name;
     }
 
     static ZipEntry getEntry(ZipFile aFile, String aEntryName)
     {
-        var entryName = new ZipEntry(removeStart(normalize(aEntryName, true), "/"));
-        var entry = aFile.getEntry(aEntryName);
+        var normalizedEntryName = normalize(aEntryName, true);
+
+        var entry = aFile.getEntry(normalizedEntryName);
         if (entry != null) {
             return entry;
         }
-        return aFile.getEntry("/" + entryName);
+
+        if (normalizedEntryName.startsWith("/")) {
+            return aFile.getEntry(removeStart(normalizedEntryName, "/"));
+        }
+        else {
+            return aFile.getEntry("/" + normalizedEntryName);
+        }
     }
 
     static void writeEntry(ZipOutputStream aStage, String aEntryName,