Index: dna-graph/src/main/java/org/jboss/dna/graph/ExecutionContext.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/ExecutionContext.java (revision 941) +++ dna-graph/src/main/java/org/jboss/dna/graph/ExecutionContext.java (working copy) @@ -520,7 +520,7 @@ protected UserPasswordCallbackHandler( String userId, char[] password ) { this.userId = userId; - this.password = password; + this.password = password.clone(); } /** Index: dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java =================================================================== --- dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java (revision 939) +++ dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java (working copy) @@ -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); } /** Index: dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java =================================================================== --- dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java (revision 939) +++ dna-jcr/src/main/java/org/jboss/dna/jcr/SessionCache.java (working copy) @@ -510,10 +510,36 @@ NodeInfo nodeInfo = findNodeInfo(nodeUuid); AbstractJcrNode node = findJcrNode(nodeUuid); - Name primaryTypeName = node.getPrimaryTypeName(); - List 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 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. + *

+ * 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. + *

+ * @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 create = operations.createUnder(currentLocation) - .nodeNamed(name) - .with(desiredUuid) - .with(primaryTypeProp); + Graph.Create 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); Index: dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java =================================================================== --- dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java (revision 940) +++ dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java (working copy) @@ -43,6 +43,7 @@ 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 +72,11 @@ 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.WorkspaceCopyVersionableTest; import org.apache.jackrabbit.test.api.WorkspaceMoveReferenceableTest; +import org.apache.jackrabbit.test.api.WorkspaceMoveSameNameSibsTest; import org.apache.jackrabbit.test.api.WorkspaceMoveVersionableTest; /** @@ -200,7 +204,7 @@ addTestSuite(SetPropertyStringTest.class); addTestSuite(SetPropertyValueTest.class); addTestSuite(SetPropertyConstraintViolationExceptionTest.class); - // addTestSuite(SetPropertyAssumeTypeTest.class); + addTestSuite(SetPropertyAssumeTypeTest.class); addTestSuite(NodeItemIsModifiedTest.class); addTestSuite(NodeItemIsNewTest.class); @@ -219,12 +223,12 @@ addTestSuite(WorkspaceCopyBetweenWorkspacesSameNameSibsTest.class); addTestSuite(WorkspaceCopyBetweenWorkspacesTest.class); addTestSuite(WorkspaceCopyBetweenWorkspacesVersionableTest.class); - // addTestSuite(WorkspaceCopyReferenceableTest.class); - // addTestSuite(WorkspaceCopySameNameSibsTest.class); + addTestSuite(WorkspaceCopyReferenceableTest.class); + addTestSuite(WorkspaceCopySameNameSibsTest.class); // addTestSuite(WorkspaceCopyTest.class); addTestSuite(WorkspaceCopyVersionableTest.class); addTestSuite(WorkspaceMoveReferenceableTest.class); - // addTestSuite(WorkspaceMoveSameNameSibsTest.class); + addTestSuite(WorkspaceMoveSameNameSibsTest.class); // addTestSuite(WorkspaceMoveTest.class); addTestSuite(WorkspaceMoveVersionableTest.class); @@ -248,7 +252,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()); Index: dna-jcr/src/test/java/org/jboss/dna/jcr/JcrWorkspaceTest.java =================================================================== --- dna-jcr/src/test/java/org/jboss/dna/jcr/JcrWorkspaceTest.java (revision 938) +++ dna-jcr/src/test/java/org/jboss/dna/jcr/JcrWorkspaceTest.java (working copy) @@ -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");