Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java (revision 1769063) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java (working copy) @@ -343,6 +343,17 @@ } } + public Set getBundledChildNodeNames(){ + return bundlingContext.getBundledChildNodeNames(); + } + + public boolean hasOnlyBundledChildren(){ + if (bundlingContext.isBundled()){ + return bundlingContext.hasOnlyBundledChildren(); + } + return false; + } + String getPropertyAsString(String propertyName) { return asString(properties.get(propertyName)); } @@ -803,7 +814,7 @@ } public boolean hasChildNode(String relativePath){ - String key = concat(relativePath, DocumentBundlor.META_PROP_NODE); + String key = concat(relativePath, DocumentBundlor.META_PROP_BUNDLING_PATH); return rootProperties.containsKey(key); } @@ -815,11 +826,11 @@ return !hasNonBundledChildren; } - public List getBundledChildNodeNames(){ + public Set getBundledChildNodeNames(){ if (isBundled()) { return BundlorUtils.getChildNodeNames(rootProperties.keySet(), matcher); } - return Collections.emptyList(); + return Collections.emptySet(); } private boolean hasBundledChildren(Matcher matcher){ Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (revision 1769063) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (working copy) @@ -86,6 +86,7 @@ import org.apache.jackrabbit.oak.plugins.blob.MarkSweepGarbageCollector; import org.apache.jackrabbit.oak.plugins.blob.ReferencedBlob; import org.apache.jackrabbit.oak.plugins.document.Branch.BranchCommit; +import org.apache.jackrabbit.oak.plugins.document.bundlor.BundledDocumentDiffer; import org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingConfigHandler; import org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor; import org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache; @@ -145,7 +146,10 @@ * in DocumentBundlor as those are only required by DocumentNodeState */ public static final List META_PROP_NAMES = ImmutableList.of( - DocumentBundlor.META_PROP_PATTERN + DocumentBundlor.META_PROP_PATTERN, + DocumentBundlor.META_PROP_BUNDLING_PATH, + DocumentBundlor.META_PROP_NON_BUNDLED_CHILD, + DocumentBundlor.META_PROP_BUNDLED_CHILD ); /** @@ -168,11 +172,13 @@ private long recoveryWaitTimeoutMS = Long.getLong("oak.recoveryWaitTimeoutMS", 60000); + public static final String SYS_PROP_DISABLE_JOURNAL = "oak.disableJournalDiff"; + /** * Feature flag to disable the journal diff mechanism. See OAK-4528. */ private boolean disableJournalDiff = - Boolean.getBoolean("oak.disableJournalDiff"); + Boolean.getBoolean(SYS_PROP_DISABLE_JOURNAL); /** * The document store (might be used by multiple node stores). @@ -430,6 +436,8 @@ private final BundlingConfigHandler bundlingConfigHandler = new BundlingConfigHandler(); + private final BundledDocumentDiffer bundledDocDiffer = new BundledDocumentDiffer(this); + public DocumentNodeStore(DocumentMK.Builder builder) { this.blobStore = builder.getBlobStore(); this.statisticsProvider = builder.getStatisticsProvider(); @@ -913,7 +921,7 @@ * given revision. */ @CheckForNull - DocumentNodeState getNode(@Nonnull final String path, + public DocumentNodeState getNode(@Nonnull final String path, @Nonnull final RevisionVector rev) { checkNotNull(rev); checkNotNull(path); @@ -2276,30 +2284,36 @@ diffAlgo = "diffJournalChildren"; diff = new JournalDiffLoader(from, to, this).call(); } else { - DocumentNodeState.Children fromChildren, toChildren; - fromChildren = getChildren(from, null, max); - toChildren = getChildren(to, null, max); - getChildrenDoneIn = debug ? now() : 0; + JsopWriter w = new JsopStream(); + boolean continueDiff = bundledDocDiffer.diff(from, to, w); - JsopWriter w = new JsopStream(); - if (!fromChildren.hasMore && !toChildren.hasMore) { - diffAlgo = "diffFewChildren"; - diffFewChildren(w, from.getPath(), fromChildren, - fromRev, toChildren, toRev); - } else { - if (FAST_DIFF) { - diffAlgo = "diffManyChildren"; - fromRev = from.getRootRevision(); - toRev = to.getRootRevision(); - diffManyChildren(w, from.getPath(), fromRev, toRev); - } else { - diffAlgo = "diffAllChildren"; - max = Integer.MAX_VALUE; - fromChildren = getChildren(from, null, max); - toChildren = getChildren(to, null, max); + if (continueDiff) { + DocumentNodeState.Children fromChildren, toChildren; + fromChildren = getChildren(from, null, max); + toChildren = getChildren(to, null, max); + getChildrenDoneIn = debug ? now() : 0; + + if (!fromChildren.hasMore && !toChildren.hasMore) { + diffAlgo = "diffFewChildren"; diffFewChildren(w, from.getPath(), fromChildren, fromRev, toChildren, toRev); + } else { + if (FAST_DIFF) { + diffAlgo = "diffManyChildren"; + fromRev = from.getRootRevision(); + toRev = to.getRootRevision(); + diffManyChildren(w, from.getPath(), fromRev, toRev); + } else { + diffAlgo = "diffAllChildren"; + max = Integer.MAX_VALUE; + fromChildren = getChildren(from, null, max); + toChildren = getChildren(to, null, max); + diffFewChildren(w, from.getPath(), fromChildren, + fromRev, toChildren, toRev); + } } + } else { + diffAlgo = "allBundledChildren"; } diff = w.toString(); } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java (nonexistent) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java (working copy) @@ -0,0 +1,116 @@ +/* + * 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.plugins.document.bundlor; + +import java.util.Collection; + +import org.apache.jackrabbit.oak.commons.PathUtils; +import org.apache.jackrabbit.oak.commons.json.JsopWriter; +import org.apache.jackrabbit.oak.plugins.document.AbstractDocumentNodeState; +import org.apache.jackrabbit.oak.plugins.document.DocumentNodeState; +import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore; +import org.apache.jackrabbit.oak.spi.state.NodeStateUtils; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; + +public class BundledDocumentDiffer { + private final DocumentNodeStore nodeStore; + + public BundledDocumentDiffer(DocumentNodeStore nodeStore) { + this.nodeStore = nodeStore; + } + + /** + * Performs diff for bundled nodes. The passed state can be DocumentNodeState or + * one from secondary nodestore i.e. {@code DelegatingDocumentNodeState}. So the + * passed states cannot be cast down to DocumentNodeState + * + * @param from from state + * @param to to state + * @param w jsop diff + * @return true if the diff needs to be continued. In case diff is complete it would return false + */ + public boolean diff(AbstractDocumentNodeState from, AbstractDocumentNodeState to, JsopWriter w){ + boolean fromBundled = BundlorUtils.isBundledNode(from); + boolean toBundled = BundlorUtils.isBundledNode(to); + + //Neither of the nodes bundled + if (!fromBundled && !toBundled){ + return true; + } + + DocumentNodeState fromDocState = getDocumentNodeState(from); + DocumentNodeState toDocState = getDocumentNodeState(to); + + diffChildren(fromDocState.getBundledChildNodeNames(), toDocState.getBundledChildNodeNames(), w); + + //If all child nodes are bundled then diff is complete + if (fromDocState.hasOnlyBundledChildren() + && toDocState.hasOnlyBundledChildren()){ + return false; + } + + return true; + } + + void diffChildren(Collection fromChildren, Collection toChildren, JsopWriter w){ + for (String n : fromChildren){ + if (!toChildren.contains(n)){ + w.tag('-').value(n); + } else { + //As lastRev for bundled node is same as parent node and they differ it means + //children "may" also diff + w.tag('^').key(n).object().endObject(); + } + } + + for (String n : toChildren){ + if (!fromChildren.contains(n)){ + w.tag('+').key(n).object().endObject(); + } + } + } + + private DocumentNodeState getDocumentNodeState(AbstractDocumentNodeState state) { + DocumentNodeState result; + + //Shortcut - If already a DocumentNodeState use as it. In case of SecondaryNodeStore + //it can be DelegatingDocumentNodeState. In that case we need to read DocumentNodeState from + //DocumentNodeStore and then get to DocumentNodeState for given path + + if (state instanceof DocumentNodeState) { + result = (DocumentNodeState) state; + } else if (BundlorUtils.isBundledChild(state)) { + //In case of bundle child determine the bundling root + //and from there traverse down to the actual child node + checkState(BundlorUtils.isBundledChild(state)); + String bundlingPath = state.getString(DocumentBundlor.META_PROP_BUNDLING_PATH); + String bundlingRootPath = PathUtils.getAncestorPath(state.getPath(), PathUtils.getDepth(bundlingPath)); + DocumentNodeState bundlingRoot = nodeStore.getNode(bundlingRootPath, state.getLastRevision()); + result = (DocumentNodeState) NodeStateUtils.getNode(bundlingRoot, bundlingPath); + } else { + result = nodeStore.getNode(state.getPath(), state.getLastRevision()); + } + + checkNotNull(result, "Node at [%s] not found for fromRev [%s]", state.getPath(), state.getLastRevision()); + return result; + } +} Property changes on: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java ___________________________________________________________________ Added: svn:eol-style ## -0,0 +1 ## +native \ No newline at end of property Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlingHandler.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlingHandler.java (revision 1769063) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlingHandler.java (working copy) @@ -24,20 +24,15 @@ 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.PropertyStates; import org.apache.jackrabbit.oak.spi.state.NodeState; import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.jackrabbit.oak.commons.PathUtils.ROOT_PATH; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; +import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty; public class BundlingHandler { - /** - * True property which is used to mark the presence of relative node - * This needs to be set when a bundled relative node is added - */ - private static final PropertyState NODE_PRESENCE_MARKER = - PropertyStates.createProperty(DocumentBundlor.META_PROP_NODE, Boolean.TRUE); + private final BundledTypesRegistry registry; private final String path; private final BundlingContext ctx; @@ -101,7 +96,7 @@ Matcher childMatcher = ctx.matcher.next(name); if (childMatcher.isMatch()) { childContext = createChildContext(childMatcher); - childContext.addMetaProp(NODE_PRESENCE_MARKER); + childContext.addMetaProp(createProperty(DocumentBundlor.META_PROP_BUNDLING_PATH, childMatcher.getMatchedPath())); } else { DocumentBundlor bundlor = registry.getBundlor(state); if (bundlor != null){ @@ -179,7 +174,7 @@ } private static void removeDeletedChildProperties(NodeState state, BundlingContext childContext) { - childContext.removeProperty(DocumentBundlor.META_PROP_NODE); + childContext.removeProperty(DocumentBundlor.META_PROP_BUNDLING_PATH); for (PropertyState ps : state.getProperties()){ String propName = ps.getName(); //In deletion never touch child status related meta props Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorUtils.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorUtils.java (revision 1769063) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorUtils.java (working copy) @@ -23,17 +23,19 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import javax.annotation.Nonnull; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; 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.spi.state.NodeState; -import static org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor.META_PROP_NODE; +import static org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor.META_PROP_BUNDLING_PATH; public final class BundlorUtils { public static final Predicate NOT_BUNDLOR_PROPS = new Predicate() { @@ -84,8 +86,8 @@ return result; } - public static List getChildNodeNames(Collection keys, Matcher matcher){ - List childNodeNames = Lists.newArrayList(); + public static Set getChildNodeNames(Collection keys, Matcher matcher){ + Set childNodeNames = Sets.newHashSet(); //Immediate child should have depth 1 more than matcher depth int expectedDepth = matcher.depth() + 1; @@ -96,7 +98,7 @@ if (depth == expectedDepth && key.startsWith(matcher.getMatchedPath()) - && elements.get(elements.size() - 1).equals(META_PROP_NODE)){ + && elements.get(elements.size() - 1).equals(META_PROP_BUNDLING_PATH)){ //Child node name is the second last element //[jcr:content/:self -> [jcr:content, :self] childNodeNames.add(elements.get(elements.size() - 2)); @@ -104,4 +106,16 @@ } return childNodeNames; } + + static boolean isBundlingRoot(NodeState state){ + return state.hasProperty(DocumentBundlor.META_PROP_PATTERN); + } + + static boolean isBundledChild(NodeState state){ + return state.hasProperty(DocumentBundlor.META_PROP_BUNDLING_PATH); + } + + static boolean isBundledNode(NodeState state){ + return isBundlingRoot(state) || isBundledChild(state); + } } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlor.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlor.java (revision 1769063) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlor.java (working copy) @@ -47,9 +47,11 @@ /** * Hidden property name used as suffix for relative node path * to indicate presence of that node. So for a relative node 'jcr:content' - * the parent node must have a property 'jcr:content/:self + * the parent node must have a property 'jcr:content/:doc-self-path. + * + *

Its value is the depth of the bundled child node */ - public static final String META_PROP_NODE = ":doc-self"; + public static final String META_PROP_BUNDLING_PATH = ":doc-self-path"; public static final String HAS_CHILD_PROP_PREFIX = ":doc-has-child-"; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/DelegatingDocumentNodeState.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/DelegatingDocumentNodeState.java (revision 1769063) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/DelegatingDocumentNodeState.java (working copy) @@ -46,7 +46,7 @@ * so as to expose it as an {@link AbstractDocumentNodeState} by extracting * the meta properties which are stored as hidden properties */ -class DelegatingDocumentNodeState extends AbstractDocumentNodeState { +public class DelegatingDocumentNodeState extends AbstractDocumentNodeState { //Hidden props holding DocumentNodeState meta properties static final String PROP_REVISION = ":doc-rev"; static final String PROP_LAST_REV = ":doc-lastRev"; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/SecondaryStoreCache.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/SecondaryStoreCache.java (revision 1769063) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/SecondaryStoreCache.java (working copy) @@ -39,7 +39,7 @@ import org.slf4j.LoggerFactory; -class SecondaryStoreCache implements DocumentNodeStateCache, SecondaryStoreRootObserver { +public class SecondaryStoreCache implements DocumentNodeStateCache, SecondaryStoreRootObserver { private final Logger log = LoggerFactory.getLogger(getClass()); private static final AbstractDocumentNodeState[] EMPTY = new AbstractDocumentNodeState[0]; private final NodeStore store; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/SecondaryStoreObserver.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/SecondaryStoreObserver.java (revision 1769063) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/secondary/SecondaryStoreObserver.java (working copy) @@ -43,7 +43,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -class SecondaryStoreObserver implements Observer { +public class SecondaryStoreObserver implements Observer { private final Logger log = LoggerFactory.getLogger(getClass()); private final NodeStore nodeStore; private final PathFilter pathFilter; Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java (revision 1769063) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java (working copy) @@ -23,6 +23,7 @@ import com.google.common.base.Predicate; import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.spi.commit.CommitInfo; import org.apache.jackrabbit.oak.spi.commit.EmptyHook; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; @@ -29,6 +30,8 @@ import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStore; +import static org.junit.Assert.fail; + public class TestUtils { public static final Predicate IS_LAST_REV_UPDATE = new Predicate() { @@ -60,4 +63,27 @@ throws CommitFailedException { return store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); } + + public static NodeBuilder createChild(NodeBuilder root, String ... paths){ + for (String path : paths){ + childBuilder(root, path); + } + return root; + } + + public static NodeBuilder childBuilder(NodeBuilder root, String path){ + NodeBuilder nb = root; + for (String nodeName : PathUtils.elements(path)){ + nb = nb.child(nodeName); + } + return nb; + } + + public static DocumentNodeState asDocumentState(NodeState state){ + if (state instanceof DocumentNodeState){ + return (DocumentNodeState) state; + } + fail("Not of type AbstractDoucmentNodeState"); + return null; + } } Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java (nonexistent) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java (working copy) @@ -0,0 +1,251 @@ +/* + * 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.plugins.document.bundlor; + +import java.util.Collections; + + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.commons.json.JsopBuilder; +import org.apache.jackrabbit.oak.commons.json.JsopWriter; +import org.apache.jackrabbit.oak.plugins.document.AbstractDocumentNodeState; +import org.apache.jackrabbit.oak.plugins.document.DocumentMKBuilderProvider; +import org.apache.jackrabbit.oak.plugins.document.DocumentNodeState; +import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore; +import org.apache.jackrabbit.oak.plugins.document.secondary.DelegatingDocumentNodeState; +import org.apache.jackrabbit.oak.plugins.document.secondary.SecondaryStoreBuilder; +import org.apache.jackrabbit.oak.plugins.document.secondary.SecondaryStoreCache; +import org.apache.jackrabbit.oak.plugins.document.secondary.SecondaryStoreObserver; +import org.apache.jackrabbit.oak.plugins.index.PathFilter; +import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore; +import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent; +import org.apache.jackrabbit.oak.spi.state.DefaultNodeStateDiff; +import org.apache.jackrabbit.oak.spi.state.NodeBuilder; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.apache.jackrabbit.oak.spi.state.NodeStateUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import static com.google.inject.internal.util.$ImmutableList.of; +import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore.SYS_PROP_DISABLE_JOURNAL; +import static org.apache.jackrabbit.oak.plugins.document.TestUtils.childBuilder; +import static org.apache.jackrabbit.oak.plugins.document.TestUtils.createChild; +import static org.apache.jackrabbit.oak.plugins.document.TestUtils.merge; +import static org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingConfigHandler.BUNDLOR; +import static org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingConfigHandler.DOCUMENT_NODE_STORE; +import static org.apache.jackrabbit.oak.plugins.document.TestUtils.asDocumentState; +import static org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlingTest.newNode; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +public class BundledDocumentDifferTest { + @Rule + public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider(); + private DocumentNodeStore store; + private String journalDisabledProp; + private BundledDocumentDiffer differ; + private MemoryNodeStore secondary = new MemoryNodeStore(); + + + @Before + public void setUpBundlor() throws CommitFailedException { + journalDisabledProp = System.getProperty(SYS_PROP_DISABLE_JOURNAL); + System.setProperty(SYS_PROP_DISABLE_JOURNAL, "true"); + store = builderProvider + .newBuilder() + .memoryCacheSize(0) + .getNodeStore(); + NodeState registryState = BundledTypesRegistry.builder() + .forType("app:Asset") + .include("jcr:content") + .include("jcr:content/metadata") + .include("jcr:content/renditions") + .include("jcr:content/renditions/**") + .build(); + + NodeBuilder builder = store.getRoot().builder(); + new InitialContent().initialize(builder); + builder.getChildNode("jcr:system") + .getChildNode(DOCUMENT_NODE_STORE) + .getChildNode(BUNDLOR) + .setChildNode("app:Asset", registryState.getChildNode("app:Asset")); + merge(store, builder); + differ = new BundledDocumentDiffer(store); + store.runBackgroundOperations(); + } + + @After + public void resetJournalUsage(){ + if (journalDisabledProp != null) { + System.setProperty(SYS_PROP_DISABLE_JOURNAL, journalDisabledProp); + } else { + System.clearProperty(SYS_PROP_DISABLE_JOURNAL); + } + } + + @Test + public void testDiff() throws Exception { + NodeBuilder builder = createContentStructure(); + NodeState r1 = merge(store, builder); + + builder = store.getRoot().builder(); + childBuilder(builder, "/test/book.jpg/jcr:content").setProperty("foo", "bar"); + NodeState r2 = merge(store, builder); + + JsopWriter w = new JsopBuilder(); + String path = "/test"; + assertTrue(differ.diff(dns(r1, path), dns(r2, path), w)); + assertTrue(w.toString().isEmpty()); + + w = new JsopBuilder(); + path = "/test/book.jpg"; + assertFalse(differ.diff(dns(r1, path), dns(r2, path), w)); + assertEquals("^\"jcr:content\":{}", w.toString()); + + builder = store.getRoot().builder(); + childBuilder(builder, "/test/book.jpg/foo"); + NodeState r3 = merge(store, builder); + + w = new JsopBuilder(); + path = "/test/book.jpg"; + //As there is a non bundled child differ should return true to continue diffing + assertTrue(differ.diff(dns(r1, path), dns(r3, path), w)); + assertEquals("^\"jcr:content\":{}", w.toString()); + } + + @Test + public void diffWithSecondary() throws Exception{ + configureSecondary(); + NodeBuilder builder = createContentStructure(); + NodeState r1 = merge(store, builder); + NodeState rs1 = DelegatingDocumentNodeState.wrap(secondary.getRoot(), store); + + builder = store.getRoot().builder(); + childBuilder(builder, "/test/book.jpg/jcr:content").setProperty("foo", "bar"); + NodeState r2 = merge(store, builder); + + JsopWriter w = new JsopBuilder(); + String path = "/test/book.jpg"; + assertFalse(differ.diff(adns(rs1, path), adns(r2, path), w)); + assertEquals("^\"jcr:content\":{}", w.toString()); + } + + @Test + public void diffFewChildren() throws Exception{ + NodeBuilder builder = createContentStructure(); + NodeState r1 = merge(store, builder); + + builder = store.getRoot().builder(); + childBuilder(builder, "/test/book.jpg/jcr:content").setProperty("foo", "bar"); + childBuilder(builder, "/test/book.jpg/jcr:content/renditions/newChild2").setProperty("foo", "bar"); + childBuilder(builder, "/test/book.jpg/newChild1"); + NodeState r2 = merge(store, builder); + + String path = "/test/book.jpg"; + CollectingDiff diff = new CollectingDiff(); + adns(r2, path).compareAgainstBaseState(adns(r1, path), diff); + + assertThat(diff.changes.get("added"), hasItem("newChild1")); + assertThat(diff.changes.get("changed"), hasItem("jcr:content")); + + diff = new CollectingDiff(); + path = "/test/book.jpg/jcr:content/renditions"; + adns(r2, path).compareAgainstBaseState(adns(r1, path), diff); + + System.out.println(diff); + assertThat(diff.changes.get("added"), hasItem("newChild2")); + + } + + + + private NodeBuilder createContentStructure() { + NodeBuilder builder = store.getRoot().builder(); + NodeBuilder appNB = newNode("app:Asset"); + createChild(appNB, + "jcr:content", + "jcr:content/comments", //not bundled + "jcr:content/metadata", + "jcr:content/metadata/xmp", //not bundled + "jcr:content/renditions", //includes all + "jcr:content/renditions/original", + "jcr:content/renditions/original/jcr:content" + ); + + builder.child("test").setChildNode("book.jpg", appNB.getNodeState()); + return builder; + } + + private static class CollectingDiff extends DefaultNodeStateDiff { + private ListMultimap changes = ArrayListMultimap.create(); + + @Override + public boolean childNodeAdded(String name, NodeState after) { + changes.get("added").add(name); + return true; + } + + @Override + public boolean childNodeChanged(String name, NodeState before, NodeState after) { + changes.get("changed").add(name); + return super.childNodeChanged(name, before, after); + } + + @Override + public boolean childNodeDeleted(String name, NodeState before) { + changes.get("deleted").add(name); + return super.childNodeDeleted(name, before); + } + + @Override + public String toString() { + return changes.toString(); + } + } + + private DocumentNodeState dns(NodeState root, String path){ + return asDocumentState(NodeStateUtils.getNode(root, path)); + } + + private AbstractDocumentNodeState adns(NodeState root, String path){ + return (AbstractDocumentNodeState) NodeStateUtils.getNode(root, path); + } + + private SecondaryStoreCache configureSecondary(){ + SecondaryStoreBuilder builder = createBuilder(new PathFilter(of("/"), Collections.emptyList())); + builder.metaPropNames(DocumentNodeStore.META_PROP_NAMES); + SecondaryStoreCache cache = builder.buildCache(); + SecondaryStoreObserver observer = builder.buildObserver(cache); + store.addObserver(observer); + return cache; + } + + private SecondaryStoreBuilder createBuilder(PathFilter pathFilter) { + return new SecondaryStoreBuilder(secondary).pathFilter(pathFilter); + } + +} \ No newline at end of file Property changes on: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java ___________________________________________________________________ Added: svn:eol-style ## -0,0 +1 ## +native \ No newline at end of property Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorUtilsTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorUtilsTest.java (revision 1769063) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorUtilsTest.java (working copy) @@ -22,6 +22,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; @@ -29,7 +30,7 @@ import static java.util.Arrays.asList; import static org.apache.jackrabbit.oak.commons.PathUtils.concat; -import static org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor.META_PROP_NODE; +import static org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor.META_PROP_BUNDLING_PATH; import static org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor.META_PROP_PATTERN; import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.CoreMatchers.hasItems; @@ -52,7 +53,7 @@ Matcher m = new Include("jcr:content").createMatcher(); Map result = BundlorUtils.getMatchingProperties( create("a", - concat("jcr:content", META_PROP_NODE), + concat("jcr:content", META_PROP_BUNDLING_PATH), "jcr:content/jcr:data", "jcr:primaryType", META_PROP_PATTERN @@ -66,7 +67,7 @@ Matcher m = new Include("jcr:content").createMatcher().next("jcr:content"); Map result = BundlorUtils.getMatchingProperties( create("a", - concat("jcr:content", META_PROP_NODE), + concat("jcr:content", META_PROP_BUNDLING_PATH), "jcr:content/jcr:data", "jcr:content/metadata/format", "jcr:primaryType", @@ -80,16 +81,16 @@ @Test public void childNodeNames() throws Exception{ List testData = asList("x", - concat("jcr:content", META_PROP_NODE), + concat("jcr:content", META_PROP_BUNDLING_PATH), "jcr:content/jcr:data", - concat("jcr:content/metadata", META_PROP_NODE), + concat("jcr:content/metadata", META_PROP_BUNDLING_PATH), "jcr:content/metadata/format", - concat("jcr:content/comments", META_PROP_NODE), - concat("jcr:content/renditions/original", META_PROP_NODE) + concat("jcr:content/comments", META_PROP_BUNDLING_PATH), + concat("jcr:content/renditions/original", META_PROP_BUNDLING_PATH) ); Matcher m = new Include("jcr:content/*").createMatcher(); - List names = BundlorUtils.getChildNodeNames(testData, m); + Set names = BundlorUtils.getChildNodeNames(testData, m); assertThat(names, hasItem("jcr:content")); names = BundlorUtils.getChildNodeNames(testData, m.next("jcr:content")); Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java (revision 1769063) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java (working copy) @@ -41,6 +41,7 @@ import org.apache.jackrabbit.oak.plugins.document.NodeDocument; import org.apache.jackrabbit.oak.plugins.document.PathRev; import org.apache.jackrabbit.oak.plugins.document.TestNodeObserver; +import org.apache.jackrabbit.oak.plugins.document.TestUtils; import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore; import org.apache.jackrabbit.oak.plugins.document.util.Utils; import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent; @@ -52,15 +53,20 @@ import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; import org.apache.jackrabbit.oak.spi.state.NodeStateUtils; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.ImmutableList.copyOf; import static java.lang.String.format; import static org.apache.commons.io.FileUtils.ONE_MB; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.oak.commons.PathUtils.concat; +import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore.SYS_PROP_DISABLE_JOURNAL; +import static org.apache.jackrabbit.oak.plugins.document.TestUtils.childBuilder; +import static org.apache.jackrabbit.oak.plugins.document.TestUtils.createChild; import static org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingConfigHandler.BUNDLOR; import static org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingConfigHandler.DOCUMENT_NODE_STORE; import static org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor.META_PROP_BUNDLED_CHILD; @@ -80,6 +86,7 @@ public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider(); private DocumentNodeStore store; private RecordingDocumentStore ds = new RecordingDocumentStore(); + private String journalDisabledProp; @Before public void setUpBundlor() throws CommitFailedException { @@ -104,8 +111,18 @@ .setChildNode("app:Asset", registryState.getChildNode("app:Asset")); merge(builder); store.runBackgroundOperations(); + journalDisabledProp = System.getProperty(SYS_PROP_DISABLE_JOURNAL); } + @After + public void resetSysProps(){ + if (journalDisabledProp != null) { + System.setProperty(SYS_PROP_DISABLE_JOURNAL, journalDisabledProp); + } else { + System.clearProperty(SYS_PROP_DISABLE_JOURNAL); + } + } + @Test public void saveAndReadNtFile() throws Exception{ NodeBuilder builder = store.getRoot().builder(); @@ -120,6 +137,7 @@ assertTrue(fileNodeState.getChildNode("book.jpg").exists()); NodeState contentNode = fileNodeState.getChildNode("book.jpg").getChildNode("jcr:content"); assertTrue(contentNode.exists()); + assertEquals("jcr:content", getBundlingPath(contentNode)); assertEquals(1, contentNode.getPropertyCount()); assertEquals(1, Iterables.size(contentNode.getProperties())); @@ -126,16 +144,34 @@ assertNull(getNodeDocument("/test/book.jpg/jcr:content")); assertNotNull(getNodeDocument("/test/book.jpg")); - assertTrue(hasNodeProperty("/test/book.jpg", concat("jcr:content", DocumentBundlor.META_PROP_NODE))); + assertTrue(hasNodeProperty("/test/book.jpg", concat("jcr:content", DocumentBundlor.META_PROP_BUNDLING_PATH))); AssertingDiff.assertEquals(fileNode.getNodeState(), fileNodeState.getChildNode("book.jpg")); - DocumentNodeState dns = asDocumentState(fileNodeState.getChildNode("book.jpg")); + DocumentNodeState dns = TestUtils.asDocumentState(fileNodeState.getChildNode("book.jpg")); AssertingDiff.assertEquals(fileNode.getNodeState(), dns.withRootRevision(dns.getRootRevision(), true)); AssertingDiff.assertEquals(fileNode.getNodeState(), dns.fromExternalChange()); } + @Test + public void bundlorUtilsIsBundledMethods() throws Exception{ + NodeBuilder builder = store.getRoot().builder(); + NodeBuilder fileNode = newNode("nt:file"); + fileNode.child("jcr:content").setProperty("jcr:data", "foo"); + builder.child("test").setChildNode("book.jpg", fileNode.getNodeState()); + merge(builder); + NodeState fileNodeState = NodeStateUtils.getNode(store.getRoot(), "/test/book.jpg"); + assertTrue(BundlorUtils.isBundledNode(fileNodeState)); + assertFalse(BundlorUtils.isBundledChild(fileNodeState)); + assertTrue(BundlorUtils.isBundlingRoot(fileNodeState)); + + NodeState contentNode = NodeStateUtils.getNode(store.getRoot(), "/test/book.jpg/jcr:content"); + assertTrue(BundlorUtils.isBundledNode(contentNode)); + assertTrue(BundlorUtils.isBundledChild(contentNode)); + assertFalse(BundlorUtils.isBundlingRoot(contentNode)); + } + @Test public void bundledParent() throws Exception{ NodeBuilder builder = store.getRoot().builder(); @@ -149,6 +185,31 @@ merge(builder); } + + @Test + public void bundlingPath() throws Exception{ + NodeBuilder builder = store.getRoot().builder(); + NodeBuilder appNB = newNode("app:Asset"); + createChild(appNB, + "jcr:content", + "jcr:content/comments", //not bundled + "jcr:content/metadata", + "jcr:content/metadata/xmp", //not bundled + "jcr:content/renditions", //includes all + "jcr:content/renditions/original", + "jcr:content/renditions/original/jcr:content" + ); + builder.child("test").setChildNode("book.jpg", appNB.getNodeState()); + + merge(builder); + NodeState appNode = getNode(store.getRoot(), "test/book.jpg"); + + assertEquals("jcr:content", getBundlingPath(appNode.getChildNode("jcr:content"))); + assertEquals("jcr:content/metadata", + getBundlingPath(appNode.getChildNode("jcr:content").getChildNode("metadata"))); + assertEquals("jcr:content/renditions/original", + getBundlingPath(appNode.getChildNode("jcr:content").getChildNode("renditions").getChildNode("original"))); + } @Test public void queryChildren() throws Exception{ @@ -469,6 +530,36 @@ } @Test + public void documentNodeStateBundledMethods() throws Exception{ + NodeBuilder builder = store.getRoot().builder(); + NodeBuilder appNB = newNode("app:Asset"); + createChild(appNB, + "jcr:content", + "jcr:content/comments", //not bundled + "jcr:content/metadata", + "jcr:content/metadata/xmp", //not bundled + "jcr:content/renditions", //includes all + "jcr:content/renditions/original", + "jcr:content/renditions/original/jcr:content" + ); + builder.child("test").setChildNode("book.jpg", appNB.getNodeState()); + + merge(builder); + DocumentNodeState appNode = (DocumentNodeState) getNode(store.getRoot(), "test/book.jpg"); + assertTrue(appNode.hasOnlyBundledChildren()); + assertEquals(ImmutableSet.of("jcr:content"), appNode.getBundledChildNodeNames()); + assertEquals(ImmutableSet.of("metadata", "renditions"), + TestUtils.asDocumentState(appNode.getChildNode("jcr:content")).getBundledChildNodeNames()); + + builder = store.getRoot().builder(); + childBuilder(builder, "/test/book.jpg/foo"); + merge(builder); + + DocumentNodeState appNode2 = (DocumentNodeState) getNode(store.getRoot(), "test/book.jpg"); + assertFalse(appNode2.hasOnlyBundledChildren()); + } + + @Test public void bundledDocsShouldNotBePartOfBackgroundUpdate() throws Exception{ NodeBuilder builder = store.getRoot().builder(); NodeBuilder fileNode = newNode("nt:file"); @@ -577,7 +668,7 @@ builder.child("test").setChildNode("book.jpg", fileNode.getNodeState()); merge(builder); - assertTrue(hasNodeProperty("/test/book.jpg", concat("jcr:content", DocumentBundlor.META_PROP_NODE))); + assertTrue(hasNodeProperty("/test/book.jpg", concat("jcr:content", DocumentBundlor.META_PROP_BUNDLING_PATH))); //Delete the bundled node structure builder = store.getRoot().builder(); @@ -600,8 +691,8 @@ merge(builder); //New pattern should be effective - assertTrue(hasNodeProperty("/test/book.jpg", concat("metadata", DocumentBundlor.META_PROP_NODE))); - assertFalse(hasNodeProperty("/test/book.jpg", concat("comments", DocumentBundlor.META_PROP_NODE))); + assertTrue(hasNodeProperty("/test/book.jpg", concat("metadata", DocumentBundlor.META_PROP_BUNDLING_PATH))); + assertFalse(hasNodeProperty("/test/book.jpg", concat("comments", DocumentBundlor.META_PROP_BUNDLING_PATH))); } @Test @@ -612,7 +703,7 @@ builder.child("test").setChildNode("book.jpg", fileNode.getNodeState()); merge(builder); - assertTrue(hasNodeProperty("/test/book.jpg", concat("jcr:content", DocumentBundlor.META_PROP_NODE))); + assertTrue(hasNodeProperty("/test/book.jpg", concat("jcr:content", DocumentBundlor.META_PROP_BUNDLING_PATH))); //Delete the bundled node structure builder = store.getRoot().builder(); @@ -636,8 +727,8 @@ //New pattern should be effective assertNotNull(getNodeDocument("/test/book.jpg")); - assertFalse(hasNodeProperty("/test/book.jpg", concat("metadata", DocumentBundlor.META_PROP_NODE))); - assertFalse(hasNodeProperty("/test/book.jpg", concat("comments", DocumentBundlor.META_PROP_NODE))); + assertFalse(hasNodeProperty("/test/book.jpg", concat("metadata", DocumentBundlor.META_PROP_BUNDLING_PATH))); + assertFalse(hasNodeProperty("/test/book.jpg", concat("comments", DocumentBundlor.META_PROP_BUNDLING_PATH))); } @Test @@ -672,6 +763,37 @@ assertTrue("No change reported for /test/book.jpg/jcr:content", addedPropertyNames.contains("fooContent")); } + @Test + public void bundledNodeAndDiffFew() throws Exception{ + store.dispose(); + disableJournalUse(); + store = builderProvider + .newBuilder() + .setDocumentStore(ds) + .memoryCacheSize(0) + .getNodeStore(); + + NodeBuilder builder = store.getRoot().builder(); + NodeBuilder fileNode = newNode("nt:file"); + fileNode.child("jcr:content").setProperty("jcr:data", "foo"); + builder.child("test").setChildNode("book.jpg", fileNode.getNodeState()); + merge(builder); + + //Make some modifications under the bundled node + //This would cause an entry for bundled node in Commit modified set + builder = store.getRoot().builder(); + childBuilder(builder, "/test/book.jpg/jcr:content").setProperty("foo", "bar"); + + TestNodeObserver o = new TestNodeObserver("test"); + store.addObserver(o); + merge(builder); + assertEquals(1, o.changed.size()); + } + + private static void disableJournalUse() { + System.setProperty(SYS_PROP_DISABLE_JOURNAL, "true"); + } + private void createTestNode(String path, NodeState state) throws CommitFailedException { String parentPath = PathUtils.getParentPath(path); NodeBuilder builder = store.getRoot().builder(); @@ -697,12 +819,9 @@ return store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); } - private static DocumentNodeState asDocumentState(NodeState state){ - if (state instanceof DocumentNodeState){ - return (DocumentNodeState) state; - } - fail("Not of type AbstractDoucmentNodeState"); - return null; + private static String getBundlingPath(NodeState contentNode) { + PropertyState ps = contentNode.getProperty(DocumentBundlor.META_PROP_BUNDLING_PATH); + return checkNotNull(ps).getValue(Type.STRING); } private static void dump(NodeState state){ @@ -713,27 +832,12 @@ return copyOf(getNode(state, path).getChildNodeNames()); } - private static NodeBuilder newNode(String typeName){ + static NodeBuilder newNode(String typeName){ NodeBuilder builder = EMPTY_NODE.builder(); builder.setProperty(JCR_PRIMARYTYPE, typeName); return builder; } - private static NodeBuilder createChild(NodeBuilder root, String ... paths){ - for (String path : paths){ - childBuilder(root, path); - } - return root; - } - - private static NodeBuilder childBuilder(NodeBuilder root, String path){ - NodeBuilder nb = root; - for (String nodeName : PathUtils.elements(path)){ - nb = nb.child(nodeName); - } - return nb; - } - private static class RecordingDocumentStore extends MemoryDocumentStore { final List queryPaths = new ArrayList<>(); final List findPaths = new ArrayList<>();