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 2014/02/19 00:39:08 UTC
svn commit: r1569561 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/api/
oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/
oak-core/src/main/java/org/apache/jac...
Author: jukka
Date: Tue Feb 18 23:39:07 2014
New Revision: 1569561
URL: http://svn.apache.org/r1569561
Log:
OAK-1431: Handling of empty or invalid names
Make NodeState.getChildNode() and Tree.getChild() (but not hasChild) throw
IllegalArgumentException on invalid names. Adjust other pieces of code
accordingly.
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java
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/plugins/segment/SegmentNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/tree/AbstractTree.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionReader.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsProvider.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeState.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java
jackrabbit/oak/trunk/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/JackrabbitNodeState.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java Tue Feb 18 23:39:07 2014
@@ -200,9 +200,11 @@ public interface Tree {
* Get a possibly non existing child of this {@code Tree}.
* @param name The name of the child to retrieve.
* @return The child with the given {@code name}.
+ * @throws IllegalArgumentException if the given name string is empty
+ * or contains the forward slash character
*/
@Nonnull
- Tree getChild(@Nonnull String name);
+ Tree getChild(@Nonnull String name) throws IllegalArgumentException;
/**
* Determine if a child of this {@code Tree} instance exists. If no child
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/MutableTree.java Tue Feb 18 23:39:07 2014
@@ -36,6 +36,7 @@ import javax.annotation.Nonnull;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
+
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.api.Type;
@@ -77,7 +78,8 @@ class MutableTree extends AbstractTree {
//-----------------------------------------------------< AbstractTree >---
@Override
- protected MutableTree createChild(String name) {
+ protected MutableTree createChild(String name)
+ throws IllegalArgumentException {
return new MutableTree(root, this, name, pendingMoves);
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java Tue Feb 18 23:39:07 2014
@@ -103,8 +103,20 @@ class SecureNodeState extends AbstractNo
}
@Override
+ public boolean hasChildNode(@Nonnull String name) {
+ if (!state.hasChildNode(name)) {
+ return false;
+ } else if (treePermission.canReadAll()) {
+ return true;
+ } else {
+ NodeState child = state.getChildNode(name);
+ return treePermission.getChildPermission(name, child).canRead();
+ }
+ }
+
+ @Override
public NodeState getChildNode(@Nonnull String name) {
- NodeState child = state.getChildNode(checkNotNull(name));
+ NodeState child = state.getChildNode(name);
if (child.exists() && !treePermission.canReadAll()) {
ChildNodeEntry entry = new MemoryChildNodeEntry(name, child);
return new WrapChildEntryFunction().apply(entry).getNodeState();
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java Tue Feb 18 23:39:07 2014
@@ -75,7 +75,7 @@ public final class KernelNodeState exten
/**
* Maximum number of child nodes kept in memory.
*/
- public static final int MAX_CHILD_NODE_NAMES = 100;
+ public static final int MAX_CHILD_NAMES = 100;
/**
* Number of child nodes beyond which {@link MicroKernel#diff(String, String, String, int)}
@@ -166,7 +166,7 @@ public final class KernelNodeState exten
synchronized (this) {
if (properties == null) {
String json = kernel.getNodes(
- path, revision, 0, 0, MAX_CHILD_NODE_NAMES,
+ path, revision, 0, 0, MAX_CHILD_NAMES,
"{\"properties\":[\"*\",\":hash\",\":id\"]}");
checkNotNull(json,"No node found at path [%s] for revision [%s]",path,revision);
@@ -304,39 +304,42 @@ public final class KernelNodeState exten
@Override
public boolean hasChildNode(String name) {
- checkNotNull(name);
init();
- return childNames.contains(name)
- || (getChildNodeCount(MAX_CHILD_NODE_NAMES) > MAX_CHILD_NODE_NAMES
- && getChildNode(name).exists());
+ if (childNames.contains(name)) {
+ return true; // the named child node exits for sure
+ } else if (getChildNodeCount(MAX_CHILD_NAMES) <= MAX_CHILD_NAMES) {
+ return false; // all child node names are cached, and none match
+ } else {
+ return isValidName(name) && getChildNode(name).exists();
+ }
}
@Override
public NodeState getChildNode(String name) {
- // checkArgument(!checkNotNull(name).isEmpty()); // TODO: check in higher level
init();
String childPath = null;
if (childNames.contains(name)) {
- childPath = getChildPath(name);
- }
- if (childPath == null && getChildNodeCount(MAX_CHILD_NODE_NAMES) > MAX_CHILD_NODE_NAMES) {
- String path = getChildPath(name);
+ childPath = PathUtils.concat(path, name);
+ } else if (!isValidName(name)) {
+ throw new IllegalArgumentException("Invalid name: " + name);
+ } else if (getChildNodeCount(MAX_CHILD_NAMES) <= MAX_CHILD_NAMES) {
+ return MISSING_NODE;
+ } else {
+ childPath = PathUtils.concat(path, name);
// OAK-506: Avoid the nodeExists() call when already cached
- NodeState state = cache.getIfPresent(revision + path);
- if (state != null) {
- return state == NULL ? MISSING_NODE : state;
+ NodeState state = cache.getIfPresent(revision + childPath);
+ if (state == NULL) {
+ return MISSING_NODE;
+ } else if (state != null) {
+ return state;
}
// not able to tell from cache if node exists
// need to ask MicroKernel
- if (kernel.nodeExists(path, revision)) {
- childPath = path;
- } else {
- cache.put(revision + path, NULL);
+ if (!kernel.nodeExists(childPath, revision)) {
+ cache.put(revision + childPath, NULL);
+ return MISSING_NODE;
}
}
- if (childPath == null) {
- return MISSING_NODE;
- }
try {
return cache.get(revision + childPath);
} catch (ExecutionException e) {
@@ -347,7 +350,7 @@ public final class KernelNodeState exten
@Override
public Iterable<? extends ChildNodeEntry> getChildNodeEntries() {
init();
- if (childNodeCount <= MAX_CHILD_NODE_NAMES && childNodeCount <= childNames.size()) {
+ if (childNodeCount <= MAX_CHILD_NAMES && childNodeCount <= childNames.size()) {
return iterable(childNames);
}
List<Iterable<ChildNodeEntry>> iterables = Lists.newArrayList();
@@ -370,9 +373,9 @@ public final class KernelNodeState exten
private void fetchEntries() {
List<ChildNodeEntry> entries = Lists
- .newArrayListWithCapacity(MAX_CHILD_NODE_NAMES);
+ .newArrayListWithCapacity(MAX_CHILD_NAMES);
String json = kernel.getNodes(path, revision, 0,
- currentOffset, MAX_CHILD_NODE_NAMES, null);
+ currentOffset, MAX_CHILD_NAMES, null);
JsopReader reader = new JsopTokenizer(json);
reader.read('{');
do {
@@ -697,14 +700,6 @@ public final class KernelNodeState exten
return continueComparison;
}
- private String getChildPath(String name) {
- if ("/".equals(path)) {
- return '/' + name;
- } else {
- return path + '/' + name;
- }
- }
-
private Iterable<ChildNodeEntry> iterable(
Iterable<String> names) {
return Iterables.transform(
@@ -738,7 +733,7 @@ public final class KernelNodeState exten
@Override
public NodeState getNodeState() {
try {
- return cache.get(revision + getChildPath(name));
+ return cache.get(revision + PathUtils.concat(path, name));
} catch (ExecutionException e) {
throw new MicroKernelException(e);
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java Tue Feb 18 23:39:07 2014
@@ -151,15 +151,27 @@ final class DocumentNodeState extends Ab
});
}
+ @Override
+ public boolean hasChildNode(String name) {
+ if (node.hasNoChildren() || !isValidName(name)) {
+ return false;
+ } else {
+ String p = PathUtils.concat(getPath(), name);
+ return store.getNode(p, node.getLastRevision()) != null;
+ }
+ }
+
@Nonnull
@Override
public NodeState getChildNode(@Nonnull String name) {
if (node.hasNoChildren()) {
+ checkValidName(name);
return EmptyNodeState.MISSING_NODE;
}
String p = PathUtils.concat(getPath(), name);
Node child = store.getNode(p, node.getLastRevision());
if (child == null) {
+ checkValidName(name);
return EmptyNodeState.MISSING_NODE;
} else {
return new DocumentNodeState(store, child);
@@ -401,4 +413,5 @@ final class DocumentNodeState extends Ab
}
}
}
+
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/EmptyNodeState.java Tue Feb 18 23:39:07 2014
@@ -16,9 +16,8 @@
*/
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 java.util.Collections.emptyList;
+import static org.apache.jackrabbit.oak.spi.state.AbstractNodeState.checkValidName;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
@@ -107,13 +106,12 @@ public final class EmptyNodeState implem
@Override
public boolean hasChildNode(@Nonnull String name) {
- checkArgument(!checkNotNull(name).isEmpty());
return false;
}
@Override @Nonnull
public NodeState getChildNode(@Nonnull String name) {
- checkArgument(!checkNotNull(name).isEmpty());
+ checkValidName(name);
return MISSING_NODE;
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeState.java Tue Feb 18 23:39:07 2014
@@ -16,8 +16,6 @@
*/
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;
@@ -85,19 +83,17 @@ class MemoryNodeState extends AbstractNo
@Override
public boolean hasChildNode(String name) {
- checkArgument(!checkNotNull(name).isEmpty());
return nodes.containsKey(name);
}
@Override
public NodeState getChildNode(String name) {
- checkArgument(!checkNotNull(name).isEmpty());
NodeState state = nodes.get(name);
- if (state != null) {
- return state;
- } else {
- return MISSING_NODE;
+ if (state == null) {
+ checkValidName(name);
+ state = MISSING_NODE;
}
+ return state;
}
@Override
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=1569561&r1=1569560&r2=1569561&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 Feb 18 23:39:07 2014
@@ -311,8 +311,17 @@ public class ModifiedNodeState extends A
}
@Override
+ public boolean hasChildNode(String name) {
+ NodeState child = nodes.get(name);
+ if (child != null) {
+ return child.exists();
+ } else {
+ return base.hasChildNode(name);
+ }
+ }
+
+ @Override
public NodeState getChildNode(String name) {
- // checkArgument(!checkNotNull(name).isEmpty()); // TODO: should be caught earlier
NodeState child = nodes.get(name);
if (child == null) {
child = base.getChildNode(name);
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=1569561&r1=1569560&r2=1569561&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 Feb 18 23:39:07 2014
@@ -97,6 +97,7 @@ class MutableNodeState extends AbstractN
MutableNodeState child = nodes.get(name);
if (child == null) {
+ checkValidName(name);
child = new MutableNodeState(state);
nodes.put(name, child);
} else {
@@ -190,7 +191,9 @@ class MutableNodeState extends AbstractN
* Set the value of a property
*/
void setProperty(PropertyState property) {
- properties.put(property.getName(), property);
+ String name = property.getName();
+ checkValidName(name);
+ properties.put(name, property);
}
@Override
@@ -251,8 +254,6 @@ class MutableNodeState extends AbstractN
@Override
public boolean hasChildNode(String name) {
assert base != null;
- checkNotNull(name);
- // checkArgument(!name.isEmpty()); TODO: should be caught earlier
NodeState child = nodes.get(name);
if (child != null) {
return child.exists();
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java Tue Feb 18 23:39:07 2014
@@ -46,6 +46,7 @@ import static org.apache.jackrabbit.oak.
import static org.apache.jackrabbit.oak.api.Type.STRINGS;
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.spi.state.AbstractNodeState.checkValidName;
public class SegmentNodeState extends Record implements NodeState {
@@ -317,7 +318,6 @@ public class SegmentNodeState extends Re
@Override
public boolean hasChildNode(String name) {
- checkArgument(!checkNotNull(name).isEmpty());
String childName = getTemplate().getChildName();
if (childName == Template.ZERO_CHILD_NODES) {
return false;
@@ -330,26 +330,20 @@ public class SegmentNodeState extends Re
@Override @Nonnull
public NodeState getChildNode(String name) {
- // checkArgument(!checkNotNull(name).isEmpty()); // TODO
String childName = getTemplate().getChildName();
- if (childName == Template.ZERO_CHILD_NODES) {
- return MISSING_NODE;
- } else if (childName == Template.MANY_CHILD_NODES) {
+ if (childName == Template.MANY_CHILD_NODES) {
MapEntry child = getChildNodeMap().getEntry(name);
if (child != null) {
return child.getNodeState();
- } else {
- return MISSING_NODE;
- }
- } else {
- if (childName.equals(name)) {
- Segment segment = getSegment();
- RecordId childNodeId = segment.readRecordId(getOffset(0, 1));
- return new SegmentNodeState(segment, childNodeId);
- } else {
- return MISSING_NODE;
}
+ } else if (childName != Template.ZERO_CHILD_NODES
+ && childName.equals(name)) {
+ Segment segment = getSegment();
+ RecordId childNodeId = segment.readRecordId(getOffset(0, 1));
+ return new SegmentNodeState(segment, childNodeId);
}
+ checkValidName(name);
+ return MISSING_NODE;
}
@Override @Nonnull
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/tree/AbstractTree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/tree/AbstractTree.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/tree/AbstractTree.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/tree/AbstractTree.java Tue Feb 18 23:39:07 2014
@@ -90,10 +90,13 @@ public abstract class AbstractTree imple
/**
* Factory method for creating child trees
* @param name name of the child tree
- * @return child tree of this tree with the given {@code name}
+ * @return child tree of this tree with the given {@code name}
+ * @throws IllegalArgumentException if the given name string is empty
+ * or contains the forward slash character
*/
@Nonnull
- protected abstract AbstractTree createChild(@Nonnull String name);
+ protected abstract AbstractTree createChild(@Nonnull String name)
+ throws IllegalArgumentException;
/**
* @return {@code true} iff {@code getStatus() == Status.NEW}
@@ -198,7 +201,7 @@ public abstract class AbstractTree imple
@Override
public boolean exists() {
- return !isHidden(name) && nodeBuilder.exists();
+ return nodeBuilder.exists() && !isHidden(name);
}
@Override
@@ -247,7 +250,7 @@ public abstract class AbstractTree imple
@Override
public boolean hasChild(String name) {
- return createChild(checkNotNull(name)).exists();
+ return nodeBuilder.hasChildNode(name) && !isHidden(name);
}
@Override
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionReader.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionReader.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionReader.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/privilege/PrivilegeDefinitionReader.java Tue Feb 18 23:39:07 2014
@@ -67,7 +67,7 @@ class PrivilegeDefinitionReader implemen
*/
@CheckForNull
PrivilegeDefinition readDefinition(String privilegeName) {
- if (privilegesTree == null) {
+ if (privilegesTree == null || !privilegesTree.hasChild(privilegeName)) {
return null;
} else {
Tree definitionTree = privilegesTree.getChild(privilegeName);
@@ -76,6 +76,6 @@ class PrivilegeDefinitionReader implemen
}
private static boolean isPrivilegeDefinition(@Nonnull Tree tree) {
- return tree.exists() && NT_REP_PRIVILEGE.equals(TreeUtil.getPrimaryTypeName(tree));
+ return NT_REP_PRIVILEGE.equals(TreeUtil.getPrimaryTypeName(tree));
}
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsProvider.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsProvider.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/privilege/PrivilegeBitsProvider.java Tue Feb 18 23:39:07 2014
@@ -92,13 +92,11 @@ public final class PrivilegeBitsProvider
}
PrivilegeBits bits = PrivilegeBits.getInstance();
for (String privilegeName : privilegeNames) {
- if (privilegeName != null) {
+ if (privilegesTree.hasChild(privilegeName)) {
Tree defTree = privilegesTree.getChild(privilegeName);
- if (defTree.exists()) {
- bits.add(PrivilegeBits.getInstance(defTree));
- }
+ bits.add(PrivilegeBits.getInstance(defTree));
} else {
- log.debug("Ignoring 'null' privilege name");
+ log.debug("Ignoring privilege name " + privilegeName);
}
}
return bits.unmodifiable();
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java Tue Feb 18 23:39:07 2014
@@ -31,11 +31,11 @@ import java.util.Set;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
+import org.apache.jackrabbit.oak.api.PropertyState;
+
import com.google.common.base.Function;
import com.google.common.collect.Iterables;
-import org.apache.jackrabbit.oak.api.PropertyState;
-
/**
* Abstract base class for {@link NodeState} implementations.
* This base class contains default implementations of the
@@ -51,6 +51,17 @@ import org.apache.jackrabbit.oak.api.Pro
*/
public abstract class AbstractNodeState implements NodeState {
+ public static boolean isValidName(String name) {
+ return name != null && !name.isEmpty() && name.indexOf('/') == -1;
+ }
+
+ public static void checkValidName(String name)
+ throws IllegalArgumentException {
+ if (!isValidName(name)) {
+ throw new IllegalArgumentException("Invalid name: " + name);
+ }
+ }
+
public static boolean getBoolean(NodeState state, String name) {
PropertyState property = state.getProperty(name);
return property != null
@@ -256,11 +267,6 @@ public abstract class AbstractNodeState
}
@Override
- public boolean hasChildNode(String name) {
- return getChildNode(name).exists();
- }
-
- @Override
public long getChildNodeCount(long max) {
long n = 0;
Iterator<?> iterator = getChildNodeEntries().iterator();
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=1569561&r1=1569560&r2=1569561&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 Feb 18 23:39:07 2014
@@ -27,9 +27,14 @@ 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
- * interface represents a specific, immutable state of a node. The state
- * consists of an unordered set of name -> item mappings, where
- * each item is either a property or a child node.
+ * interface represents a specific, immutable state of a node.
+ * <p>
+ * The state of a node consists of named properties and child nodes. Names
+ * are non-empty strings that never contain the forward slash character, "/".
+ * Implementations may place additional restrictions on possible name strings.
+ * The properties and child nodes are unordered, and no two properties or
+ * two child nodes may have the same name. An implementation may additionally
+ * restrict a property and a child node from having the same name.
* <p>
* Depending on context, a NodeState instance can be interpreted as
* representing the state of just that node, of the subtree starting at
@@ -142,13 +147,7 @@ public interface NodeState {
boolean hasProperty(@Nonnull String name);
/**
- * Returns the named property. The name is an opaque string and
- * is not parsed or otherwise interpreted by this method.
- * <p>
- * 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
- * a <em>non-existing</em> {@link NodeState} for the same name.
+ * Returns the named property, or {@code null} if no such property exists.
*
* @param name name of the property to return
* @return named property, or {@code null} if not found
@@ -280,7 +279,9 @@ public interface NodeState {
/**
* Checks whether the named child node exists. The implementation
- * is equivalent to {@code getChildNode(name).exists()}.
+ * is equivalent to {@code getChildNode(name).exists()}, except that
+ * passing an invalid name as argument will result in a {@code false}
+ * return value instead of an {@link IllegalArgumentException}.
*
* @param name name of the child node
* @return {@code true} if the named child node exists,
@@ -289,21 +290,17 @@ public interface NodeState {
boolean hasChildNode(@Nonnull String name);
/**
- * 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.
- * <p>
- * The namespace of properties and child nodes is shared, so if
- * this method returns an <em>existing</em> {@link NodeState} for
- * a given name, then {@link #getProperty(String)} is guaranteed
- * to return {@code null} for the same name.
+ * Returns the named, possibly non-existent, child node. Use the
+ * {@link #exists()} method on the returned child node to determine
+ * whether the node exists or not.
*
* @param name name of the child node to return
* @return named child node
+ * @throws IllegalArgumentException if the given name string is is empty
+ * or contains a forward slash character
*/
@Nonnull
- NodeState getChildNode(@Nonnull String name);
+ NodeState getChildNode(@Nonnull String name) throws IllegalArgumentException;
/**
* Returns the number of <em>iterable</em> child nodes of this node.
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/LargeKernelNodeStateTest.java Tue Feb 18 23:39:07 2014
@@ -36,7 +36,7 @@ import org.junit.Test;
public class LargeKernelNodeStateTest {
- private static final int N = KernelNodeState.MAX_CHILD_NODE_NAMES;
+ private static final int N = KernelNodeState.MAX_CHILD_NAMES;
private NodeState state;
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java Tue Feb 18 23:39:07 2014
@@ -218,7 +218,7 @@ public class NodeStoreTest {
public void manyChildNodes() throws CommitFailedException {
NodeBuilder root = store.getRoot().builder();
NodeBuilder parent = root.child("parent");
- for (int i = 0; i <= KernelNodeState.MAX_CHILD_NODE_NAMES; i++) {
+ for (int i = 0; i <= KernelNodeState.MAX_CHILD_NAMES; i++) {
parent.child("child-" + i);
}
store.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);
@@ -445,7 +445,7 @@ public class NodeStoreTest {
@Test
public void compareAgainstBaseState100() throws CommitFailedException {
- compareAgainstBaseState(KernelNodeState.MAX_CHILD_NODE_NAMES);
+ compareAgainstBaseState(KernelNodeState.MAX_CHILD_NAMES);
}
@Test // OAK-1320
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java Tue Feb 18 23:39:07 2014
@@ -109,7 +109,7 @@ public class DocumentNodeStoreTest {
public void childNodeCache() throws Exception {
DocumentNodeStore store = new DocumentMK.Builder().getNodeStore();
NodeBuilder builder = store.getRoot().builder();
- int max = (int) (KernelNodeState.MAX_CHILD_NODE_NAMES * 1.5);
+ int max = (int) (KernelNodeState.MAX_CHILD_NAMES * 1.5);
SortedSet<String> children = new TreeSet<String>();
for (int i = 0; i < max; i++) {
String name = "c" + i;
@@ -119,7 +119,7 @@ public class DocumentNodeStoreTest {
store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
builder = store.getRoot().builder();
String name = new ArrayList<String>(children).get(
- KernelNodeState.MAX_CHILD_NODE_NAMES / 2);
+ KernelNodeState.MAX_CHILD_NAMES / 2);
builder.child(name).remove();
store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
int numEntries = Iterables.size(store.getRoot().getChildNodeEntries());
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=1569561&r1=1569560&r2=1569561&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 Tue Feb 18 23:39:07 2014
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.plugin
import javax.annotation.Nonnull;
import com.google.common.collect.ImmutableSet;
+
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
@@ -507,12 +508,18 @@ public class MemoryNodeBuilderTest {
return ImmutableSet.of();
}
+ @Override
+ public boolean hasChildNode(String name) {
+ return "c".equals(name);
+ }
+
@Nonnull
@Override
public NodeState getChildNode(@Nonnull String name) {
if ("c".equals(name)) {
return C;
} else {
+ checkValidName(name);
return EmptyNodeState.MISSING_NODE;
}
}
Modified: jackrabbit/oak/trunk/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/JackrabbitNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/JackrabbitNodeState.java?rev=1569561&r1=1569560&r2=1569561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/JackrabbitNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-upgrade/src/main/java/org/apache/jackrabbit/oak/upgrade/JackrabbitNodeState.java Tue Feb 18 23:39:07 2014
@@ -121,12 +121,23 @@ class JackrabbitNodeState extends Abstra
}
@Override
+ public boolean hasChildNode(String name) {
+ for (MemoryChildNodeEntry entry : getChildNodeEntries()) {
+ if (name.equals(entry.getName())) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
public org.apache.jackrabbit.oak.spi.state.NodeState getChildNode(String name) {
for (MemoryChildNodeEntry entry : getChildNodeEntries()) {
if (name.equals(entry.getName())) {
return entry.getNodeState();
}
}
+ checkValidName(name);
return EmptyNodeState.MISSING_NODE;
}