SLING-10412 : Unify and simplify internal configuration handling
diff --git a/docs/api-regions.md b/docs/api-regions.md
index f288d27..fa164c6 100644
--- a/docs/api-regions.md
+++ b/docs/api-regions.md
@@ -314,16 +314,20 @@
### Internal Configurations
-Some OSGi configurations and factory configurations are not part of the public API and cannot be created/updated by application configuration. Same applies to framework properties. The PIDs, factory PIDs and names of these can be specified as well:
+Some OSGi configurations and factory configurations are not part of the public API and cannot be created/updated by application configuration. Same applies to framework properties. For configurations and factory configurations, define a configuration description without any properties. Internal framework properties are listed with just their names:
``` json
"configuration-api:JSON" : {
- "internal-configurations" : [
- // list of PIDs
- ],
- "internal-factory-configurations" : [
- // list of factory PIDs
- ],
+ "configurations" : {
+ "org.apache.sling.engine.impl.InternalRequestHandling" : {
+ // no properties
+ }
+ },
+ "factory-configurations" : {
+ "org.apache.sling.engine.impl.InternalLogger" : {
+ // no properties
+ }
+ },
"internal-framework-properties" : {
// list of property names
}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationApi.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationApi.java
index b65f14e..1152260 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationApi.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationApi.java
@@ -266,6 +266,7 @@
/**
* Get the internal configuration pids
* @return Mutable set of internal configuration pids
+ * @deprecated Please use empty configuration descriptions via {@link #getConfigurationDescriptions()}
*/
public Set<String> getInternalConfigurations() {
return internalConfigurations;
@@ -274,6 +275,7 @@
/**
* Get the internal factory pids
* @return Mutable set of internal factory configuration pids
+ * @deprecated Please use empty factory configuration descriptions via {@link #getFactoryConfigurationDescriptions()}
*/
public Set<String> getInternalFactoryConfigurations() {
return internalFactories;
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 6ac6f43..64d49cb 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.0")
+@org.osgi.annotation.versioning.Version("1.2.1")
package org.apache.sling.feature.extension.apiregions.api.config;
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
index 7066ff1..4c4ccb6 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
@@ -33,6 +33,12 @@
private final List<String> warnings = new ArrayList<>();
+ /**
+ * Should the default configuration be used?
+ * @since 1.3
+ */
+ private boolean useDefault = false;
+
/**
* Is the configuration valid?
* @return {@code true} if it is valid
@@ -73,4 +79,23 @@
public List<String> getWarnings() {
return this.warnings;
}
+
+ /**
+ * Should the default be used instead of the configuration values?
+ * @return {@code true} if the default should be used.
+ * @see #getDefaultValue()
+ * @since 1.3
+ */
+ public boolean isUseDefaultValue() {
+ return useDefault;
+ }
+
+ /**
+ * Set whether the default values should be used
+ * @param useDefault boolean flag
+ * @since 1.3
+ */
+ public void setUseDefaultValue(final boolean useDefault) {
+ this.useDefault = useDefault;
+ }
}
\ No newline at end of file
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 ced08c7..b73022a 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
@@ -77,13 +77,21 @@
if ( !(desc instanceof FactoryConfigurationDescription) ) {
result.getErrors().add("Factory configuration cannot be validated against non factory configuration description");
} else {
- validateProperties(config, desc, result.getPropertyResults(), region, validationMode);
+ if ( desc.getPropertyDescriptions().isEmpty()) {
+ setResult(result, region, validationMode, "Factory configuration is not allowed");
+ } else {
+ validateProperties(config, desc, result.getPropertyResults(), region, validationMode);
+ }
}
} else {
if ( !(desc instanceof ConfigurationDescription) ) {
result.getErrors().add("Configuration cannot be validated against factory configuration description");
} else {
- validateProperties(config, desc, result.getPropertyResults(), region, validationMode);
+ if ( desc.getPropertyDescriptions().isEmpty()) {
+ setResult(result, region, validationMode, "Configuration is not allowed");
+ } else {
+ validateProperties(config, desc, result.getPropertyResults(), region, validationMode);
+ }
}
}
@@ -131,6 +139,19 @@
}
}
+ void setResult(final ConfigurationValidationResult result, final Region region, final Mode validationMode, final String msg) {
+ if ( region != Region.INTERNAL ) {
+ if ( validationMode == Mode.STRICT ) {
+ result.getErrors().add(msg);
+ } else if ( validationMode == Mode.LENIENT || validationMode == Mode.DEFINITIVE ) {
+ result.getWarnings().add(msg);
+ }
+ if ( validationMode == Mode.DEFINITIVE || validationMode == Mode.SILENT_DEFINITIVE ) {
+ result.setUseDefaultValue(true);
+ }
+ }
+ }
+
void setResult(final PropertyValidationResult result, final Mode validationMode, final String msg) {
if ( validationMode == Mode.STRICT ) {
result.getErrors().add(msg);
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 e26f773..7ab6187 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
@@ -16,6 +16,8 @@
*/
package org.apache.sling.feature.extension.apiregions.api.config.validation;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -172,6 +174,15 @@
boolean changed = false;
for(final Map.Entry<String, ConfigurationValidationResult> entry : result.getConfigurationResults().entrySet()) {
+ if ( entry.getValue().isUseDefaultValue() ) {
+ final Configuration cfg = feature.getConfigurations().getConfiguration(entry.getKey());
+ if ( cfg != null ) {
+ final List<String> keys = new ArrayList<>(Collections.list(cfg.getConfigurationProperties().keys()));
+ for(final String k : keys ) {
+ cfg.getProperties().remove(k);
+ }
+ }
+ }
for(final Map.Entry<String, PropertyValidationResult> propEntry : entry.getValue().getPropertyResults().entrySet()) {
if ( propEntry.getValue().isUseDefaultValue() ) {
final Configuration cfg = feature.getConfigurations().getConfiguration(entry.getKey());
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/package-info.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/package-info.java
index 0cc3da8..332f38d 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/package-info.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/package-info.java
@@ -17,7 +17,7 @@
* under the License.
*/
-@org.osgi.annotation.versioning.Version("1.2.0")
+@org.osgi.annotation.versioning.Version("1.3.0")
package org.apache.sling.feature.extension.apiregions.api.config.validation;
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidatorTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidatorTest.java
index e09d24d..b233ee0 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidatorTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidatorTest.java
@@ -23,6 +23,7 @@
import org.apache.sling.feature.Configuration;
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.Mode;
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.Region;
@@ -54,6 +55,8 @@
@Test public void testDeprecated() {
final Configuration cfg = new Configuration("org.apache");
final ConfigurationDescription cd = new ConfigurationDescription();
+ final PropertyDescription prop = new PropertyDescription();
+ cd.getPropertyDescriptions().put("a", prop);
ConfigurationValidationResult result = validator.validate(cfg, cd, null);
assertTrue(result.isValid());
@@ -70,6 +73,8 @@
final Configuration cfg = new Configuration("org.apache");
final ConfigurationDescription cd = new ConfigurationDescription();
cfg.getProperties().put(Constants.SERVICE_RANKING, 5);
+ final PropertyDescription prop = new PropertyDescription();
+ cd.getPropertyDescriptions().put("a", prop);
ConfigurationValidationResult result = validator.validate(cfg, cd, null);
assertTrue(result.isValid());
@@ -82,6 +87,8 @@
@Test public void testAllowedProperties() {
final Configuration cfg = new Configuration("org.apache");
final ConfigurationDescription cd = new ConfigurationDescription();
+ final PropertyDescription prop = new PropertyDescription();
+ cd.getPropertyDescriptions().put("a", prop);
cfg.getProperties().put(Constants.SERVICE_DESCRIPTION, "desc");
cfg.getProperties().put(Constants.SERVICE_VENDOR, "vendor");
@@ -133,4 +140,124 @@
assertTrue(result.getPropertyResults().get("a").isValid());
assertFalse(result.getPropertyResults().get("b").isValid());
}
+
+ @Test public void testInternalConfiguration() {
+ // internal -> valid
+ final Configuration cfg = new Configuration("org.apache");
+ cfg.getProperties().put("a", "desc");
+ cfg.getProperties().put("b", "vendor");
+
+ // description without properties -> internal
+ final ConfigurationDescription cd = new ConfigurationDescription();
+
+ ConfigurationValidationResult result = validator.validate(cfg, cd, Region.INTERNAL);
+ assertTrue(result.isValid());
+ assertFalse(result.isUseDefaultValue());
+ assertTrue(result.getWarnings().isEmpty());
+ assertEquals(2, cfg.getProperties().size());
+
+ // global -> invalid
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertFalse(result.isValid());
+ assertFalse(result.isUseDefaultValue());
+ assertEquals(1, result.getErrors().size());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getWarnings().isEmpty());
+
+ // global -> invalid, but mode DEFINITIVE
+ cd.setMode(Mode.DEFINITIVE);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertTrue(result.isValid());
+ assertTrue(result.isUseDefaultValue());
+ assertEquals(1, result.getWarnings().size());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getErrors().isEmpty());
+
+ // global -> invalid, but mode LENIENT
+ cd.setMode(Mode.LENIENT);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertTrue(result.isValid());
+ assertFalse(result.isUseDefaultValue());
+ assertEquals(1, result.getWarnings().size());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getErrors().isEmpty());
+
+ // global -> invalid, but mode SILENT
+ cd.setMode(Mode.SILENT);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertTrue(result.isValid());
+ assertFalse(result.isUseDefaultValue());
+ assertTrue(result.getWarnings().isEmpty());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getErrors().isEmpty());
+
+ // global -> invalid, but mode SILENT
+ cd.setMode(Mode.SILENT_DEFINITIVE);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertTrue(result.isValid());
+ assertTrue(result.isUseDefaultValue());
+ assertTrue(result.getWarnings().isEmpty());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getErrors().isEmpty());
+ }
+
+ @Test public void testInternalFactoryConfiguration() {
+ // internal -> valid
+ final Configuration cfg = new Configuration("org.apache~foo");
+ cfg.getProperties().put("a", "desc");
+ cfg.getProperties().put("b", "vendor");
+
+ // description without properties -> internal
+ final FactoryConfigurationDescription cd = new FactoryConfigurationDescription();
+
+ ConfigurationValidationResult result = validator.validate(cfg, cd, Region.INTERNAL);
+ assertTrue(result.isValid());
+ assertFalse(result.isUseDefaultValue());
+ assertTrue(result.getWarnings().isEmpty());
+ assertEquals(2, cfg.getProperties().size());
+
+ // global -> invalid
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertFalse(result.isValid());
+ assertFalse(result.isUseDefaultValue());
+ assertEquals(1, result.getErrors().size());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getWarnings().isEmpty());
+
+ // global -> invalid, but mode DEFINITIVE
+ cd.setMode(Mode.DEFINITIVE);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertTrue(result.isValid());
+ assertTrue(result.isUseDefaultValue());
+ assertEquals(1, result.getWarnings().size());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getErrors().isEmpty());
+
+ // global -> invalid, but mode LENIENT
+ cd.setMode(Mode.LENIENT);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertTrue(result.isValid());
+ assertFalse(result.isUseDefaultValue());
+ assertEquals(1, result.getWarnings().size());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getErrors().isEmpty());
+
+ // global -> invalid, but mode SILENT
+ cd.setMode(Mode.SILENT);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertTrue(result.isValid());
+ assertFalse(result.isUseDefaultValue());
+ assertTrue(result.getWarnings().isEmpty());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getErrors().isEmpty());
+
+ // global -> invalid, but mode SILENT
+ cd.setMode(Mode.SILENT_DEFINITIVE);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
+ assertTrue(result.isValid());
+ assertTrue(result.isUseDefaultValue());
+ assertTrue(result.getWarnings().isEmpty());
+ assertTrue(result.getPropertyResults().isEmpty());
+ assertTrue(result.getErrors().isEmpty());
+ }
}
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 20330cc..09bc337 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
@@ -794,4 +794,146 @@
assertEquals("hi", f.getFrameworkProperties().get("c"));
}
}
+
+ @Test public void testInternalConfigurationNoPropertyDescriptions() {
+ Feature f1 = createFeature("g:a:1");
+ ConfigurationApi api = createApi();
+ // no property descriptions -> internal
+ api.getConfigurationDescriptions().get(PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.INTERNAL);
+
+ // internal -> valid
+ FeatureValidationResult result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(1, f1.getConfigurations().getConfiguration(PID).getConfigurationProperties().size());
+
+ // global -> invalid
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getConfigurationDescriptions().get(PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+
+ // global -> invalid, but mode DEFINITIVE
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getConfigurationDescriptions().get(PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ api.setMode(Mode.DEFINITIVE);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(0, f1.getConfigurations().getConfiguration(PID).getConfigurationProperties().size());
+
+ // global -> invalid, but mode LENIENT
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getConfigurationDescriptions().get(PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ api.setMode(Mode.LENIENT);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(1, f1.getConfigurations().getConfiguration(PID).getConfigurationProperties().size());
+
+ // global -> invalid, but mode SILENT
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getConfigurationDescriptions().get(PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ api.setMode(Mode.SILENT);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(1, f1.getConfigurations().getConfiguration(PID).getConfigurationProperties().size());
+
+ // global -> invalid, but mode SILENT_DEFINITIVE
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getConfigurationDescriptions().get(PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ api.setMode(Mode.SILENT_DEFINITIVE);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(0, f1.getConfigurations().getConfiguration(PID).getConfigurationProperties().size());
+ }
+
+ @Test public void testInternalFactoryConfigurationNoPropertyDescriptions() {
+ Feature f1 = createFeature("g:a:1");
+ ConfigurationApi api = createApi();
+ // no property descriptions -> internal
+ api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.INTERNAL);
+
+ // internal -> valid
+ FeatureValidationResult result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(1, f1.getConfigurations().getConfiguration(FACTORY_PID.concat("~print")).getConfigurationProperties().size());
+
+ // global -> invalid
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+
+ // global -> invalid, but mode DEFINITIVE
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ api.setMode(Mode.DEFINITIVE);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(0, f1.getConfigurations().getConfiguration(FACTORY_PID.concat("~print")).getConfigurationProperties().size());
+
+ // global -> invalid, but mode LENIENT
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ api.setMode(Mode.LENIENT);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(1, f1.getConfigurations().getConfiguration(FACTORY_PID.concat("~print")).getConfigurationProperties().size());
+
+ // global -> invalid, but mode SILENT
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ api.setMode(Mode.SILENT);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(1, f1.getConfigurations().getConfiguration(FACTORY_PID.concat("~print")).getConfigurationProperties().size());
+
+ // global -> invalid, but mode SILENT_DEFINITIVE
+ f1 = createFeature("g:a:1");
+ api = createApi();
+ // no property descriptions -> internal
+ api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getPropertyDescriptions().clear();
+ api.setRegion(Region.GLOBAL);
+ api.setMode(Mode.SILENT_DEFINITIVE);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ validator.applyDefaultValues(f1, result);
+ assertEquals(0, f1.getConfigurations().getConfiguration(FACTORY_PID.concat("~print")).getConfigurationProperties().size());
+ }
}
