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 ju...@apache.org on 2012/05/23 13:29:25 UTC

svn commit: r1341833 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory: MemoryNodeStateBuilder.java ModifiedNodeState.java

Author: jukka
Date: Wed May 23 11:29:25 2012
New Revision: 1341833

URL: http://svn.apache.org/viewvc?rev=1341833&view=rev
Log:
OAK-110: NPE in KernelNodeStoreBranch.diffToJsop

It's better if we consolidate a transient add/remove directly to a no-op
already in the NodeStateBuilder instead of adding a special case handling
for that in the resulting NodeState.

Also added a more optimized way of snapshotting the change set in common
NodeStateBuilder cases where no or just a single property or child node
have been modified.

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStateBuilder.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStateBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStateBuilder.java?rev=1341833&r1=1341832&r2=1341833&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStateBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeStateBuilder.java Wed May 23 11:29:25 2012
@@ -22,6 +22,7 @@ import org.apache.jackrabbit.oak.kernel.
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.spi.state.NodeStateBuilder;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -33,9 +34,15 @@ class MemoryNodeStateBuilder implements 
 
     private final NodeState base;
 
+    /**
+     * Set of added, modified or removed ({@code null} value) property states.
+     */
     private final Map<String, PropertyState> properties =
             new HashMap<String, PropertyState>();
 
+    /**
+     * Set of added, modified or removed ({@code null} value) child node states.
+     */
     private final Map<String, NodeState> nodes =
             new HashMap<String, NodeState>();
 
@@ -50,9 +57,24 @@ class MemoryNodeStateBuilder implements 
             return base; // shortcut
         } else {
             return new ModifiedNodeState(
-                    base,
-                    new HashMap<String, PropertyState>(properties),
-                    new HashMap<String, NodeState>(nodes));
+                    base, snapshot(properties), snapshot(nodes));
+        }
+    }
+
+    /**
+     * Returns an optimized snapshot of the current state of the given map.
+     *
+     * @param map mutable map
+     * @return optimized snapshot
+     */
+    private static <T> Map<String, T> snapshot(Map<String, T> map) {
+        if (map.isEmpty()) {
+            return Collections.emptyMap();
+        } else if (map.size() == 1) {
+            Map.Entry<String, T> entry = map.entrySet().iterator().next();
+            return Collections.singletonMap(entry.getKey(), entry.getValue());
+        } else {
+            return new HashMap<String, T>(map);
         }
     }
 
@@ -63,7 +85,11 @@ class MemoryNodeStateBuilder implements 
 
     @Override
     public void removeNode(String name) {
-        nodes.put(name, null);
+        if (base.getChildNode(name) != null) {
+            nodes.put(name, null);
+        } else {
+            nodes.remove(name);
+        }
     }
 
     @Override
@@ -80,7 +106,11 @@ class MemoryNodeStateBuilder implements 
 
     @Override
     public void removeProperty(String name) {
-        properties.put(name, null);
+        if (base.getProperty(name) != null) {
+            properties.put(name, null);
+        } else {
+            properties.remove(name);
+        }
     }
 
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java?rev=1341833&r1=1341832&r2=1341833&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java Wed May 23 11:29:25 2012
@@ -51,12 +51,11 @@ public class ModifiedNodeState extends P
         for (Map.Entry<String, PropertyState> entry : properties.entrySet()) {
             PropertyState before = super.getProperty(entry.getKey());
             PropertyState after = entry.getValue();
-            if (before == null) {
-                if (after != null) {
-                    diff.propertyAdded(after);
-                }
-            } else if (after == null) {
+            if (after == null) {
+                assert before != null;
                 diff.propertyDeleted(before);
+            } else if (before == null) {
+                diff.propertyAdded(after);
             } else {
                 diff.propertyChanged(before, after);
             }
@@ -66,12 +65,11 @@ public class ModifiedNodeState extends P
             String name = entry.getKey();
             NodeState before = super.getChildNode(name);
             NodeState after = entry.getValue();
-            if (before == null) {
-                if (after != null) {
-                    diff.childNodeAdded(name, after);
-                }
-            } else if (after == null) {
+            if (after == null) {
+                assert before != null;
                 diff.childNodeDeleted(name, before);
+            } else if (before == null) {
+                diff.childNodeAdded(name, after);
             } else {
                 diff.childNodeChanged(name, before, after);
             }