You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2014/11/13 11:29:49 UTC

svn commit: r1639292 - in /sling/trunk/contrib/validation: api/src/main/java/org/apache/sling/validation/api/ core/ core/src/main/java/org/apache/sling/validation/impl/ core/src/main/java/org/apache/sling/validation/impl/util/ core/src/test/java/org/ap...

Author: kwin
Date: Thu Nov 13 10:29:48 2014
New Revision: 1639292

URL: http://svn.apache.org/r1639292
Log:
SLING-4158, make applicablePaths optional
log in case invalid applicablePaths are provided from a model
refactoring of TrieTest

Modified:
    sling/trunk/contrib/validation/api/src/main/java/org/apache/sling/validation/api/ValidationModel.java
    sling/trunk/contrib/validation/core/pom.xml
    sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/JCRValidationModel.java
    sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/ValidationServiceImpl.java
    sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/util/Trie.java
    sling/trunk/contrib/validation/core/src/test/java/org/apache/sling/validation/impl/util/TrieTest.java

Modified: sling/trunk/contrib/validation/api/src/main/java/org/apache/sling/validation/api/ValidationModel.java
URL: http://svn.apache.org/viewvc/sling/trunk/contrib/validation/api/src/main/java/org/apache/sling/validation/api/ValidationModel.java?rev=1639292&r1=1639291&r2=1639292&view=diff
==============================================================================
--- sling/trunk/contrib/validation/api/src/main/java/org/apache/sling/validation/api/ValidationModel.java (original)
+++ sling/trunk/contrib/validation/api/src/main/java/org/apache/sling/validation/api/ValidationModel.java Thu Nov 13 10:29:48 2014
@@ -41,7 +41,9 @@ public interface ValidationModel {
     String getValidatedResourceType();
 
     /**
-     * Returns the paths under which resources will be validated by this model.
+     * Returns the paths under which resources will be validated by this model. 
+     * Is never null nor an empty array. Might return a single element array containing only the empty string, 
+     * in which case the validation model has no path restriction.
      *
      * @return a path array
      */

Modified: sling/trunk/contrib/validation/core/pom.xml
URL: http://svn.apache.org/viewvc/sling/trunk/contrib/validation/core/pom.xml?rev=1639292&r1=1639291&r2=1639292&view=diff
==============================================================================
--- sling/trunk/contrib/validation/core/pom.xml (original)
+++ sling/trunk/contrib/validation/core/pom.xml Thu Nov 13 10:29:48 2014
@@ -143,6 +143,12 @@
             <scope>provided</scope>
         </dependency>
         <dependency>
+            <groupId>commons-lang</groupId>
+            <artifactId>commons-lang</artifactId>
+            <version>2.5</version>
+            <scope>provided</scope>
+        </dependency>
+        <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
             <scope>test</scope>

Modified: sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/JCRValidationModel.java
URL: http://svn.apache.org/viewvc/sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/JCRValidationModel.java?rev=1639292&r1=1639291&r2=1639292&view=diff
==============================================================================
--- sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/JCRValidationModel.java (original)
+++ sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/JCRValidationModel.java Thu Nov 13 10:29:48 2014
@@ -21,6 +21,7 @@ package org.apache.sling.validation.impl
 import java.util.List;
 import java.util.Set;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.sling.validation.api.ChildResource;
 import org.apache.sling.validation.api.ResourceProperty;
 import org.apache.sling.validation.api.ValidationModel;
@@ -38,7 +39,21 @@ public class JCRValidationModel implemen
         this.jcrPath = jcrPath;
         this.resourceProperties = resourceProperties;
         this.validatedResourceType = validatedResourceType;
-        this.applicablePaths = applicablePaths;
+        // if this property was not set
+        if (applicablePaths == null) {
+            // set this to the empty string (which matches all paths)
+            this.applicablePaths = new String[] {""};
+        } else {
+            if (applicablePaths.length == 0) {
+                throw new IllegalArgumentException("applicablePaths cannot be an empty array!");
+            }
+            for (String applicablePath : applicablePaths) {
+                if (StringUtils.isBlank(applicablePath)) {
+                    throw new IllegalArgumentException("applicablePaths may not contain empty values!");
+                }
+            }
+            this.applicablePaths = applicablePaths;
+        }
         this.children = children;
     }
 

