SLING-9952 : Skip validation of properties if they contain a placeholder
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 f07df67..584db9c 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
@@ -220,10 +220,18 @@
/**
* Set the cardinality
- * @param cardinality the cardinality to set
+ * The default cardinality is {@code 1}. If the value is greater than zero
+ * the property can contain up to that number of values.
+ * If the cardinality is {@code -1} the property can hold an unlimited number
+ * of values.
+ * @param value the cardinality to set
+ * @throws IllegalArgumentException If the value is {@code 0} or below {@code -1}.
*/
- public void setCardinality(final int cardinality) {
- this.cardinality = cardinality;
+ public void setCardinality(final int value) {
+ if ( value == 0 || value < -1 ) {
+ throw new IllegalArgumentException();
+ }
+ this.cardinality = value;
}
/**
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 a9d5554..ecfdb5a 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
@@ -26,6 +26,7 @@
import org.apache.sling.feature.extension.apiregions.api.config.Option;
import org.apache.sling.feature.extension.apiregions.api.config.PropertyDescription;
+import org.apache.sling.feature.extension.apiregions.api.config.PropertyType;
/**
* Validate a configuration property or framework property
@@ -122,61 +123,80 @@
private static final List<String> PLACEHOLDERS = Arrays.asList("$[env:", "$[secret:", "$[prop:");
- void validateValue(final PropertyDescription prop, final Object value, final PropertyValidationResult result) {
+ void validateValue(final PropertyDescription desc, final Object value, final PropertyValidationResult result) {
final List<String> messages = result.getErrors();
if ( value != null ) {
// check for placeholder
- boolean checkValue = true;
+ boolean hasPlaceholder = false;
if ( value instanceof String ) {
final String strVal = (String)value;
for(final String p : PLACEHOLDERS) {
if ( strVal.contains(p) ) {
- checkValue = false;
+ hasPlaceholder = true;
break;
}
}
}
- if ( checkValue ) {
- switch ( prop.getType() ) {
- case BOOLEAN : validateBoolean(prop, value, messages);
+ if ( !hasPlaceholder ) {
+ switch ( desc.getType() ) {
+ case BOOLEAN : validateBoolean(desc, value, messages);
break;
- case BYTE : validateByte(prop, value, messages);
+ case BYTE : validateByte(desc, value, messages);
break;
- case CHARACTER : validateCharacter(prop, value, messages);
+ case CHARACTER : validateCharacter(desc, value, messages);
break;
- case DOUBLE : validateDouble(prop, value, messages);
+ case DOUBLE : validateDouble(desc, value, messages);
break;
- case FLOAT : validateFloat(prop, value, messages);
+ case FLOAT : validateFloat(desc, value, messages);
break;
- case INTEGER : validateInteger(prop, value, messages);
+ case INTEGER : validateInteger(desc, value, messages);
break;
- case LONG : validateLong(prop, value, messages);
+ case LONG : validateLong(desc, value, messages);
break;
- case SHORT : validateShort(prop, value, messages);
+ case SHORT : validateShort(desc, value, messages);
break;
- case STRING : // no special validation for string
+ case STRING : validateRequired(desc, value, messages);
break;
- case EMAIL : validateEmail(prop, value, messages);
+ case EMAIL : validateEmail(desc, value, messages);
break;
- case PASSWORD : validatePassword(prop, value, messages);
+ case PASSWORD : validatePassword(desc, value, messages, false);
break;
- case URL : validateURL(prop, value, messages);
+ case URL : validateURL(desc, value, messages);
break;
- case PATH : validatePath(prop, value, messages);
+ case PATH : validatePath(desc, value, messages);
break;
- default : messages.add("Unable to validate value - unknown property type : " + prop.getType());
+ default : messages.add("Unable to validate value - unknown property type : " + desc.getType());
}
- validateRegex(prop, value, messages);
- validateOptions(prop, value, messages);
+ validateRegex(desc, value, messages);
+ validateOptions(desc, value, messages);
} else {
- result.markSkipped();
+ // placeholder is present
+ if ( desc.getType() == PropertyType.PASSWORD ) {
+ validatePassword(desc, value, messages, true);
+ } else if ( desc.getType() == PropertyType.STRING ) {
+ // any string is valid, we only mark the result as skipped if a regex or options are set
+ if ( desc.getRegex() != null || desc.getOptions() != null || desc.isRequired() ) {
+ result.markSkipped();
+ }
+ } else {
+ result.markSkipped();
+ }
}
} else {
messages.add("Null value provided for validation");
}
}
- void validateBoolean(final PropertyDescription prop, final Object value, final List<String> messages) {
+ void validateRequired(final PropertyDescription prop, final Object value, final List<String> messages) {
+ if ( prop.isRequired() ) {
+ final String val = value.toString();
+ if ( val.isEmpty() ) {
+ messages.add("Value is required");
+ }
+ }
+ }
+
+ void validateBoolean(final PropertyDescription prop, final Object value, final List<String> messages) {
if ( ! (value instanceof Boolean) ) {
if ( value instanceof String ) {
final String v = (String)value;
@@ -333,10 +353,10 @@
}
}
- void validatePassword(final PropertyDescription prop, final Object value, final List<String> messages) {
- if ( prop.getVariable() == null ) {
- messages.add("Value for a password must use a variable");
- }
+ void validatePassword(final PropertyDescription desc, final Object value, final List<String> messages, final boolean hasPlaceholder) {
+ if ( !hasPlaceholder ) {
+ messages.add("Value for a password must use a placeholder");
+ }
}
void validatePath(final PropertyDescription prop, final Object value, final List<String> messages) {
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 d9b50c2..5e64e62 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
@@ -22,6 +22,7 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import java.io.IOException;
import java.util.Arrays;
@@ -133,4 +134,27 @@
assertEquals(ext.getJSONStructure().asJsonObject(), entity.toJSONObject());
}
+
+ @Test public void testSetCardinality() {
+ final PropertyDescription desc = new PropertyDescription();
+ desc.setCardinality(5);
+ assertEquals(5, desc.getCardinality());
+ desc.setCardinality(1);
+ assertEquals(1, desc.getCardinality());
+ desc.setCardinality(-1);
+ assertEquals(-1, desc.getCardinality());
+
+ try {
+ desc.setCardinality(0);
+ fail();
+ } catch ( final IllegalArgumentException iae) {
+ // expected
+ }
+ try {
+ desc.setCardinality(-2);
+ fail();
+ } catch ( final IllegalArgumentException iae) {
+ // expected
+ }
+ }
}
\ 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 fd8d1c0..dd544bd 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
@@ -21,6 +21,7 @@
import static org.junit.Assert.assertTrue;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import org.apache.sling.feature.extension.apiregions.api.config.Option;
@@ -278,8 +279,10 @@
assertTrue(result.isValid());
assertFalse(result.isSkipped());
- prop.setVariable("secret");
- result = validator.validate(null, prop);
+ result = validator.validate("secret", prop);
+ assertFalse(result.isValid());
+
+ result = validator.validate("$[secret:dbpassword]", prop);
assertTrue(result.isValid());
assertFalse(result.isSkipped());
}
@@ -299,6 +302,52 @@
assertFalse(result.isSkipped());
}
+ @Test public void testValidateString() {
+ final PropertyDescription desc = new PropertyDescription();
+ PropertyValidationResult result;
+
+ result = validator.validate("hello world", desc);
+ assertTrue(result.isValid());
+ assertFalse(result.isSkipped());
+
+ result = validator.validate("$[prop:KEY]", desc);
+ assertTrue(result.isValid());
+ assertFalse(result.isSkipped());
+
+ // skip if required
+ desc.setRequired(true);
+ result = validator.validate("$[prop:KEY]", desc);
+ assertTrue(result.isValid());
+ assertTrue(result.isSkipped());
+ desc.setRequired(false);
+
+ // skip if options
+ desc.setOptions(Collections.singletonList(new Option()));
+ result = validator.validate("$[prop:KEY]", desc);
+ assertTrue(result.isValid());
+ assertTrue(result.isSkipped());
+ desc.setOptions(null);
+
+ // skip if regexp
+ desc.setRegex(".*");
+ result = validator.validate("$[prop:KEY]", desc);
+ assertTrue(result.isValid());
+ assertTrue(result.isSkipped());
+ desc.setRegex(null);
+
+ // empty string - not required
+ result = validator.validate("", desc);
+ assertTrue(result.isValid());
+ assertFalse(result.isSkipped());
+
+ // empty string - required
+ desc.setRequired(true);
+ result = validator.validate("", desc);
+ assertFalse(result.isValid());
+ assertFalse(result.isSkipped());
+ desc.setRequired(false);
+ }
+
@Test public void testValidateRange() {
final List<String> messages = new ArrayList<>();
final PropertyDescription prop = new PropertyDescription();
@@ -510,6 +559,7 @@
@Test public void testPlaceholdersString() {
final PropertyDescription desc = new PropertyDescription();
+ desc.setType(PropertyType.PATH);
PropertyValidationResult result = null;
