SLING-7759 introducing FilterPredicate (#3)
authorNicolas Peltier <peltier.nicolas@gmail.com>
Thu, 5 Jul 2018 12:52:55 +0000 (14:52 +0200)
committerGitHub <noreply@github.com>
Thu, 5 Jul 2018 12:52:55 +0000 (14:52 +0200)
- extending sling.filter.pattern to other classic request attributes,
- FilterHandler aggregates a FilterPredicate that is tested for inclusion, with 0 to n sub-predicates based on filter configuration,
- added unit tests for most cases,
- added debug & error logs to help understanding presence/absence of a given filter

reviewed and drastically improved by @jsedding

src/main/java/org/apache/sling/engine/EngineConstants.java
src/main/java/org/apache/sling/engine/impl/filter/AbstractSlingFilterChain.java
src/main/java/org/apache/sling/engine/impl/filter/FilterHandle.java
src/main/java/org/apache/sling/engine/impl/filter/FilterPredicate.java [new file with mode: 0644]
src/main/java/org/apache/sling/engine/impl/filter/ServletFilterManager.java
src/main/java/org/apache/sling/engine/impl/filter/SlingFilterChainHelper.java
src/main/java/org/apache/sling/engine/package-info.java
src/test/java/org/apache/sling/engine/impl/filter/AbstractFilterTest.java [new file with mode: 0644]
src/test/java/org/apache/sling/engine/impl/filter/FilterHandleTest.java [new file with mode: 0644]
src/test/java/org/apache/sling/engine/impl/filter/FilterPredicateTest.java [new file with mode: 0644]

index 923b0ff..3c0aa67 100644 (file)
@@ -156,10 +156,42 @@ public class EngineConstants {
     public static final String SLING_FILTER_SCOPE = "sling.filter.scope";
     
     /**
+     * Regular expression pattern for enabling a filter on a matching request path
      *@since 2.2, Sling Engine 2.4
      */
     public static final String SLING_FILTER_PATTERN = "sling.filter.pattern";
 
+
+    /**
+     * Regular expression pattern for enabling a filter on a matching request suffix
+     *@since Sling Engine 2.7
+     */
+    public static final String SLING_FILTER_SUFFIX_PATTERN = "sling.filter.suffix.pattern";
+
+    /**
+     * Set of selectors enabling a filter if contains one or more request selectors
+     *@since Sling Engine 2.7
+     */
+    public static final String SLING_FILTER_SELECTORS = "sling.filter.selectors";
+
+    /**
+     * Set of methods enabling a filter on contains request method
+     *@since Sling Engine 2.7
+     */
+    public static final String SLING_FILTER_METHODS = "sling.filter.methods";
+
+    /**
+     * Set of resource types enabling a filter if contains request resource type
+     *@since Sling Engine 2.7
+     */
+    public static final String SLING_FILTER_RESOURCETYPES = "sling.filter.resourceTypes";
+
+    /**
+     * Set of extensions enabling a filter if contains request extension
+     *@since Sling Engine 2.7
+     */
+    public static final String SLING_FILTER_EXTENSIONS = "sling.filter.extensions";
+
     /**
      * Filter scope value identifying a component level filter.
      * <p>
index 248c89f..0841015 100644 (file)
@@ -29,8 +29,11 @@ import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.SlingHttpServletResponse;
 import org.apache.sling.api.request.RequestProgressTracker;
 import org.apache.sling.engine.impl.request.RequestData;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public abstract class AbstractSlingFilterChain implements FilterChain {
+    private static final Logger LOG = LoggerFactory.getLogger(AbstractSlingFilterChain.class);
 
     private FilterHandle[] filters;
 
@@ -64,9 +67,11 @@ public abstract class AbstractSlingFilterChain implements FilterChain {
                 FilterHandle filter = this.filters[this.current];
                 
                 if (filter.select(slingRequest)) {
+                    LOG.debug("{} got selected for this request", filter);
                     trackFilter(slingRequest, filter);
                     filter.getFilter().doFilter(slingRequest, slingResponse, this);
                 } else {
+                    LOG.debug("{} was not selected for this request", filter);
                     if (this.current == this.filters.length-1) {
                         this.render(slingRequest, slingResponse);
                     } else {
index 718c51b..7e6db26 100644 (file)
@@ -19,7 +19,6 @@
 package org.apache.sling.engine.impl.filter;
 
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.regex.Pattern;
 
 import javax.servlet.Filter;
 
@@ -28,8 +27,6 @@ import org.apache.sling.api.SlingHttpServletRequest;
 public class FilterHandle implements Comparable<FilterHandle> {
 
     private final Filter filter;
-    
-    private final Pattern regex;
 
     private final Long filterId;
 
@@ -40,17 +37,14 @@ public class FilterHandle implements Comparable<FilterHandle> {
     private AtomicLong calls;
 
     private AtomicLong time;
+
+    private FilterPredicate predicate;
     
     FilterProcessorMBeanImpl mbean;
 
-    FilterHandle(Filter filter, String pattern, Long filterId, int order, final String orderSource, FilterProcessorMBeanImpl mbean) {
+    FilterHandle(Filter filter, FilterPredicate predicate, Long filterId, int order, final String orderSource, FilterProcessorMBeanImpl mbean) {
         this.filter = filter;
-        if (pattern != null && pattern.length() > 0) {
-            this.regex = Pattern.compile(pattern);
-        } else {
-            this.regex = null;
-        }
-        
+        this.predicate = predicate;
         this.filterId = filterId;
         this.order = order;
         this.orderSource = orderSource;
@@ -74,19 +68,12 @@ public class FilterHandle implements Comparable<FilterHandle> {
     public String getOrderSource() {
         return orderSource;
     }
-    
+
     boolean select(SlingHttpServletRequest slingHttpServletRequest) {
-        boolean select = true;        
-        if (regex != null) {
-            String uri = slingHttpServletRequest.getPathInfo();
-            // assume root if uri is null
-            if (uri == null)
-            {
-                uri = "/";
-            }
-            select = this.regex.matcher(uri).matches();
-        }        
-        return select;
+      if (predicate != null){
+          return predicate.test(slingHttpServletRequest);
+      }
+      return true;
     }
 
     public long getCalls() {
diff --git a/src/main/java/org/apache/sling/engine/impl/filter/FilterPredicate.java b/src/main/java/org/apache/sling/engine/impl/filter/FilterPredicate.java
new file mode 100644 (file)
index 0000000..6af6630
--- /dev/null
@@ -0,0 +1,156 @@
+/*
+ * 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.sling.engine.impl.filter;
+
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.request.RequestPathInfo;
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.commons.osgi.PropertiesUtil;
+import org.osgi.framework.ServiceReference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.servlet.Filter;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.regex.Pattern;
+
+import static java.util.Arrays.asList;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_EXTENSIONS;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_METHODS;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_PATTERN;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_SUFFIX_PATTERN;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_RESOURCETYPES;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_SELECTORS;
+
+/**
+ * Contains a set of predicates that helps testing whether to enable a filter for a request or not
+ * it can only be constructed from a filter service reference, from whose properties it builds its
+ * predicates
+ */
+public class FilterPredicate {
+
+    private static final Logger LOG = LoggerFactory.getLogger(FilterPredicate.class);
+
+    Collection<String> methods;
+    Collection<String> selectors;
+    Collection<String> extensions;
+    Collection<String> resourceTypes;
+    Pattern pathRegex;
+    Pattern suffixRegex;
+
+    /*
+     * @param reference osgi service configuration
+     */
+    public FilterPredicate(ServiceReference<Filter> reference) {
+        selectors = asCollection(reference, SLING_FILTER_SELECTORS);
+        extensions = asCollection(reference, SLING_FILTER_EXTENSIONS);
+        resourceTypes = asCollection(reference, SLING_FILTER_RESOURCETYPES);
+        methods = asCollection(reference, SLING_FILTER_METHODS);
+        pathRegex = asPattern(reference, SLING_FILTER_PATTERN);
+        suffixRegex = asPattern(reference, SLING_FILTER_SUFFIX_PATTERN);
+    }
+
+    /**
+     * @param reference osgi service reference
+     * @param propertyName configuration property name
+     * @return value of the given property, as a collection, or null if it does not exist
+     */
+    private Collection<String> asCollection(final ServiceReference<Filter> reference, final String propertyName) {
+        String[] value = PropertiesUtil.toStringArray(reference.getProperty(propertyName));
+        return value != null && value.length > 0 ? asList(value) : null;
+    }
+
+    /**
+     * @param reference osgi service reference
+     * @param propertyName configuration property name
+     * @return value of the given property, as a compiled pattern, or null if it does not exist
+     */
+    private Pattern asPattern(final ServiceReference<Filter> reference, String propertyName) {
+        String pattern = PropertiesUtil.toString(reference.getProperty(propertyName), null);
+        return pattern != null && pattern.length() > 0 ? Pattern.compile(pattern) : null;
+    }
+
+    /**
+     * @param allowed configured element
+     * @param actual elements of the given request
+     * @return true if any elements matches the configured ones, or if not or misconfigured
+     */
+    private boolean anyElementMatches(final Collection<String> allowed, final String... actual) {
+        return allowed == null || !Collections.disjoint(allowed, asList(actual));
+    }
+
+    /**
+     * @param resourceTypes configured resourceTypes
+     * @param request request that is being tested
+     * @return true if the request's resource is of one of the types, or if not or miscongfigured
+     */
+    private boolean anyResourceTypeMatches(final Collection<String> resourceTypes, final SlingHttpServletRequest request) {
+        if (resourceTypes == null) {
+            return true;
+        }
+        Resource resource = request.getResource();
+        for (final String resourceType : resourceTypes) {
+            if (resource.isResourceType(resourceType)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * @param pattern configured compiled pattern
+     * @param candidate tested string
+     * @return true if candidate is matching the given pattern, or if not or misconfigured
+     */
+    private boolean patternMatches(final Pattern pattern, final String candidate) {
+        return pattern == null || candidate == null || pattern.matcher(candidate).matches();
+    }
+
+    /**
+     * @param req request that is tested upon this predicate
+     * @return true if this predicate's configuration match the request
+     */
+    boolean test(SlingHttpServletRequest req) {
+        LOG.debug("starting filter test against {} request", req);
+        RequestPathInfo requestPathInfo = req.getRequestPathInfo();
+        String path = requestPathInfo.getResourcePath();
+        boolean select = anyElementMatches(methods, req.getMethod())
+                && anyElementMatches(selectors, requestPathInfo.getSelectors())
+                && anyElementMatches(extensions, requestPathInfo.getExtension())
+                && anyResourceTypeMatches(resourceTypes, req)
+                && patternMatches(pathRegex, path == null || path.isEmpty() ? "/" : path)
+                && patternMatches(suffixRegex, requestPathInfo.getSuffix());
+        LOG.debug("selection of {} returned {}", this, select);
+        return select;
+    }
+
+    @Override
+    public String toString() {
+        return "FilterPredicate{" +
+                "methods='" + methods + '\'' +
+                ", pathRegex=" + pathRegex +
+                ", suffixRegex=" + suffixRegex +
+                ", selectors='" + selectors + '\'' +
+                ", extensions='" + extensions + '\'' +
+                ", resourceTypes='" + resourceTypes + '\'' +
+                '}';
+    }
+
+}
\ No newline at end of file
index b2319f9..82b847c 100644 (file)
@@ -222,7 +222,7 @@ public class ServletFilterManager extends ServiceTracker<Filter, Filter> {
             String[] scopes = OsgiUtil.toStringArray(
                     reference.getProperty(EngineConstants.SLING_FILTER_SCOPE), null);
 
-            String pattern = OsgiUtil.toString(reference.getProperty(EngineConstants.SLING_FILTER_PATTERN), "");
+            FilterPredicate predicate = new FilterPredicate(reference);
 
             if ( scopes == null ) {
                 scopes = OsgiUtil.toStringArray(
@@ -235,14 +235,14 @@ public class ServletFilterManager extends ServiceTracker<Filter, Filter> {
                     scope = scope.toUpperCase();
                     try {
                         FilterChainType type = FilterChainType.valueOf(scope.toString());
-                        getFilterChain(type).addFilter(filter, pattern, serviceId,
+                        getFilterChain(type).addFilter(filter, predicate, serviceId,
                             order, orderSource, mbean);
 
                         if (type == FilterChainType.COMPONENT) {
                             getFilterChain(FilterChainType.INCLUDE).addFilter(
-                                filter, pattern, serviceId, order, orderSource, mbean);
+                                filter, predicate, serviceId, order, orderSource, mbean);
                             getFilterChain(FilterChainType.FORWARD).addFilter(
-                                filter, pattern, serviceId, order, orderSource, mbean);
+                                filter, predicate, serviceId, order, orderSource, mbean);
                         }
 
                     } catch (IllegalArgumentException iae) {
@@ -253,7 +253,7 @@ public class ServletFilterManager extends ServiceTracker<Filter, Filter> {
                 log.warn(String.format(
                     "A Filter (Service ID %s) has been registered without a filter.scope property.",
                     reference.getProperty(Constants.SERVICE_ID)));
-                getFilterChain(FilterChainType.REQUEST).addFilter(filter, pattern,
+                getFilterChain(FilterChainType.REQUEST).addFilter(filter, predicate,
                     serviceId, order, orderSource,mbean);
             }
 
index ac1ba4b..01bc9ed 100644 (file)
@@ -42,7 +42,7 @@ public class SlingFilterChainHelper {
     SlingFilterChainHelper() {
     }
 
-    public synchronized Filter addFilter(final Filter filter,  String pattern,
+    public synchronized Filter addFilter(final Filter filter,  FilterPredicate pattern,
             final Long filterId, final int order, final String orderSource, FilterProcessorMBeanImpl mbean) {
         if (filterList == null) {
             filterList = new TreeSet<FilterHandle>();
index 0fb8f1e..b3aee91 100644 (file)
@@ -17,6 +17,6 @@
  * under the License.
  */
 
-@org.osgi.annotation.versioning.Version("2.2.1")
+@org.osgi.annotation.versioning.Version("2.3.0")
 package org.apache.sling.engine;
 
diff --git a/src/test/java/org/apache/sling/engine/impl/filter/AbstractFilterTest.java b/src/test/java/org/apache/sling/engine/impl/filter/AbstractFilterTest.java
new file mode 100644 (file)
index 0000000..ec36ca0
--- /dev/null
@@ -0,0 +1,113 @@
+/*
+ * 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.sling.engine.impl.filter;
+
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.request.RequestPathInfo;
+import org.jmock.Expectations;
+import org.jmock.Mockery;
+import org.jmock.integration.junit4.JMock;
+import org.jmock.integration.junit4.JUnit4Mockery;
+import org.junit.runner.RunWith;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.ServiceReference;
+
+import javax.servlet.Filter;
+import java.util.HashMap;
+import java.util.Map;
+
+@RunWith(JMock.class)
+public abstract class AbstractFilterTest {
+    protected final Mockery context = new JUnit4Mockery();
+    protected ServiceReference<Filter> mockService(Object... map){
+
+        final Map props = new HashMap();
+        for (int i = 0;  i < map.length; i += 2){
+            props.put(map[i], map[i + 1]);
+        }
+
+        ServiceReference<Filter> ref = new ServiceReference<Filter>() {
+            @Override
+            public Object getProperty(String key) {
+                return props.get(key);
+            }
+
+            @Override
+            public String[] getPropertyKeys() {
+                return new String[0];
+            }
+
+            @Override
+            public Bundle getBundle() {
+                return null;
+            }
+
+            @Override
+            public Bundle[] getUsingBundles() {
+                return new Bundle[0];
+            }
+
+            @Override
+            public boolean isAssignableTo(Bundle bundle, String className) {
+                return false;
+            }
+
+            @Override
+            public int compareTo(Object reference) {
+                return 0;
+            }
+        };
+        return ref;
+    }
+    protected SlingHttpServletRequest mockRequest(final String path,
+                                                  final String extension,
+                                                  final String[] selectors,
+                                                  final String method,
+                                                  final String suffix
+                                                  ) {
+        final RequestPathInfo info = context.mock(RequestPathInfo.class, "info " + path + extension + method + suffix);
+        context.checking(new Expectations() {{
+            allowing(info).getExtension();
+            will(returnValue(extension));
+            allowing(info).getSuffix();
+            will(returnValue(suffix));
+            allowing(info).getSelectors();
+            will(returnValue(selectors == null ? new String[0] : selectors));
+            allowing(info).getResourcePath();
+            will(returnValue(path));
+        }});
+
+        final SlingHttpServletRequest req = context.mock(SlingHttpServletRequest.class, "req " + path + extension + method + suffix);
+        context.checking(new Expectations() {{
+            allowing(req).getRequestPathInfo();
+            will(returnValue(info));
+            allowing(req).getMethod();
+            will(returnValue(method));
+        }});
+        return req;
+    }
+    protected FilterPredicate predicate(Object... args){
+        FilterPredicate predicate = new FilterPredicate(mockService(args));
+        return predicate;
+    }
+
+    protected SlingHttpServletRequest whateverRequest() {
+        return mockRequest("/content/test/what/ever","json", new String[]{"test"}, "GET", null);
+    }
+}
diff --git a/src/test/java/org/apache/sling/engine/impl/filter/FilterHandleTest.java b/src/test/java/org/apache/sling/engine/impl/filter/FilterHandleTest.java
new file mode 100644 (file)
index 0000000..5d71da7
--- /dev/null
@@ -0,0 +1,40 @@
+/*
+ * 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.sling.engine.impl.filter;
+
+import org.junit.Test;
+
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_PATTERN;
+import static org.junit.Assert.*;
+
+public class FilterHandleTest extends AbstractFilterTest {
+
+    /**
+     * a filter with no predicate should be selected always, a filter with select only if predicate is good
+     */
+    @Test
+    public void testSelect(){
+        FilterHandle handle = new FilterHandle(null, predicate(), 0L, 0, "", null);
+        assertTrue("filter should be selected when no predicate", handle.select(mockRequest("/content/test/no/predicate", null, null, null, null)));
+        handle = new FilterHandle(null, predicate(SLING_FILTER_PATTERN,"/content/test/.*"), 0L, 0, "", null);
+        assertTrue("filter should be selected when matching predicate", handle.select(mockRequest("/content/test/matching/predicate", null, null, null, null)));
+        handle = new FilterHandle(null, predicate(SLING_FILTER_PATTERN,"/content/foo/.*"), 0L, 0, "", null);
+        assertFalse("filter should not be selected when no matching predicate", handle.select(mockRequest("/content/test/no/matching/predicate", null, null, null, null)));
+    }
+}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/engine/impl/filter/FilterPredicateTest.java b/src/test/java/org/apache/sling/engine/impl/filter/FilterPredicateTest.java
new file mode 100644 (file)
index 0000000..6b3c97b
--- /dev/null
@@ -0,0 +1,103 @@
+/*
+ * 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.sling.engine.impl.filter;
+
+import org.junit.Test;
+
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_EXTENSIONS;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_METHODS;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_PATTERN;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_SUFFIX_PATTERN;
+import static org.apache.sling.engine.EngineConstants.SLING_FILTER_SELECTORS;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class FilterPredicateTest extends AbstractFilterTest {
+
+    /**
+     * This one is especially important because it represents >99% of the requests
+     */
+    @Test
+    public void testNoPredicate() {
+        assertTrue("predicate with no recognised configuration should pass", predicate().test(whateverRequest()));
+    }
+
+    @Test
+    public void testPathPattern() {
+        FilterPredicate predicate = predicate(SLING_FILTER_PATTERN,"/content/test/.*");
+        assertTrue("/content/test/foo should be selected", predicate.test(mockRequest("/content/test/foo","json", null, null, null)));
+        assertFalse("/content/bar/foo should not be selected", predicate.test(mockRequest("/content/bar/foo","json", null, null, null)));
+    }
+
+    @Test
+    public void testExtensions() {
+        FilterPredicate predicate = predicate(SLING_FILTER_PATTERN,
+                "/content/test/.*",
+                SLING_FILTER_EXTENSIONS,
+                new String[]{"txt","xml"});
+        assertTrue("/content/test/foo.txt should be selected", predicate.test(mockRequest("/content/test/foo","txt", null, null, null)));
+        assertFalse("/content/test/foo.json should not be selected", predicate.test(mockRequest("/content/test/foo","json", null, null, null)));
+    }
+
+    @Test
+    public void testSuffix() {
+        FilterPredicate predicate = predicate(SLING_FILTER_PATTERN,
+                "/content/test/.*",
+                SLING_FILTER_SUFFIX_PATTERN,
+                "/foo/.*");
+        assertTrue("/content/test/foo /foo/bar should be selected", predicate.test(mockRequest("/content/test/foo",null, null, null, "/foo/bar")));
+        assertFalse("/content/test/foo /bar/foo should not be selected", predicate.test(mockRequest("/content/test/foo",null, null, null, "/bar/foo")));
+    }
+
+    @Test
+    public void testMethod() {
+        FilterPredicate predicate = predicate(SLING_FILTER_PATTERN,
+                "/content/test/.*",
+                SLING_FILTER_METHODS,
+                new String[]{"POST","PUT"});
+        assertTrue("POST /content/test/foo should be selected", predicate.test(mockRequest("/content/test/foo",null, null, "POST", null)));
+        assertFalse("GET /content/test/foo should not be selected", predicate.test(mockRequest("/content/test/foo",null, null, "GET", null)));
+    }
+
+    @Test
+    public void testSelectors() {
+        FilterPredicate predicate = predicate(SLING_FILTER_PATTERN,
+                "/content/test/.*",
+                SLING_FILTER_SELECTORS,
+                new String[]{"test","foo","bar"});
+        assertTrue("POST /content/test/foo.foo.test.someother.json should be selected", predicate.test(mockRequest("/content/test/two","json", new String[]{"foo","test","someother"}, "POST", null)));
+        assertTrue("POST /content/test/foo.test.someother.json should be selected", predicate.test(mockRequest("/content/test/one","json", new String[]{"test","someother"}, "PUT", null)));
+        assertFalse("GET /content/test/foo.json should not be selected", predicate.test(mockRequest("/content/test/no","json",  null, "GET", null)));
+    }
+
+    @Test
+    public void testRootWithNoPath() {
+        FilterPredicate predicate = predicate(SLING_FILTER_PATTERN,"/");
+        assertTrue("'' path based request should be selected with a slash", predicate.test(mockRequest("",null, null, null, null)));
+    }
+
+    @Test
+    public void testToString() {
+        FilterPredicate predicate = predicate(SLING_FILTER_PATTERN, "/content/test",SLING_FILTER_EXTENSIONS, new String[]{"json"}, SLING_FILTER_METHODS, new String[]{"GET"});
+        String patternString = predicate.toString();
+        assertTrue("there should be /content/test in the string representation", patternString.contains("/content/test"));
+        assertTrue("there should be json in the string representation", patternString.contains("json"));
+        assertTrue("there should be GET in the string representation", patternString.contains("GET"));
+    }
+}
\ No newline at end of file