Jackrabbit Oak
  1. Jackrabbit Oak
  2. OAK-898

Session#move with a fresh session doesn't work properly

    Details

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

      Description

      The scenario is as follows: create a node, then try to move it using a new session.

      Will attach a test case.

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        1d 6h 8m 1 Michael Dürig 12/Jul/13 22:06
        Resolved Resolved Closed Closed
        69d 9h 55m 1 Alex Parvulescu 20/Sep/13 08:01
        Alex Parvulescu made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Alex Parvulescu added a comment -

        Bulk close for the 0.9 release

        Show
        Alex Parvulescu added a comment - Bulk close for the 0.9 release
        Michael Dürig made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 0.9 [ 12324541 ]
        Resolution Fixed [ 1 ]
        Hide
        Michael Dürig added a comment -

        Fixed at revision 1502689. Thanks for hunting this one down Alex!

        Show
        Michael Dürig added a comment - Fixed at revision 1502689. Thanks for hunting this one down Alex!
        Michael Dürig made changes -
        Field Original Value New Value
        Assignee Michael Dürig [ mduerig ]
        Hide
        Michael Dürig added a comment -

        Good catch. It seems Root.move doesn't touch the pending changes state. I'll have a look.

        Show
        Michael Dürig added a comment - Good catch. It seems Root.move doesn't touch the pending changes state. I'll have a look.
        Hide
        Alex Parvulescu added a comment -

        fyi I quickly checked, jackrabbit returns true on the #hasPendingChanges call

        Show
        Alex Parvulescu added a comment - fyi I quickly checked, jackrabbit returns true on the #hasPendingChanges call
        Hide
        Alex Parvulescu added a comment -

        I think I have more info. I tracked down the missing #save call in sling [0].
        It turns out the #save call is conditioned by a #hasPendingChanges check, to which oak returns false.

        I have updated the test to reflect this latest details.

        [0] http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java?view=markup#l522

        Show
        Alex Parvulescu added a comment - I think I have more info. I tracked down the missing #save call in sling [0] . It turns out the #save call is conditioned by a #hasPendingChanges check, to which oak returns false. I have updated the test to reflect this latest details. [0] http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java?view=markup#l522
        Hide
        Alex Parvulescu added a comment -

        as discussed with Michael offline, the latest version of the test is lacking a #save call after the move.
        I'm unsure at which point in the http post call the save happens, so I cannot yet fully reproduce the failing move test.

        I'll try to find more info on this topic.

        Show
        Alex Parvulescu added a comment - as discussed with Michael offline, the latest version of the test is lacking a #save call after the move. I'm unsure at which point in the http post call the save happens, so I cannot yet fully reproduce the failing move test. I'll try to find more info on this topic.
        Hide
        Alex Parvulescu added a comment -

        makes sense, I've tweaked the test to make it closer to what happens behind the scenes in my failing scenario: removed the failing assert (as Jukka mentioned it was not an issue), and added a getNode call with a new session which fails with a PathNotFoundException.

        Worth to mention that the failure I see is based on the @MoveFrom functionality from the sling post servlet, and I think all the steps outlined in the provided test are done via http POST calls using new sessions, that's why I'm having a bit of a hard time reproducing this in a pure oak environment.
        For reference, here are some sling integration tests that cover these bits.

        [0] http://svn.apache.org/viewvc/sling/trunk/launchpad/integration-tests/src/main/java/org/apache/sling/launchpad/webapp/integrationtest/servlets/post/PostServletAtMoveTest.java?view=markup#l37

        Show
        Alex Parvulescu added a comment - makes sense, I've tweaked the test to make it closer to what happens behind the scenes in my failing scenario: removed the failing assert (as Jukka mentioned it was not an issue), and added a getNode call with a new session which fails with a PathNotFoundException . Worth to mention that the failure I see is based on the @MoveFrom functionality from the sling post servlet, and I think all the steps outlined in the provided test are done via http POST calls using new sessions, that's why I'm having a bit of a hard time reproducing this in a pure oak environment. For reference, here are some sling integration tests that cover these bits. [0] http://svn.apache.org/viewvc/sling/trunk/launchpad/integration-tests/src/main/java/org/apache/sling/launchpad/webapp/integrationtest/servlets/post/PostServletAtMoveTest.java?view=markup#l37
        Hide
        Michael Dürig added a comment -

        Jukka is right here, I didn't realise this before.

        The expectation encoded in the test case is apparently what the Sling PostServlet expects (personal communication with Alex, correct me when I got this wrong).

        Show
        Michael Dürig added a comment - Jukka is right here, I didn't realise this before. The expectation encoded in the test case is apparently what the Sling PostServlet expects (personal communication with Alex, correct me when I got this wrong).
        Hide
        Jukka Zitting added a comment -

        Looking at the test case, a node1.getSession().refresh() call seems to be missing. Note that with Oak sessions are based on snapshots of the repository state, so a refresh, either explicit or implicit (triggered for example by the auto-refresh feature), is needed for changes from another session to become visible. Also, we currently only track moves within a single session, so in this case node1 would actually become invalid after the refresh.

        Show
        Jukka Zitting added a comment - Looking at the test case, a node1.getSession().refresh() call seems to be missing. Note that with Oak sessions are based on snapshots of the repository state, so a refresh, either explicit or implicit (triggered for example by the auto-refresh feature), is needed for changes from another session to become visible. Also, we currently only track moves within a single session, so in this case node1 would actually become invalid after the refresh.
        Hide
        Alex Parvulescu added a comment -

        I pushed the test showing the issue (ignored currently) with rev http://svn.apache.org/r1502240

        Show
        Alex Parvulescu added a comment - I pushed the test showing the issue (ignored currently) with rev http://svn.apache.org/r1502240
        Alex Parvulescu created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development