SLING-11243 merge multivalue restriction values for eace/eacl json (#13)

If a parent node has one set of values for a restriction and the child node has a different set of values for the same restriction, then the effective ACE view of the child node should have both of those value sets merged together.
diff --git a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/LocalPrivilege.java b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/LocalPrivilege.java
index ea36f0a..b898abc 100644
--- a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/LocalPrivilege.java
+++ b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/LocalPrivilege.java
@@ -16,9 +16,15 @@
  */
 package org.apache.sling.jcr.jackrabbit.accessmanager;
 
+import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Stream;
 
+import javax.jcr.Value;
 import javax.jcr.security.Privilege;
 
 import org.jetbrains.annotations.NotNull;
@@ -73,11 +79,59 @@
         return Collections.unmodifiableSet(denyRestrictions);
     }
 
+    protected Set<LocalRestriction> mergeRestrictions(Set<LocalRestriction> currentRestrictions, Set<LocalRestriction> newRestrictions) {
+        Set<LocalRestriction> mergedRestrictons;
+        if (newRestrictions == null) {
+            mergedRestrictons = null;
+        } else {
+            mergedRestrictons = new HashSet<>(currentRestrictions);
+            for (LocalRestriction lr : newRestrictions) {
+                if (lr.isMultiValue()) {
+                    String expectedName = lr.getName();
+                    Optional<LocalRestriction> existing = currentRestrictions.stream()
+                            .filter(r ->  r.getName().equals(expectedName))
+                            .findFirst();
+                    if (existing.isPresent()) {
+                        //remove the old one that we are replacing
+                        mergedRestrictons.removeIf(k -> expectedName.equals(k.getName()));
+
+                        Set<Value> mergedValues = new LinkedHashSet<>();
+                        // add the current values
+                        LocalRestriction existingLr = existing.get();
+                        Stream.of(existingLr.getValues())
+                            .forEach(mergedValues::add);
+                        // merge the new values
+                        Stream.of(lr.getValues())
+                            .forEach(mergedValues::add);
+                        Value[] newValues = mergedValues.toArray(new Value[mergedValues.size()]);
+                        // construct the replacement object
+                        lr = LocalRestriction.cloneWithNewValues(lr, newValues);
+                    }
+                }
+                // add to the set
+                mergedRestrictons.add(lr);
+            }
+        }
+        return mergedRestrictons;
+    }
+
     public void setAllowRestrictions(Set<LocalRestriction> restrictions) {
-        this.allowRestrictions = restrictions;
+        this.allowRestrictions = mergeRestrictions(this.allowRestrictions, restrictions);
     }
     public void setDenyRestrictions(Set<LocalRestriction> restrictions) {
-        this.denyRestrictions = restrictions;
+        this.denyRestrictions = mergeRestrictions(this.denyRestrictions, restrictions);
+    }
+    public void unsetAllowRestrictions(Collection<String> restrictionNames) {
+        this.allowRestrictions.removeIf(k -> restrictionNames.contains(k.getName()));
+    }
+    public void unsetDenyRestrictions(Collection<String> restrictionNames) {
+        this.denyRestrictions.removeIf(k -> restrictionNames.contains(k.getName()));
+    }
+    public void clearAllowRestrictions() {
+        this.allowRestrictions.clear();
+    }
+    public void clearDenyRestrictions() {
+        this.denyRestrictions.clear();
     }
 
     /**
diff --git a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/LocalRestriction.java b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/LocalRestriction.java
index 8647d4f..58e5a61 100644
--- a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/LocalRestriction.java
+++ b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/LocalRestriction.java
@@ -102,4 +102,11 @@
         return Arrays.equals(values, other.values);
     }
 
+    /**
+     * Clone from an existing object and then assign the new values
+     */
+    public static @NotNull LocalRestriction cloneWithNewValues(@NotNull LocalRestriction lr, @NotNull Value[] newValues) {
+        return new LocalRestriction(lr.rd, newValues);
+    }
+
 }
