You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Lars Michele (JIRA)" <ji...@apache.org> on 2009/04/27 13:53:30 UTC

[jira] Issue Comment Edited: (JCR-1773) shareable nodes: wrong path returned, causes remove() to delete wrong node

    [ https://issues.apache.org/jira/browse/JCR-1773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12702733#action_12702733 ] 

Lars Michele edited comment on JCR-1773 at 4/27/09 4:51 AM:
------------------------------------------------------------

Sorry for the noise yesterday, this was not a solution and even not pointing in the right direction.

The test above makes a little mistake, there is nothing like a shareable item in JackRabbit, there are only shareable nodes. OK, the node path is also not correct, but that is just a small bug ;-) So here comes the solution for that :-)

Change in ItemManager.java
    /**
     * @param path
     * @return
     * @throws PathNotFoundException
     * @throws AccessDeniedException
     * @throws RepositoryException
     */
    public NodeImpl getNode(Path path)
            throws PathNotFoundException, AccessDeniedException, RepositoryException {
        NodeId id = hierMgr.resolveNodePath(path);
        NodeId parentId = hierMgr.resolveNodePath(path.getAncestor(1));
        if (id == null) {
            throw new PathNotFoundException(safeGetJCRPath(path));
        }
        try {
        	if (parentId == null)
        		// TODO add shortcut, only the rootnode has no parent
        		return (NodeImpl) getItem(id, path);
        	// if the node is shareable, it now returns the node with the right parent 
        	return getNode(id, parentId);
        } catch (ItemNotFoundException infe) {
            throw new PathNotFoundException(safeGetJCRPath(path));
        }
    }

Changes in NodeImpl.java

New method added
    /**
     * Returns the id of the parent node at <code>relPath</code> or <code>null</code>
     * if no node exists at <code>relPath</code>.
     * <p/>
     * Note that access rights are not checked.
     *
     * @param relPath relative path of a (possible) node
     * @return the id of the parent node at <code>relPath</code> or
     *         <code>null</code> if no node exists at <code>relPath</code>
     * @throws RepositoryException if <code>relPath</code> is not a valid
     *                             relative path
     */
    protected NodeId resolveRelativeParentPath(String relPath)
            throws RepositoryException {
        try {
            /**
             * first check if relPath is just a name (in which case we don't
             * have to build & resolve absolute path)
             */
            Path p = session.getQPath(relPath).getAncestor(1);
            if (p.getLength() == 1) {
                Path.Element pe = p.getNameElement();
                if (pe.denotesName()) {
                    // check if node entry exists
                    NodeState thisState = data.getNodeState();
                    int index = pe.getIndex();
                    if (index == 0) {
                        index = 1;
                    }
                    ChildNodeEntry cne =
                            thisState.getChildNodeEntry(pe.getName(), index);
                    if (cne != null) {
                        return cne.getId();
                    } else {
                        // there's no child node with that name
                        return null;
                    }
                }
            }
            /**
             * build and resolve absolute path
             */
            p = PathFactoryImpl.getInstance().create(getPrimaryPath(), p, true);
            return session.getHierarchyManager().resolveNodePath(p);
        } catch (NameException e) {
            String msg = "failed to resolve path " + relPath + " relative to " + this;
            log.debug(msg);
            throw new RepositoryException(msg, e);
        }
    }

Just forget to add the complete method ;-)

Changed method
    /**
     * {@inheritDoc}
     */
    public Node getNode(String relPath)
            throws PathNotFoundException, RepositoryException {
        // check state of this instance
        sanityCheck();
        NodeId id = resolveRelativeNodePath(relPath);
        NodeId parentId = resolveRelativeParentPath(relPath);
        if (id == null) {
            throw new PathNotFoundException(relPath);
        }
        try {
        	if (parentId == null)
        		// TODO add shortcut, only the rootnode has no parent
                return (NodeImpl) itemMgr.getItem(id);
        	// if the node is shareable, it now returns the node with the right parent 
            return itemMgr.getNode(id, parentId);
        } catch (AccessDeniedException ade) {
            throw new PathNotFoundException(relPath);
        } catch (ItemNotFoundException infe) {
            throw new PathNotFoundException(relPath);
        }
    }

Now if you try the test with
Node item = session.getNode(path)
instead of
Item item = session.getItem(path)
the test works as expected.

The problem will still exist in the WebDAV layer, because it uses mostly Item instead of Node.

      was (Author: lmichele):
    Sorry for the noise yesterday, this was not a solution and even not pointing in the right direction.

The test above makes a little mistake, there is nothing like a shareable item in JackRabbit, there are only shareable nodes. OK, the node path is also not correct, but that is just a small bug ;-) So here comes the solution for that :-)

