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 2014/01/30 09:28:17 UTC
svn commit: r1562720 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/core/
main/java/org/apache/jackrabbit/oak/security/authorization/permission/
main/java/org/apache/jackrabbit/oak/spi/commit/
test/java/org/apache/jackrabb...
Author: angela
Date: Thu Jan 30 08:28:17 2014
New Revision: 1562720
URL: http://svn.apache.org/r1562720
Log:
OAK-710 : PermissionValidator: Proper permission evaluation for moving/renaming nodes (wip)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/commit/MoveTracker.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/commit/MoveTrackerTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java?rev=1562720&r1=1562719&r2=1562720&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java Thu Jan 30 08:28:17 2014
@@ -18,18 +18,10 @@
*/
package org.apache.jackrabbit.oak.core;
-import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.collect.Lists.newArrayList;
-import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
-import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
-import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
-import static org.apache.jackrabbit.oak.commons.PathUtils.isAncestor;
-
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
-
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.security.auth.Subject;
@@ -40,7 +32,6 @@ import org.apache.jackrabbit.oak.api.Con
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.QueryEngine;
import org.apache.jackrabbit.oak.api.Root;
-import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.plugins.index.diffindex.UUIDDiffIndexProviderWrapper;
import org.apache.jackrabbit.oak.query.ExecutionContext;
@@ -70,6 +61,13 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStore;
import org.apache.jackrabbit.oak.util.LazyValue;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Lists.newArrayList;
+import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
+import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
+import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
+import static org.apache.jackrabbit.oak.commons.PathUtils.isAncestor;
+
public abstract class AbstractRoot implements Root {
/**
@@ -187,7 +185,6 @@ public abstract class AbstractRoot imple
return false;
}
- Tree.Status status = source.getStatus();
String newName = getName(destPath);
MutableTree newParent = rootTree.getTree(getParentPath(destPath));
if (!newParent.exists() || newParent.hasChild(newName)) {
@@ -201,7 +198,7 @@ public abstract class AbstractRoot imple
lastMove = lastMove.setMove(sourcePath, newParent, newName);
updated();
// remember all move operations for further processing in the commit hooks.
- moveTracker.addMove(sourcePath, destPath, status);
+ moveTracker.addMove(sourcePath, destPath);
}
return success;
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java?rev=1562720&r1=1562719&r2=1562720&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java Thu Jan 30 08:28:17 2014
@@ -127,7 +127,7 @@ public class MoveAwarePermissionValidato
private boolean processAdd(ImmutableTree child, MoveAwarePermissionValidator validator) throws CommitFailedException {
// FIXME: respect and properly handle move-operations in the subtree
- String sourcePath = moveTracker.getOriginalSourcePath(child.getPath());
+ String sourcePath = moveTracker.getSourcePath(child.getPath());
if (sourcePath != null) {
ImmutableTree source = rootBefore.getTree(sourcePath);
if (source.exists()) {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/commit/MoveTracker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/commit/MoveTracker.java?rev=1562720&r1=1562719&r2=1562720&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/commit/MoveTracker.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/commit/MoveTracker.java Thu Jan 30 08:28:17 2014
@@ -25,7 +25,6 @@ import javax.annotation.Nonnull;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
-import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.util.Text;
@@ -46,28 +45,21 @@ public class MoveTracker {
}
public void addMove(@Nonnull String sourcePath, @Nonnull String destPath) {
- addMove(sourcePath, destPath, Tree.Status.EXISTING);
- }
-
-
- public void addMove(@Nonnull String sourcePath, @Nonnull String destPath, Tree.Status status) {
// calculate original source path
String originalSource = sourcePath;
for (MoveEntry me : Lists.reverse(entries)) {
if (Text.isDescendantOrEqual(me.destPath, sourcePath)) {
String relPath = PathUtils.relativize(me.destPath, sourcePath);
if (!relPath.isEmpty()) {
- originalSource = me.originalSourcePath + '/' + relPath;
+ originalSource = me.sourcePath + '/' + relPath;
} else {
- originalSource = me.originalSourcePath;
+ originalSource = me.sourcePath;
}
break;
}
}
- MoveEntry moveEntry = new MoveEntry(sourcePath, destPath, status, originalSource);
- // TODO: clean up entry list
- entries.add(moveEntry);
+ entries.add(new MoveEntry(originalSource, destPath));
}
public boolean isEmpty() {
@@ -76,28 +68,18 @@ public class MoveTracker {
@CheckForNull
public String getSourcePath(String destPath) {
- for (MoveEntry me : entries) {
- if (me.destPath.equals(destPath)) {
- return me.sourcePath;
- }
- }
- return null;
- }
-
- @CheckForNull
- public String getOriginalSourcePath(String destPath) {
for (MoveEntry me : Lists.reverse(entries)) {
if (me.destPath.equals(destPath)) {
- return me.originalSourcePath;
+ return me.sourcePath;
}
}
return null;
}
@CheckForNull
- public String getDestPath(String originalSource) {
+ public String getDestPath(String sourcePath) {
for (MoveEntry me : Lists.reverse(entries)) {
- if (me.originalSourcePath.equals(originalSource)) {
+ if (me.sourcePath.equals(sourcePath)) {
return me.destPath;
}
}
@@ -124,27 +106,19 @@ public class MoveTracker {
private final class MoveEntry {
private final String sourcePath;
- private final String destPath;
- private final Tree.Status status;
-
- private final String originalSourcePath;
-
- private MoveEntry(@Nonnull String sourcePath, @Nonnull String destPath,
- @Nonnull Tree.Status status) {
- this(sourcePath, destPath, status, sourcePath);
- }
+ private String destPath;
- private MoveEntry(@Nonnull String sourcePath, @Nonnull String destPath,
- @Nonnull Tree.Status status,
- @Nonnull String originalSourcePath) {
+ private MoveEntry(@Nonnull String sourcePath,
+ @Nonnull String destPath) {
this.sourcePath = sourcePath;
this.destPath = destPath;
- this.status = status;
- this.originalSourcePath = originalSourcePath;
-
- parentSourcePaths.add(Text.getRelativeParent(originalSourcePath, 1));
+ parentSourcePaths.add(Text.getRelativeParent(sourcePath, 1));
parentDestPaths.add(Text.getRelativeParent(destPath, 1));
}
+
+ private void updateDestPath(@Nonnull String destPath) {
+ this.destPath = destPath;
+ }
}
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/commit/MoveTrackerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/commit/MoveTrackerTest.java?rev=1562720&r1=1562719&r2=1562720&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/commit/MoveTrackerTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/commit/MoveTrackerTest.java Thu Jan 30 08:28:17 2014
@@ -16,18 +16,17 @@
*/
package org.apache.jackrabbit.oak.spi.commit;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
import java.util.LinkedHashMap;
import java.util.Map;
import com.google.common.collect.ImmutableMap;
-import org.apache.jackrabbit.oak.api.Tree;
import org.junit.Ignore;
import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
/**
* MoveTrackerTest... TODO
*/
@@ -49,8 +48,8 @@ public class MoveTrackerTest {
mt.addMove("/a", "/b");
mt.addMove("/c", "/d");
- assertEquals("/a", mt.getOriginalSourcePath("/b"));
- assertEquals("/c", mt.getOriginalSourcePath("/d"));
+ assertEquals("/a", mt.getSourcePath("/b"));
+ assertEquals("/c", mt.getSourcePath("/d"));
assertEquals("/b", mt.getDestPath("/a"));
assertEquals("/d", mt.getDestPath("/c"));
}
@@ -66,8 +65,8 @@ public class MoveTrackerTest {
mt1.addMove("/a/b", "/c");
mt1.addMove("/a", "/d");
- assertEquals("/a/b", mt1.getOriginalSourcePath("/c"));
- assertEquals("/a", mt1.getOriginalSourcePath("/d"));
+ assertEquals("/a/b", mt1.getSourcePath("/c"));
+ assertEquals("/a", mt1.getSourcePath("/d"));
assertEquals("/c", mt1.getDestPath("/a/b"));
assertEquals("/d", mt1.getDestPath("/a"));
}
@@ -83,8 +82,8 @@ public class MoveTrackerTest {
mt2.addMove("/a/b", "/c");
mt2.addMove("/a", "/c/d");
- assertEquals("/a/b", mt2.getOriginalSourcePath("/c"));
- assertEquals("/a", mt2.getOriginalSourcePath("/c/d"));
+ assertEquals("/a/b", mt2.getSourcePath("/c"));
+ assertEquals("/a", mt2.getSourcePath("/c/d"));
assertEquals("/c", mt2.getDestPath("/a/b"));
assertEquals("/c/d", mt2.getDestPath("/a"));
}
@@ -100,8 +99,8 @@ public class MoveTrackerTest {
mt3.addMove("/a", "/b");
mt3.addMove("/c", "/a");
- assertEquals("/a", mt3.getOriginalSourcePath("/b"));
- assertEquals("/c", mt3.getOriginalSourcePath("/a"));
+ assertEquals("/a", mt3.getSourcePath("/b"));
+ assertEquals("/c", mt3.getSourcePath("/a"));
assertEquals("/b", mt3.getDestPath("/a"));
assertEquals("/a", mt3.getDestPath("/c"));
}
@@ -117,8 +116,8 @@ public class MoveTrackerTest {
mt.addMove("/a", "/b");
mt.addMove("/b/c", "/d");
- assertEquals("/a", mt.getOriginalSourcePath("/b"));
- assertEquals("/a/c", mt.getOriginalSourcePath("/d"));
+ assertEquals("/a", mt.getSourcePath("/b"));
+ assertEquals("/a/c", mt.getSourcePath("/d"));
assertEquals("/b", mt.getDestPath("/a"));
assertEquals("/d", mt.getDestPath("/a/c"));
}
@@ -134,7 +133,7 @@ public class MoveTrackerTest {
mt.addMove("/a", "/b");
mt.addMove("/b", "/c");
- assertEquals("/a", mt.getOriginalSourcePath("/c"));
+ assertEquals("/a", mt.getSourcePath("/c"));
assertEquals("/c", mt.getDestPath("/a"));
}
@@ -150,8 +149,8 @@ public class MoveTrackerTest {
mt4.addMove("/a", "/b/c");
mt4.addMove("/b", "/d");
- assertEquals("/a", mt4.getOriginalSourcePath("/d/c"));
- assertEquals("/b", mt4.getOriginalSourcePath("/d"));
+ assertEquals("/a", mt4.getSourcePath("/d/c"));
+ assertEquals("/b", mt4.getSourcePath("/d"));
assertEquals("/d/c", mt4.getDestPath("/a"));
assertEquals("/d", mt4.getDestPath("/b"));
}
@@ -167,8 +166,8 @@ public class MoveTrackerTest {
mt5.addMove("/a", "/b");
mt5.addMove("/c", "/b/d");
- assertEquals("/a", mt5.getOriginalSourcePath("/b"));
- assertEquals("/c", mt5.getOriginalSourcePath("/b/d"));
+ assertEquals("/a", mt5.getSourcePath("/b"));
+ assertEquals("/c", mt5.getSourcePath("/b/d"));
assertEquals("/b", mt5.getDestPath("/a"));
assertEquals("/b/d", mt5.getDestPath("/c"));
}
@@ -184,19 +183,17 @@ public class MoveTrackerTest {
mt6.addMove("/a/b", "/b");
mt6.addMove("/a", "/b/d");
- assertEquals("/a/b", mt6.getOriginalSourcePath("/b"));
- assertEquals("/a", mt6.getOriginalSourcePath("/b/d"));
+ assertEquals("/a/b", mt6.getSourcePath("/b"));
+ assertEquals("/a", mt6.getSourcePath("/b/d"));
assertEquals("/b", mt6.getDestPath("/a/b"));
assertEquals("/b/d", mt6.getDestPath("/a"));
}
@Test
public void testNotEmpty() {
- for (Tree.Status st : Tree.Status.values()) {
- MoveTracker mt = new MoveTracker();
- mt.addMove("/a/b/c", "/d/e/f", st);
- assertFalse(mt.isEmpty());
- }
+ MoveTracker mt = new MoveTracker();
+ mt.addMove("/a/b/c", "/d/e/f");
+ assertFalse(mt.isEmpty());
}
@Test
@@ -211,7 +208,7 @@ public class MoveTrackerTest {
String dest = moves.get(src);
mt.addMove(src, dest);
- assertEquals(src, mt.getOriginalSourcePath(dest));
+ assertEquals(src, mt.getSourcePath(dest));
assertEquals(dest, mt.getDestPath(src));
}
}
@@ -231,7 +228,7 @@ public class MoveTrackerTest {
for (String src : m.keySet()) {
String dest = m.get(src);
- assertEquals(src, mt.getOriginalSourcePath(dest));
+ assertEquals(src, mt.getSourcePath(dest));
assertEquals(dest, mt.getDestPath(src));
}
}
@@ -345,20 +342,11 @@ public class MoveTrackerTest {
return this;
}
- MoveTest addMove(String src, String dst, String originalSrc, Tree.Status status) {
- src2dest.put(src, dst);
- dest2orig.put(dst, originalSrc);
- orig2dest.put(originalSrc, dst);
-
- mt.addMove(src, dst, status);
- return this;
- }
-
void assertResult() {
// map destination -> original path
for (String dest : src2dest.values()) {
String expectedOrgSource = dest2orig.get(dest);
- assertEquals(expectedOrgSource, mt.getOriginalSourcePath(dest));
+ assertEquals(expectedOrgSource, mt.getSourcePath(dest));
}
// map original path -> destination