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 th...@apache.org on 2012/04/05 15:06:42 UTC

svn commit: r1309835 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/mk/index/ main/java/org/apache/jackrabbit/mk/simple/ test/java/org/apache/jackrabbit/mk/ test/java/org/apache/jackrabbit/mk/index/

Author: thomasm
Date: Thu Apr  5 13:06:42 2012
New Revision: 1309835

URL: http://svn.apache.org/viewvc?rev=1309835&view=rev
Log:
OAK-47 Wrong results and NPE with copy operation (support the copy operation in the Indexer and the SimpleKernel)

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/index/Indexer.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/simple/SimpleKernelImpl.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/MoveNodeTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/index/IndexTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/index/Indexer.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/index/Indexer.java?rev=1309835&r1=1309834&r2=1309835&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/index/Indexer.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/index/Indexer.java Thu Apr  5 13:06:42 2012
@@ -299,6 +299,7 @@ public class Indexer {
     /**
      * Update the index with the given changes.
      *
+     * @param rootPath the root path
      * @param t the changes
      * @param lastRevision
      */
@@ -309,6 +310,7 @@ public class Indexer {
                 break;
             }
             String path = PathUtils.concat(rootPath, t.readString());
+            String target;
             switch (r) {
             case '+': {
                 t.read(':');
@@ -327,8 +329,16 @@ public class Indexer {
                 }
                 break;
             }
+            case '*':
+                // TODO support and test copy operation ("*"),
+                // specially in combination with other operations
+                // possibly split up the commit in this case
+                t.read(':');
+                target = t.readString();
+                moveOrCopyNode(path, false, target, lastRevision);
+                break;
             case '-':
-                moveNode(path, null, lastRevision);
+                moveOrCopyNode(path, true, null, lastRevision);
                 break;
             case '^': {
                 removeProperty(path, lastRevision);
@@ -342,9 +352,12 @@ public class Indexer {
                 break;
             }
             case '>':
+                // TODO does move work correctly
+                // in combination with other operations?
+                // possibly split up the commit in this case
                 t.read(':');
                 String name = PathUtils.getName(path);
-                String target, position;
+                String position;
                 if (t.matches('{')) {
                     position = t.readString();
                     t.read(':');
@@ -364,7 +377,7 @@ public class Indexer {
                 } else {
                     throw ExceptionFactory.get("position: " + position);
                 }
-                moveNode(path, target, lastRevision);
+                moveOrCopyNode(path, true, target, lastRevision);
                 break;
             default:
                 throw new AssertionError("token: " + (char) t.getTokenType());
@@ -430,7 +443,7 @@ public class Indexer {
         }
     }
 
-    private void moveNode(String sourcePath, String targetPath, String lastRevision) {
+    private void moveOrCopyNode(String sourcePath, boolean remove, String targetPath, String lastRevision) {
         if (isInIndex(sourcePath)) {
             // don't index the index
             return;
@@ -444,7 +457,9 @@ public class Indexer {
         NodeMap map = new NodeMap();
         t.read('{');
         NodeImpl n = NodeImpl.parse(map, t, 0, sourcePath);
-        addOrRemoveRecursive(n, true, false);
+        if (remove) {
+            addOrRemoveRecursive(n, true, false);
+        }
         if (targetPath != null) {
             t = new JsopTokenizer(node);
             map = new NodeMap();

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/simple/SimpleKernelImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/simple/SimpleKernelImpl.java?rev=1309835&r1=1309834&r2=1309835&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/simple/SimpleKernelImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/simple/SimpleKernelImpl.java Thu Apr  5 13:06:42 2012
@@ -278,18 +278,20 @@ public class SimpleKernelImpl extends Mi
                 break;
             }
             case '*': {
-                // TODO is it really required?
                 // TODO possibly support target position notation
-                // TODO support copy in wrappers, index,...
                 t.read(':');
                 String target = t.readString();
-                diff.tag('*').key(path).value(target);
                 if (!PathUtils.isAbsolute(target)) {
                     target = PathUtils.concat(rootPath, target);
                 }
-                NodeImpl node = data.getNode(from);
+                diff.tag('*').key(path).value(target);
                 String to = PathUtils.relativize("/", target);
-                data = data.cloneAndAddChildNode(to, false, null, node, rev);
+                NodeImpl node = data.getNode(from);
+                JsopStream json = new JsopStream();
+                node.append(json, Integer.MAX_VALUE, 0, Integer.MAX_VALUE, false);
+                json.read('{');
+                NodeImpl n2 = NodeImpl.parse(nodeMap, json, rev);
+                data = data.cloneAndAddChildNode(to, false, null, n2, rev);
                 break;
             }
             default:
@@ -555,6 +557,7 @@ public class SimpleKernelImpl extends Mi
         }
     }
 
+    @Override
     public String toString() {
         return "simple:" + name;
     }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/MoveNodeTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/MoveNodeTest.java?rev=1309835&r1=1309834&r2=1309835&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/MoveNodeTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/MoveNodeTest.java Thu Apr  5 13:06:42 2012
@@ -39,6 +39,7 @@ public class MoveNodeTest extends MultiM
     }
 
     @Before
+    @Override
     public void setUp() throws Exception {
         super.setUp();
         head = mk.getHeadRevision();

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/index/IndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/index/IndexTest.java?rev=1309835&r1=1309834&r2=1309835&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/index/IndexTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/mk/index/IndexTest.java Thu Apr  5 13:06:42 2012
@@ -91,6 +91,30 @@ public class IndexTest extends MultiMkTe
     }
 
     @Test
+    public void copy() {
+        Indexer indexer = new Indexer(mk);
+        PropertyIndex index = indexer.createPropertyIndex("id", false);
+
+        mk.commit("/", "+ \"test\": { \"test2\": { \"id\": 2 }, \"id\": 1 }", mk.getHeadRevision(), "");
+        Assert.assertEquals("/test", index.getPath("1", mk.getHeadRevision()));
+        Assert.assertEquals("/test/test2", index.getPath("2", mk.getHeadRevision()));
+
+        mk.commit("/", "* \"test\": \"copied\"", mk.getHeadRevision(), "");
+        Iterator<String> it = index.getPaths("1", mk.getHeadRevision());
+        Assert.assertTrue(it.hasNext());
+        Assert.assertEquals("/copied", it.next());
+        Assert.assertTrue(it.hasNext());
+        Assert.assertEquals("/test", it.next());
+        Assert.assertFalse(it.hasNext());
+        it = index.getPaths("2", mk.getHeadRevision());
+        Assert.assertTrue(it.hasNext());
+        Assert.assertEquals("/copied/test2", it.next());
+        Assert.assertTrue(it.hasNext());
+        Assert.assertEquals("/test/test2", it.next());
+        Assert.assertFalse(it.hasNext());
+    }
+
+    @Test
     public void ascending() {
         Indexer indexer = new Indexer(mk);
         BTree tree = new BTree(indexer, "test", true);