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 st...@apache.org on 2013/09/04 17:08:57 UTC

svn commit: r1520046 - in /jackrabbit/oak/trunk/oak-mk/src: main/java/org/apache/jackrabbit/mk/core/ main/java/org/apache/jackrabbit/mk/model/ main/java/org/apache/jackrabbit/mk/model/tree/ main/java/org/apache/jackrabbit/mk/store/ test/java/org/apache...

Author: stefan
Date: Wed Sep  4 15:08:56 2013
New Revision: 1520046

URL: http://svn.apache.org/r1520046
Log:
OAK-997: cleanup codebase: remove unneeded internal abstraction

Added:
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/DiffBuilder.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java
      - copied, changed from r1519581, jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/NodeDelta.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java
Removed:
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/AbstractRevisionStore.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java
Modified:
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/MicroKernelImpl.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/Repository.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/Id.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StagedNodeTree.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java
    jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/MicroKernelImplTest.java

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/MicroKernelImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/MicroKernelImpl.java?rev=1520046&r1=1520045&r2=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/MicroKernelImpl.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/MicroKernelImpl.java Wed Sep  4 15:08:56 2013
@@ -18,9 +18,9 @@ package org.apache.jackrabbit.mk.core;
 
 import java.io.InputStream;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
-
-import javax.annotation.Nonnull;
+import java.util.Map;
 
 import org.apache.jackrabbit.mk.api.MicroKernel;
 import org.apache.jackrabbit.mk.api.MicroKernelException;
@@ -28,29 +28,25 @@ import org.apache.jackrabbit.mk.json.Jso
 import org.apache.jackrabbit.mk.json.JsopBuilder;
 import org.apache.jackrabbit.mk.json.JsopReader;
 import org.apache.jackrabbit.mk.json.JsopTokenizer;
+import org.apache.jackrabbit.mk.model.ChildNodeEntry;
 import org.apache.jackrabbit.mk.model.Commit;
 import org.apache.jackrabbit.mk.model.CommitBuilder;
+import org.apache.jackrabbit.mk.model.DiffBuilder;
 import org.apache.jackrabbit.mk.model.Id;
 import org.apache.jackrabbit.mk.model.StoredCommit;
-import org.apache.jackrabbit.mk.model.tree.ChildNode;
-import org.apache.jackrabbit.mk.model.tree.DiffBuilder;
-import org.apache.jackrabbit.mk.model.tree.NodeState;
-import org.apache.jackrabbit.mk.model.tree.PropertyState;
+import org.apache.jackrabbit.mk.model.StoredNode;
+import org.apache.jackrabbit.mk.store.NotFoundException;
 import org.apache.jackrabbit.mk.store.RevisionStore;
 import org.apache.jackrabbit.mk.util.CommitGate;
 import org.apache.jackrabbit.mk.util.NameFilter;
 import org.apache.jackrabbit.mk.util.NodeFilter;
 import org.apache.jackrabbit.oak.commons.PathUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  *
  */
 public class MicroKernelImpl implements MicroKernel {
 
-    private static final Logger LOG = LoggerFactory.getLogger(MicroKernelImpl.class);
-
     protected Repository rep;
     private final CommitGate gate = new CommitGate();
 
@@ -113,7 +109,6 @@ public class MicroKernelImpl implements 
         return getHeadRevisionId().toString();
     }
 
