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 12:58:09 UTC

svn commit: r1802932 - in /jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite: CommitHookEnhancer.java CompositeNodeState.java CompositionContext.java

Author: tomekr
Date: Tue Jul 25 12:58:09 2017
New Revision: 1802932

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

Log diagnostic info if there's no node state for given mounted node store.

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/CompositionContext.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=1802932&r1=1802931&r2=1802932&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 12:58:09 2017
@@ -62,7 +62,7 @@ class CommitHookEnhancer implements Comm
 
         NodeState result = hook.processCommit(compositeBefore, compositeAfter, info);
         updatedBuilder = Optional.of(toComposite(result, compositeBefore));
-        return updatedBuilder.get().getNodeState().getNodeStates().get(ctx.getGlobalStore());
+        return updatedBuilder.get().getNodeState().getNodeState(ctx.getGlobalStore());
     }
 
     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=1802932&r1=1802931&r2=1802932&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 12:58:09 2017
@@ -19,6 +19,9 @@
 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;
@@ -27,7 +30,10 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import javax.annotation.Nullable;
 import java.util.List;
 import java.util.Map;
 
@@ -36,7 +42,9 @@ import static com.google.common.base.Pre
 import static com.google.common.collect.Iterables.concat;
 import static com.google.common.collect.Iterables.filter;
 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;
@@ -44,6 +52,8 @@ import static org.apache.jackrabbit.oak.
 
 class CompositeNodeState extends AbstractNodeState {
 
+    private static final Logger LOG = LoggerFactory.getLogger(CompositeNodeState.class);
+
     // A note on content held by node stores which is outside the mount boundaries
     //
     // As a matter of design, mounted stores will definitely hold information _above_
@@ -75,8 +85,16 @@ class CompositeNodeState extends Abstrac
         this.owningStore = ctx.getOwningStore(path);
     }
 
