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 2013/04/30 19:25:47 UTC

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

Author: jukka
Date: Tue Apr 30 17:25:47 2013
New Revision: 1477731

URL: http://svn.apache.org/r1477731
Log:
OAK-781: Clarify / fix effects of MISSING_NODE as base state of NodeBuilder

Simplify and streamline the interactions between Mutable- and ModifiedNodeState

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

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=1477731&r1=1477730&r2=1477731&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 Tue Apr 30 17:25:47 2013
@@ -26,7 +26,7 @@ import static com.google.common.collect.
 import static com.google.common.collect.Maps.filterValues;
 import static com.google.common.collect.Maps.newHashMap;
 import static java.util.Collections.emptyList;
-import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE;
+import static java.util.Collections.emptyMap;
 import static org.apache.jackrabbit.oak.plugins.memory.MemoryChildNodeEntry.iterable;
 
 import java.util.Map;
@@ -49,6 +49,52 @@ import com.google.common.base.Predicates
  */
 public class ModifiedNodeState extends AbstractNodeState {
 
+    /**
+     * Unwraps the given {@code NodeState} instance into the given internals
+     * of a {@link MutableNodeState} instance that is being created or reset.
+     * <p>
+     * If the given base state is a {@code ModifiedNodeState} instance,
+     * then the contained modifications are applied to the given properties
+     * property and child node maps and the contained base state is returned
+     * for use as the base state of the {@link MutableNodeState} instance.
+     * <p>
+     * If the given base state is not a {@code ModifiedNodeState}, then
+     * the given property and child node maps are simply reset and the given
+     * base state is returned as-is for use as the base state of the
+     * {@link MutableNodeState} instance.
+     *
+     * @param base new base state
+     * @param properties {@link MutableNodeState} property map
+     * @param nodes {@link MutableNodeState} child node map
+     * @return new {@link MutableNodeState} base state
+     */
+    static NodeState unwrap(
+            @Nonnull NodeState base,
+            @Nonnull Map<String, PropertyState> properties,
+            @Nonnull Map<String, MutableNodeState> nodes) {
+        properties.clear();
+        for (Entry<String, MutableNodeState> entry : nodes.entrySet()) {
+            entry.getValue().reset(base.getChildNode(entry.getKey()));
+        }
+
+        if (base instanceof ModifiedNodeState) {
+            ModifiedNodeState modified = (ModifiedNodeState) base;
+
+            properties.putAll(modified.properties);
+            for (Entry<String, NodeState> entry : modified.nodes.entrySet()) {
+                String name = entry.getKey();
+                if (!nodes.containsKey(name)) {
+                    nodes.put(name, new MutableNodeState(entry.getValue()));
+                }
+            }
+
+            return modified.base;
+        } else {
+            return base;
+        }
+    }
+
+
     static long getPropertyCount(
             NodeState base, Map<String, PropertyState> properties) {
         long count = 0;
@@ -110,9 +156,9 @@ public class ModifiedNodeState extends A
         long count = 0;
         if (base.exists()) {
             count = base.getChildNodeCount();
-            for (Map.Entry<String, ? extends NodeState> entry
+            for (Entry<String, ? extends NodeState> entry
                     : nodes.entrySet()) {
-                if (base.getChildNode(entry.getKey()).exists()) {
+                if (base.hasChildNode(entry.getKey())) {
                     count--;
                 }
                 if (entry.getValue().exists()) {
@@ -157,21 +203,32 @@ public class ModifiedNodeState extends A
      */
     private final Map<String, NodeState> nodes;
 
+    /**
+     * Creates an immutable snapshot of the given internal state of a
+     * {@link MutableNodeState} instance.
+     *
+     * @param base base state
+     * @param properties current property modifications
+     * @param nodes current child node modifications
+     */
     ModifiedNodeState(
             @Nonnull NodeState base,
             @Nonnull Map<String, PropertyState> properties,
             @Nonnull Map<String, MutableNodeState> nodes) {
         this.base = checkNotNull(base);
-        this.properties = newHashMap(checkNotNull(properties));
-        this.nodes = newHashMap();
-        for (Entry<String, MutableNodeState> entry
-                : checkNotNull(nodes).entrySet()) {
-            String name = entry.getKey();
-            MutableNodeState child = entry.getValue();
-            if (child != null) {
-                this.nodes.put(name, child.snapshot());
-            } else {
-                this.nodes.put(name, MISSING_NODE);
+
+        if (checkNotNull(properties).isEmpty()) {
+            this.properties = emptyMap();
+        } else {
+            this.properties = newHashMap(properties);
+        }
+
+        if (checkNotNull(nodes).isEmpty()) {
+            this.nodes = emptyMap();
+        } else {
+            this.nodes = newHashMap();
+            for (Entry<String, MutableNodeState> entry : nodes.entrySet()) {
+                this.nodes.put(entry.getKey(), entry.getValue().snapshot());
             }
         }
     }
@@ -251,15 +308,50 @@ public class ModifiedNodeState extends A
     /**
      * Since we keep track of an explicit base node state for a
      * {@link ModifiedNodeState} instance, we can do this in two steps:
-     * first compare the base states to each other (often a fast operation),
-     * ignoring all changed properties and child nodes for which we have
-     * further modifications, and then compare all the modified properties
-     * and child nodes to those in the given base state.
+     * first compare all the modified properties and child nodes to those
+     * of the given base state, and then compare the base states to each
+     * other, ignoring all changed properties and child nodes that were
+     * already covered earlier.
      */
     @Override
     public boolean compareAgainstBaseState(
             NodeState base, final NodeStateDiff diff) {
-        if (!this.base.compareAgainstBaseState(base, new NodeStateDiff() {
+        for (Map.Entry<String, PropertyState> entry : properties.entrySet()) {
+            PropertyState before = base.getProperty(entry.getKey());
+            PropertyState after = entry.getValue();
+            if (after == null) {
+                if (before != null && !diff.propertyDeleted(before)) {
+                    return false;
+                }
+            } else if (before == null) {
+                if (!diff.propertyAdded(after)) {
+                    return false;
+                }
+            } else if (!before.equals(after)
+                    && !diff.propertyChanged(before, after)) {
+                return false;
+            }
+        }
+
+        for (Map.Entry<String, NodeState> entry : nodes.entrySet()) {
+            String name = entry.getKey();
+            NodeState before = base.getChildNode(name);
+            NodeState after = entry.getValue();
+            if (!after.exists()) {
+                if (before.exists() && !diff.childNodeDeleted(name, before)) {
+                    return false;
+                }
+            } else if (!before.exists()) {
+                if (!diff.childNodeAdded(name, after)) {
+                    return false;
+                }
+            } else if (!before.equals(after)
+                    && !diff.childNodeChanged(name, before, after)) {
+                return false;
+            }
+        }
+
+        return this.base.compareAgainstBaseState(base, new NodeStateDiff() {
             @Override
             public boolean propertyAdded(PropertyState after) {
                 return properties.containsKey(after.getName())
@@ -291,81 +383,7 @@ public class ModifiedNodeState extends A
                 return nodes.containsKey(name)
                         || diff.childNodeDeleted(name, before);
             }
-        })) {
-            return false;
-        }
-
-        for (Map.Entry<String, PropertyState> entry : properties.entrySet()) {
-            PropertyState before = base.getProperty(entry.getKey());
-            PropertyState after = entry.getValue();
-            if (before == null && after == null) {
-                // do nothing
-            } else if (after == null) {
-                if (!diff.propertyDeleted(before)) {
-                    return false; 
-                }
-            } else if (before == null) {
-                if (!diff.propertyAdded(after)) {
-                    return false;
-                }
-            } else if (!before.equals(after)) {
-                if (!diff.propertyChanged(before, after)) {
-                    return false;
-                }
-            }
-        }
-
-        for (Map.Entry<String, NodeState> entry : nodes.entrySet()) {
-            String name = entry.getKey();
-            NodeState before = base.getChildNode(name);
-            NodeState after = entry.getValue();
-            if (!after.exists()) {
-                if (before.exists()) {
-                    if (!diff.childNodeDeleted(name, before)) {
-                        return false;
-                    }
-                }
-            } else if (!before.exists()) {
-                if (!diff.childNodeAdded(name, after)) {
-                    return false;
-                }
-            } else if (!before.equals(after)) {
-                if (!diff.childNodeChanged(name, before, after)) {
-                    return false;
-                }
-            }
-        }
-
-        return true;
-    }
-
-    public void compareAgainstBaseState(NodeStateDiff diff) {
-        for (Entry<String, PropertyState> entry : properties.entrySet()) {
-            PropertyState before = base.getProperty(entry.getKey());
-            PropertyState after = entry.getValue();
-            if (after == null) {
-                diff.propertyDeleted(before);
-            } else if (before == null) {
-                diff.propertyAdded(after);
-            } else if (!before.equals(after)) { // TODO: can we assume this?
-                diff.propertyChanged(before, after);
-            }
-        }
-
-        for (Entry<String, NodeState> entry : nodes.entrySet()) {
-            String name = entry.getKey();
-            NodeState before = base.getChildNode(name);
-            NodeState after = entry.getValue();
-            if (!after.exists()) {
-                if (before.exists()) { // TODO: can we assume this?
-                    diff.childNodeDeleted(name, before);
-                }
-            } else if (!before.exists()) {
-                diff.childNodeAdded(name, after);
-            } else if (!before.equals(after)) { // TODO: can we assume this?
-                diff.childNodeChanged(name, before, after);
-            }
-        }
+        });
     }
 
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java?rev=1477731&r1=1477730&r2=1477731&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java Tue Apr 30 17:25:47 2013
@@ -63,48 +63,8 @@ class MutableNodeState extends AbstractN
     private final Map<String, MutableNodeState> nodes = newHashMap();
 
     MutableNodeState(@Nonnull NodeState base) {
-        this.base = checkNotNull(base);
-
-        // unwrap ModifiedNodeState instances
-        if (base instanceof ModifiedNodeState) {
-            ModifiedNodeState modified = (ModifiedNodeState) base;
-            this.base = modified.getBaseState();
-
-            modified.compareAgainstBaseState(new NodeStateDiff() {
-                @Override
-                public boolean propertyAdded(PropertyState after) {
-                    properties.put(after.getName(), after);
-                    return true;
-                }
-                @Override
-                public boolean propertyChanged(
-                        PropertyState before, PropertyState after) {
-                    properties.put(after.getName(), after);
-                    return true;
-                }
-                @Override
-                public boolean propertyDeleted(PropertyState before) {
-                    properties.put(before.getName(), null);
-                    return true;
-                }
-                @Override
-                public boolean childNodeAdded(String name, NodeState after) {
-                    nodes.put(name, new MutableNodeState(after));
-                    return true;
-                }
-                @Override
-                public boolean childNodeChanged(
-                        String name, NodeState before, NodeState after) {
-                    nodes.put(name, new MutableNodeState(after));
-                    return true;
-                }
-                @Override
-                public boolean childNodeDeleted(String name, NodeState before) {
-                    nodes.put(name, new MutableNodeState(MISSING_NODE));
-                    return true;
-                }
-            });
-        }
+        checkNotNull(base);
+        this.base = ModifiedNodeState.unwrap(base, properties, nodes);
     }
 
     NodeState snapshot() {
@@ -116,61 +76,10 @@ class MutableNodeState extends AbstractN
         }
     }
 
-    private void reset(NodeState newBase) {
+    void reset(NodeState newBase) {
         assert base != null;
-
-        base = newBase;
-        properties.clear();
-        for (Entry<String, MutableNodeState> entry : nodes.entrySet()) {
-            entry.getValue().reset(base.getChildNode(entry.getKey()));
-        }
-
-        // unwrap ModifiedNodeState instances
-        if (base instanceof ModifiedNodeState) {
-            ModifiedNodeState modified = (ModifiedNodeState) base;
-            base = modified.getBaseState();
-
-            modified.compareAgainstBaseState(new NodeStateDiff() {
-                @Override
-                public boolean propertyAdded(PropertyState after) {
-                    properties.put(after.getName(), after);
-                    return true;
-                }
-                @Override
-                public boolean propertyChanged(
-                        PropertyState before, PropertyState after) {
-                    properties.put(after.getName(), after);
-                    return true;
-                }
-                @Override
-                public boolean propertyDeleted(PropertyState before) {
-                    properties.put(before.getName(), null);
-                    return true;
-                }
-                @Override
-                public boolean childNodeAdded(String name, NodeState after) {
-                    if (!nodes.containsKey(name)) {
-                        nodes.put(name, new MutableNodeState(after));
-                    }
-                    return true;
-                }
-                @Override
-                public boolean childNodeChanged(
-                        String name, NodeState before, NodeState after) {
-                    if (!nodes.containsKey(name)) {
-                        nodes.put(name, new MutableNodeState(after));
-                    }
-                    return true;
-                }
-                @Override
-                public boolean childNodeDeleted(String name, NodeState before) {
-                    if (!nodes.containsKey(name)) {
-                        nodes.put(name, new MutableNodeState(MISSING_NODE));
-                    }
-                    return true;
-                }
-            });
-        }
+        checkNotNull(newBase);
+        base = ModifiedNodeState.unwrap(newBase, properties, nodes);
     }
 
     /**