-    @Override @Nonnull
     public String checkpoint(long lifetime) throws MicroKernelException {
         // FIXME: need to signal to the garbage collector that this revision
         // should not be collected until the requested lifetime is over
@@ -159,9 +154,10 @@ public class MicroKernelImpl implements 
                     && commit.getCommitTS() >= since) {
                 if (filtered) {
                     try {
+                        RevisionStore rs = rep.getRevisionStore();
                         String diff = new DiffBuilder(
-                                rep.getNodeState(commit.getParentId(), "/"),
-                                rep.getNodeState(commit.getId(), "/"),
+                                rs.getRootNode(commit.getParentId()),
+                                rs.getNode(commit.getRootNodeId()),
                                 "/", -1, rep.getRevisionStore(), path).build();
                         if (!diff.isEmpty()) {
                             history.add(commit);
@@ -270,9 +266,10 @@ public class MicroKernelImpl implements 
             String diff = commit.getChanges();
             if (filtered) {
                 try {
+                    RevisionStore rs = rep.getRevisionStore();
                     diff = new DiffBuilder(
-                            rep.getNodeState(commit.getParentId(), "/"),
-                            rep.getNodeState(commit.getId(), "/"),
+                            rs.getRootNode(commit.getParentId()),
+                            rs.getNode(commit.getRootNodeId()),
                             "/", -1, rep.getRevisionStore(), path).build();
                     if (diff.isEmpty()) {
                         continue;
@@ -323,10 +320,17 @@ public class MicroKernelImpl implements 
                     return toCommit.getChanges();
                 }
             }
-            NodeState before = rep.getNodeState(fromRevisionId, path);
-            NodeState after = rep.getNodeState(toRevisionId, path);
 
-            return new DiffBuilder(before, after, path, depth, rep.getRevisionStore(), path).build();
+            StoredNode from = null, to = null;
+            try {
+                from = rep.getNode(fromRevisionId, path);
+            } catch (NotFoundException ignore) {
+            }
+            try {
+                to = rep.getNode(toRevisionId, path);
+            } catch (NotFoundException ignore) {
+            }
+            return new DiffBuilder(from, to, path, depth, rep.getRevisionStore(), path).build();
         } catch (Exception e) {
             throw new MicroKernelException(e);
         }
@@ -352,17 +356,13 @@ public class MicroKernelImpl implements 
 
         Id revId = revisionId == null ? getHeadRevisionId() : Id.fromString(revisionId);
 
-        NodeState nodeState;
         try {
-            nodeState = rep.getNodeState(revId, path);
+            return rep.getNode(revId, path).getChildNodeCount();
+        } catch (NotFoundException e) {
+            throw new MicroKernelException("Path " + path + " not found in revision " + revisionId);
         } catch (Exception e) {
             throw new MicroKernelException(e);
         }
-        if (nodeState != null) {
-            return nodeState.getChildNodeCount();
-        } else {
-            throw new MicroKernelException("Path " + path + " not found in revision " + revisionId);
-        }
     }
 
     public String getNodes(String path, String revisionId, int depth, long offset, int maxChildNodes, String filter) throws MicroKernelException {
@@ -384,19 +384,19 @@ public class MicroKernelImpl implements 
         }
 
         try {
-            NodeState nodeState;
-            if (id != null) {
-                RevisionStore rs = rep.getRevisionStore();
-                nodeState = rs.getNodeState(rs.getNode(id));
-            } else {
-                nodeState = rep.getNodeState(revId, path);
-            }
-            if (nodeState == null) {
+            StoredNode node;
+            try {
+                if (id != null) {
+                    node = rep.getRevisionStore().getNode(id);
+                } else {
+                    node = rep.getNode(revId, path);
+                }
+            } catch (NotFoundException e) {
                 return null;
             }
 
             JsopBuilder buf = new JsopBuilder().object();
-            toJson(buf, nodeState, depth, (int) offset, maxChildNodes, true, nodeFilter);
+            toJson(buf, node, depth, (int) offset, maxChildNodes, true, nodeFilter);
             return buf.endObject().toString();
         } catch (Exception e) {
             throw new MicroKernelException(e);
@@ -558,7 +558,6 @@ public class MicroKernelImpl implements 
         }
     }
 
-    @Override
     public String rebase(String branchRevisionId, String newBaseRevisionId) {
         Id branchId = Id.fromString(branchRevisionId);
         Id baseId = getBaseRevisionId(branchId);
@@ -615,12 +614,12 @@ public class MicroKernelImpl implements 
 
     //-------------------------------------------------------< implementation >
 
-    void toJson(JsopBuilder builder, NodeState node,
+    void toJson(JsopBuilder builder, StoredNode node,
                 int depth, int offset, int maxChildNodes,
-                boolean inclVirtualProps, NodeFilter filter) {
-        for (PropertyState property : node.getProperties()) {
-            if (filter == null || filter.includeProperty(property.getName())) {
-                builder.key(property.getName()).encodedValue(property.getEncodedValue());
+                boolean inclVirtualProps, NodeFilter filter) throws Exception {
+        for (Map.Entry<String, String> prop : node.getProperties().entrySet()) {
+            if (filter == null || filter.includeProperty(prop.getKey())) {
+                builder.key(prop.getKey()).encodedValue(prop.getValue());
             }
         }
         long childCount = node.getChildNodeCount();
@@ -636,12 +635,12 @@ public class MicroKernelImpl implements 
                     // check whether :id has been explicitly included
                     if (nf.getInclusionPatterns().contains(":hash")
                             && !nf.getExclusionPatterns().contains(":hash")) {
-                        builder.key(":hash").value(rep.getRevisionStore().getId(node).toString());
+                        builder.key(":hash").value(node.getId().toString());
                     }
                     // check whether :id has been explicitly included
                     if (nf.getInclusionPatterns().contains(":id")
                             && !nf.getExclusionPatterns().contains(":id")) {
-                        builder.key(":id").value(rep.getRevisionStore().getId(node).toString());
+                        builder.key(":id").value(node.getId().toString());
                     }
                 }
             }
@@ -655,8 +654,8 @@ public class MicroKernelImpl implements 
                     // does not include wildcards
                     int count = maxChildNodes == -1 ? Integer.MAX_VALUE : maxChildNodes;
                     for (String name : childFilter.getInclusionPatterns()) {
-                        NodeState child = node.getChildNode(name);
-                        if (child != null) {
+                        ChildNodeEntry cne = node.getChildNodeEntry(name);
+                        if (cne != null) {
                             boolean incl = true;
                             for (String exclName : childFilter.getExclusionPatterns()) {
                                 if (name.equals(exclName)) {
@@ -670,7 +669,7 @@ public class MicroKernelImpl implements 
                                 }
                                 builder.key(name).object();
                                 if (depth > 0) {
-                                    toJson(builder, child, depth - 1, 0, maxChildNodes, inclVirtualProps, filter);
+                                    toJson(builder, rep.getRevisionStore().getNode(cne.getId()), depth - 1, 0, maxChildNodes, inclVirtualProps, filter);
                                 }
                                 builder.endObject();
                             }
@@ -688,14 +687,16 @@ public class MicroKernelImpl implements 
                 count = -1;
             }
             int numSiblings = 0;
-            for (ChildNode entry : node.getChildNodeEntries(offset, count)) {
-                if (filter == null || filter.includeNode(entry.getName())) {
+
+            for (Iterator<ChildNodeEntry> it = node.getChildNodeEntries(offset, count); it.hasNext(); ) {
+                ChildNodeEntry cne = it.next();
+                if (filter == null || filter.includeNode(cne.getName())) {
                     if (maxChildNodes != -1 && ++numSiblings > maxChildNodes) {
                         break;
                     }
-                    builder.key(entry.getName()).object();
+                    builder.key(cne.getName()).object();
                     if (depth > 0) {
-                        toJson(builder, entry.getNode(), depth - 1, 0, maxChildNodes, inclVirtualProps, filter);
+                        toJson(builder, rep.getRevisionStore().getNode(cne.getId()), depth - 1, 0, maxChildNodes, inclVirtualProps, filter);
                     }
                     builder.endObject();
                 }

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/Repository.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/Repository.java?rev=1520046&r1=1520045&r2=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/Repository.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/core/Repository.java Wed Sep  4 15:08:56 2013
@@ -22,10 +22,11 @@ import java.io.File;
 import org.apache.jackrabbit.mk.blobs.BlobStore;
 import org.apache.jackrabbit.mk.blobs.FileBlobStore;
 import org.apache.jackrabbit.mk.blobs.MemoryBlobStore;
+import org.apache.jackrabbit.mk.model.ChildNodeEntry;
 import org.apache.jackrabbit.mk.model.CommitBuilder;
 import org.apache.jackrabbit.mk.model.Id;
 import org.apache.jackrabbit.mk.model.StoredCommit;
-import org.apache.jackrabbit.mk.model.tree.NodeState;
+import org.apache.jackrabbit.mk.model.StoredNode;
 import org.apache.jackrabbit.mk.persistence.H2Persistence;
 import org.apache.jackrabbit.mk.persistence.InMemPersistence;
 import org.apache.jackrabbit.mk.store.DefaultRevisionStore;
@@ -166,19 +167,20 @@ public class Repository {
         return rs.getCommit(id);
     }
 
-    public NodeState getNodeState(Id revId, String path) throws Exception {
+    public StoredNode getNode(Id revId, String path) throws NotFoundException, Exception {
         if (!initialized) {
             throw new IllegalStateException("not initialized");
         } else if (!PathUtils.isAbsolute(path)) {
             throw new IllegalArgumentException("illegal path");
         }
 
-        NodeState node = rs.getNodeState(rs.getRootNode(revId));
+        StoredNode node = rs.getRootNode(revId);
         for (String name : PathUtils.elements(path)) {
-            node = node.getChildNode(name);
-            if (node == null) {
-                break;
+            ChildNodeEntry cne = node.getChildNodeEntry(name);
+            if (cne == null) {
+                throw new NotFoundException();
             }
+            node = rs.getNode(cne.getId()) ;
         }
         return node;
     }
@@ -190,12 +192,13 @@ public class Repository {
             throw new IllegalArgumentException("illegal path");
         }
 
-        NodeState node = rs.getNodeState(rs.getRootNode(revId));
+        StoredNode node = rs.getRootNode(revId);
         for (String name : PathUtils.elements(path)) {
-            node = node.getChildNode(name);
-            if (node == null) {
+            ChildNodeEntry cne = node.getChildNodeEntry(name);
+            if (cne == null) {
                 return false;
             }
+            node = rs.getNode(cne.getId()) ;
         }
         return true;
     }

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java?rev=1520046&r1=1520045&r2=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java Wed Sep  4 15:08:56 2013
@@ -21,7 +21,6 @@ import java.util.List;
 
 import org.apache.jackrabbit.mk.json.JsonObject;
 import org.apache.jackrabbit.mk.json.JsopBuilder;
-import org.apache.jackrabbit.mk.model.tree.DiffBuilder;
 import org.apache.jackrabbit.mk.store.NotFoundException;
 import org.apache.jackrabbit.mk.store.RevisionStore;
 import org.apache.jackrabbit.oak.commons.PathUtils;
@@ -212,8 +211,8 @@ public class CommitBuilder {
         newCommit.setMsg(msg);
         // dynamically build diff for rebased commit
         String diff = new DiffBuilder(
-                store.getNodeState(store.getRootNode(toId)),
-                store.getNodeState(store.getNode(rebasedId)),
+                store.getRootNode(toId),
+                store.getNode(rebasedId),
                 "/", -1, store, "").build();
         newCommit.setChanges(diff);
         newCommit.setRootNodeId(rebasedId);
@@ -259,8 +258,8 @@ public class CommitBuilder {
             newCommit.setMsg(msg);
             // dynamically build diff of merged commit
             String diff = new DiffBuilder(
-                    store.getNodeState(store.getRootNode(currentHead)),
-                    store.getNodeState(store.getNode(rootNodeId)),
+                    store.getRootNode(currentHead),
+                    store.getNode(rootNodeId),
                     "/", -1, store, "").build();
             if (diff.isEmpty()) {
                 LOG.debug("merge of empty branch {} with differing content hashes encountered, ignore and keep current head {}",

Added: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/DiffBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/DiffBuilder.java?rev=1520046&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/DiffBuilder.java (added)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/DiffBuilder.java Wed Sep  4 15:08:56 2013
@@ -0,0 +1,228 @@
+/*
+ * 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.mk.model;
+
+import org.apache.jackrabbit.mk.json.JsopBuilder;
+import org.apache.jackrabbit.mk.store.RevisionProvider;
+import org.apache.jackrabbit.oak.commons.PathUtils;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+
+/**
+ * JSOP Diff Builder
+ */
+public class DiffBuilder {
+
+    private final Node before;
+    private final Node after;
+    private final String path;
+    private final int depth;
+    private final String pathFilter;
+    private final RevisionProvider rp;
+
+    public DiffBuilder(Node before, Node after, String path, int depth,
+                       RevisionProvider rp, String pathFilter) {
+        this.before = before;
+        this.after = after;
+        this.path = path;
+        this.depth = depth;
+        this.rp = rp;
+        this.pathFilter = (pathFilter == null || "".equals(pathFilter)) ? "/" : pathFilter;
+    }
+
+    public String build() throws Exception {
+        final JsopBuilder buff = new JsopBuilder();
+
+        // maps (key: id of target node, value: list of paths to the target)
+        // for tracking added/removed nodes; this allows us
+        // to detect 'move' operations
+
+        final HashMap<Id, ArrayList<String>> addedNodes = new HashMap<Id, ArrayList<String>>();
+        final HashMap<Id, ArrayList<String>> removedNodes = new HashMap<Id, ArrayList<String>>();
+
+        if (!PathUtils.isAncestor(path, pathFilter)
+                && !path.startsWith(pathFilter)) {
+            return "";
+        }
+
+        if (before == null) {
+            if (after != null) {
+                buff.tag('+').key(path).object();
+                toJson(buff, after, depth);
+                return buff.endObject().newline().toString();
+            } else {
+                // path doesn't exist in the specified revisions
+                return "";
+            }
+        } else if (after == null) {
+            buff.tag('-');
+            buff.value(path);
+            return buff.newline().toString();
+        }
+
+        TraversingNodeDiffHandler diffHandler = new TraversingNodeDiffHandler(rp) {
+            int levels = depth < 0 ? Integer.MAX_VALUE : depth;
+            @Override
+            public void propAdded(String propName, String value) {
+                String p = PathUtils.concat(getCurrentPath(), propName);
+                if (p.startsWith(pathFilter)) {
+                    buff.tag('^').
+                            key(p).
+                            encodedValue(value).
+                            newline();
+                }
+            }
+
+            @Override
+            public void propChanged(String propName, String oldValue, String newValue) {
+                String p = PathUtils.concat(getCurrentPath(), propName);
+                if (p.startsWith(pathFilter)) {
+                    buff.tag('^').
+                            key(p).
+                            encodedValue(newValue).
+                            newline();
+                }
+            }
+
+            @Override
+            public void propDeleted(String propName, String value) {
+                String p = PathUtils.concat(getCurrentPath(), propName);
+                if (p.startsWith(pathFilter)) {
+                    // since property and node deletions can't be distinguished
+                    // using the "- <path>" notation we're representing
+                    // property deletions as "^ <path>:null"
+                    buff.tag('^').
+                            key(p).
+                            value(null).
+                            newline();
+                }
+            }
+
+            @Override
+            public void childNodeAdded(ChildNodeEntry added) {
+                String p = PathUtils.concat(getCurrentPath(), added.getName());
+                if (p.startsWith(pathFilter)) {
+                    ArrayList<String> removedPaths = removedNodes.get(added.getId());
+                    if (removedPaths != null) {
+                        // move detected
+                        String removedPath = removedPaths.remove(0);
+                        if (removedPaths.isEmpty()) {
+                            removedNodes.remove(added.getId());
+                        }
+                        buff.tag('>').
+                                // path/to/deleted/node
+                                key(removedPath).
+                                // path/to/added/node
+                                value(p).
+                                newline();
+                    } else {
+                        ArrayList<String> addedPaths = addedNodes.get(added.getId());
+                        if (addedPaths == null) {
+                            addedPaths = new ArrayList<String>();
+                            addedNodes.put(added.getId(), addedPaths);
+                        }
+                        addedPaths.add(p);
+                    }
+                }
+            }
+
+            @Override
+            public void childNodeDeleted(ChildNodeEntry deleted) {
+                String p = PathUtils.concat(getCurrentPath(), deleted.getName());
+                if (p.startsWith(pathFilter)) {
+                    ArrayList<String> addedPaths = addedNodes.get(deleted.getId());
+                    if (addedPaths != null) {
+                        // move detected
+                        String addedPath = addedPaths.remove(0);
+                        if (addedPaths.isEmpty()) {
+                            addedNodes.remove(deleted.getId());
+                        }
+                        buff.tag('>').
+                                // path/to/deleted/node
+                                key(p).
+                                // path/to/added/node
+                                value(addedPath).
+                                newline();
+                    } else {
+                        ArrayList<String> removedPaths = removedNodes.get(deleted.getId());
+                        if (removedPaths == null) {
+                            removedPaths = new ArrayList<String>();
+                            removedNodes.put(deleted.getId(), removedPaths);
+                        }
+                        removedPaths.add(p);
+                    }
+                }
+            }
+
+            @Override
+            public void childNodeChanged(ChildNodeEntry changed, Id newId) {
+                String p = PathUtils.concat(getCurrentPath(), changed.getName());
+                if (PathUtils.isAncestor(p, pathFilter)
+                        || p.startsWith(pathFilter)) {
+                    --levels;
+                    if (levels >= 0) {
+                        // recurse
+                        super.childNodeChanged(changed, newId);
+                    } else {
+                        buff.tag('^');
+                        buff.key(p);
+                        buff.object().endObject();
+                        buff.newline();
+                    }
+                    ++levels;
+                }
+            }
+        };
+        diffHandler.start(before, after, path);
+
+        // finally process remaining added nodes ...
+        for (Map.Entry<Id, ArrayList<String>> entry : addedNodes.entrySet()) {
+            for (String p : entry.getValue()) {
+                buff.tag('+').
+                        key(p).object();
+                toJson(buff, rp.getNode(entry.getKey()), depth);
+                buff.endObject().newline();
+            }
+        }
+        //  ... and removed nodes
+        for (Map.Entry<Id, ArrayList<String>> entry : removedNodes.entrySet()) {
+            for (String p : entry.getValue()) {
+                buff.tag('-');
+                buff.value(p);
+                buff.newline();
+            }
+        }
+        return buff.toString();
+    }
+
+    private void toJson(JsopBuilder builder, Node node, int depth) throws Exception {
+        for (Map.Entry<String, String> entry : node.getProperties().entrySet()) {
+            builder.key(entry.getKey()).encodedValue(entry.getValue());
+        }
+        if (depth != 0) {
+            for (Iterator<ChildNodeEntry> it = node.getChildNodeEntries(0, -1); it.hasNext(); ) {
+                ChildNodeEntry entry = it.next();
+                builder.key(entry.getName()).object();
+                toJson(builder, rp.getNode(entry.getId()), depth < 0 ? depth : depth - 1);
+                builder.endObject();
+            }
+        }
+    }
+}

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/Id.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/Id.java?rev=1520046&r1=1520045&r2=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/Id.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/Id.java Wed Sep  4 15:08:56 2013
@@ -72,7 +72,7 @@ public class Id implements Comparable<Id
     /**
      * Creates an {@code Id} instance from a long.
      *
-     * @param l a long
+     * @param value a long
      * @return an {@code Id} instance
      */
     public static Id fromLong(long value) {

Copied: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java (from r1519581, jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/NodeDelta.java)
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java?p2=jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java&p1=jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/NodeDelta.java&r1=1519581&r2=1520046&rev=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/NodeDelta.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java Wed Sep  4 15:08:56 2013
@@ -14,10 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.jackrabbit.mk.model.tree;
-
-import org.apache.jackrabbit.mk.model.Id;
-import org.apache.jackrabbit.mk.store.RevisionProvider;
+package org.apache.jackrabbit.mk.model;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -27,7 +24,7 @@ import java.util.Map;
 /**
  *
  */
-public class NodeDelta {
+public class NodeDelta implements NodeDiffHandler {
 
     public static enum ConflictType {
         /**
@@ -50,9 +47,8 @@ public class NodeDelta {
         REMOVED_DIRTY_NODE_CONFLICT
     }
 
-    private final RevisionProvider provider;
-
-    private final NodeState node1;
+    private final StoredNode node1;
+    private final StoredNode node2;
 
     Map<String, String> addedProperties = new HashMap<String, String>();
     Map<String, String> removedProperties = new HashMap<String, String>();
@@ -62,11 +58,10 @@ public class NodeDelta {
     Map<String, Id> removedChildNodes = new HashMap<String, Id>();
     Map<String, Id> changedChildNodes = new HashMap<String, Id>();
 
-    public NodeDelta(
-            RevisionProvider provider, NodeState node1, NodeState node2) {
-        this.provider = provider;
+    public NodeDelta(StoredNode node1, StoredNode node2) {
         this.node1 = node1;
-        provider.compare(node1,  node2, new DiffHandler());
+        this.node2 = node2;
+        node1.diff(node2, this);
     }
 
     public Map<String, String> getAddedProperties() {
@@ -99,7 +94,7 @@ public class NodeDelta {
 
     public List<Conflict> listConflicts(NodeDelta other) {
         // assume that both delta's were built using the *same* base node revision
-        if (!node1.equals(other.node1)) {
+        if (!node1.getId().equals(other.node1.getId())) {
             throw new IllegalArgumentException("other and this NodeDelta object are expected to share common node1 instance");
         }
 
@@ -172,43 +167,40 @@ public class NodeDelta {
         return conflicts;
     }
 
-    //--------------------------------------------------------< inner classes >
+    //--------------------------------------------------------< NodeDiffHandler >
 
-    private class DiffHandler implements NodeStateDiff {
-
-        @Override
-        public void propertyAdded(PropertyState after) {
-            addedProperties.put(after.getName(), after.getEncodedValue());
-        }
-
-        @Override
-        public void propertyChanged(PropertyState before, PropertyState after) {
-            changedProperties.put(after.getName(), after.getEncodedValue());
-        }
+    @Override
+    public void propAdded(String propName, String value) {
+        addedProperties.put(propName, value);
+    }
 
-        @Override
-        public void propertyDeleted(PropertyState before) {
-            removedProperties.put(before.getName(), before.getEncodedValue());
-        }
+    @Override
+    public void propChanged(String propName, String oldValue, String newValue) {
+        changedProperties.put(propName, newValue);
+    }
 
-        @Override
-        public void childNodeAdded(String name, NodeState after) {
-            addedChildNodes.put(name, provider.getId(after));
-        }
+    @Override
+    public void propDeleted(String propName, String value) {
+        removedProperties.put(propName, value);
+    }
 
-        @Override
-        public void childNodeChanged(
-                String name, NodeState before, NodeState after) {
-            changedChildNodes.put(name, provider.getId(after));
-        }
+    @Override
+    public void childNodeAdded(ChildNodeEntry added) {
+        addedChildNodes.put(added.getName(), added.getId());
+    }
 
-        @Override
-        public void childNodeDeleted(String name, NodeState before) {
-            removedChildNodes.put(name, provider.getId(before));
-        }
+    @Override
+    public void childNodeDeleted(ChildNodeEntry deleted) {
+        removedChildNodes.put(deleted.getName(), deleted.getId());
+    }
 
+    @Override
+    public void childNodeChanged(ChildNodeEntry changed, Id newId) {
+        changedChildNodes.put(changed.getName(), newId);
     }
 
+    //--------------------------------------------------------< inner classes >
+
     public static class Conflict {
 
         final ConflictType type;

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StagedNodeTree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StagedNodeTree.java?rev=1520046&r1=1520045&r2=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StagedNodeTree.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/StagedNodeTree.java Wed Sep  4 15:08:56 2013
@@ -22,7 +22,6 @@ import java.util.Map;
 import java.util.Map.Entry;
 
 import org.apache.jackrabbit.mk.json.JsonObject;
-import org.apache.jackrabbit.mk.model.tree.NodeDelta;
 import org.apache.jackrabbit.mk.store.NotFoundException;
 import org.apache.jackrabbit.mk.store.RevisionStore;
 import org.apache.jackrabbit.mk.store.RevisionStore.PutToken;
@@ -385,8 +384,8 @@ public class StagedNodeTree {
         assert to != null;
         assert base != null;
 
-        NodeDelta theirDelta = new NodeDelta(store, store.getNodeState(from), store.getNodeState(base));
-        NodeDelta ourDelta = new NodeDelta(store, store.getNodeState(from), store.getNodeState(to));
+        NodeDelta theirDelta = new NodeDelta(from, base);
+        NodeDelta ourDelta = new NodeDelta(from, to);
 
         // apply the changes
         StagedNode stagedNode = getStagedNode(path, true);
@@ -520,10 +519,8 @@ public class StagedNodeTree {
      * {@code theirRoot}, using the tree at {@code baseRoot} as reference.
      */
     private void mergeNode(StoredNode baseNode, StoredNode ourNode, StoredNode theirNode, String path) throws Exception {
-        NodeDelta theirChanges = new NodeDelta(
-                store, store.getNodeState(baseNode), store.getNodeState(theirNode));
-        NodeDelta ourChanges = new NodeDelta(
-                store, store.getNodeState(baseNode), store.getNodeState(ourNode));
+        NodeDelta theirChanges = new NodeDelta(baseNode, theirNode);
+        NodeDelta ourChanges = new NodeDelta(baseNode, ourNode);
 
         StagedNode stagedNode = getStagedNode(path, true);
 

Added: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java?rev=1520046&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java (added)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java Wed Sep  4 15:08:56 2013
@@ -0,0 +1,57 @@
+/*
+ * 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.mk.model;
+
+import org.apache.jackrabbit.mk.store.RevisionProvider;
+import org.apache.jackrabbit.oak.commons.PathUtils;
+
+import java.util.Stack;
+
+/**
+ *
+ */
+public abstract class TraversingNodeDiffHandler implements NodeDiffHandler {
+
+    protected Stack<String> paths = new Stack<String>();
+
+    protected final RevisionProvider rp;
+
+    public TraversingNodeDiffHandler(RevisionProvider rp) {
+        this.rp = rp;
+    }
+
+    public void start(Node before, Node after, String path) {
+        paths.clear();
+        paths.push(path);
+        before.diff(after, this);
+    }
+
+    protected String getCurrentPath() {
+        return paths.peek();
+    }
+
+    @Override
+    public void childNodeChanged(ChildNodeEntry changed, Id newId) {
+        paths.push(PathUtils.concat(getCurrentPath(), changed.getName()));
+        try {
+            rp.getNode(changed.getId()).diff(rp.getNode(newId), this);
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+        paths.pop();
+    }
+}

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java?rev=1520046&r1=1520045&r2=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java Wed Sep  4 15:08:56 2013
@@ -22,12 +22,8 @@ import org.apache.jackrabbit.mk.model.Ch
 import org.apache.jackrabbit.mk.model.Id;
 import org.apache.jackrabbit.mk.model.MutableCommit;
 import org.apache.jackrabbit.mk.model.MutableNode;
-import org.apache.jackrabbit.mk.model.Node;
-import org.apache.jackrabbit.mk.model.NodeDiffHandler;
 import org.apache.jackrabbit.mk.model.StoredCommit;
 import org.apache.jackrabbit.mk.model.StoredNode;
-import org.apache.jackrabbit.mk.model.tree.NodeState;
-import org.apache.jackrabbit.mk.model.tree.NodeStateDiff;
 import org.apache.jackrabbit.mk.persistence.GCPersistence;
 import org.apache.jackrabbit.mk.persistence.Persistence;
 import org.apache.jackrabbit.mk.util.IOUtils;
@@ -58,8 +54,7 @@ import java.util.concurrent.locks.Reentr
  * Default revision store implementation, passing calls to a {@code Persistence}
  * and a {@code BlobStore}, respectively and providing caching.
  */
-public class DefaultRevisionStore extends AbstractRevisionStore implements
-        Closeable {
+public class DefaultRevisionStore implements RevisionStore, Closeable {
 
     private static final Logger LOG = LoggerFactory.getLogger(DefaultRevisionStore.class);
     
@@ -469,55 +464,6 @@ public class DefaultRevisionStore extend
         }
     }
 
-    // ------------------------------------------------------------< overrides >
-
-    @Override
-    public void compare(final NodeState before, final NodeState after,
-            final NodeStateDiff diff) {
-        // OAK-46: Efficient diffing of large child node lists
-
-        Node beforeNode = ((StoredNodeAsState) before).unwrap();
-        Node afterNode = ((StoredNodeAsState) after).unwrap();
-
-        beforeNode.diff(afterNode, new NodeDiffHandler() {
-            @Override
-            public void propAdded(String propName, String value) {
-                diff.propertyAdded(after.getProperty(propName));
-            }
-
-            @Override
-            public void propChanged(String propName, String oldValue,
-                    String newValue) {
-                diff.propertyChanged(before.getProperty(propName),
-                        after.getProperty(propName));
-            }
-
-            @Override
-            public void propDeleted(String propName, String value) {
-                diff.propertyDeleted(before.getProperty(propName));
-            }
-
-            @Override
-            public void childNodeAdded(ChildNodeEntry added) {
-                String name = added.getName();
-                diff.childNodeAdded(name, after.getChildNode(name));
-            }
-
-            @Override
-            public void childNodeDeleted(ChildNodeEntry deleted) {
-                String name = deleted.getName();
-                diff.childNodeDeleted(name, before.getChildNode(name));
-            }
-
-            @Override
-            public void childNodeChanged(ChildNodeEntry changed, Id newId) {
-                String name = changed.getName();
-                diff.childNodeChanged(name, before.getChildNode(name),
-                        after.getChildNode(name));
-            }
-        });
-    }
-
     // -----------------------------------------------------------------------
     // GC
 

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java?rev=1520046&r1=1520045&r2=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java Wed Sep  4 15:08:56 2013
@@ -20,31 +20,11 @@ import org.apache.jackrabbit.mk.model.Ch
 import org.apache.jackrabbit.mk.model.Id;
 import org.apache.jackrabbit.mk.model.StoredCommit;
 import org.apache.jackrabbit.mk.model.StoredNode;
-import org.apache.jackrabbit.mk.model.tree.NodeState;
-import org.apache.jackrabbit.mk.model.tree.NodeStore;
 
 /**
  * Read operations.
  */
-public interface RevisionProvider extends NodeStore {
-
-    /**
-     * Adapts the given {@link StoredNode} to a corresponding
-     * {@link NodeState} instance.
-     *
-     * @param node stored node instance
-     * @return node state adapter
-     */
-    NodeState getNodeState(StoredNode node);
-
-    /**
-     * Adapts the given {@link NodeState} to the corresponding identifier.
-     *
-     * @param node node state
-     * @return node identifier
-     */
-    Id getId(NodeState node);
-
+public interface RevisionProvider {
     StoredNode getNode(Id id) throws NotFoundException, Exception;
     StoredCommit getCommit(Id id) throws NotFoundException, Exception;
     ChildNodeEntriesMap getCNEMap(Id id) throws NotFoundException, Exception;

Modified: jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/MicroKernelImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/MicroKernelImplTest.java?rev=1520046&r1=1520045&r2=1520046&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/MicroKernelImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/MicroKernelImplTest.java Wed Sep  4 15:08:56 2013
@@ -51,6 +51,15 @@ public class MicroKernelImplTest {
         }
     }
 
+    @Test
+    public void diffWithMove() {
+        String base = mk.commit("", "+\"/a\":{\"b\":{}}", mk.getHeadRevision(), null);
+        String head = mk.commit("", "+\"/a/c\":{}+\"/a/d\":{}-\"/a/b\"", base, null);
+        String diff = mk.diff(base, head, "/a", 0);
+        assertTrue(diff.contains("c"));
+        assertTrue(diff.contains("d"));
+    }
+    
     /**
      * OAK-276: potential clash of commit id's after restart.
      */