Merge pull request #15 from bosschaert/placeholder-regex2

SLING-10482 Make it possible to validate values with property placeholders
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/InternalConstants.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/InternalConstants.java
index 8954115..e1e4979 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/InternalConstants.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/InternalConstants.java
@@ -78,4 +78,6 @@
     static final String KEY_MODE = "mode";
 
     static final String KEY_PLACEHOLDER_POLICY = "placeholder-policy";
+
+    static final String KEY_PLACEHOLDER_REGEX = "placeholder-regex";
 }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
index 1be609f..1441e7c 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
@@ -81,6 +81,12 @@
      */
     private PlaceholderPolicy placeholderPolicy;
 
+	/**
+     * A pattern to validate values which contain substitution placeholders
+     * @since 1.3
+     */
+	private Pattern placeholderPattern;
+
     /**
      * Create a new description
      */
@@ -175,6 +181,7 @@
 			if ( policyVal != null ) {
                 this.setPlaceholderPolicy(PlaceholderPolicy.valueOf(policyVal.toUpperCase()));
 			}
+			this.setPlaceholderRegex(this.getString(InternalConstants.KEY_PLACEHOLDER_REGEX));
  		} catch (final JsonException | IllegalArgumentException e) {
             throw new IOException(e);
         }
@@ -235,6 +242,7 @@
         if ( this.getPlaceholderPolicy() != PlaceholderPolicy.DEFAULT ) {
             objectBuilder.add(InternalConstants.KEY_PLACEHOLDER_POLICY, this.getPlaceholderPolicy().name());
         }
+        this.setString(objectBuilder, InternalConstants.KEY_PLACEHOLDER_REGEX, this.getPlaceholderRegex());
 
         return objectBuilder;
 	}
@@ -457,4 +465,36 @@
     public void setPlaceholderPolicy(final PlaceholderPolicy policy) {
         this.placeholderPolicy = policy == null ? PlaceholderPolicy.DEFAULT : policy;
     }
+
+	/**
+	 * Get the placeholder regex
+	 * @return the placeholder regex or {@code null}
+     * @since 1.3
+	 */
+    public String getPlaceholderRegex() {
+        return placeholderPattern == null ? null : placeholderPattern.pattern();
+    }
+
+	/**
+	 * Set the placeholder regex
+	 * @param regex the placeholder regex to set
+     * @throws IllegalArgumentException If the pattern is not valid
+     * @since 1.3
+	 */
+    public void setPlaceholderRegex(final String regex) {
+        if ( regex == null ) {
+            this.placeholderPattern = null;
+        } else {
+            this.placeholderPattern = Pattern.compile(regex);
+        }
+    }
+
+    /**
+     * Get the placeholder regex pattern
+     * @return The pattern or {@code null}
+     * @since 1.3
+     */
+    public Pattern getPlaceholderRegexPattern() {
+        return this.placeholderPattern;
+    }
 }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/package-info.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/package-info.java
index 64d49cb..b8a44cf 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/package-info.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/package-info.java
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-@org.osgi.annotation.versioning.Version("1.2.1")
+@org.osgi.annotation.versioning.Version("1.3.0")
 package org.apache.sling.feature.extension.apiregions.api.config;
 
 
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidator.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidator.java
index 728945e..1039be4 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidator.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidator.java
@@ -23,6 +23,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.regex.Pattern;
 
 import org.apache.sling.feature.extension.apiregions.api.config.Mode;
 import org.apache.sling.feature.extension.apiregions.api.config.Option;
@@ -215,7 +216,7 @@
                                 break;
                     default : context.result.getErrors().add("Unable to validate value - unknown property type : " + context.description.getType());
                 }
-                validateRegex(context, value);
+                validateRegex(context, context.description.getRegexPattern(), value);
                 validateOptions(context, value);
                 if ( context.description.getType() != PropertyType.PASSWORD ) {
                     validatePlaceholderPolicy(context, value, false);              
@@ -225,7 +226,9 @@
                 if ( context.description.getType() == PropertyType.PASSWORD ) {
                     validatePassword(context, value, true);
                 } else if ( context.description.getType() == PropertyType.STRING ) {
-                    // any string is valid, we only mark the result as skipped if a regex or options are set
+                    validateRegex(context, context.description.getPlaceholderRegexPattern(), value);
+
+                    // we mark the result as skipped if a regex or options are set or if a value is marked as required.
                     if ( context.description.getRegex() != null || context.description.getOptions() != null || context.description.isRequired() ) {
                         context.result.markSkipped();
                     }
@@ -452,10 +455,10 @@
 		}
 	}
 
-    void validateRegex(final Context context, final Object value) {
-        if ( context.description.getRegexPattern() != null ) {
-            if ( !context.description.getRegexPattern().matcher(value.toString()).matches() ) {
-                setResult(context, "Value " + value + " does not match regex " + context.description.getRegex());
+    void validateRegex(final Context context, final Pattern pattern, final Object value) {
+        if ( pattern != null ) {
+            if ( !pattern.matcher(value.toString()).matches() ) {
+                setResult(context, "Value " + value + " does not match regex " + pattern.pattern());
             }
         }
     }
@@ -494,4 +497,4 @@
 
         public Mode validationMode;
     }
-}
\ No newline at end of file
+}
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescriptionTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescriptionTest.java
index ed5f02f..086e0a3 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescriptionTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescriptionTest.java
@@ -50,7 +50,7 @@
         entity.setRegex(".");
         entity.setRequired(true);
         entity.setVariable("var");
