jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From un...@apache.org
Subject svn commit: r1350224 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
Date Thu, 14 Jun 2012 13:15:38 GMT
Author: unico
Date: Thu Jun 14 13:15:37 2012
New Revision: 1350224

URL: http://svn.apache.org/viewvc?rev=1350224&view=rev
Log:
JCR-3269 fix 'disconnected' nodes by removing them from the node that references them as a
child

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java?rev=1350224&r1=1350223&r2=1350224&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
Thu Jun 14 13:15:37 2012
@@ -222,6 +222,7 @@ public class ConsistencyCheckerImpl {
 
         // look at the node's children
         Collection<NodePropBundle.ChildNodeEntry> missingChildren = new ArrayList<NodePropBundle.ChildNodeEntry>();
+        Collection<NodePropBundle.ChildNodeEntry> disconnectedChildren = new ArrayList<NodePropBundle.ChildNodeEntry>();
         for (NodePropBundle.ChildNodeEntry entry : bundle.getChildNodeEntries()) {
 
             final NodeId childNodeId = entry.getId();
@@ -261,7 +262,7 @@ public class ConsistencyCheckerImpl {
                     }
                 } else {
                     NodeId cp = childBundle.getParentId();
-                    if (cp == null || !cp.equals(id)) {
+                    if (!id.equals(cp)) {
                         // double check whether the child entry is still there
                         bundle = pm.loadBundle(id);
                         if (bundle != null) {
@@ -273,14 +274,10 @@ public class ConsistencyCheckerImpl {
                                 }
                             }
                             if (stillThere) {
-                                if (cp == null) {
-                                    message = "ChildNode has invalid parent id: <null>";
-                                    log.error(message);
-                                } else if (!cp.equals(id)) {
-                                    message = "ChildNode has invalid parent id: '" + cp
-                                            + "' (instead of '" + id + "')";
-                                    log.error(message);
-                                }
+                                // indeed we have a disconnected child
+                                message = "ChildNode has invalid parent id: '" + cp + "'
(instead of '" + id + "')";
+                                log.error(message);
+                                disconnectedChildren.add(entry);
                             }
                         } else {
                             return;
@@ -297,10 +294,13 @@ public class ConsistencyCheckerImpl {
             }
         }
         // remove child node entry (if fixing is enabled)
-        if (fix && !missingChildren.isEmpty()) {
+        if (fix && (!missingChildren.isEmpty() || !disconnectedChildren.isEmpty()))
{
             for (NodePropBundle.ChildNodeEntry entry : missingChildren) {
                 bundle.getChildNodeEntries().remove(entry);
             }
+            for (NodePropBundle.ChildNodeEntry entry : disconnectedChildren) {
+                bundle.getChildNodeEntries().remove(entry);
+            }
             fixBundle(bundle);
         }
 

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java?rev=1350224&r1=1350223&r2=1350224&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
Thu Jun 14 13:15:37 2012
@@ -27,6 +27,7 @@ import javax.jcr.RepositoryException;
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.persistence.bundle.AbstractBundlePersistenceManager;
 import org.apache.jackrabbit.core.persistence.bundle.ConsistencyCheckerImpl;
+import org.apache.jackrabbit.core.persistence.check.ConsistencyChecker;
 import org.apache.jackrabbit.core.persistence.util.BLOBStore;
 import org.apache.jackrabbit.core.persistence.util.NodePropBundle;
 import org.apache.jackrabbit.core.state.ItemStateException;
@@ -46,6 +47,8 @@ public class ConsistencyCheckerImplTest 
 
     private static final NameFactory nameFactory = NameFactoryImpl.getInstance();
 
+    // Abandoned nodes are nodes that have a link to a parent but that
+    // parent does not have a link back to the child
     public void testFixAbandonedNode() throws RepositoryException {
         NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
         NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
@@ -60,6 +63,7 @@ public class ConsistencyCheckerImplTest 
         // run checker with fix = true
         checker.check(null, false, true, null);
 
+        // node1 should now have a child node entry for node2
         bundle1 = pm.loadBundle(bundle1.getId());
         assertEquals(1, bundle1.getChildNodeEntries().size());
         assertEquals(bundle2.getId(), bundle1.getChildNodeEntries().get(0).getId());
@@ -86,12 +90,14 @@ public class ConsistencyCheckerImplTest 
         // run checker with fix = true
         checker.check(null, false, true, null);
 
+        // node1 should now have child node entries for node2 and node3
         bundle1 = pm.loadBundle(bundle1.getId());
         assertEquals(2, bundle1.getChildNodeEntries().size());
         assertEquals(bundle2.getId(), bundle1.getChildNodeEntries().get(0).getId());
         assertEquals(bundle3.getId(), bundle1.getChildNodeEntries().get(1).getId());
     }
 
+    // Orphaned nodes are those nodes who's parent does not exist
     public void testAddOrphanedNodeToLostAndFound() throws RepositoryException {
         NodePropBundle lostAndFound = new NodePropBundle(new NodeId(0, 0));
         // lost and found must be of type nt:unstructured
@@ -107,6 +113,7 @@ public class ConsistencyCheckerImplTest 
         // run checker with fix = true
         checker.check(null, false, true, lostAndFound.getId().toString());
 
+        // orphan should have been added to lost+found
         lostAndFound = pm.loadBundle(lostAndFound.getId());
         assertEquals(1, lostAndFound.getChildNodeEntries().size());
         assertEquals(orphaned.getId(), lostAndFound.getChildNodeEntries().get(0).getId());
@@ -115,6 +122,81 @@ public class ConsistencyCheckerImplTest 
         assertEquals(lostAndFound.getId(), orphaned.getParentId());
     }
 
+    // Disconnected nodes are those nodes for which there are nodes
+    // that have the node as its child, but the node itself does not
+    // have those nodes as its parent
+    public void testFixDisconnectedNode() throws RepositoryException {
+        NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
+        NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
+        NodePropBundle bundle3 = new NodePropBundle(new NodeId(1, 0));
+
+        // node1 has child node3
+        bundle1.addChildNodeEntry(nameFactory.create("", "test"), bundle3.getId());
+        // node2 also has child node3
+        bundle2.addChildNodeEntry(nameFactory.create("", "test"), bundle3.getId());
+        // node3 has node2 as parent
+        bundle3.setParentId(bundle2.getId());
+
+        MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle1, bundle2,
bundle3));
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+
+        // run checker with fix = true
+        checker.check(null, false, true, null);
+
+        bundle1 = pm.loadBundle(bundle1.getId());
+        bundle2 = pm.loadBundle(bundle2.getId());
+        bundle3 = pm.loadBundle(bundle3.getId());
+
+        // node3 should have been removed as child node entry of node1
+        assertEquals(0, bundle1.getChildNodeEntries().size());
+
+        // node3 should still be a child of node2
+        assertEquals(1, bundle2.getChildNodeEntries().size());
+        assertEquals(bundle2.getId(), bundle3.getParentId());
+    }
+
+    // make sure we don't fix anything in check mode, we can't be careful enough
+    public void testDontFixInCheckMode() throws RepositoryException {
+        /** abandoned node, also see {@link #testFixAbandonedNode} */
+        NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
+        NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
+        bundle2.setParentId(bundle1.getId());
+
+        /** orphaned node, also see {@link #testAddOrphanedNodeToLostAndFound} */
+        NodePropBundle lostAndFound = new NodePropBundle(new NodeId(1, 0));
+        lostAndFound.setNodeTypeName(NameConstants.NT_UNSTRUCTURED);
+        NodePropBundle orphaned = new NodePropBundle(new NodeId(1, 1));
+        orphaned.setParentId(new NodeId(0, 2));
+
+        /** disconnected node, also see {@link #testFixDisconnectedNode} */
+        NodePropBundle bundle3 = new NodePropBundle(new NodeId(1, 2));
+        NodePropBundle bundle4 = new NodePropBundle(new NodeId(2, 2));
+        NodePropBundle bundle5 = new NodePropBundle(new NodeId(0, 3));
+        bundle3.addChildNodeEntry(nameFactory.create("", "test"), bundle5.getId());
+        bundle4.addChildNodeEntry(nameFactory.create("", "test"), bundle5.getId());
+        bundle5.setParentId(bundle4.getId());
+
+        MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle1, bundle2,
bundle3, bundle4, bundle5, lostAndFound, orphaned));
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+
+        // run checker with fix = false
+        checker.check(null, false, false, null);
+
+        bundle1 = pm.loadBundle(bundle1.getId());
+        lostAndFound = pm.loadBundle(lostAndFound.getId());
+        bundle3 = pm.loadBundle(bundle3.getId());
+
+        // abandoned node should not have been un-abandoned
+        assertEquals(0, bundle1.getChildNodeEntries().size());
+
+        // orphan should not have been added to lost and found
+        assertEquals(0, lostAndFound.getChildNodeEntries().size());
+
+        // disconnected entries should not have been removed
+        assertEquals(1, bundle3.getChildNodeEntries().size());
+
+    }
+    
     private static class MockPersistenceManager extends AbstractBundlePersistenceManager
{
 
         private Map<NodeId, NodePropBundle> bundles = new LinkedHashMap<NodeId,
NodePropBundle>();



Mime
View raw message