You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2019/02/12 13:49:40 UTC

svn commit: r1853436 - in /jackrabbit/oak/trunk/oak-security-spi: ./ src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/

Author: angela
Date: Tue Feb 12 13:49:40 2019
New Revision: 1853436

URL: http://svn.apache.org/viewvc?rev=1853436&view=rev
Log:
OAK-8036 : Improve test in oak-security-spi
minor improvement: missing @NotNull/@Nullable  annotations with ImmutablePrivilegeDefinition constructor

Added:
    jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtilTest.java
Modified:
    jackrabbit/oak/trunk/oak-security-spi/pom.xml
    jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java
    jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinitionTest.java
    jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsTest.java

Modified: jackrabbit/oak/trunk/oak-security-spi/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/pom.xml?rev=1853436&r1=1853435&r2=1853436&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-security-spi/pom.xml Tue Feb 12 13:49:40 2019
@@ -34,7 +34,7 @@
   <properties>
     <!-- enable execution of jacoco and set minimal line coverage -->
     <skip.coverage>false</skip.coverage>
-    <minimum.coverage>0.91</minimum.coverage>
+    <minimum.coverage>0.92</minimum.coverage>
   </properties>
 
   <build>

Modified: jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java?rev=1853436&r1=1853435&r2=1853436&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java (original)
+++ jackrabbit/oak/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinition.java Tue Feb 12 13:49:40 2019
@@ -21,6 +21,7 @@ import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableSet;
 
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Default implementation of the {@code PrivilegeDefinition} interface.
@@ -31,9 +32,7 @@ public final class ImmutablePrivilegeDef
     private final boolean isAbstract;
     private final Set<String> declaredAggregateNames;
 