diff --git a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java
index 30902d5..6404271 100644
--- a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java
+++ b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java
@@ -21,7 +21,6 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -268,26 +267,20 @@
         forAllow &= localPrivilege.isAllow();
         forDeny &= localPrivilege.isDeny();
         if (forAllow) {
-            Set<LocalRestriction> allowRestrictions = new HashSet<>(localPrivilege.getAllowRestrictions());
-            if (allowRestrictions.removeIf(lr -> restrictionNames.contains(lr.getName()))) {
-                localPrivilege.setAllowRestrictions(allowRestrictions);
-            }
+            localPrivilege.unsetAllowRestrictions(restrictionNames);
         }
         if (forDeny) {
-            Set<LocalRestriction> denyRestrictions = new HashSet<>(localPrivilege.getDenyRestrictions());
-            if (denyRestrictions.removeIf(lr -> restrictionNames.contains(lr.getName()))) {
-                localPrivilege.setDenyRestrictions(denyRestrictions);
-            }
+            localPrivilege.unsetDenyRestrictions(restrictionNames);
         }
 
         if (localPrivilege.sameAllowAndDenyRestrictions()) {
             // same restrictions so we can unset one of them
             if (forAllow) {
                 localPrivilege.setDeny(false);
-                localPrivilege.setDenyRestrictions(Collections.emptySet());
+                localPrivilege.clearDenyRestrictions();
             } else if (forDeny) {
                 localPrivilege.setAllow(false);
-                localPrivilege.setAllowRestrictions(Collections.emptySet());
+                localPrivilege.clearAllowRestrictions();
             }
         }
 
@@ -341,22 +334,18 @@
             if (requireAllowOrDenyAlreadySet && !localPrivilege.isDeny()) {
                 //skip it
             } else {
-                Set<LocalRestriction> denyRestrictions = new HashSet<>(localPrivilege.getDenyRestrictions());
-                denyRestrictions.removeIf(lr -> lr.getName().equals(restriction.getName()));
-                denyRestrictions.add(restriction);
                 localPrivilege.setDeny(true);
-                localPrivilege.setDenyRestrictions(denyRestrictions);
+                localPrivilege.unsetDenyRestrictions(Collections.singleton(restriction.getName()));
+                localPrivilege.setDenyRestrictions(Collections.singleton(restriction));
             }
         }
         if (forAllow) {
             if (requireAllowOrDenyAlreadySet && !localPrivilege.isAllow()) {
                 //skip it
             } else {
-                Set<LocalRestriction> allowRestrictions = new HashSet<>(localPrivilege.getAllowRestrictions());
-                allowRestrictions.removeIf(lr -> lr.getName().equals(restriction.getName()));
-                allowRestrictions.add(restriction);
                 localPrivilege.setAllow(true);
-                localPrivilege.setAllowRestrictions(allowRestrictions);
+                localPrivilege.unsetAllowRestrictions(Collections.singleton(restriction.getName()));
+                localPrivilege.setAllowRestrictions(Collections.singleton(restriction));
             }
         }
 
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/AccessManagerClientTestSupport.java b/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/AccessManagerClientTestSupport.java
index bd71185..68c19d6 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/AccessManagerClientTestSupport.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/AccessManagerClientTestSupport.java
@@ -273,7 +273,7 @@
     }
     protected void assertPrivilege(JsonObject privilegesObject, boolean expectedPrivilege, PrivilegeValues privilegeState, String privilegeName,
             VerifyAce verifyAce) {
-        assertPrivilege(privilegesObject, expectedPrivilege, privilegeState, privilegeName, true, null);
+        assertPrivilege(privilegesObject, expectedPrivilege, privilegeState, privilegeName, true, verifyAce);
     }
     protected void assertPrivilege(JsonObject privilegesObject, boolean expectedPrivilege, PrivilegeValues privilegeState, String privilegeName,
             boolean expectedForAllow,
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/GetEaceIT.java b/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/GetEaceIT.java
index 14bb7b7..0e71730 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/GetEaceIT.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/GetEaceIT.java
@@ -17,6 +17,7 @@
 package org.apache.sling.jcr.jackrabbit.accessmanager.it;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
@@ -266,4 +267,73 @@
         assertEquals(testFolderPath, ((JsonString)nodeArray.get(1)).getString());
     }
 
