4 Replies Latest reply on Aug 7, 2013 11:38 AM by ashelepko

    Merge of versioned checked-in nodes fails to set mergeFailed correctly

    ashelepko

      Hello,

       

      I've bumped into a problem with merging versionable records.

       

      First, here are some snippets from JCR 2.0 specification on Merge operation, that is a bit self-contradictory:

       

      15.9 Merge

      Like update, merge does not respect the checked-in status of nodes. A merge may change a node even if it is currently checked-in.

      ...

      ...

      The merge test is performed by comparing N with its corresponding node in srcWorkspace, call it N'.

      The merge test is done by comparing the base version of N (call it V) and the base version of N' (call it V').

      For any versionable node N there are three possible outcomes of the merge test: update, leave or failed.

      If N does not have a corresponding node then the merge result for N is leave.

      If N is currently checked-in then:

      • If V' is an eventual successor of V, then the merge result for N is update.
      • If V' is an eventual predecessor of V or if V and V' are identical (i.e., are actually the same version), then the merge result for N is leave.
      • If V is neither an eventual successor of, eventual predecessor of, nor identical with V', then the merge result for N is failed. This is the case where N and N' represent divergent branches of the version graph.

      If N is currently checked-out then:

      • If V' is an eventual predecessor of V or if V and V' are identical (i.e., are actually the same version), then the merge result for N is leave.
      • If any other relationship holds between V and V', then the merge result for N is fail.

      ...

      ...

      If bestEffort is true then:

      • Each versionable node N with result update is updated to reflect the state of N'. The state of a node in this context refers to its set of properties and child node links.
      • Each versionable node N with result leave is left unchanged, unless N is the child of a node with status update and N does not have a corresponding node in srcWorkspace. I such a case, N is removed.
      • Each versionable node N with result failed is left unchanged except that the identifier of V' (which is, in some sense, the “offending” version; the one that caused the merge to fail on that N) is added to the multi-value REFERENCE property jcr:mergeFailed of N. If the identifier of V' is already in jcr:mergeFailed, it is not added again. The jcr:mergeFailed property never contains repeated references to the same version. If the jcr:mergeFailed property does not yet exist then it is created. If present, the jcr:mergeFailed property will always contain at least one value. If not present on a node, this indicates that no merge failure has occurred on that node. Note that the presence of this property on a node will in any case prevent it from being checked-in because the OnParentVersion setting of jcr:mergeFailed is ABORT.
      • This property can later be used by the application to find those nodes in the subgraph that have failed to merge and thus require special attention (see §15.9.2 Merging Branches). This property is multi-valued so that a record of successive failed merges can be kept.

       

      First, the spec states that merge doesn't pay attention to checked-in status of nodes.

      Then, a little later it lists different flows for nodes that are checked-in and not checked-in.

      After that, it describes behavior of setting jcr:mergeFailed property when bestEffort is true. Note that no difference is made for checked-in and non-checked-in nodes.

       

      From what I understand, if I want a node to be updated, I have to check it in.

      When a node is checked in, and a best-effort merge result for it is failed, merge operation should set jcr:mergeFailed property on the node.

      However, it fails to do so, because the node is checked-in and thus read-only.

      Checking out the node will lead to forfeiting any attempts to update it during merge.

       

      So, I'm a bit confused here.

       

      The way I see it now, jcr:mergeFailed should be set irrespectively of node's checked-in status. Please correct me if I'm wrong.

       

      I tried to dig through the code to propose a pull request for this, but ended up being afraid to break something.

      I've found out that there are methods

          AbstractJcrNode: /*package*/ final AbstractJcrProperty void setProperty( Name name, JcrValue value, boolean skipReferenceValidation, boolean skipProtectedValidation, boolean skipVersioningValidation )

          JcrSingleValueProperty: protected void setValue( JcrValue jcrValue )

      that allow bypassing versioning check when setting a single-value property.

      But, jcr:mergeFailed is multi-value, and there are no methods for bypassing versioning check when setting a multi-value property.

       

      I can offer a test case that demonstrates the problem, please find in attached. I've run it from org.modeshape.jcr.JcrVersioningTest, but you can run it from anywhere by adding some code that sets up a standard repository.

       

      Thanks.

        • 1. Re: Merge of versioned checked-in nodes fails to set mergeFailed correctly
          rhauch

          I think the spec says that the checked-in status doesn't matter, but then goes on to detail the expected behavior in all situations.

           

           

          From what I understand, if I want a node to be updated, I have to check it in.

          When a node is checked in, and a best-effort merge result for it is failed, merge operation should set jcr:mergeFailed property on the node.

          However, it fails to do so, because the node is checked-in and thus read-only.

          Checking out the node will lead to forfeiting any attempts to update it during merge.

           

          Can you clarify what you mean by "fails to do so"? Does your application get an error? Does it just not do anything?

           

          The way I see it now, jcr:mergeFailed should be set irrespectively of node's checked-in status. Please correct me if I'm wrong.

          Yes, I think you are correct in that the 'jcr:mergedFailed' property should be set regardless of the node's checked-in status, except in the situations mentioned earlier in section 15.9. For example, if there is no corresponding node, or the corresponding node is not versionable, the method will return silently. There are a couple of cases outlined in section 15.9 where it says that nothing is done: "the merge method returns quietly and no changes are made", or "Otherwise, it is left unchanged."

           

           

          I tried to dig through the code to propose a pull request for this, but ended up being afraid to break something.

          I've found out that there are methods

              AbstractJcrNode: /*package*/ final AbstractJcrProperty void setProperty( Name name, JcrValue value, boolean skipReferenceValidation, boolean skipProtectedValidation, boolean skipVersioningValidation )

              JcrSingleValueProperty: protected void setValue( JcrValue jcrValue )

          that allow bypassing versioning check when setting a single-value property.

          But, jcr:mergeFailed is multi-value, and there are no methods for bypassing versioning check when setting a multi-value property.

           

          I can offer a test case that demonstrates the problem, please find in attached. I've run it from org.modeshape.jcr.JcrVersioningTest, but you can run it from anywhere by adding some code that sets up a standard repository.

          A single (or better yet, multiple) test cases would be very good. Merging and updating are quire complicated to even talk about how they're used and what they're behavior is, let alone try to debug the situation.

          • 2. Re: Merge of versioned checked-in nodes fails to set mergeFailed correctly
            ashelepko

            Please, find an attachment with a test case at the bottom of my first post.

             

            It should be inserted into org.modeshape.jcr.JcrVersioningTest to be run out of the box.

             

            The test case creates a situation when merging should set jcr:mergeFailed on a node, checks the node in and tries to run merge() on it.

            This is what i get when i run it:

             

            javax.jcr.version.VersionException: '/record' (or its nearest versionable ancestor) is checked in, preventing this action

                at org.modeshape.jcr.AbstractJcrNode.checkForCheckedOut(AbstractJcrNode.java:661)

                at org.modeshape.jcr.AbstractJcrNode.setProperty(AbstractJcrNode.java:1777)

                at org.modeshape.jcr.JcrVersionManager$MergeCommand.doFail(JcrVersionManager.java:1824)

                at org.modeshape.jcr.JcrVersionManager$MergeCommand.doMerge(JcrVersionManager.java:1716)

                at org.modeshape.jcr.JcrVersionManager$MergeCommand.execute(JcrVersionManager.java:1662)

                at org.modeshape.jcr.JcrVersionManager.merge(JcrVersionManager.java:944)

                at org.modeshape.jcr.JcrVersionManager.merge(JcrVersionManager.java:788)

                at org.modeshape.jcr.JcrVersionManager.merge(JcrVersionManager.java:773)

                at org.modeshape.jcr.JcrVersioningTest.shouldSetMergeFailedOnCheckedInNodes(JcrVersioningTest.java:659)

                at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

                at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

                at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)

                at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)

                at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)

                at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)

                at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)

                at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)

                at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)

                at org.junit.rules.RunRules.evaluate(RunRules.java:18)

                at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)

                at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)

                at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)

                at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)

                at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)

                at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)

                at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)

                at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)

                at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)

                at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)

                at org.junit.runners.ParentRunner.run(ParentRunner.java:300)

                at org.junit.runner.JUnitCore.run(JUnitCore.java:157)

                at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:77)

                at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:195)

                at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:63)

                at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

                at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

                at com.intellij.rt.execution.application.AppMain.main(AppMain.java:120)

            • 3. Re: Merge of versioned checked-in nodes fails to set mergeFailed correctly
              hchiorean

              @ashelepko could you please update the test case using the latest codebase from master ? The fix for https://issues.jboss.org/browse/MODE-2002 makes the attached test case pass, because there are no merge failures reported anymore.

               

              Thanks

               

              EDIT: NVM, I was able to reproduce the issue locally.

              • 4. Re: Merge of versioned checked-in nodes fails to set mergeFailed correctly
                ashelepko

                Gentlemen,

                 

                I've seen MODE-2005. Thanks for your fix, it works, but it doesn't cover all cases I'm afraid.

                 

                Your fix covers the case when a new jcr:mergeFailed property is added, but it doesn't cover the case when an existing jcr:mergeFailed property is augmented.

                 

                I've sent you a pull request that augments the test case: https://github.com/ModeShape/modeshape/pull/907

                 

                I get the following stack trace when i run it:

                 

                javax.jcr.version.VersionException: '/record' (or its nearest versionable ancestor) is checked in, preventing this action

                    at org.modeshape.jcr.AbstractJcrProperty.checkForCheckedOut(AbstractJcrProperty.java:213)

                    at org.modeshape.jcr.JcrMultiValueProperty.setValue(JcrMultiValueProperty.java:149)

                    at org.modeshape.jcr.AbstractJcrNode.setProperty(AbstractJcrNode.java:1872)

                    at org.modeshape.jcr.JcrVersionManager$MergeCommand.doFail(JcrVersionManager.java:1866)

                    at org.modeshape.jcr.JcrVersionManager$MergeCommand.doMerge(JcrVersionManager.java:1717)

                    at org.modeshape.jcr.JcrVersionManager$MergeCommand.execute(JcrVersionManager.java:1663)

                    at org.modeshape.jcr.JcrVersionManager.merge(JcrVersionManager.java:944)

                    at org.modeshape.jcr.JcrVersionManager.merge(JcrVersionManager.java:788)

                    at org.modeshape.jcr.JcrVersionManager.merge(JcrVersionManager.java:773)

                    at org.modeshape.jcr.JcrVersioningTest.shouldSetMergeFailedPropertyIfNodeIsCheckedIn(JcrVersioningTest.java:666)

                    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

                    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

                    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)

                    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)

                    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)

                    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)

                    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)

                    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)

                    at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)

                    at org.junit.rules.RunRules.evaluate(RunRules.java:18)

                    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)

                    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)

                    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)

                    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)

                    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)

                    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)

                    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)

                    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)

                    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)

                    at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)

                    at org.junit.runners.ParentRunner.run(ParentRunner.java:300)

                    at org.junit.runner.JUnitCore.run(JUnitCore.java:157)

                    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:77)

                    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:195)

                    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:63)

                    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

                    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

                    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:120)

                 

                Thanks and sorry for distracting you from doing the release.