Modified: sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/ValidationServiceImpl.java
URL: http://svn.apache.org/viewvc/sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/ValidationServiceImpl.java?rev=1639292&r1=1639291&r2=1639292&view=diff
==============================================================================
--- sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/ValidationServiceImpl.java (original)
+++ sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/ValidationServiceImpl.java Thu Nov 13 10:29:48 2014
@@ -18,6 +18,7 @@
  */
 package org.apache.sling.validation.impl;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.Service;
@@ -50,6 +51,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.jcr.query.Query;
+
 import java.util.Dictionary;
 import java.util.Hashtable;
 import java.util.Iterator;
@@ -283,6 +285,9 @@ public class ValidationServiceImpl imple
         Trie<JCRValidationModel> modelsForResourceType = null;
         ResourceResolver rr = null;
         JCRValidationModel vm;
+        if (StringUtils.isBlank(validatedResourceType)) {
+            throw new IllegalArgumentException("validatedResourceType cannot be null or blank!");
+        }
         try {
             rr = rrf.getAdministrativeResourceResolver(null);
             String[] searchPaths = rr.getSearchPath();
@@ -297,10 +302,10 @@ public class ValidationServiceImpl imple
                     Resource model = models.next();
                     LOG.info("Found validation model resource {}.", model.getPath());
                     String jcrPath = model.getPath();
-                    ValueMap validationModelProperties = model.adaptTo(ValueMap.class);
-                    String[] applicablePaths = PropertiesUtil.toStringArray(validationModelProperties.get(Constants.APPLICABLE_PATHS,
-                            String[].class));
-                    if (validatedResourceType != null && !"".equals(validatedResourceType)) {
+                    try {
+                        ValueMap validationModelProperties = model.adaptTo(ValueMap.class);
+                        String[] applicablePaths = PropertiesUtil.toStringArray(validationModelProperties.get(Constants.APPLICABLE_PATHS,
+                                String[].class));
                         Resource r = model.getChild(Constants.PROPERTIES);
                         if (r != null) {
                             Set<ResourceProperty> resourceProperties = JCRBuilder.buildProperties(validatorLookupService, r);
@@ -312,7 +317,7 @@ public class ValidationServiceImpl imple
                                  * if the modelsForResourceType is null the canAcceptModel will return true: performance optimisation so that
                                  * the Trie is created only if the model is accepted
                                  */
-
+    
                                 if (canAcceptModel(vm, searchPath, searchPaths, modelsForResourceType)) {
                                     if (modelsForResourceType == null) {
                                         modelsForResourceType = new Trie<JCRValidationModel>();
@@ -324,6 +329,8 @@ public class ValidationServiceImpl imple
                                 }
                             }
                         }
+                    } catch (IllegalArgumentException e) {
+                        LOG.error("Found invalid validation model in '{}': {}", jcrPath, e.getMessage());
                     }
                 }
             }

