Jackrabbit Oak
  1. Jackrabbit Oak
  2. OAK-84

Delegates for Session, Node, Property and Item

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.3
    • Component/s: jcr
    • Labels:
      None

      Description

      Instead of passing around Nodes internally and casting them down to NodeImpl we should use the façade pattern and delegate from NodeImpl back to an implementation class which is used internally. This also avoids the problem of API clients accessing stuff they shouldn't by casting to the implementation.

      Some initial work has been done already. What's left to do it:

      • Push down as much as possible from ItemImpl, NodeImpl and PropertyImpl to the respective delegate classes
      • Introduce the same pattern for SessionImpl and do away with SessionContext.

        Activity

        Michael Dürig created issue -
        Michael Dürig made changes -
        Field Original Value New Value
        Component/s jcr [ 12317804 ]
        Component/s it [ 12317805 ]
        Hide
        Michael Dürig added a comment -

        While working on this for Session, I came across

        org.apache.jackrabbit.test.api.NodeReadMethodsTest.testGetSession() 
        

        which mandates reference equality for node.getSession() and the session through which that node was acquired.

        Is this mandated by the spec.? Does this make sense? If so, we can't implement delegates for Session.

        Show
        Michael Dürig added a comment - While working on this for Session, I came across org.apache.jackrabbit.test.api.NodeReadMethodsTest.testGetSession() which mandates reference equality for node.getSession() and the session through which that node was acquired. Is this mandated by the spec.? Does this make sense? If so, we can't implement delegates for Session.
        Hide
        Julian Reschke added a comment -

        The prose in the spec doesn't seem to say. Item.getSession's javadoc doesn't help either. But Workspace.getSession's javadoc says "return the Session object ...".

        Show
        Julian Reschke added a comment - The prose in the spec doesn't seem to say. Item.getSession's javadoc doesn't help either. But Workspace.getSession's javadoc says "return the Session object ...".
        Hide
        Michael Dürig added a comment -

        Ah... in contrast, Item.getSession() says "Return the Session through which this item was acquired".

        Should we try to get rid of this requirement? It seems overly strict, not consistent (i.e. different for Workspace and Item) and not clearly speced.

        Show
        Michael Dürig added a comment - Ah... in contrast, Item.getSession() says "Return the Session through which this item was acquired". Should we try to get rid of this requirement? It seems overly strict, not consistent (i.e. different for Workspace and Item) and not clearly speced.
        Hide
        Julian Reschke added a comment -

        But there is no other way to find out whether two sessions are the same. I think it's good that it can be checked.

        Show
        Julian Reschke added a comment - But there is no other way to find out whether two sessions are the same. I think it's good that it can be checked.
        Hide
        Jukka Zitting added a comment -

        Why can't ItemImpl just keep a reference to the related SessionImpl and simply return it from getSession()?

        Show
        Jukka Zitting added a comment - Why can't ItemImpl just keep a reference to the related SessionImpl and simply return it from getSession()?
        Hide
        Jukka Zitting added a comment -

        Re: revision 1333500.

        Item.getDepth() is already implemented in the AbstractItem class of jcr-commons. I suggest we reuse that code unless there are valid performance or other reasons why a separate implementation in oak-jcr is needed.

        The attached patch makes ItemImpl extend AbstractItem and removes the getDepth() and getAncestor() methods that we then get for free from jcr-commons.

        Show
        Jukka Zitting added a comment - Re: revision 1333500. Item.getDepth() is already implemented in the AbstractItem class of jcr-commons. I suggest we reuse that code unless there are valid performance or other reasons why a separate implementation in oak-jcr is needed. The attached patch makes ItemImpl extend AbstractItem and removes the getDepth() and getAncestor() methods that we then get for free from jcr-commons.
        Jukka Zitting made changes -
        Hide
        Michael Dürig added a comment -

        Why can't ItemImpl just keep a reference to the related SessionImpl and simply return it from getSession()?

        The simplest ideas are always the best This seems to work!

        Show
        Michael Dürig added a comment - Why can't ItemImpl just keep a reference to the related SessionImpl and simply return it from getSession()? The simplest ideas are always the best This seems to work!
        Michael Dürig committed 1333607 (16 files)
        Reviews: none

        OAK-84: Delegates for Session, Node, Property and Item
        - Initial implementation of SessionDelegate
        - Remove SessionContext

        jackrabbit root:
        Michael Dürig committed 1333615 (2 files)
        Hide
        Michael Dürig added a comment -

        Item.getDepth() is already implemented in the AbstractItem class of jcr-commons.

        Revision 1333652 incorporates the patch.

        We should now start using the *Delegate classes internally instead of the respective *Impl classes since one can always go from the delegate to the impl. but not necessarily vice versa.

        Show
        Michael Dürig added a comment - Item.getDepth() is already implemented in the AbstractItem class of jcr-commons. Revision 1333652 incorporates the patch. We should now start using the *Delegate classes internally instead of the respective *Impl classes since one can always go from the delegate to the impl. but not necessarily vice versa.
        Michael Dürig committed 1334457 (1 file)
        Reviews: none

        OAK-84: Delegates for Session, Node, Property and Item
        Simplify setProperty methods

        Michael Dürig committed 1335051 (1 file)
        Reviews: none

        OAK-84: Delegates for Session, Node, Property and Item
        remove left overs

        Michael Dürig committed 1335161 (1 file)
        Reviews: none

        OAK-84: Delegates for Session, Node, Property and Item
        - clean up SessionDelegate

        Michael Dürig committed 1335169 (3 files)
        Hide
        Michael Dürig added a comment -

        Fixed at revision 1335171

        Show
        Michael Dürig added a comment - Fixed at revision 1335171
        Michael Dürig made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Michael Dürig [ mduerig ]
        Fix Version/s 0.3 [ 12320345 ]
        Resolution Fixed [ 1 ]
        Alex Parvulescu made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Michael Dürig
            Reporter:
            Michael Dürig
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development