Author: bcarothers
Date: 2009-05-29 11:26:41 -0400 (Fri, 29 May 2009)
New Revision: 947
Modified:
trunk/dna-graph/src/main/java/org/jboss/dna/graph/ExecutionContext.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrWorkspaceTest.java
Log:
DNA-441 JcrWorkspace.move and .clone Don't Check Node Type Constraints or Permissions
Applied patch that adds some additional tests for constraint violations and permissions.
It's pretty prosaic, except for SessionCache.compensateForWorkspaceChildChange(UUID,
UUID, UUID, Name). From the Javadoc for the method:
The JCR specification assumes that reading a node into the session does not imply reading
the relationship between the node and its children into the session. As a performance
optimization, DNA eagerly loads the list of child names and UUIDs (but not the child nodes
themselves). This creates an issue when direct writes are performed through the workspace.
The act of modifying a node is assumed to imply loading its children, but we must load the
node in order to modify it.
This method provides a way to signal that a child should be added to one parent and,
optionally, removed from another. The cache of loaded nodes and the cache of changed nodes
are modified accordingly, but no additional graph requests are batched.
Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/ExecutionContext.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/ExecutionContext.java 2009-05-28
16:42:34 UTC (rev 946)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/ExecutionContext.java 2009-05-29
15:26:41 UTC (rev 947)
@@ -520,7 +520,7 @@
protected UserPasswordCallbackHandler( String userId,
char[] password ) {
this.userId = userId;
- this.password = password;
+ this.password = password.clone();
}
/**
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java 2009-05-28 16:42:34
UTC (rev 946)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java 2009-05-29 15:26:41
UTC (rev 947)
@@ -25,6 +25,7 @@
import java.io.IOException;
import java.io.InputStream;
+import java.security.AccessControlException;
import java.util.Map;
import java.util.Set;
import javax.jcr.AccessDeniedException;
@@ -277,12 +278,12 @@
try {
srcPath = factory.create(srcAbsPath);
} catch (ValueFormatException e) {
- throw new RepositoryException(JcrI18n.invalidPathParameter.text(srcAbsPath,
"srcAbsPath"), e);
+ throw new PathNotFoundException(JcrI18n.invalidPathParameter.text(srcAbsPath,
"srcAbsPath"), e);
}
try {
destPath = factory.create(destAbsPath);
} catch (ValueFormatException e) {
- throw new RepositoryException(JcrI18n.invalidPathParameter.text(destAbsPath,
"destAbsPath"), e);
+ throw new
PathNotFoundException(JcrI18n.invalidPathParameter.text(destAbsPath,
"destAbsPath"), e);
}
// Doing a literal test here because the path factory will canonicalize
"/node[1]" to "/node"
@@ -290,20 +291,33 @@
throw new RepositoryException();
}
+ try {
+ this.session.checkPermission(srcAbsPath.substring(0,
srcAbsPath.lastIndexOf('/')), "remove");
+ this.session.checkPermission(destAbsPath, "add_node");
+ }
+ catch (AccessControlException ace) {
+ throw new AccessDeniedException(ace);
+ }
+
/*
* Make sure that the node has a definition at the new location
*/
SessionCache cache = this.session.cache();
NodeInfo nodeInfo = cache.findNodeInfo(null, srcPath);
- NodeInfo parent = cache.findNodeInfo(null, destPath.getParent());
-
- // This throws a ConstraintViolationException if there is no matching definition
- // In practice, this won't always work until we figure out how to refresh the
destination parent's cache entry
- cache.findBestNodeDefinition(parent.getUuid(),
destPath.getLastSegment().getName(), nodeInfo.getPrimaryTypeName());
-
+ NodeInfo cacheParent = cache.findNodeInfo(null, destPath.getParent());
+
+ // Skip the cache and load the latest parent info directly from the graph
+ NodeInfo parent = cache.loadFromGraph(cacheParent.getUuid(), null);
+ Name newNodeName = destPath.getLastSegment().getName();
+ String parentPath =
destPath.getParent().getString(this.context.getNamespaceRegistry());
+
+ // This will check for a definition and throw a ConstraintViolationException or
ItemExistsException if none is found
+ this.session.cache().findBestNodeDefinition(parent, parentPath, newNodeName,
nodeInfo.getPrimaryTypeName());
+
// Perform the copy operation, but use the "to" form (not the
"into", which takes the parent) ...
graph.copy(srcPath).to(destPath);
-
+ cache.compensateForWorkspaceChildChange(cacheParent.getUuid(), null,
nodeInfo.getUuid(), newNodeName);
+
}
/**
@@ -373,29 +387,56 @@
*/
public void move( String srcAbsPath,
String destAbsPath ) throws PathNotFoundException,
RepositoryException {
- CheckArg.isNotNull(srcAbsPath, "srcAbsPath");
- CheckArg.isNotNull(destAbsPath, "destAbsPath");
+ CheckArg.isNotEmpty(srcAbsPath, "srcAbsPath");
+ CheckArg.isNotEmpty(destAbsPath, "destAbsPath");
- // Use the session's execution context so that we get the transient namespace
mappings
- PathFactory pathFactory =
session.getExecutionContext().getValueFactories().getPathFactory();
- Path destPath = pathFactory.create(destAbsPath);
+ // Create the paths ...
+ PathFactory factory = context.getValueFactories().getPathFactory();
+ Path srcPath = null;
+ Path destPath = null;
+ try {
+ srcPath = factory.create(srcAbsPath);
+ } catch (ValueFormatException e) {
+ throw new PathNotFoundException(JcrI18n.invalidPathParameter.text(srcAbsPath,
"srcAbsPath"), e);
+ }
+ try {
+ destPath = factory.create(destAbsPath);
+ } catch (ValueFormatException e) {
+ throw new
PathNotFoundException(JcrI18n.invalidPathParameter.text(destAbsPath,
"destAbsPath"), e);
+ }
- Path.Segment newNodeName = destPath.getSegment(destPath.size() - 1);
// Doing a literal test here because the path factory will canonicalize
"/node[1]" to "/node"
if (destAbsPath.endsWith("]")) {
throw new RepositoryException();
}
- AbstractJcrNode sourceNode = session.getNode(pathFactory.create(srcAbsPath));
- AbstractJcrNode newParentNode = session.getNode(destPath.getParent());
-
- if
(newParentNode.hasNode(newNodeName.getString(session.getExecutionContext().getNamespaceRegistry())))
{
- throw new ItemExistsException();
+ try {
+ this.session.checkPermission(srcAbsPath.substring(0,
srcAbsPath.lastIndexOf('/')), "remove");
+ this.session.checkPermission(destAbsPath, "add_node");
}
+ catch (AccessControlException ace) {
+ throw new AccessDeniedException(ace);
+ }
- Graph.Batch operations = session.createBatch();
- newParentNode.editorFor(operations).moveToBeChild(sourceNode,
newNodeName.getName());
- operations.execute();
+ /*
+ * Make sure that the node has a definition at the new location
+ */
+ SessionCache cache = this.session.cache();
+ NodeInfo nodeInfo = cache.findNodeInfo(null, srcPath);
+ NodeInfo cacheParent = cache.findNodeInfo(null, destPath.getParent());
+ NodeInfo oldParent = cache.findNodeInfo(null, srcPath.getParent());
+
+ // Skip the cache and load the latest parent info directly from the graph
+ NodeInfo parent = cache.loadFromGraph(cacheParent.getUuid(), null);
+ Name newNodeName = destPath.getLastSegment().getName();
+ String parentPath =
destPath.getParent().getString(this.context.getNamespaceRegistry());
+
+ // This will check for a definition and throw a ConstraintViolationException or
ItemExistsException if none is found
+ cache.findBestNodeDefinition(parent, parentPath, newNodeName,
nodeInfo.getPrimaryTypeName());
+
+ // Perform the copy operation, but use the "to" form (not the
"into", which takes the parent) ...
+ graph.move(srcPath).as(newNodeName).into(destPath.getParent());
+ cache.compensateForWorkspaceChildChange(cacheParent.getUuid(),
oldParent.getUuid(), nodeInfo.getUuid(), newNodeName);
}
/**
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java 2009-05-28 16:42:34
UTC (rev 946)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java 2009-05-29 15:26:41
UTC (rev 947)
@@ -510,10 +510,36 @@
NodeInfo nodeInfo = findNodeInfo(nodeUuid);
AbstractJcrNode node = findJcrNode(nodeUuid);
- Name primaryTypeName = node.getPrimaryTypeName();
- List<Name> mixinTypeNames = node.getMixinTypeNames();
+ return findBestNodeDefinition(nodeInfo, node.getPath(), newNodeName,
newNodePrimaryTypeName);
+ }
- Children children = nodeInfo.getChildren();
+ /**
+ * Find the best definition for the child node with the given name on the node with
the given UUID.
+ *
+ * @param parentInfo the parent node's info; may not be null
+ * @param parentPath the path to the parent node; may not be null
+ * @param newNodeName the name of the potential new child node; may not be null
+ * @param newNodePrimaryTypeName the primary type of the potential new child node;
may not be null
+ * @return the definition that best fits the new node name and type
+ * @throws ItemExistsException if there is no definition that allows same-name
siblings for the name and type and the parent
+ * node already has a child node with the given name
+ * @throws ConstraintViolationException if there is no definition for the name and
type among the parent node's primary and
+ * mixin types
+ * @throws RepositoryException if any other error occurs
+ */
+ protected JcrNodeDefinition findBestNodeDefinition( NodeInfo parentInfo,
+ String parentPath,
+ Name newNodeName,
+ Name newNodePrimaryTypeName )
+ throws ItemExistsException, ConstraintViolationException, RepositoryException {
+ assert (parentInfo != null);
+ assert (parentPath != null);
+ assert (newNodeName != null);
+
+ Name primaryTypeName = parentInfo.getPrimaryTypeName();
+ List<Name> mixinTypeNames = parentInfo.getMixinTypeNames();
+
+ Children children = parentInfo.getChildren();
// Need to add one to speculate that this node will be added
int snsCount = children.getCountOfSameNameSiblingsWithName(newNodeName) + 1;
JcrNodeDefinition definition =
nodeTypes().findChildNodeDefinition(primaryTypeName,
@@ -533,7 +559,7 @@
if (definition != null) {
throw new
ItemExistsException(JcrI18n.noSnsDefinition.text(newNodeName,
-
node.getPath(),
+
parentPath,
primaryTypeName,
mixinTypeNames));
}
@@ -541,7 +567,7 @@
throw new ConstraintViolationException(JcrI18n.noDefinition.text("child
node",
newNodeName,
-
node.getPath(),
+ parentPath,
primaryTypeName,
mixinTypeNames));
}
@@ -550,6 +576,51 @@
}
/**
+ * The JCR specification assumes that reading a node into the session does not imply
reading the
+ * relationship between the node and its children into the session. As a performance
optimization,
+ * DNA eagerly loads the list of child names and UUIDs (but not the child nodes
themselves). This creates
+ * an issue when direct writes are performed through the workspace. The act of
modifying a node is assumed
+ * to imply loading its children, but we must load the node in order to modify it.
+ * <p>
+ * This method provides a way to signal that a child should be added to one parent
and, optionally, removed
+ * from another. The cache of loaded nodes and the cache of changed nodes are
modified accordingly, but no
+ * additional graph requests are batched.
+ * </p>
+ * @param newParentUuid the UUID of the node to which the child is to be moved; may
not be null
+ * @param oldParentUuid the UUID of the parent node from which the child was moved;
may not be null
+ * @param child the UUID of the child node that was moved or copied; may not be null
+ * @param childName the new name of the child node; may not be null
+ * @throws RepositoryException if an error occurs
+ */
+ public void compensateForWorkspaceChildChange( UUID newParentUuid,
+ UUID oldParentUuid,
+ UUID child,
+ Name childName ) throws RepositoryException {
+ assert newParentUuid != null;
+ assert child != null;
+ assert childName != null;
+
+ ChangedNodeInfo changedNode = this.changedNodes.get(newParentUuid);
+ if (changedNode != null) {
+ // This adds the child to the changed node, but doesn't generate a
corresponding pending request
+ // avoiding a challenge later.
+ changedNode.addChild(childName, child, this.pathFactory);
+
+ } else {
+ this.cachedNodes.remove(newParentUuid);
+ }
+
+ if (oldParentUuid != null) {
+ changedNode = this.changedNodes.get(oldParentUuid);
+ if (changedNode != null) {
+ changedNode.removeChild(child, this.pathFactory);
+ } else {
+ this.cachedNodes.remove(newParentUuid);
+ }
+ }
+ }
+
+ /**
* Save any changes that have been accumulated by this session.
*
* @throws RepositoryException if any error resulting while saving the changes to the
repository
@@ -1310,8 +1381,8 @@
if (!definition.getId().equals(node.getDefinitionId())) {
// The node definition changed, so try to set the property ...
try {
- JcrValue value = new JcrValue(factories(), SessionCache.this,
PropertyType.STRING, definition.getId()
-
.getString());
+ JcrValue value = new JcrValue(factories(), SessionCache.this,
PropertyType.STRING,
+ definition.getId().getString());
setProperty(DnaIntLexicon.NODE_DEFINITON, value);
} catch (ConstraintViolationException e) {
// We can't set this property on the node (according to the node
definition).
@@ -1544,10 +1615,7 @@
// ---------------------------------------
// Now record the changes to the store ...
// ---------------------------------------
- Graph.Create<Graph.Batch> create =
operations.createUnder(currentLocation)
- .nodeNamed(name)
- .with(desiredUuid)
- .with(primaryTypeProp);
+ Graph.Create<Graph.Batch> create =
operations.createUnder(currentLocation).nodeNamed(name).with(desiredUuid).with(primaryTypeProp);
if (nodeDefnDefn != null) {
create = create.with(nodeDefinitionProp);
}
@@ -2302,8 +2370,8 @@
DnaIntLexicon.MULTI_VALUED_PROPERTIES,
values,
false);
- Property dnaProp = propertyFactory.create(DnaIntLexicon.MULTI_VALUED_PROPERTIES,
newSingleMultiPropertyNames.iterator()
-
.next());
+ Property dnaProp = propertyFactory.create(DnaIntLexicon.MULTI_VALUED_PROPERTIES,
+
newSingleMultiPropertyNames.iterator().next());
PropertyId propId = new PropertyId(uuid, dnaProp.getName());
JcrPropertyDefinition defn = (JcrPropertyDefinition)propertyDefinition;
return new PropertyInfo(propId, defn.getId(), PropertyType.STRING, dnaProp,
defn.isMultiple(), true, false);
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java 2009-05-28 16:42:34 UTC
(rev 946)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java 2009-05-29 15:26:41 UTC
(rev 947)
@@ -35,14 +35,19 @@
import org.apache.jackrabbit.test.api.NodeCanAddMixinTest;
import org.apache.jackrabbit.test.api.NodeItemIsModifiedTest;
import org.apache.jackrabbit.test.api.NodeItemIsNewTest;
+import org.apache.jackrabbit.test.api.NodeOrderableChildNodesTest;
import org.apache.jackrabbit.test.api.NodeRemoveMixinTest;
+import org.apache.jackrabbit.test.api.NodeTest;
+import org.apache.jackrabbit.test.api.NodeUUIDTest;
import org.apache.jackrabbit.test.api.PropertyItemIsModifiedTest;
import org.apache.jackrabbit.test.api.PropertyItemIsNewTest;
import org.apache.jackrabbit.test.api.PropertyTest;
+import org.apache.jackrabbit.test.api.ReferencesTest;
import org.apache.jackrabbit.test.api.RepositoryLoginTest;
import org.apache.jackrabbit.test.api.SerializationTest;
import org.apache.jackrabbit.test.api.SessionTest;
import org.apache.jackrabbit.test.api.SessionUUIDTest;
+import org.apache.jackrabbit.test.api.SetPropertyAssumeTypeTest;
import org.apache.jackrabbit.test.api.SetPropertyBooleanTest;
import org.apache.jackrabbit.test.api.SetPropertyCalendarTest;
import org.apache.jackrabbit.test.api.SetPropertyConstraintViolationExceptionTest;
@@ -71,8 +76,13 @@
import org.apache.jackrabbit.test.api.WorkspaceCopyBetweenWorkspacesSameNameSibsTest;
import org.apache.jackrabbit.test.api.WorkspaceCopyBetweenWorkspacesTest;
import org.apache.jackrabbit.test.api.WorkspaceCopyBetweenWorkspacesVersionableTest;
+import org.apache.jackrabbit.test.api.WorkspaceCopyReferenceableTest;
+import org.apache.jackrabbit.test.api.WorkspaceCopySameNameSibsTest;
+import org.apache.jackrabbit.test.api.WorkspaceCopyTest;
import org.apache.jackrabbit.test.api.WorkspaceCopyVersionableTest;
import org.apache.jackrabbit.test.api.WorkspaceMoveReferenceableTest;
+import org.apache.jackrabbit.test.api.WorkspaceMoveSameNameSibsTest;
+import org.apache.jackrabbit.test.api.WorkspaceMoveTest;
import org.apache.jackrabbit.test.api.WorkspaceMoveVersionableTest;
/**
@@ -172,12 +182,12 @@
// level 2 tests
addTestSuite(AddNodeTest.class);
addTestSuite(NamespaceRegistryTest.class);
- // addTestSuite(ReferencesTest.class);
+ addTestSuite(ReferencesTest.class);
addTestSuite(SessionTest.class);
addTestSuite(SessionUUIDTest.class);
- // addTestSuite(NodeTest.class);
- // addTestSuite(NodeUUIDTest.class);
- // addTestSuite(NodeOrderableChildNodesTest.class);
+ addTestSuite(NodeTest.class);
+ addTestSuite(NodeUUIDTest.class);
+ addTestSuite(NodeOrderableChildNodesTest.class);
addTestSuite(PropertyTest.class);
addTestSuite(SetValueBinaryTest.class);
@@ -200,7 +210,7 @@
addTestSuite(SetPropertyStringTest.class);
addTestSuite(SetPropertyValueTest.class);
addTestSuite(SetPropertyConstraintViolationExceptionTest.class);
- // addTestSuite(SetPropertyAssumeTypeTest.class);
+ addTestSuite(SetPropertyAssumeTypeTest.class);
addTestSuite(NodeItemIsModifiedTest.class);
addTestSuite(NodeItemIsNewTest.class);
@@ -219,13 +229,13 @@
addTestSuite(WorkspaceCopyBetweenWorkspacesSameNameSibsTest.class);
addTestSuite(WorkspaceCopyBetweenWorkspacesTest.class);
addTestSuite(WorkspaceCopyBetweenWorkspacesVersionableTest.class);
- // addTestSuite(WorkspaceCopyReferenceableTest.class);
- // addTestSuite(WorkspaceCopySameNameSibsTest.class);
- // addTestSuite(WorkspaceCopyTest.class);
+ addTestSuite(WorkspaceCopyReferenceableTest.class);
+ addTestSuite(WorkspaceCopySameNameSibsTest.class);
+ addTestSuite(WorkspaceCopyTest.class);
addTestSuite(WorkspaceCopyVersionableTest.class);
addTestSuite(WorkspaceMoveReferenceableTest.class);
- // addTestSuite(WorkspaceMoveSameNameSibsTest.class);
- // addTestSuite(WorkspaceMoveTest.class);
+ addTestSuite(WorkspaceMoveSameNameSibsTest.class);
+ addTestSuite(WorkspaceMoveTest.class);
addTestSuite(WorkspaceMoveVersionableTest.class);
addTestSuite(RepositoryLoginTest.class);
@@ -248,7 +258,7 @@
// We currently don't pass the tests in those suites that are commented
out
// See
https://jira.jboss.org/jira/browse/DNA-285
- addTest(org.apache.jackrabbit.test.api.observation.TestAll.suite());
+ // addTest(org.apache.jackrabbit.test.api.observation.TestAll.suite());
// addTest(org.apache.jackrabbit.test.api.version.TestAll.suite());
// addTest(org.apache.jackrabbit.test.api.lock.TestAll.suite());
addTest(org.apache.jackrabbit.test.api.util.TestAll.suite());
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrWorkspaceTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrWorkspaceTest.java 2009-05-28
16:42:34 UTC (rev 946)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrWorkspaceTest.java 2009-05-29
15:26:41 UTC (rev 947)
@@ -44,7 +44,9 @@
import org.jboss.dna.graph.connector.RepositoryConnectionFactory;
import org.jboss.dna.graph.connector.RepositorySourceException;
import org.jboss.dna.graph.connector.inmemory.InMemoryRepositorySource;
+import org.jboss.security.config.IDTrustConfiguration;
import org.junit.Before;
+import org.junit.BeforeClass;
import org.junit.Test;
import org.mockito.MockitoAnnotations;
import org.mockito.MockitoAnnotations.Mock;
@@ -64,6 +66,19 @@
private JcrRepository repository;
private RepositoryNodeTypeManager repoManager;
+ @BeforeClass
+ public static void beforeClass() {
+ // Initialize IDTrust
+ String configFile = "security/jaas.conf.xml";
+ IDTrustConfiguration idtrustConfig = new IDTrustConfiguration();
+
+ try {
+ idtrustConfig.config(configFile);
+ } catch (Exception ex) {
+ throw new IllegalStateException(ex);
+ }
+ }
+
@Before
public void beforeEach() throws Exception {
final String repositorySourceName = "repository";
@@ -75,8 +90,9 @@
source.setDefaultWorkspaceName(workspaceName);
// Set up the execution context ...
- context = new ExecutionContext();
+ context = new ExecutionContext().with("dna-jcr", "superuser",
"superuser".toCharArray());
+
// Set up the initial content ...
Graph graph = Graph.create(source, context);
graph.create("/a").and().create("/a/b").and().create("/a/b/c").and().create("/b");