You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Michael Dürig (JIRA)" <ji...@apache.org> on 2008/02/20 14:55:43 UTC
[jira] Created: (JCR-1402) Path.getAncestor and
Path.getAncestorCount are misnomers
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
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.
[jira] Commented: (JCR-1402) Path.getAncestor and
Path.getAncestorCount are misnomers
Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Commented: (JCR-1402) Path.getAncestor and
Path.getAncestorCount are misnomers
Posted by "angela (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Updated: (JCR-1402) Path.getAncestor and
Path.getAncestorCount are misnomers
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Dürig updated JCR-1402:
-------------------------------
Attachment: path.patch
Patch for renaming getAncestor, clarifying javadoc and deprecating getAncestorCount. Also contains a test case
> 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.
[jira] Commented: (JCR-1402) Path.getAncestor and Path.isAncestor
are not symmetric
Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Updated: (JCR-1402) Path.getAncestor and Path.isAncestor are
not symmetric
Posted by "angela (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
angela updated JCR-1402:
------------------------
Summary: Path.getAncestor and Path.isAncestor are not symmetric (was: Path.getAncestor and Path.getAncestorCount are misnomers)
> 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.
[jira] Commented: (JCR-1402) Path.getAncestor and
Path.getAncestorCount are misnomers
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571402#action_12571402 ]
Michael Dürig commented on JCR-1402:
------------------------------------
> i am not sure about the IllegalArgumentException
We could also leave the behavior undefined for not normalized paths. However I think throwing an exception is more explicit and make it easier to detect cases where some code (inadvertently) relies on such behavior.
> 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.
[jira] Issue Comment Edited: (JCR-1402) Path.getAncestor and
Path.isAncestor are not symmetric
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573796#action_12573796 ]
mduerig edited comment on JCR-1402 at 2/29/08 7:48 AM:
-------------------------------------------------------------
Ok, I suggest the following for getAncestor (credits due to Angela):
- If the path is absolute, normalize and determine ancestor of given degree. Throw if this would result in a 'negative' path.
- If the path is relative just add an appropriate number of parent elements.
Together with my proposal for isAncestorOf and isDescendant this is in accordance with Angela's suggestion while still handling relative paths correctly (i.e. isAncestor(getAncestor()) == true). Moreover there is no problem for above usage patterns.
was (Author: mduerig):
Ok, I suggest the following for getAncestor (credits due to Angela):
- If the path is absolute, normalize and determine ancestor of given degree. Throw if this would result in a 'negative' path.
- If the path is relative just an appropriate number of parent elements.
Together with my proposal for isAncestorOf and isDescendant this is in accordance with Angela's suggestion while still handling relative paths correctly (i.e. isAncestor(getAncestor()) == true). Moreover there is no problem for above usage patterns.
> 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.
[jira] Commented: (JCR-1402) Path.getAncestor and
Path.getAncestorCount are misnomers
Posted by "Stefan Guggisberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571397#action_12571397 ]
Stefan Guggisberg commented on JCR-1402:
----------------------------------------
[...]
> > they're expected to operate on normalized paths.
> Then the documentation of the relevant methods of the Path interface should say so.
>
> I's suggest: remove "Note that there migth be an unexpected result if this path is not normalized, e.g. the ancestor of degree = 1 of the path "../.." would be ".." although this is not the parent of "../.."." and add "IllegalArgumentException if this path is not normalized" to the javadoc of getAncestor.
+1 for javadoc improvement. i am not sure about the IllegalArgumentException...
>
>
> > it seems like the Path semantics got unfortunately somehow compromised
> There are two different things which got mixed up: paths and the hierarchy they denote. Some methods make that separation pretty clear: getDepth refers to the hirarchy and getLength to the path. However with the ancestor/descendant methods this separation is currently not that clear: although one expects them to refer to the hierarchy the current default implementation does suggest otherwise and the documentation is not clear enough (see above).
the implementation suggesting otherwise is a bug, see JCR-1409 ;)
IMO getAncestor clearly refers to the hierarchical nature of a path.
>
> 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.
[jira] Commented: (JCR-1402) Path.getAncestor and
Path.getAncestorCount are misnomers
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571383#action_12571383 ]
Michael Dürig commented on JCR-1402:
------------------------------------
> they're expected to operate on normalized paths.
Then the documentation of the relevant methods of the Path interface should say so.
I's suggest: remove "Note that there migth be an unexpected result if this path is not normalized, e.g. the ancestor of degree = 1 of the path "../.." would be ".." although this is not the parent of "../.."." and add "IllegalArgumentException if this path is not normalized" to the javadoc of getAncestor.
> it seems like the Path semantics got unfortunately somehow compromised
There are two different things which got mixed up: paths and the hierarchy they denote. Some methods make that separation pretty clear: getDepth refers to the hirarchy and getLength to the path. However with the ancestor/descendant methods this separation is currently not that clear: although one expects them to refer to the hierarchy the current default implementation does suggest otherwise and the documentation is not clear enough (see above).
> 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.
[jira] Commented: (JCR-1402) Path.getAncestor and Path.isAncestor
are not symmetric
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573796#action_12573796 ]
Michael Dürig commented on JCR-1402:
------------------------------------
Ok, I suggest the following for getAncestor (credits due to Angela):
- If the path is absolute, normalize and determine ancestor of given degree. Throw if this would result in a 'negative' path.
- If the path is relative just an appropriate number of parent elements.
Together with my proposal for isAncestorOf and isDescendant this is in accordance with Angela's suggestion while still handling relative paths correctly (i.e. isAncestor(getAncestor()) == true). Moreover there is no problem for above usage patterns.
> 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.
[jira] Commented: (JCR-1402) Path.getAncestor and
Path.getAncestorCount are misnomers
Posted by "angela (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571446#action_12571446 ]
angela commented on JCR-1402:
-----------------------------
i'm in favor of the IllegalArgumentException. having undefined behaviour with an interface
looks strange to me.... and there are quite some other possibilities for illegalargumentexception.
> 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.
[jira] Resolved: (JCR-1402) Path.getAncestor and Path.isAncestor
are not symmetric
Posted by "angela (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
angela resolved JCR-1402.
-------------------------
Resolution: Fixed
Assignee: angela
fixed with resolution of JCR-1526.
> 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
> Assignee: angela
> 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.
[jira] Closed: (JCR-1402) Path.getAncestor and Path.isAncestor are
not symmetric
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Dürig closed JCR-1402.
------------------------------
> 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
> Assignee: angela
> 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.
[jira] Commented: (JCR-1402) Path.getAncestor and Path.isAncestor
are not symmetric
Posted by "Michael Dürig (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573206#action_12573206 ]
Michael Dürig 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.
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.