jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1477716 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak: plugins/memory/ModifiedNodeState.java plugins/memory/MutableNodeState.java spi/state/ChildNodeEntry.java spi/state/NodeState.java
Date Tue, 30 Apr 2013 16:35:10 GMT
Author: jukka
Date: Tue Apr 30 16:35:10 2013
New Revision: 1477716

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

Replace null values in the nodes maps of Mutable- and ModifiedNodeState with non-existing
NodeState instances

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
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ChildNodeEntry.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.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=1477716&r1=1477715&r2=1477716&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 16:35:10 2013
@@ -33,7 +33,6 @@ import java.util.Map;
 import java.util.Map.Entry;
 
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
@@ -116,7 +115,7 @@ public class ModifiedNodeState extends A
                 if (base.getChildNode(entry.getKey()).exists()) {
                     count--;
                 }
-                if (entry.getValue() != null && entry.getValue().exists()) {
+                if (entry.getValue().exists()) {
                     count++;
                 }
             }
@@ -137,7 +136,7 @@ public class ModifiedNodeState extends A
             }
             return concat(
                     filter(base.getChildNodeNames(), not(in(nodes.keySet()))),
-                    filterValues(nodes, notNull()).keySet());
+                    filterValues(nodes, NodeState.EXISTS).keySet());
         }
     }
 
@@ -153,25 +152,11 @@ public class ModifiedNodeState extends A
     private final Map<String, PropertyState> properties;
 
     /**
-     * Set of added, modified or removed ({@code null} value)
+     * Set of added, modified or removed (non-existent value)
      * child nodes.
      */
     private final Map<String, NodeState> nodes;
 
-    private final Predicate<ChildNodeEntry> unmodifiedNodes = new Predicate<ChildNodeEntry>()
{
-        @Override
-        public boolean apply(ChildNodeEntry input) {
-            return !nodes.containsKey(input.getName());
-        }
-    };
-
-    private final Predicate<NodeState> existingNodes = new Predicate<NodeState>()
{
-        @Override
-        public boolean apply(@Nullable NodeState node) {
-            return node != null && node.exists();
-        }
-    };
-
     ModifiedNodeState(
             @Nonnull NodeState base,
             @Nonnull Map<String, PropertyState> properties,
@@ -186,7 +171,7 @@ public class ModifiedNodeState extends A
             if (child != null) {
                 this.nodes.put(name, child.snapshot());
             } else {
-                this.nodes.put(name, null);
+                this.nodes.put(name, MISSING_NODE);
             }
         }
     }
@@ -237,13 +222,10 @@ public class ModifiedNodeState extends A
     public NodeState getChildNode(String name) {
         // checkArgument(!checkNotNull(name).isEmpty());  // TODO: should be caught earlier
         NodeState child = nodes.get(name);
-        if (child != null) {
-            return child;
-        } else if (nodes.containsKey(name)) {
-            return MISSING_NODE;
-        } else {
-            return base.getChildNode(name);
+        if (child == null) {
+            child = base.getChildNode(name);
         }
+        return child;
     }
 
     @Override
@@ -253,15 +235,17 @@ public class ModifiedNodeState extends A
 
     @Override
     public Iterable<? extends ChildNodeEntry> getChildNodeEntries() {
-        if (!exists()) {
+        if (!base.exists()) {
             return emptyList();
-        }
-        if (nodes.isEmpty()) {
+        } else if (nodes.isEmpty()) {
             return base.getChildNodeEntries(); // shortcut
+        } else {
+            Predicate<ChildNodeEntry> predicate = Predicates.compose(
+                    not(in(nodes.keySet())), ChildNodeEntry.GET_NAME);
+            return concat(
+                    filter(base.getChildNodeEntries(), predicate),
+                    iterable(filterValues(nodes, NodeState.EXISTS).entrySet()));
         }
-        return concat(
-                filter(base.getChildNodeEntries(), unmodifiedNodes),
-                iterable(filterValues(nodes, existingNodes).entrySet()));
     }
 
     /**
@@ -311,7 +295,7 @@ public class ModifiedNodeState extends A
             return false;
         }
 
-        for (Map.Entry<String, ? extends PropertyState> entry : properties.entrySet())
{
+        for (Map.Entry<String, PropertyState> entry : properties.entrySet()) {
             PropertyState before = base.getProperty(entry.getKey());
             PropertyState after = entry.getValue();
             if (before == null && after == null) {
@@ -331,11 +315,11 @@ public class ModifiedNodeState extends A
             }
         }
 
-        for (Map.Entry<String, ? extends NodeState> entry : nodes.entrySet()) {
+        for (Map.Entry<String, NodeState> entry : nodes.entrySet()) {
             String name = entry.getKey();
             NodeState before = base.getChildNode(name);
             NodeState after = entry.getValue();
-            if (after == null) {
+            if (!after.exists()) {
                 if (before.exists()) {
                     if (!diff.childNodeDeleted(name, before)) {
                         return false;
@@ -356,8 +340,7 @@ public class ModifiedNodeState extends A
     }
 
     public void compareAgainstBaseState(NodeStateDiff diff) {
-        for (Map.Entry<String, ? extends PropertyState> entry
-                : properties.entrySet()) {
+        for (Entry<String, PropertyState> entry : properties.entrySet()) {
             PropertyState before = base.getProperty(entry.getKey());
             PropertyState after = entry.getValue();
             if (after == null) {
@@ -369,11 +352,11 @@ public class ModifiedNodeState extends A
             }
         }
 
-        for (Map.Entry<String, ? extends NodeState> entry : nodes.entrySet()) {
+        for (Entry<String, NodeState> entry : nodes.entrySet()) {
             String name = entry.getKey();
             NodeState before = base.getChildNode(name);
             NodeState after = entry.getValue();
-            if (after == null) {
+            if (!after.exists()) {
                 if (before.exists()) { // TODO: can we assume this?
                     diff.childNodeDeleted(name, before);
                 }

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=1477716&r1=1477715&r2=1477716&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 16:35:10 2013
@@ -21,10 +21,8 @@ package org.apache.jackrabbit.oak.plugin
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.collect.Maps.newHashMap;
-import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
 import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE;
 
-import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
 
@@ -59,19 +57,19 @@ class MutableNodeState extends AbstractN
     private final Map<String, PropertyState> properties = newHashMap();
 
     /**
-     * Set of added, modified or removed ({@code null} value)
+     * Set of added, modified or removed (non-existent value)
      * child nodes.
      */
     private final Map<String, MutableNodeState> nodes = newHashMap();
 
-    private MutableNodeState(boolean exists) {
-        this.base = exists ? EMPTY_NODE : MISSING_NODE;
-    }
-
     MutableNodeState(@Nonnull NodeState base) {
-        if (checkNotNull(base) instanceof ModifiedNodeState) {
+        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) {
@@ -102,12 +100,10 @@ class MutableNodeState extends AbstractN
                 }
                 @Override
                 public boolean childNodeDeleted(String name, NodeState before) {
-                    nodes.put(name, null);
+                    nodes.put(name, new MutableNodeState(MISSING_NODE));
                     return true;
                 }
             });
-        } else {
-            this.base = base;
         }
     }
 
