You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by to...@apache.org on 2017/07/25 20:52:26 UTC

svn commit: r1803002 - in /jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite: CommitHookEnhancer.java CompositeNodeState.java CompositeNodeStore.java MountedNodeStore.java

Author: tomekr
Date: Tue Jul 25 20:52:25 2017
New Revision: 1803002

URL: http://svn.apache.org/viewvc?rev=1803002&view=rev
Log:
OAK-6486: NPE in CompositeNodeStore

-implemented MountedNodeStore#toString()
-treat the NodeState returned from the commit hook as a composite one in the CommitHookEnhancer
-throw an exception if the commit hooks were not invoked by the global store
-return the root node state as a partial state in the merge() method for read-only stores
-run the commit hooks and rebase the partial builders even if there are no changes on the global builder

Modified:
    jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CommitHookEnhancer.java
    jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeState.java
    jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java
    jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java

Modified: jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CommitHookEnhancer.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CommitHookEnhancer.java?rev=1803002&r1=1803001&r2=1803002&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CommitHookEnhancer.java (original)
+++ jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CommitHookEnhancer.java Tue Jul 25 20:52:25 2017
@@ -50,19 +50,24 @@ class CommitHookEnhancer implements Comm
     public NodeState processCommit(NodeState before, NodeState after, CommitInfo info) throws CommitFailedException {
         Map<MountedNodeStore, NodeState> beforeStates = newHashMap();
         Map<MountedNodeStore, NodeState> afterStates = newHashMap();
-        for (Map.Entry<MountedNodeStore, NodeBuilder> e : builders.entrySet()) {
-            afterStates.put(e.getKey(), e.getKey().getNodeStore().rebase(e.getValue()));
-            beforeStates.put(e.getKey(), e.getValue().getBaseState());
+        for (MountedNodeStore mns : ctx.getNonDefaultStores()) {
+            afterStates.put(mns, mns.getNodeStore().rebase(builders.get(mns)));
+            beforeStates.put(mns, builders.get(mns).getBaseState());
         }
-        beforeStates.put(ctx.getGlobalStore(), before);
         afterStates.put(ctx.getGlobalStore(), after);
+        beforeStates.put(ctx.getGlobalStore(), before);
 
         CompositeNodeState compositeBefore = ctx.createRootNodeState(beforeStates);
         CompositeNodeState compositeAfter = ctx.createRootNodeState(afterStates);
 
         NodeState result = hook.processCommit(compositeBefore, compositeAfter, info);
         updatedBuilder = Optional.of(toComposite(result, compositeBefore));
-        return updatedBuilder.get().getNodeState().getNodeState(ctx.getGlobalStore());
+
+        if (result instanceof CompositeNodeState) {
+            return ((CompositeNodeState) result).getNodeState(ctx.getGlobalStore());
+        } else {
+            throw new IllegalStateException("The commit hook result should be a composite node state");
+        }
     }
 
     Optional<CompositeNodeBuilder> getUpdatedBuilder() {

Modified: jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeState.java?rev=1803002&r1=1803001&r2=1803002&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeState.java Tue Jul 25 20:52:25 2017
@@ -19,9 +19,6 @@
 package org.apache.jackrabbit.oak.composite;
 
 import com.google.common.base.Function;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryChildNodeEntry;
@@ -33,7 +30,6 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.annotation.Nullable;
 import java.util.List;
 import java.util.Map;
 
@@ -44,7 +40,6 @@ import static com.google.common.collect.
 import static com.google.common.collect.Iterables.transform;
 import static com.google.common.collect.Maps.asMap;
 import static com.google.common.collect.Maps.transformValues;
-import static com.google.common.collect.Sets.newHashSet;
 import static java.lang.Long.MAX_VALUE;
 import static java.util.Collections.singleton;
 import static org.apache.jackrabbit.oak.composite.CompositeNodeBuilder.simpleConcat;
@@ -92,9 +87,8 @@ class CompositeNodeState extends Abstrac
         }
 
         // this shouldn't happen, so we need to log some more debug info
-        String mountName = mns.getMount().isDefault() ? "[default]" : mns.getMount().getName();
-        LOG.warn("Can't find node state for path {} and mount {}. The node state map: {}", path, mountName, nodeStates);
-        throw new IllegalStateException("Can't find the node state for mount " + mountName);
+        LOG.warn("Can't find node state for path {} and mount {}. The node state map: {}", path, mns, nodeStates);
+        throw new IllegalStateException("Can't find the node state for mount " + mns);
     }
 
     @Override

