Author: chetanm
Date: Fri Nov 25 09:28:55 2016
New Revision: 1771261
URL: http://svn.apache.org/viewvc?rev=1771261&view=rev
Log:
OAK-5126 - Support ChangeSet merging and serialization
Refactor the code flow
-- Use isxxx for boolean properties
-- ChangeSetBuilder now does not expose the internal set and instead allow
caller to just add the required change data
-- Inlined access to ChangeSetBuilder in ChangeCollectorProvider
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java?rev=1771261&r1=1771260&r2=1771261&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeCollectorProvider.java
Fri Nov 25 09:28:55 2016
@@ -23,7 +23,6 @@ import static org.apache.jackrabbit.oak.
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.Compon
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 ext
* 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 ext
public String toString() {
return "CollectorSupport with " + changeSetBuilder;
}
-
- private CommitInfo getInfo() {
- return info;
- }
-
- private int getMaxPathDepth() {
- return maxPathDepth;
- }
-
- private ChangeSetBuilder getChangeSetBuilder() {
- return changeSetBuilder;
- }
-
- private Set<String> getParentPaths() {
- return changeSetBuilder.getParentPaths();
- }
-
- private Set<String> getParentNodeNames() {
- return changeSetBuilder.getParentNodeNames();
- }
-
- private Set<String> getParentNodeTypes() {
- return changeSetBuilder.getParentNodeTypes();
- }
-
- private Set<String> getPropertyNames() {
- return changeSetBuilder.getPropertyNames();
- }
-
- private Set<String> getAllNodeTypes() {
- return changeSetBuilder.getAllNodeTypes();
- }
-
}
/**
@@ -206,25 +170,16 @@ public class ChangeCollectorProvider ext
@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 ext
// 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 ext
@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 ext
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());
}
}
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java?rev=1771261&r1=1771260&r2=1771261&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeSetBuilder.java
Fri Nov 25 09:28:55 2016
@@ -53,69 +53,62 @@ public class ChangeSetBuilder {
+ ", allNodeTypes=" + allNodeTypes + "}";
}
- public boolean getParentPathOverflown() {
+ public boolean isParentPathOverflown() {
return parentPathOverflow;
}
- public Set<String> 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<String> 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<String> getParentNodeTypes() {
- if (parentNodeTypeOverflow || parentNodeTypes.size() > maxItems) {
- // if already overflown, reset the buffers anyway
- parentNodeTypeOverflow = true;
- parentNodeTypes.clear();
+ public ChangeSetBuilder addParentNodeTypes(Iterable<String> 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<String> 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<String> getAllNodeTypes() {
- if (allNodeTypeOverflow || allNodeTypes.size() > maxItems) {
- // if already overflown, reset the buffers anyway
- allNodeTypeOverflow = true;
- allNodeTypes.clear();
+ public ChangeSetBuilder addNodeTypes(Iterable<String> 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<String> 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;
+ }
+ }
+
}
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java?rev=1771261&r1=1771260&r2=1771261&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImplTest.java
Fri Nov 25 09:28:55 2016
@@ -49,60 +49,64 @@ public class ChangeSetFilterImplTest {
Set<String> propertyNames,
Set<String> 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();
}
private ChangeSetBuilder newBuilder(int maxItems, int maxPathDepth) {
- ChangeSetBuilder changeSetBuilder = new ChangeSetBuilder(maxItems,
maxPathDepth);
- return changeSetBuilder;
+ return new ChangeSetBuilder(maxItems, maxPathDepth);
}
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;
}
private ChangeSetBuilder overflowParentPaths(ChangeSetBuilder builder) {
int i = 0;
- while (!builder.getParentPathOverflown()) {
- builder.getParentPaths().add("foo" + i++);
- builder.getParentPaths();
+ while (!builder.isParentPathOverflown()) {
+ 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 +181,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(overflowParentPaths(builder).getParentPathOverflown());
- assertTrue(overflowPropertyNames(builder).getPropertyNameOverflown());
+ assertTrue(overflowAllNodeTypes(builder).isAllNodeTypeOverflown());
+
assertTrue(overflowParentNodeTypes(builder).isParentNodeTypeOverflown());
+
assertTrue(overflowParentNodeNames(builder).isParentNodeNameOverflown());
+ assertTrue(overflowParentPaths(builder).isParentPathOverflown());
+ 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;
}