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 2020/09/25 08:53:16 UTC

svn commit: r1882008 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/security/authorization/permission/ test/java/org/apache/jackrabbit/oak/security/authorization/permission/

Author: angela
Date: Fri Sep 25 08:53:16 2020
New Revision: 1882008

URL: http://svn.apache.org/viewvc?rev=1882008&view=rev
Log:
OAK-9233 : Simplify ChildOrderDiff

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiffTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java?rev=1882008&r1=1882007&r2=1882008&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java Fri Sep 25 08:53:16 2020
@@ -18,6 +18,8 @@ package org.apache.jackrabbit.oak.securi
 
 import java.util.Iterator;
 import java.util.Set;
+
+import com.google.common.collect.Iterables;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.plugins.tree.TreeConstants;
@@ -41,38 +43,19 @@ final class ChildOrderDiff {
      *
      * @param before
      * @param after
-     * @return the name of the first reordered child if any user-supplied node
-     * reorder happened; {@code null} otherwise.
+     * @return {@code true} if any user-supplied node
+     * reorder happened; {@code false} otherwise.
      */
-    @Nullable
-    static String firstReordered(@NotNull PropertyState before, @NotNull PropertyState after) {
+    static boolean isReordered(@NotNull PropertyState before, @NotNull PropertyState after) {
         Set<String> afterNames = newLinkedHashSet(after.getValue(Type.NAMES));
         Set<String> beforeNames = newLinkedHashSet(before.getValue(Type.NAMES));
 
-        Iterator<String> a = afterNames.iterator();
-        Iterator<String> b = beforeNames.iterator();
-        while (a.hasNext() && b.hasNext()) {
-            String aName = a.next();
-            String bName = b.next();
-            while (!aName.equals(bName)) {
-                if (!beforeNames.contains(aName)) {
-                    if (a.hasNext()) {
-                        aName = a.next();
-                    } else {
-                        return null;
-                    }
-                } else if (!afterNames.contains(bName)) {
-                    if (b.hasNext()) {
-                        bName = b.next();
-                    } else {
-                        return null;
-                    }
-                } else {
-                    return aName;
-                }
-            }
-        }
+        // drop all newly added values from 'afterNames'
+        afterNames.retainAll(beforeNames);
+        // drop all removed values from 'beforeNames'
+        beforeNames.retainAll(afterNames);
 
-        return null;
+        // names got reordered if the elements in the 2 intersections aren't equal
+        return !Iterables.elementsEqual(afterNames, beforeNames);
     }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java?rev=1882008&r1=1882007&r2=1882008&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java Fri Sep 25 08:53:16 2020
@@ -113,8 +113,7 @@ class PermissionValidator extends Defaul
     public void propertyChanged(PropertyState before, PropertyState after) throws CommitFailedException {
         String name = after.getName();
         if (TreeConstants.OAK_CHILD_ORDER.equals(name)) {
-            String childName = ChildOrderDiff.firstReordered(before, after);
-            if (childName != null) {
+            if (ChildOrderDiff.isReordered(before, after)) {
                 checkPermissions(parentAfter, false, Permissions.MODIFY_CHILD_NODE_COLLECTION);
             } // else: no re-order but only internal update
         } else if (isImmutableProperty(name, parentAfter)) {

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiffTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiffTest.java?rev=1882008&r1=1882007&r2=1882008&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiffTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiffTest.java Fri Sep 25 08:53:16 2020
@@ -23,8 +23,8 @@ import org.apache.jackrabbit.oak.plugins
 import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 public class ChildOrderDiffTest {
 
@@ -37,76 +37,76 @@ public class ChildOrderDiffTest {
     public void testBeforeEmptyAfterEmpty() {
         PropertyState before = createPropertyState();
         PropertyState after = createPropertyState();
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testBeforeEmpty() {
         PropertyState before = createPropertyState();
         PropertyState after = createPropertyState("n1", "n2");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testAfterEmpty() {
         PropertyState before = createPropertyState("n1", "n2");
         PropertyState after = createPropertyState();
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
         public void testAfterEqualsBefore() {
         PropertyState eq = createPropertyState("n1", "n2");
-        assertNull(ChildOrderDiff.firstReordered(eq, eq));
+        assertFalse(ChildOrderDiff.isReordered(eq, eq));
     }
 
     @Test
     public void testAppendedAtEnd() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n1", "n2", "n3", "n4");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testInsertedAtBeginning() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n0", "n1", "n2", "n3");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testInserted() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n1", "n11", "n2", "n3");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testLastReplaced() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n1", "n2", "n4");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testFirstRemoved() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n2", "n3");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testSecondRemoved() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n1", "n3");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testLastRemoved() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n1", "n2");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
 
@@ -114,55 +114,55 @@ public class ChildOrderDiffTest {
     public void testReorderedFirstToEnd() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n2", "n3", "n1");
-        assertEquals("n2", ChildOrderDiff.firstReordered(before, after));
+        assertTrue(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testReorderedLastBeforeSecond() {
         PropertyState before = createPropertyState("n1", "n2", "n3");
         PropertyState after = createPropertyState("n1", "n3", "n2");
-        assertEquals("n3", ChildOrderDiff.firstReordered(before, after));
+        assertTrue(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testRemovedAndReordered() {
         PropertyState before = createPropertyState("n1", "n2", "n3", "n4");
         PropertyState after = createPropertyState("n1", "n4", "n3");
-        assertEquals("n4", ChildOrderDiff.firstReordered(before, after));
+        assertTrue(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testInsertedRemovedAndReordered() {
         PropertyState before = createPropertyState("n1", "n2", "n3", "n4");
         PropertyState after = createPropertyState("n1", "n11", "n4", "n3");
-        assertEquals("n4", ChildOrderDiff.firstReordered(before, after));
+        assertTrue(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testRemovedAndReorderedAppended() {
         PropertyState before = createPropertyState("n1", "n2", "n3", "n4");
         PropertyState after = createPropertyState("n1", "n4", "n3", "n33");
-        assertEquals("n4", ChildOrderDiff.firstReordered(before, after));
+        assertTrue(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testReorderedAndReplaced() {
         PropertyState before = createPropertyState("n1", "n2", "n3", "n4");
         PropertyState after = createPropertyState("n4", "n1", "n6");
-        assertEquals("n4", ChildOrderDiff.firstReordered(before, after));
+        assertTrue(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testOnlyLastEquals() {
         PropertyState before = createPropertyState("n1", "n2");
         PropertyState after = createPropertyState("n5", "n6", "n7", "n2");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 
     @Test
     public void testAllDifferent() {
         PropertyState before = createPropertyState("n1", "n2", "n3", "n4");
         PropertyState after = createPropertyState("n5", "n6", "n7", "n8", "n9");
-        assertNull(ChildOrderDiff.firstReordered(before, after));
+        assertFalse(ChildOrderDiff.isReordered(before, after));
     }
 }
\ No newline at end of file