You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Stefan Guggisberg (JIRA)" <ji...@apache.org> on 2008/02/22 12:56:20 UTC

[jira] Commented: (JCR-1402) Path.getAncestor and Path.getAncestorCount are misnomers

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

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

-1 for the proposed change. 

the specified Path methods are IMO adequately named. they're expected to operate on normalized paths. 
the javadoc warns about unexpected results when called on non-normalized paths (such as "../..").

the real issue at hand is IMO a bug in the PathFactory implementation. 
PathFactoryImpl creates illegal and inconsistent Path objects. see JCR-1409.

Path#isAncestorOf expects normalized paths and throws if that's not the case.   
however, both paths used in your example (i.e. "../'.." and "..") are non-normailzed
but Path#isNormalized returns true for both of them...

it seems like the Path semantics got unfortunately somehow compromised by the
introduction of the PathFactory interface and the related refactoring to spi-commons.

> Path.getAncestor and Path.getAncestorCount are misnomers
> --------------------------------------------------------
>
>                 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.