jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stefan Guggisberg (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JCR-1402) Path.getAncestor and Path.isAncestor are not symmetric
Date Fri, 29 Feb 2008 10:40:51 GMT

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

Stefan Guggisberg commented on JCR-1402:
----------------------------------------

>  Another idea I recently had:
>  
>  Implement Path.getAncestor(n) by adding n parent elements to the end of the path. That
way the ancestor is defined for any path. normalized or not, relative or absolute.

while i think it's an elegant approach and the result would be certainly correct i don't think
it's what the api consumer needs.

Path.getAncestor(int) is heavily used in jackrabbit core. here's a common usage pattern:

                parentPath = targetPath.getAncestor(1);
                parentId = hierMgr.resolveNodePath(parentPath);

the suggested approach would probably not improve efficiency ;)

i'm further afraid that the change could break the current CachingHierarchyManager implementation;
here's a usage example:

            PathMap.Element parent = pathCache.map(path.getAncestor(1), true);


i'd therefore prefer angela's suggestion.

>  
>  Implement Path.isAncestorOf as follows: (isDescendant is similar)
>  
>  - if not both paths are either relative or absolute, throw some exception.
>  - If both paths are relative determine their depths n and m. Say n < m. If n <
0, add -n arbitrary elements at the beginning of both paths.
>  Normalize the paths and check if this paths elements are a prefix of that paths elements.
>  
>  With this approach the interface will not have to change and the implementation will
be correct (i.e. isAncestor(getAncestor()) == true) for every path accepted by those methods.
There might be a compatibility issue with implementations which rely on the current behavior
of getAncestor returning ../ when called with ../../ However, I think this should be fixed
in the implementation rather than keeping a core interface with a 'bogus' specification.

> Path.getAncestor and Path.isAncestor are not symmetric
> ------------------------------------------------------
>
>                 Key: JCR-1402
>                 URL: https://issues.apache.org/jira/browse/JCR-1402
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>    Affects Versions: 1.4
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: path.patch
>
>
> Although the method names refer to ancestors they operate on sub-paths. Consider:
> PathFactory pf = PathFactoryImpl.getInstance();
> Path.Element p = pf.getParentElement();
> Path path = pf.create(new Path.Element[]{p, p});
> Path ancestor = path.getAncestor(1);
> assertFalse(ancestor.isAncestorOf(path) )  
> This is not what one would expect from looking an the method signatures. 
> I suggest to rename getAncestor to getSubPath, clarify the javadoc, and deprecate getAncestorCount.

> A patch follows.

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


Mime
View raw message