You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-issues@jackrabbit.apache.org by "Alex Parvulescu (JIRA)" <ji...@apache.org> on 2012/10/02 13:47:08 UTC
[jira] [Created] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Alex Parvulescu created OAK-355:
-----------------------------------
Summary: Move QueryEngine reference from ContentSession to Root
Key: OAK-355
URL: https://issues.apache.org/jira/browse/OAK-355
Project: Jackrabbit Oak
Issue Type: Improvement
Components: core
Reporter: Alex Parvulescu
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13484115#comment-13484115 ]
Jukka Zitting commented on OAK-355:
-----------------------------------
bq. the reason why this was changed
To give more background, we wanted to get rid of having to pass the Root instance around and cast it back to RootImpl when making queries. A better alternative was to acquire the query engine from the Root, so it could already contain direct references to the RootImpl internals.
BTW, looking at the code now, I notice that the query engine is using NodeStore instead of the NodeState currently associated with that Root. That's a bit troublesome since it breaks session isolation. I'll fix that in a moment.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13483158#comment-13483158 ]
Alex Parvulescu commented on OAK-355:
-------------------------------------
> Alex, could you quickly tell me what was the reason for this change?
During the hackathon, we refactored bits of that code, and we felt this was a better fit.
OAK-241 doesn't explain why the QueryEngine needs a session reference, it just states that is does, then it follows with details on how that can happen.
I couldn't figure out from the code why that is the case, and after removing it, everything still works :)
But I may have missed something, if that is the case, please shed some light on the matter.
I also agree about the name of the SessionQueryEngineImpl class, it probably need a new one now.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Reopened] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Thomas Mueller reopened OAK-355:
--------------------------------
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13484139#comment-13484139 ]
Thomas Mueller commented on OAK-355:
------------------------------------
> we wanted to get rid of having to pass the Root instance around
Sorry I can't follow this, you still have to pass a Root instance around, to get the query engine, right? I guess you mean "pass the ContentSession around"?
> cast it back to RootImpl when making queries
Yes that's nasty, but I don't currently see where this was required.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13478968#comment-13478968 ]
Thomas Mueller commented on OAK-355:
------------------------------------
Related to OAK-241
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Marcel Reutegger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488661#comment-13488661 ]
Marcel Reutegger commented on OAK-355:
--------------------------------------
Jukka, are you still working on this? Otherwise I can take over, since I stumble upon this as well.
I'm also in favor of removing the Root parameter from the SessionQueryEngine.executeQuery() method. It's not just troublesome when you need to cast it, but you also don't know where the root is coming from. It may be from a different session or outdated. I think its better to use the Root instance that was used to acquire the query engine.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu resolved OAK-355.
---------------------------------
Resolution: Fixed
Fix Version/s: 0.6
fixed with revision 1392859.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Marcel Reutegger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488686#comment-13488686 ]
Marcel Reutegger commented on OAK-355:
--------------------------------------
Removed Root param from SessionQueryEngine.executeQuery() in revision: 1404582. Queries now run with the root NodeState from the ContentSession Root instance.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Jukka Zitting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488692#comment-13488692 ]
Jukka Zitting commented on OAK-355:
-----------------------------------
bq. Jukka, are you still working on this? Otherwise I can take over, since I stumble upon this as well.
Thanks, feel free to take over.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13484083#comment-13484083 ]
Thomas Mueller commented on OAK-355:
------------------------------------
> doesn't explain why the QueryEngine needs a session reference
The idea was that the QueryEngine needed a ContentSession to check access rights (getAuthInfo) and the workspace name (getWorkspaceName). But it seems the AuthInfo is (now) in Root as well, and workspaces are not supported, so moving it to Root was (is) possible without problems.
> I may have missed something, if that is the case, please shed some light on the matter.
I just wanted to understand the reason why this was changed, and the description of this issue is empty...
> I also agree about the name of the SessionQueryEngineImpl class, it probably need a new one now.
Ideas are welcome :-) but for me it's not urgent.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-355) Move QueryEngine reference from
ContentSession to Root
Posted by "Thomas Mueller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OAK-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13478890#comment-13478890 ]
Thomas Mueller commented on OAK-355:
------------------------------------
Alex, could you quickly tell me what was the reason for this change? I'm not against the change, but would like to understand why it was done.
I reopened the issue because arguably the class "SessionQueryEngineImpl" now has the wrong name as it is no longer bound to a session. I guess the class should be renamed, but not sure what the new name should be. Or maybe it should be refactored somehow.
> Move QueryEngine reference from ContentSession to Root
> ------------------------------------------------------
>
> Key: OAK-355
> URL: https://issues.apache.org/jira/browse/OAK-355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Alex Parvulescu
> Fix For: 0.6
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira