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 ch...@apache.org on 2016/10/19 15:09:32 UTC

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

Author: chetanm
Date: Wed Oct 19 15:09:31 2016
New Revision: 1765633

URL: http://svn.apache.org/viewvc?rev=1765633&view=rev
Log:
OAK-1312 -  [bundling] Bundle nodes into a document

Adding support for hasChildren related optimizations. For each bundled node two meta boolean flags are maintained recording presence of either bundled child or non bundled child.

 If any child is added then corresponding flag is set. DocumentNodeState then rely on this to optimize children related calls

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/CommitDiff.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlingHandler.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlor.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/CommitDiff.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/CommitDiff.java?rev=1765633&r1=1765632&r2=1765633&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/CommitDiff.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/CommitDiff.java Wed Oct 19 15:09:31 2016
@@ -24,12 +24,14 @@ import org.apache.jackrabbit.oak.api.Typ
 import org.apache.jackrabbit.oak.json.BlobSerializer;
 import org.apache.jackrabbit.oak.json.JsonSerializer;
 import org.apache.jackrabbit.oak.plugins.document.bundlor.BundlingHandler;
+import org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
 import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE;
+import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty;
 
 /**
  * Implementation of a {@link NodeStateDiff}, which translates the diffs into
@@ -85,10 +87,10 @@ class CommitDiff implements NodeStateDif
     public boolean childNodeAdded(String name, NodeState after) {
         BundlingHandler child = bundlingHandler.childAdded(name, after);
         if (child.isBundlingRoot()) {
-            //TODO Handle case for leaf node optimization
             commit.addNode(new DocumentNodeState(store, child.getRootBundlePath(),
                     new RevisionVector(commit.getRevision())));
         }
+        setChildrenFlagOnAdd(child);
         return after.compareAgainstBaseState(EMPTY_NODE,
                 new CommitDiff(store, commit, child, builder, blobs));
     }
@@ -141,6 +143,30 @@ class CommitDiff implements NodeStateDif
         }
     }
 
+    private void setChildrenFlagOnAdd(BundlingHandler child) {
+        NodeState currentNode = bundlingHandler.getNodeState();
+        //Add hasChildren marker for bundling case
+        String propName = null;
+        if (child.isBundledNode()){
+            //1. Child is a bundled node. In that case current node would be part
+            //   same NodeDocument in which the child would be saved
+            propName = DocumentBundlor.META_PROP_BUNDLED_CHILD;
+        } else if (bundlingHandler.isBundledNode()){
+            //2. Child is a non bundled node but current node was bundled. This would
+            //   be the case where child node is not covered by bundling pattern. In
+            //   that case also add marker to current node
+            //   For case when current node is bundled  but is bundling root
+            //   this info is already captured in _hasChildren flag
+            propName = DocumentBundlor.META_PROP_NON_BUNDLED_CHILD;
+        }
+
+        //Avoid having multiple revision of same prop i.e. once
+        //child related flag is set its not touched
+        if (propName != null && !currentNode.hasProperty(propName)){
+            setProperty(createProperty(propName, Boolean.TRUE));
+        }
+    }
+
     private void setProperty(PropertyState property) {
         builder.resetWriter();
         JsonSerializer serializer = new JsonSerializer(builder, blobs);

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java?rev=1765633&r1=1765632&r2=1765633&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java Wed Oct 19 15:09:31 2016
@@ -57,6 +57,7 @@ import com.google.common.collect.Iterabl
 import com.google.common.collect.Iterators;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.commons.PathUtils.concat;
 import static org.apache.jackrabbit.oak.commons.StringUtils.estimateMemoryUsage;
 
 /**
@@ -110,15 +111,14 @@ public class DocumentNodeState extends A
                               boolean hasChildren,
                               @Nullable RevisionVector lastRevision,
                               boolean fromExternalChange) {
-        this(store, path, hasChildren, rootRevision, lastRevision,
-                fromExternalChange, createBundlingContext(checkNotNull(properties)));
+        this(store, path, rootRevision, lastRevision,
+                fromExternalChange, createBundlingContext(checkNotNull(properties), hasChildren));
     }
 
     private DocumentNodeState(@Nonnull DocumentNodeStore store,
                               @Nonnull String path,
-                              boolean hasChildren,
-                              @Nonnull RevisionVector rootRevision,
                               @Nullable RevisionVector lastRevision,
+                              @Nullable RevisionVector rootRevision,
                               boolean fromExternalChange,
                               BundlingContext bundlingContext) {
         this.store = checkNotNull(store);
@@ -128,10 +128,7 @@ public class DocumentNodeState extends A
         this.fromExternalChange = fromExternalChange;
         this.properties = bundlingContext.getProperties();
         this.bundlingContext = bundlingContext;
-
-        //TODO [bundling] hasChildren optimization
-        this.hasChildren = this.bundlingContext.matcher.isMatch() || hasChildren;
-
+        this.hasChildren = this.bundlingContext.hasChildren();
     }
 
     /**
@@ -258,7 +255,7 @@ public class DocumentNodeState extends A
         }
 
         int bundledChildCount = bundlingContext.getBundledChildNodeNames().size();
-        if (bundlingContext.matcher.matchesAllChildren()){
+        if (bundlingContext.hasOnlyBundledChildren()){
             return bundledChildCount;
         }
 
@@ -295,7 +292,8 @@ public class DocumentNodeState extends A
         return new Iterable<ChildNodeEntry>() {
             @Override
             public Iterator<ChildNodeEntry> iterator() {
-                if (bundlingContext.matcher.matchesAllChildren()){
+                //If all the children are bundled
+                if (bundlingContext.hasOnlyBundledChildren()){
                     return getBundledChildren();
                 }
                 return Iterators.concat(getBundledChildren(), new ChildNodeEntryIterator());
@@ -452,7 +450,7 @@ public class DocumentNodeState extends A
             }
         }
 
-        return store.getNode(PathUtils.concat(getPath(), childNodeName), lastRevision);
+        return store.getNode(concat(getPath(), childNodeName), lastRevision);
     }
 
     @CheckForNull
@@ -712,8 +710,7 @@ public class DocumentNodeState extends A
     private AbstractDocumentNodeState createBundledState(String childNodeName, Matcher child) {
         return new DocumentNodeState(
                 store,
-                PathUtils.concat(path, childNodeName),
-                true, //TODO Determine child node
+                concat(path, childNodeName),
                 lastRevision,
                 rootRevision,
                 fromExternalChange,
@@ -742,26 +739,40 @@ public class DocumentNodeState extends A
         });
     }
 
-    private static BundlingContext createBundlingContext(Map<String, PropertyState> properties) {
+    private static BundlingContext createBundlingContext(Map<String, PropertyState> properties,
+                                                         boolean hasNonBundledChildren) {
         PropertyState bundlorConfig = properties.get(DocumentBundlor.META_PROP_PATTERN);
         Matcher matcher = Matcher.NON_MATCHING;
+        boolean hasBundledChildren = false;
         if (bundlorConfig != null){
             matcher = DocumentBundlor.from(bundlorConfig).createMatcher();
+            hasBundledChildren = hasBundledProperty(properties, matcher, DocumentBundlor.META_PROP_BUNDLED_CHILD);
         }
-        return new BundlingContext(matcher, properties);
+        return new BundlingContext(matcher, properties, hasBundledChildren, hasNonBundledChildren);
+    }
+
+    private static boolean hasBundledProperty(Map<String, PropertyState> props, Matcher matcher, String propName){
+        String key = concat(matcher.getMatchedPath(), propName);
+        return props.containsKey(key);
     }
 
     private static class BundlingContext {
         final Matcher matcher;
         final Map<String, PropertyState> rootProperties;
+        final boolean hasBundledChildren;
+        final boolean hasNonBundledChildren;
 
-        public BundlingContext(Matcher matcher, Map<String, PropertyState> rootProperties) {
+        public BundlingContext(Matcher matcher, Map<String, PropertyState> rootProperties,
+                               boolean hasBundledChildren, boolean hasNonBundledChildren) {
             this.matcher = matcher;
             this.rootProperties = rootProperties;
+            this.hasBundledChildren = hasBundledChildren;
+            this.hasNonBundledChildren = hasNonBundledChildren;
         }
 
         public BundlingContext childContext(Matcher childMatcher){
-            return new BundlingContext(childMatcher, rootProperties);
+            return new BundlingContext(childMatcher, rootProperties,
+                    hasBundledChildren(childMatcher), hasNonBundledChildren(childMatcher));
         }
 
         public Map<String, PropertyState> getProperties(){
@@ -772,15 +783,38 @@ public class DocumentNodeState extends A
         }
 
         public boolean hasChildNode(String relativePath){
-            String key = PathUtils.concat(relativePath, DocumentBundlor.META_PROP_NODE);
+            String key = concat(relativePath, DocumentBundlor.META_PROP_NODE);
             return rootProperties.containsKey(key);
         }
 
+        public boolean hasChildren(){
+            return hasNonBundledChildren || hasBundledChildren;
+        }
+
+        public boolean hasOnlyBundledChildren(){
+            return !hasNonBundledChildren;
+        }
+
         public List<String> getBundledChildNodeNames(){
             if (matcher.isMatch()) {
                 return BundlorUtils.getChildNodeNames(rootProperties.keySet(), matcher);
             }
             return Collections.emptyList();
         }
+
+        private boolean hasBundledChildren(Matcher matcher){
+            if (matcher.isMatch()){
+                return hasBundledProperty(rootProperties, matcher, DocumentBundlor.META_PROP_BUNDLED_CHILD);
+            }
+            return false;
+        }
+
+        private boolean hasNonBundledChildren(Matcher matcher){
+            if (matcher.isMatch()){
+                return hasBundledProperty(rootProperties, matcher, DocumentBundlor.META_PROP_NON_BUNDLED_CHILD);
+            }
+            return false;
+        }
+
     }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlingHandler.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlingHandler.java?rev=1765633&r1=1765632&r2=1765633&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlingHandler.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/BundlingHandler.java Wed Oct 19 15:09:31 2016
@@ -28,6 +28,7 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 
 import static org.apache.jackrabbit.oak.commons.PathUtils.ROOT_PATH;
+import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
 
 public class BundlingHandler {
     /**
@@ -39,17 +40,23 @@ public class BundlingHandler {
     private final BundledTypesRegistry registry;
     private final String path;
     private final BundlingContext ctx;
+    private final NodeState nodeState;
 
     public BundlingHandler(BundledTypesRegistry registry) {
-        this(registry, BundlingContext.NULL, ROOT_PATH);
+        this(registry, BundlingContext.NULL, ROOT_PATH, EMPTY_NODE);
     }
 
-    private BundlingHandler(BundledTypesRegistry registry, BundlingContext ctx, String path) {
+    private BundlingHandler(BundledTypesRegistry registry, BundlingContext ctx, String path, NodeState nodeState) {
         this.registry = registry;
         this.path = path;
         this.ctx = ctx;
+        this.nodeState = nodeState;
     }
 
+    /**
+     * Returns property path. For non bundling case this is the actual property name
+     * while for bundling case this is the relative path from bundling root
+     */
     public String getPropertyPath(String propertyName) {
         return ctx.isBundling() ? ctx.getPropertyPath(propertyName) : propertyName;
     }
@@ -68,10 +75,17 @@ public class BundlingHandler {
         return path;
     }
 
+    public NodeState getNodeState() {
+        return nodeState;
+    }
+
     public Set<PropertyState> getMetaProps() {
         return ctx.metaProps;
     }
 
+    /**
+     * Returns name of properties which needs to be removed or marked as deleted
+     */
     public Set<String> getRemovedProps(){
         return ctx.removedProps;
     }
@@ -97,7 +111,7 @@ public class BundlingHandler {
                 childContext = BundlingContext.NULL;
             }
         }
-        return new BundlingHandler(registry, childContext, childPath);
+        return new BundlingHandler(registry, childContext, childPath, state);
     }
 
     public BundlingHandler childDeleted(String name, NodeState state){
@@ -106,14 +120,11 @@ public class BundlingHandler {
         Matcher childMatcher = ctx.matcher.next(name);
         if (childMatcher.isMatch()) {
             childContext = createChildContext(childMatcher);
-            childContext.removeProperty(DocumentBundlor.META_PROP_NODE);
-            for (PropertyState ps : state.getProperties()){
-                childContext.removeProperty(ps.getName());
-            }
+            removeDeletedChildProperties(state, childContext);
         } else {
             childContext = getBundlorContext(childPath, state);
         }
-        return new BundlingHandler(registry, childContext, childPath);
+        return new BundlingHandler(registry, childContext, childPath, state);
     }
 
     public BundlingHandler childChanged(String name, NodeState state){
@@ -126,7 +137,7 @@ public class BundlingHandler {
             childContext = getBundlorContext(childPath, state);
         }
 
-        return new BundlingHandler(registry, childContext,  childPath);
+        return new BundlingHandler(registry, childContext,  childPath, state);
     }
 
     public boolean isBundlingRoot() {
@@ -154,6 +165,15 @@ public class BundlingHandler {
         return result;
     }
 
+    private static void removeDeletedChildProperties(NodeState state, BundlingContext childContext) {
+        childContext.removeProperty(DocumentBundlor.META_PROP_NODE);
+        for (PropertyState ps : state.getProperties()){
+            String propName = ps.getName();
+            if (!propName.startsWith(DocumentBundlor.HAS_CHILD_PROP_PREFIX))
+                childContext.removeProperty(ps.getName());
+        }
+    }
+
     private static class BundlingContext {
         static final BundlingContext NULL = new BundlingContext("", Matcher.NON_MATCHING);
         final String bundlingPath;

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlor.java?rev=1765633&r1=1765632&r2=1765633&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlor.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlor.java Wed Oct 19 15:09:31 2016
@@ -36,15 +36,32 @@ import static org.apache.jackrabbit.oak.
 public class DocumentBundlor {
     /**
      * Hidden property to store the pattern as part of NodeState
+     * TODO - Also store the NodeType
      */
-    public static final String META_PROP_PATTERN = ":pattern";
+    public static final String META_PROP_PATTERN = ":doc-pattern";
 
     /**
      * 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
      */
-    public static final String META_PROP_NODE = ":self";
+    public static final String META_PROP_NODE = ":doc-self";
+
+    public static final String HAS_CHILD_PROP_PREFIX = ":doc-has-child-";
+
+    /**
+     * Hidden property name having boolean value indicating that
+     * current node has children which are bundled
+     */
+    public static final String META_PROP_BUNDLED_CHILD = HAS_CHILD_PROP_PREFIX + "bundled";
+
+    /**
+     * Hidden property name having boolean value indicating that
+     * current node has children which are not bundled
+     */
+    public static final String META_PROP_NON_BUNDLED_CHILD = HAS_CHILD_PROP_PREFIX + "non-bundled";
+
+
 
     public static final String PROP_PATTERN = "pattern";
     private final List<Include> includes;

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java?rev=1765633&r1=1765632&r2=1765633&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/bundlor/DocumentBundlingTest.java Wed Oct 19 15:09:31 2016
@@ -25,8 +25,8 @@ import java.util.Set;
 
 import javax.annotation.Nonnull;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.commons.PathUtils;
@@ -34,6 +34,7 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.plugins.document.Document;
 import org.apache.jackrabbit.oak.plugins.document.DocumentMKBuilderProvider;
 import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore;
+import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
 import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
@@ -47,11 +48,17 @@ import org.junit.Rule;
 import org.junit.Test;
 
 import static com.google.common.collect.ImmutableList.copyOf;
+import static java.lang.String.format;
 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.bundlor.DocumentBundlor.META_PROP_BUNDLED_CHILD;
 import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
 import static org.apache.jackrabbit.oak.spi.state.NodeStateUtils.getNode;
 import static org.hamcrest.CoreMatchers.hasItems;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
@@ -80,7 +87,7 @@ public class DocumentBundlingTest {
 
         NodeBuilder builder = store.getRoot().builder();
         builder.child("jcr:system").child("documentstore").setChildNode("bundlor", registryState);
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
     }
 
     @Test
@@ -90,18 +97,21 @@ public class DocumentBundlingTest {
         fileNode.child("jcr:content").setProperty("jcr:data", "foo");
         builder.child("test").setChildNode("book.jpg", fileNode.getNodeState());
 
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
-
-        System.out.println(store.getDocumentStore().find(Collection.NODES, "2:/test/book.jpg").asString());
+        merge(builder);
 
         NodeState root = store.getRoot();
         NodeState fileNodeState = root.getChildNode("test");
         assertTrue(fileNodeState.getChildNode("book.jpg").exists());
         assertTrue(fileNodeState.getChildNode("book.jpg").getChildNode("jcr:content").exists());
 
-        assertTrue(PartialEqualsDiff.equals(fileNode.getNodeState(), fileNodeState.getChildNode("book.jpg")));
+        assertNull(getNodeDocument("/test/book.jpg/jcr:content"));
+        assertNotNull(getNodeDocument("/test/book.jpg"));
+
+        AssertingDiff.assertEquals(fileNode.getNodeState(), fileNodeState.getChildNode("book.jpg"));
     }
+
     //TODO Test _bin being set
+    //TODO Test with asserts on expected properties present in NodeDocument like :self etc
 
     @Test
     public void bundledParent() throws Exception{
@@ -114,7 +124,7 @@ public class DocumentBundlingTest {
         dump(appNB.getNodeState());
         builder.child("test").setChildNode("book.jpg", appNB.getNodeState());
 
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
     }
     
     @Test
@@ -132,14 +142,14 @@ public class DocumentBundlingTest {
         );
         builder.child("test").setChildNode("book.jpg", appNB.getNodeState());
 
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
         NodeState appNode = getNode(store.getRoot(), "test/book.jpg");
 
         ds.reset();
 
         int childCount = Iterables.size(appNode.getChildNodeEntries());
         assertEquals(1, childCount);
-        assertEquals(1, ds.queryPaths.size());
+        assertEquals(0, ds.queryPaths.size());
 
         assertThat(childNames(appNode, "jcr:content"), hasItems("comments", "metadata", "renditions"));
         assertEquals(3, getNode(appNode, "jcr:content").getChildNodeCount(100));
@@ -157,7 +167,146 @@ public class DocumentBundlingTest {
         assertEquals(1, getNode(appNode, "jcr:content/renditions/original").getChildNodeCount(100));
         assertEquals(0, ds.queryPaths.size());
 
-        assertTrue(PartialEqualsDiff.equals(appNB.getNodeState(), appNode));
+        AssertingDiff.assertEquals(appNB.getNodeState(), appNode);
+    }
+
+    @Test
+    public void hasChildren() throws Exception{
+        createTestNode("/test/book.jpg", createChild(newNode("app:Asset"),
+                "jcr:content",
+                "jcr:content/comments", //not bundled
+                "jcr:content/metadata"
+        ).getNodeState());
+
+        ds.reset();
+
+        assertEquals(0, Iterables.size(getLatestNode("test/book.jpg/jcr:content/metadata").getChildNodeNames()));
+        assertEquals(0, ds.queryPaths.size());
+
+        //Case 1 - Bundled root but no bundled child
+        //Case 2 - Bundled root but non bundled child
+        //Case 3 - Bundled root but  bundled child
+        //Case 3 - Bundled node but  no bundled child
+        //Case 3 - Bundled leaf node but child can be present in non bundled form
+        //Case 3 - Bundled leaf node but all child bundled
+    }
+
+    @Test
+    public void hasChildren_BundledRoot_NoChild() throws Exception{
+        createTestNode("/test/book.jpg", createChild(newNode("app:Asset")).getNodeState());
+
+        ds.reset();
+
+        assertEquals(0, Iterables.size(getLatestNode("test/book.jpg").getChildNodeNames()));
+        assertEquals(0, getLatestNode("test/book.jpg").getChildNodeCount(100));
+        assertEquals(0, ds.queryPaths.size());
+
+        NodeBuilder builder = store.getRoot().builder();
+        childBuilder(builder, "/test/book.jpg/jcr:content");
+        merge(builder);
+
+        ds.reset();
+        assertEquals(1, Iterables.size(getLatestNode("test/book.jpg").getChildNodeNames()));
+        assertEquals(1, getLatestNode("test/book.jpg").getChildNodeCount(100));
+        assertEquals(0, ds.queryPaths.size());
+        assertTrue(hasNodeProperty("/test/book.jpg", META_PROP_BUNDLED_CHILD));
+        assertFalse(hasNodeProperty("/test/book.jpg", "_children"));
+    }
+
+    @Test
+    public void hasChildren_BundledRoot_BundledChild() 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/fooContent");
+        merge(builder);
+
+        ds.reset();
+        assertEquals(2, Iterables.size(getLatestNode("test/book.jpg").getChildNodeNames()));
+        assertEquals(1, ds.queryPaths.size());
+
+        assertTrue(hasNodeProperty("/test/book.jpg", META_PROP_BUNDLED_CHILD));
+        assertTrue(hasNodeProperty("/test/book.jpg", "_children"));
+    }
+
+
+    @Test
+    public void hasChildren_BundledRoot_NonBundledChild() throws Exception{
+        createTestNode("/test/book.jpg", createChild(newNode("app:Asset"), "fooContent").getNodeState());
+
+        ds.reset();
+
+        assertEquals(1, Iterables.size(getLatestNode("test/book.jpg").getChildNodeNames()));
+        assertEquals(1, ds.queryPaths.size());
+
+        assertFalse(hasNodeProperty("/test/book.jpg", META_PROP_BUNDLED_CHILD));
+        assertTrue(hasNodeProperty("/test/book.jpg", "_children"));
+    }
+
+    @Test
+    public void hasChildren_BundledNode_NoChild() throws Exception{
+        createTestNode("/test/book.jpg", createChild(newNode("app:Asset"),
+                "jcr:content"
+        ).getNodeState());
+
+        ds.reset();
+
+        assertEquals(0, Iterables.size(getLatestNode("test/book.jpg/jcr:content").getChildNodeNames()));
+        assertEquals(0, ds.queryPaths.size());
+
+        NodeBuilder builder = store.getRoot().builder();
+        childBuilder(builder, "/test/book.jpg/jcr:content/metadata");
+        merge(builder);
+
+        ds.reset();
+        assertEquals(1, Iterables.size(getLatestNode("test/book.jpg/jcr:content").getChildNodeNames()));
+        assertEquals(0, ds.queryPaths.size());
+    }
+
+    @Test
+    public void hasChildren_BundledLeafNode_NoChild() throws Exception{
+        createTestNode("/test/book.jpg", createChild(newNode("app:Asset"),
+                "jcr:content/metadata"
+        ).getNodeState());
+
+        ds.reset();
+
+        assertEquals(0, Iterables.size(getLatestNode("test/book.jpg/jcr:content/metadata").getChildNodeNames()));
+        assertEquals(0, ds.queryPaths.size());
+
+        NodeBuilder builder = store.getRoot().builder();
+        childBuilder(builder, "/test/book.jpg/jcr:content/metadata/xmp");
+        merge(builder);
+
+        ds.reset();
+        assertEquals(1, Iterables.size(getLatestNode("test/book.jpg/jcr:content/metadata").getChildNodeNames()));
+        assertEquals(1, ds.queryPaths.size());
+    }
+
+    @Test
+    public void hasChildren_SingleVersion() throws Exception{
+        createTestNode("/test/book.jpg", createChild(newNode("app:Asset"),
+                "jcr:content/metadata"
+        ).getNodeState());
+
+        assertEquals(1, getNodeDocument("/test/book.jpg").getValueMap(concat("jcr:content", META_PROP_BUNDLED_CHILD)).size());
+
+        NodeBuilder builder = store.getRoot().builder();
+        childBuilder(builder, "/test/book.jpg/jcr:content/renditions");
+        merge(builder);
+
+        ds.reset();
+
+        //Assert that there is only one revision for meta :doc-has-child-bundled
+        //Even on adding multiple child node there is only version kept
+        assertEquals(1, getNodeDocument("/test/book.jpg").getValueMap(concat("jcr:content", META_PROP_BUNDLED_CHILD)).size());
+        assertEquals(2, Iterables.size(getLatestNode("test/book.jpg/jcr:content").getChildNodeNames()));
+        assertEquals(0, ds.queryPaths.size());
     }
 
     @Test
@@ -175,17 +324,17 @@ public class DocumentBundlingTest {
         );
         builder.child("test").setChildNode("book.jpg", appNB.getNodeState());
 
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
         builder = store.getRoot().builder();
         NodeBuilder renditions = childBuilder(builder, "/test/book.jpg/jcr:content/renditions");
         renditions.child("small").child("jcr:content");
         NodeState appNode_v2 = childBuilder(builder, "/test/book.jpg").getNodeState();
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
         assertThat(childNames(getLatestNode("/test/book.jpg"), "jcr:content/renditions"),
                 hasItems("original", "small"));
-        assertTrue(PartialEqualsDiff.equals(getLatestNode("/test/book.jpg"), appNode_v2));
+        assertTrue(AssertingDiff.assertEquals(getLatestNode("/test/book.jpg"), appNode_v2));
     }
 
     @Test
@@ -203,34 +352,34 @@ public class DocumentBundlingTest {
         );
         builder.child("test").setChildNode("book.jpg", appNB.getNodeState());
 
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
         //Modify bundled property
         builder = store.getRoot().builder();
         childBuilder(builder, "/test/book.jpg/jcr:content").setProperty("foo", "bar");
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
         NodeState state = childBuilder(builder, "/test/book.jpg").getNodeState();
         assertEquals("bar", getLatestNode("/test/book.jpg/jcr:content").getString("foo"));
-        assertTrue(PartialEqualsDiff.equals(state, getLatestNode("/test/book.jpg")));
+        assertTrue(AssertingDiff.assertEquals(state, getLatestNode("/test/book.jpg")));
 
         //Modify deep bundled property
         builder = store.getRoot().builder();
         childBuilder(builder, "/test/book.jpg/jcr:content/renditions").setProperty("foo", "bar");
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
         state = childBuilder(builder, "/test/book.jpg").getNodeState();
         assertEquals("bar", getLatestNode("/test/book.jpg/jcr:content/renditions").getString("foo"));
-        assertTrue(PartialEqualsDiff.equals(state, getLatestNode("/test/book.jpg")));
+        assertTrue(AssertingDiff.assertEquals(state, getLatestNode("/test/book.jpg")));
 
         //Modify deep unbundled property - jcr:content/comments/@foo
         builder = store.getRoot().builder();
         childBuilder(builder, "/test/book.jpg/jcr:content/comments").setProperty("foo", "bar");
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
         state = childBuilder(builder, "/test/book.jpg").getNodeState();
         assertEquals("bar", getLatestNode("/test/book.jpg/jcr:content/comments").getString("foo"));
-        assertTrue(PartialEqualsDiff.equals(state, getLatestNode("/test/book.jpg")));
+        assertTrue(AssertingDiff.assertEquals(state, getLatestNode("/test/book.jpg")));
     }
 
     @Test
@@ -251,31 +400,52 @@ public class DocumentBundlingTest {
         childBuilder(appNB, "jcr:content/comments").setProperty("foo", "bar");
         builder.child("test").setChildNode("book.jpg", appNB.getNodeState());
 
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
         //Delete a bundled node jcr:content/metadata
         builder = store.getRoot().builder();
         childBuilder(builder, "/test/book.jpg/jcr:content/metadata").remove();
         NodeState appNode_v2 = childBuilder(builder, "/test/book.jpg").getNodeState();
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
-        assertTrue(PartialEqualsDiff.equals(appNode_v2, getLatestNode("/test/book.jpg")));
+        assertTrue(AssertingDiff.assertEquals(appNode_v2, getLatestNode("/test/book.jpg")));
 
 
         //Delete unbundled child jcr:content/comments
         builder = store.getRoot().builder();
         childBuilder(builder, "/test/book.jpg/jcr:content/comments").remove();
         NodeState appNode_v3 = childBuilder(builder, "/test/book.jpg").getNodeState();
-        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+        merge(builder);
 
-        assertTrue(PartialEqualsDiff.equals(appNode_v3, getLatestNode("/test/book.jpg")));
+        assertTrue(AssertingDiff.assertEquals(appNode_v3, getLatestNode("/test/book.jpg")));
 
     }
 
+    private void createTestNode(String path, NodeState state) throws CommitFailedException {
+        String parentPath = PathUtils.getParentPath(path);
+        NodeBuilder builder = store.getRoot().builder();
+        NodeBuilder parent = childBuilder(builder, parentPath);
+        parent.setChildNode(PathUtils.getName(path), state);
+        merge(builder);
+    }
+
     private NodeState getLatestNode(String path){
         return getNode(store.getRoot(), path);
     }
 
+    private boolean hasNodeProperty(String path, String propName) {
+        NodeDocument doc = getNodeDocument(path);
+        return doc.keySet().contains(propName);
+    }
+
+    private NodeDocument getNodeDocument(String path) {
+        return ds.find(Collection.NODES, Utils.getIdFromPath(path));
+    }
+
+    private void merge(NodeBuilder builder) throws CommitFailedException {
+        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+    }
+
     private static void dump(NodeState state){
         System.out.println(NodeStateUtils.toString(state));
     }
@@ -333,30 +503,44 @@ public class DocumentBundlingTest {
         }
     }
 
-    private static class PartialEqualsDiff extends EqualsDiff {
-        private final Set<String> ignoredProps = Sets.newHashSet(DocumentBundlor.META_PROP_PATTERN);
+    private static class AssertingDiff extends EqualsDiff {
+        private final Set<String> ignoredProps = ImmutableSet.of(
+                DocumentBundlor.META_PROP_PATTERN,
+                META_PROP_BUNDLED_CHILD,
+                DocumentBundlor.META_PROP_NON_BUNDLED_CHILD
+        );
 
-        public static boolean equals(NodeState before, NodeState after) {
+        public static boolean assertEquals(NodeState before, NodeState after) {
             return before.exists() == after.exists()
-                    && after.compareAgainstBaseState(before, new PartialEqualsDiff());
+                    && after.compareAgainstBaseState(before, new AssertingDiff());
         }
 
         @Override
         public boolean propertyAdded(PropertyState after) {
             if (ignore(after)) return true;
-            return super.propertyAdded(after);
+            throw new AssertionError("Added property: " + after);
         }
 
         @Override
         public boolean propertyChanged(PropertyState before, PropertyState after) {
             if (ignore(after)) return true;
-            return super.propertyChanged(before, after);
+            throw new AssertionError("Property changed: Before " + before + " , after " + after);
         }
 
         @Override
         public boolean propertyDeleted(PropertyState before) {
             if (ignore(before)) return true;
-            return super.propertyDeleted(before);
+            throw new AssertionError("Deleted property: " + before);
+        }
+
+        @Override
+        public boolean childNodeAdded(String name, NodeState after) {
+            throw new AssertionError(format("Added child: %s -  %s", name, after));
+        }
+
+        @Override
+        public boolean childNodeDeleted(String name, NodeState before) {
+            throw new AssertionError(format("Deleted child : %s -  %s", name, before));
         }
 
         private boolean ignore(PropertyState state){