jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From un...@apache.org
Subject svn commit: r1350221 - 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 12:55:29 GMT
Author: unico
Date: Thu Jun 14 12:55:29 2012
New Revision: 1350221

URL: http://svn.apache.org/viewvc?rev=1350221&view=rev
Log:
JCR-3267 fix bundles immediately during checkbundle in order to avoid lost update problem;
also add unit test for consistency fixing

Added:
    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

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=1350221&r1=1350220&r2=1350221&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 12:55:29 2012
@@ -76,8 +76,6 @@ public class ConsistencyCheckerImpl {
             boolean fix, Set<ReportItem> reports, String lostNFoundId)
             throws RepositoryException {
         int count = 0;
-        Collection<NodePropBundle> modifications = new ArrayList<NodePropBundle>();
-        Set<NodeId> orphaned = new HashSet<NodeId>();
 
         NodeId lostNFound = null;
         if (fix && lostNFoundId != null) {
@@ -116,9 +114,7 @@ public class ConsistencyCheckerImpl {
                                 error(id.toString(), "No bundle found for id '"
                                         + id + "'");
                             } else {
-                                checkBundleConsistency(id, bundle, fix,
-                                        modifications, lostNFound, orphaned,
-                                        reports);
+                                checkBundleConsistency(id, bundle, fix, lostNFound, reports);
 
                                 count++;
                                 if (count % 1000 == 0 && listener == null) {
@@ -174,8 +170,7 @@ public class ConsistencyCheckerImpl {
                                     + id + "'");
                         }
                     } else {
-                        checkBundleConsistency(id, bundle, fix, modifications,
-                                lostNFound, orphaned, reports);
+                        checkBundleConsistency(id, bundle, fix, lostNFound, reports);
 
                         if (recursive) {
                             for (NodePropBundle.ChildNodeEntry entry : bundle
@@ -197,49 +192,6 @@ public class ConsistencyCheckerImpl {
             }
         }
 
-        // repair collected broken bundles
-        if (fix && !modifications.isEmpty()) {
-            info(null, pm + ": Fixing " + modifications.size()
-                    + " inconsistent bundle(s)...");
-            for (NodePropBundle bundle : modifications) {
-                try {
-                    info(bundle.getId().toString(), pm + ": Fixing bundle '"
-                            + bundle.getId() + "'");
-                    bundle.markOld(); // use UPDATE instead of INSERT
-                    pm.storeBundle(bundle);
-                    pm.evictBundle(bundle.getId());
-                } catch (ItemStateException e) {
-                    error(bundle.getId().toString(), pm
-                            + ": Error storing fixed bundle: " + e);
-                }
-            }
-        }
-
-        if (fix && lostNFoundId != null && !orphaned.isEmpty()) {
-            // do we have things to add to "lost+found"?
-            try {
-                NodePropBundle lfBundle = pm.loadBundle(lostNFound);
-                if (lfBundle == null) {
-                    error(lostNFoundId, "specified 'lost+found' node does not exist");
-                } else if (!NameConstants.NT_UNSTRUCTURED.equals(lfBundle
-                        .getNodeTypeName())) {
-                    error(lostNFoundId, "specified 'lost+found' node is not of type nt:unstructered");
-                } else {
-                    lfBundle.markOld();
-                    for (NodeId orphan : orphaned) {
-                        String nodeName = orphan + "-"
-                                + System.currentTimeMillis();
-                        lfBundle.addChildNodeEntry(NF.create("", nodeName),
-                                orphan);
-                    }
-                    pm.storeBundle(lfBundle);
-                    pm.evictBundle(lfBundle.getId());
-                }
-            } catch (Exception ex) {
-                error(null, "trying orphan adoption", ex);
-            }
-        }
-
         log.info(pm + ": checked " + count + " bundles.");
 
         return count;
@@ -255,13 +207,8 @@ public class ConsistencyCheckerImpl {
      *            the bundle to check
      * @param fix
      *            if <code>true</code>, repair things that can be repaired
-     * @param modifications
-     *            if <code>fix == true</code>, collect the repaired
-     *            {@linkplain NodePropBundle bundles} here
      */
-    private void checkBundleConsistency(NodeId id, NodePropBundle bundle,
-            boolean fix, Collection<NodePropBundle> modifications,
-            NodeId lostNFoundId, Set<NodeId> orphaned, Set<ReportItem> reports)
{
+    private void checkBundleConsistency(NodeId id, NodePropBundle bundle, boolean fix, NodeId
lostNFoundId, Set<ReportItem> reports) {
         // log.info(name + ": checking bundle '" + id + "'");
 
         // skip all virtual nodes
@@ -354,7 +301,7 @@ public class ConsistencyCheckerImpl {
             for (NodePropBundle.ChildNodeEntry entry : missingChildren) {
                 bundle.getChildNodeEntries().remove(entry);
             }
-            modifications.add(bundle);
+            fixBundle(bundle);
         }
 
         // check parent reference
@@ -374,10 +321,18 @@ public class ConsistencyCheckerImpl {
                                     + "'";
                             log.error(message);
                             addMessage(reports, id, message);
-                            orphaned.add(id);
-                            if (lostNFoundId != null) {
+                            if (fix && lostNFoundId != null) {
+                                // add a child to lost+found
+                                NodePropBundle lfBundle = pm.loadBundle(lostNFoundId);
+                                lfBundle.markOld();
+                                String nodeName = id + "-" + System.currentTimeMillis();
+                                lfBundle.addChildNodeEntry(NF.create("", nodeName), id);
+                                pm.storeBundle(lfBundle);
+                                pm.evictBundle(lostNFoundId);
+
+                                // set lost+found parent
                                 bundle.setParentId(lostNFoundId);
-                                modifications.add(bundle);
+                                fixBundle(bundle);
                             }
                         }
                     } else {
@@ -403,18 +358,18 @@ public class ConsistencyCheckerImpl {
                                         + parentId + "'";
                                 log.error(message);
                                 addMessage(reports, id, message);
-
-                                int l = (int) System.currentTimeMillis();
-                                int r = new Random().nextInt();
-                                int n = l + r;
-                                String nodeName = Integer.toHexString(n);
-                                parentBundle.addChildNodeEntry(
-                                        NF.create("{}" + nodeName), id);
-                                log.info("NodeState '" + id
-                                        + "' adds itself to its parent node '"
-                                        + parentId + "' with a new name '" + nodeName
-                                        + "'");
-                                modifications.add(parentBundle);
+                                if (fix) {
+                                    int l = (int) System.currentTimeMillis();
+                                    int r = new Random().nextInt();
+                                    int n = l + r;
+                                    String nodeName = Integer.toHexString(n);
+                                    parentBundle.addChildNodeEntry(NF.create("{}" + nodeName),
id);
+                                    log.info("NodeState '" + id
+                                            + "' adds itself to its parent node '"
+                                            + parentId + "' with a new name '" + nodeName
+                                            + "'");
+                                    fixBundle(parentBundle);
+                                }
                             }
                         } else {
                             return;
@@ -486,4 +441,15 @@ public class ConsistencyCheckerImpl {
             listener.error(id, message);
         }
     }
+
+    private void fixBundle(NodePropBundle bundle) {
+        try {
+            log.info(pm + ": Fixing bundle '" + bundle.getId() + "'");
+            bundle.markOld(); // use UPDATE instead of INSERT
+            pm.storeBundle(bundle);
+            pm.evictBundle(bundle.getId());
+        } catch (ItemStateException e) {
+            log.error(pm + ": Error storing fixed bundle: " + e);
+        }
+    }
 }

Added: 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=1350221&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
(added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/ConsistencyCheckerImplTest.java
Thu Jun 14 12:55:29 2012
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.data;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+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.util.BLOBStore;
+import org.apache.jackrabbit.core.persistence.util.NodePropBundle;
+import org.apache.jackrabbit.core.state.ItemStateException;
+import org.apache.jackrabbit.core.state.NoSuchItemStateException;
+import org.apache.jackrabbit.core.state.NodeReferences;
+import org.apache.jackrabbit.spi.NameFactory;
+import org.apache.jackrabbit.spi.commons.name.NameConstants;
+import org.apache.jackrabbit.spi.commons.name.NameFactoryImpl;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import junit.framework.TestCase;
+
+public class ConsistencyCheckerImplTest extends TestCase {
+
+    private static final Logger log = LoggerFactory.getLogger(ConsistencyCheckerImplTest.class);
+
+    private static final NameFactory nameFactory = NameFactoryImpl.getInstance();
+
+    public void testFixAbandonedNode() throws RepositoryException {
+        NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
+        NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
+
+        // node2 has a reference to node 1 as its parent, but node 1 doesn't have
+        // a corresponding child node entry
+        bundle2.setParentId(bundle1.getId());
+
+        MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle1, bundle2));
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+
+        // run checker with fix = true
+        checker.check(null, false, true, null);
+
+        bundle1 = pm.loadBundle(bundle1.getId());
+        assertEquals(1, bundle1.getChildNodeEntries().size());
+        assertEquals(bundle2.getId(), bundle1.getChildNodeEntries().get(0).getId());
+    }
+
+    /*
+     * There was a bug where when there were multiple abandoned nodes by the same parent
+     * only one of them was fixed. Hence this separate test case for this scenario.
+     */
+    public void testFixMultipleAbandonedNodesBySameParent() 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));
+
+
+        // node2 and node3 have a reference to node1 as its parent, but node1 doesn't have
+        // corresponding child node entries
+        bundle2.setParentId(bundle1.getId());
+        bundle3.setParentId(bundle1.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());
+        assertEquals(2, bundle1.getChildNodeEntries().size());
+        assertEquals(bundle2.getId(), bundle1.getChildNodeEntries().get(0).getId());
+        assertEquals(bundle3.getId(), bundle1.getChildNodeEntries().get(1).getId());
+    }
+
+    public void testAddOrphanedNodeToLostAndFound() throws RepositoryException {
+        NodePropBundle lostAndFound = new NodePropBundle(new NodeId(0, 0));
+        // lost and found must be of type nt:unstructured
+        lostAndFound.setNodeTypeName(NameConstants.NT_UNSTRUCTURED);
+
+        NodePropBundle orphaned = new NodePropBundle(new NodeId(0, 1));
+        // set non-existent parent node id
+        orphaned.setParentId(new NodeId(1, 0));
+
+        MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(lostAndFound,
orphaned));
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+
+        // run checker with fix = true
+        checker.check(null, false, true, lostAndFound.getId().toString());
+
+        lostAndFound = pm.loadBundle(lostAndFound.getId());
+        assertEquals(1, lostAndFound.getChildNodeEntries().size());
+        assertEquals(orphaned.getId(), lostAndFound.getChildNodeEntries().get(0).getId());
+
+        orphaned = pm.loadBundle(orphaned.getId());
+        assertEquals(lostAndFound.getId(), orphaned.getParentId());
+    }
+
+    private static class MockPersistenceManager extends AbstractBundlePersistenceManager
{
+
+        private Map<NodeId, NodePropBundle> bundles = new LinkedHashMap<NodeId,
NodePropBundle>();
+
+        private MockPersistenceManager(List<NodePropBundle> bundles) {
+            for (NodePropBundle bundle : bundles) {
+                this.bundles.put(bundle.getId(), bundle);
+            }
+        }
+
+        public List<NodeId> getAllNodeIds(final NodeId after, final int maxCount) throws
ItemStateException, RepositoryException {
+            List<NodeId> allNodeIds = new ArrayList<NodeId>();
+            boolean add = after == null;
+            for (NodeId nodeId : bundles.keySet()) {
+                if (add) {
+                    allNodeIds.add(nodeId);
+                }
+                if (!add) {
+                    add = nodeId.equals(after);
+                }
+            }
+            return allNodeIds;
+        }
+
+        @Override
+        protected NodePropBundle loadBundle(final NodeId id) {
+            return bundles.get(id);
+        }
+
+        @Override
+        protected void evictBundle(final NodeId id) {
+        }
+
+        @Override
+        protected void storeBundle(final NodePropBundle bundle) throws ItemStateException
{
+            bundles.put(bundle.getId(), bundle);
+        }
+
+        @Override
+        protected void destroyBundle(final NodePropBundle bundle) throws ItemStateException
{
+            bundles.remove(bundle.getId());
+        }
+
+        @Override
+        protected void destroy(final NodeReferences refs) throws ItemStateException {
+        }
+
+        @Override
+        protected void store(final NodeReferences refs) throws ItemStateException {
+        }
+
+        @Override
+        protected BLOBStore getBlobStore() {
+            return null;
+        }
+
+        public NodeReferences loadReferencesTo(final NodeId id) throws NoSuchItemStateException,
ItemStateException {
+            return null;
+        }
+
+        public boolean existsReferencesTo(final NodeId targetId) throws ItemStateException
{
+            return false;
+        }
+    }
+}
\ No newline at end of file



Mime
View raw message