jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From un...@apache.org
Subject svn commit: r1448252 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/ main/java/org/apache/jackrabbit/core/persistence/bundle/ main/java/org/apache/jackrabbit/core/persistence/check/ main/java/org/apache/jackrabbit/core...
Date Wed, 20 Feb 2013 15:42:29 GMT
Author: unico
Date: Wed Feb 20 15:42:29 2013
New Revision: 1448252

URL: http://svn.apache.org/r1448252
Log:
JCR-3525 inform cluster of changes made during consistency repairs
- added a lot more unit test coverage, including verification of cluster update events
- refactored the code to be more modular
- report items now also report whether inconsistency error was fixed

Added:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerError.java
Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ConsistencyChecker.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItem.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItemImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.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/RepositoryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?rev=1448252&r1=1448251&r2=1448252&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java Wed Feb 20 15:42:29 2013
@@ -89,6 +89,7 @@ import org.apache.jackrabbit.core.observ
 import org.apache.jackrabbit.core.persistence.IterablePersistenceManager;
 import org.apache.jackrabbit.core.persistence.PMContext;
 import org.apache.jackrabbit.core.persistence.PersistenceManager;
+import org.apache.jackrabbit.core.persistence.check.ConsistencyChecker;
 import org.apache.jackrabbit.core.retention.RetentionRegistry;
 import org.apache.jackrabbit.core.retention.RetentionRegistryImpl;
 import org.apache.jackrabbit.core.security.JackrabbitSecurityManager;
@@ -2041,6 +2042,9 @@ public class RepositoryImpl extends Abst
                     updateChannel = clusterNode.createUpdateChannel(getName());
                     itemStateMgr.setEventChannel(updateChannel);
                     updateChannel.setListener(this);
