Author: chetanm
Date: Fri Nov 11 12:13:24 2016
New Revision: 1769278
URL: http://svn.apache.org/viewvc?rev=1769278&view=rev
Log:
OAK-5079 - Diff would not work for bundled nodes when done without journal
support
Actual fix - diff impl now delegates to BundledDocumentDiffer which understand
Node bundling and provides correct diff
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java
(with props)
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java
(with props)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1769278&r1=1769277&r2=1769278&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
Fri Nov 11 12:13:24 2016
@@ -86,6 +86,7 @@ import org.apache.jackrabbit.oak.plugins
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;
@@ -432,6 +433,8 @@ public final class DocumentNodeStore
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();
@@ -915,7 +918,7 @@ public final class DocumentNodeStore
* given revision.
*/
@CheckForNull
- DocumentNodeState getNode(@Nonnull final String path,
+ public DocumentNodeState getNode(@Nonnull final String path,
@Nonnull final RevisionVector rev) {
checkNotNull(rev);
checkNotNull(path);
@@ -2278,30 +2281,36 @@ public final class DocumentNodeStore
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();
- 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);
+ boolean continueDiff = bundledDocDiffer.diff(from, to, w);
+
+ 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();
}
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java?rev=1769278&view=auto
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java
(added)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java
Fri Nov 11 12:13:24 2016
@@ -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. {@link 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<String> fromChildren, Collection<String>
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,
bundlingRootPath);
+ } else {
+ result = nodeStore.getNode(state.getPath(),
state.getLastRevision());
+ }
+
+ checkNotNull(result, "Node at [%s] not found for fromRev [%s]",
state.getPath(), state.getLastRevision());
+ return result;
+ }
+}
Propchange:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDiffer.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java?rev=1769278&r1=1769277&r2=1769278&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java
Fri Nov 11 12:13:24 2016
@@ -23,6 +23,7 @@ import javax.annotation.Nullable;
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;
@@ -60,4 +61,19 @@ public class TestUtils {
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;
+ }
}
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java?rev=1769278&view=auto
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java
(added)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java
Fri Nov 11 12:13:24 2016
@@ -0,0 +1,145 @@
+/*
+ * 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 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.DocumentNodeStore;
+import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent;
+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
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.bundlor.DocumentBundlingTest.asDocumentState;
+import static
org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlingTest.newNode;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class BundledDocumentDifferTest {
+ @Rule
+ public DocumentMKBuilderProvider builderProvider = new
DocumentMKBuilderProvider();
+ private DocumentNodeStore store;
+ private String journalDisabledProp;
+ private BundledDocumentDiffer differ;
+
+
+ @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());
+ }
+
+ 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 AbstractDocumentNodeState dns(NodeState root, String path){
+ return asDocumentState(NodeStateUtils.getNode(root, path));
+ }
+
+
+}
\ No newline at end of file
Propchange:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundledDocumentDifferTest.java
------------------------------------------------------------------------------
svn:eol-style = native
