[sling-org-apache-sling-pipes] branch master updated: SLING-9979 Filter pipe improvements

[npeltier]

This is an automated email from the ASF dual-hosted git repository.

npeltier pushed a commit to branch master
in repository 
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-pipes.git


The following commit(s) were added to refs/heads/master by this push:
     new e861c70  SLING-9979 Filter pipe improvements
e861c70 is described below

commit e861c7009910916fcff64ec86166aa3c62290171
Author: Nicolas Peltier <npelt...@apache.org>
AuthorDate: Sat Dec 5 11:32:13 2020 +0100

    SLING-9979 Filter pipe improvements
    
    - fix NPE,
    - cache patterns,
    - better test coverages
---
 .../apache/sling/pipes/internal/FilterPipe.java    | 25 +++++++++++++++++-----
 .../sling/pipes/internal/FilterPipeTest.java       | 15 ++++++++++++-
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/apache/sling/pipes/internal/FilterPipe.java 
b/src/main/java/org/apache/sling/pipes/internal/FilterPipe.java
index 6bb115e..cfedb69 100644
--- a/src/main/java/org/apache/sling/pipes/internal/FilterPipe.java
+++ b/src/main/java/org/apache/sling/pipes/internal/FilterPipe.java
@@ -28,7 +28,9 @@ import org.slf4j.LoggerFactory;
 import javax.jcr.Node;
 import javax.jcr.NodeIterator;
 import javax.jcr.RepositoryException;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.regex.Pattern;
 
 /**
@@ -43,24 +45,37 @@ public class FilterPipe extends BasePipe {
     public static final String PN_TEST = PREFIX_FILTER + "test";
     public static final String PN_INJECTCHILDRENCOUNT = PREFIX_FILTER + 
"injectChildrenCount";
     public static final String BINDING_CHILDREN_COUNT = "childrenCount";
+    Map<String, Pattern> propertiesPatterns;
 
     public FilterPipe(Plumber plumber, Resource resource, PipeBindings 
upperBindings) {
         super(plumber, resource, upperBindings);
     }
 
-    boolean propertiesPass(ValueMap current, ValueMap filter) {
+    Pattern getPattern(Resource filterResource, final String propertyKey) {
+        if (propertiesPatterns == null) {
+            propertiesPatterns = new HashMap<>();
+        }
+        String key = filterResource.getPath() + SLASH + propertyKey;
+        return propertiesPatterns.computeIfAbsent(key, x -> {
+            String value = filterResource.getValueMap().get(propertyKey, 
String.class);
+            return value != null ? 
Pattern.compile(filterResource.getValueMap().get(propertyKey, String.class))  : 
null;
+        });
+    }
+
+    boolean propertiesPass(ValueMap current, Resource filterResource) {
+        ValueMap filter = filterResource.getValueMap();
         if (filter.containsKey(PN_TEST)){
             Object test = bindings.instantiateObject(filter.get(PN_TEST, 
PipeBindings.FALSE_BINDING));
             if (! (test instanceof Boolean)){
-                logger.error("instatiated test {} is not a boolean, filtering 
out", test);
+                logger.error("instantiated test {} is not a boolean, filtering 
out", test);
                 return false;
             }
             return (Boolean) test;
         }
         for (String key : filter.keySet()){
             if (! IGNORED_PROPERTIES.contains(key) && 
!key.startsWith(PREFIX_FILTER)){
-                Pattern pattern = Pattern.compile(filter.get(key, 
String.class));
-                if (!pattern.matcher(current.get(key, String.class)).find()){
+                Pattern pattern = getPattern(filterResource, key);
+                if (!current.containsKey(key) || 
!pattern.matcher(current.get(key, String.class)).find()){
                     return false;
                 }
             }
@@ -114,7 +129,7 @@ public class FilterPipe extends BasePipe {
                     bindings.addBinding(BINDING_CHILDREN_COUNT, childrenCount);
                 }
             }
-            if (propertiesPass(current, filter)) {
+            if (propertiesPass(current, filterResource)) {
                 return childrenPass(currentResource, filterResource);
             }
         }
diff --git a/src/test/java/org/apache/sling/pipes/internal/FilterPipeTest.java 
b/src/test/java/org/apache/sling/pipes/internal/FilterPipeTest.java
index 5c22e43..ed86e92 100644
--- a/src/test/java/org/apache/sling/pipes/internal/FilterPipeTest.java
+++ b/src/test/java/org/apache/sling/pipes/internal/FilterPipeTest.java
@@ -22,9 +22,11 @@ import org.apache.sling.api.resource.PersistenceException;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ValueMap;
 import org.apache.sling.pipes.AbstractPipeTest;
+import org.apache.sling.pipes.ExecutionResult;
 import org.apache.sling.pipes.Pipe;
 import org.junit.Test;
 
+import java.lang.reflect.InvocationTargetException;
 import java.util.Collection;
 import java.util.Iterator;
 
@@ -65,7 +67,6 @@ public class FilterPipeTest extends AbstractPipeTest {
         assertFalse("...and only One", resourceIterator.hasNext());
     }
 
-
     @Test
     public void testNoChildrenFails(){
         assertFalse("output has no resource...", getOutput(PATH_PIPE + "/" + 
NN_NOCHILDREN_FAILS).hasNext());
@@ -105,6 +106,18 @@ public class FilterPipeTest extends AbstractPipeTest {
     }
 
     @Test
+    public void testMultiplePattern() throws InvocationTargetException, 
IllegalAccessException {
+        ExecutionResult result = execute("echo /content/fruits/apple/isnota | 
children nt:unstructured | grep sling:resourceType=.+ @ with 
slingPipesFilter_not=true");
+        assertTrue("there should be some results", result.size() > 0);
+    }
+
+    @Test
+    public void testBadTestFails() throws InvocationTargetException, 
IllegalAccessException {
+        ExecutionResult result = execute("echo /content/fruits | grep 
slingPipesFilter_test=${'some string'}");
+        assertEquals(0, result.size());
+    }
+
+    @Test
     public void testPropertyRegexp() throws Exception {
         Collection<String> outputs = 
plumber.newPipe(context.resourceResolver())
             .echo(PATH_APPLE)