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 md...@apache.org on 2013/04/25 16:29:00 UTC
svn commit: r1475796 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/core/
main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/
main/java/org/apache/jackrabbit/oak/plugins/memory/
main/java/org/apache/jackra...
Author: mduerig
Date: Thu Apr 25 14:28:59 2013
New Revision: 1475796
URL: http://svn.apache.org/r1475796
Log:
OAK-781: Clarify / fix effects of MISSING_NODE as base state of NodeBuilder
replace isConnected with exists
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java?rev=1475796&r1=1475795&r2=1475796&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java Thu Apr 25 14:28:59 2013
@@ -197,7 +197,7 @@ public class TreeImpl implements Tree {
@Override
public boolean isConnected() {
enterNoStateCheck();
- if (parent == null || nodeBuilder.isConnected()) {
+ if (parent == null || nodeBuilder.exists()) {
return true;
}
@@ -206,16 +206,9 @@ public class TreeImpl implements Tree {
private boolean reconnect() {
if (parent != null && parent.reconnect()) {
- if (parent.nodeBuilder.hasChildNode(name)) {
- nodeBuilder = parent.nodeBuilder.child(name);
- }
- else {
- // make this builder disconnected from its new parent
- nodeBuilder = parent.nodeBuilder.child(name);
- parent.nodeBuilder.removeChildNode(name);
- }
+ nodeBuilder = parent.nodeBuilder.getChildNode(name);
}
- return nodeBuilder.isConnected();
+ return nodeBuilder.exists();
}
@Override
@@ -401,11 +394,7 @@ public class TreeImpl implements Tree {
@Override
public String toString() {
- if (nodeBuilder.isConnected()) {
- return getPathInternal() + ": " + getNodeState();
- } else {
- return "disconnected";
- }
+ return getPathInternal() + ": " + getNodeState();
}
//-----------------------------------------------------------< internal >---
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java?rev=1475796&r1=1475795&r2=1475796&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java Thu Apr 25 14:28:59 2013
@@ -119,7 +119,7 @@ public class ContentMirrorStoreStrategy
}
private static void pruneNode(NodeBuilder parent) {
- if (!parent.isConnected()) {
+ if (!parent.exists()) {
return;
}
for (String name : parent.getChildNodeNames()) {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java?rev=1475796&r1=1475795&r2=1475796&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java Thu Apr 25 14:28:59 2013
@@ -132,12 +132,6 @@ public class MemoryNodeBuilder implement
return this == root;
}
- private void checkConnected() {
- if (!isConnected()) {
- throw new IllegalStateException("This builder is not connected: " + name);
- }
- }
-
/**
* Update the base state of this builder by recursively retrieving it
* from the parent builder.
@@ -176,6 +170,10 @@ public class MemoryNodeBuilder implement
*/
@Nonnull
private MutableNodeState write() {
+ // TODO avoid traversing the parent hierarchy twice: once for exist and once for write
+ if (!exists()) {
+ throw new IllegalStateException("This builder does not exist: " + name);
+ }
return write(root.headRevision + 1);
}
@@ -217,7 +215,6 @@ public class MemoryNodeBuilder implement
@Override
public NodeState getNodeState() {
- checkConnected();
return read().snapshot();
}
@@ -228,24 +225,16 @@ public class MemoryNodeBuilder implement
@Override
public boolean exists() {
- checkConnected();
return read().exists();
}
@Override
public boolean isNew() {
- checkConnected();
return !isRoot() && !parent.base().hasChildNode(name) && parent.hasChildNode(name);
}
@Override
- public boolean isConnected() {
- return isRoot() || parent.read().isConnected(name);
- }
-
- @Override
public boolean isModified() {
- checkConnected();
return read().isModified(base());
}
@@ -260,19 +249,16 @@ public class MemoryNodeBuilder implement
@Override
public long getChildNodeCount() {
- checkConnected();
return read().getChildNodeCount();
}
@Override
public Iterable<String> getChildNodeNames() {
- checkConnected();
return read().getChildNodeNames();
}
@Override
public boolean hasChildNode(String name) {
- checkConnected();
return read().hasChildNode(checkNotNull(name));
}
@@ -287,7 +273,6 @@ public class MemoryNodeBuilder implement
@Override
public NodeBuilder getChildNode(String name) {
- checkConnected();
return createChildBuilder(checkNotNull(name));
}
@@ -298,7 +283,6 @@ public class MemoryNodeBuilder implement
@Override
public NodeBuilder setChildNode(String name, NodeState state) {
- checkConnected();
write().setChildNode(checkNotNull(name), checkNotNull(state));
MemoryNodeBuilder builder = createChildBuilder(name);
updated();
@@ -307,7 +291,6 @@ public class MemoryNodeBuilder implement
@Override
public NodeBuilder removeChildNode(String name) {
- checkConnected();
if (write().removeChildNode(checkNotNull(name))) {
updated();
}
@@ -316,25 +299,21 @@ public class MemoryNodeBuilder implement
@Override
public long getPropertyCount() {
- checkConnected();
return read().getPropertyCount();
}
@Override
public Iterable<? extends PropertyState> getProperties() {
- checkConnected();
return read().getProperties();
}
@Override
public boolean hasProperty(String name) {
- checkConnected();
return read().hasProperty(checkNotNull(name));
}
@Override
public PropertyState getProperty(String name) {
- checkConnected();
return read().getProperty(checkNotNull(name));
}
@@ -368,7 +347,6 @@ public class MemoryNodeBuilder implement
@Override
public NodeBuilder setProperty(PropertyState property) {
- checkConnected();
write().setProperty(checkNotNull(property));
updated();
return this;
@@ -388,7 +366,6 @@ public class MemoryNodeBuilder implement
@Override
public NodeBuilder removeProperty(String name) {
- checkConnected();
if (write().removeProperty(checkNotNull(name))) {
updated();
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java?rev=1475796&r1=1475795&r2=1475796&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java Thu Apr 25 14:28:59 2013
@@ -102,13 +102,6 @@ public interface NodeBuilder {
boolean isNew();
/**
- * Check whether this builder represents a disconnected node. A node is disconnected when it has been
- * part of a tree but has been subsequently removed.
- * @return {@code true} for a disconnected node
- */
- boolean isConnected();
-
- /**
* Check whether this builder represents a modified node, which has either modified properties
* or removed or added child nodes.
* @return {@code true} for a modified node
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java?rev=1475796&r1=1475795&r2=1475796&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java Thu Apr 25 14:28:59 2013
@@ -49,11 +49,6 @@ public class ReadOnlyBuilder implements
}
@Override
- public boolean isConnected() {
- return true;
- }
-
- @Override
public boolean isModified() {
return false;
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java?rev=1475796&r1=1475795&r2=1475796&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java Thu Apr 25 14:28:59 2013
@@ -32,7 +32,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
public class MemoryNodeBuilderTest {
@@ -107,25 +106,6 @@ public class MemoryNodeBuilderTest {
}
@Test
- public void testReadOnRemoveNode() {
- for (String name : new String[] {"x", "new"}) {
- NodeBuilder root = base.builder();
- NodeBuilder child = root.child(name);
-
- root.removeChildNode(name);
- try {
- child.getChildNodeCount();
- fail();
- } catch (IllegalStateException e) {
- // expected
- }
-
- root.child(name);
- assertEquals(0, child.getChildNodeCount()); // reconnect!
- }
- }
-
- @Test
public void testWriteOnRemoveNode() {
for (String name : new String[] {"x", "new"}) {
NodeBuilder root = base.builder();
@@ -187,21 +167,24 @@ public class MemoryNodeBuilderTest {
assertEquals(x.getBaseState(), x.getNodeState());
}
- @Test(expected = IllegalStateException.class)
- public void testReadOnRemovedNode() {
+ @Test
+ public void transitiveRemove() {
NodeBuilder root = base.builder();
- NodeBuilder m = root.child("m");
- NodeBuilder n = m.child("n");
+ NodeBuilder x = root.getChildNode("x");
+ NodeBuilder q = x.getChildNode("q");
+
+ assertTrue(x.exists());
+ assertTrue(q.exists());
- root.removeChildNode("m");
- n.hasChildNode("any");
+ root.removeChildNode("x");
+ assertFalse(q.exists());
+ assertFalse(x.exists());
}
@Test
public void testExistingStatus() {
NodeBuilder root = base.builder();
NodeBuilder x = root.child("x");
- assertTrue(x.isConnected());
assertTrue(x.exists());
assertFalse(x.isNew());
assertFalse(x.isModified());
@@ -212,7 +195,6 @@ public class MemoryNodeBuilderTest {
NodeBuilder root = base.builder();
NodeBuilder x = root.child("x");
x.setProperty("p", "pValue");
- assertTrue(x.isConnected());
assertTrue(x.exists());
assertFalse(x.isNew());
assertTrue(x.isModified());
@@ -223,26 +205,15 @@ public class MemoryNodeBuilderTest {
NodeBuilder root = base.builder();
NodeBuilder x = root.child("x");
root.removeChildNode("x");
- assertFalse(x.isConnected());
- try {
- assertTrue(x.exists());
- fail();
- } catch (IllegalStateException expected) {}
- try {
- assertFalse(x.isNew());
- fail();
- } catch (IllegalStateException expected) {}
- try {
- assertFalse(x.isModified());
- fail();
- } catch (IllegalStateException expected) {}
+ assertFalse(x.exists());
+ assertFalse(x.isNew());
+ assertFalse(x.isModified());
}
@Test
public void testNewStatus() {
NodeBuilder root = base.builder();
NodeBuilder n = root.child("n");
- assertTrue(n.isConnected());
assertTrue(n.exists());
assertTrue(n.isNew());
assertFalse(n.isModified());
@@ -260,15 +231,8 @@ public class MemoryNodeBuilderTest {
public void getNonExistingChildTest() {
NodeBuilder rootBuilder = base.builder();
NodeBuilder any = rootBuilder.getChildNode("any");
- assertFalse(any.isConnected());
- try {
- any.getChildNode("any");
- fail();
- } catch (IllegalStateException expected) {}
- try {
- any.exists();
- fail();
- } catch (IllegalStateException expected) {}
+ assertFalse(any.getChildNode("other").exists());
+ assertFalse(any.exists());
try {
any.setChildNode("any");
fail();
@@ -348,7 +312,6 @@ public class MemoryNodeBuilderTest {
}
@Test
- @Ignore("OAK-781")
public void modifyChildNodeOfNonExistingNode() {
NodeBuilder rootBuilder = EMPTY_NODE.builder();
@@ -448,7 +411,6 @@ public class MemoryNodeBuilderTest {
}
@Test
- @Ignore("OAK-781")
public void navigateNonExistingNode() {
NodeBuilder rootBuilder = EMPTY_NODE.builder();
@@ -470,7 +432,6 @@ public class MemoryNodeBuilderTest {
NodeBuilder cBuilder = bBuilder.getChildNode("c");
assertTrue(aBuilder.exists());
- assertFalse(bBuilder.isConnected());
assertTrue(cBuilder.exists());
cBuilder.setProperty("c2", "c2Value");