diff --git oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java index 164aa78..2413160 100644 --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java @@ -23,7 +23,6 @@ import static org.apache.jackrabbit.oak.commons.PropertiesUtil.toInteger; import static org.apache.jackrabbit.oak.commons.PropertiesUtil.toBoolean; import java.util.Map; -import java.util.Set; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -45,8 +44,6 @@ import org.osgi.service.component.ComponentContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.collect.Iterables; - /** * A ChangeCollectorProvider can be hooked into Oak thus enabling the collection * of ChangeSets of changed items of a commit, which downstream Observers can @@ -99,11 +96,11 @@ public class ChangeCollectorProvider extends ValidatorProvider { * can remain unsynchronized as validators are executed single-threaded. */ private static class CollectorSupport { - private final CommitInfo info; - private final int maxPathDepth; - private final ChangeSetBuilder changeSetBuilder; + final CommitInfo info; + final int maxPathDepth; + final ChangeSetBuilder changeSetBuilder; - private CollectorSupport(@Nonnull CommitInfo info, @Nonnull ChangeSetBuilder changeSetBuilder, + public CollectorSupport(@Nonnull CommitInfo info, @Nonnull ChangeSetBuilder changeSetBuilder, int maxPathDepth) { this.info = info; this.changeSetBuilder = changeSetBuilder; @@ -114,39 +111,6 @@ public class ChangeCollectorProvider extends ValidatorProvider { public String toString() { return "CollectorSupport with " + changeSetBuilder; } - - private CommitInfo getInfo() { - return info; - } - - private int getMaxPathDepth() { - return maxPathDepth; - } - - private ChangeSetBuilder getChangeSetBuilder() { - return changeSetBuilder; - } - - private Set getParentPaths() { - return changeSetBuilder.getParentPaths(); - } - - private Set getParentNodeNames() { - return changeSetBuilder.getParentNodeNames(); - } - - private Set getParentNodeTypes() { - return changeSetBuilder.getParentNodeTypes(); - } - - private Set getPropertyNames() { - return changeSetBuilder.getPropertyNames(); - } - - private Set getAllNodeTypes() { - return changeSetBuilder.getAllNodeTypes(); - } - } /** @@ -206,25 +170,16 @@ public class ChangeCollectorProvider extends ValidatorProvider { @Override public void leave(NodeState before, NodeState after) throws CommitFailedException { // first check if we have to add anything to paths and/or nodeNames - if (changed && level <= support.getMaxPathDepth()) { - support.getParentPaths().add(path); + if (changed && level <= support.maxPathDepth) { + support.changeSetBuilder.addParentPath(path); } if (changed && childName != null) { - support.getParentNodeNames().add(childName); - } - if (changed && beforeParentNodeOrNull != null) { - String primaryType = beforeParentNodeOrNull.getName(JcrConstants.JCR_PRIMARYTYPE); - if (primaryType != null) { - support.getParentNodeTypes().add(primaryType); - } - Iterables.addAll(support.getParentNodeTypes(), beforeParentNodeOrNull.getNames(JcrConstants.JCR_MIXINTYPES)); + support.changeSetBuilder.addParentNodeName(childName); } - if (changed && afterParentNodeOrNull != null) { - String primaryType = afterParentNodeOrNull.getName(JcrConstants.JCR_PRIMARYTYPE); - if (primaryType != null) { - support.getParentNodeTypes().add(primaryType); - } - Iterables.addAll(support.getParentNodeTypes(), afterParentNodeOrNull.getNames(JcrConstants.JCR_MIXINTYPES)); + + if (changed){ + addParentNodeType(beforeParentNodeOrNull); + addParentNodeType(afterParentNodeOrNull); } // then if we're not at the root, we're done @@ -234,28 +189,25 @@ public class ChangeCollectorProvider extends ValidatorProvider { // but if we're at the root, then we add the ChangeSet to the // CommitContext of the CommitInfo - CommitContext commitContext = (CommitContext) support.getInfo().getInfo().get(CommitContext.NAME); - ChangeSet changeSet = support.getChangeSetBuilder().build(); + CommitContext commitContext = (CommitContext) support.info.getInfo().get(CommitContext.NAME); + ChangeSet changeSet = support.changeSetBuilder.build(); commitContext.set(COMMIT_CONTEXT_OBSERVATION_CHANGESET, changeSet); - LOG.debug("Collected changeSet for commit {} is {}", support.getInfo(), changeSet); + LOG.debug("Collected changeSet for commit {} is {}", support.info, changeSet); } @Override public void propertyAdded(PropertyState after) throws CommitFailedException { - changed = true; - support.getPropertyNames().add(after.getName()); + addPropertyName(after); } @Override public void propertyChanged(PropertyState before, PropertyState after) throws CommitFailedException { - changed = true; - support.getPropertyNames().add(before.getName()); + addPropertyName(before); } @Override public void propertyDeleted(PropertyState before) throws CommitFailedException { - changed = true; - support.getPropertyNames().add(before.getName()); + addPropertyName(before); } @Override @@ -268,10 +220,10 @@ public class ChangeCollectorProvider extends ValidatorProvider { @Override public Validator childNodeChanged(String childName, NodeState before, NodeState after) throws CommitFailedException { - if (level == support.getMaxPathDepth()) { + if (level == support.maxPathDepth) { // then we'll cut off further paths below. // to compensate, add the current path at this level - support.getParentPaths().add(path); + support.changeSetBuilder.addParentPath(path); // however, continue normally to handle names/types/properties // below @@ -294,9 +246,25 @@ public class ChangeCollectorProvider extends ValidatorProvider { private void addToAllNodeType(NodeState state) { String primaryType = state.getName(JcrConstants.JCR_PRIMARYTYPE); if (primaryType != null) { - support.getAllNodeTypes().add(primaryType); + support.changeSetBuilder.addNodeType(primaryType); + } + support.changeSetBuilder.addNodeTypes(state.getNames(JcrConstants.JCR_MIXINTYPES)); + } + + private void addParentNodeType(@Nullable NodeState state) { + if (state == null){ + return; + } + String primaryType = state.getName(JcrConstants.JCR_PRIMARYTYPE); + if (primaryType != null) { + support.changeSetBuilder.addParentNodeType(primaryType); } - Iterables.addAll(support.getAllNodeTypes(), state.getNames(JcrConstants.JCR_MIXINTYPES)); + support.changeSetBuilder.addParentNodeTypes(state.getNames(JcrConstants.JCR_MIXINTYPES)); + } + + private void addPropertyName(PropertyState after) { + changed = true; + support.changeSetBuilder.addPropertyName(after.getName()); } } diff --git oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java index 425e566..14fe2b3 100644 --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java @@ -57,65 +57,58 @@ public class ChangeSetBuilder { return parentPathOverflow; } - public Set getParentPaths() { - if (parentPathOverflow || parentPaths.size() > maxItems) { - // if already overflown, reset the buffers anyway - parentPathOverflow = true; - parentPaths.clear(); - } - return parentPaths; + public ChangeSetBuilder addParentPath(String path){ + parentPathOverflow = addAndCheckOverflow(parentPaths, path, maxItems, parentPathOverflow); + return this; } - public boolean getParentNodeNameOverflown() { + public boolean isParentNodeNameOverflown() { return parentNodeNameOverflow; } - public Set getParentNodeNames() { - if (parentNodeNameOverflow || parentNodeNames.size() > maxItems) { - // if already overflown, reset the buffers anyway - parentNodeNameOverflow = true; - parentNodeNames.clear(); - } - return parentNodeNames; + public ChangeSetBuilder addParentNodeName(String parentNodeName) { + parentNodeNameOverflow = addAndCheckOverflow(parentNodeNames, parentNodeName, maxItems, parentNodeNameOverflow); + return this; } - public boolean getParentNodeTypeOverflown() { + public boolean isParentNodeTypeOverflown() { return parentNodeTypeOverflow; } - public Set getParentNodeTypes() { - if (parentNodeTypeOverflow || parentNodeTypes.size() > maxItems) { - // if already overflown, reset the buffers anyway - parentNodeTypeOverflow = true; - parentNodeTypes.clear(); + public ChangeSetBuilder addParentNodeTypes(Iterable nodeTypes){ + for (String nodeType : nodeTypes){ + addParentNodeType(nodeType); } - return parentNodeTypes; + return this; } - public boolean getPropertyNameOverflown() { - return propertyNameOverflow; + public ChangeSetBuilder addParentNodeType(String parentNodeType) { + parentNodeTypeOverflow = addAndCheckOverflow(parentNodeTypes, parentNodeType, maxItems, parentNodeTypeOverflow); + return this; } - public Set getPropertyNames() { - if (propertyNameOverflow || propertyNames.size() > maxItems) { - // if already overflown, reset the buffers anyway - propertyNameOverflow = true; - propertyNames.clear(); - } - return propertyNames; + public boolean isPropertyNameOverflown() { + return propertyNameOverflow; } - public boolean getAllNodeTypeOverflown() { + public ChangeSetBuilder addPropertyName(String propertyName) { + propertyNameOverflow = addAndCheckOverflow(propertyNames, propertyName, maxItems, propertyNameOverflow); + return this; + } + public boolean isAllNodeTypeOverflown() { return allNodeTypeOverflow; } - public Set getAllNodeTypes() { - if (allNodeTypeOverflow || allNodeTypes.size() > maxItems) { - // if already overflown, reset the buffers anyway - allNodeTypeOverflow = true; - allNodeTypes.clear(); + public ChangeSetBuilder addNodeTypes(Iterable nodeTypes){ + for (String nodeType : nodeTypes){ + addNodeType(nodeType); } - return allNodeTypes; + return this; + } + + public ChangeSetBuilder addNodeType(String nodeType) { + allNodeTypeOverflow = addAndCheckOverflow(allNodeTypes, nodeType, maxItems, allNodeTypeOverflow); + return this; } public int getMaxPrefilterPathDepth() { @@ -123,12 +116,6 @@ public class ChangeSetBuilder { } public ChangeSet build() { - // invoke accessors to get overflow evaluated one last time - getParentPaths(); - getParentNodeNames(); - getParentNodeTypes(); - getPropertyNames(); - getAllNodeTypes(); return new ChangeSet(maxPathDepth, parentPathOverflow ? null : parentPaths, parentNodeNameOverflow ? null : parentNodeNames, parentNodeTypeOverflow ? null : parentNodeTypes, @@ -136,4 +123,22 @@ public class ChangeSetBuilder { allNodeTypeOverflow ? null : allNodeTypes); } + /** + * Add data to dataSet if dataSet size is less than maxSize. + * + * @return true if dataSet size is at maxSize i.e. overflow condition reached + */ + private static boolean addAndCheckOverflow(Set dataSet, String data, int maxSize, boolean overflow) { + if (overflow) { + return true; + } else { + dataSet.add(data); + if (dataSet.size() > maxSize){ + dataSet.clear(); + return true; + } + return false; + } + } + } diff --git oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java index f8c405f..47bb4ce 100644 --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java @@ -49,11 +49,21 @@ public class ChangeSetFilterImplTest { Set propertyNames, Set allNodeTypes) { ChangeSetBuilder changeSetBuilder = new ChangeSetBuilder(Integer.MAX_VALUE, maxPathDepth); - changeSetBuilder.getParentPaths().addAll(parentPaths); - changeSetBuilder.getParentNodeNames().addAll(parentNodeNames); - changeSetBuilder.getParentNodeTypes().addAll(parentNodeTypes); - changeSetBuilder.getPropertyNames().addAll(propertyNames); - changeSetBuilder.getAllNodeTypes().addAll(allNodeTypes); + for (String path : parentPaths){ + changeSetBuilder.addParentPath(path); + } + for (String nodeName : parentNodeNames){ + changeSetBuilder.addParentNodeName(nodeName); + } + for (String parentNodeType : parentNodeTypes){ + changeSetBuilder.addParentNodeType(parentNodeType); + } + for (String propertyName : propertyNames){ + changeSetBuilder.addPropertyName(propertyName); + } + for (String nodeType : allNodeTypes){ + changeSetBuilder.addNodeType(nodeType); + } return changeSetBuilder.build(); } @@ -64,27 +74,24 @@ public class ChangeSetFilterImplTest { private ChangeSetBuilder overflowAllNodeTypes(ChangeSetBuilder builder) { int i = 0; - while (!builder.getAllNodeTypeOverflown()) { - builder.getAllNodeTypes().add("foo" + i++); - builder.getAllNodeTypes(); + while (!builder.isAllNodeTypeOverflown()) { + builder.addNodeType("foo" + i++); } return builder; } private ChangeSetBuilder overflowParentNodeTypes(ChangeSetBuilder builder) { int i = 0; - while (!builder.getParentNodeTypeOverflown()) { - builder.getParentNodeTypes().add("foo" + i++); - builder.getParentNodeTypes(); + while (!builder.isParentNodeTypeOverflown()) { + builder.addParentNodeType("foo" + i++); } return builder; } private ChangeSetBuilder overflowParentNodeNames(ChangeSetBuilder builder) { int i = 0; - while (!builder.getParentNodeNameOverflown()) { - builder.getParentNodeNames().add("foo" + i++); - builder.getParentNodeNames(); + while (!builder.isParentNodeNameOverflown()) { + builder.addParentNodeName("foo" + i++); } return builder; } @@ -92,17 +99,15 @@ public class ChangeSetFilterImplTest { private ChangeSetBuilder overflowParentPaths(ChangeSetBuilder builder) { int i = 0; while (!builder.getParentPathOverflown()) { - builder.getParentPaths().add("foo" + i++); - builder.getParentPaths(); + builder.addParentPath("foo" + i++); } return builder; } private ChangeSetBuilder overflowPropertyNames(ChangeSetBuilder builder) { int i = 0; - while (!builder.getPropertyNameOverflown()) { - builder.getPropertyNames().add("foo" + i++); - builder.getPropertyNames(); + while (!builder.isPropertyNameOverflown()) { + builder.addPropertyName("foo" + i++); } return builder; } @@ -177,21 +182,21 @@ public class ChangeSetFilterImplTest { @Test public void testOverflowing() throws Exception { ChangeSetBuilder builder = newBuilder(5, 5); - assertTrue(overflowAllNodeTypes(builder).getAllNodeTypeOverflown()); - assertTrue(overflowParentNodeTypes(builder).getParentNodeTypeOverflown()); - assertTrue(overflowParentNodeNames(builder).getParentNodeNameOverflown()); + assertTrue(overflowAllNodeTypes(builder).isAllNodeTypeOverflown()); + assertTrue(overflowParentNodeTypes(builder).isParentNodeTypeOverflown()); + assertTrue(overflowParentNodeNames(builder).isParentNodeNameOverflown()); assertTrue(overflowParentPaths(builder).getParentPathOverflown()); - assertTrue(overflowPropertyNames(builder).getPropertyNameOverflown()); + assertTrue(overflowPropertyNames(builder).isPropertyNameOverflown()); } private ChangeSetBuilder sampleBuilder() { ChangeSetBuilder builder = newBuilder(5, 5); - builder.getAllNodeTypes().add("nt:file"); - builder.getParentNodeTypes().add("nt:file"); - builder.getParentPaths().add("/bar"); - builder.getParentNodeNames().add("bar"); - builder.getPropertyNames().add("a"); - builder.getPropertyNames().add("b"); + builder.addNodeType("nt:file"); + builder.addParentNodeType("nt:file"); + builder.addParentPath("/bar"); + builder.addParentNodeName("bar"); + builder.addPropertyName("a"); + builder.addPropertyName("b"); return builder; }