Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2.11, 2.4
    • Fix Version/s: 2.2.12, 2.4.2, 2.5
    • Component/s: None
    • Labels:
      None

      Description

      The scenario is as follows:

      We start out with the following repository structure:
      /folder1/node1
      /folder2

      Session s1 modifies /folder1 (for instance set a property or add a node, it doesn't matter, as long as it creates a transient item state for /folder1)
      Session s2 moves /folder1/node1 to /folder2/node1
      Session s2 does a save
      Session s1 modifies /folder2/node1 (for instance set a property or add a node, it doesn't matter, as long as it creates a transient item state for node2)
      Session s1 does a save

      The repository is now inconsistent: /folder1 has a child node entry for node1 whereas node1 has parent id for /folder2

      The problem is in the NodeStateMerger.merge method. When on s1.save a merge is attempted on the child node entries of /folder1 the transient state has a child node entry for node1 (which was actually moved to /folder2). This expected. The merger decides to add this node if one of two conditions is true: the child node entry is either in the added set of the change log or it is in the modified set of the change log (the latter because the child node will be in the modified set if it was moved to this node). The latter condition is the problem. Because node1 was modified it is indeed in the modified set and thus added.

      However it has a different parent id (the parent id of the folder it was moved to). That means that /folder1 is saved with a child node entry for node1.

      Attached is a test case that shows that the above scenario leads to an inconsistency.

      Also attached is a proposal for a fix. The fix checks whether the child node entry to be added actually has the state that is currently being merged as its parent and only adds it when it does.

      1. SISMMerger.patch
        5 kB
        Unico Hommes
      2. AddMoveTest.patch
        3 kB
        Unico Hommes

        Activity

        Unico Hommes created issue -
        Unico Hommes made changes -
        Field Original Value New Value
        Attachment AddMoveTest.patch [ 12522125 ]
        Unico Hommes made changes -
        Attachment SISMMerge.patch [ 12522126 ]
        Unico Hommes made changes -
        Description The scenario is as follows:

        We start out with the following repository structure:
        /folder1/node1
        /folder2

        Session s1 modifies /folder1 (for instance set a property or add a node, it doesn't matter, as long as it creates a transient item state for /folder1)
        Session s2 moves /folder1/node1 to /folder2/node1
        Session s2 does a save
        Session s1 modifies /folder2/node1 (for instance set a property or add a node, it doesn't matter, as long as it creates a transient item state for node2)
        Session s1 does a save

        The repository is now inconsistent: /folder1 has a child node entry for node1 whereas node1 has parent id for /folder2

        The problem is in the NodeStateMerger.merge method. When on s1.save a merge is attempted on the child node entries of /folder1 the transient state has a child node entry for node1 (which was actually moved to /folder2). This expected. The merger decides to add this node if one of two conditions is true: the child node entry is either in the added set of the change log or it is in the modified set of the change log (the latter because the child node will be in the modified set if it was moved to this node). The latter condition is the problem. Because node1 was modified it is indeed in the modified set and thus added.

        However it has a different parent id (the parent id of the folder it was moved to). That means that /folder1 is saved with a child node entry for node1.

        Attached is a test case that shows that the above scenario leads to an inconsistency.

        Also attached is a proposal for a fix. The patch modifies the semantics of NodeStateMerger.MergerContext#isAdded somewhat: it returns true if either the child is in the added set or it is both in the modified set and its parent is the state that is currently being merged. Perhaps the method should be renamed to isAddedTo instead of just isAdded.
        The scenario is as follows:

        We start out with the following repository structure:
        /folder1/node1
        /folder2

        Session s1 modifies /folder1 (for instance set a property or add a node, it doesn't matter, as long as it creates a transient item state for /folder1)
        Session s2 moves /folder1/node1 to /folder2/node1
        Session s2 does a save
        Session s1 modifies /folder2/node1 (for instance set a property or add a node, it doesn't matter, as long as it creates a transient item state for node2)
        Session s1 does a save

        The repository is now inconsistent: /folder1 has a child node entry for node1 whereas node1 has parent id for /folder2

        The problem is in the NodeStateMerger.merge method. When on s1.save a merge is attempted on the child node entries of /folder1 the transient state has a child node entry for node1 (which was actually moved to /folder2). This expected. The merger decides to add this node if one of two conditions is true: the child node entry is either in the added set of the change log or it is in the modified set of the change log (the latter because the child node will be in the modified set if it was moved to this node). The latter condition is the problem. Because node1 was modified it is indeed in the modified set and thus added.

        However it has a different parent id (the parent id of the folder it was moved to). That means that /folder1 is saved with a child node entry for node1.

        Attached is a test case that shows that the above scenario leads to an inconsistency.

        Also attached is a proposal for a fix. The fix checks whether the child node entry to be added actually has the state that is currently being merged as its parent and only adds it when it does.
        Unico Hommes made changes -
        Attachment SISMMerge.patch [ 12522126 ]
        Unico Hommes made changes -
        Attachment SISMMerger.patch [ 12522150 ]
        Bart van der Schans made changes -
        Assignee Bart van der Schans [ schans ]
        Bart van der Schans made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 2.6 [ 12319480 ]
        Resolution Fixed [ 1 ]
        Alex Parvulescu made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Jukka Zitting made changes -
        Fix Version/s 2.5 [ 12319280 ]
        Fix Version/s 2.6 [ 12319480 ]

          People

          • Assignee:
            Bart van der Schans
            Reporter:
            Unico Hommes
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development