-        entity.setType(PropertyType.BYTE);        
+        entity.setType(PropertyType.BYTE);
         entity.setDefaultValue("default");
         entity.setMode(Mode.SILENT);
         entity.setPlaceholderPolicy(PlaceholderPolicy.ALLOW);
@@ -78,7 +78,7 @@
         final Extension ext = new Extension(ExtensionType.JSON, "a", ExtensionState.OPTIONAL);
         ext.setJSON("{ \"type\" : \"BYTE\", \"cardinality\": 5, \"required\" : true, \"variable\" : \"var\"," +
         "\"range\" : {}, \"includes\" : [\"in\"], \"excludes\" : [\"ex\"] , \"options\": [{}], \"regex\": \".\"," +
-        "\"default\" : \"def\", \"placeholder-policy\" : \"DENY\"}");
+        "\"default\" : \"def\", \"placeholder-policy\" : \"DENY\", \"placeholder-regex\": \"my-regex\"}");
 
         final PropertyDescription entity = new PropertyDescription();
         entity.fromJSONObject(ext.getJSONStructure().asJsonObject());
@@ -95,6 +95,7 @@
         assertNotNull(entity.getRegexPattern());
         assertEquals("def", entity.getDefaultValue());
         assertEquals(PlaceholderPolicy.DENY, entity.getPlaceholderPolicy());
+        assertEquals("my-regex", entity.getPlaceholderRegex());
 
         // test defaults and empty values
         ext.setJSON("{ \"variable\" : \"var\", \"regex\": \".\"}");
@@ -111,6 +112,7 @@
         assertEquals(".", entity.getRegex());
         assertNotNull(entity.getRegexPattern());
         assertEquals(PlaceholderPolicy.DEFAULT, entity.getPlaceholderPolicy());
+        assertNull(entity.getPlaceholderRegex());
    }
 
     @Test public void testToJSONObject() throws IOException {
@@ -126,11 +128,12 @@
         entity.setType(PropertyType.BYTE);
         entity.setDefaultValue("def");
         entity.setPlaceholderPolicy(PlaceholderPolicy.DENY);
+        entity.setPlaceholderRegex("^.*$");
 
         final Extension ext = new Extension(ExtensionType.JSON, "a", ExtensionState.OPTIONAL);
         ext.setJSON("{ \"type\" : \"BYTE\", \"cardinality\": 5, \"required\" : true, \"variable\" : \"var\"," +
             "\"range\" : {}, \"includes\" : [\"in\"], \"excludes\" : [\"ex\"] , \"options\": [{}], \"regex\": \".\"," +
-            "\"default\" : \"def\", \"placeholder-policy\" : \"DENY\"}");
+            "\"default\" : \"def\", \"placeholder-policy\" : \"DENY\", \"placeholder-regex\": \"^.*$\"}");
 
         assertEquals(ext.getJSONStructure().asJsonObject(), entity.toJSONObject());
 
@@ -144,7 +147,8 @@
         entity.setIncludes(null);
         entity.setDefaultValue(null);
         entity.setPlaceholderPolicy(null);
-        
+        entity.setPlaceholderRegex(null);
+
         ext.setJSON("{ \"variable\" : \"var\", \"regex\": \".\"}");
 
         assertEquals(ext.getJSONStructure().asJsonObject(), entity.toJSONObject());
@@ -185,4 +189,4 @@
         entity.fromJSONObject(ext.getJSONStructure().asJsonObject());
         assertEquals(Mode.SILENT, entity.getMode());
     }
-}
\ No newline at end of file
+}
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidatorTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidatorTest.java
index e3a8333..29fc3cc 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidatorTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidatorTest.java
@@ -509,6 +509,23 @@
         assertFalse(result.isSkipped());
     }
 
+    @Test
+    public void testPropertyDescRegex() {
+        final PropertyDescription desc = new PropertyDescription();
+        desc.setPlaceholderPolicy(PlaceholderPolicy.ALLOW);
+        desc.setPlaceholderRegex("^\\w+ [^ ]+$");
+        PropertyValidationResult result = validator.validate("local $[env:AEM_EXTERNALIZER_LOCAL;default=http://localhost:4502]", desc);
+        assertTrue(result.isValid());
+        assertFalse(result.isSkipped());
+
+        result = validator.validate("author $[env:AEM_EXTERNALIZER_LOCAL;default=http://localhost:4502] http://abc.def.com:9091", desc);
+        assertFalse(result.isValid());
+        assertFalse(result.isSkipped());
+
+        result = validator.validate("local http://abc.ghi.com:9091", desc);
+        assertTrue(result.isValid());
+        assertFalse(result.isSkipped());
+    }
 
     @Test
     public void testLiveValidation() {
@@ -534,4 +551,4 @@
             this.validator.setLiveValues(false);
         }
     }
-}
\ No newline at end of file
+}