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 mr...@apache.org on 2015/10/08 14:07:35 UTC

svn commit: r1707509 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/ test/java/org/apache/jackrabbit/oak/plugins/document/

Author: mreutegg
Date: Thu Oct  8 12:07:35 2015
New Revision: 1707509

URL: http://svn.apache.org/viewvc?rev=1707509&view=rev
Log:
OAK-3494: MemoryDiffCache should also check parent paths before falling to Loader (or returning null)

Move parsing of diff cache jsop into a static method

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DiffCache.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LocalDiffCache.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/TieredDiffCache.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AmnesiaDiffCache.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DiffCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DiffCache.java?rev=1707509&r1=1707508&r2=1707509&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DiffCache.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DiffCache.java Thu Oct  8 12:07:35 2015
@@ -21,11 +21,15 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.apache.jackrabbit.oak.cache.CacheStats;
+import org.apache.jackrabbit.oak.commons.json.JsopReader;
+import org.apache.jackrabbit.oak.commons.json.JsopTokenizer;
+
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.unshareString;
 
 /**
  * A cache for child node diffs.
  */
-public interface DiffCache {
+abstract class DiffCache {
 
     /**
      * Returns a jsop diff for the child nodes at the given path. The returned
@@ -45,10 +49,10 @@ public interface DiffCache {
      * @return the diff or {@code null} if unknown and no loader was passed.
      */
     @CheckForNull
-    String getChanges(@Nonnull Revision from,
-                      @Nonnull Revision to,
-                      @Nonnull String path,
-                      @Nullable Loader loader);
+    abstract String getChanges(@Nonnull Revision from,
+                               @Nonnull Revision to,
+                               @Nonnull String path,
+                               @Nullable Loader loader);
 
     /**
      * Starts a new cache entry for the diff cache. Actual changes are added
@@ -61,15 +65,70 @@ public interface DiffCache {
      * @return the cache entry.
      */
     @Nonnull
-    Entry newEntry(@Nonnull Revision from,
-                   @Nonnull Revision to,
-                   boolean local);
+    abstract Entry newEntry(@Nonnull Revision from,
+                            @Nonnull Revision to,
+                            boolean local);
 
     /**
      * @return the statistics for this cache.
      */
     @Nonnull
-    Iterable<CacheStats> getStats();
+    abstract Iterable<CacheStats> getStats();
+
+    /**
+     * Parses the jsop diff returned by
+     * {@link #getChanges(Revision, Revision, String, Loader)} and reports the
+     * changes by calling the appropriate methods on {@link Diff}.
+     *
+     * @param jsop the jsop diff to parse.
+     * @param diff the diff handler.
+     * @return {@code true} it the complete jsop was processed or {@code false}
+     *      if one of the {@code diff} callbacks requested a stop.
+     * @throws IllegalArgumentException if {@code jsop} is malformed.
+     */
+    static boolean parseJsopDiff(@Nonnull String jsop,
+                                 @Nonnull Diff diff) {
+        if (jsop.trim().isEmpty()) {
+            return true;
+        }
+        JsopTokenizer t = new JsopTokenizer(jsop);
+        boolean continueComparison = true;
+        while (continueComparison) {
+            int r = t.read();
+            if (r == JsopReader.END) {
+                break;
+            }
+            switch (r) {
+                case '+': {
+                    String name = unshareString(t.readString());
+                    t.read(':');
+                    t.read('{');
+                    while (t.read() != '}') {
+                        // skip properties
+                    }
+                    continueComparison = diff.childNodeAdded(name);
+                    break;
+                }
+                case '-': {
+                    String name = unshareString(t.readString());
+                    continueComparison = diff.childNodeDeleted(name);
+                    break;
+                }
+                case '^': {
+                    String name = unshareString(t.readString());
+                    t.read(':');
+                    t.read('{');
+                    t.read('}');
+                    continueComparison = diff.childNodeChanged(name);
+                    break;
+                }
+                default:
+                    throw new IllegalArgumentException("jsonDiff: illegal token '"
+                            + t.getToken() + "' at pos: " + t.getLastPos() + ' ' + jsop);
+            }
+        }
+        return continueComparison;
+    }
 
     interface Entry {
 
@@ -96,4 +155,14 @@ public interface DiffCache {
 
         String call();
     }
+
+    interface Diff {
+
+        boolean childNodeAdded(String name);
+
+        boolean childNodeChanged(String name);
+
+        boolean childNodeDeleted(String name);
+
+    }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1707509&r1=1707508&r2=1707509&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Thu Oct  8 12:07:35 2015
@@ -33,7 +33,6 @@ import static org.apache.jackrabbit.oak.
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.asStringValueIterable;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.pathToId;
-import static org.apache.jackrabbit.oak.plugins.document.util.Utils.unshareString;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -77,8 +76,6 @@ import com.google.common.util.concurrent
 
 import org.apache.jackrabbit.oak.commons.IOUtils;
 import org.apache.jackrabbit.oak.commons.jmx.AnnotatedStandardMBean;
-import org.apache.jackrabbit.oak.commons.json.JsopReader;
-import org.apache.jackrabbit.oak.commons.json.JsopTokenizer;
 import org.apache.jackrabbit.oak.plugins.blob.BlobStoreBlob;
 import org.apache.jackrabbit.oak.plugins.blob.MarkSweepGarbageCollector;
 import org.apache.jackrabbit.oak.plugins.blob.ReferencedBlob;
@@ -2100,67 +2097,45 @@ public final class DocumentNodeStore
         }
     }
 
-    private boolean dispatch(@Nonnull String jsonDiff,
-                             @Nonnull DocumentNodeState node,
-                             @Nonnull DocumentNodeState base,
-                             @Nonnull NodeStateDiff diff) {
-        if (jsonDiff.trim().isEmpty()) {
-            return true;
-        }
-        JsopTokenizer t = new JsopTokenizer(jsonDiff);
-        boolean continueComparison = true;
-        while (continueComparison) {
-            int r = t.read();
-            if (r == JsopReader.END) {
-                break;
-            }
-            switch (r) {
-                case '+': {
-                    String name = unshareString(t.readString());
-                    t.read(':');
-                    t.read('{');
-                    while (t.read() != '}') {
-                        // skip properties
-                    }
-                    continueComparison = diff.childNodeAdded(name,
-                            node.getChildNode(name));
-                    break;
-                }
-                case '-': {
-                    String name = unshareString(t.readString());
-                    continueComparison = diff.childNodeDeleted(name,
-                            base.getChildNode(name));
-                    break;
-                }
-                case '^': {
-                    String name = unshareString(t.readString());
-                    t.read(':');
-                    t.read('{');
-                    t.read('}');
-                    NodeState baseChild = base.getChildNode(name);
-                    NodeState nodeChild = node.getChildNode(name);
-                    if (baseChild.exists()) {
-                        if (nodeChild.exists()) {
-                            continueComparison = diff.childNodeChanged(name,
-                                    baseChild, nodeChild);
-                        } else {
-                            continueComparison = diff.childNodeDeleted(name,
-                                    baseChild);
-                        }
+    private boolean dispatch(@Nonnull final String jsonDiff,
+                             @Nonnull final DocumentNodeState node,
+                             @Nonnull final DocumentNodeState base,
+                             @Nonnull final NodeStateDiff diff) {
+        return DiffCache.parseJsopDiff(jsonDiff, new DiffCache.Diff() {
+            @Override
+            public boolean childNodeAdded(String name) {
+                return diff.childNodeAdded(name,
+                        node.getChildNode(name));
+            }
+
+            @Override
+            public boolean childNodeChanged(String name) {
+                boolean continueComparison = true;
+                NodeState baseChild = base.getChildNode(name);
+                NodeState nodeChild = node.getChildNode(name);
+                if (baseChild.exists()) {
+                    if (nodeChild.exists()) {
+                        continueComparison = diff.childNodeChanged(name,
+                                baseChild, nodeChild);
                     } else {
-                        if (nodeChild.exists()) {
-                            continueComparison = diff.childNodeAdded(name,
-                                    nodeChild);
-                        }
+                        continueComparison = diff.childNodeDeleted(name,
+                                baseChild);
+                    }
+                } else {
+                    if (nodeChild.exists()) {
+                        continueComparison = diff.childNodeAdded(name,
+                                nodeChild);
                     }
-                    break;
                 }
-                default:
-                    throw new IllegalArgumentException("jsonDiff: illegal token '"
-                            + t.getToken() + "' at pos: " + t.getLastPos() + ' ' + jsonDiff);
+                return continueComparison;
             }
-        }
-        return continueComparison;
+
+            @Override
+            public boolean childNodeDeleted(String name) {
+                return diff.childNodeDeleted(name,
+                        base.getChildNode(name));
+            }
+        });
     }
 
     /**

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LocalDiffCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LocalDiffCache.java?rev=1707509&r1=1707508&r2=1707509&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LocalDiffCache.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LocalDiffCache.java Thu Oct  8 12:07:35 2015
@@ -36,7 +36,7 @@ import org.apache.jackrabbit.oak.plugins
 /**
  * A diff cache, which is pro-actively filled after a commit.
  */
-public class LocalDiffCache implements DiffCache {
+public class LocalDiffCache extends DiffCache {
 
     /**
      * Limit is arbitrary for now i.e. 16 MB. Same as in MongoDiffCache

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java?rev=1707509&r1=1707508&r2=1707509&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java Thu Oct  8 12:07:35 2015
@@ -34,7 +34,7 @@ import static com.google.common.base.Pre
 /**
  * An in-memory diff cache implementation.
  */
-public class MemoryDiffCache implements DiffCache {
+public class MemoryDiffCache extends DiffCache {
 
     /**
      * Diff cache.

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/TieredDiffCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/TieredDiffCache.java?rev=1707509&r1=1707508&r2=1707509&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/TieredDiffCache.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/TieredDiffCache.java Thu Oct  8 12:07:35 2015
@@ -27,7 +27,7 @@ import org.apache.jackrabbit.oak.cache.C
  * Implements a tiered diff cache which consists of a {@link LocalDiffCache} and
  * a {@link MemoryDiffCache}.
  */
-class TieredDiffCache implements DiffCache {
+class TieredDiffCache extends DiffCache {
 
     private final DiffCache localCache;
     private final DiffCache memoryCache;

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AmnesiaDiffCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AmnesiaDiffCache.java?rev=1707509&r1=1707508&r2=1707509&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AmnesiaDiffCache.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/AmnesiaDiffCache.java Thu Oct  8 12:07:35 2015
@@ -26,7 +26,7 @@ import org.apache.jackrabbit.oak.cache.C
 /**
  * A diff cache implementation, which immediately forgets the diff.
  */
-class AmnesiaDiffCache implements DiffCache {
+class AmnesiaDiffCache extends DiffCache {
 
     static final DiffCache INSTANCE = new AmnesiaDiffCache();