+    /**
+     * Effective ACE servlet returns correct information
+     */
+    @Test
+    public void testEffectiveAceForUserWithMergedRestrictionValues() throws IOException, JsonException {
+        testUserId = createTestUser();
+        testFolderUrl = createTestFolder(null, "sling-tests2",
+                "{ \"jcr:primaryType\": \"nt:unstructured\", \"child\" : { \"childPropOne\" : true, \"childPropTwo\" : \"two\" } }");
+
+        //1. create an initial set of privileges
+        List<NameValuePair> postParams = new AcePostParamsBuilder(testUserId)
+                .withPrivilege(PrivilegeConstants.REP_READ_PROPERTIES, PrivilegeValues.DENY)
+                .withPrivilegeRestriction(PrivilegeValues.ALLOW, PrivilegeConstants.REP_READ_PROPERTIES, AccessControlConstants.REP_ITEM_NAMES, "childPropOne")
+                .build();
+        addOrUpdateAce(testFolderUrl, postParams);
+
+        // at this point the child JSON should have only one of the properties visible
+        Credentials testUserCreds = new UsernamePasswordCredentials(testUserId, "testPwd");
+        String getJsonContentUrl = testFolderUrl + "/child.json";
+        String jsonContent = getAuthenticatedContent(testUserCreds, getJsonContentUrl, CONTENT_TYPE_JSON, HttpServletResponse.SC_OK);
+        assertNotNull(jsonContent);
+        JsonObject jsonObject = parseJson(jsonContent);
+        assertTrue("Expected childPropOne property", jsonObject.containsKey("childPropOne"));
+        assertFalse("Did not expected childPropTwo property", jsonObject.containsKey("childPropTwo"));
+
+        // add ACE to the child to make the other property readable
+        List<NameValuePair> postParams2 = new AcePostParamsBuilder(testUserId)
+                .withPrivilegeRestriction(PrivilegeValues.ALLOW, PrivilegeConstants.REP_READ_PROPERTIES, AccessControlConstants.REP_ITEM_NAMES, "childPropTwo")
+                .build();
+        addOrUpdateAce(testFolderUrl + "/child", postParams2);
+
+        //fetch the JSON for the ace to verify the settings.
+        Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+        String getUrl = testFolderUrl + "/child.eace.json?pid=" + testUserId;
+        String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, HttpServletResponse.SC_OK);
+        assertNotNull(json);
+        JsonObject aceObject = parseJson(json);
+
+        String principalString = aceObject.getString("principal");
+        assertEquals(testUserId, principalString);
+
+        JsonObject privilegesObject = aceObject.getJsonObject("privileges");
+        assertNotNull(privilegesObject);
+        assertEquals(1, privilegesObject.size());
+        //allow privilege
+        assertPrivilege(privilegesObject, true, PrivilegeValues.ALLOW, PrivilegeConstants.REP_READ_PROPERTIES, jsonValue -> {
+            assertNotNull(jsonValue);
+            assertTrue(jsonValue instanceof JsonObject);
+            JsonObject restrictionsObj = (JsonObject)jsonValue;
+
+            // verify we have the merged restriction values
+            JsonValue itemNamesValue = restrictionsObj.get(AccessControlConstants.REP_ITEM_NAMES);
+            assertNotNull(itemNamesValue);
+            assertTrue(itemNamesValue instanceof JsonArray);
+            JsonArray itemNamesArray = (JsonArray)itemNamesValue;
+            assertEquals(2, itemNamesArray.size());
+            assertEquals("childPropOne", itemNamesArray.getString(0));
+            assertEquals("childPropTwo", itemNamesArray.getString(1));
+        });
+
+        // now verify that the JSON has both properties visible
+        jsonContent = getAuthenticatedContent(testUserCreds, getJsonContentUrl, CONTENT_TYPE_JSON, HttpServletResponse.SC_OK);
+        assertNotNull(jsonContent);
+        jsonObject = parseJson(jsonContent);
+        assertTrue("Expected childPropOne property", jsonObject.containsKey("childPropOne"));
+        assertTrue("Expected childPropTwo property", jsonObject.containsKey("childPropTwo"));
+    }
+
+
 }
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/ModifyAceIT.java b/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/ModifyAceIT.java
index d394ff1..0f36503 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/ModifyAceIT.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/it/ModifyAceIT.java
@@ -2218,13 +2218,13 @@
         // update the ACE
         List<NameValuePair> postParams = new AcePostParamsBuilder(testGroupId)
             .withPrivilegeRestriction(PrivilegeValues.ALLOW, PrivilegeConstants.JCR_READ, AccessControlConstants.REP_GLOB, "/hello")