Modified: jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java?rev=1803002&r1=1803001&r2=1803002&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java Tue Jul 25 20:52:25 2017
@@ -144,22 +144,25 @@ public class CompositeNodeStore implemen
             CommitHookEnhancer hookEnhancer = new CommitHookEnhancer(commitHook, ctx, nodeBuilder.getBuilders());
             NodeState globalResult = globalStore.getNodeStore().merge(nodeBuilder.getBuilders().get(globalStore), hookEnhancer, info);
             resultStates.put(globalStore, globalResult);
-            CompositeNodeBuilder updatedBuilder = hookEnhancer.getUpdatedBuilder().orElse(nodeBuilder);
+
+            if (!hookEnhancer.getUpdatedBuilder().isPresent()) {
+                // it means that the commit hook wasn't invoked, because there were
+                // no changes on the global store. we should invoke it anyway.
+                hookEnhancer.processCommit(globalResult, globalResult, info);
+            }
+            CompositeNodeBuilder updatedBuilder = hookEnhancer.getUpdatedBuilder().get();
 
             // merge the partial builders
             for (MountedNodeStore mns : ctx.getNonDefaultStores()) {
                 NodeBuilder partialBuilder = updatedBuilder.getBuilders().get(mns);
 
                 if (mns.getMount().isReadOnly()) {
-                    if (!partialBuilder.getNodeState().equals(partialBuilder.getBaseState())) {
-                        throw new CommitFailedException("CompositeStore", 31, "Unable to perform changes on read-only mount " + mns.getMount().getName());
-                    }
-                    resultStates.put(mns, partialBuilder.getBaseState());
-                    continue;
+                    assertNoChange(mns, partialBuilder);
+                    resultStates.put(mns, mns.getNodeStore().getRoot());
+                } else {
+                    NodeState partialState = mns.getNodeStore().merge(partialBuilder, EmptyHook.INSTANCE, info);
+                    resultStates.put(mns, partialState);
                 }
-
-                NodeState partialState = mns.getNodeStore().merge(partialBuilder, EmptyHook.INSTANCE, info);
-                resultStates.put(mns, partialState);
             }
 
             CompositeNodeState newRoot = ctx.createRootNodeState(resultStates);
@@ -178,18 +181,22 @@ public class CompositeNodeStore implemen
                 continue;
             }
             NodeBuilder partialBuilder = nodeBuilder.getBuilders().get(mountedNodeStore);
-            NodeState baseState = partialBuilder.getBaseState();
-            NodeState nodeState = partialBuilder.getNodeState();
-            if (!nodeState.equals(baseState)) {
-                Set<String> changedPaths = getModifiedPaths(baseState, nodeState);
-                Set<String> ignoredChangedPaths = getIgnoredPaths(changedPaths);
-                if (!ignoredChangedPaths.isEmpty()) {
-                    LOG.debug("Can't merge following read-only paths (they are configured to be ignored): {}.", ignoredChangedPaths);
-                }
-                Set<String> failingChangedPaths = difference(changedPaths, ignoredChangedPaths);
-                if (!failingChangedPaths.isEmpty()) {
-                    throw new CommitFailedException("CompositeStore", 31, "Unable to perform changes on read-only mount " + mountedNodeStore.getMount().getName() + ". Failing paths: " + failingChangedPaths.toString());
-                }
+            assertNoChange(mountedNodeStore, partialBuilder);
+        }
+    }
+
+    private void assertNoChange(MountedNodeStore mountedNodeStore, NodeBuilder partialBuilder) throws CommitFailedException {
+        NodeState baseState = partialBuilder.getBaseState();
+        NodeState nodeState = partialBuilder.getNodeState();
+        if (!nodeState.equals(baseState)) {
+            Set<String> changedPaths = getModifiedPaths(baseState, nodeState);
+            Set<String> ignoredChangedPaths = getIgnoredPaths(changedPaths);
+            if (!ignoredChangedPaths.isEmpty()) {
+                LOG.debug("Can't merge following read-only paths (they are configured to be ignored): {}.", ignoredChangedPaths);
+            }
+            Set<String> failingChangedPaths = difference(changedPaths, ignoredChangedPaths);
+            if (!failingChangedPaths.isEmpty()) {
+                throw new CommitFailedException("CompositeStore", 31, "Unable to perform changes on read-only mount " + mountedNodeStore.getMount().getName() + ". Failing paths: " + failingChangedPaths.toString());
             }
         }
     }

Modified: jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java?rev=1803002&r1=1803001&r2=1803002&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java Tue Jul 25 20:52:25 2017
@@ -39,4 +39,17 @@ class MountedNodeStore {
     public NodeStore getNodeStore() {
         return nodeStore;
     }
+
+    @Override
+    public String toString() {
+        StringBuilder result = new StringBuilder(super.toString());
+        result.append('[');
+        if (mount.isDefault()) {
+            result.append("default");
+        } else {
+            result.append(mount.getName());
+        }
+        result.append(']');
+        return result.toString();
+    }
 }
\ No newline at end of file