This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-extension-apiregions.git
The following commit(s) were added to refs/heads/master by this push:
new 7a2d220 SLING-10117 : Support different configuration validation modes
7a2d220 is described below
commit 7a2d2204406ecfb76a2df42732516138ee603696
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Sat Feb 13 12:33:16 2021 +0100
SLING-10117 : Support different configuration validation modes
---
.../apiregions/ConfigurationApiMergeHandler.java | 4 ++
.../config/validation/ConfigurationValidator.java | 3 +
.../api/config/validation/FeatureValidator.java | 42 +++++++++++
.../validation/PropertyValidationResult.java | 51 ++++++++++++++
.../api/config/validation/PropertyValidator.java | 4 ++
.../config/validation/FeatureValidatorTest.java | 81 ++++++++++++++++++++++
.../config/validation/PropertyValidatorTest.java | 11 +++
7 files changed, 196 insertions(+)
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 @@ public class ConfigurationApiMergeHandler implements
MergeHandler {
// 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 @@ public class ConfigurationApiMergeHandler implements
MergeHandler {
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 @@ public class ConfigurationValidator {
} 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 @@ public class FeatureValidator {
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 @@ public class PropertyValidationResult {
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 class PropertyValidationResult {
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 @@ public class PropertyValidator {
} 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.ConfigurationApi
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 @@ public class FeatureValidatorTest {
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 @@ package
org.apache.sling.feature.extension.apiregions.api.config.validation;
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 @@ public class PropertyValidatorTest {
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 @@ public class PropertyValidatorTest {
assertTrue(result.isValid());
assertFalse(result.isSkipped());
assertTrue(result.getErrors().isEmpty());
+ assertFalse(result.isUseDefaultValue());
+ assertNull(result.getDefaultValue());
}
@Test public void testValidateWithNull() {
@@ -309,6 +316,10 @@ public class PropertyValidatorTest {
validateValid(prop, "hello world");
validateError(prop, "world");
+
+ // apply default
+ prop.setDefaultValue("hello world");
+ validateError(prop, "world");
}
@Test public void testValidateOptions() {