Modified: sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/util/Trie.java
URL: http://svn.apache.org/viewvc/sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/util/Trie.java?rev=1639292&r1=1639291&r2=1639292&view=diff
==============================================================================
--- sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/util/Trie.java (original)
+++ sling/trunk/contrib/validation/core/src/main/java/org/apache/sling/validation/impl/util/Trie.java Thu Nov 13 10:29:48 2014
@@ -34,14 +34,16 @@ public class Trie<T> {
     /**
      * Inserts an object {@link T} under the specified {@code key}.
      *
-     * @param key   the key under which the object will be stored
+     * @param key   the key under which the object will be stored. If empty String, the value of the root node will be overwritten!
      * @param value the object to be stored
      */
     public void insert(String key, T value) {
-        if (key != null && !"".equals(key)) {
-            int length = key.length();
-            TrieNode<T> node = ROOT;
-            for (int index = 0; index < length; index++) {
+        if (key == null) {
+            throw new IllegalArgumentException("Key cannot be null!");
+        }
+        int length = key.length();
+        TrieNode<T> node = ROOT;
+        for (int index = 0; index < length; index++) {
                 Map<Character, TrieNode<T>> children = node.getChildren();
                 char character = key.charAt(index);
                 node = children.get(character);
@@ -49,10 +51,9 @@ public class Trie<T> {
                     node = new TrieNode<T>(character);
                     children.put(character, node);
                 }
-            }
-            node.setLeaf(true);
-            node.setValue(value);
         }
+        node.setLeaf(true);
+        node.setValue(value);
     }
 
     /**

Modified: sling/trunk/contrib/validation/core/src/test/java/org/apache/sling/validation/impl/util/TrieTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/contrib/validation/core/src/test/java/org/apache/sling/validation/impl/util/TrieTest.java?rev=1639292&r1=1639291&r2=1639292&view=diff
==============================================================================
--- sling/trunk/contrib/validation/core/src/test/java/org/apache/sling/validation/impl/util/TrieTest.java (original)
+++ sling/trunk/contrib/validation/core/src/test/java/org/apache/sling/validation/impl/util/TrieTest.java Thu Nov 13 10:29:48 2014
@@ -18,48 +18,67 @@
  */
 package org.apache.sling.validation.impl.util;
 
+import org.hamcrest.Matchers;
 import org.junit.Before;
 import org.junit.Test;
 
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertThat;
 
 public class TrieTest {
 
-    private Trie<Object> dictionary;
+    private Trie<String> dictionary;
 
     @Before
     public void setUp() {
-        dictionary = new Trie<Object>();
+        dictionary = new Trie<String>();
         dictionary.insert("/apps/example", "/apps/example");
         dictionary.insert("/apps/examples/node/jcr:content", "/apps/examples/node/jcr:content");
         dictionary.insert("/apps/examples/node/jcr:content/nodes", "/apps/examples/node/jcr:content/nodes");
     }
 
+    @Test(expected=IllegalArgumentException.class)
+    public void testInsertNullValue() {
+        dictionary.insert(null, "invalid");
+    }
+    
+    @Test
+    public void testOverwriteRootNode() {
+        TrieNode<String> node = dictionary.getElement("invalid key");
+        assertThat(node.getValue(), Matchers.equalTo(dictionary.ROOT.getValue()));
+        dictionary.insert("", "new root value");
+        node = dictionary.getElement("invalid key");
+        assertThat(node.getValue(), Matchers.equalTo("new root value"));
+    }
+    
     @Test
     public void testLongestMatchingKey() throws Exception {
-        TrieNode<Object> node;
+        TrieNode<String> node;
         node = dictionary.getElementForLongestMatchingKey("/apps/examples/node/jcr:content/nodes/1");
-        assertTrue("/apps/examples/node/jcr:content/nodes".equals(node.getValue()));
+        assertThat(node.getValue(), Matchers.equalTo("/apps/examples/node/jcr:content/nodes"));
 
         node = dictionary.getElementForLongestMatchingKey("/apps/example/node/jcr:content/nodes/1");
-        assertTrue("/apps/example".equals(node.getValue()));
+        assertThat(node.getValue(), Matchers.equalTo("/apps/example"));
 
         node = dictionary.getElementForLongestMatchingKey("/libs");
-        assertTrue(node.getValue() == null);
+        assertThat(node, Matchers.equalTo(dictionary.ROOT));
+        
+        // test empty key (not allowed!!!)
+        dictionary.insert("", "emptyKey");
+        node = dictionary.getElementForLongestMatchingKey("/apps/test");
+        assertThat(node.getValue(), Matchers.equalTo("emptyKey"));
     }
 
     @Test
     public void testExactKey() {
-        TrieNode<Object> node;
-
+        TrieNode<String> node;
         node = dictionary.getElement("/apps/examples/node/jcr:content/nodes");
-        assertTrue("/apps/examples/node/jcr:content/nodes".equals(node.getValue()));
+        assertThat(node.getValue(), Matchers.equalTo("/apps/examples/node/jcr:content/nodes"));
 
         node = dictionary.getElement("/apps/example");
-        assertTrue("/apps/example".equals(node.getValue()));
+        assertThat(node.getValue(), Matchers.equalTo("/apps/example"));
 
         node = dictionary.getElement("/libs");
-        assertTrue(dictionary.ROOT.equals(node));
+        assertThat(node, Matchers.equalTo(dictionary.ROOT));
     }
 
 }