You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Angela Schreiber <an...@adobe.com> on 2012/11/21 09:29:22 UTC
Re: svn commit: r1411900 - in /jackrabbit/oak/trunk/oak-jcr: pom.xml
src/main/java/org/apache/jackrabbit/oak/jcr/NodeDelegate.java src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
hi michael
> + TreeLocation loc = getLocation();
> + for (String element : PathUtils.elements(relPath)) {
> + if (PathUtils.denotesParent(element)) {
> + loc = loc.getParent();
> + } else if (!PathUtils.denotesCurrent(element)) {
> + loc = loc.getChild(element);
> + } // else . -> skip to next element
> + }
> + return loc;
this is exactly the reason why i was asking for having that
included in TreeLocation and Tree... we keep adding the exact
same code everywhere and i am quite sure that you didn't
cover all usages of relativePath defined by JCR API by this
fix.
this really looks troublesome and hacky to me.
kind regards
angela
Re: svn commit: r1411900 - in /jackrabbit/oak/trunk/oak-jcr: pom.xml
src/main/java/org/apache/jackrabbit/oak/jcr/NodeDelegate.java src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Posted by Michael Dürig <md...@apache.org>.
On 21.11.12 9:29, Angela Schreiber wrote:
> hi michael
>
>> + TreeLocation loc = getLocation();
>> + for (String element : PathUtils.elements(relPath)) {
>> + if (PathUtils.denotesParent(element)) {
>> + loc = loc.getParent();
>> + } else if (!PathUtils.denotesCurrent(element)) {
>> + loc = loc.getChild(element);
>> + } // else . -> skip to next element
>> + }
>> + return loc;
>
> this is exactly the reason why i was asking for having that
> included in TreeLocation and Tree...
Which is the wrong place to do that. See OAK-426. TreeLocation supports
relative paths without interpreting special names. If we start
interpreting special names in Tree, we tear functionality from plugins
into core which is definitely wrong.
we keep adding the exact
> same code everywhere and i am quite sure that you didn't
> cover all usages of relativePath defined by JCR API by this
> fix.
File an issue if it doesn't.
>
> this really looks troublesome and hacky to me.
One reason I didn't use LocationUtil instead is since you decided to use
different methods for handling paths there instead of using PathUtils we
use everywhere else. This breaks encapsulation and uniformity.
Michael
>
> kind regards
> angela
Re: svn commit: r1411900 - in /jackrabbit/oak/trunk/oak-jcr: pom.xml
src/main/java/org/apache/jackrabbit/oak/jcr/NodeDelegate.java src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Posted by Michael Dürig <md...@apache.org>.
On 21.11.12 11:30, Jukka Zitting wrote:
> Hi,
>
> On Wed, Nov 21, 2012 at 11:13 AM, Thomas Mueller <mu...@adobe.com> wrote:
>> For the query engine, I will need a "validate and normalize path" function
>> that verifies a path is syntactically correct, and simplifies it if possible,
>
> That should be doable in PathUtils as a purely syntactic operation,
> something like this:
>
> String resolvedPath = PathUtils.resolve(originalPath, relativePath);
> if (resolvedPath == null) {
> throw new Exception("Invalid path");
> }
>
> Or we could have a convenience method that automatically handles the throwing:
>
> String resolvedPath = PathUtils.resolveOrThrow(originalPath, relativePath);
Normalisation is already done within the PathMapper. I'll follow up with
a separate mail with details in a moment.
Michael
>
> BR,
>
> Jukka Zitting
>
Re: svn commit: r1411900 - in /jackrabbit/oak/trunk/oak-jcr: pom.xml
src/main/java/org/apache/jackrabbit/oak/jcr/NodeDelegate.java src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Wed, Nov 21, 2012 at 11:13 AM, Thomas Mueller <mu...@adobe.com> wrote:
> For the query engine, I will need a "validate and normalize path" function
> that verifies a path is syntactically correct, and simplifies it if possible,
That should be doable in PathUtils as a purely syntactic operation,
something like this:
String resolvedPath = PathUtils.resolve(originalPath, relativePath);
if (resolvedPath == null) {
throw new Exception("Invalid path");
}
Or we could have a convenience method that automatically handles the throwing:
String resolvedPath = PathUtils.resolveOrThrow(originalPath, relativePath);
BR,
Jukka Zitting
Re: svn commit: r1411900 - in /jackrabbit/oak/trunk/oak-jcr:
pom.xml src/main/java/org/apache/jackrabbit/oak/jcr/NodeDelegate.java
src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
For the query engine, I will need a "validate and normalize path" function
that verifies a path is syntactically correct, and simplifies it if
possible, for example:
/a/./b => /a/b
/a/../b => /b
a/.. => .
./a => a
. =>
a/. => a
/.. => invalid
.. => ..
/./. => /
This will not solve the case where a relative path starts with ".."
(possibly multiple levels). I'm wondering if that would be the only
remaining case (a normalize path would never contain "current path", plus
"parent" would always be at the beginning).
Regards,
Thomas
On 11/21/12 9:29 AM, "Angela Schreiber" <an...@adobe.com> wrote:
>hi michael
>
>> + TreeLocation loc = getLocation();
>> + for (String element : PathUtils.elements(relPath)) {
>> + if (PathUtils.denotesParent(element)) {
>> + loc = loc.getParent();
>> + } else if (!PathUtils.denotesCurrent(element)) {
>> + loc = loc.getChild(element);
>> + } // else . -> skip to next element
>> + }
>> + return loc;
>
>this is exactly the reason why i was asking for having that
>included in TreeLocation and Tree... we keep adding the exact
>same code everywhere and i am quite sure that you didn't
>cover all usages of relativePath defined by JCR API by this
>fix.
>
>this really looks troublesome and hacky to me.
>
>kind regards
>angela