-    Map<MountedNodeStore, NodeState> getNodeStates() {
-        return nodeStates;
+    NodeState getNodeState(MountedNodeStore mns) {
+        NodeState nodeState = nodeStates.get(mns);
+        if (nodeState != null) {
+            return nodeState;
+        }
+
+        // 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);
     }
 
     @Override
@@ -110,16 +128,16 @@ class CompositeNodeState extends Abstrac
     public boolean hasChildNode(String name) {
         String childPath = simpleConcat(path, name);
         MountedNodeStore mountedStore = ctx.getOwningStore(childPath);
-        return nodeStates.get(mountedStore).hasChildNode(name);
+        return getNodeState(mountedStore).hasChildNode(name);
     }
 
     @Override
     public NodeState getChildNode(final String name) {
         String childPath = simpleConcat(path, name);
         if (!ctx.shouldBeComposite(childPath)) {
-            return nodeStates.get(ctx.getOwningStore(childPath)).getChildNode(name);
+            return getNodeState(ctx.getOwningStore(childPath)).getChildNode(name);
         }
-        Map<MountedNodeStore, NodeState> newNodeStates = transformValues(nodeStates, new Function<NodeState, NodeState>() {
+        Map<MountedNodeStore, NodeState> newNodeStates = transformValues(safeGetMap(), new Function<NodeState, NodeState>() {
             @Override
             public NodeState apply(NodeState input) {
                 return input.getChildNode(name);
@@ -130,7 +148,7 @@ class CompositeNodeState extends Abstrac
 
     @Override
     public long getChildNodeCount(final long max) {
-        List<MountedNodeStore> contributingStores = ctx.getContributingStoresForNodes(path, nodeStates);
+        List<MountedNodeStore> contributingStores = ctx.getContributingStoresForNodes(path, safeGetMap());
         if (contributingStores.isEmpty()) {
             return 0; // this shouldn't happen
         } else if (contributingStores.size() == 1) {
@@ -140,7 +158,7 @@ class CompositeNodeState extends Abstrac
             return accumulateChildSizes(concat(transform(contributingStores, new Function<MountedNodeStore, Iterable<String>>() {
                 @Override
                 public Iterable<String> apply(MountedNodeStore mns) {
-                    NodeState contributing = nodeStates.get(mns);
+                    NodeState contributing = getNodeState(mns);
                     if (contributing.getChildNodeCount(max) == MAX_VALUE) {
                         return singleton(STOP_COUNTING_CHILDREN);
                     } else {
@@ -164,10 +182,10 @@ class CompositeNodeState extends Abstrac
 
     @Override
     public Iterable<? extends ChildNodeEntry> getChildNodeEntries() {
-        Iterable<? extends ChildNodeEntry> nativeChildren = concat(transform(ctx.getContributingStoresForNodes(path, nodeStates), new Function<MountedNodeStore, Iterable<? extends ChildNodeEntry>>() {
+        Iterable<? extends ChildNodeEntry> nativeChildren = concat(transform(ctx.getContributingStoresForNodes(path, safeGetMap()), new Function<MountedNodeStore, Iterable<? extends ChildNodeEntry>>() {
             @Override
             public Iterable<? extends ChildNodeEntry> apply(final MountedNodeStore mountedNodeStore) {
-                return filter(nodeStates.get(mountedNodeStore).getChildNodeEntries(), compose(ctx.belongsToStore(mountedNodeStore, path), GET_NAME));
+                return filter(getNodeState(mountedNodeStore).getChildNodeEntries(), compose(ctx.belongsToStore(mountedNodeStore, path), GET_NAME));
             }
         }));
         return transform(nativeChildren, new Function<ChildNodeEntry, ChildNodeEntry>() {
@@ -185,13 +203,13 @@ class CompositeNodeState extends Abstrac
             CompositeNodeState multiBase = (CompositeNodeState) base;
             NodeStateDiff wrappingDiff = new WrappingDiff(diff, multiBase);
             boolean full = getWrappedNodeState().compareAgainstBaseState(multiBase.getWrappedNodeState(), new ChildrenDiffFilter(wrappingDiff, owningStore, true));
-            for (MountedNodeStore mns : ctx.getContributingStoresForNodes(path, nodeStates)) {
+            for (MountedNodeStore mns : ctx.getContributingStoresForNodes(path, safeGetMap())) {
                 if (owningStore == mns) {
                     continue;
                 }
                 NodeStateDiff childrenDiffFilter = new ChildrenDiffFilter(wrappingDiff, mns, false);
-                NodeState contributing = nodeStates.get(mns);
-                NodeState contributingBase = multiBase.nodeStates.get(mns);
+                NodeState contributing = getNodeState(mns);
+                NodeState contributingBase = multiBase.getNodeState(mns);
                 full = full && contributing.compareAgainstBaseState(contributingBase, childrenDiffFilter);
             }
             return full;
@@ -203,7 +221,7 @@ class CompositeNodeState extends Abstrac
     // write operations
     @Override
     public CompositeNodeBuilder builder() {
-        Map<MountedNodeStore, NodeBuilder> nodeBuilders = transformValues(nodeStates, new Function<NodeState, NodeBuilder>() {
+        Map<MountedNodeStore, NodeBuilder> nodeBuilders = transformValues(safeGetMap(), new Function<NodeState, NodeBuilder>() {
             @Override
             public NodeBuilder apply(NodeState input) {
                 return input.builder();
@@ -213,7 +231,11 @@ class CompositeNodeState extends Abstrac
     }
 
     private NodeState getWrappedNodeState() {
-        return nodeStates.get(owningStore);
+        return getNodeState(owningStore);
+    }
+
+    private Map<MountedNodeStore, NodeState> safeGetMap() {
+        return asMap(ctx.getAllMountedNodeStores(), this::getNodeState);
     }
 
     private class ChildrenDiffFilter implements NodeStateDiff {

Modified: jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositionContext.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositionContext.java?rev=1802932&r1=1802931&r2=1802932&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositionContext.java (original)
+++ jackrabbit/oak/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositionContext.java Tue Jul 25 12:58:09 2017
@@ -18,6 +18,8 @@ package org.apache.jackrabbit.oak.compos
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import org.apache.jackrabbit.oak.api.Blob;
 import org.apache.jackrabbit.oak.commons.PathUtils;
@@ -32,6 +34,7 @@ import java.io.InputStream;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static com.google.common.collect.ImmutableMap.copyOf;
 import static com.google.common.collect.Iterables.concat;
@@ -51,16 +54,25 @@ class CompositionContext {
 
     private final Map<Mount, MountedNodeStore> nodeStoresByMount;
 
+    private final Set<MountedNodeStore> allStores;
+
     CompositionContext(MountInfoProvider mip, NodeStore globalStore, List<MountedNodeStore> nonDefaultStores) {
         this.mip = mip;
         this.globalStore = new MountedNodeStore(mip.getDefaultMount(), globalStore);
         this.nonDefaultStores = nonDefaultStores;
-        this.nodeStoresByMount = copyOf(uniqueIndex(getAllMountedNodeStores(), new Function<MountedNodeStore, Mount>() {
+
+        ImmutableSet.Builder<MountedNodeStore> b = ImmutableSet.builder();
+        b.add(this.globalStore);
+        b.addAll(this.nonDefaultStores);
+        allStores = b.build();
+
+        this.nodeStoresByMount = copyOf(uniqueIndex(allStores, new Function<MountedNodeStore, Mount>() {
             @Override
             public Mount apply(MountedNodeStore input) {
                 return input.getMount();
             }
         }));
+
     }
 
     MountedNodeStore getGlobalStore() {
@@ -152,8 +164,8 @@ class CompositionContext {
         }).isPresent();
     }
 
-    Iterable<MountedNodeStore> getAllMountedNodeStores() {
-        return concat(singleton(globalStore), nonDefaultStores);
+    Set<MountedNodeStore> getAllMountedNodeStores() {
+        return allStores;
     }
 
     Blob createBlob(InputStream inputStream) throws IOException {