SLING-10117 : Support different configuration validation modes
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/ConfigurationApiMergeHandler.java b/src/main/java/org/apache/sling/feature/extension/apiregions/ConfigurationApiMergeHandler.java
index 8a43382..082ee61 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/ConfigurationApiMergeHandler.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/ConfigurationApiMergeHandler.java
@@ -58,6 +58,7 @@
// region merging
if ( context.isInitialMerge() ) {
targetApi.setRegion(sourceApi.getRegion());
+ targetApi.setMode(sourceApi.getMode());
} else {
// region merging is different for prototypes
if ( sourceApi.getRegion() != targetApi.getRegion() ) {
@@ -69,6 +70,9 @@
targetApi.setRegion(Region.GLOBAL);
}
}
+ if ( targetApi.getMode().ordinal() > sourceApi.getMode().ordinal() ) {
+ targetApi.setMode(sourceApi.getMode());
+ }
}
// merge - but throw on duplicates
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java
index f817e73..ced08c7 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java
@@ -137,6 +137,9 @@
} else if ( validationMode == Mode.LENIENT || validationMode == Mode.DEFINITIVE ) {
result.getWarnings().add(msg);
}
+ if ( validationMode == Mode.DEFINITIVE || validationMode == Mode.SILENT_DEFINITIVE ) {
+ result.setUseDefaultValue(true);
+ }
}
private boolean isAllowedProperty(final String name) {
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java
index 919511d..e26f773 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java
@@ -159,6 +159,48 @@
return result;
}
+ /**
+ * Apply default values from the result of a validation run.
+ * Defaults should be applied, if configuration properties are invalid and the validation mode
+ * for such a properties is definitive.
+ * @param feature The feature containing the configurations
+ * @param result The result
+ * @return {@code true} if a default value has been applied (the feature has been changed)
+ * @since 1.2
+ */
+ public boolean applyDefaultValues(final Feature feature, final FeatureValidationResult result) {
+ boolean changed = false;
+
+ for(final Map.Entry<String, ConfigurationValidationResult> entry : result.getConfigurationResults().entrySet()) {
+ for(final Map.Entry<String, PropertyValidationResult> propEntry : entry.getValue().getPropertyResults().entrySet()) {
+ if ( propEntry.getValue().isUseDefaultValue() ) {
+ final Configuration cfg = feature.getConfigurations().getConfiguration(entry.getKey());
+ if ( cfg != null ) {
+ if ( propEntry.getValue().getDefaultValue() == null ) {
+ cfg.getProperties().remove(propEntry.getKey());
+ } else {
+ cfg.getProperties().put(propEntry.getKey(), propEntry.getValue().getDefaultValue());
+ }
+ changed = true;
+ }
+ }
+ }
+ }
+
+ for(final Map.Entry<String, PropertyValidationResult> propEntry : result.getFrameworkPropertyResults().entrySet()) {
+ if ( propEntry.getValue().isUseDefaultValue() ) {
+ if ( propEntry.getValue().getDefaultValue() == null ) {
+ feature.getFrameworkProperties().remove(propEntry.getKey());
+ } else {
+ feature.getFrameworkProperties().put(propEntry.getKey(), propEntry.getValue().getDefaultValue().toString());
+ }
+ changed = true;
+ }
+ }
+
+ return changed;
+ }
+
Region getConfigurationApiRegion(final ArtifactId id, final Map<ArtifactId, Region> cache) {
Region result = cache.get(id);
if ( result == null ) {
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidationResult.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidationResult.java
index e5bdbed..b93680c 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidationResult.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidationResult.java
@@ -31,6 +31,18 @@
private boolean skipped = false;
+ /**
+ * Should the default be used?
+ * @since 1.2
+ */
+ private boolean useDefault = false;
+
+ /**
+ * The default value
+ * @since 1.2
+ */
+ private Object defaultValue;
+
/**
* Is the property value valid?
* @return {@code true} if the value is valid
@@ -70,4 +82,43 @@
public void markSkipped() {
this.skipped = true;
}
+
+ /**
+ * Should the default be used instead of the configuration value?
+ * @return {@code true} if the default should be used.
+ * @see #getDefaultValue()
+ * @since 1.2
+ */
+ public boolean isUseDefaultValue() {
+ return useDefault;
+ }
+
+ /**
+ * Set whether the default value should be used
+ * @param useDefault boolean flag
+ * @since 1.2
+ */
+ public void setUseDefaultValue(final boolean useDefault) {
+ this.useDefault = useDefault;
+ }
+
+ /**
+ * Get the default value. The default value is only returned
+ * if {@link #isUseDefaultValue()} returns {@code true}.
+ * @return the defaultValue (it might be {@code null})
+ * @since 1.2
+ */
+ public Object getDefaultValue() {
+ return defaultValue;
+ }
+
+ /**
+ * Set the default value
+ * @param defaultValue the defaultValue to set
+ * @since 1.2
+ */
+ public void setDefaultValue(final Object defaultValue) {
+ this.defaultValue = defaultValue;
+ }
+
}
\ No newline at end of file
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 b643d9d..6cfc4f1 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
@@ -103,6 +103,10 @@
} else if ( context.validationMode == Mode.LENIENT || context.validationMode == Mode.DEFINITIVE ) {
context.result.getWarnings().add(msg);
}
+ if ( context.validationMode == Mode.DEFINITIVE || context.validationMode == Mode.SILENT_DEFINITIVE ) {
+ context.result.setUseDefaultValue(true);
+ context.result.setDefaultValue(context.description.getDefaultValue());
+ }
}
/**
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java
index 3bd6d6b..20330cc 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java
@@ -37,9 +37,11 @@
import org.apache.sling.feature.extension.apiregions.api.config.ConfigurationDescription;
import org.apache.sling.feature.extension.apiregions.api.config.FactoryConfigurationDescription;
import org.apache.sling.feature.extension.apiregions.api.config.FrameworkPropertyDescription;
+import org.apache.sling.feature.extension.apiregions.api.config.Mode;
import org.apache.sling.feature.extension.apiregions.api.config.Operation;
import org.apache.sling.feature.extension.apiregions.api.config.PropertyDescription;
import org.apache.sling.feature.extension.apiregions.api.config.PropertyType;
+import org.apache.sling.feature.extension.apiregions.api.config.Range;
import org.apache.sling.feature.extension.apiregions.api.config.Region;
import org.junit.Before;
import org.junit.Test;
@@ -713,4 +715,83 @@
FeatureValidationResult result = validator.validate(feature);
assertTrue(result.isValid());
}
+
+ @Test public void testDefinitiveModeForConfigurationProperties() {
+ for(int i=0; i<2;i++) {
+ final Feature f = new Feature(ArtifactId.parse("g:a:1"));
+ final Configuration cfg = new Configuration("org.apache.sling");
+ cfg.getProperties().put("a", 1);
+ cfg.getProperties().put("b", 1);
+ cfg.getProperties().put("c", 1);
+ f.getConfigurations().add(cfg);
+
+ final ConfigurationApi api = new ConfigurationApi();
+ api.setMode(i == 0 ? Mode.DEFINITIVE : Mode.SILENT_DEFINITIVE);
+ final ConfigurationDescription desc = new ConfigurationDescription();
+ final PropertyDescription pda = new PropertyDescription();
+ pda.setType(PropertyType.INTEGER);
+ final PropertyDescription pdb = new PropertyDescription();
+ pdb.setType(PropertyType.INTEGER);
+ pdb.setRange(new Range());
+ pdb.getRange().setMin(2);
+ final PropertyDescription pdc = new PropertyDescription();
+ pdc.setType(PropertyType.INTEGER);
+ pdc.setRange(new Range());
+ pdc.getRange().setMin(2);
+ pdc.setDefaultValue(4);
+ desc.getPropertyDescriptions().put("a", pda);
+ desc.getPropertyDescriptions().put("b", pdb);
+ desc.getPropertyDescriptions().put("c", pdc);
+ api.getConfigurationDescriptions().put("org.apache.sling", desc);
+
+ final FeatureValidationResult result = this.validator.validate(f, api);
+ assertTrue(result.isValid());
+
+ // values have not changed
+ assertEquals(1, cfg.getConfigurationProperties().get("a"));
+ assertEquals(1, cfg.getConfigurationProperties().get("b"));
+ assertEquals(1, cfg.getConfigurationProperties().get("c"));
+
+ // apply changes
+ this.validator.applyDefaultValues(f, result);
+ assertEquals(1, cfg.getConfigurationProperties().get("a"));
+ assertNull(cfg.getConfigurationProperties().get("b"));
+ assertEquals(4, cfg.getConfigurationProperties().get("c"));
+ }
+ }
+
+ @Test public void testDefinitiveModeForFrameworkProperties() {
+ for(int i=0; i<2;i++) {
+ final Feature f = new Feature(ArtifactId.parse("g:a:1"));
+ f.getFrameworkProperties().put("a", "hello");
+ f.getFrameworkProperties().put("b", "world");
+ f.getFrameworkProperties().put("c", "world");
+
+ final ConfigurationApi api = new ConfigurationApi();
+ api.setMode(i == 0 ? Mode.DEFINITIVE : Mode.SILENT_DEFINITIVE);
+ final FrameworkPropertyDescription pda = new FrameworkPropertyDescription();
+ final FrameworkPropertyDescription pdb = new FrameworkPropertyDescription();
+ pdb.setRegex("h(.*)");
+ final FrameworkPropertyDescription pdc = new FrameworkPropertyDescription();
+ pdc.setRegex("h(.*)");
+ pdc.setDefaultValue("hi");
+ api.getFrameworkPropertyDescriptions().put("a", pda);
+ api.getFrameworkPropertyDescriptions().put("b", pdb);
+ api.getFrameworkPropertyDescriptions().put("c", pdc);
+
+ final FeatureValidationResult result = this.validator.validate(f, api);
+ assertTrue(result.isValid());
+
+ // values have not changed
+ assertEquals("hello", f.getFrameworkProperties().get("a"));
+ assertEquals("world", f.getFrameworkProperties().get("b"));
+ assertEquals("world", f.getFrameworkProperties().get("c"));
+
+ // apply changes
+ this.validator.applyDefaultValues(f, result);
+ assertEquals("hello", f.getFrameworkProperties().get("a"));
+ assertNull(f.getFrameworkProperties().get("b"));
+ assertEquals("hi", f.getFrameworkProperties().get("c"));
+ }
+ }
}
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 b2803f4..f62b9e9 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
@@ -18,6 +18,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.util.ArrayList;
@@ -71,12 +72,16 @@
assertEquals(errors, result.getWarnings().size());
assertTrue(result.isValid());
assertFalse(result.isSkipped());
+ assertTrue(result.isUseDefaultValue());
+ assertEquals(result.getDefaultValue(), prop.getDefaultValue());
// error - mode silent definitive
result = validator.validate(value, prop, Mode.SILENT_DEFINITIVE);
assertTrue(result.getWarnings().isEmpty());
assertTrue(result.isValid());
assertFalse(result.isSkipped());
+ assertTrue(result.isUseDefaultValue());
+ assertEquals(result.getDefaultValue(), prop.getDefaultValue());
}
/**
@@ -87,6 +92,8 @@
assertTrue(result.isValid());
assertFalse(result.isSkipped());
assertTrue(result.getErrors().isEmpty());
+ assertFalse(result.isUseDefaultValue());
+ assertNull(result.getDefaultValue());
}
@Test public void testValidateWithNull() {
@@ -309,6 +316,10 @@
validateValid(prop, "hello world");
validateError(prop, "world");
+
+ // apply default
+ prop.setDefaultValue("hello world");
+ validateError(prop, "world");
}
@Test public void testValidateOptions() {