@@ -123,23 +119,16 @@ class MutableNodeState extends AbstractN
     private void reset(NodeState newBase) {
         assert base != null;
 
-        if (newBase instanceof ModifiedNodeState) {
-            ModifiedNodeState modified = (ModifiedNodeState) newBase;
-            base = modified.getBaseState();
-            properties.clear();
+        base = newBase;
+        properties.clear();
+        for (Entry<String, MutableNodeState> entry : nodes.entrySet()) {
+            entry.getValue().reset(base.getChildNode(entry.getKey()));
+        }
 
-            Iterator<Entry<String, MutableNodeState>> iterator =
-                    nodes.entrySet().iterator();
-            while (iterator.hasNext()) {
-                Entry<String, MutableNodeState> entry = iterator.next();
-                MutableNodeState cstate = entry.getValue();
-                NodeState cbase = newBase.getChildNode(entry.getKey());
-                if (!cbase.exists() || cstate == null) {
-                    iterator.remove();
-                } else {
-                    cstate.reset(cbase);
-                }
-            }
+        // unwrap ModifiedNodeState instances
+        if (base instanceof ModifiedNodeState) {
+            ModifiedNodeState modified = (ModifiedNodeState) base;
+            base = modified.getBaseState();
 
             modified.compareAgainstBaseState(new NodeStateDiff() {
                 @Override
@@ -160,10 +149,7 @@ class MutableNodeState extends AbstractN
                 }
                 @Override
                 public boolean childNodeAdded(String name, NodeState after) {
-                    MutableNodeState cstate = nodes.get(name);
-                    if (cstate != null) {
-                        cstate.reset(after);
-                    } else {
+                    if (!nodes.containsKey(name)) {
                         nodes.put(name, new MutableNodeState(after));
                     }
                     return true;
@@ -171,36 +157,19 @@ class MutableNodeState extends AbstractN
                 @Override
                 public boolean childNodeChanged(
                         String name, NodeState before, NodeState after) {
-                    MutableNodeState cstate = nodes.get(name);
-                    if (cstate != null) {
-                        cstate.reset(after);
-                    } else {
+                    if (!nodes.containsKey(name)) {
                         nodes.put(name, new MutableNodeState(after));
                     }
                     return true;
                 }
                 @Override
                 public boolean childNodeDeleted(String name, NodeState before) {
-                    nodes.put(name, null);
+                    if (!nodes.containsKey(name)) {
+                        nodes.put(name, new MutableNodeState(MISSING_NODE));
+                    }
                     return true;
                 }
             });
-        } else {
-            base = newBase;
-            properties.clear();
-
-            Iterator<Entry<String, MutableNodeState>> iterator =
-                    nodes.entrySet().iterator();
-            while (iterator.hasNext()) {
-                Entry<String, MutableNodeState> entry = iterator.next();
-                MutableNodeState cstate = entry.getValue();
-                NodeState cbase = newBase.getChildNode(entry.getKey());
-                if (!cbase.exists() || cstate == null) {
-                    iterator.remove();
-                } else {
-                    cstate.reset(cbase);
-                }
-            }
         }
     }
 
@@ -211,21 +180,12 @@ class MutableNodeState extends AbstractN
      */
     MutableNodeState getChildNode(String name, boolean connect) {
         assert base != null;
-
         MutableNodeState child = nodes.get(name);
-        if (child != null) {
-            return child;
-        }
-
-        if (nodes.containsKey(name)) {
-            // deleted: create new existing node if connect, otherwise non existing
-            child = new MutableNodeState(connect);
-        } else {
+        if (child == null) {
             child = new MutableNodeState(base.getChildNode(name));
-        }
-
-        if (connect) {
-            nodes.put(name, child);
+            if (connect) {
+                nodes.put(name, child);
+            }
         }
         return child;
     }
@@ -265,27 +225,28 @@ class MutableNodeState extends AbstractN
      * the set of child node names of {@code before}.
      */
     boolean isModified(NodeState before) {
-        if (nodes.isEmpty() && properties.isEmpty()) {
+        if (!exists()) {
+            return false;
+        } else if (nodes.isEmpty() && properties.isEmpty()) {
             return false;
         }
 
+        // was a child node added or removed?
         for (Entry<String, MutableNodeState> n : nodes.entrySet()) {
-            if (n.getValue() == null) {
-                return true;
-            }
-            if (!(before.hasChildNode(n.getKey()))) {
+            if (n.getValue().exists() != before.hasChildNode(n.getKey())) {
                 return true;
             }
         }
+
+        // was a property added, removed or modified
         for (Entry<String, PropertyState> p : properties.entrySet()) {
             PropertyState pState = p.getValue();
-            if (pState == null) {
-                return true;
-            }
-            if (!before.exists() || !pState.equals(before.getProperty(p.getKey()))) {
+            if (pState == null
+                    || !pState.equals(before.getProperty(p.getKey()))) {
                 return true;
             }
         }
+
         return false;
 
     }
@@ -297,12 +258,14 @@ class MutableNodeState extends AbstractN
      */
     boolean removeChildNode(String name) {
         assert base != null;
-
-        if (base.getChildNode(name).exists()) {
-            nodes.put(name, null);
-            return true;
+        MutableNodeState child = nodes.get(name);
+        if (child != null) {
+            boolean existed = child.exists();
+            child.reset(MISSING_NODE);
+            return existed;
         } else {
-            return nodes.remove(name) != null;
+            nodes.put(name, new MutableNodeState(MISSING_NODE));
+            return base.hasChildNode(name);
         }
     }
 
@@ -392,8 +355,6 @@ class MutableNodeState extends AbstractN
         NodeState child = nodes.get(name);
         if (child != null) {
             return child.exists();
-        } else if (nodes.containsKey(name)) {
-            return false;
         } else {
             return base.hasChildNode(name);
         }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ChildNodeEntry.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ChildNodeEntry.java?rev=1477716&r1=1477715&r2=1477716&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ChildNodeEntry.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ChildNodeEntry.java
Tue Apr 30 16:35:10 2013
@@ -18,6 +18,9 @@ package org.apache.jackrabbit.oak.spi.st
 
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import com.google.common.base.Function;
 
 /**
  * A {@code ChildNodeEntry} instance represents the child node states of a
@@ -47,4 +50,19 @@ public interface ChildNodeEntry {
     @Nonnull
     NodeState getNodeState();
 
+    /**
+     * Mapping from a ChildNodeEntry instance to its name.
+     */
+    Function<ChildNodeEntry, String> GET_NAME =
+            new Function<ChildNodeEntry, String>() {
+                @Override @Nullable
+                public String apply(@Nullable ChildNodeEntry input) {
+                    if (input != null) {
+                        return input.getName();
+                    } else {
+                        return null;
+                    }
+                }
+            };
+
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java?rev=1477716&r1=1477715&r2=1477716&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java
Tue Apr 30 16:35:10 2013
@@ -18,9 +18,12 @@ package org.apache.jackrabbit.oak.spi.st
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import org.apache.jackrabbit.oak.api.PropertyState;
 
+import com.google.common.base.Predicate;
+
 /**
  * A node in a content tree consists of child nodes and properties, each
  * of which evolves through different states during its lifecycle. This
@@ -307,4 +310,14 @@ public interface NodeState {
      */
     boolean compareAgainstBaseState(NodeState base, NodeStateDiff diff);
 
+    /**
+     * Predicate that checks the existence of NodeState instances.
+     */
+    Predicate<NodeState> EXISTS = new Predicate<NodeState>() {
+        @Override
+        public boolean apply(@Nullable NodeState input) {
+            return input != null && input.exists();
+        }
+    };
+
 }



Mime
View raw message