You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by re...@apache.org on 2012/05/24 17:50:05 UTC

svn commit: r1342304 - /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java

Author: reschke
Date: Thu May 24 15:50:05 2012
New Revision: 1342304

URL: http://svn.apache.org/viewvc?rev=1342304&view=rev
Log:
OAK-23: implement lookup by identifier using Query (WIP)

Modified:
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java?rev=1342304&r1=1342303&r2=1342304&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java Thu May 24 15:50:05 2012
@@ -138,7 +138,7 @@ public class SessionDelegate {
         }
         else {
             // referenceable
-            return findByJcrUuid(getTree(""), id);
+            return findByJcrUuid(id);
         }
     }
 
@@ -350,7 +350,7 @@ public class SessionDelegate {
     }
 
     @CheckForNull
-    NodeDelegate findByJcrUuid(Tree tree, String id) {
+    NodeDelegate findByJcrUuid(String id) {
 
         try {
             Map<String, CoreValue> bindings = Collections.singletonMap("id", getValueFactory().getCoreValueFactory()
@@ -377,24 +377,31 @@ public class SessionDelegate {
             log.error("query failed", ex);
             return null;
         }
-
-        // Tree-walking implementation...
-        // PropertyState p = tree.getProperty("jcr:uuid");
-        // if (p != null && id.equals(p.getValue().getString())) {
-        // return new NodeDelegate(this, tree);
-        // }
-        // else {
-        // for (Tree c : tree.getChildren()) {
-        // NodeDelegate found = findByJcrUuid(c, id);
-        // if (found != null) {
-        // return found;
-        // }
-        // }
-        // }
-        //
-        // return null;
     }
 
+//    @CheckForNull
+//    NodeDelegate slowFindByJcrUuid(String id) {
+//        return slowFindByJcrUuid(getTree(""), id);
+//    }
+//
+//    @CheckForNull
+//    NodeDelegate slowFindByJcrUuid(Tree tree, String id) {
+//        // Tree-walking implementation...
+//        PropertyState p = tree.getProperty("jcr:uuid");
+//        if (p != null && id.equals(p.getValue().getString())) {
+//            return new NodeDelegate(this, tree);
+//        } else {
+//            for (Tree c : tree.getChildren()) {
+//                NodeDelegate found = slowFindByJcrUuid(c, id);
+//                if (found != null) {
+//                    return found;
+//                }
+//            }
+//        }
+//
+//        return null;
+//    }
+
     //--------------------------------------------------< SessionNameMapper >---
 
     private class SessionNameMapper extends AbstractNameMapper {



Re: Identifier (UUID) handling

Posted by Michael Dürig <md...@apache.org>.

On 27.5.12 1:14, Jukka Zitting wrote:
> Hi,
>
> On Thu, May 24, 2012 at 6:05 PM, Julian Reschke<ju...@gmx.de>  wrote:
>> 1) NodeImpl tries to set jcr:uuid when needed (this is still a hack because
>> it should happen down in the stack, and take the actual effective node type
>> into account)
>
> Yes. This should go into a commit hook.

See OAK-100.

Michael

>
>> - a more general issue: do we have a requirement to keep
>> session.getNodeByUUID() working even though the query system is in trouble?
>> (like index broken?)
>
> I'm fine with getNodeByUUID() relying on the query system. The query
> system on the other hand should be able to work even when no indexes
> are available, falling back to tree traversal when all else fails.
>
> BR,
>
> Jukka Zitting

Re: Identifier (UUID) handling

Posted by Thomas Mueller <mu...@adobe.com>.
Hi,

>I'm fine with getNodeByUUID() relying on the query system. The query
>system on the other hand should be able to work even when no indexes
>are available, falling back to tree traversal when all else fails.

Yes, that's the current plan.

This is not an urgent feature, but if no index is available, I think a
warning should be logged. Otherwise people wonder why getNodeByUUID is
slow (at least for larger repositories) if no index is available (for
example because something is misconfigured). One way to do that is to
extend the query syntax, similar to the feature available in SQLite -
http://www.sqlite.org/lang_indexedby.html :

[indexed [by <indexName>] [or warn]]

For example the following query would log a warning (but still work) if no
index is available:
 
  select * from [nt:base] indexed or warn where [jcr:uuid] = 'xxx'

the following query would fail if no index is available:

  select * from [nt:base] indexed where [jcr:uuid] = 'xxx'

the following query would use an index if available, but not log a warning
if non is available:


  select * from [nt:base] where [jcr:uuid] = 'xxx'

Regards,

Thomas


Re: Identifier (UUID) handling

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Thu, May 24, 2012 at 6:05 PM, Julian Reschke <ju...@gmx.de> wrote:
> 1) NodeImpl tries to set jcr:uuid when needed (this is still a hack because
> it should happen down in the stack, and take the actual effective node type
> into account)

Yes. This should go into a commit hook.

> - a more general issue: do we have a requirement to keep
> session.getNodeByUUID() working even though the query system is in trouble?
> (like index broken?)

I'm fine with getNodeByUUID() relying on the query system. The query
system on the other hand should be able to work even when no indexes
are available, falling back to tree traversal when all else fails.

BR,

Jukka Zitting

Identifier (UUID) handling

Posted by Julian Reschke <ju...@gmx.de>.
So...

1) NodeImpl tries to set jcr:uuid when needed (this is still a hack 
because it should happen down in the stack, and take the actual 
effective node type into account)

2) identifiers are the jcr:uuids for referenceable nodes (right now 
actual UUIDs, but they don't have to) or absolute oak paths otherwise.

This needs to be changed to ID-if-closest-parent-plus-relative path.

3) Lookup by UUID happens through a query.

