Author: stefanegli
Date: Mon Nov 14 10:46:59 2016
New Revision: 1769591
URL: http://svn.apache.org/viewvc?rev=1769591&view=rev
Log:
OAK-5102 : improved precision of prefiltering paths by not just adding /** but
the actual ancestor paths - doing so ignoring the deep flag as that's
independent. added more variants of testAggregate6 for this.
Added:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImplTest.java
(with props)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImpl.java
jackrabbit/oak/trunk/oak-jcr/pom.xml
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImpl.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ObservationManagerImpl.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImpl.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImpl.java?rev=1769591&r1=1769590&r2=1769591&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImpl.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/filter/ChangeSetFilterImpl.java
Mon Nov 14 10:46:59 2016
@@ -27,6 +27,7 @@ import java.util.Set;
import java.util.regex.Pattern;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import org.apache.jackrabbit.oak.plugins.observation.ChangeSet;
@@ -41,6 +42,12 @@ public class ChangeSetFilterImpl impleme
public ChangeSetFilterImpl(@Nonnull Set<String> includedParentPaths,
boolean isDeep, Set<String> excludedParentPaths,
Set<String> parentNodeNames, Set<String> parentNodeTypes,
Set<String> propertyNames) {
+ this(includedParentPaths, isDeep, null, excludedParentPaths,
parentNodeNames, parentNodeTypes, propertyNames);
+ }
+
+ public ChangeSetFilterImpl(@Nonnull Set<String> includedParentPaths,
boolean isDeep,
+ @Nullable Set<String> additionalIncludedParentPaths, Set<String>
excludedParentPaths,
+ Set<String> parentNodeNames, Set<String> parentNodeTypes,
Set<String> propertyNames) {
this.rootIncludePaths = new HashSet<String>();
this.includePathPatterns = new HashSet<Pattern>();
for (String aRawIncludePath : includedParentPaths) {
@@ -54,6 +61,12 @@ public class ChangeSetFilterImpl impleme
this.rootIncludePaths.add(aRawIncludePath);
this.includePathPatterns.add(asPattern(aGlobbingIncludePath));
}
+ if (additionalIncludedParentPaths != null) {
+ for (String path : additionalIncludedParentPaths) {
+ this.rootIncludePaths.add(path);
+ this.includePathPatterns.add(asPattern(path));
+ }
+ }
this.excludePathPatterns = new HashSet<Pattern>();
for (String aRawExcludePath : excludedParentPaths) {
this.excludePathPatterns.add(asPattern(concat(aRawExcludePath,
"**")));
@@ -62,6 +75,11 @@ public class ChangeSetFilterImpl impleme
this.parentNodeTypes = parentNodeTypes == null ? null : new
HashSet<String>(parentNodeTypes);
this.parentNodeNames = parentNodeNames == null ? null : new
HashSet<String>(parentNodeNames);
}
+
+ /** for testing only **/
+ public Set<String> getRootIncludePaths() {
+ return rootIncludePaths;
+ }
private Pattern asPattern(String patternWithGlobs) {
return
Pattern.compile(GlobbingPathHelper.globPathAsRegex(patternWithGlobs));
Modified: jackrabbit/oak/trunk/oak-jcr/pom.xml
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/pom.xml?rev=1769591&r1=1769590&r2=1769591&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-jcr/pom.xml Mon Nov 14 10:46:59 2016
@@ -389,5 +389,12 @@
<version>1.3.2</version>
<scope>test</scope>
</dependency>
+
+ <dependency>
+ <groupId>junit-addons</groupId>
+ <artifactId>junit-addons</artifactId>
+ <version>1.4</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImpl.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImpl.java?rev=1769591&r1=1769590&r2=1769591&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImpl.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImpl.java
Mon Nov 14 10:46:59 2016
@@ -380,24 +380,27 @@ public class OakEventFilterImpl extends
return includeAncestorRemove;
}
- private void addAncestorsRemoveCondition(Set<String> parentPaths, String
globPath) {
+ static void addAncestorPaths(Set<String> ancestorPaths, String globPath) {
if (globPath == null || !globPath.contains("/")) {
return;
}
- // from /a/b/c => add /a and /a/b
- // from /a/b/** => add /a
- // from /a => add nothing
+ // from /a/b/c => add /*, /a/* and /a/b/*
+ // from /a/b/** => add /*, /a/*
+ // from /a => add /*, nothing
// from / => add nothing
- // from /a/b/**/*.html => add /a
- // from /a/b/*/*.html => add /a
+ // from /a/b/**/*.html => add /*, /a/*
+ // from /a/b/*/*.html => add /*, /a/*
Iterator<String> it = PathUtils.elements(globPath).iterator();
StringBuffer sb = new StringBuffer();
+ if (it.hasNext()) {
+ ancestorPaths.add("/*");
+ }
while(it.hasNext()) {
String element = it.next();
if (element.contains("*")) {
- if (parentPaths.size() > 0) {
- parentPaths.remove(parentPaths.size()-1);
+ if (ancestorPaths.size() > 0) {
+ ancestorPaths.remove(ancestorPaths.size()-1);
}
break;
} else if (!it.hasNext()) {
@@ -405,7 +408,7 @@ public class OakEventFilterImpl extends
}
sb.append("/");
sb.append(element);
- parentPaths.add(sb.toString() + "/*");
+ ancestorPaths.add(sb.toString() + "/*");
}
}
@@ -414,29 +417,29 @@ public class OakEventFilterImpl extends
return mainCondition;
}
Set<String> parentPaths = new HashSet<String>();
- addAncestorsRemoveCondition(parentPaths, getAbsPath());
+ addAncestorPaths(parentPaths, getAbsPath());
if (getAdditionalPaths() != null) {
for (String absPath : getAdditionalPaths()) {
- addAncestorsRemoveCondition(parentPaths, absPath);
+ addAncestorPaths(parentPaths, absPath);
}
}
if (globPaths != null) {
for (String globPath : globPaths) {
- addAncestorsRemoveCondition(parentPaths, globPath);
+ addAncestorPaths(parentPaths, globPath);
}
}
if (parentPaths.size() == 0) {
return mainCondition;
}
- List<Condition> ancestorsRemoveConditions = new
LinkedList<Condition>();
+ List<Condition> ancestorsIncludeConditions = new
LinkedList<Condition>();
for (String aParentPath : parentPaths) {
- ancestorsRemoveConditions.add(filterBuilder.path(aParentPath));
+ ancestorsIncludeConditions.add(filterBuilder.path(aParentPath));
}
return filterBuilder.any(
mainCondition,
filterBuilder.all(
filterBuilder.eventType(NODE_REMOVED),
- filterBuilder.any(ancestorsRemoveConditions),
+ filterBuilder.any(ancestorsIncludeConditions),
filterBuilder.deleteSubtree(),
filterBuilder.accessControl(permissionProviderFactory)
)
@@ -553,30 +556,31 @@ public class OakEventFilterImpl extends
* not apply the normally applied prefilter paths.
* @param includePaths the set to adjust depending on filter flags
*/
- void adjustPrefilterIncludePaths(Set<String> includePaths) {
+ Set<String> calcPrefilterIncludePaths(Set<String> includePaths) {
+ Set<String> paths = new HashSet<String>();
if (includeAncestorRemove) {
- includePaths.clear();
- includePaths.add("/**");
- } else if (withNodeTypeAggregate) {
+ for (String includePath : includePaths) {
+ addAncestorPaths(paths, includePath);
+ }
+ }
+ if (withNodeTypeAggregate) {
// ensure that, for prefixing, all includePaths allow additional
// subpaths for the aggregation - this can be simplified
// to just allow anything (**) below there, as this is just
// about prefiltering, not actual (precise) filtering.
// so the goal is just to ensure nothing is erroneously excluded
// so more including is fine.
- Set<String> originalPaths = new HashSet<String>(includePaths);
- for (String includePath : originalPaths) {
- if (includePath.equals("**") || includePath.endsWith("/*") ||
includePath.endsWith("/**")) {
+ for (String includePath : includePaths) {
+ if (includePath.equals("**") || includePath.endsWith("/**")) {
// skip, this is fine
} else if (includePath.endsWith("/")) {
- includePaths.remove(includePath);
- includePaths.add(includePath + "**");
+ paths.add(includePath + "**");
} else {
- includePaths.remove(includePath);
- includePaths.add(includePath + "/**");
+ paths.add(includePath + "/**");
}
}
}
+ return paths;
}
}
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ObservationManagerImpl.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ObservationManagerImpl.java?rev=1769591&r1=1769590&r2=1769591&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ObservationManagerImpl.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/observation/ObservationManagerImpl.java
Mon Nov 14 10:46:59 2016
@@ -323,8 +323,9 @@ public class ObservationManagerImpl impl
ListenerTracker tracker = new WarningListenerTracker(
!noExternal, listener, eventTypes, absPath, isDeep, uuids,
nodeTypeName, noLocal);
+ Set<String> additionalIncludePaths = null;
if (oakEventFilter != null) {
- oakEventFilter.adjustPrefilterIncludePaths(includePaths);
+ additionalIncludePaths =
oakEventFilter.calcPrefilterIncludePaths(includePaths);
}
// OAK-5082 : node type filtering should not only be direct but
include derived types
@@ -346,7 +347,7 @@ public class ObservationManagerImpl impl
// OAK-4908 : prefiltering support. here we have explicit yes/no/maybe
filtering
// for things like propertyNames/nodeTypes/nodeNames/paths which
cannot be
// applied on the full-fledged filterBuilder above but requires an
explicit 'prefilter' for that.
- filterBuilder.setChangeSetFilter(new ChangeSetFilterImpl(includePaths,
isDeep, excludedPaths, null,
+ filterBuilder.setChangeSetFilter(new ChangeSetFilterImpl(includePaths,
isDeep, additionalIncludePaths, excludedPaths, null,
explodedNodeTypes, null));
addEventListener(listener, tracker, filterBuilder.build());
Added:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImplTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImplTest.java?rev=1769591&view=auto
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImplTest.java
(added)
+++
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImplTest.java
Mon Nov 14 10:46:59 2016
@@ -0,0 +1,55 @@
+/*
+ * 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.jcr.observation;
+
+import static org.junit.Assert.assertEquals;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.junit.Test;
+
+public class OakEventFilterImplTest {
+
+ @Test
+ public void testAddAncestorPaths() throws Exception {
+ // parent of / doesnt exist, hence no ancestor path. "" will anyway
resolve to "/" in FilterBuilder.getSubTrees()
+ assertMatches(new String[] {}, "/");
+
+ assertMatches(new String[] {"/*"}, "/*");
+ assertMatches(new String[] {"/*"}, "/**");
+ assertMatches(new String[] {"/*"}, "/a");
+ assertMatches(new String[] {"/*", "/a/*"}, "/a/b");
+ assertMatches(new String[] {"/*", "/a/*"}, "/a/*");
+ assertMatches(new String[] {"/*", "/a/*"}, "/a/**");
+ assertMatches(new String[] {"/*", "/a/*", "/a/b/*"}, "/a/b/c");
+ assertMatches(new String[] {"/*", "/a/*", "/a/b/*"}, "/a/b/*");
+ assertMatches(new String[] {"/*", "/a/*", "/a/b/*"}, "/a/b/**");
+ assertMatches(new String[] {"/*", "/a/*", "/a/b/*", "/a/b/c/*"},
"/a/b/c/d");
+ assertMatches(new String[] {"/*", "/a/*", "/a/b/*", "/a/b/c/*"},
"/a/b/c/*");
+ assertMatches(new String[] {"/*", "/a/*", "/a/b/*", "/a/b/c/*"},
"/a/b/c/**");
+ }
+
+ private void assertMatches(String[] expectedPaths, String globPath) {
+ Set<String> ancestorPaths = new HashSet<String>();
+ OakEventFilterImpl.addAncestorPaths(ancestorPaths, globPath);
+ Set<String> expected = new
HashSet<String>(Arrays.asList(expectedPaths));
+ assertEquals(expected, ancestorPaths);
+ }
+}
Propchange:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/OakEventFilterImplTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java?rev=1769591&r1=1769590&r2=1769591&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java
Mon Nov 14 10:46:59 2016
@@ -84,6 +84,9 @@ import com.google.common.util.concurrent
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
+
+import junitx.util.PrivateAccessor;
+
import org.apache.jackrabbit.JcrConstants;
import org.apache.jackrabbit.api.JackrabbitNode;
import org.apache.jackrabbit.api.observation.JackrabbitEventFilter;
@@ -94,6 +97,7 @@ import org.apache.jackrabbit.oak.fixture
import org.apache.jackrabbit.oak.jcr.AbstractRepositoryTest;
import org.apache.jackrabbit.oak.jcr.observation.filter.FilterFactory;
import org.apache.jackrabbit.oak.jcr.observation.filter.OakEventFilter;
+import
org.apache.jackrabbit.oak.plugins.observation.filter.ChangeSetFilterImpl;
import org.apache.jackrabbit.oak.plugins.observation.filter.FilterBuilder;
import org.apache.jackrabbit.oak.plugins.observation.filter.FilterProvider;
import org.apache.jackrabbit.oak.plugins.observation.filter.Selectors;
@@ -1903,44 +1907,72 @@ public class ObservationTest extends Abs
public void testAggregate6() throws Exception {
OakEventFilter oef = FilterFactory.wrap(new JackrabbitEventFilter());
oef.setEventTypes(ALL_EVENTS);
+ oef.setIsDeep(true);
oef.withIncludeAncestorsRemove()
.withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
.withIncludeGlobPaths("/**/*.jsp");
- doTestAggregate6(oef);
+ doTestAggregate6(oef, new String[] {"/"}, new String[] {"/*",
"/**/*.jsp", "/**/*.jsp/**"});
oef = FilterFactory.wrap(new JackrabbitEventFilter());
oef.setEventTypes(ALL_EVENTS);
+ oef.setIsDeep(false);
+ oef.withIncludeAncestorsRemove()
+ .withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
+ .withIncludeGlobPaths("/**/*.jsp");
+ doTestAggregate6(oef, new String[] {"/"}, new String[] {"/*",
"/**/*.jsp", "/**/*.jsp/**"});
+
+ oef = FilterFactory.wrap(new JackrabbitEventFilter());
+ oef.setEventTypes(ALL_EVENTS);
oef.withIncludeAncestorsRemove()
.withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
.withIncludeGlobPaths("**/*.jsp");
- doTestAggregate6(oef);
+ doTestAggregate6(oef, new String[] {"/"}, new String[] {"/*",
"**/*.jsp", "**/*.jsp/**"});
oef = FilterFactory.wrap(new JackrabbitEventFilter());
oef.setEventTypes(ALL_EVENTS);
oef // without includeAncestorsRemove this time
.withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
.withIncludeGlobPaths("/**/*.jsp");
- doTestAggregate6(oef);
+ doTestAggregate6(oef, new String[] {""}, new String[] {"/**/*.jsp",
"/**/*.jsp/**"});
oef = FilterFactory.wrap(new JackrabbitEventFilter());
oef.setEventTypes(ALL_EVENTS);
oef // without includeAncestorsRemove this time
.withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
.withIncludeGlobPaths("**/*.jsp");
- doTestAggregate6(oef);
+ doTestAggregate6(oef, new String[] {""}, new String[] {"**/*.jsp",
"**/*.jsp/**"});
oef = FilterFactory.wrap(new JackrabbitEventFilter());
oef.setEventTypes(ALL_EVENTS);
+ oef.withIncludeAncestorsRemove()
+ .withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
+ .withIncludeGlobPaths("/parent/**/*.jsp");
+ doTestAggregate6(oef, new String[] {"/"}, new String[] {"/*",
"/parent/*", "/parent/**/*.jsp", "/parent/**/*.jsp/**"});
+
+ oef = FilterFactory.wrap(new JackrabbitEventFilter());
+ oef.setEventTypes(ALL_EVENTS);
oef // without includeAncestorsRemove this time
.withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
.withIncludeGlobPaths("/parent/**/*.jsp");
- doTestAggregate6(oef);
+ doTestAggregate6(oef, new String[] {"/parent"}, new String[]
{"/parent/**/*.jsp", "/parent/**/*.jsp/**"});
+
+ oef = FilterFactory.wrap(new JackrabbitEventFilter());
+ oef.setEventTypes(ALL_EVENTS);
+ oef // without includeAncestorsRemove this time
+ .withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
+ .withIncludeGlobPaths("/parent/**/*.jsp", "/foo/bar/**");
+ doTestAggregate6(oef, new String[] {"/parent", "/foo/bar"}, new
String[] {"/foo/bar/**", "/parent/**/*.jsp", "/parent/**/*.jsp/**"});
+
+ oef = FilterFactory.wrap(new JackrabbitEventFilter());
+ oef.setEventTypes(ALL_EVENTS);
+ oef.withIncludeAncestorsRemove()
+ .withNodeTypeAggregate(new String[] { "oak:Unstructured" },
new String[] { "", "jcr:content" } )
+ .withIncludeGlobPaths("/parent/**/*.jsp", "/foo/bar/**");
+ doTestAggregate6(oef, new String[] {"/"}, new String[] {"/*",
"/foo/*", "/foo/bar/*", "/foo/bar/**", "/parent/*", "/parent/**/*.jsp",
"/parent/**/*.jsp/**"});
}
- private void doTestAggregate6(OakEventFilter oef)
- throws RepositoryException, ItemExistsException,
PathNotFoundException, NoSuchNodeTypeException,
- LockException, VersionException, ConstraintViolationException,
ValueFormatException, AccessDeniedException,
- ReferentialIntegrityException, InvalidItemStateException,
InterruptedException, ExecutionException {
+ private void doTestAggregate6(OakEventFilter oef, String[]
expectedSubTrees, String[] expectedPrefilterPaths)
+ throws Exception {
assumeTrue(observationManager instanceof ObservationManagerImpl);
ObservationManagerImpl oManager = (ObservationManagerImpl)
observationManager;
ExpectationListener listener = new ExpectationListener();
@@ -1949,6 +1981,10 @@ public class ObservationTest extends Abs
assertNotNull(cp);
FilterProvider filterProvider = cp.getFilterProvider();
assertNotNull(filterProvider);
+ assertMatches(filterProvider.getSubTrees(), expectedSubTrees);
+ ChangeSetFilterImpl changeSetFilter =
(ChangeSetFilterImpl)PrivateAccessor.getField(filterProvider,
"changeSetFilter");
+ assertNotNull(changeSetFilter);
+ assertMatches(changeSetFilter.getRootIncludePaths(),
expectedPrefilterPaths);
Node parent = getAdminSession().getRootNode().addNode("parent",
"nt:unstructured");
Node bar = parent.addNode("bar", "nt:unstructured");
