From 30691b6d276b81d4043396ae1113a6cd6a943d4d Mon Sep 17 00:00:00 2001 From: Robert Munteanu Date: Wed, 12 Jul 2017 12:02:09 +0300 Subject: [PATCH] OAK-6445 - Ensure mounted node stores don't contain versionable nodes --- .../composite/CompositeNodeStoreBuilderTest.java | 37 ++++++++++++++ .../oak/composite/CompositeNodeStore.java | 11 ++++ .../oak/composite/CompositeNodeStoreService.java | 18 ++++--- .../jackrabbit/oak/composite/MountedNodeStore.java | 2 +- .../oak/composite/checks/ErrorHolder.java | 47 +++++++++++++++++ .../composite/checks/MountedNodeStoreChecker.java | 37 ++++++++++++++ .../oak/composite/checks/NodeStoreChecks.java | 25 +++++++++ .../composite/checks/NodeStoreChecksService.java | 56 ++++++++++++++++++++ .../VersionableNodesMountedNodeStoreChecker.java | 59 ++++++++++++++++++++++ 9 files changed, 285 insertions(+), 7 deletions(-) create mode 100644 oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/ErrorHolder.java create mode 100644 oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/MountedNodeStoreChecker.java create mode 100644 oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/NodeStoreChecks.java create mode 100644 oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/NodeStoreChecksService.java create mode 100644 oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/VersionableNodesMountedNodeStoreChecker.java diff --git a/oak-it/src/test/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreBuilderTest.java b/oak-it/src/test/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreBuilderTest.java index 35bf5de178..11b0add028 100644 --- a/oak-it/src/test/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreBuilderTest.java +++ b/oak-it/src/test/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreBuilderTest.java @@ -18,9 +18,21 @@ */ package org.apache.jackrabbit.oak.composite; +import java.util.Collections; + +import org.apache.jackrabbit.JcrConstants; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.IllegalRepositoryStateException; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.composite.checks.NodeStoreChecksService; +import org.apache.jackrabbit.oak.composite.checks.VersionableNodesMountedNodeStoreChecker; import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore; +import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; +import org.apache.jackrabbit.oak.spi.commit.CommitInfo; +import org.apache.jackrabbit.oak.spi.commit.EmptyHook; import org.apache.jackrabbit.oak.spi.mount.MountInfoProvider; import org.apache.jackrabbit.oak.spi.mount.Mounts; +import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.junit.Test; public class CompositeNodeStoreBuilderTest { @@ -82,4 +94,29 @@ public class CompositeNodeStoreBuilderTest { .addMount("not-temp", new MemoryNodeStore()) .build(); } + + @Test(expected = IllegalRepositoryStateException.class) + public void versionableNode() throws CommitFailedException { + + MemoryNodeStore root = new MemoryNodeStore(); + MemoryNodeStore mount = new MemoryNodeStore(); + + // create a child node that is versionable + // note that we won't cover all checks here, we are only interested in seeing that at least one check is triggered + NodeBuilder rootBuilder = mount.getRoot().builder(); + NodeBuilder childNode = rootBuilder.setChildNode("first").setChildNode("second").setChildNode("third"); + childNode.setProperty(JcrConstants.JCR_ISCHECKEDOUT, false); + childNode.setProperty(PropertyStates.createProperty(JcrConstants.JCR_MIXINTYPES , Collections.singletonList(JcrConstants.MIX_VERSIONABLE), Type.NAMES)); + mount.merge(rootBuilder, EmptyHook.INSTANCE, CommitInfo.EMPTY); + + MountInfoProvider mip = Mounts.newBuilder() + .readOnlyMount("readOnly", "/readOnly") + .build(); + + new CompositeNodeStore.Builder(mip, root) + .addMount("readOnly", mount) + .with(new NodeStoreChecksService(root, Collections.singletonList(new VersionableNodesMountedNodeStoreChecker()))) + .build(); + + } } \ No newline at end of file diff --git a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java index 74e8a96b6a..06b943345c 100644 --- a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java +++ b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStore.java @@ -25,6 +25,7 @@ import org.apache.jackrabbit.oak.api.Blob; import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.commons.PathUtils; +import org.apache.jackrabbit.oak.composite.checks.NodeStoreChecks; import org.apache.jackrabbit.oak.spi.commit.CommitHook; import org.apache.jackrabbit.oak.spi.commit.CommitInfo; import org.apache.jackrabbit.oak.spi.commit.EmptyHook; @@ -454,10 +455,17 @@ public class CompositeNodeStore implements NodeStore, Observable { private boolean partialReadOnly = true; + private NodeStoreChecks checks; + public Builder(MountInfoProvider mip, NodeStore globalStore) { this.mip = checkNotNull(mip, "mountInfoProvider"); this.globalStore = checkNotNull(globalStore, "globalStore"); } + + public Builder with(NodeStoreChecks checks) { + this.checks = checks; + return this; + } public Builder addMount(String mountName, NodeStore store) { checkNotNull(store, "store"); @@ -483,6 +491,9 @@ public class CompositeNodeStore implements NodeStore, Observable { if (partialReadOnly) { assertPartialMountsAreReadOnly(); } + if ( checks != null ) { + nonDefaultStores.forEach( s -> checks.check(s)); + } return new CompositeNodeStore(mip, globalStore, nonDefaultStores, ignoreReadOnlyWritePaths); } diff --git a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreService.java b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreService.java index 46fed44a7d..1e5b3a5b6e 100644 --- a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreService.java +++ b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreService.java @@ -27,6 +27,7 @@ import org.apache.felix.scr.annotations.ReferenceCardinality; import org.apache.felix.scr.annotations.ReferencePolicy; import org.apache.jackrabbit.oak.api.jmx.CheckpointMBean; import org.apache.jackrabbit.oak.commons.PropertiesUtil; +import org.apache.jackrabbit.oak.composite.checks.NodeStoreChecks; import org.apache.jackrabbit.oak.osgi.OsgiWhiteboard; import org.apache.jackrabbit.oak.spi.commit.ObserverTracker; import org.apache.jackrabbit.oak.spi.mount.Mount; @@ -67,6 +68,9 @@ public class CompositeNodeStoreService { @Reference(cardinality = ReferenceCardinality.MANDATORY_MULTIPLE, policy = ReferencePolicy.DYNAMIC, bind = "bindNodeStore", unbind = "unbindNodeStore", referenceInterface = NodeStoreProvider.class, target="(!(service.pid=org.apache.jackrabbit.oak.composite.CompositeNodeStore))") private List nodeStores = new ArrayList<>(); + + @Reference + private NodeStoreChecks checks; @Property(label = "Ignore read only writes", unbounded = PropertyUnbounded.ARRAY, @@ -126,7 +130,7 @@ public class CompositeNodeStoreService { LOG.info("Composite node store registration is deferred until there's a global node store registered in OSGi"); return; } else { - LOG.info("Found global node store: {}", getDescription(globalNs)); + LOG.info("Found global node store: {}", globalNs.getDescription()); } for (Mount m : mountInfoProvider.getNonDefaultMounts()) { @@ -140,6 +144,7 @@ public class CompositeNodeStoreService { CompositeNodeStore.Builder builder = new CompositeNodeStore.Builder(mountInfoProvider, globalNs.getNodeStoreProvider().getNodeStore()); nodeStoresInUse.add(globalNs.getNodeStoreProvider()); + builder.with(checks); builder.setPartialReadOnly(partialReadOnly); for (String p : ignoreReadOnlyWritePaths) { builder.addIgnoredReadOnlyWritePath(p); @@ -152,7 +157,7 @@ public class CompositeNodeStoreService { String mountName = getMountName(ns); if (mountName != null) { builder.addMount(mountName, ns.getNodeStoreProvider().getNodeStore()); - LOG.info("Mounting {} as {}", getDescription(ns), mountName); + LOG.info("Mounting {} as {}", ns.getDescription(), mountName); nodeStoresInUse.add(ns.getNodeStoreProvider()); } } @@ -197,10 +202,6 @@ public class CompositeNodeStoreService { return role.substring(MOUNT_ROLE_PREFIX.length()); } - private String getDescription(NodeStoreWithProps ns) { - return PropertiesUtil.toString(ns.getProps().get("oak.nodestore.description"), ns.getNodeStoreProvider().getClass().toString()); - } - private void unregisterCompositeNodeStore() { if (nsReg != null) { LOG.info("Unregistering the composite node store"); @@ -272,5 +273,10 @@ public class CompositeNodeStoreService { public String getRole() { return PropertiesUtil.toString(props.get(NodeStoreProvider.ROLE), null); } + + public String getDescription() { + return PropertiesUtil.toString(getProps().get("oak.nodestore.description"), + getNodeStoreProvider().getClass().toString()); + } } } \ No newline at end of file diff --git a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java index 26803dbad6..9f9eadde24 100644 --- a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java +++ b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/MountedNodeStore.java @@ -21,7 +21,7 @@ package org.apache.jackrabbit.oak.composite; import org.apache.jackrabbit.oak.spi.mount.Mount; import org.apache.jackrabbit.oak.spi.state.NodeStore; -class MountedNodeStore { +public class MountedNodeStore { private final Mount mount; diff --git a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/ErrorHolder.java b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/ErrorHolder.java new file mode 100644 index 0000000000..1255df4961 --- /dev/null +++ b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/ErrorHolder.java @@ -0,0 +1,47 @@ +/* + * 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.oak.composite.checks; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.jackrabbit.oak.api.IllegalRepositoryStateException; +import org.apache.jackrabbit.oak.composite.MountedNodeStore; + +class ErrorHolder { + + private static final int FAIL_IMMEDIATELY_THRESHOLD = 100; + private final List errors = new ArrayList<>(); + + public void report(MountedNodeStore mountedStore, String path, String error) { + errors.add(String.format("For NodeStore mount %s, path %s, encountered the following problem: '%s'", mountedStore.getMount().getName(), path, error)); + if ( errors.size() == FAIL_IMMEDIATELY_THRESHOLD ) { + end(); + } + } + + public void end() { + if ( errors.isEmpty() ) { + return; + } + StringBuilder out = new StringBuilder(); + out.append(errors.size()).append(" errors were found: \n"); + errors.forEach( e -> out.append(e).append('\n')); + + throw new IllegalRepositoryStateException(errors.toString()); + } +} \ No newline at end of file diff --git a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/MountedNodeStoreChecker.java b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/MountedNodeStoreChecker.java new file mode 100644 index 0000000000..4a5870e643 --- /dev/null +++ b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/MountedNodeStoreChecker.java @@ -0,0 +1,37 @@ +/* + * 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.oak.composite.checks; + +import org.apache.jackrabbit.oak.composite.MountedNodeStore; +import org.apache.jackrabbit.oak.spi.state.NodeStore; + +/** + * Applies a category of consistence checks specific to NodeStore mounts + * + *

Checks are only performed on non-default mounts.

+ * + *

Named 'Checker' to clarify that it is not a Validator in the Oak sense.

+ * + */ +public interface MountedNodeStoreChecker { + + void validate(NodeStore root, MountedNodeStore mount, ErrorHolder handler); + +} diff --git a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/NodeStoreChecks.java b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/NodeStoreChecks.java new file mode 100644 index 0000000000..7501bd0913 --- /dev/null +++ b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/NodeStoreChecks.java @@ -0,0 +1,25 @@ +/* + * 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.oak.composite.checks; + +import org.apache.jackrabbit.oak.composite.MountedNodeStore; + +public interface NodeStoreChecks { + + void check(MountedNodeStore mountedStore); + +} diff --git a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/NodeStoreChecksService.java b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/NodeStoreChecksService.java new file mode 100644 index 0000000000..e8e9be60ff --- /dev/null +++ b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/NodeStoreChecksService.java @@ -0,0 +1,56 @@ +/* + * 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.oak.composite.checks; + +import java.util.List; + +import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Reference; +import org.apache.felix.scr.annotations.Service; +import org.apache.jackrabbit.oak.composite.MountedNodeStore; +import org.apache.jackrabbit.oak.spi.state.NodeStore; + +@Component +@Service(NodeStoreChecks.class) +public class NodeStoreChecksService implements NodeStoreChecks { + + private final NodeStore globalStore; + + @Reference + private List checkers; + + public NodeStoreChecksService(NodeStore globalStore) { + this.globalStore = globalStore; + } + + // visible for testing + public NodeStoreChecksService(NodeStore globalStore, List checkers) { + this(globalStore); + this.checkers = checkers; + } + + @Override + public void check(MountedNodeStore mountedStore) { + + ErrorHolder errorHolder = new ErrorHolder(); + + checkers.forEach( c -> c.validate(globalStore, mountedStore, errorHolder)); + + errorHolder.end(); + + } +} diff --git a/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/VersionableNodesMountedNodeStoreChecker.java b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/VersionableNodesMountedNodeStoreChecker.java new file mode 100644 index 0000000000..9e7efbc269 --- /dev/null +++ b/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/checks/VersionableNodesMountedNodeStoreChecker.java @@ -0,0 +1,59 @@ +/* + * 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.oak.composite.checks; + +import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Service; +import org.apache.jackrabbit.oak.api.Root; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.composite.MountedNodeStore; +import org.apache.jackrabbit.oak.namepath.NamePathMapper; +import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager; +import org.apache.jackrabbit.oak.plugins.tree.RootFactory; +import org.apache.jackrabbit.oak.plugins.tree.TreeFactory; +import org.apache.jackrabbit.oak.plugins.version.VersionConstants; +import org.apache.jackrabbit.oak.spi.state.NodeStore; + +/** + * Checks that no versionable nodes are present in a non-default NodeStore + */ +@Component +@Service(MountedNodeStoreChecker.class) +public class VersionableNodesMountedNodeStoreChecker implements MountedNodeStoreChecker { + + @Override + public void validate(NodeStore globalStore, MountedNodeStore mountedStore, ErrorHolder errorHolder) { + + Root globalRoot = RootFactory.createReadOnlyRoot(globalStore.getRoot()); + + ReadOnlyNodeTypeManager typeManager = ReadOnlyNodeTypeManager.getInstance(globalRoot, NamePathMapper.DEFAULT); + + Tree mountRoot = TreeFactory.createReadOnlyTree(mountedStore.getNodeStore().getRoot()); + + visit(mountRoot, mountedStore, typeManager, errorHolder); + } + + private void visit(Tree tree, MountedNodeStore mountedStore, ReadOnlyNodeTypeManager typeManager, ErrorHolder validationErrorHolder) { + + if ( typeManager.isNodeType(tree, VersionConstants.MIX_VERSIONABLE) ) { + validationErrorHolder.report(mountedStore, tree.getPath(), "versionable node"); + } + + tree.getChildren().forEach( child -> visit(child, mountedStore, typeManager, validationErrorHolder)); + } + +} -- 2.13.2