Implement framework property validation
diff --git a/pom.xml b/pom.xml
index d8e03c0..5b2e7a0 100644
--- a/pom.xml
+++ b/pom.xml
@@ -56,7 +56,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.feature</artifactId>
- <version>1.2.14</version>
+ <version>1.2.15-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java
index 09def1a..d757845 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java
@@ -26,6 +26,8 @@
private final Map<String, ConfigurationValidationResult> configurationResults = new HashMap<>();
+ private final Map<String, PropertyValidationResult> frameworkPropertyResults = new HashMap<>();
+
/**
* Is the configuration of the feature valid?
* @return {@code true} if it is valid
@@ -38,6 +40,14 @@
break;
}
}
+ if ( valid ) {
+ for(final PropertyValidationResult r : this.frameworkPropertyResults.values()) {
+ if ( !r.isValid() ) {
+ valid = false;
+ break;
+ }
+ }
+ }
return valid;
}
@@ -48,4 +58,12 @@
public Map<String, ConfigurationValidationResult> getConfigurationResults() {
return this.configurationResults;
}
+
+ /**
+ * Get the framework property validation results
+ * @return The results keyed by framework property name
+ */
+ public Map<String, PropertyValidationResult> getFrameworkPropertyResults() {
+ return this.frameworkPropertyResults;
+ }
}
\ No newline at end of file
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 95df5db..bdc19b0 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
@@ -25,6 +25,7 @@
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.Operation;
import org.apache.sling.feature.extension.apiregions.api.config.Region;
@@ -35,6 +36,8 @@
private final ConfigurationValidator configurationValidator = new ConfigurationValidator();
+ private final PropertyValidator propertyValidator = new PropertyValidator();
+
private volatile FeatureProvider featureProvider;
/**
@@ -111,9 +114,32 @@
}
}
}
+
// make sure a result exists
result.getConfigurationResults().computeIfAbsent(config.getPid(), id -> new ConfigurationValidationResult());
}
+
+ for(final String frameworkProperty : feature.getFrameworkProperties().keySet()) {
+ final RegionInfo regionInfo = getRegionInfo(feature, frameworkProperty);
+ if ( regionInfo == null ) {
+ final PropertyValidationResult pvr = new PropertyValidationResult();
+ pvr.getErrors().add("Unable to properly validate framework property, region info cannot be determined");
+ result.getFrameworkPropertyResults().put(frameworkProperty, pvr);
+ } else {
+ final FrameworkPropertyDescription fpd = api.getFrameworkPropertyDescriptions().get(frameworkProperty);
+ if ( fpd != null ) {
+ final PropertyValidationResult pvr = propertyValidator.validate(feature.getFrameworkProperties().get(frameworkProperty), fpd);
+ result.getFrameworkPropertyResults().put(frameworkProperty, pvr);
+ } else if ( regionInfo.region != Region.INTERNAL && api.getInternalFrameworkProperties().contains(frameworkProperty) ) {
+ final PropertyValidationResult pvr = new PropertyValidationResult();
+ pvr.getErrors().add("Framework property is not allowed");
+ result.getFrameworkPropertyResults().put(frameworkProperty, pvr);
+ }
+ }
+ // make sure a result exists
+ result.getFrameworkPropertyResults().computeIfAbsent(frameworkProperty, id -> new PropertyValidationResult());
+ }
+
return result;
}
@@ -155,4 +181,32 @@
}
return result;
}
+
+ RegionInfo getRegionInfo(final Feature feature, final String frameworkProperty) {
+ final FeatureProvider provider = this.getFeatureProvider();
+
+ final List<ArtifactId> list = feature.getFeatureOrigins(feature.getFrameworkPropertyMetadata(frameworkProperty));
+ boolean global = false;
+ for(final ArtifactId id : list) {
+ Feature found = null;
+ if ( feature.getId().equals(id) ) {
+ found = feature;
+ } else {
+ found = provider == null ? null : provider.provide(id);
+ }
+ if ( found == null ) {
+ return null;
+ }
+ final ConfigurationApi api = ConfigurationApi.getConfigurationApi(found);
+ if ( api == null || api.getRegion() != Region.INTERNAL ) {
+ global = true;
+ break;
+ }
+ }
+ final RegionInfo result = new RegionInfo();
+ result.region = global ? Region.GLOBAL : Region.INTERNAL;
+ result.isUpdate = list.size() > 1;
+
+ return result;
+ }
}
\ No newline at end of file
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 fb4fa0c..8d4130a 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
@@ -31,8 +31,10 @@
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.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.Region;
import org.junit.Before;
import org.junit.Test;
@@ -59,6 +61,8 @@
fc.getProperties().put("key", "value");
f.getConfigurations().add(fc);
+ f.getFrameworkProperties().put("prop", "1");
+
return f;
}
@@ -75,10 +79,14 @@
api.getFactoryConfigurationDescriptions().put(FACTORY_PID, fd);
+ final FrameworkPropertyDescription fpd = new FrameworkPropertyDescription();
+ fpd.setType(PropertyType.INTEGER);
+ api.getFrameworkPropertyDescriptions().put("prop", fpd);
+
return api;
}
- @Test public void testGetRegionInfoNoOrigin() {
+ @Test public void testGetRegionInfoConfigurationNoOrigin() {
final Feature f1 = createFeature("g:a:1");
final Configuration cfg = f1.getConfigurations().getConfiguration(PID);
@@ -109,7 +117,7 @@
assertFalse(info.isUpdate);
}
- @Test public void testGetRegionInfoSingleOrigin() {
+ @Test public void testGetRegionInfoConfigurationSingleOrigin() {
final Feature f1 = createFeature("g:a:1");
final Configuration cfg = f1.getConfigurations().getConfiguration(PID);
@@ -151,7 +159,7 @@
assertNull(info);
}
- @Test public void testGetRegionInfoMultipleOrigins() {
+ @Test public void testGetRegionInfoConfigurationMultipleOrigins() {
final Feature f1 = createFeature("g:a:1");
final Configuration cfg = f1.getConfigurations().getConfiguration(PID);
@@ -224,7 +232,152 @@
assertEquals(Region.GLOBAL, info.region);
assertTrue(info.isUpdate);
}
- @Test public void testSingleValidation() {
+
+ @Test public void testGetRegionInfoFrameworkPropertyNoOrigin() {
+ final Feature f1 = createFeature("g:a:1");
+
+ // no api set
+ FeatureValidator.RegionInfo info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // empty region in api
+ final ConfigurationApi api = createApi();
+ ConfigurationApi.setConfigurationApi(f1, api);
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // global region in api
+ api.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // internal region in api
+ api.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.INTERNAL, info.region);
+ assertFalse(info.isUpdate);
+ }
+
+ @Test public void testGetRegionInfoFrameworkPropertySingleOrigin() {
+ final Feature f1 = createFeature("g:a:1");
+
+ final Feature f2 = createFeature("g:b:1");
+ f1.setFeatureOrigins(f1.getFrameworkPropertyMetadata("prop"), Collections.singletonList(f2.getId()));
+
+ // set feature provider to always provide f2
+ this.validator.setFeatureProvider(id -> f2);
+ // no api in origin
+ FeatureValidator.RegionInfo info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // no region in api
+ final ConfigurationApi api2 = new ConfigurationApi();
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // global in api
+ api2.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // internal in api
+ api2.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.INTERNAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // unknown id
+ this.validator.setFeatureProvider(id -> null);
+ f1.setFeatureOrigins(f1.getFrameworkPropertyMetadata("prop"), Collections.singletonList(ArtifactId.parse("g:xy:1")));
+ info = validator.getRegionInfo(f1, "prop");
+ assertNull(info);
+ }
+
+ @Test public void testGetRegionInfoFrameworkPropertyMultipleOrigins() {
+ final Feature f1 = createFeature("g:a:1");
+
+ final Feature f2 = createFeature("g:b:1");
+ final Feature f3 = createFeature("g:c:1");
+ f1.setFeatureOrigins(f1.getFrameworkPropertyMetadata("prop"), Arrays.asList(f2.getId(), f3.getId()));
+
+ final FeatureProvider provider = new FeatureProvider() {
+
+ @Override
+ public Feature provide(final ArtifactId id) {
+ if ( f1.getId().equals(id) ) {
+ return f1;
+ } else if ( f2.getId().equals(id)) {
+ return f2;
+ } else if ( f3.getId().equals(id)) {
+ return f3;
+ }
+ return null;
+ }
+
+ };
+
+ this.validator.setFeatureProvider(provider);
+
+ // no api in origins
+ FeatureValidator.RegionInfo info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertTrue(info.isUpdate);
+
+ // global-internal
+ final ConfigurationApi api2 = new ConfigurationApi();
+ final ConfigurationApi api3 = new ConfigurationApi();
+ api2.setRegion(Region.GLOBAL);
+ api3.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ ConfigurationApi.setConfigurationApi(f3, api3);
+
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertTrue(info.isUpdate);
+
+ // global-global
+ api2.setRegion(Region.GLOBAL);
+ api3.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ ConfigurationApi.setConfigurationApi(f3, api3);
+
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertTrue(info.isUpdate);
+
+ // internal-internal
+ api2.setRegion(Region.INTERNAL);
+ api3.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ ConfigurationApi.setConfigurationApi(f3, api3);
+
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.INTERNAL, info.region);
+ assertTrue(info.isUpdate);
+
+ // internal-global
+ api2.setRegion(Region.INTERNAL);
+ api3.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ ConfigurationApi.setConfigurationApi(f3, api3);
+
+ info = validator.getRegionInfo(f1, "prop");
+ assertEquals(Region.GLOBAL, info.region);
+ assertTrue(info.isUpdate);
+ }
+
+ @Test public void testSingleConfigurationValidation() {
final Feature f1 = createFeature("g:a:1");
final ConfigurationApi api = createApi();
ConfigurationApi.setConfigurationApi(f1, api);
@@ -433,4 +586,50 @@
result = validator.validate(f1, api);
assertTrue(result.isValid());
}
+
+ @Test public void testInternalFrameworkProperty() {
+ final Feature f1 = createFeature("g:a:1");
+ final ConfigurationApi api = new ConfigurationApi();
+ ConfigurationApi.setConfigurationApi(f1, api);
+
+ // global region
+ FeatureValidationResult result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+ // mark framework property as internal
+ api.getInternalFrameworkProperties().add("prop");
+ ConfigurationApi.setConfigurationApi(f1, api);
+
+ // global region
+ result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+ assertFalse(result.getFrameworkPropertyResults().get("prop").isValid());
+
+ // internal region
+ api.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ }
+
+ @Test public void testFrameworkProperty() {
+ final Feature f1 = createFeature("g:a:1");
+ final ConfigurationApi api = createApi();
+ ConfigurationApi.setConfigurationApi(f1, api);
+
+ // value is valid
+ FeatureValidationResult result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+ // no value -> valid
+ f1.getFrameworkProperties().remove("prop");
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+ // invalid value
+ f1.getFrameworkProperties().put("prop", "foo");
+ result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+ assertFalse(result.getFrameworkPropertyResults().get("prop").isValid());
+ }
}
