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 mr...@apache.org on 2019/09/18 15:51:41 UTC

svn commit: r1867136 - in /jackrabbit/oak/trunk/oak-store-document/src: main/java/org/apache/jackrabbit/oak/plugins/document/ test/java/org/apache/jackrabbit/oak/plugins/document/ test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/

Author: mreutegg
Date: Wed Sep 18 15:51:41 2019
New Revision: 1867136

URL: http://svn.apache.org/viewvc?rev=1867136&view=rev
Log:
OAK-8629: Node bundling exposes hidden property

More tests

Added:
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitDiffTest.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorConflictTest.java
    jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java

Modified: jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java?rev=1867136&r1=1867135&r2=1867136&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/UpdateOp.java Wed Sep 18 15:51:41 2019
@@ -20,6 +20,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 
 import com.google.common.collect.Maps;
 
@@ -452,6 +453,21 @@ public final class UpdateOp {
             return type + " " + value;
         }
 
+        @Override
+        public boolean equals(Object obj) {
+            if (obj instanceof Operation) {
+                Operation other = (Operation) obj;
+                return this.type.equals(other.type)
+                        && Objects.equals(this.value, other.value);
+            }
+            return false;
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(type, value);
+        }
+
         public Operation getReverse() {
             Operation reverse = null;
             switch (type) {

Added: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitDiffTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitDiffTest.java?rev=1867136&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitDiffTest.java (added)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitDiffTest.java Wed Sep 18 15:51:41 2019
@@ -0,0 +1,200 @@
+/*
+ * 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;
+
+import java.util.Map;
+
+import org.apache.jackrabbit.oak.InitialContent;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation;
+import org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingConfigInitializer;
+import org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingHandler;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.JcrConstants.JCR_CONTENT;
+import static org.apache.jackrabbit.JcrConstants.JCR_DATA;
+import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS;
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.merge;
+import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation.Type.SET_MAP_ENTRY;
+import static org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor.META_PROP_BUNDLED_CHILD;
+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.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
+import static org.hamcrest.Matchers.hasEntry;
+import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+public class CommitDiffTest {
+
+    @Rule
+    public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider();
+
+    private BundlingHandler bundlingHandler;
+
+    private DocumentNodeStore ns = builderProvider.newBuilder().build();
+
+    @Before
+    public void before() throws Exception {
+        NodeBuilder builder = ns.getRoot().builder();
+        new InitialContent().initialize(builder);
+        BundlingConfigInitializer.INSTANCE.initialize(builder);
+        merge(ns, builder);
+        ns.runBackgroundOperations();
+        bundlingHandler = ns.getBundlingConfigHandler().newBundlingHandler();
+    }
+
+    @Test
+    public void addBundlingRoot() {
+        CommitBuilder builder = new CommitBuilder(ns, null);
+        CommitDiff diff = newCommitDiff(builder);
+        NodeBuilder ntFile = newNtFile().builder();
+        ntFile.child(JCR_CONTENT).remove();
+        diff.childNodeAdded("file", ntFile.getNodeState());
+        assertEquals(1, diff.getNumChanges());
+        Commit c = builder.build(Revision.newRevision(1));
+        Revision r = c.getRevision();
+        UpdateOp op = c.getUpdateOperationForNode(Path.fromString("/file"));
+        Map<Key, Operation> changes = op.getChanges();
+        assertThat(changes.keySet(), hasSize(4));
+        assertThat(changes, hasKey(new Key("_deleted", r)));
+        assertThat(changes, hasKey(new Key(MODIFIED_IN_SECS, null)));
+        assertThat(changes, hasKey(new Key(JCR_PRIMARYTYPE, r)));
+        assertThat(changes, hasKey(new Key(META_PROP_PATTERN, r)));
+    }
+
+    @Test
+    public void addBundledNodeWithRoot() {
+        CommitBuilder builder = new CommitBuilder(ns, null);
+        CommitDiff diff = newCommitDiff(builder);
+        diff.childNodeAdded("file", newNtFile());
+        assertEquals(1, diff.getNumChanges());
+        Commit c = builder.build(Revision.newRevision(1));
+        Revision r = c.getRevision();
+        UpdateOp op = c.getUpdateOperationForNode(Path.fromString("/file"));
+        Map<Key, Operation> changes = op.getChanges();
+        assertThat(changes.keySet(), hasSize(8));
+        assertThat(changes, hasKey(new Key("_deleted", r)));
+        assertThat(changes, hasKey(new Key(MODIFIED_IN_SECS, null)));
+        assertThat(changes, hasKey(new Key(JCR_PRIMARYTYPE, r)));
+        assertThat(changes, hasKey(new Key(META_PROP_PATTERN, r)));
+        assertThat(changes, hasEntry(new Key(META_PROP_BUNDLED_CHILD, r), new Operation(SET_MAP_ENTRY, "true")));
+        // changes for jcr:content child node
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + JCR_PRIMARYTYPE, r)));
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + JCR_DATA, r)));
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + META_PROP_BUNDLING_PATH, r)));
+    }
+
+    @Ignore("OAK-8629")
+    @Test
+    public void removeBundledNodeWithRoot() throws Exception {
+        addTestFile();
+
+        CommitBuilder builder = new CommitBuilder(ns, null);
+        CommitDiff diff = newCommitDiff(builder);
+        diff.childNodeDeleted("file", ns.getRoot().getChildNode("file"));
+        assertEquals(1, diff.getNumChanges());
+        Commit c = builder.build(Revision.newRevision(1));
+        Revision r = c.getRevision();
+        UpdateOp op = c.getUpdateOperationForNode(Path.fromString("/file"));
+        Map<Key, Operation> changes = op.getChanges();
+        assertThat(changes.keySet(), hasSize(9));
+        assertThat(changes, hasKey(new Key("_deleted", r)));
+        assertThat(changes, hasKey(new Key("_deletedOnce", null)));
+        assertThat(changes, hasKey(new Key(MODIFIED_IN_SECS, null)));
+        assertThat(changes, hasKey(new Key(JCR_PRIMARYTYPE, r)));
+        assertThat(changes, hasEntry(new Key(META_PROP_PATTERN, r), new Operation(SET_MAP_ENTRY, null)));
+        assertThat(changes, hasEntry(new Key(META_PROP_BUNDLED_CHILD, r), new Operation(SET_MAP_ENTRY, null)));
+        // changes for jcr:content child node
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + JCR_PRIMARYTYPE, r)));
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + JCR_DATA, r)));
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + META_PROP_BUNDLING_PATH, r)));
+    }
+
+    @Ignore("OAK-8629")
+    @Test
+    public void removeBundledNode() throws Exception {
+        addTestFile();
+        NodeBuilder after = ns.getRoot().builder();
+        after.child("file").child(JCR_CONTENT).remove();
+
+        CommitBuilder builder = new CommitBuilder(ns, null);
+        CommitDiff diff = newCommitDiff(builder);
+        after.getNodeState().compareAgainstBaseState(ns.getRoot(), diff);
+
+        assertEquals(1, diff.getNumChanges());
+        Commit c = builder.build(Revision.newRevision(1));
+        Revision r = c.getRevision();
+        UpdateOp op = c.getUpdateOperationForNode(Path.fromString("/file"));
+        Map<Key, Operation> changes = op.getChanges();
+        assertThat(changes.keySet(), hasSize(4));
+        assertThat(changes, hasKey(new Key(MODIFIED_IN_SECS, null)));
+        // changes for jcr:content child node
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + JCR_PRIMARYTYPE, r)));
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + JCR_DATA, r)));
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + META_PROP_BUNDLING_PATH, r)));
+    }
+
+    @Test
+    public void changeBundledNode() throws Exception {
+        addTestFile();
+        NodeBuilder after = ns.getRoot().builder();
+        after.child("file").child(JCR_CONTENT).setProperty(JCR_DATA, "modified");
+
+        CommitBuilder builder = new CommitBuilder(ns, null);
+        CommitDiff diff = newCommitDiff(builder);
+        after.getNodeState().compareAgainstBaseState(ns.getRoot(), diff);
+
+        assertEquals(1, diff.getNumChanges());
+        Commit c = builder.build(Revision.newRevision(1));
+        Revision r = c.getRevision();
+        UpdateOp op = c.getUpdateOperationForNode(Path.fromString("/file"));
+        Map<Key, Operation> changes = op.getChanges();
+        assertThat(changes.keySet(), hasSize(2));
+        assertThat(changes, hasKey(new Key(MODIFIED_IN_SECS, null)));
+        // changes for jcr:content child node
+        assertThat(changes, hasKey(new Key(JCR_CONTENT + "/" + JCR_DATA, r)));
+    }
+
+    private void addTestFile() throws Exception {
+        NodeBuilder builder = ns.getRoot().builder();
+        builder.setChildNode("file", newNtFile());
+        merge(ns, builder);
+    }
+
+    private CommitDiff newCommitDiff(CommitBuilder builder) {
+        return new CommitDiff(bundlingHandler, builder, ns.getBlobSerializer());
+    }
+
+    private static NodeState newNtFile() {
+        NodeBuilder builder = EMPTY_NODE.builder();
+        builder.setProperty(JCR_PRIMARYTYPE, "nt:file", Type.NAME);
+        NodeBuilder content = builder.child("jcr:content");
+        content.setProperty(JCR_PRIMARYTYPE, "nt:resource", Type.NAME);
+        content.setProperty(JCR_DATA, "test");
+        return builder.getNodeState();
+
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitDiffTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorConflictTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorConflictTest.java?rev=1867136&r1=1867135&r2=1867136&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorConflictTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlorConflictTest.java Wed Sep 18 15:51:41 2019
@@ -34,6 +34,10 @@ import org.junit.rules.ExpectedException
 import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
 import static org.apache.jackrabbit.JcrConstants.NT_FILE;
 import static org.apache.jackrabbit.JcrConstants.NT_RESOURCE;
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.childBuilder;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 
 public class BundlorConflictTest {
 
@@ -113,6 +117,141 @@ public class BundlorConflictTest {
         merge(store2, root2);
     }
 
+    @Test
+    public void removeChangedNode() throws Exception {
+        // do not perform retries on merge
+        store2.setMaxBackOffMillis(0);
+
+        NodeBuilder builder = store1.getRoot().builder();
+        createFile(builder.child("test"), "book.jpg")
+                .child("jcr:content").setProperty("jcr:data", "test");
+        merge(store1, builder);
+
+        syncStores();
+
+        NodeBuilder b1 = store1.getRoot().builder();
+        NodeBuilder b2 = store2.getRoot().builder();
+
+        childBuilder(b1, "/test/book.jpg/jcr:content").setProperty("jcr:data", "modified");
+        merge(store1, b1);
+
+        childBuilder(b2, "/test/book.jpg/jcr:content").remove();
+        try {
+            merge(store2, b2);
+            fail("must fail with CommitFailedException");
+        } catch (CommitFailedException e) {
+            assertThat(e.getMessage(), containsString("ConflictException"));
+        }
+    }
+
+    @Test
+    public void changeRemovedNode() throws Exception {
+        // do not perform retries on merge
+        store2.setMaxBackOffMillis(0);
+
+        NodeBuilder builder = store1.getRoot().builder();
+        createFile(builder.child("test"), "book.jpg")
+                .child("jcr:content").setProperty("jcr:data", "test");
+        merge(store1, builder);
+
+        syncStores();
+
+        NodeBuilder b1 = store1.getRoot().builder();
+        NodeBuilder b2 = store2.getRoot().builder();
+
+        childBuilder(b1, "/test/book.jpg/jcr:content").remove();
+        merge(store1, b1);
+
+        childBuilder(b2, "/test/book.jpg/jcr:content").setProperty("jcr:data", "modified");
+        try {
+            merge(store2, b2);
+            fail("must fail with CommitFailedException");
+        } catch (CommitFailedException e) {
+            assertThat(e.getMessage(), containsString("ConflictException"));
+        }
+    }
+
+    @Test
+    public void removeRemovedNode() throws Exception {
+        // do not perform retries on merge
+        store2.setMaxBackOffMillis(0);
+
+        NodeBuilder builder = store1.getRoot().builder();
+        createFile(builder.child("test"), "book.jpg")
+                .child("jcr:content").setProperty("jcr:data", "test");
+        merge(store1, builder);
+
+        syncStores();
+
+        NodeBuilder b1 = store1.getRoot().builder();
+        NodeBuilder b2 = store2.getRoot().builder();
+
+        childBuilder(b1, "/test/book.jpg/jcr:content").remove();
+        merge(store1, b1);
+
+        childBuilder(b2, "/test/book.jpg/jcr:content").remove();
+        try {
+            merge(store2, b2);
+            fail("must fail with CommitFailedException");
+        } catch (CommitFailedException e) {
+            assertThat(e.getMessage(), containsString("ConflictException"));
+        }
+    }
+
+    @Test
+    public void addChildOnRemovedNode() throws Exception {
+        // do not perform retries on merge
+        store2.setMaxBackOffMillis(0);
+
+        NodeBuilder builder = store1.getRoot().builder();
+        createFile(builder.child("test"), "book.jpg")
+                .child("jcr:content").setProperty("jcr:data", "test");
+        merge(store1, builder);
+
+        syncStores();
+
+        NodeBuilder b1 = store1.getRoot().builder();
+        NodeBuilder b2 = store2.getRoot().builder();
+
+        childBuilder(b1, "/test/book.jpg/jcr:content").remove();
+        merge(store1, b1);
+
+        childBuilder(b2, "/test/book.jpg/jcr:content").child("extra");
+        try {
+            merge(store2, b2);
+            fail("must fail with CommitFailedException");
+        } catch (CommitFailedException e) {
+            assertThat(e.getMessage(), containsString("ConflictException"));
+        }
+    }
+
+    @Test
+    public void removeStaleTree() throws Exception {
+        // do not perform retries on merge
+        store2.setMaxBackOffMillis(0);
+
+        NodeBuilder builder = store1.getRoot().builder();
+        createFile(builder.child("test"), "book.jpg")
+                .child("jcr:content").setProperty("jcr:data", "test");
+        merge(store1, builder);
+
+        syncStores();
+
+        NodeBuilder b1 = store1.getRoot().builder();
+        NodeBuilder b2 = store2.getRoot().builder();
+
+        childBuilder(b1, "/test/book.jpg/jcr:content").child("extra");
+        merge(store1, b1);
+
+        childBuilder(b2, "/test/book.jpg/jcr:content").remove();
+        try {
+            merge(store2, b2);
+            fail("must fail with CommitFailedException");
+        } catch (CommitFailedException e) {
+            assertThat(e.getMessage(), containsString("ConflictException"));
+        }
+    }
+
     private static void merge(NodeStore store,
                               NodeBuilder root)
             throws CommitFailedException {

Modified: jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java?rev=1867136&r1=1867135&r2=1867136&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java (original)
+++ jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java Wed Sep 18 15:51:41 2019
@@ -397,6 +397,26 @@ public class DocumentBundlingTest {
         assertTrue(hasNodeProperty("/test/book.jpg", "_children"));
     }
 
+    @Test
+    public void hasChildren_BundledRoot_BundledChildRemoved() throws Exception{
+        createTestNode("/test/book.jpg", createChild(newNode("app:Asset"), "jcr:content").getNodeState());
+
+        ds.reset();
+
+        assertEquals(1, Iterables.size(getLatestNode("test/book.jpg").getChildNodeNames()));
+        assertEquals(0, ds.queryPaths.size());
+
+        NodeBuilder builder = store.getRoot().builder();
+        childBuilder(builder, "/test/book.jpg/jcr:content").remove();
+        merge(builder);
+
+        ds.reset();
+        assertEquals(0, Iterables.size(getLatestNode("test/book.jpg").getChildNodeNames()));
+        assertFalse(getLatestNode("test/book.jpg").hasChildNode("jcr:content"));
+        assertEquals(0, ds.queryPaths.size());
+
+        assertTrue(hasNodeProperty("/test/book.jpg", META_PROP_BUNDLED_CHILD));
+    }
 
     @Test
     public void hasChildren_BundledRoot_NonBundledChild() throws Exception{
@@ -814,10 +834,8 @@ public class DocumentBundlingTest {
     @Test
     public void deleteAndRecreateAsNonBundledNode() throws Exception {
         NodeBuilder builder = store.getRoot().builder();
-        NodeBuilder fileNode = newNode("nt:file");
-        NodeBuilder contentNode = fileNode.child("jcr:content");
-        contentNode.setProperty("jcr:data", "foo");
-        contentNode.child("extra");
+        NodeBuilder fileNode = newNtFileWithContent();
+        fileNode.child("jcr:content").child("extra");
         builder.child("test").setChildNode("book.jpg", fileNode.getNodeState());
         merge(builder);
 
@@ -841,11 +859,7 @@ public class DocumentBundlingTest {
 
     private Set<String> propertyNamesFor(String path) {
         Set<String> names = new HashSet<>();
-        NodeState state = store.getRoot();
-        for (String name : PathUtils.elements(path)) {
-            state = state.getChildNode(name);
-        }
-        for (PropertyState p : state.getProperties()) {
+        for (PropertyState p : getLatestNode(path).getProperties()) {
             names.add(p.getName());
         }
         return names;
@@ -1029,6 +1043,12 @@ public class DocumentBundlingTest {
         return builder;
     }
 
+    private static NodeBuilder newNtFileWithContent() {
+        NodeBuilder builder = newNode("nt:file");
+        builder.child("jcr:content").setProperty("jcr:data", "test");
+        return builder;
+    }
+
     private static class RecordingDocumentStore extends MemoryDocumentStore {
         final List<String> queryPaths = new ArrayList<>();
         final List<String> findPaths = new ArrayList<>();