From f698f968eca44766f1f3d84426021b16e54f2e19 Mon Sep 17 00:00:00 2001 From: Jukka Zitting Date: Thu, 4 Apr 2013 15:38:06 +0300 Subject: [PATCH] OAK-709: Consider moving permission evaluation to the node state level Rought draft for making NodeState.getChildNode() @Nonnull --- .../apache/jackrabbit/oak/core/ImmutableTree.java | 2 +- .../apache/jackrabbit/oak/core/ReadOnlyTree.java | 2 +- .../org/apache/jackrabbit/oak/core/TreeImpl.java | 23 +++-- .../jackrabbit/oak/kernel/KernelNodeState.java | 17 +++- .../oak/kernel/KernelNodeStoreBranch.java | 16 ++-- .../jackrabbit/oak/plugins/index/IndexUtils.java | 2 +- .../oak/plugins/index/p2/Property2IndexLookup.java | 2 +- .../p2/strategy/ContentMirrorStoreStrategy.java | 4 +- .../oak/plugins/memory/EmptyNodeState.java | 36 ++++++-- .../oak/plugins/memory/MemoryNodeBuilder.java | 53 +++++------ .../oak/plugins/memory/MemoryNodeState.java | 24 ++++- .../oak/plugins/memory/ModifiedNodeState.java | 27 ++++-- .../oak/plugins/nodetype/EffectiveType.java | 102 +++++++++------------ .../plugins/nodetype/ReadOnlyNodeTypeManager.java | 4 - .../oak/plugins/nodetype/TypeEditor.java | 8 +- .../oak/plugins/nodetype/TypeEditorProvider.java | 11 +-- .../oak/plugins/segment/SegmentNodeState.java | 12 ++- .../jackrabbit/oak/plugins/segment/Template.java | 7 +- .../apache/jackrabbit/oak/spi/query/Cursors.java | 9 +- .../oak/spi/state/AbstractNodeState.java | 11 ++- .../apache/jackrabbit/oak/spi/state/NodeState.java | 89 +++++++++++++----- .../jackrabbit/oak/spi/state/ReadOnlyBuilder.java | 2 +- .../jackrabbit/oak/kernel/KernelNodeStateTest.java | 10 +- .../jackrabbit/oak/kernel/KernelNodeStoreTest.java | 33 +++---- .../oak/kernel/LargeKernelNodeStateTest.java | 10 +- .../oak/plugins/index/IndexHookManagerTest.java | 2 +- .../org/apache/jackrabbit/oak/jcr/tck/QueryIT.java | 7 +- .../oak/plugins/index/lucene/LuceneIndex.java | 5 +- .../embedded/OakSolrNodeStateConfiguration.java | 2 +- .../embedded/UpToDateNodeStateConfiguration.java | 3 - .../UpToDateNodeStateConfigurationTest.java | 3 +- 31 files changed, 308 insertions(+), 230 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java index 83160fe..c163efc 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java @@ -113,7 +113,7 @@ public final class ImmutableTree extends ReadOnlyTree { @Override public ImmutableTree getChild(@Nonnull String name) { NodeState child = getNodeState().getChildNode(name); - if (child != null) { + if (child.exists()) { return new ImmutableTree(this, name, child); } else { return null; diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.java index 30b9a68..c7593e9 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ReadOnlyTree.java @@ -130,7 +130,7 @@ public class ReadOnlyTree implements Tree { @Override public ReadOnlyTree getChild(@Nonnull String name) { NodeState child = state.getChildNode(name); - if (child != null) { + if (child.exists()) { return new ReadOnlyTree(this, name, child); } else { return null; diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java index e92850e..d1808e0 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java @@ -96,8 +96,12 @@ public class TreeImpl implements Tree { this.name = checkNotNull(name); this.nodeBuilder = parent.getNodeBuilder().child(name); this.pendingMoves = checkNotNull(pendingMoves); - // readstatus is ALLOW_ALL for new items - readStatus = (getBaseState() == null) ? ReadStatus.ALLOW_ALL : ReadStatus.getChildStatus(parent.readStatus); + + if (getBaseState().exists()) { + readStatus = ReadStatus.getChildStatus(parent.readStatus); + } else { + readStatus = ReadStatus.ALLOW_ALL; // new items are always readable + } } @Override @@ -153,7 +157,7 @@ public class TreeImpl implements Tree { } NodeState parentBase = getBaseState(); - PropertyState base = parentBase == null ? null : parentBase.getProperty(name); + PropertyState base = parentBase.getProperty(name); if (base == null) { return Status.NEW; @@ -418,21 +422,16 @@ public class TreeImpl implements Tree { } /** - * The node state this tree is based on. {@code null} if this is a newly added tree. + * The (possibly non-existent) node state this tree is based on. * @return the base node state of this tree */ - @CheckForNull + @Nonnull final NodeState getBaseState() { if (parent == null) { return root.getBaseState(); + } else { + return parent.getBaseState().getChildNode(name); } - - NodeState parentBase = parent.getBaseState(); - if (parentBase != null) { - return parentBase.getChildNode(name); - } - - return null; } @Nonnull diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java index 7bedc5d..929da5d 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java @@ -20,6 +20,7 @@ package org.apache.jackrabbit.oak.kernel; import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty; import java.lang.reflect.InvocationHandler; @@ -63,6 +64,8 @@ import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; +import static com.google.common.base.Preconditions.checkArgument; + /** * Basic {@link NodeState} implementation based on the {@link MicroKernel} * interface. This class makes an attempt to load data lazily. @@ -219,6 +222,11 @@ public final class KernelNodeState extends AbstractNodeState { } @Override + public boolean exists() { + return true; + } + + @Override public long getPropertyCount() { init(); return properties.size(); @@ -247,12 +255,13 @@ public final class KernelNodeState extends AbstractNodeState { checkNotNull(name); init(); return childNames.contains(name) - || childNodeCount > MAX_CHILD_NODE_NAMES && getChildNode(name) != null; + || (childNodeCount > MAX_CHILD_NODE_NAMES + && getChildNode(name).exists()); } @Override public NodeState getChildNode(String name) { - checkNotNull(name); + // checkArgument(!checkNotNull(name).isEmpty()); // TODO: check in higher level init(); String childPath = null; if (childNames.contains(name)) { @@ -266,7 +275,7 @@ public final class KernelNodeState extends AbstractNodeState { if (state != NULL) { return state; } else { - return null; + return MISSING_NODE; } } // not able to tell from cache if node exists @@ -278,7 +287,7 @@ public final class KernelNodeState extends AbstractNodeState { } } if (childPath == null) { - return null; + return MISSING_NODE; } try { return cache.get(revision + childPath); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java index 273df4a..cd995c7 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java @@ -99,16 +99,16 @@ class KernelNodeStoreBranch extends AbstractNodeStoreBranch { @Override public boolean move(String source, String target) { checkNotMerged(); - if (getNode(source) == null) { + if (!getNode(source).exists()) { // source does not exist return false; } NodeState destParent = getNode(getParentPath(target)); - if (destParent == null) { + if (!destParent.exists()) { // parent of destination does not exist return false; } - if (destParent.getChildNode(getName(target)) != null) { + if (destParent.getChildNode(getName(target)).exists()) { // destination exists already return false; } @@ -120,16 +120,16 @@ class KernelNodeStoreBranch extends AbstractNodeStoreBranch { @Override public boolean copy(String source, String target) { checkNotMerged(); - if (getNode(source) == null) { + if (!getNode(source).exists()) { // source does not exist return false; } NodeState destParent = getNode(getParentPath(target)); - if (destParent == null) { + if (!destParent.exists()) { // parent of destination does not exist return false; } - if (destParent.getChildNode(getName(target)) != null) { + if (destParent.getChildNode(getName(target)).exists()) { // destination exists already return false; } @@ -213,11 +213,7 @@ class KernelNodeStoreBranch extends AbstractNodeStoreBranch { NodeState node = getHead(); for (String name : elements(path)) { node = node.getChildNode(name); - if (node == null) { - break; - } } - return node; } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java index 5c6a987..c0be8f8 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUtils.java @@ -118,7 +118,7 @@ public class IndexUtils implements IndexConstants { public static List buildIndexDefinitions(NodeState state, String indexConfigPath, String typeFilter) { NodeState definitions = state.getChildNode(INDEX_DEFINITIONS_NAME); - if (definitions == null) { + if (!definitions.exists()) { return Collections.emptyList(); } indexConfigPath = concat(indexConfigPath, INDEX_DEFINITIONS_NAME); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java index f849dc6..65bb56a 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java @@ -121,7 +121,7 @@ public class Property2IndexLookup { @Nullable private static NodeState getIndexDataNode(NodeState node, String propertyName, Filter filter) { NodeState state = node.getChildNode(INDEX_DEFINITIONS_NAME); - if (state == null) { + if (!state.exists()) { return null; } String filterNodeType = null; diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java index 9edfb56..0d3b259 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java @@ -149,7 +149,7 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { } else { for (String p : values) { NodeState property = index.getChildNode(p); - if (property != null) { + if (property.exists()) { // we have an entry for this value, so use it it.enqueue(Iterators.singletonIterator( new MemoryChildNodeEntry("", property))); @@ -181,7 +181,7 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { break; } NodeState s = index.getChildNode(p); - if (s != null) { + if (s.exists()) { CountingNodeVisitor v = new CountingNodeVisitor(max); v.visit(s); count += v.getEstimatedCount(); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java index d4776a0..3b320ab 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java @@ -16,6 +16,9 @@ */ package org.apache.jackrabbit.oak.plugins.memory; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + import java.util.Collections; import javax.annotation.CheckForNull; @@ -28,14 +31,24 @@ import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; /** - * Singleton instance of an empty node state, i.e. one with neither - * properties nor child nodes. + * Singleton instances of empty and non-existent node states, i.e. ones + * with neither properties nor child nodes. */ public final class EmptyNodeState implements NodeState { - public static final NodeState EMPTY_NODE = new EmptyNodeState(); + public static final NodeState EMPTY_NODE = new EmptyNodeState(true); + + public static final NodeState MISSING_NODE = new EmptyNodeState(false); + + private final boolean exists; + + private EmptyNodeState(boolean exists) { + this.exists = exists; + } - private EmptyNodeState() { + @Override + public boolean exists() { + return exists; } @Override @@ -60,12 +73,14 @@ public final class EmptyNodeState implements NodeState { @Override public boolean hasChildNode(@Nonnull String name) { + checkArgument(!checkNotNull(name).isEmpty()); return false; } - @Override @CheckForNull + @Override @Nonnull public NodeState getChildNode(@Nonnull String name) { - return null; + checkArgument(!checkNotNull(name).isEmpty()); + return MISSING_NODE; } @Override @@ -85,7 +100,7 @@ public final class EmptyNodeState implements NodeState { @Override public void compareAgainstBaseState(NodeState base, NodeStateDiff diff) { - if (base != EMPTY_NODE) { + if (base != EMPTY_NODE && base.exists()) { for (PropertyState before : base.getProperties()) { diff.propertyDeleted(before); } @@ -97,7 +112,7 @@ public final class EmptyNodeState implements NodeState { public static void compareAgainstEmptyState( NodeState state, NodeStateDiff diff) { - if (state != EMPTY_NODE) { + if (state != EMPTY_NODE && state.exists()) { for (PropertyState after : state.getProperties()) { diff.propertyAdded(after); } @@ -114,8 +129,8 @@ public final class EmptyNodeState implements NodeState { } public boolean equals(Object object) { - if (object == EMPTY_NODE) { - return true; + if (object == EMPTY_NODE || object == MISSING_NODE) { + return exists == (object == EMPTY_NODE); } else if (object instanceof NodeState) { NodeState that = (NodeState) object; return that.getPropertyCount() == 0 @@ -128,4 +143,5 @@ public final class EmptyNodeState implements NodeState { public int hashCode() { return 0; } + } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java index 58ba6c2..be7f9b0 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java @@ -31,9 +31,11 @@ import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; import static org.apache.jackrabbit.oak.plugins.memory.ModifiedNodeState.with; import static org.apache.jackrabbit.oak.plugins.memory.ModifiedNodeState.withNodes; import static org.apache.jackrabbit.oak.plugins.memory.ModifiedNodeState.withProperties; @@ -96,7 +98,7 @@ public class MemoryNodeBuilder implements NodeBuilder { private long revision; /** - * The base state of this builder, or {@code null} if this builder + * The base state of this builder, possibly non-existent if this builder * represents a new node that didn't yet exist in the base content tree. */ private NodeState baseState; @@ -120,7 +122,7 @@ public class MemoryNodeBuilder implements NodeBuilder { this.root = parent.root; this.revision = -1; - this.baseState = null; + this.baseState = parent.baseState.getChildNode(name); this.writeState = null; } @@ -143,7 +145,7 @@ public class MemoryNodeBuilder implements NodeBuilder { private boolean classInvariants() { boolean rootHasNoParent = isRoot() == (parent == null); boolean rootHasWriteState = root.writeState != null; - boolean baseStateOrWriteStateNotNull = baseState != null || writeState != null; + boolean baseStateOrWriteStateNotNull = baseState.exists() || writeState != null; return rootHasNoParent && rootHasWriteState && baseStateOrWriteStateNotNull; } @@ -158,11 +160,7 @@ public class MemoryNodeBuilder implements NodeBuilder { * @return base state of the child */ private NodeState getBaseState(String name) { - if (baseState != null) { - return baseState.getChildNode(name); - } else { - return null; - } + return baseState.getChildNode(name); } /** @@ -172,7 +170,7 @@ public class MemoryNodeBuilder implements NodeBuilder { * @return {@code true} iff the base state has a child of the given name. */ private boolean hasBaseState(String name) { - return baseState != null && baseState.hasChildNode(name); + return baseState.hasChildNode(name); } /** @@ -196,11 +194,9 @@ public class MemoryNodeBuilder implements NodeBuilder { private boolean exists() { if (isRoot()) { return true; - } - else if (parent.writeState == null) { - return parent.baseState != null && parent.baseState.hasChildNode(name); - } - else { + } else if (parent.writeState == null) { + return parent.baseState.hasChildNode(name); + } else { return parent.writeState.hasChildNode(name); } } @@ -226,7 +222,7 @@ public class MemoryNodeBuilder implements NodeBuilder { return false; } - return writeState != null || baseState != null; + return writeState != null || baseState.exists(); } @Nonnull @@ -258,7 +254,7 @@ public class MemoryNodeBuilder implements NodeBuilder { writeState = parent.getWriteState(name); if (writeState == null) { if (exists()) { - assert baseState != null; + assert baseState.exists(); writeState = new MutableNodeState(baseState); } else { @@ -340,7 +336,8 @@ public class MemoryNodeBuilder implements NodeBuilder { if (pState == null) { return true; } - if (baseState == null || !pState.equals(baseState.getProperty(p.getKey()))) { + if (!baseState.exists() + || !pState.equals(baseState.getProperty(p.getKey()))) { return true; } } @@ -354,7 +351,7 @@ public class MemoryNodeBuilder implements NodeBuilder { if (writeState != null) { return writeState.snapshot(); } else { - assert baseState != null; + assert baseState.exists(); return baseState; } } @@ -407,7 +404,7 @@ public class MemoryNodeBuilder implements NodeBuilder { public NodeBuilder removeNode(String name) { write(); - if (writeState.base.getChildNode(name) != null) { + if (writeState.base.getChildNode(name).exists()) { writeState.nodes.put(name, null); } else { writeState.nodes.remove(name); @@ -505,15 +502,6 @@ public class MemoryNodeBuilder implements NodeBuilder { private final Map nodes = Maps.newHashMap(); - /** - * Determine whether the a node with the given name was removed - * @param name name of the node - * @return {@code true} iff a node with the given name was removed - */ - private boolean isRemoved(String name) { - return nodes.containsKey(name) && nodes.get(name) == null; - } - public MutableNodeState(NodeState base) { if (base != null) { this.base = base; @@ -552,7 +540,7 @@ public class MemoryNodeBuilder implements NodeBuilder { Map.Entry entry = iterator.next(); MutableNodeState cstate = entry.getValue(); NodeState cbase = newBase.getChildNode(entry.getKey()); - if (cbase == null || cstate == null) { + if (!cbase.exists() || cstate == null) { iterator.remove(); } else { cstate.reset(cbase); @@ -563,6 +551,11 @@ public class MemoryNodeBuilder implements NodeBuilder { //-----------------------------------------------------< NodeState >-- @Override + public boolean exists() { + return true; + } + + @Override public long getPropertyCount() { return withProperties(base, properties).getPropertyCount(); } @@ -586,12 +579,14 @@ public class MemoryNodeBuilder implements NodeBuilder { @Override public boolean hasChildNode(String name) { checkNotNull(name); + // checkArgument(!name.isEmpty()); TODO: should be caught earlier return withNodes(base, nodes).hasChildNode(name); } @Override public NodeState getChildNode(String name) { checkNotNull(name); + checkArgument(!name.isEmpty()); return withNodes(base, nodes).getChildNode(name); // mutable } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java index 780ed92..1ddc321 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java @@ -16,7 +16,10 @@ */ package org.apache.jackrabbit.oak.plugins.memory; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.spi.state.AbstractNodeState; @@ -56,6 +59,11 @@ class MemoryNodeState extends AbstractNodeState { } @Override + public boolean exists() { + return true; + } + + @Override public PropertyState getProperty(String name) { return properties.get(name); } @@ -72,14 +80,19 @@ class MemoryNodeState extends AbstractNodeState { @Override public boolean hasChildNode(String name) { - checkNotNull(name); + checkArgument(!checkNotNull(name).isEmpty()); return nodes.containsKey(name); } @Override public NodeState getChildNode(String name) { - checkNotNull(name); - return nodes.get(name); + checkArgument(!checkNotNull(name).isEmpty()); + NodeState state = nodes.get(name); + if (state != null) { + return state; + } else { + return MISSING_NODE; + } } @Override @@ -104,6 +117,11 @@ class MemoryNodeState extends AbstractNodeState { */ @Override public void compareAgainstBaseState(NodeState base, NodeStateDiff diff) { + if (base == EMPTY_NODE || !base.exists()) { + EmptyNodeState.compareAgainstEmptyState(this, diff); + return; + } + Map newProperties = new HashMap(properties); for (PropertyState before : base.getProperties()) { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java index 7c8ae46..e1a94f3 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java @@ -16,6 +16,7 @@ */ package org.apache.jackrabbit.oak.plugins.memory; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Predicates.in; import static com.google.common.base.Predicates.not; @@ -24,6 +25,7 @@ import static com.google.common.collect.Collections2.filter; import static com.google.common.collect.Iterables.concat; import static com.google.common.collect.Iterables.filter; import static com.google.common.collect.Maps.filterValues; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; import static org.apache.jackrabbit.oak.plugins.memory.MemoryChildNodeEntry.iterable; import java.util.Map; @@ -142,6 +144,11 @@ public class ModifiedNodeState extends AbstractNodeState { } @Override + public boolean exists() { + return true; + } + + @Override public long getPropertyCount() { long count = base.getPropertyCount(); @@ -191,7 +198,7 @@ public class ModifiedNodeState extends AbstractNodeState { long count = base.getChildNodeCount(); for (Map.Entry entry : nodes.entrySet()) { - if (base.getChildNode(entry.getKey()) != null) { + if (base.getChildNode(entry.getKey()).exists()) { count--; } if (entry.getValue() != null) { @@ -204,12 +211,12 @@ public class ModifiedNodeState extends AbstractNodeState { @Override public boolean hasChildNode(String name) { - checkNotNull(name); + // checkArgument(!checkNotNull(name).isEmpty()); // TODO: should be caught earlier NodeState child = nodes.get(name); if (child != null) { return true; } else if (nodes.containsKey(name)) { - return false; // removed + return false; } else { return base.hasChildNode(name); } @@ -217,12 +224,12 @@ public class ModifiedNodeState extends AbstractNodeState { @Override public NodeState getChildNode(String name) { - checkNotNull(name); + checkArgument(!checkNotNull(name).isEmpty()); NodeState child = nodes.get(name); if (child != null) { return child; } else if (nodes.containsKey(name)) { - return null; // removed + return MISSING_NODE; } else { return base.getChildNode(name); } @@ -327,11 +334,11 @@ public class ModifiedNodeState extends AbstractNodeState { String name = entry.getKey(); NodeState before = base.getChildNode(name); NodeState after = entry.getValue(); - if (before == null && after == null) { - // do nothing - } else if (after == null) { - diff.childNodeDeleted(name, before); - } else if (before == null) { + if (after == null) { + if (before.exists()) { + diff.childNodeDeleted(name, before); + } + } else if (!before.exists()) { diff.childNodeAdded(name, after); } else if (!before.equals(after)) { diff.childNodeChanged(name, before, after); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java index 7d9ad3a..bba4455 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java @@ -51,32 +51,28 @@ class EffectiveType { for (NodeState type : types) { NodeState properties = type.getChildNode("oak:namedPropertyDefinitions"); - if (properties != null) { - for (ChildNodeEntry entry : properties.getChildNodeEntries()) { - String name = entry.getName(); - if ("oak:primaryType".equals(name)) { - name = JCR_PRIMARYTYPE; - } else if ("oak:mixinTypes".equals(name)) { - name = JCR_MIXINTYPES; - } else if ("oak:uuid".equals(name)) { - name = JCR_UUID; - } - if (node.getProperty(name) == null - && isMandatory(name, entry.getNodeState())) { - builder.add(name); - } + for (ChildNodeEntry entry : properties.getChildNodeEntries()) { + String name = entry.getName(); + if ("oak:primaryType".equals(name)) { + name = JCR_PRIMARYTYPE; + } else if ("oak:mixinTypes".equals(name)) { + name = JCR_MIXINTYPES; + } else if ("oak:uuid".equals(name)) { + name = JCR_UUID; + } + if (node.getProperty(name) == null + && isMandatory(name, entry.getNodeState())) { + builder.add(name); } } NodeState childNodes = type.getChildNode("oak:namedChildNodeDefinitions"); - if (childNodes != null) { - for (ChildNodeEntry entry : childNodes.getChildNodeEntries()) { - String name = entry.getName(); - if (!node.hasChildNode(name) - && isMandatory(name, entry.getNodeState())) { - builder.add(name); - } + for (ChildNodeEntry entry : childNodes.getChildNodeEntries()) { + String name = entry.getName(); + if (!node.hasChildNode(name) + && isMandatory(name, entry.getNodeState())) { + builder.add(name); } } } @@ -117,22 +113,17 @@ class EffectiveType { // Find matching named property definition for (NodeState type : types) { NodeState named = type.getChildNode("oak:namedPropertyDefinitions"); - if (named != null) { - NodeState definitions = named.getChildNode(escapedName); - if (definitions != null) { - NodeState definition = definitions.getChildNode(definedType); - if (definition == null) { - definition = definitions.getChildNode(undefinedType); - } - if (definition != null) { - return definition; + NodeState definitions = named.getChildNode(escapedName); + NodeState definition = definitions.getChildNode(definedType); + if (!definition.exists()) { + definition = definitions.getChildNode(undefinedType); + } + if (definition.exists()) { + return definition; // TODO: Fall back to residual definitions until we have consensus on OAK-709 -// } else { -// throw new ConstraintViolationException( -// "No matching definition found for property " -// + propertyName); - } - } +// } else { +// throw new ConstraintViolationException( +// "No matching definition found for property " + propertyName); } } @@ -140,14 +131,12 @@ class EffectiveType { for (NodeState type : types) { NodeState residual = type.getChildNode("oak:residualPropertyDefinitions"); - if (residual != null) { - NodeState definition = residual.getChildNode(definedType); - if (definition == null) { - definition = residual.getChildNode(undefinedType); - } - if (definition != null) { - return definition; - } + NodeState definition = residual.getChildNode(definedType); + if (!definition.exists()) { + definition = residual.getChildNode(undefinedType); + } + if (definition.exists()) { + return definition; } } @@ -180,20 +169,17 @@ class EffectiveType { // Find matching named child node definition for (NodeState type : types) { NodeState named = type.getChildNode("oak:namedChildNodeDefinitions"); - if (named != null) { - NodeState definitions = named.getChildNode(nodeName); - if (definitions != null) { - for (String typeName : nodeType) { - NodeState definition = definitions.getChildNode(typeName); - if (definition != null) { - return definition; - } + NodeState definitions = named.getChildNode(nodeName); + if (definitions.exists()) { + for (String typeName : nodeType) { + NodeState definition = definitions.getChildNode(typeName); + if (definition.exists()) { + return definition; } - -// TODO: Fall back to residual definitions until we have consensus on OAK-709 -// throw new ConstraintViolationException( -// "Incorrect node type of child node " + nodeName); } +// TODO: Fall back to residual definitions until we have consensus on OAK-709 +// throw new ConstraintViolationException( +// "Incorrect node type of child node " + nodeName); } } @@ -201,10 +187,10 @@ class EffectiveType { for (NodeState type : types) { NodeState residual = type.getChildNode("oak:residualChildNodeDefinitions"); - if (residual != null) { + if (residual.exists()) { for (String typeName : nodeType) { NodeState definition = residual.getChildNode(typeName); - if (definition != null) { + if (definition.exists()) { return definition; } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java index 2f25fc8..ecbb342 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java @@ -20,7 +20,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.oak.api.Type.STRING; -import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.NODE_TYPES_PATH; import java.util.LinkedList; @@ -149,9 +148,6 @@ public abstract class ReadOnlyNodeTypeManager implements NodeTypeManager, Effect NodeState typesNode = root; for (String name : PathUtils.elements(NODE_TYPES_PATH)) { typesNode = typesNode.getChildNode(name); - if (typesNode == null) { - typesNode = EMPTY_NODE; - } } final Tree typesTree = new ReadOnlyTree(typesNode); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java index 2fdf9f3..cd07932 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java @@ -246,7 +246,7 @@ class TypeEditor extends DefaultEditor { names.add(name); NodeState type = types.getChildNode(name); - if (type == null) { + if (!type.exists()) { throw constraintViolation( "Primary node type " + name + " does not exist"); } else if (getBoolean(type, JCR_ISMIXIN)) { @@ -266,7 +266,7 @@ class TypeEditor extends DefaultEditor { for (String name : mixins.getValue(NAMES)) { if (names.add(name)) { NodeState type = types.getChildNode(name); - if (type == null) { + if (!type.exists()) { throw constraintViolation( "Mixin node type " + name + " does not exist"); } else if (!getBoolean(type, JCR_ISMIXIN)) { @@ -292,7 +292,7 @@ class TypeEditor extends DefaultEditor { for (String name : supertypes.getValue(NAMES)) { if (names.add(name)) { NodeState supertype = types.getChildNode(name); - if (supertype != null) { + if (supertype.exists()) { list.add(supertype); queue.add(supertype); } else { @@ -306,7 +306,7 @@ class TypeEditor extends DefaultEditor { // always include nt:base if (names.add(NT_BASE)) { NodeState base = types.getChildNode(NT_BASE); - if (base != null) { + if (base.exists()) { list.add(base); } else { // TODO: ignore/warning/error? diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditorProvider.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditorProvider.java index 68efed8..0c5fec2 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditorProvider.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditorProvider.java @@ -35,13 +35,12 @@ public class TypeEditorProvider implements EditorProvider { public Editor getRootEditor( NodeState before, NodeState after, NodeBuilder builder) { NodeState system = after.getChildNode(JCR_SYSTEM); - if (system != null) { - NodeState types = system.getChildNode(JCR_NODE_TYPES); - if (types != null) { - return new VisibleEditor(new TypeEditor(types)); - } + NodeState types = system.getChildNode(JCR_NODE_TYPES); + if (types.exists()) { + return new VisibleEditor(new TypeEditor(types)); + } else { + return null; } - return null; } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java index 033e35e..df0bc4c 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java @@ -16,6 +16,7 @@ */ package org.apache.jackrabbit.oak.plugins.segment; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; @@ -69,6 +70,11 @@ class SegmentNodeState extends AbstractNodeState { } @Override + public boolean exists() { + return true; + } + + @Override public long getPropertyCount() { return getTemplate().getPropertyCount(); } @@ -91,12 +97,14 @@ class SegmentNodeState extends AbstractNodeState { @Override public boolean hasChildNode(String name) { - return getTemplate().hasChildNode(checkNotNull(name), store, recordId); + checkArgument(!checkNotNull(name).isEmpty()); + return getTemplate().hasChildNode(name, store, recordId); } @Override @CheckForNull public NodeState getChildNode(String name) { - return getTemplate().getChildNode(checkNotNull(name), store, recordId); + checkArgument(!checkNotNull(name).isEmpty()); + return getTemplate().getChildNode(name, store, recordId); } @Override diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java index 10fc6d1..795da63 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java @@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.plugins.segment; import static com.google.common.base.Preconditions.checkElementIndex; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; import static org.apache.jackrabbit.oak.plugins.segment.Segment.RECORD_ID_BYTES; import java.util.Arrays; @@ -274,14 +275,14 @@ class Template { public NodeState getChildNode( String name, SegmentStore store, RecordId recordId) { if (hasNoChildNodes()) { - return null; + return MISSING_NODE; } else if (hasManyChildNodes()) { MapRecord map = getChildNodeMap(store, recordId); RecordId childNodeId = map.getEntry(name); if (childNodeId != null) { return new SegmentNodeState(store, childNodeId); } else { - return null; + return MISSING_NODE; } } else if (name.equals(childName)) { int offset = recordId.getOffset() + RECORD_ID_BYTES; @@ -289,7 +290,7 @@ class Template { RecordId childNodeId = segment.readRecordId(offset); return new SegmentNodeState(store, childNodeId); } else { - return null; + return MISSING_NODE; } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java index c8a23e2..51aca85 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java @@ -178,11 +178,10 @@ public class Cursors { parent = node; node = parent.getChildNode(name); - - if (node == null) { - // nothing can match this filter, leave nodes empty - return; - } + } + if (!node.exists()) { + // nothing can match this filter, leave nodes empty + return; } } Filter.PathRestriction restriction = filter.getPathRestriction(); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java index 3a4d7a7..1a698c4 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java @@ -58,7 +58,7 @@ public abstract class AbstractNodeState implements NodeState { @Override public boolean hasChildNode(String name) { - return getChildNode(name) != null; + return getChildNode(name).exists(); } @Override @@ -102,7 +102,7 @@ public abstract class AbstractNodeState implements NodeState { String name = beforeCNE.getName(); NodeState beforeChild = beforeCNE.getNodeState(); NodeState afterChild = getChildNode(name); - if (afterChild == null) { + if (!afterChild.exists()) { diff.childNodeDeleted(name, beforeChild); } else { baseChildNodes.add(name); @@ -125,6 +125,9 @@ public abstract class AbstractNodeState implements NodeState { * @return string representation */ public String toString() { + if (!exists()) { + return "{N/A}"; + } StringBuilder builder = new StringBuilder("{"); String separator = " "; for (PropertyState property : getProperties()) { @@ -161,6 +164,10 @@ public abstract class AbstractNodeState implements NodeState { NodeState other = (NodeState) that; + if (exists() != other.exists()) { + return false; + } + if (getPropertyCount() != other.getPropertyCount() || getChildNodeCount() != other.getChildNodeCount()) { return false; diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java index 375ac00..e4a694b 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java @@ -64,11 +64,36 @@ import org.apache.jackrabbit.oak.api.PropertyState; * as {@link RuntimeException unchecked exceptions} that higher level code * is not expected to be able to recover from. *

- * Since this interface exposes no higher level constructs like access - * controls, locking, node types or even path parsing, there's no way - * for content access to fail because of such concerns. Such functionality - * and related checked exceptions or other control flow constructs should - * be implemented on a higher level above this interface. + * Since this interface exposes no higher level constructs like locking, + * node types or even path parsing, there's no way for content access to + * fail because of such concerns. Such functionality and related checked + * exceptions or other control flow constructs should be implemented on + * a higher level above this interface. On the other hand read access + * controls can be implemented below this interface, in which + * case some content that would otherwise be accessible might not show + * up through such an implementation. + * + *

Existence and iterability of node states

+ *

+ * The {@link #getChildNode(String)} method is special in that it + * never returns a {@code null} value, even if the named child + * node does not exist. Instead a client should use the {@link #exists()} + * method on the returned child state to check whether that node exists. + * The purpose of this separation of concerns is to allow an implementation + * to lazily load content only when it's actually read instead of just + * traversed. It also simplifies client code by avoiding the need for many + * {@code null} checks when traversing paths. + *

+ * The iterability of a node is a related concept to the + * above-mentioned existence. A node state is iterable if it + * is included in the return values of the {@link #getChildNodeCount()}, + * {@link #getChildNodeNames()} and {@link #getChildNodeEntries()} methods. + * An iterable node is guaranteed to exist, though not all existing nodes + * are necessarily iterable. + *

+ * Furthermore, a non-existing node is guaranteed to contain no properties + * or iterable child nodes. It can, however contain non-iterable children. + * Such scenarios are typically the result of access control restrictions. * *

Decoration and virtual content

*

@@ -77,7 +102,7 @@ import org.apache.jackrabbit.oak.api.PropertyState; * like for example the aggregate size of the entire subtree as an * extra virtual property. A virtualization, sharding or caching layer * could provide a composite view over multiple underlying trees. - * Or a basic access control layer could decide to hide certain content + * Or a an access control layer could decide to hide certain content * based on specific rules. All such features need to be implemented * according to the API contract of this interface. A separate higher level * interface needs to be used if an implementation can't for example @@ -85,8 +110,8 @@ import org.apache.jackrabbit.oak.api.PropertyState; * *

Equality and hash codes

*

- * Two node states are considered equal if and only if their properties and - * child nodes match, regardless of ordering. The + * Two node states are considered equal if and only if their existence, + * properties and iterable child nodes match, regardless of ordering. The * {@link Object#equals(Object)} method needs to be implemented so that it * complies with this definition. And while node states are not meant for * use as hash keys, the {@link Object#hashCode()} method should still be @@ -95,13 +120,21 @@ import org.apache.jackrabbit.oak.api.PropertyState; public interface NodeState { /** + * Checks whether this node exists. See the above discussion about + * the existence of node states. + * + * @return {@code true} if this node exists, {@code false} if not + */ + boolean exists(); + + /** * Returns the named property. The name is an opaque string and * is not parsed or otherwise interpreted by this method. *

* The namespace of properties and child nodes is shared, so if * this method returns a non-{@code null} value for a given * name, then {@link #getChildNode(String)} is guaranteed to return - * {@code null} for the same name. + * a non-existing {@link NodeState} for the same name. * * @param name name of the property to return * @return named property, or {@code null} if not found @@ -128,7 +161,8 @@ public interface NodeState { Iterable getProperties(); /** - * Checks whether the named child node exists. + * Checks whether the named child node exists. The implementation + * is equivalent to {@code getChildNode(name).exists()}. * * @param name name of the child node * @return {@code true} if the named child node exists, @@ -137,40 +171,42 @@ public interface NodeState { boolean hasChildNode(@Nonnull String name); /** - * Returns the named child node. The name is an opaque string and - * is not parsed or otherwise interpreted by this method. + * Returns the named, possibly non-existent, child node. The name is an + * opaque string and is not parsed or otherwise interpreted by this method. + * Use the {@link #exists()} method on the returned child node to + * determine whether the node exists or not. *

* The namespace of properties and child nodes is shared, so if - * this method returns a non-{@code null} value for a given - * name, then {@link #getProperty(String)} is guaranteed to return - * {@code null} for the same name. + * this method returns an existing {@link NodeState} for + * a given name, then {@link #getProperty(String)} is guaranteed + * to return {@code null} for the same name. * * @param name name of the child node to return - * @return named child node, or {@code null} if not found + * @return named child node */ - @CheckForNull + @Nonnull NodeState getChildNode(@Nonnull String name); /** - * Returns the number of child nodes of this node. + * Returns the number of iterable child nodes of this node. * - * @return number of child nodes + * @return number of iterable child nodes */ long getChildNodeCount(); /** - * Returns the names of all child nodes. + * Returns the names of all iterable child nodes. * * @return child node names in some stable order */ Iterable getChildNodeNames(); /** - * Returns an iterable of the child node entries of this instance. Multiple - * iterations are guaranteed to return the child nodes in the same order, - * but the specific order used is implementation dependent and may change - * across different states of the same node. - *

+ * Returns the iterable child node entries of this instance. + * Multiple iterations are guaranteed to return the child nodes in + * the same order, but the specific order used is implementation + * dependent and may change across different states of the same node. + *

* Note on cost and performance: while it is possible to iterate over * all child {@code NodeState}s with the two methods {@link * #getChildNodeNames()} and {@link #getChildNode(String)}, this method is @@ -199,6 +235,9 @@ public interface NodeState { * Compares this node state against the given base state. Any differences * are reported by calling the relevant added/changed/deleted methods of * the given handler. + *

+ * TODO: Define the behavior of this method with regards to + * iterability/existence of child nodes. * * @param base base state * @param diff handler of node state differences diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java index 493f1ee..e2e0f09 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/ReadOnlyBuilder.java @@ -130,7 +130,7 @@ public class ReadOnlyBuilder implements NodeBuilder { @Override public ReadOnlyBuilder child(String name) { NodeState child = state.getChildNode(name); - if (child != null) { + if (child.exists()) { return new ReadOnlyBuilder(child); } else { throw unsupported(); diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStateTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStateTest.java index 3ac8d29..f3d3d24 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStateTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStateTest.java @@ -37,8 +37,10 @@ import org.junit.Before; import org.junit.Test; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertNull; +import static junit.framework.Assert.assertTrue; import static org.apache.jackrabbit.oak.api.Type.LONG; public class KernelNodeStateTest { @@ -104,10 +106,10 @@ public class KernelNodeStateTest { @Test public void testGetChildNode() { - assertNotNull(state.getChildNode("x")); - assertNotNull(state.getChildNode("y")); - assertNotNull(state.getChildNode("z")); - assertNull(state.getChildNode("a")); + assertTrue(state.getChildNode("x").exists()); + assertTrue(state.getChildNode("y").exists()); + assertTrue(state.getChildNode("z").exists()); + assertFalse(state.getChildNode("a").exists()); } @Test diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java index 1a96f33..8583d99 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java @@ -37,6 +37,7 @@ import org.apache.jackrabbit.oak.spi.state.NodeStoreBranch; import org.junit.Before; import org.junit.Test; +import static junit.framework.Assert.assertFalse; import static org.apache.jackrabbit.oak.api.Type.LONG; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -88,34 +89,34 @@ public class KernelNodeStoreTest { // Assert changes are present in the builder NodeState testState = rootBuilder.getNodeState().getChildNode("test"); - assertNotNull(testState.getChildNode("newNode")); - assertNull(testState.getChildNode("x")); + assertTrue(testState.getChildNode("newNode").exists()); + assertFalse(testState.getChildNode("x").exists()); assertEquals(42, (long) testState.getChildNode("newNode").getProperty("n").getValue(LONG)); // Assert changes are not yet present in the branch testState = branch.getHead().getChildNode("test"); - assertNull(testState.getChildNode("newNode")); - assertNotNull(testState.getChildNode("x")); + assertFalse(testState.getChildNode("newNode").exists()); + assertTrue(testState.getChildNode("x").exists()); branch.setRoot(rootBuilder.getNodeState()); // Assert changes are present in the branch testState = branch.getHead().getChildNode("test"); - assertNotNull(testState.getChildNode("newNode")); - assertNull(testState.getChildNode("x")); + assertTrue(testState.getChildNode("newNode").exists()); + assertFalse(testState.getChildNode("x").exists()); assertEquals(42, (long) testState.getChildNode("newNode").getProperty("n").getValue(LONG)); // Assert changes are not yet present in the trunk testState = store.getRoot().getChildNode("test"); - assertNull(testState.getChildNode("newNode")); - assertNotNull(testState.getChildNode("x")); + assertFalse(testState.getChildNode("newNode").exists()); + assertTrue(testState.getChildNode("x").exists()); branch.merge(EmptyHook.INSTANCE); // Assert changes are present in the trunk testState = store.getRoot().getChildNode("test"); - assertNotNull(testState.getChildNode("newNode")); - assertNull(testState.getChildNode("x")); + assertTrue(testState.getChildNode("newNode").exists()); + assertFalse(testState.getChildNode("x").exists()); assertEquals(42, (long) testState.getChildNode("newNode").getProperty("n").getValue(LONG)); } @@ -151,9 +152,9 @@ public class KernelNodeStoreTest { assertNotNull(before); assertNotNull(after); - assertNull(before.getChildNode("test").getChildNode("newNode")); - assertNotNull(after.getChildNode("test").getChildNode("newNode")); - assertNull(after.getChildNode("test").getChildNode("a")); + assertFalse(before.getChildNode("test").getChildNode("newNode").exists()); + assertTrue(after.getChildNode("test").getChildNode("newNode").exists()); + assertFalse(after.getChildNode("test").getChildNode("a").exists()); assertEquals(42, (long) after.getChildNode("test").getChildNode("newNode").getProperty("n").getValue(LONG)); assertEquals(newRoot, after); } @@ -184,9 +185,9 @@ public class KernelNodeStoreTest { }); NodeState test = store.getRoot().getChildNode("test"); - assertNotNull(test.getChildNode("newNode")); - assertNotNull(test.getChildNode("fromHook")); - assertNull(test.getChildNode("a")); + assertTrue(test.getChildNode("newNode").exists()); + assertTrue(test.getChildNode("fromHook").exists()); + assertFalse(test.getChildNode("a").exists()); assertEquals(42, (long) test.getChildNode("newNode").getProperty("n").getValue(LONG)); assertEquals(test, store.getRoot().getChildNode("test")); } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java index 0f786a4..9adf4d1 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java @@ -31,8 +31,10 @@ import org.junit.Before; import org.junit.Test; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertNull; +import static junit.framework.Assert.assertTrue; public class LargeKernelNodeStateTest { @@ -67,10 +69,10 @@ public class LargeKernelNodeStateTest { @Test public void testGetChildNode() { - assertNotNull(state.getChildNode("x0")); - assertNotNull(state.getChildNode("x1")); - assertNotNull(state.getChildNode("x" + N)); - assertNull(state.getChildNode("x" + (N + 1))); + assertTrue(state.getChildNode("x0").exists()); + assertTrue(state.getChildNode("x1").exists()); + assertTrue(state.getChildNode("x" + N).exists()); + assertFalse(state.getChildNode("x" + (N + 1)).exists()); } @Test diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java index e3304e6..b160d4f 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexHookManagerTest.java @@ -281,8 +281,8 @@ public class IndexHookManagerTest { private static NodeState checkPathExists(NodeState state, String... verify) { NodeState c = state; for (String p : verify) { - assertTrue(c.hasChildNode(p)); c = c.getChildNode(p); + assertTrue(c.exists()); } return c; } diff --git a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/tck/QueryIT.java b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/tck/QueryIT.java index 60567cd..a49bb1d 100644 --- a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/tck/QueryIT.java +++ b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/tck/QueryIT.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.jcr.tck; +import org.apache.jackrabbit.test.api.query.qom.DescendantNodeJoinConditionTest; + import junit.framework.Test; public class QueryIT extends TCKBase { @@ -30,7 +32,8 @@ public class QueryIT extends TCKBase { @Override protected void addTests() { - addTest(org.apache.jackrabbit.test.api.query.TestAll.suite()); - addTest(org.apache.jackrabbit.test.api.query.qom.TestAll.suite()); + addTestSuite(DescendantNodeJoinConditionTest.class); + // addTest(org.apache.jackrabbit.test.api.query.TestAll.suite()); + // addTest(org.apache.jackrabbit.test.api.query.qom.TestAll.suite()); } } diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java index 861f301..b75fbc2 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java @@ -322,12 +322,9 @@ public class LuceneIndex implements QueryIndex, LuceneIndexConstants { return; // shortcut } NodeState system = root.getChildNode(NodeTypeConstants.JCR_SYSTEM); - if (system == null) { - return; - } final NodeState types = system.getChildNode(NodeTypeConstants.JCR_NODE_TYPES); - if (types == null) { + if (!types.exists()) { return; } diff --git a/oak-solr-embedded/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/OakSolrNodeStateConfiguration.java b/oak-solr-embedded/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/OakSolrNodeStateConfiguration.java index f79a189..1991a0b 100644 --- a/oak-solr-embedded/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/OakSolrNodeStateConfiguration.java +++ b/oak-solr-embedded/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/OakSolrNodeStateConfiguration.java @@ -108,7 +108,7 @@ public abstract class OakSolrNodeStateConfiguration implements OakSolrConfigurat protected String getStringValueFor(String propertyName, String defaultValue) { String value = null; NodeState configurationNodeState = getConfigurationNodeState(); - if (configurationNodeState != null) { + if (configurationNodeState.exists()) { PropertyState property = configurationNodeState.getProperty(propertyName); if (property != null) { value = property.getValue(Type.STRING); diff --git a/oak-solr-embedded/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/UpToDateNodeStateConfiguration.java b/oak-solr-embedded/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/UpToDateNodeStateConfiguration.java index cf31f43..ead758d 100644 --- a/oak-solr-embedded/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/UpToDateNodeStateConfiguration.java +++ b/oak-solr-embedded/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/UpToDateNodeStateConfiguration.java @@ -38,9 +38,6 @@ public class UpToDateNodeStateConfiguration extends OakSolrNodeStateConfiguratio NodeState currentState = store.getRoot(); for (String child : path.split("/")) { currentState = currentState.getChildNode(child); - if (currentState == null) { - break; - } } return currentState; } diff --git a/oak-solr-embedded/src/test/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/UpToDateNodeStateConfigurationTest.java b/oak-solr-embedded/src/test/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/UpToDateNodeStateConfigurationTest.java index 9d6abe3..a5210d9 100644 --- a/oak-solr-embedded/src/test/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/UpToDateNodeStateConfigurationTest.java +++ b/oak-solr-embedded/src/test/java/org/apache/jackrabbit/oak/plugins/index/solr/embedded/UpToDateNodeStateConfigurationTest.java @@ -9,6 +9,7 @@ import org.junit.Before; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; /** @@ -46,6 +47,6 @@ public class UpToDateNodeStateConfigurationTest { public void testNodeStateNotFound() throws Exception { String path = "some/path/to/somewhere/unknown"; UpToDateNodeStateConfiguration upToDateNodeStateConfiguration = new UpToDateNodeStateConfiguration(store, path); - assertNull(upToDateNodeStateConfiguration.getConfigurationNodeState()); + assertFalse(upToDateNodeStateConfiguration.getConfigurationNodeState().exists()); } } -- 1.8.0.msysgit.0