This works well so far (thanks, Thomas!); the main issues here are

- the internal query API disagrees on the path format (it includes the 
workspace name), so I needed to hack around it

- a more general issue: do we have a requirement to keep 
session.getNodeByUUID() working even though the query system is in 
trouble? (like index broken?)

Feedback appreciated, Julian


On 2012-05-24 17:50, reschke@apache.org wrote:
> Author: reschke
> Date: Thu May 24 15:50:05 2012
> New Revision: 1342304
>
> URL: http://svn.apache.org/viewvc?rev=1342304&view=rev
> Log:
> OAK-23: implement lookup by identifier using Query (WIP)
>
> Modified:
>      jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java
>
> Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java?rev=1342304&r1=1342303&r2=1342304&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java (original)
> +++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java Thu May 24 15:50:05 2012
> @@ -138,7 +138,7 @@ public class SessionDelegate {
>           }
>           else {
>               // referenceable
> -            return findByJcrUuid(getTree(""), id);
> +            return findByJcrUuid(id);
>           }
>       }
>
> @@ -350,7 +350,7 @@ public class SessionDelegate {
>       }
>
>       @CheckForNull
> -    NodeDelegate findByJcrUuid(Tree tree, String id) {
> +    NodeDelegate findByJcrUuid(String id) {
>
>           try {
>               Map<String, CoreValue>  bindings = Collections.singletonMap("id", getValueFactory().getCoreValueFactory()
> @@ -377,24 +377,31 @@ public class SessionDelegate {
>               log.error("query failed", ex);
>               return null;
>           }
> -
> -        // Tree-walking implementation...
> -        // PropertyState p = tree.getProperty("jcr:uuid");
> -        // if (p != null&&  id.equals(p.getValue().getString())) {
> -        // return new NodeDelegate(this, tree);
> -        // }
> -        // else {
> -        // for (Tree c : tree.getChildren()) {
> -        // NodeDelegate found = findByJcrUuid(c, id);
> -        // if (found != null) {
> -        // return found;
> -        // }
> -        // }
> -        // }
> -        //
> -        // return null;
>       }
>
> +//    @CheckForNull
> +//    NodeDelegate slowFindByJcrUuid(String id) {
> +//        return slowFindByJcrUuid(getTree(""), id);
> +//    }
> +//
> +//    @CheckForNull
> +//    NodeDelegate slowFindByJcrUuid(Tree tree, String id) {
> +//        // Tree-walking implementation...
> +//        PropertyState p = tree.getProperty("jcr:uuid");
> +//        if (p != null&&  id.equals(p.getValue().getString())) {
> +//            return new NodeDelegate(this, tree);
> +//        } else {
> +//            for (Tree c : tree.getChildren()) {
> +//                NodeDelegate found = slowFindByJcrUuid(c, id);
> +//                if (found != null) {
> +//                    return found;
> +//                }
> +//            }
> +//        }
> +//
> +//        return null;
> +//    }
> +
>       //--------------------------------------------------<  SessionNameMapper>---
>
>       private class SessionNameMapper extends AbstractNameMapper {
>
>
>