-    public ImmutablePrivilegeDefinition(
-            String name, boolean isAbstract,
-            Iterable<String> declaredAggregateNames) {
+    public ImmutablePrivilegeDefinition(@NotNull String name, boolean isAbstract, @Nullable Iterable<String> declaredAggregateNames) {
         this.name = name;
         this.isAbstract = isAbstract;
         ImmutableSet.Builder<String> builder = ImmutableSet.builder();

Modified: jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinitionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinitionTest.java?rev=1853436&r1=1853435&r2=1853436&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinitionTest.java (original)
+++ jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/ImmutablePrivilegeDefinitionTest.java Tue Feb 12 13:49:40 2019
@@ -77,4 +77,11 @@ public class ImmutablePrivilegeDefinitio
         assertNotEquals(def, new ImmutablePrivilegeDefinition("name", true, ImmutableList.of()));
         assertNotEquals(def, new ImmutablePrivilegeDefinition("otherName", false, ImmutableList.of("aggrName","aggrName2")));
     }
+
+    @Test
+    public void testToString() {
+        assertEquals(def.toString(), def.toString());
+        assertEquals(def.toString(), new ImmutablePrivilegeDefinition(def.getName(), def.isAbstract(), def.getDeclaredAggregateNames()).toString());
+        assertNotEquals(def.toString(), new ImmutablePrivilegeDefinition(def.getName(), def.isAbstract(), ImmutableList.of()));
+    }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsTest.java?rev=1853436&r1=1853435&r2=1853436&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsTest.java (original)
+++ jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsTest.java Tue Feb 12 13:49:40 2019
@@ -16,14 +16,8 @@
  */
 package org.apache.jackrabbit.oak.spi.security.privilege;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Random;
-
 import com.google.common.collect.ImmutableSet;
+import com.google.common.primitives.Longs;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Root;
 import org.apache.jackrabbit.oak.api.Tree;
@@ -33,6 +27,15 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.junit.Test;
 import org.mockito.Mockito;
 
+import javax.jcr.security.Privilege;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits.BUILT_IN;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
@@ -40,12 +43,14 @@ import static org.junit.Assert.assertNot
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.when;
 
 public class PrivilegeBitsTest implements PrivilegeConstants {
 
     private static final long NO_PRIVILEGE = 0;
-    private static final PrivilegeBits READ_NODES_PRIVILEGE_BITS = PrivilegeBits.BUILT_IN.get(REP_READ_NODES);
+    private static final PrivilegeBits READ_NODES_PRIVILEGE_BITS = BUILT_IN.get(REP_READ_NODES);
 
     private static final long[] LONGS = new long[]{1, 2, 13, 199, 512, Long.MAX_VALUE / 2, Long.MAX_VALUE - 1, Long.MAX_VALUE};
 
@@ -184,7 +189,7 @@ public class PrivilegeBitsTest implement
         PrivilegeBits pb = READ_NODES_PRIVILEGE_BITS;
         PrivilegeBits mod = PrivilegeBits.getInstance();
 
-        for (int i = 0; i < 100; i++) {
+        for (int i = 0; i < 200; i++) {
 
             assertFalse(PrivilegeBits.EMPTY.includes(pb));
             assertTrue(pb.includes(PrivilegeBits.EMPTY));
@@ -207,7 +212,7 @@ public class PrivilegeBitsTest implement
 
     @Test
     public void testBuiltIn() {
-        for (PrivilegeBits bits : PrivilegeBits.BUILT_IN.values()) {
+        for (PrivilegeBits bits : BUILT_IN.values()) {
             assertTrue(bits.isBuiltin());
             assertTrue(PrivilegeBits.getInstance(bits).isBuiltin());
         }
@@ -216,7 +221,7 @@ public class PrivilegeBitsTest implement
     @Test
     public void testCombinationNotBuiltIn() {
         PrivilegeBits combination = PrivilegeBits.getInstance();
-        for (PrivilegeBits bits : PrivilegeBits.BUILT_IN.values()) {
+        for (PrivilegeBits bits : BUILT_IN.values()) {
             combination.add(bits);
         }
         assertFalse(combination.isBuiltin());
@@ -230,7 +235,7 @@ public class PrivilegeBitsTest implement
     @Test
     public void testCombinationWithNextNotBuiltIn() {
         PrivilegeBits bits = PrivilegeBits.NEXT_AFTER_BUILT_INS;
-        PrivilegeBits toTest = PrivilegeBits.getInstance(PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_READ));
+        PrivilegeBits toTest = PrivilegeBits.getInstance(BUILT_IN.get(PrivilegeConstants.JCR_READ));
 
         for (int i = 0; i<100; i++) {
             bits = bits.nextBits();
@@ -242,7 +247,10 @@ public class PrivilegeBitsTest implement
     public void testIsEmpty() {
         // empty
         assertTrue(PrivilegeBits.EMPTY.isEmpty());
+    }
 
+    @Test
+    public void testNotIsEmpty() {
         // any other bits should not be empty
         PrivilegeBits pb = READ_NODES_PRIVILEGE_BITS;
         PrivilegeBits mod = PrivilegeBits.getInstance(pb);
@@ -253,10 +261,6 @@ public class PrivilegeBitsTest implement
             pb = pb.nextBits();
             mod.add(pb);
             assertFalse(mod.isEmpty());
-
-            PrivilegeBits tmp = PrivilegeBits.getInstance(pb);
-            tmp.diff(pb);
-            assertTrue(tmp.toString(), tmp.isEmpty());
         }
     }
 
@@ -273,27 +277,7 @@ public class PrivilegeBitsTest implement
         PrivilegeBits mod = PrivilegeBits.getInstance(pb);
 
         for (int i = 0; i < 100; i++) {
-            try {
-                pb.add(PrivilegeBits.EMPTY);
-                fail("UnsupportedOperation expected");
-            } catch (UnsupportedOperationException e) {
-                // success
-            }
-
-            try {
-                pb.add(mod);
-                fail("UnsupportedOperation expected");
-            } catch (UnsupportedOperationException e) {
-                // success
-            }
-
             PrivilegeBits nxt = pb.nextBits();
-            try {
-                pb.add(nxt);
-                fail("UnsupportedOperation expected");
-            } catch (UnsupportedOperationException e) {
-                // success
-            }
 
             long before = getLongValue(mod);
             long nxtLong = getLongValue(nxt);
@@ -308,9 +292,9 @@ public class PrivilegeBitsTest implement
             assertTrue(tmp.includes(pb));
             assertFalse(tmp.includes(nxt));
             if (READ_NODES_PRIVILEGE_BITS.equals(pb)) {
-                assertTrue(tmp.includes(PrivilegeBits.BUILT_IN.get(PrivilegeConstants.REP_READ_NODES)));
+                assertTrue(tmp.includes(BUILT_IN.get(PrivilegeConstants.REP_READ_NODES)));
             } else {
-                assertFalse(tmp.includes(PrivilegeBits.BUILT_IN.get(PrivilegeConstants.REP_READ_NODES)));
+                assertFalse(tmp.includes(BUILT_IN.get(PrivilegeConstants.REP_READ_NODES)));
             }
             tmp.add(nxt);
             assertTrue(tmp.includes(pb) && tmp.includes(nxt));
@@ -326,6 +310,39 @@ public class PrivilegeBitsTest implement
         }
     }
 
+    @Test
+    public void testAddSame() {
+        PrivilegeBits mod = PrivilegeBits.getInstance(READ_NODES_PRIVILEGE_BITS);
+        assertSame(mod, mod.add(mod));
+    }
+
+    @Test
+    public void testAddEquivalent() {
+        PrivilegeBits mod = PrivilegeBits.getInstance(READ_NODES_PRIVILEGE_BITS);
+        assertSame(mod, mod.add(READ_NODES_PRIVILEGE_BITS));
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void testAddSameToUnmodifiable() {
+        READ_NODES_PRIVILEGE_BITS.add(READ_NODES_PRIVILEGE_BITS);
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void testAddNextToUnmodifiable() {
+        READ_NODES_PRIVILEGE_BITS.add(READ_NODES_PRIVILEGE_BITS.nextBits());
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void testAddEmptyToUnmodifiable() {
+        READ_NODES_PRIVILEGE_BITS.nextBits().add(PrivilegeBits.EMPTY);
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void testAddModifiableToUnmodifiable() {
+        PrivilegeBits pb = PrivilegeBits.getInstance(PropertyStates.createProperty("any", Longs.asList(1, 16, 512), Type.LONGS));
+        pb.unmodifiable().add(READ_NODES_PRIVILEGE_BITS.modifiable());
+    }
+
     @Test(expected = UnsupportedOperationException.class)
     public void testEmptyDiff() {
         // empty privilege bits
@@ -410,6 +427,15 @@ public class PrivilegeBitsTest implement
         }
     }
 
+    @Test
+    public void testDiffEquivalent() {
+        PrivilegeBits pb = READ_NODES_PRIVILEGE_BITS;
+        for (int i = 0; i < 100; i++) {
+            assertTrue(pb.toString(), PrivilegeBits.getInstance(pb).diff(pb).isEmpty());
+            pb = pb.nextBits();
+        }
+    }
+
     @Test(expected = UnsupportedOperationException.class)
     public void testEmptyAddDifference() {
         // empty privilege bits
@@ -496,7 +522,7 @@ public class PrivilegeBitsTest implement
         pb.retain(PrivilegeBits.EMPTY);
         assertTrue(pb.isEmpty());
 
-        PrivilegeBits write = PrivilegeBits.BUILT_IN.get(PrivilegeBits.REP_WRITE);
+        PrivilegeBits write = BUILT_IN.get(PrivilegeBits.REP_WRITE);
         pb = PrivilegeBits.getInstance().add(write);
         assertEquals(pb, pb.retain(pb));
         assertEquals(pb, pb.retain(write));
@@ -509,7 +535,7 @@ public class PrivilegeBitsTest implement
         assertEquivalent(write, pb);
         assertFalse(pb.includes(READ_NODES_PRIVILEGE_BITS));
 
-        PrivilegeBits lock = PrivilegeBits.BUILT_IN.get(PrivilegeBits.JCR_LOCK_MANAGEMENT);
+        PrivilegeBits lock = BUILT_IN.get(PrivilegeBits.JCR_LOCK_MANAGEMENT);
         PrivilegeBits lw = PrivilegeBits.getInstance(write, lock);
 
         pb.add(READ_NODES_PRIVILEGE_BITS).add(write).add(lock);
@@ -566,6 +592,26 @@ public class PrivilegeBitsTest implement
     }
 
     @Test
+    public void testWriteToTree() {
+        Tree tree = when(mock(Tree.class).getName()).thenReturn("anyName").getMock();
+
+        PrivilegeBits bits = READ_NODES_PRIVILEGE_BITS;
+        bits.writeTo(tree);
+
+        Mockito.verify(tree, times(1)).setProperty(bits.asPropertyState(REP_BITS));
+    }
+
+    @Test
+    public void testWriteToPrivilegesRootTree() {
+        Tree tree = when(mock(Tree.class).getName()).thenReturn(REP_PRIVILEGES).getMock();
+
+        PrivilegeBits bits = READ_NODES_PRIVILEGE_BITS;
+        bits.writeTo(tree);
+
+        Mockito.verify(tree, times(1)).setProperty(bits.asPropertyState(REP_NEXT));
+    }
+
+    @Test
     public void testGetInstance() {
         PrivilegeBits pb = PrivilegeBits.getInstance();
         assertEquivalent(PrivilegeBits.EMPTY, pb);
@@ -618,12 +664,12 @@ public class PrivilegeBitsTest implement
     @Test
     public void testGetInstanceFromBase() {
         PrivilegeBits pb = PrivilegeBits.getInstance(READ_NODES_PRIVILEGE_BITS);
-        pb.add(PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
-        pb.add(PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT));
+        pb.add(BUILT_IN.get(PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
+        pb.add(BUILT_IN.get(PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT));
 
         PrivilegeBits pb2 = PrivilegeBits.getInstance(READ_NODES_PRIVILEGE_BITS,
-                PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_READ_ACCESS_CONTROL),
-                PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT));
+                BUILT_IN.get(PrivilegeConstants.JCR_READ_ACCESS_CONTROL),
+                BUILT_IN.get(PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT));
 
         assertEquivalent(pb, pb2);
     }
@@ -699,11 +745,16 @@ public class PrivilegeBitsTest implement
         assertSame(PrivilegeBits.EMPTY, PrivilegeBits.getInstance((PropertyState) null));
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testGetInstanceFromNegativeLong() {
+        PrivilegeBits.getInstance(PropertyStates.createProperty("name", Collections.singleton(NO_PRIVILEGE-1), Type.LONGS));
+    }
+
     @Test
     public void testGetInstanceFromTreeCustomPriv() {
         PrivilegeBits next = PrivilegeBits.NEXT_AFTER_BUILT_INS;
 
-        Tree tmp = Mockito.mock(Tree.class);
+        Tree tmp = mock(Tree.class);
         when(tmp.getName()).thenReturn("tmpPrivilege");
         when(tmp.getProperty(REP_BITS)).thenReturn(next.asPropertyState(REP_BITS));
 
@@ -712,10 +763,10 @@ public class PrivilegeBitsTest implement
 
     @Test
     public void testGetInstanceFromTreeJcrRead() {
-        Tree readPrivTree = Mockito.mock(Tree.class);
+        Tree readPrivTree = mock(Tree.class);
         when(readPrivTree.getName()).thenReturn(JCR_READ);
 
-        assertEquals(PrivilegeBits.BUILT_IN.get(JCR_READ), PrivilegeBits.getInstance(readPrivTree));
+        assertEquals(BUILT_IN.get(JCR_READ), PrivilegeBits.getInstance(readPrivTree));
     }
 
     @Test
@@ -724,15 +775,22 @@ public class PrivilegeBitsTest implement
     }
 
     @Test
-    public void testCalculatePermissions() {
-        PrivilegeBitsProvider provider = new PrivilegeBitsProvider(Mockito.mock(Root.class));
+    public void testGetInstanceFromPrivilegesRootTree() {
+        Tree tree = when(mock(Tree.class).getName()).thenReturn(REP_PRIVILEGES).getMock();
 
+        PrivilegeBits.getInstance(tree);
+        Mockito.verify(tree, times(1)).getProperty(REP_NEXT);
+    }
+
+    @Test
+    public void testCalculatePermissionsFromSimpleBuiltIn() {
+        PrivilegeBitsProvider provider = new PrivilegeBitsProvider(mock(Root.class));
         Map<PrivilegeBits, Long> simple = new HashMap<>();
         simple.put(PrivilegeBits.EMPTY, Permissions.NO_PERMISSION);
-        simple.put(provider.getBits(JCR_READ), Permissions.READ);
-        simple.put(provider.getBits(JCR_LOCK_MANAGEMENT), Permissions.LOCK_MANAGEMENT);
-        simple.put(provider.getBits(JCR_VERSION_MANAGEMENT), Permissions.VERSION_MANAGEMENT);
-        simple.put(provider.getBits(JCR_READ_ACCESS_CONTROL), Permissions.READ_ACCESS_CONTROL);
+        simple.put(PrivilegeBits.BUILT_IN.get(JCR_READ), Permissions.READ);
+        simple.put(PrivilegeBits.BUILT_IN.get(JCR_LOCK_MANAGEMENT), Permissions.LOCK_MANAGEMENT);
+        simple.put(PrivilegeBits.BUILT_IN.get(JCR_VERSION_MANAGEMENT), Permissions.VERSION_MANAGEMENT);
+        simple.put(PrivilegeBits.BUILT_IN.get(JCR_READ_ACCESS_CONTROL), Permissions.READ_ACCESS_CONTROL);
         simple.put(provider.getBits(JCR_MODIFY_ACCESS_CONTROL), Permissions.MODIFY_ACCESS_CONTROL);
         simple.put(provider.getBits(REP_READ_NODES), Permissions.READ_NODE);
         simple.put(provider.getBits(REP_READ_PROPERTIES), Permissions.READ_PROPERTY);
@@ -742,11 +800,30 @@ public class PrivilegeBitsTest implement
         simple.put(provider.getBits(REP_ALTER_PROPERTIES), Permissions.MODIFY_PROPERTY);
         simple.put(provider.getBits(REP_REMOVE_PROPERTIES), Permissions.REMOVE_PROPERTY);
         simple.put(provider.getBits(REP_INDEX_DEFINITION_MANAGEMENT), Permissions.INDEX_DEFINITION_MANAGEMENT);
-        for (PrivilegeBits pb : simple.keySet()) {
-            long expected = simple.get(pb);
+        simple.put(provider.getBits(JCR_NODE_TYPE_DEFINITION_MANAGEMENT), Permissions.NODE_TYPE_DEFINITION_MANAGEMENT);
+        simple.put(provider.getBits(JCR_NAMESPACE_MANAGEMENT), Permissions.NAMESPACE_MANAGEMENT);
+        simple.put(provider.getBits(REP_PRIVILEGE_MANAGEMENT), Permissions.PRIVILEGE_MANAGEMENT);
+        simple.put(provider.getBits(JCR_RETENTION_MANAGEMENT), Permissions.RETENTION_MANAGEMENT);
+        simple.put(provider.getBits(JCR_LIFECYCLE_MANAGEMENT), Permissions.LIFECYCLE_MANAGEMENT);
+        simple.put(provider.getBits(JCR_NODE_TYPE_MANAGEMENT), Permissions.NODE_TYPE_MANAGEMENT);
+        simple.put(provider.getBits(JCR_WORKSPACE_MANAGEMENT), Permissions.WORKSPACE_MANAGEMENT);
+        Map.Entry<PrivilegeBits, Long> previous = null;
+        for (Map.Entry<PrivilegeBits, Long> entry : simple.entrySet()) {
+            long expected = entry.getValue();
+            PrivilegeBits pb = entry.getKey();
             assertEquals(expected, PrivilegeBits.calculatePermissions(pb, PrivilegeBits.EMPTY, true));
-            assertEquals(expected, PrivilegeBits.calculatePermissions(pb, pb, true));
+            assertEquals(expected, PrivilegeBits.calculatePermissions(pb, pb, false));
+            if (previous != null) {
+                assertEquals(expected, PrivilegeBits.calculatePermissions(pb, previous.getKey(), false));
+                assertEquals(expected|previous.getValue(), PrivilegeBits.calculatePermissions(PrivilegeBits.getInstance(pb, previous.getKey()), PrivilegeBits.EMPTY, true));
+            }
+            previous = entry;
         }
+    }
+
+    @Test
+    public void testCalculatePermissionsModifyPropertyAggregated() {
+        PrivilegeBitsProvider provider = new PrivilegeBitsProvider(mock(Root.class));
 
         // jcr:modifyProperty aggregate
         PrivilegeBits add_change = provider.getBits(REP_ADD_PROPERTIES, REP_ALTER_PROPERTIES);
@@ -767,7 +844,7 @@ public class PrivilegeBitsTest implement
 
     @Test
     public void testCalculatePermissionsParentAwareAllow() {
-        PrivilegeBitsProvider provider = new PrivilegeBitsProvider(Mockito.mock(Root.class));
+        PrivilegeBitsProvider provider = new PrivilegeBitsProvider(mock(Root.class));
 
         // parent aware permissions
         // a) jcr:addChildNodes
@@ -792,7 +869,7 @@ public class PrivilegeBitsTest implement
 
     @Test
     public void testCalculatePermissionsParentAwareDeny() {
-        PrivilegeBitsProvider provider = new PrivilegeBitsProvider(Mockito.mock(Root.class));
+        PrivilegeBitsProvider provider = new PrivilegeBitsProvider(mock(Root.class));
 
         // parent aware permissions
         // a) jcr:addChildNodes
@@ -816,11 +893,35 @@ public class PrivilegeBitsTest implement
     }
 
     @Test
+    public void testCalculatePermissionsAddAndRemoveChild() {
+        PrivilegeBits addRemoveChild = PrivilegeBits.getInstance(BUILT_IN.get(JCR_ADD_CHILD_NODES), BUILT_IN.get(JCR_REMOVE_CHILD_NODES));
+        assertEquals(Permissions.MODIFY_CHILD_NODE_COLLECTION, PrivilegeBits.calculatePermissions(addRemoveChild, PrivilegeBits.EMPTY, true));
+        assertEquals(Permissions.MODIFY_CHILD_NODE_COLLECTION, PrivilegeBits.calculatePermissions(addRemoveChild, PrivilegeBits.EMPTY, false));
+    }
+
+    @Test
     public void testEquals() {
         assertEquals(READ_NODES_PRIVILEGE_BITS, READ_NODES_PRIVILEGE_BITS);
         assertEquals(READ_NODES_PRIVILEGE_BITS, PrivilegeBits.getInstance(READ_NODES_PRIVILEGE_BITS).unmodifiable());
 
         assertNotEquals(READ_NODES_PRIVILEGE_BITS, PrivilegeBits.getInstance(READ_NODES_PRIVILEGE_BITS));
         assertNotEquals(PrivilegeBits.getInstance(READ_NODES_PRIVILEGE_BITS), READ_NODES_PRIVILEGE_BITS);
+        assertNotEquals(READ_NODES_PRIVILEGE_BITS, when(mock(Privilege.class).getName()).thenReturn(REP_READ_NODES).getMock());
+        assertNotEquals(READ_NODES_PRIVILEGE_BITS, null);
+    }
+
+    @Test
+    public void testHashCode() {
+        PrivilegeBits pb = READ_NODES_PRIVILEGE_BITS;
+        for (int i = 0; i < 100; i++) {
+            PrivilegeBits modifiable = PrivilegeBits.getInstance(pb);
+            assertEquals(pb.hashCode(), modifiable.unmodifiable().hashCode());
+            assertNotEquals(pb.hashCode(), modifiable.hashCode());
+
+            PrivilegeBits nxt = pb.nextBits();
+            assertNotEquals(pb.hashCode(), nxt.hashCode());
+
+            pb = nxt;
+        }
     }
 }
\ No newline at end of file

Added: jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtilTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtilTest.java?rev=1853436&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtilTest.java (added)
+++ jackrabbit/oak/trunk/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeUtilTest.java Tue Feb 12 13:49:40 2019
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.  You may obtain a copy of the License at
+ *
+ *      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 org.apache.jackrabbit.oak.spi.security.privilege;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_READ;
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.PRIVILEGES_PATH;
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.REP_AGGREGATES;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class PrivilegeUtilTest {
+
+    @Test
+    public void testGetPrivilegesTree() {
+        Root root = when(mock(Root.class).getTree(PRIVILEGES_PATH)).thenReturn(mock(Tree.class)).getMock();
+
+        PrivilegeUtil.getPrivilegesTree(root);
+        verify(root, times(1)).getTree(PRIVILEGES_PATH);
+    }
+
+    @Test
+    public void testReadDefinitions() {
+        Tree defTree = when(mock(Tree.class).getName()).thenReturn("name").getMock();
+        when(defTree.getProperty(PrivilegeConstants.REP_IS_ABSTRACT)).thenReturn(PropertyStates.createProperty(PrivilegeConstants.REP_IS_ABSTRACT, true));
+
+        PrivilegeDefinition def = PrivilegeUtil.readDefinition(defTree);
+        assertEquals("name", def.getName());
+        assertTrue(def.isAbstract());
+        assertTrue(def.getDeclaredAggregateNames().isEmpty());
+    }
+
+    @Test
+    public void testReadDefinitionsWithAggregates() {
+        Iterable<String> aggregateNames = ImmutableSet.of(JCR_READ);
+        Tree defTree = when(mock(Tree.class).getProperty(REP_AGGREGATES)).thenReturn(PropertyStates.createProperty(REP_AGGREGATES, aggregateNames, Type.NAMES)).getMock();
+        when(defTree.getName()).thenReturn("name");
+
+
+        PrivilegeDefinition def = PrivilegeUtil.readDefinition(defTree);
+        assertEquals("name", def.getName());
+        assertTrue(Iterables.elementsEqual(aggregateNames, PrivilegeUtil.readDefinition(defTree).getDeclaredAggregateNames()));
+
+    }
+}
\ No newline at end of file