You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by un...@apache.org on 2012/06/16 08:55:36 UTC
svn commit: r1350872 - in /jackrabbit/branches/2.4/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
Author: unico
Date: Sat Jun 16 06:55:36 2012
New Revision: 1350872
URL: http://svn.apache.org/viewvc?rev=1350872&view=rev
Log:
JCR-3269 fix 'disconnected' nodes by removing them from the node that references them as a child (backport)
Modified:
jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java?rev=1350872&r1=1350871&r2=1350872&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java Sat Jun 16 06:55:36 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/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java?rev=1350872&r1=1350871&r2=1350872&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java Sat Jun 16 06:55:36 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>();