Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.5
    • Fix Version/s: 1.6
    • Component/s: jackrabbit-core
    • Labels:
      None

      Description

      Moving a node and then refreshing it can make it disappear.

      deleteDirectory(new File("repository"));
      Repository rep = new TransientRepository();
      Session session = rep.login(new SimpleCredentials("", new char[0]));
      Node root = session.getRootNode();
      Node a = root.addNode("a");
      Node b = a.addNode("b");
      session.save();
      session.move("/a/b", "/b");
      b.refresh(false);
      // session.save(); // no effect
      for (NodeIterator it = root.getNodes(); it.hasNext() {
      Node n = it.nextNode();
      System.out.println(n.getName());
      for (NodeIterator it2 = n.getNodes(); it2.hasNext()

      { System.out.println(" " + it2.nextNode().getName()); }

      }

      In the trunk, the node 'b' is not listed after the refresh (not under the root page, and not under a). The output is:
      jcr:system
      jcr:versionStorage
      jcr:nodeTypes
      a

      Jackrabbit 1.4.x throws an exception:

      jcr:system
      jcr:versionStorage
      jcr:nodeTypes
      a
      Exception in thread "main" javax.jcr.RepositoryException: failed to resolve name of acee31c4-c33b-4ed4-b1b5-39db6f17fb09
      at org.apache.jackrabbit.core.HierarchyManagerImpl.getName(HierarchyManagerImpl.java:451)
      at org.apache.jackrabbit.core.CachingHierarchyManager.getName(CachingHierarchyManager.java:287)
      at org.apache.jackrabbit.core.NodeImpl.getName(NodeImpl.java:1931)
      at org.apache.jackrabbit.core.fuzz.TestMoveRemoveRefresh.test(TestMoveRemoveRefresh.java:33)
      at org.apache.jackrabbit.core.fuzz.TestMoveRemoveRefresh.main(TestMoveRemoveRefresh.java:15)

      void deleteDirectory(File file) {
      if (file.isDirectory()) {
      File[] list = file.listFiles();
      for(int i=0; i<list.length; i++)

      { deleteDirectory(list[i]); }

      }
      file.delete();
      }

        Activity

        Thomas Mueller created issue -
        Hide
        Thomas Mueller added a comment -

        There are multiple solutions to the problem.

        One is, refresh will undo the move operation. But that would also change the old parent node, which may not be expected.

        Another solution is to reject refreshing this node. This is what I have implemented. Here is a proposed patch. I also want to add a test case for this issue.

        Index: src/main/java/org/apache/jackrabbit/core/ItemImpl.java
        ===================================================================
        — src/main/java/org/apache/jackrabbit/core/ItemImpl.java (revision 722453)
        +++ src/main/java/org/apache/jackrabbit/core/ItemImpl.java (working copy)
        @@ -1171,10 +1171,19 @@
        switch (transientState.getStatus()) {
        case ItemState.STATUS_STALE_MODIFIED:
        case ItemState.STATUS_STALE_DESTROYED:

        • case ItemState.STATUS_EXISTING_MODIFIED:
          // add this item's state to the list
          list.add(transientState);
          break;
          +
          + case ItemState.STATUS_EXISTING_MODIFIED:
          + if (!transientState.getParentId().equals(
          + transientState.getOverlayedState().getParentId())) { + throw new RepositoryException( + "Cannot refresh a moved item: " + this + + " - possible solution: refresh the parent"); + }

          + list.add(transientState);
          + break;

        case ItemState.STATUS_NEW:
        throw new RepositoryException(

        Show
        Thomas Mueller added a comment - There are multiple solutions to the problem. One is, refresh will undo the move operation. But that would also change the old parent node, which may not be expected. Another solution is to reject refreshing this node. This is what I have implemented. Here is a proposed patch. I also want to add a test case for this issue. Index: src/main/java/org/apache/jackrabbit/core/ItemImpl.java =================================================================== — src/main/java/org/apache/jackrabbit/core/ItemImpl.java (revision 722453) +++ src/main/java/org/apache/jackrabbit/core/ItemImpl.java (working copy) @@ -1171,10 +1171,19 @@ switch (transientState.getStatus()) { case ItemState.STATUS_STALE_MODIFIED: case ItemState.STATUS_STALE_DESTROYED: case ItemState.STATUS_EXISTING_MODIFIED: // add this item's state to the list list.add(transientState); break; + + case ItemState.STATUS_EXISTING_MODIFIED: + if (!transientState.getParentId().equals( + transientState.getOverlayedState().getParentId())) { + throw new RepositoryException( + "Cannot refresh a moved item: " + this + + " - possible solution: refresh the parent"); + } + list.add(transientState); + break; case ItemState.STATUS_NEW: throw new RepositoryException(
        Hide
        Thomas Mueller added a comment -

        Fixed in revision 725292. The unit tests succeed, but I'm not sure if we should backport this fix yet - it may have some unwanted side effects.

        Show
        Thomas Mueller added a comment - Fixed in revision 725292. The unit tests succeed, but I'm not sure if we should backport this fix yet - it may have some unwanted side effects.
        Thomas Mueller made changes -
        Field Original Value New Value
        Assignee Thomas Mueller [ tmueller ]
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Hide
        Jukka Zitting added a comment -

        OK, marking this for 1.6.0. If there's end user demand for a fix in 1.5.x then we can reconsider merging the fix to the 1.5 branch.

        Show
        Jukka Zitting added a comment - OK, marking this for 1.6.0. If there's end user demand for a fix in 1.5.x then we can reconsider merging the fix to the 1.5 branch.
        Jukka Zitting made changes -
        Affects Version/s 1.5.0 [ 12312920 ]
        Fix Version/s 1.6.0 [ 12313459 ]
        Jukka Zitting made changes -
        Workflow jira [ 12447194 ] no-reopen-closed, patch-avail [ 12468211 ]
        Jukka Zitting made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Thomas Mueller
            Reporter:
            Thomas Mueller
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development