[dna-commits] DNA SVN: r947 - in trunk: dna-jcr/src/main/java/org/jboss/dna/jcr and 1 other directories.
dna-commits at lists.jboss.org
dna-commits at lists.jboss.org
Fri May 29 11:26:41 EDT 2009
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");
More information about the dna-commits
mailing list