Change in ItemManager.java
    /**
     * @param path
     * @return
     * @throws PathNotFoundException
     * @throws AccessDeniedException
     * @throws RepositoryException
     */
    public NodeImpl getNode(Path path)
            throws PathNotFoundException, AccessDeniedException, RepositoryException {
        NodeId id = hierMgr.resolveNodePath(path);
        NodeId parentId = hierMgr.resolveNodePath(path.getAncestor(1));
        if (id == null) {
            throw new PathNotFoundException(safeGetJCRPath(path));
        }
        try {
        	if (parentId == null)
        		// TODO add shortcut, only the rootnode has no parent
        		return (NodeImpl) getItem(id, path);
        	// if the node is shareable, it now returns the node with the right parent 
        	return getNode(id, parentId);
        } catch (ItemNotFoundException infe) {
            throw new PathNotFoundException(safeGetJCRPath(path));
        }
    }

Changes in NodeImpl.java

New method added
    /**
     * Returns the id of the parent node at <code>relPath</code> or <code>null</code>
     * if no node exists at <code>relPath</code>.
     * <p/>
     * Note that access rights are not checked.
     *
     * @param relPath relative path of a (possible) node
     * @return the id of the parent node at <code>relPath</code> or
     *         <code>null</code> if no node exists at <code>relPath</code>
     * @throws RepositoryException if <code>relPath</code> is not a valid
     *                             relative path
     */
    protected NodeId resolveRelativeParentPath(String relPath)
            throws RepositoryException {
        try {
            /**
             * first check if relPath is just a name (in which case we don't
             * have to build & resolve absolute path)
             */
            Path p = session.getQPath(relPath).getAncestor(1);
            if (p.getLength() == 1) {
                Path.Element pe = p.getNameElement();
                if (pe.denotesName()) {
                    // check if node entry exists
                    NodeState thisState = data.getNodeState();
                    int index = pe.getIndex();
                    if (index == 0) {
                        index = 1;
                    }
                    ChildNodeEntry cne =
                            thisState.getChildNodeEntry(pe.getName(), index);
                    if (cne != null) {
                        return cne.getId();
                    } else {
                        // there's no child node with that name
                        return null;
                    }
                }
            }

Changed method
    /**
     * {@inheritDoc}
     */
    public Node getNode(String relPath)
            throws PathNotFoundException, RepositoryException {
        // check state of this instance
        sanityCheck();
        NodeId id = resolveRelativeNodePath(relPath);
        NodeId parentId = resolveRelativeParentPath(relPath);
        if (id == null) {
            throw new PathNotFoundException(relPath);
        }
        try {
        	if (parentId == null)
        		// TODO add shortcut, only the rootnode has no parent
                return (NodeImpl) itemMgr.getItem(id);
        	// if the node is shareable, it now returns the node with the right parent 
            return itemMgr.getNode(id, parentId);
        } catch (AccessDeniedException ade) {
            throw new PathNotFoundException(relPath);
        } catch (ItemNotFoundException infe) {
            throw new PathNotFoundException(relPath);
        }
    }

Now if you try the test with
Node item = session.getNode(path)
instead of
Item item = session.getItem(path)
the test works as expected.

The problem will still exist in the WebDAV layer, because it uses mostly Item instead of Node.
  
> shareable nodes: wrong path returned, causes remove() to delete wrong node
> --------------------------------------------------------------------------
>
>                 Key: JCR-1773
>                 URL: https://issues.apache.org/jira/browse/JCR-1773
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-core
>            Reporter: Julian Reschke
>
> It seems that for shareable nodes it can happen that getPath() returns the wrong path (one of another node in the shared set):
> /**
> * Verify that shared nodes return correct paths.
> */
> public void testPath() throws Exception {
>    Node a1 = testRootNode.addNode("a1");
>    Node a2 = a1.addNode("a2");
>    Node b1 = a1.addNode("b1");
>    b1.addMixin("mix:shareable");
>    testRootNode.save();
>    //now we have a shareable node N with path a1/b1
>    Session session = testRootNode.getSession();
>    Workspace workspace = session.getWorkspace();
>    String path = a2.getPath() + "/b2";
>    workspace.clone(workspace.getName(), b1.getPath(), path, false);
>    //now we have another shareable node N' in the same shared set as N with path a1/a2/b2
>    //using the path a1/a2/b2, we should get the node N' here
>    Item item = session.getItem(path);
>    String p = item.getPath();
>    assertFalse("unexpectedly got the path from another node from the same shared set ", p.equals(b1.getPath()));
> } 
> Note that when this happens, a subsequent remove() deletes the wrong node.
> (Thanks Manfred for spotting this one).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.