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 18:35:10 UTC
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
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();
+ }
+ };
+
}