-            .withPrivilegeRestriction(PrivilegeValues.NONE, PrivilegeConstants.JCR_READ, AccessControlConstants.REP_GLOB, "/hello")
+            .withPrivilegeRestriction(PrivilegeValues.DENY, PrivilegeConstants.JCR_READ, AccessControlConstants.REP_GLOB, "/hello")
             .build();
         addOrUpdateAce(testFolderUrl, postParams);
         JsonObject groupPrivilegesObject = getAcePrivleges(testFolderUrl, testGroupId);
         assertEquals(1, groupPrivilegesObject.size());
 
-        //allow privilege is there (allow prefererred over deny)
+        //allow privilege is there (allow preferred over deny)
         //allow privilege
         assertPrivilege(groupPrivilegesObject, true, PrivilegeValues.ALLOW, PrivilegeConstants.JCR_READ, true, jsonValue -> {
             assertNotNull(jsonValue);
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/LocalPrivilegeTest.java b/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/LocalPrivilegeTest.java
index 9aef6f5..857db6b 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/LocalPrivilegeTest.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/LocalPrivilegeTest.java
@@ -36,6 +36,7 @@
 import javax.jcr.security.Privilege;
 
 import org.apache.jackrabbit.oak.security.authorization.restriction.RestrictionProviderImpl;
+import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionDefinition;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
@@ -196,7 +197,7 @@
         assertTrue(allowRestrictions.isEmpty());
 
         Set<LocalRestriction> newAllowRestrictions = new HashSet<>();
-        newAllowRestrictions.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
+        newAllowRestrictions.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
         lp1.setAllowRestrictions(newAllowRestrictions);
         Set<LocalRestriction> allowRestrictions2 = lp1.getAllowRestrictions();
         assertNotNull(allowRestrictions2);
@@ -215,7 +216,7 @@
         assertTrue(denyRestrictions.isEmpty());
 
         Set<LocalRestriction> newDenyRestrictions = new HashSet<>();
-        newDenyRestrictions.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
+        newDenyRestrictions.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
         lp1.setDenyRestrictions(newDenyRestrictions);
         Set<LocalRestriction> denyRestrictions2 = lp1.getDenyRestrictions();
         assertNotNull(denyRestrictions2);
@@ -233,20 +234,20 @@
         assertTrue(lp1.sameAllowRestrictions(lp2.getAllowRestrictions()));
 
         Set<LocalRestriction> newAllowRestrictions1 = new HashSet<>();
-        newAllowRestrictions1.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newAllowRestrictions1.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2")));
+        newAllowRestrictions1.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newAllowRestrictions1.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2")));
         lp1.setAllowRestrictions(newAllowRestrictions1);
         assertFalse(lp1.sameAllowRestrictions(lp2.getAllowRestrictions()));
 
         Set<LocalRestriction> newAllowRestrictions2 = new HashSet<>();
-        newAllowRestrictions2.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newAllowRestrictions2.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2")));
+        newAllowRestrictions2.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newAllowRestrictions2.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2")));
         lp2.setAllowRestrictions(newAllowRestrictions2);
         assertTrue(lp1.sameAllowRestrictions(lp2.getAllowRestrictions()));
 
         Set<LocalRestriction> newAllowRestrictions3 = new HashSet<>();
-        newAllowRestrictions3.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newAllowRestrictions3.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2_changed")));
+        newAllowRestrictions3.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newAllowRestrictions3.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2_changed")));
         lp2.setAllowRestrictions(newAllowRestrictions3);
         assertFalse(lp1.sameAllowRestrictions(lp2.getAllowRestrictions()));
     }
@@ -261,20 +262,20 @@
         assertTrue(lp1.sameDenyRestrictions(lp2.getDenyRestrictions()));
 
         Set<LocalRestriction> newDenyRestrictions1 = new HashSet<>();
-        newDenyRestrictions1.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newDenyRestrictions1.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2")));
+        newDenyRestrictions1.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newDenyRestrictions1.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2")));
         lp1.setDenyRestrictions(newDenyRestrictions1);
         assertFalse(lp1.sameDenyRestrictions(lp2.getDenyRestrictions()));
 
         Set<LocalRestriction> newDenyRestrictions2 = new HashSet<>();
-        newDenyRestrictions2.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newDenyRestrictions2.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2")));
+        newDenyRestrictions2.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newDenyRestrictions2.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2")));
         lp2.setDenyRestrictions(newDenyRestrictions2);
         assertTrue(lp1.sameDenyRestrictions(lp2.getDenyRestrictions()));
 
         Set<LocalRestriction> newDenyRestrictions3 = new HashSet<>();
-        newDenyRestrictions3.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newDenyRestrictions3.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2_changed")));
+        newDenyRestrictions3.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newDenyRestrictions3.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2_changed")));
         lp2.setDenyRestrictions(newDenyRestrictions3);
         assertFalse(lp1.sameDenyRestrictions(lp2.getDenyRestrictions()));
     }
