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

angela commented on JCR-1402:
-----------------------------

i took a closer look at isAncestor, getAncestor and the original/current 
definition of a normalized path:

- .. and ../.. are normalized according to the definition Path (normalized = no 
redundant elements)
- the problem is that getAncestor does not require the Path to be normalized 
(and nor does any of the
  implementation normalize it) before retrieving the ancestor. 
- in contrast 'isAncestor' does convert non-normalized path first (and throws 
if that fails)
- the problem with normalized paths containing the parent element is, that the 
parent of .. was ../.. 
  this special case not handled properly neither by isAncestor nor by 
getAncestor.

i would therefore suggest to:

= require Path.getAncestor to normalize the path before retrieving the ancestor.
= add RepositoryException to the method signature of Path.getAncestor 
(RepositoryException if the path
   cannot be normalized).
   [ question: would this be a huge backward compat problem?]

= Clarify that 'isAncestor' must treat the parent element properly
= Clarify that 'getAncestor' must treat the parent element properly
= Clarify that 'isAncestor' and 'getAncestor' must be symmetric.

what do you think?
angela

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

Reply via email to