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/01 18:52:44 UTC
svn commit: r1519289 -
/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java
Author: stefan
Date: Sun Sep 1 16:52:44 2013
New Revision: 1519289
URL: http://svn.apache.org/r1519289
Log:
OAK-979: MicroKernel.diff() returns incomplete JSON when moves are involved (H2)
Modified:
jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java
Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java?rev=1519289&r1=1519288&r2=1519289&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java Sun Sep 1 16:52:44 2013
@@ -17,8 +17,10 @@
package org.apache.jackrabbit.mk.model.tree;
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.Map;
@@ -32,10 +34,10 @@ public class DiffBuilder {
private final String path;
private final int depth;
private final String pathFilter;
- private final NodeStore store;
+ private final RevisionProvider store;
public DiffBuilder(NodeState before, NodeState after, String path, int depth,
- NodeStore store, String pathFilter) {
+ RevisionProvider store, String pathFilter) {
this.before = before;
this.after = after;
this.path = path;
@@ -55,8 +57,8 @@ public class DiffBuilder {
// which is not recommended because the hashCode and equals methods
// of those classes are slow
- final HashMap<NodeState, String> addedNodes = new HashMap<NodeState, String>();
- final HashMap<NodeState, String> removedNodes = new HashMap<NodeState, String>();
+ final HashMap<NodeState, ArrayList<String>> addedNodes = new HashMap<NodeState, ArrayList<String>>();
+ final HashMap<NodeState, ArrayList<String>> removedNodes = new HashMap<NodeState, ArrayList<String>>();
if (!PathUtils.isAncestor(path, pathFilter)
&& !path.startsWith(pathFilter)) {
@@ -120,22 +122,56 @@ public class DiffBuilder {
public void childNodeAdded(String name, NodeState after) {
String p = PathUtils.concat(getCurrentPath(), name);
if (p.startsWith(pathFilter)) {
- addedNodes.put(after, p);
- buff.tag('+').
- key(p).object();
- toJson(buff, after, depth);
- buff.endObject().newline();
+ ArrayList<String> removedPaths = removedNodes.get(after);
+ if (removedPaths != null) {
+ // move detected
+ String removedPath = removedPaths.remove(0);
+ if (removedPaths.isEmpty()) {
+ removedNodes.remove(after);
+ }
+ buff.tag('>').
+ // path/to/deleted/node
+ key(removedPath).
+ // path/to/added/node
+ value(p).
+ newline();
+ } else {
+ ArrayList<String> addedPaths = addedNodes.get(after);
+ if (addedPaths == null) {
+ addedPaths = new ArrayList<String>();
+ addedNodes.put(after, addedPaths);
+ }
+ addedPaths.add(p);
+ }
}
+
}
@Override
public void childNodeDeleted(String name, NodeState before) {
String p = PathUtils.concat(getCurrentPath(), name);
if (p.startsWith(pathFilter)) {
- removedNodes.put(before, p);
- buff.tag('-');
- buff.value(p);
- buff.newline();
+ ArrayList<String> addedPaths = addedNodes.get(before);
+ if (addedPaths != null) {
+ // move detected
+ String addedPath = addedPaths.remove(0);
+ if (addedPaths.isEmpty()) {
+ addedNodes.remove(before);
+ }
+ buff.tag('>').
+ // path/to/deleted/node
+ key(p).
+ // path/to/added/node
+ value(addedPath).
+ newline();
+ } else {
+ ArrayList<String> removedPaths = removedNodes.get(before);
+ if (removedPaths == null) {
+ removedPaths = new ArrayList<String>();
+ removedNodes.put(before, removedPaths);
+ }
+ removedPaths.add(p);
+ }
}
}
@@ -160,115 +196,21 @@ public class DiffBuilder {
};
diffHandler.start(before, after, path);
- // check if this commit includes 'move' operations
- // by building intersection of added and removed nodes
- addedNodes.keySet().retainAll(removedNodes.keySet());
- if (!addedNodes.isEmpty()) {
- // this commit includes 'move' operations
- removedNodes.keySet().retainAll(addedNodes.keySet());
- // addedNodes & removedNodes now only contain information about moved nodes
-
- // re-build the diff in a 2nd pass, this time representing moves correctly
- buff.resetWriter();
-
- // TODO refactor code, avoid duplication
-
- diffHandler = new TraversingNodeDiffHandler(store) {
- int levels = depth < 0 ? Integer.MAX_VALUE : depth;
- @Override
- public void propertyAdded(PropertyState after) {
- String p = PathUtils.concat(getCurrentPath(), after.getName());
- if (p.startsWith(pathFilter)) {
- buff.tag('^').
- key(p).
- encodedValue(after.getEncodedValue()).
- newline();
- }
- }
-
- @Override
- public void propertyChanged(PropertyState before, PropertyState after) {
- String p = PathUtils.concat(getCurrentPath(), after.getName());
- if (p.startsWith(pathFilter)) {
- buff.tag('^').
- key(p).
- encodedValue(after.getEncodedValue()).
- newline();
- }
- }
-
- @Override
- public void propertyDeleted(PropertyState before) {
- String p = PathUtils.concat(getCurrentPath(), before.getName());
- 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(String name, NodeState after) {
- if (addedNodes.containsKey(after)) {
- // moved node, will be processed separately
- return;
- }
- String p = PathUtils.concat(getCurrentPath(), name);
- if (p.startsWith(pathFilter)) {
- buff.tag('+').
- key(p).object();
- toJson(buff, after, depth);
- buff.endObject().newline();
- }
- }
-
- @Override
- public void childNodeDeleted(String name, NodeState before) {
- if (addedNodes.containsKey(before)) {
- // moved node, will be processed separately
- return;
- }
- String p = PathUtils.concat(getCurrentPath(), name);
- if (p.startsWith(pathFilter)) {
- buff.tag('-');
- buff.value(p);
- buff.newline();
- }
- }
-
- @Override
- public void childNodeChanged(String name, NodeState before, NodeState after) {
- String p = PathUtils.concat(getCurrentPath(), name);
- if (PathUtils.isAncestor(p, pathFilter)
- || p.startsWith(pathFilter)) {
- --levels;
- if (levels >= 0) {
- // recurse
- super.childNodeChanged(name, before, after);
- } else {
- buff.tag('^');
- buff.key(p);
- buff.object().endObject();
- buff.newline();
- }
- ++levels;
- }
- }
- };
- diffHandler.start(before, after, path);
-
- // finally process moved nodes
- for (Map.Entry<NodeState, String> entry : addedNodes.entrySet()) {
- buff.tag('>').
- // path/to/deleted/node
- key(removedNodes.get(entry.getKey())).
- // path/to/added/node
- value(entry.getValue()).
- newline();
+ // finally process remaining added nodes ...
+ for (Map.Entry<NodeState, ArrayList<String>> entry : addedNodes.entrySet()) {
+ for (String p : entry.getValue()) {
+ buff.tag('+').
+ key(p).object();
+ toJson(buff, entry.getKey(), depth);
+ buff.endObject().newline();
+ }
+ }
+ // ... and removed nodes
+ for (Map.Entry<NodeState, ArrayList<String>> entry : removedNodes.entrySet()) {
+ for (String p : entry.getValue()) {
+ buff.tag('-');
+ buff.value(p);
+ buff.newline();
}
}
return buff.toString();