@@ -288,20 +289,20 @@
         assertTrue(lp1.sameAllowAndDenyRestrictions());
 
         Set<LocalRestriction> newDenyRestrictions1 = new HashSet<>();
-        newDenyRestrictions1.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newDenyRestrictions1.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2")));
+        newDenyRestrictions1.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newDenyRestrictions1.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2")));
         lp1.setDenyRestrictions(newDenyRestrictions1);
         assertFalse(lp1.sameAllowAndDenyRestrictions());
 
         Set<LocalRestriction> newAllowRestrictions1 = new HashSet<>();
-        newAllowRestrictions1.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newAllowRestrictions1.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2")));
+        newAllowRestrictions1.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newAllowRestrictions1.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2")));
         lp1.setAllowRestrictions(newAllowRestrictions1);
         assertTrue(lp1.sameAllowAndDenyRestrictions());
 
         Set<LocalRestriction> newAllowRestrictions2 = new HashSet<>();
-        newAllowRestrictions2.add(new LocalRestriction(rd("rep:glob"), val("/hello")));
-        newAllowRestrictions2.add(new LocalRestriction(rd("nt:itemNames"), vals("item1", "item2_changed")));
+        newAllowRestrictions2.add(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")));
+        newAllowRestrictions2.add(new LocalRestriction(rd(AccessControlConstants.REP_ITEM_NAMES), vals("item1", "item2_changed")));
         lp1.setAllowRestrictions(newAllowRestrictions2);
         assertFalse(lp1.sameAllowAndDenyRestrictions());
     }
@@ -358,12 +359,12 @@
         assertNotEquals(lp10, lp11);
 
         LocalPrivilege lp12 = new LocalPrivilege(priv(PrivilegeConstants.JCR_READ));
-        lp12.setAllowRestrictions(new HashSet<>(Arrays.asList(new LocalRestriction(rd("rep:glob"), val("/hello")))));
+        lp12.setAllowRestrictions(new HashSet<>(Arrays.asList(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")))));
         LocalPrivilege lp13 = new LocalPrivilege(priv(PrivilegeConstants.JCR_READ));
         assertNotEquals(lp12, lp13);
 
         LocalPrivilege lp14 = new LocalPrivilege(priv(PrivilegeConstants.JCR_READ));
-        lp14.setDenyRestrictions(new HashSet<>(Arrays.asList(new LocalRestriction(rd("rep:glob"), val("/hello")))));
+        lp14.setDenyRestrictions(new HashSet<>(Arrays.asList(new LocalRestriction(rd(AccessControlConstants.REP_GLOB), val("/hello")))));
         LocalPrivilege lp15 = new LocalPrivilege(priv(PrivilegeConstants.JCR_READ));
         assertNotEquals(lp14, lp15);