+                    if (persistMgr instanceof ConsistencyChecker) {
+                        ((ConsistencyChecker) persistMgr).setEventChannel(updateChannel);
+                    }
                 }
             } catch (ItemStateException ise) {
                 String msg = "failed to instantiate shared item state manager";

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java?rev=1448252&r1=1448251&r2=1448252&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java Wed Feb 20 15:42:29 2013
@@ -34,6 +34,7 @@ import org.apache.jackrabbit.api.stats.R
 import org.apache.jackrabbit.core.cache.Cache;
 import org.apache.jackrabbit.core.cache.CacheAccessListener;
 import org.apache.jackrabbit.core.cache.ConcurrentCache;
+import org.apache.jackrabbit.core.cluster.UpdateEventChannel;
 import org.apache.jackrabbit.core.fs.FileSystem;
 import org.apache.jackrabbit.core.fs.FileSystemResource;
 import org.apache.jackrabbit.core.id.ItemId;
@@ -171,10 +172,12 @@ public abstract class AbstractBundlePers
     /** Duration of bundle read operations. */
     private AtomicLong cacheMissDuration;
 
-    
     /** Counter of bundle cache size. */
     private AtomicLong cacheSizeCounter;
 
+    /** The update event channel to use by the consistency checker when fixing inconsistencies */
+    private UpdateEventChannel eventChannel;
+
     /**
      * Returns the size of the bundle cache in megabytes.
      * @return the size of the bundle cache in megabytes.
@@ -817,21 +820,34 @@ public abstract class AbstractBundlePers
      */
     public void checkConsistency(String[] uuids, boolean recursive, boolean fix) {
         try {
-            ConsistencyCheckerImpl cs = new ConsistencyCheckerImpl(this, null);
-            cs.check(uuids, recursive, fix, null);
+            ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(this, null, null, eventChannel);
+            checker.check(uuids, recursive);
+            checker.doubleCheckErrors();
+            if (fix) {
+                checker.repair();
+            }
         } catch (RepositoryException ex) {
             log.error("While running consistency check.", ex);
         }
     }
 
+    public void setEventChannel(UpdateEventChannel eventChannel) {
+        this.eventChannel = eventChannel;
+    }
+
     /**
      * {@inheritDoc}
      */
     public ConsistencyReport check(String[] uuids, boolean recursive,
             boolean fix, String lostNFoundId, ConsistencyCheckListener listener)
             throws RepositoryException {
-        ConsistencyCheckerImpl cs = new ConsistencyCheckerImpl(this, listener);
-        return cs.check(uuids, recursive, fix, lostNFoundId);
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(this, listener, lostNFoundId, eventChannel);
+        checker.check(uuids, recursive);
+        checker.doubleCheckErrors();
+        if (fix) {
+            checker.repair();
+        }
+        return checker.getReport();
     }
 
     /**

Added: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerError.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerError.java?rev=1448252&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerError.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/ConsistencyCheckerError.java Wed Feb 20 15:42:29 2013
@@ -0,0 +1,78 @@
+/*
+ * 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.persistence.bundle;
+
+import org.apache.jackrabbit.core.id.NodeId;
+import org.apache.jackrabbit.core.persistence.check.ReportItem;
+import org.apache.jackrabbit.core.persistence.check.ReportItemImpl;
+import org.apache.jackrabbit.core.state.ChangeLog;
+import org.apache.jackrabbit.core.state.ItemStateException;
+
+/**
+ * Base class for errors reported by the {@link ConsistencyCheckerImpl}
+ */
+abstract class ConsistencyCheckerError {
+
+    protected final String message;
+    protected final NodeId nodeId;
+    protected boolean repaired;
+
+    ConsistencyCheckerError(NodeId nodeId, String message) {
+        this.nodeId = nodeId;
+        this.message = message;
+    }
+
+    final NodeId getNodeId() {
+        return nodeId;
+    }
+
+    final void repair(final ChangeLog changes) throws ItemStateException {
+        doRepair(changes);
+        repaired = true;
+    }
+
+    final ReportItem getReportItem() {
+        return new ReportItemImpl(nodeId.toString(), message, getType(), repaired);
+    }
+
+    /**
+     * @return whether this error is repairable
+     */
+    abstract boolean isRepairable();
+
+    /**
+     * Repair this error and update the changelog.
+     *
+     * @param changes  the changelog to update with the changes made.
+     * @throws ItemStateException
+     */
+    abstract void doRepair(final ChangeLog changes) throws ItemStateException;
+
+    abstract ReportItem.Type getType();
+
+    /**
+     * Double check the error to eliminate false positives in live environments.
+     * @return  whether the error was confirmed.
+     * @throws ItemStateException
+     */
+    abstract boolean doubleCheck() throws ItemStateException;
+
+    @Override
+    public String toString() {
+        return getType() + " - " + getNodeId();
+    }
+}

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=1448252&r1=1448251&r2=1448252&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 Wed Feb 20 15:42:29 2013
@@ -17,9 +17,10 @@
 package org.apache.jackrabbit.core.persistence.bundle;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
@@ -27,15 +28,22 @@ import java.util.Set;
 
 import javax.jcr.RepositoryException;
 
+import org.apache.jackrabbit.core.cluster.ClusterException;
+import org.apache.jackrabbit.core.cluster.Update;
+import org.apache.jackrabbit.core.cluster.UpdateEventChannel;
 import org.apache.jackrabbit.core.id.NodeId;
+import org.apache.jackrabbit.core.observation.EventState;
 import org.apache.jackrabbit.core.persistence.check.ConsistencyCheckListener;
 import org.apache.jackrabbit.core.persistence.check.ConsistencyReport;
 import org.apache.jackrabbit.core.persistence.check.ConsistencyReportImpl;
 import org.apache.jackrabbit.core.persistence.check.ReportItem;
-import org.apache.jackrabbit.core.persistence.check.ReportItemImpl;
 import org.apache.jackrabbit.core.persistence.util.NodeInfo;
 import org.apache.jackrabbit.core.persistence.util.NodePropBundle;
+import org.apache.jackrabbit.core.state.ChangeLog;
+import org.apache.jackrabbit.core.state.DummyUpdateEventChannel;
+import org.apache.jackrabbit.core.state.ItemState;
 import org.apache.jackrabbit.core.state.ItemStateException;
+import org.apache.jackrabbit.core.state.NodeState;
 import org.apache.jackrabbit.spi.NameFactory;
 import org.apache.jackrabbit.spi.commons.name.NameConstants;
 import org.apache.jackrabbit.spi.commons.name.NameFactoryImpl;
@@ -47,12 +55,6 @@ public class ConsistencyCheckerImpl {
     /** the default logger */
     private static Logger log = LoggerFactory.getLogger(ConsistencyCheckerImpl.class);
 
-    private final AbstractBundlePersistenceManager pm;
-
-    private final ConsistencyCheckListener listener;
-
-    private static final NameFactory NF = NameFactoryImpl.getInstance();
-
     /**
      * The number of nodes to fetch at once from the persistence manager. Defaults to 8kb
      */
@@ -64,49 +66,157 @@ public class ConsistencyCheckerImpl {
      */
     private static final boolean CHECKAFTERLOADING = Boolean.getBoolean("org.apache.jackrabbit.checker.checkafterloading");
 
-    public ConsistencyCheckerImpl(AbstractBundlePersistenceManager pm,
-            ConsistencyCheckListener listener) {
+    /**
+     * Attribute name used to store the size of the update.
+     */
+    private static final String ATTRIBUTE_UPDATE_SIZE = "updateSize";
+
+    private static final NameFactory NF = NameFactoryImpl.getInstance();
+
+    private final AbstractBundlePersistenceManager pm;
+    private final ConsistencyCheckListener listener;
+    private NodeId lostNFoundId;
+    private UpdateEventChannel eventChannel = new DummyUpdateEventChannel();
+    private Map<NodeId, NodePropBundle> bundles;
+    private final List<ConsistencyCheckerError> errors = new ArrayList<ConsistencyCheckerError>();
+    private int nodeCount;
+    private long elapsedTime;
+
+    public ConsistencyCheckerImpl(AbstractBundlePersistenceManager pm, ConsistencyCheckListener listener,
+                                  String lostNFoundId, final UpdateEventChannel eventChannel) {
         this.pm = pm;
         this.listener = listener;
+        if (lostNFoundId != null) {
+            this.lostNFoundId = new NodeId(lostNFoundId);
+        }
+        if (eventChannel != null) {
+            this.eventChannel = eventChannel;
+        }
     }
 
-    public ConsistencyReport check(String[] uuids, boolean recursive,
-            boolean fix, String lostNFoundId) throws RepositoryException {
-        Set<ReportItem> reports = new HashSet<ReportItem>();
-
+    /**
+     * Check the database for inconsistencies.
+     *
+     * @param uuids a list of node identifiers to check or {@code null} in order to check all nodes
+     * @param recursive  whether to recursively check the subtrees below the nodes identified by the provided uuids
+     * @throws RepositoryException
+     */
+    public void check(String[] uuids, boolean recursive) throws RepositoryException {
         long tstart = System.currentTimeMillis();
-        int total = internalCheckConsistency(uuids, recursive, fix, reports, lostNFoundId);
-        long elapsed = System.currentTimeMillis() - tstart;
+        nodeCount = internalCheckConsistency(uuids, recursive);
+        elapsedTime = System.currentTimeMillis() - tstart;
+    }
+
+    /**
+     * Do a double check on the errors found during {@link #check}.
+     * Removes all false positives from the report.
+     */
+    public void doubleCheckErrors() {
+        if (!errors.isEmpty()) {
+            final Iterator<ConsistencyCheckerError> errorIterator = errors.iterator();
+            while (errorIterator.hasNext()) {
+                final ConsistencyCheckerError error = errorIterator.next();
+                try {
+                    if (!error.doubleCheck()) {
+                        info(null, "False positive: " + error);
+                        errorIterator.remove();
+                    }
+                } catch (ItemStateException e) {
+                    error(null, "Failed to double check error: " + error, e);
+                }
+            }
+        }
+    }
 
-        return new ConsistencyReportImpl(total, elapsed, reports);
+    /**
+     * Return the report of a consistency {@link #check} / {@link #doubleCheckErrors()} / {@link #repair}
+     */
+    public ConsistencyReport getReport() {
+        final Set<ReportItem> reportItems = new HashSet<ReportItem>();
+        for (ConsistencyCheckerError error : errors) {
+            reportItems.add(error.getReportItem());
+        }
+        return new ConsistencyReportImpl(nodeCount, elapsedTime, reportItems);
     }
 
-    private int internalCheckConsistency(String[] uuids, boolean recursive,
-            boolean fix, Set<ReportItem> reports, String lostNFoundId)
-            throws RepositoryException {
-        int count = 0;
+    /**
+     * Repair any errors found during a {@link #check}. Should be run after a {#check} and
+     * (if needed) {@link #doubleCheckErrors}.
+     *
+     * @throws RepositoryException
+     */
+    public void repair() throws RepositoryException {
+        checkLostNFound();
+        bundles = new HashMap<NodeId, NodePropBundle>();
+        if (hasRepairableErrors()) {
+            boolean successful = false;
+            final CheckerUpdate update = new CheckerUpdate();
+            try {
+                eventChannel.updateCreated(update);
+                for (ConsistencyCheckerError error : errors) {
+                    if (error.isRepairable()) {
+                        try {
+                            error.repair(update.getChanges());
+                            info(null, "Repairing " + error);
+                        } catch (ItemStateException e) {
+                            error(null, "Failed to repair error: " + error, e);
+                        }
+                    }
+                }
 
-        NodeId lostNFound = null;
-        if (fix) {
-            if (lostNFoundId != null) {
-                // do we have a "lost+found" node?
-                try {
-                    NodeId tmpid = new NodeId(lostNFoundId);
-                    NodePropBundle lfBundle = pm.loadBundle(tmpid);
-                    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:unstructured");
-                    } else {
-                        lostNFound = lfBundle.getId();
+                final ChangeLog changes = update.getChanges();
+                if (changes.hasUpdates()) {
+                    eventChannel.updatePrepared(update);
+                    for (NodePropBundle bundle : bundles.values()) {
+                        storeBundle(bundle);
                     }
-                } catch (Exception ex) {
-                    error(lostNFoundId, "finding 'lost+found' folder", ex);
+                    update.setAttribute(ATTRIBUTE_UPDATE_SIZE, changes.getUpdateSize());
+                    successful = true;
                 }
-            } else {
-                log.info("No 'lost+found' node specified: orphans cannot be fixed");
+            } catch (ClusterException e) {
+                throw new RepositoryException("Cannot create update", e);
+            } finally {
+                if (successful) {
+                    eventChannel.updateCommitted(update, "checker@");
+                } else {
+                    eventChannel.updateCancelled(update);
+                }
+            }
+        }
+    }
+
+    private boolean hasRepairableErrors() {
+        for (ConsistencyCheckerError error : errors) {
+            if (error.isRepairable()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private void checkLostNFound() {
+        if (lostNFoundId != null) {
+            // do we have a "lost+found" node?
+            try {
+                NodePropBundle lfBundle = pm.loadBundle(lostNFoundId);
+                if (lfBundle == null) {
+                    error(lostNFoundId.toString(), "Specified 'lost+found' node does not exist");
+                    lostNFoundId = null;
+                } else if (!NameConstants.NT_UNSTRUCTURED.equals(lfBundle .getNodeTypeName())) {
+                    error(lostNFoundId.toString(), "Specified 'lost+found' node is not of type nt:unstructured");
+                    lostNFoundId = null;
+                }
+            } catch (Exception ex) {
+                error(lostNFoundId.toString(), "finding 'lost+found' folder", ex);
+                lostNFoundId = null;
             }
+        } else {
+            info(null, "No 'lost+found' node specified: orphans cannot be fixed");
         }
+    }
+
+    private int internalCheckConsistency(String[] uuids, boolean recursive) throws RepositoryException {
+        int count = 0;
 
         if (uuids == null) {
             try {
@@ -128,7 +238,7 @@ public class ConsistencyCheckerImpl {
                         if (!CHECKAFTERLOADING) {
                             // check immediately
                             NodeInfo nodeInfo = entry.getValue();
-                            checkBundleConsistency(id, nodeInfo, fix, lostNFound, reports, batch);
+                            checkBundleConsistency(id, nodeInfo, batch);
                         }
                     }
 
@@ -142,7 +252,7 @@ public class ConsistencyCheckerImpl {
                 if (CHECKAFTERLOADING) {
                     // check info
                     for (Map.Entry<NodeId, NodeInfo> entry : allInfos.entrySet()) {
-                        checkBundleConsistency(entry.getKey(), entry.getValue(), fix, lostNFound, reports, allInfos);
+                        checkBundleConsistency(entry.getKey(), entry.getValue(), allInfos);
                     }
                 }
             } catch (ItemStateException ex) {
@@ -159,13 +269,11 @@ public class ConsistencyCheckerImpl {
 
             List<NodeId> idList = new ArrayList<NodeId>(uuids.length);
             // convert uuid string array to list of UUID objects
-            for (int i = 0; i < uuids.length; i++) {
+            for (final String uuid : uuids) {
                 try {
-                    idList.add(new NodeId(uuids[i]));
+                    idList.add(new NodeId(uuid));
                 } catch (IllegalArgumentException e) {
-                    error(uuids[i],
-                            "Invalid id for consistency check, skipping: '"
-                                    + uuids[i] + "': " + e);
+                    error(uuid, "Invalid id for consistency check, skipping: '" + uuid + "': " + e);
                 }
             }
 
@@ -179,16 +287,13 @@ public class ConsistencyCheckerImpl {
 
                     if (bundle == null) {
                         if (!isVirtualNode(id)) {
-                            error(id.toString(), "No bundle found for id '"
-                                    + id + "'");
+                            error(id.toString(), "No bundle found for id '" + id + "'");
                         }
                     } else {
-                        checkBundleConsistency(id, new NodeInfo(bundle), fix, lostNFound,
-                                reports, Collections.<NodeId, NodeInfo>emptyMap());
+                        checkBundleConsistency(id, new NodeInfo(bundle), Collections.<NodeId, NodeInfo>emptyMap());
 
                         if (recursive) {
-                            for (NodePropBundle.ChildNodeEntry entry : bundle
-                                    .getChildNodeEntries()) {
+                            for (NodePropBundle.ChildNodeEntry entry : bundle.getChildNodeEntries()) {
                                 idList.add(entry.getId());
                             }
                         }
@@ -220,14 +325,9 @@ public class ConsistencyCheckerImpl {
      *
      * @param id node id for the bundle to check
      * @param nodeInfo the node info for the node to check
-     * @param fix if <code>true</code>, repair things that can be repaired
-     * {@linkplain org.apache.jackrabbit.core.persistence.util.NodePropBundle bundles} here
      * @param infos all the {@link NodeInfo}s loaded in the current batch
      */
-    private void checkBundleConsistency(NodeId id, NodeInfo nodeInfo,
-                                        boolean fix, NodeId lostNFoundId,
-                                        Set<ReportItem> reports, Map<NodeId, NodeInfo> infos) {
-        // log.info(name + ": checking bundle '" + id + "'");
+    private void checkBundleConsistency(NodeId id, NodeInfo nodeInfo, Map<NodeId, NodeInfo> infos) {
 
         // skip all virtual nodes
         if (isVirtualNode(id)) {
@@ -238,217 +338,49 @@ public class ConsistencyCheckerImpl {
             listener.startCheck(id.toString());
         }
 
-        // look at the node's children
-        Collection<NodePropBundle.ChildNodeEntry> missingChildren = new ArrayList<NodePropBundle.ChildNodeEntry>();
-        Collection<NodePropBundle.ChildNodeEntry> disconnectedChildren = new ArrayList<NodePropBundle.ChildNodeEntry>();
-
-        NodePropBundle bundle = null;
-
+        // check the children
         for (final NodeId childNodeId : nodeInfo.getChildren()) {
 
-            // skip check for system nodes (root, system root, version storage,
-            // node types)
+            // skip check for system nodes (root, system root, version storage, node types)
             if (childNodeId.toString().endsWith("babecafebabe")) {
                 continue;
             }
 
-            try {
-                // analyze child node bundles
-                NodePropBundle childBundle = null;
-                NodeInfo childNodeInfo = infos.get(childNodeId);
-
-                // does the child exist?
-                if (childNodeInfo == null) {
-                    // try to load the bundle
-                    childBundle = pm.loadBundle(childNodeId);
-                    if (childBundle == null) {
-                        // the child indeed does not exist
-                        // double check whether we still exist and the child entry is still there
-                        if (bundle == null) {
-                            bundle = pm.loadBundle(id);
-                        }
-                        if (bundle != null) {
-                            NodePropBundle.ChildNodeEntry childNodeEntry = null;
-                            for (NodePropBundle.ChildNodeEntry entry : bundle.getChildNodeEntries()) {
-                                if (entry.getId().equals(childNodeId)) {
-                                    childNodeEntry = entry;
-                                    break;
-                                }
-                            }
-                            if (childNodeEntry != null) {
-                                String message = "NodeState '" + id + "' references inexistent child '" + childNodeId + "'";
-                                log.error(message);
-                                addMessage(reports, id, message, ReportItem.Type.MISSING);
-                                missingChildren.add(childNodeEntry);
-                            }
-                        } else {
-                            return;
-                        }
-                    } else {
-                        // exists after all
-                        childNodeInfo = new NodeInfo(childBundle);
-                    }
-                }
-                if (childNodeInfo != null) {
-                    // if the child exists does it reference the current node as its parent?
-                    NodeId cp = childNodeInfo.getParentId();
-                    if (!id.equals(cp)) {
-                        // double check whether the child still has a different parent
-                        if (childBundle == null) {
-                            childBundle = pm.loadBundle(childNodeId);
-                        }
-                        if (childBundle != null && !childBundle.getParentId().equals(id)) {
-                            // double check if we still exist
-                            if (bundle == null) {
-                                bundle = pm.loadBundle(id);
-                            }
-                            if (bundle != null) {
-                                // double check if the child node entry is still there
-                                NodePropBundle.ChildNodeEntry childNodeEntry = null;
-                                for (NodePropBundle.ChildNodeEntry entry : bundle.getChildNodeEntries()) {
-                                    if (entry.getId().equals(childNodeId)) {
-                                        childNodeEntry = entry;
-                                        break;
-                                    }
-                                }
-                                if (childNodeEntry != null) {
-                                    // indeed we have a disconnected child
-                                    String message = "Node has invalid parent id: '" + cp + "' (instead of '" + id + "')";
-                                    log.error(message);
-                                    addMessage(reports, childNodeId, message, ReportItem.Type.DISCONNECTED);
-                                    disconnectedChildren.add(childNodeEntry);
-                                }
-                            } else {
-                                return;
-                            }
+            NodeInfo childNodeInfo = infos.get(childNodeId);
 
-                        }
-                    }
+            if (childNodeInfo == null) {
+                addError(new MissingChild(id, childNodeId));
+            } else {
+                if (!id.equals(childNodeInfo.getParentId())) {
+                    addError(new DisconnectedChild(id, childNodeId, childNodeInfo.getParentId()));
                 }
-            } catch (ItemStateException e) {
-                // problem already logged (loadBundle called with
-                // logDetailedErrors=true)
-                addMessage(reports, id, e.getMessage(), ReportItem.Type.ERROR);
-            }
-        }
-        // remove child node entry (if fixing is enabled)
-        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);
         }
 
-        // check parent reference
+        // check the parent
         NodeId parentId = nodeInfo.getParentId();
-        try {
-            // skip root nodes (that point to itself)
-            if (parentId != null && !id.toString().endsWith("babecafebabe")) {
-                NodePropBundle parentBundle = null;
-                NodeInfo parentInfo = infos.get(parentId);
-
-                // does the parent exist?
-                if (parentInfo == null) {
-                    // try to load the bundle
-                    parentBundle = pm.loadBundle(parentId);
-                    if (parentBundle == null) {
-                        // indeed the parent doesn't exist
-                        // double check whether we still exist and the parent is still the same\
-                        if (bundle == null) {
-                            bundle = pm.loadBundle(id);
-                        }
-                        if (bundle != null) {
-                            if (parentId.equals(bundle.getParentId())) {
-                                // indeed we have an orphaned node
-                                String message = "NodeState '" + id + "' references inexistent parent id '" + parentId + "'";
-                                log.error(message);
-                                addMessage(reports, id, message, ReportItem.Type.ORPHANED);
-                                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);
-                                    fixBundle(bundle);
-                                }
-                            }
-                        } else {
-                            return;
-                        }
-                    } else {
-                        // parent exists after all
-                        parentInfo = new NodeInfo(parentBundle);
-                    }
-                }
-                if (parentInfo != null) {
-                    // if the parent exists, does it have a child node entry for us?
-                    boolean found = false;
-
-                    for (NodeId childNodeId : parentInfo.getChildren()) {
-                        if (childNodeId.equals(id)){
-                            found = true;
-                            break;
-                        }
-                    }
+        // skip root nodes (that point to itself)
+        if (parentId != null && !id.toString().endsWith("babecafebabe")) {
+            NodeInfo parentInfo = infos.get(parentId);
 
-                    if (!found && parentBundle == null) {
-                        // double check the parent
-                        parentBundle = pm.loadBundle(parentId);
-                        if (parentBundle != null) {
-                            for (NodePropBundle.ChildNodeEntry entry : parentBundle.getChildNodeEntries()) {
-                                if (entry.getId().equals(id)) {
-                                    found = true;
-                                    break;
-                                }
-                            }
-                        }
-                    }
-                    if (!found) {
-                        // double check whether we still exist and the parent id is still the same
-                        if (bundle == null) {
-                            bundle = pm.loadBundle(id);
-                        }
-                        if (bundle != null) {
-                            if (parentId.equals(bundle.getParentId())) {
-                                // indeed we have an abandoned node
-                                String message = "NodeState '" + id
-                                        + "' is not referenced by its parent node '"
-                                        + parentId + "'";
-                                log.error(message);
-                                addMessage(reports, id, message, ReportItem.Type.ABANDONED);
-                                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;
-                        }
+            if (parentInfo == null) {
+                addError(new OrphanedNode(id, parentId));
+            } else {
+                // if the parent exists, does it have a child node entry for us?
+                boolean found = false;
+
+                for (NodeId childNodeId : parentInfo.getChildren()) {
+                    if (childNodeId.equals(id)){
+                        found = true;
+                        break;
                     }
+                }
 
+                if (!found) {
+                    addError(new AbandonedNode(id, parentId));
                 }
+
             }
-        } catch (ItemStateException e) {
-            String message = "Error reading node '" + parentId
-                    + "' (parent of '" + id + "'): " + e;
-            log.error(message);
-            addMessage(reports, id, message, ReportItem.Type.ERROR);
         }
     }
 
@@ -464,19 +396,11 @@ public class ConsistencyCheckerImpl {
         return "cafebabe-cafe-babe-cafe-babecafebabe".equals(id);
     }
 
-
-    private void addMessage(Set<ReportItem> reports, NodeId id, String message, ReportItem.Type type) {
-
-        if (reports != null || listener != null) {
-            ReportItem ri = new ReportItemImpl(id.toString(), message, type);
-
-            if (reports != null) {
-                reports.add(ri);
-            }
-            if (listener != null) {
-                listener.report(ri);
-            }
+    private void addError(ConsistencyCheckerError error) {
+        if (listener != null) {
+            listener.report(error.getReportItem());
         }
+        errors.add(error);
     }
 
     private void info(String id, String message) {
@@ -505,9 +429,8 @@ public class ConsistencyCheckerImpl {
         }
     }
 
-    private void fixBundle(NodePropBundle bundle) {
+    private void storeBundle(NodePropBundle bundle) {
         try {
-            log.info(pm + ": Fixing bundle '" + bundle.getId() + "'");
             bundle.markOld(); // use UPDATE instead of INSERT
             pm.storeBundle(bundle);
             pm.evictBundle(bundle.getId());
@@ -516,4 +439,264 @@ public class ConsistencyCheckerImpl {
         }
     }
 
+    private NodePropBundle getBundle(NodeId nodeId) throws ItemStateException {
+        if (bundles.containsKey(nodeId)) {
+            return bundles.get(nodeId);
+        }
+        return pm.loadBundle(nodeId);
+    }
+
+    private void saveBundle(NodePropBundle bundle) {
+        bundles.put(bundle.getId(), bundle);
+    }
+
+    private class MissingChild extends ConsistencyCheckerError {
+
+        private final NodeId childNodeId;
+
+        private MissingChild(final NodeId nodeId, final NodeId childNodeId) {
+            super(nodeId, "NodeState '" + nodeId + "' references inexistent child '" + childNodeId + "'");
+            this.childNodeId = childNodeId;
+        }
+
+        @Override
+        ReportItem.Type getType() {
+            return ReportItem.Type.MISSING;
+        }
+
+        @Override
+        boolean isRepairable() {
+            return true;
+        }
+
+        @Override
+        void doRepair(final ChangeLog changes) throws ItemStateException {
+            final NodePropBundle bundle = getBundle(nodeId);
+            final Iterator<NodePropBundle.ChildNodeEntry> entryIterator = bundle.getChildNodeEntries().iterator();
+            while (entryIterator.hasNext()) {
+                final NodePropBundle.ChildNodeEntry childNodeEntry = entryIterator.next();
+                if (childNodeEntry.getId().equals(childNodeId)) {
+                    entryIterator.remove();
+                    saveBundle(bundle);
+                    changes.modified(new NodeState(nodeId, null, null, ItemState.STATUS_EXISTING, false));
+                }
+            }
+        }
+
+        @Override
+        boolean doubleCheck() throws ItemStateException {
+            final NodePropBundle childBundle = pm.loadBundle(childNodeId);
+            if (childBundle == null) {
+                final NodePropBundle bundle = pm.loadBundle(nodeId);
+                if (bundle != null) {
+                    for (NodePropBundle.ChildNodeEntry entry : bundle.getChildNodeEntries()) {
+                        if (entry.getId().equals(childNodeId)) {
+                            return true;
+                        }
+                    }
+                }
+            }
+            return false;
+        }
+    }
+
+    private class DisconnectedChild extends ConsistencyCheckerError {
+
+        private final NodeId childNodeId;
+
+        DisconnectedChild(final NodeId nodeId, final NodeId childNodeId, final NodeId invalidParentId) {
+            super(nodeId, "Node has invalid parent id: '" + invalidParentId + "' (instead of '" + nodeId + "')");
+            this.childNodeId = childNodeId;
+        }
+
+        @Override
+        ReportItem.Type getType() {
+            return ReportItem.Type.DISCONNECTED;
+        }
+
+        @Override
+        boolean isRepairable() {
+            return true;
+        }
+
+        @Override
+        void doRepair(final ChangeLog changes) throws ItemStateException {
+            NodePropBundle bundle = getBundle(nodeId);
+            final Iterator<NodePropBundle.ChildNodeEntry> entryIterator = bundle.getChildNodeEntries().iterator();
+            while (entryIterator.hasNext()) {
+                final NodePropBundle.ChildNodeEntry childNodeEntry = entryIterator.next();
+                if (childNodeEntry.getId().equals(childNodeId)) {
+                    entryIterator.remove();
+                    saveBundle(bundle);
+                    changes.modified(new NodeState(nodeId, null, null, ItemState.STATUS_EXISTING, false));
+                    break;
+                }
+            }
+        }
+
+        @Override
+        boolean doubleCheck() throws ItemStateException {
+            final NodePropBundle childBundle = pm.loadBundle(childNodeId);
+            if (childBundle != null && !childBundle.getParentId().equals(nodeId)) {
+                final NodePropBundle bundle = pm.loadBundle(nodeId);
+                if (bundle != null) {
+                    // double check if the child node entry is still there
+                    for (NodePropBundle.ChildNodeEntry entry : bundle.getChildNodeEntries()) {
+                        if (entry.getId().equals(childNodeId)) {
+                            return true;
+                        }
+                    }
+                }
+            }
+            return false;
+        }
+    }
+
+    private class OrphanedNode extends ConsistencyCheckerError {
+
+        private final NodeId parentNodeId;
+
+        OrphanedNode(final NodeId nodeId, final NodeId parentNodeId) {
+            super(nodeId, "NodeState '" + nodeId + "' references inexistent parent id '" + parentNodeId + "'");
+            this.parentNodeId = parentNodeId;
+        }
+
+        @Override
+        ReportItem.Type getType() {
+            return ReportItem.Type.ORPHANED;
+        }
+
+        @Override
+        boolean isRepairable() {
+            return lostNFoundId != null;
+        }
+
+        @Override
+        void doRepair(final ChangeLog changes) throws ItemStateException {
+            if (lostNFoundId != null) {
+                final NodePropBundle bundle = getBundle(nodeId);
+                final NodePropBundle lfBundle = getBundle(lostNFoundId);
+
+                final String nodeName = nodeId + "-" + System.currentTimeMillis();
+                lfBundle.addChildNodeEntry(NF.create("", nodeName), nodeId);
+                bundle.setParentId(lostNFoundId);
+
+                saveBundle(bundle);
+                saveBundle(lfBundle);
+
+                changes.modified(new NodeState(lostNFoundId, null, null, ItemState.STATUS_EXISTING, false));
+                changes.modified(new NodeState(nodeId, null, null, ItemState.STATUS_EXISTING, false));
+            }
+        }
+
+        @Override
+        boolean doubleCheck() throws ItemStateException {
+            final NodePropBundle parentBundle = pm.loadBundle(parentNodeId);
+            if (parentBundle == null) {
+                final NodePropBundle bundle = pm.loadBundle(nodeId);
+                if (bundle != null) {
+                    if (parentNodeId.equals(bundle.getParentId())) {
+                        return true;
+                    }
+                }
+            }
+            return false;
+        }
+    }
+
+    private class AbandonedNode extends ConsistencyCheckerError {
+
+        private final NodeId nodeId;
+        private final NodeId parentNodeId;
+
+        AbandonedNode(final NodeId nodeId, final NodeId parentNodeId) {
+            super(nodeId, "NodeState '" + nodeId + "' is not referenced by its parent node '" + parentNodeId + "'");
+            this.nodeId = nodeId;
+            this.parentNodeId = parentNodeId;
+        }
+
+        @Override
+        ReportItem.Type getType() {
+            return ReportItem.Type.ABANDONED;
+        }
+
+        @Override
+        boolean isRepairable() {
+            return true;
+        }
+
+        @Override
+        void doRepair(final ChangeLog changes) throws ItemStateException {
+            final NodePropBundle parentBundle = getBundle(parentNodeId);
+
+            final String nodeName = createNodeName();
+            parentBundle.addChildNodeEntry(NF.create("{}" + nodeName), nodeId);
+
+            saveBundle(parentBundle);
+            changes.modified(new NodeState(parentNodeId, null, null, ItemState.STATUS_EXISTING, false));
+        }
+
+        private String createNodeName() {
+            int l = (int) System.currentTimeMillis();
+            int r = new Random().nextInt();
+            int n = l + r;
+            return Integer.toHexString(n);
+        }
+
+        @Override
+        boolean doubleCheck() throws ItemStateException {
+            final NodePropBundle parentBundle = pm.loadBundle(parentNodeId);
+            if (parentBundle != null) {
+                for (NodePropBundle.ChildNodeEntry entry : parentBundle.getChildNodeEntries()) {
+                    if (entry.getId().equals(nodeId)) {
+                        return false;
+                    }
+                }
+            }
+            final NodePropBundle bundle = pm.loadBundle(nodeId);
+            if (bundle != null) {
+                if (parentNodeId.equals(bundle.getParentId())) {
+                    return true;
+                }
+            }
+            return false;
+        }
+    }
+
+    private class CheckerUpdate implements Update {
+
+        private final Map<String, Object> attributes = new HashMap<String, Object>();
+        private final ChangeLog changeLog = new ChangeLog();
+        private final long timestamp = System.currentTimeMillis();
+
+        @Override
+        public void setAttribute(final String name, final Object value) {
+            attributes.put(name, value);
+        }
+
+        @Override
+        public Object getAttribute(final String name) {
+            return attributes.get(name);
+        }
+
+        @Override
+        public ChangeLog getChanges() {
+            return changeLog;
+        }
+
+        @Override
+        public List<EventState> getEvents() {
+            return Collections.emptyList();
+        }
+
+        @Override
+        public long getTimestamp() {
+            return timestamp;
+        }
+
+        @Override
+        public String getUserData() {
+            return null;
+        }
+    }
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ConsistencyChecker.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ConsistencyChecker.java?rev=1448252&r1=1448251&r2=1448252&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ConsistencyChecker.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ConsistencyChecker.java Wed Feb 20 15:42:29 2013
@@ -18,17 +18,26 @@ package org.apache.jackrabbit.core.persi
 
 import javax.jcr.RepositoryException;
 
+import org.apache.jackrabbit.core.cluster.UpdateEventChannel;
+
 /**
  * Optional interface for Persistence Managers. Allows running consistency
- * checks similar to the base one (see @link
- * {@link PersistenceManager#checkConsistency(String[], boolean, boolean)} but
- * providing a result that can be acted upon.
+ * checks similar to the base one (see
+ * {@link org.apache.jackrabbit.core.persistence.bundle.AbstractBundlePersistenceManager#checkConsistency(String[], boolean, boolean)})
+ * but providing a result that can be acted upon.
  * <p>
  * <em>Beware: this interface is designed for unit tests only.</em>
  */
 public interface ConsistencyChecker {
 
     /**
+     * Set the update event channel. Needed to inform the cluster of any changes made during repairs.
+     *
+     * @param eventChannel the update event channel
+     */
+    public void setEventChannel(UpdateEventChannel eventChannel);
+
+    /**
      * Perform a consistency check of the data. An example are non-existent
      * nodes referenced in a child node entry. The existence of this feature and
      * the scope of the implementation can vary in different PersistenceManager

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItem.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItem.java?rev=1448252&r1=1448251&r2=1448252&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItem.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItem.java Wed Feb 20 15:42:29 2013
@@ -40,4 +40,8 @@ public interface ReportItem {
      */
     public Type getType();
 
+    /**
+     * @return whether this error was repaired
+     */
+    public boolean isRepaired();
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItemImpl.java?rev=1448252&r1=1448251&r2=1448252&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItemImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/check/ReportItemImpl.java Wed Feb 20 15:42:29 2013
@@ -21,11 +21,13 @@ public class ReportItemImpl implements R
     private final String nodeId;
     private final String message;
     private final Type type;
+    private final boolean repaired;
 
-    public ReportItemImpl(String nodeId, String message, Type type) {
+    public ReportItemImpl(String nodeId, String message, Type type, boolean repaired) {
         this.nodeId = nodeId;
         this.message = message;
         this.type = type;
+        this.repaired = repaired;
     }
 
     public String getNodeId() {
@@ -42,6 +44,11 @@ public class ReportItemImpl implements R
     }
 
     @Override
+    public boolean isRepaired() {
+        return repaired;
+    }
+
+    @Override
     public String toString() {
         return type + ": " + nodeId + " -- " + message;
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java?rev=1448252&r1=1448251&r2=1448252&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java Wed Feb 20 15:42:29 2013
@@ -24,7 +24,7 @@ import org.apache.jackrabbit.core.cluste
  * Package-private utility class used as a sentinel by the
  * {@link SharedItemStateManager} class.
  */
-class DummyUpdateEventChannel implements UpdateEventChannel {
+public class DummyUpdateEventChannel implements UpdateEventChannel {
 
     public void updatePrepared(Update update) {}
 

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=1448252&r1=1448251&r2=1448252&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 Wed Feb 20 15:42:29 2013
@@ -21,36 +21,64 @@ import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import javax.jcr.RepositoryException;
 
+import org.apache.jackrabbit.core.cluster.ClusterException;
+import org.apache.jackrabbit.core.cluster.ClusterNode;
+import org.apache.jackrabbit.core.cluster.SimpleClusterContext;
+import org.apache.jackrabbit.core.cluster.UpdateEventChannel;
+import org.apache.jackrabbit.core.cluster.UpdateEventListener;
+import org.apache.jackrabbit.core.config.ClusterConfig;
 import org.apache.jackrabbit.core.id.NodeId;
+import org.apache.jackrabbit.core.journal.Journal;
+import org.apache.jackrabbit.core.journal.JournalFactory;
+import org.apache.jackrabbit.core.journal.MemoryJournal;
+import org.apache.jackrabbit.core.journal.MemoryJournal.MemoryRecord;
+import org.apache.jackrabbit.core.observation.EventState;
 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.check.ReportItem;
 import org.apache.jackrabbit.core.persistence.util.BLOBStore;
-import org.apache.jackrabbit.core.persistence.util.NodeInfo;
 import org.apache.jackrabbit.core.persistence.util.NodePropBundle;
+import org.apache.jackrabbit.core.state.ChangeLog;
 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 org.apache.jackrabbit.spi.commons.namespace.NamespaceResolver;
 
 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();
 
+    /** Default sync delay: 5 seconds. */
+    private static final long SYNC_DELAY = 5000;
+
+    private List<MemoryRecord> records = new ArrayList<MemoryRecord>();
+
+    private ClusterNode master;
+    private ClusterNode slave;
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        master = createClusterNode("master");
+        master.start();
+
+        slave = createClusterNode("slave");
+        slave.start();
+    }
+
+
     // 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 {
+    public void testFixAbandonedNode() throws RepositoryException, ClusterException {
         NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
         NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
 
@@ -59,15 +87,64 @@ public class ConsistencyCheckerImplTest 
         bundle2.setParentId(bundle1.getId());
 
         MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle1, bundle2));
-        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null, null, master.createUpdateChannel("default"));
+
+        // set up cluster event update listener
+        final TestUpdateEventListener listener = new TestUpdateEventListener();
+        final UpdateEventChannel slaveEventChannel = slave.createUpdateChannel("default");
+        slaveEventChannel.setListener(listener);
+
+        checker.check(null, false);
+
+        Set<ReportItem> reportItems = checker.getReport().getItems();
+        assertEquals(1, reportItems.size());
+        ReportItem reportItem = reportItems.iterator().next();
+        assertEquals(ReportItem.Type.ABANDONED, reportItem.getType());
+        assertEquals(bundle2.getId().toString(), reportItem.getNodeId());
 
-        // run checker with fix = true
-        checker.check(null, false, true, null);
+        checker.repair();
 
         // 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());
+
+        slave.sync();
+
+        // verify events were correctly broadcast to cluster
+        assertNotNull("Cluster node did not receive update event", listener.changes);
+        assertTrue("Expected node1 to be modified", listener.changes.isModified(bundle1.getId()));
+    }
+
+    public void testDoubleCheckAbandonedNode() 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, null, null);
+
+        checker.check(null, false);
+
+        Set<ReportItem> reportItems = checker.getReport().getItems();
+        assertEquals(1, reportItems.size());
+        ReportItem reportItem = reportItems.iterator().next();
+        assertEquals(ReportItem.Type.ABANDONED, reportItem.getType());
+        assertEquals(bundle2.getId().toString(), reportItem.getNodeId());
+
+        checker.doubleCheckErrors();
+
+        assertFalse("Double check removed valid error", checker.getReport().getItems().isEmpty());
+
+        // fix the error
+        bundle1.addChildNodeEntry(nameFactory.create("", "test"), bundle2.getId());
+
+        checker.doubleCheckErrors();
+
+        assertTrue("Double check didn't remove invalid error", checker.getReport().getItems().isEmpty());
     }
 
     /*
@@ -79,17 +156,16 @@ public class ConsistencyCheckerImplTest 
         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);
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null, null, null);
 
-        // run checker with fix = true
-        checker.check(null, false, true, null);
+        checker.check(null, false);
+        checker.repair();
 
         // node1 should now have child node entries for node2 and node3
         bundle1 = pm.loadBundle(bundle1.getId());
@@ -99,34 +175,84 @@ public class ConsistencyCheckerImplTest 
     }
 
     // Orphaned nodes are those nodes who's parent does not exist
-    public void testAddOrphanedNodeToLostAndFound() throws RepositoryException {
-        NodePropBundle lostAndFound = new NodePropBundle(new NodeId(0, 0));
+    public void testAddOrphanedNodeToLostAndFound() throws RepositoryException, ClusterException {
+        final NodeId lostAndFoundId = new NodeId(0, 0);
+        NodePropBundle lostAndFound = new NodePropBundle(lostAndFoundId);
         // lost and found must be of type nt:unstructured
         lostAndFound.setNodeTypeName(NameConstants.NT_UNSTRUCTURED);
 
-        NodePropBundle orphaned = new NodePropBundle(new NodeId(0, 1));
+        final NodeId orphanedId = new NodeId(0, 1);
+        NodePropBundle orphaned = new NodePropBundle(orphanedId);
         // 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);
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null, lostAndFoundId.toString(),
+                master.createUpdateChannel("default"));
 
-        // run checker with fix = true
-        checker.check(null, false, true, lostAndFound.getId().toString());
+        // set up cluster event update listener
+        final TestUpdateEventListener listener = new TestUpdateEventListener();
+        final UpdateEventChannel slaveEventChannel = slave.createUpdateChannel("default");
+        slaveEventChannel.setListener(listener);
+
+        checker.check(null, false);
+
+        Set<ReportItem> reportItems = checker.getReport().getItems();
+        assertEquals(1, reportItems.size());
+        ReportItem reportItem = reportItems.iterator().next();
+        assertEquals(ReportItem.Type.ORPHANED, reportItem.getType());
+        assertEquals(orphanedId.toString(), reportItem.getNodeId());
+
+        checker.repair();
 
         // orphan should have been added to lost+found
-        lostAndFound = pm.loadBundle(lostAndFound.getId());
+        lostAndFound = pm.loadBundle(lostAndFoundId);
         assertEquals(1, lostAndFound.getChildNodeEntries().size());
-        assertEquals(orphaned.getId(), lostAndFound.getChildNodeEntries().get(0).getId());
+        assertEquals(orphanedId, lostAndFound.getChildNodeEntries().get(0).getId());
+
+        orphaned = pm.loadBundle(orphanedId);
+        assertEquals(lostAndFoundId, orphaned.getParentId());
+
+        slave.sync();
+
+        // verify events were correctly broadcast to cluster
+        assertNotNull("Cluster node did not receive update event", listener.changes);
+        assertTrue("Expected lostAndFound to be modified", listener.changes.isModified(lostAndFoundId));
+        assertTrue("Expected orphan to be modified", listener.changes.isModified(orphanedId));
+    }
+
+    public void testDoubleCheckOrphanedNode() throws RepositoryException {
+        NodePropBundle orphaned = new NodePropBundle(new NodeId(0, 1));
+        orphaned.setParentId(new NodeId(1, 0));
+
+        MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(orphaned));
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null, null, null);
+
+        checker.check(null, false);
+
+        Set<ReportItem> reportItems = checker.getReport().getItems();
+        assertEquals(1, reportItems.size());
+        ReportItem reportItem = reportItems.iterator().next();
+        assertEquals(ReportItem.Type.ORPHANED, reportItem.getType());
+        assertEquals(orphaned.getId().toString(), reportItem.getNodeId());
+
+        checker.doubleCheckErrors();
 
-        orphaned = pm.loadBundle(orphaned.getId());
-        assertEquals(lostAndFound.getId(), orphaned.getParentId());
+        assertFalse("Double check removed valid error", checker.getReport().getItems().isEmpty());
+
+        // fix the error
+        NodePropBundle parent = new NodePropBundle(orphaned.getParentId());
+        pm.bundles.put(parent.getId(), parent);
+
+        checker.doubleCheckErrors();
+
+        assertTrue("Double check didn't remove invalid error", checker.getReport().getItems().isEmpty());
     }
 
     // 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 {
+    public void testFixDisconnectedNode() throws RepositoryException, ClusterException {
         NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
         NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
         NodePropBundle bundle3 = new NodePropBundle(new NodeId(1, 0));
@@ -139,10 +265,22 @@ public class ConsistencyCheckerImplTest 
         bundle3.setParentId(bundle2.getId());
 
         MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle1, bundle2, bundle3));
-        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null, null, master.createUpdateChannel("default"));
 
-        // run checker with fix = true
-        checker.check(null, false, true, null);
+        // set up cluster event update listener
+        final TestUpdateEventListener listener = new TestUpdateEventListener();
+        final UpdateEventChannel slaveEventChannel = slave.createUpdateChannel("default");
+        slaveEventChannel.setListener(listener);
+
+        checker.check(null, false);
+
+        Set<ReportItem> reportItems = checker.getReport().getItems();
+        assertEquals(1, reportItems.size());
+        ReportItem reportItem = reportItems.iterator().next();
+        assertEquals(ReportItem.Type.DISCONNECTED, reportItem.getType());
+        assertEquals(bundle1.getId().toString(), reportItem.getNodeId());
+
+        checker.repair();
 
         bundle1 = pm.loadBundle(bundle1.getId());
         bundle2 = pm.loadBundle(bundle2.getId());
@@ -154,50 +292,137 @@ public class ConsistencyCheckerImplTest 
         // node3 should still be a child of node2
         assertEquals(1, bundle2.getChildNodeEntries().size());
         assertEquals(bundle2.getId(), bundle3.getParentId());
+
+        slave.sync();
+
+        // verify events were correctly broadcast to cluster
+        assertNotNull("Cluster node did not receive update event", listener.changes);
+        assertTrue("Expected node1 to be modified", listener.changes.isModified(bundle1.getId()));
     }
 
-    // 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} */
+    public void testDoubleCheckDisonnectedNode() throws RepositoryException {
         NodePropBundle bundle1 = new NodePropBundle(new NodeId(0, 0));
         NodePropBundle bundle2 = new NodePropBundle(new NodeId(0, 1));
-        bundle2.setParentId(bundle1.getId());
+        NodePropBundle bundle3 = new NodePropBundle(new NodeId(1, 0));
 
-        /** 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));
+        // 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());
 
-        /** 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));
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null, null, null);
 
-        MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle1, bundle2, bundle3, bundle4, bundle5, lostAndFound, orphaned));
-        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null);
+        checker.check(null, false);
 
-        // run checker with fix = false
-        checker.check(null, false, false, null);
+        Set<ReportItem> reportItems = checker.getReport().getItems();
+        assertEquals(1, reportItems.size());
+        ReportItem reportItem = reportItems.iterator().next();
+        assertEquals(ReportItem.Type.DISCONNECTED, reportItem.getType());
+        assertEquals(bundle1.getId().toString(), reportItem.getNodeId());
 
-        bundle1 = pm.loadBundle(bundle1.getId());
-        lostAndFound = pm.loadBundle(lostAndFound.getId());
-        bundle3 = pm.loadBundle(bundle3.getId());
+        checker.doubleCheckErrors();
 
-        // abandoned node should not have been un-abandoned
-        assertEquals(0, bundle1.getChildNodeEntries().size());
+        assertFalse("Double check removed valid error", checker.getReport().getItems().isEmpty());
+
+        // fix the error
+        bundle1.getChildNodeEntries().remove(0);
+
+        checker.doubleCheckErrors();
+
+        assertTrue("Double check didn't remove invalid error", checker.getReport().getItems().isEmpty());
+    }
+
+    public void testFixMissingNode() throws RepositoryException, ClusterException {
+        NodePropBundle bundle = new NodePropBundle(new NodeId(0, 0));
+        bundle.addChildNodeEntry(nameFactory.create("", "test"), new NodeId(0, 1));
+
+        MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle));
+
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null, null, master.createUpdateChannel("default"));
+
+        // set up cluster event update listener
+        final TestUpdateEventListener listener = new TestUpdateEventListener();
+        final UpdateEventChannel slaveEventChannel = slave.createUpdateChannel("default");
+        slaveEventChannel.setListener(listener);
+
+        checker.check(null, false);
+
+        Set<ReportItem> reportItems = checker.getReport().getItems();
+        assertEquals(1, reportItems.size());
+        ReportItem reportItem = reportItems.iterator().next();
+        assertEquals(ReportItem.Type.MISSING, reportItem.getType());
+        assertEquals(bundle.getId().toString(), reportItem.getNodeId());
+
+        checker.repair();
 
-        // orphan should not have been added to lost and found
-        assertEquals(0, lostAndFound.getChildNodeEntries().size());
+        // node should have no child no entries
+        assertTrue(bundle.getChildNodeEntries().isEmpty());
 
-        // disconnected entries should not have been removed
-        assertEquals(1, bundle3.getChildNodeEntries().size());
+        slave.sync();
 
+        // verify events were correctly broadcast to cluster
+        assertNotNull("Cluster node did not receive update event", listener.changes);
+        assertTrue("Expected node to be modified", listener.changes.isModified(bundle.getId()));
     }
-    
+
+    public void testDoubleCheckMissingNode() throws RepositoryException {
+        NodePropBundle bundle = new NodePropBundle(new NodeId(0, 0));
+        final NodeId childNodeId = new NodeId(0, 1);
+        bundle.addChildNodeEntry(nameFactory.create("", "test"), childNodeId);
+
+        MockPersistenceManager pm = new MockPersistenceManager(Arrays.asList(bundle));
+
+        ConsistencyCheckerImpl checker = new ConsistencyCheckerImpl(pm, null, null, null);
+
+        checker.check(null, false);
+
+        Set<ReportItem> reportItems = checker.getReport().getItems();
+        assertEquals(1, reportItems.size());
+        ReportItem reportItem = reportItems.iterator().next();
+        assertEquals(ReportItem.Type.MISSING, reportItem.getType());
+        assertEquals(bundle.getId().toString(), reportItem.getNodeId());
+
+        checker.doubleCheckErrors();
+
+        assertFalse("Double check removed valid error", checker.getReport().getItems().isEmpty());
+
+        // fix the error
+        NodePropBundle child = new NodePropBundle(childNodeId);
+        pm.bundles.put(childNodeId, child);
+
+        checker.doubleCheckErrors();
+
+        assertTrue("Double check didn't remove invalid error", checker.getReport().getItems().isEmpty());
+
+    }
+
+    private ClusterNode createClusterNode(String id) throws Exception {
+        final MemoryJournal journal = new MemoryJournal() {
+            protected boolean syncAgainOnNewRecords() {
+                return true;
+            }
+        };
+        JournalFactory jf = new JournalFactory() {
+            public Journal getJournal(NamespaceResolver resolver)
+                    throws RepositoryException {
+                return journal;
+            }
+        };
+        ClusterConfig cc = new ClusterConfig(id, SYNC_DELAY, jf);
+        SimpleClusterContext context = new SimpleClusterContext(cc);
+
+        journal.setRepositoryHome(context.getRepositoryHome());
+        journal.init(id, context.getNamespaceResolver());
+        journal.setRecords(records);
+
+        ClusterNode clusterNode = new ClusterNode();
+        clusterNode.init(context);
+        return clusterNode;
+    }
+
     private static class MockPersistenceManager extends AbstractBundlePersistenceManager {
 
         private Map<NodeId, NodePropBundle> bundles = new LinkedHashMap<NodeId, NodePropBundle>();
@@ -262,4 +487,14 @@ public class ConsistencyCheckerImplTest 
             return false;
         }
     }
+
+    private static class TestUpdateEventListener implements UpdateEventListener {
+
+        private ChangeLog changes;
+
+        @Override
+        public void externalUpdate(final ChangeLog changes, final List<EventState> events, final long timestamp, final String userData) throws RepositoryException {
+            this.changes = changes;
+        }
+    }
 }
\ No newline at end of file



Mime
View raw message