Merge pull request #14 from bosschaert/SLING-9937
SLING-9937 api-regions-crossfeature-dups should work if all exports come from API Regions
diff --git a/README.md b/README.md
index e787442..10b567a 100644
--- a/README.md
+++ b/README.md
@@ -9,11 +9,11 @@
## Feature Model Analysers
-This component also contains Feature Model Analysers they are contributed through the Service Loader mechanism to the set of Analysers.
+This component also contains Feature Model Analysers. They are contributed through the Service Loader mechanism to the set of Analysers.
-Documentation can be found here: https://github.com/apache/sling-org-apache-sling-feature-analyser . These can be run as part of the 'analyse-features' goal with the [slingfeature-maven-plugin](https://github.com/apache/sling-slingfeature-maven-plugin#analyse-features-analyse-features).
+Documentation can be found here: [Apache Sling Feature Analyser](https://github.com/apache/sling-org-apache-sling-feature-analyser) . These can be run as part of the 'analyse-features' goal with the [slingfeature-maven-plugin](https://github.com/apache/sling-slingfeature-maven-plugin#analyse-features-analyse-features).
-These analysers relate to API Region definitions in Feature Models.
+These analysers relate to Java API Region definitions in Feature Models:
* `api-regions`: This analyser ensures that packages listed as exports in API-Regions sections are actually exported by a bundle that's part of the feature.
@@ -26,7 +26,7 @@
* `api-regions-duplicates`: This analyser ensures that packages are only listed in one region
in a given feature. If the same package is listed in multiple regions this will be an error.
-* `api-regions-exportsimports`: Checks bundle import/export package statements for consistency and completeness. If API Regions are used this analyser includes this
+* `api-regions-exportsimports`: Checks bundle import/export package statements for consistency and completeness. If API Regions are used this analyser includes this
information as part of the check, to ensure that bundles don't import packages of which they have no visibility because of API Regions restrictions.
* `api-regions-check-order`: This analyser checks that regions are defined in the specified
@@ -53,15 +53,20 @@
* `warningPackages`: if packages listed here are found to overlap, a warning instead of an error is reported. Supports either literal package names (e.g. `javax.servlet`) or wildcards with an asterisk at the end (e.g. `javax.*`).
* `ignoredPackages`: packages listed here are completely ignored in the analysis. Supports literal package names or wildcards with an asterisk at the end.
+These analysers relate to Configuration API Region definitions in Feature Models:
+
+* `configuration-api` : This analyser validates the OSGi configurations and framework properties based on the configuration API described in an extension.
+
## Extensions
The following extensions are registered via the ServiceLoader mechanism:
## `org.apache.sling.feature.builder.MergeHandler`
-Merge handlers are called when features are merged during the aggregation process.
-`APIRegionMergeHandler` - This handler knows how to merge API Regions extensions
+Merge handlers are called when features are merged during the aggregation process:
+* `APIRegionMergeHandler` - This handler knows how to merge API Regions extensions
+* `ConfigurationApiMergeHandler` - This handlers knows how to merge Configuration API extensions
# Additional Extensions
diff --git a/pom.xml b/pom.xml
index a980994..25a0259 100644
--- a/pom.xml
+++ b/pom.xml
@@ -62,7 +62,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.feature.analyser</artifactId>
- <version>1.3.12</version>
+ <version>1.3.13-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckConfigurationApi.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckConfigurationApi.java
index 20a5d6c..0e84ed9 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckConfigurationApi.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckConfigurationApi.java
@@ -70,7 +70,7 @@
}
}
if ( !entry.getValue().isValid() ) {
- for(final String err : entry.getValue().getGlobalErrors()) {
+ for(final String err : entry.getValue().getErrors()) {
context.reportError("Configuration " + entry.getKey() + " : " + err);
}
for(final Map.Entry<String, PropertyValidationResult> propEntry : entry.getValue().getPropertyResults().entrySet()) {
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 2d7af47..e29a629 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
@@ -17,10 +17,10 @@
package org.apache.sling.feature.extension.apiregions.api.config;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.LinkedHashMap;
-import java.util.List;
import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
import javax.json.Json;
import javax.json.JsonArrayBuilder;
@@ -46,7 +46,9 @@
/**
* Get the configuration api from the feature - if it exists.
- *
+ * If the configuration api is updated, the containing feature is left untouched.
+ * {@link #setConfigurationApi(Feature, ConfigurationApi)} can be used to update
+ * the feature.
* @param feature The feature
* @return The configuration api or {@code null}.
* @throws IllegalArgumentException If the extension is wrongly formatted
@@ -58,7 +60,7 @@
/**
* Get the configuration api from the extension.
- *
+ * If the configuration api is updated, the containing extension is left untouched.
* @param ext The extension
* @return The configuration api or {@code null} if the extension is {@code null}.
* @throws IllegalArgumentException If the extension is wrongly formatted
@@ -114,14 +116,14 @@
/** The map of framework properties */
private final Map<String, FrameworkPropertyDescription> frameworkProperties = new LinkedHashMap<>();
- /** The list of internal configuration names */
- private final List<String> internalConfigurations = new ArrayList<>();
+ /** The set of internal configuration names */
+ private final Set<String> internalConfigurations = new TreeSet<>();
- /** The list of internal factory configuration names */
- private final List<String> internalFactories = new ArrayList<>();
+ /** The set of internal factory configuration names */
+ private final Set<String> internalFactories = new TreeSet<>();
- /** The list of internal framework property names */
- private final List<String> internalFrameworkProperties = new ArrayList<>();
+ /** The set of internal framework property names */
+ private final Set<String> internalFrameworkProperties = new TreeSet<>();
/** The configuration region of this feature */
private Region region;
@@ -235,25 +237,25 @@
/**
* Get the internal configuration pids
- * @return Mutable list of internal configuration pids
+ * @return Mutable set of internal configuration pids
*/
- public List<String> getInternalConfigurations() {
+ public Set<String> getInternalConfigurations() {
return internalConfigurations;
}
/**
* Get the internal factory pids
- * @return Mutable list of internal factory configuration pids
+ * @return Mutable set of internal factory configuration pids
*/
- public List<String> getInternalFactoryConfigurations() {
+ public Set<String> getInternalFactoryConfigurations() {
return internalFactories;
}
/**
* Get the internal framework property names
- * @return Mutable list of internal framework property names
+ * @return Mutable set of internal framework property names
*/
- public List<String> getInternalFrameworkProperties() {
+ public Set<String> getInternalFrameworkProperties() {
return internalFrameworkProperties;
}
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 f1cb9f4..24595b5 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
@@ -21,16 +21,23 @@
import java.util.List;
import java.util.Map;
+/**
+ * A configuration validation result is returned by the {@code ConfigurationValidator}.
+ */
public class ConfigurationValidationResult {
private final Map<String, PropertyValidationResult> propertyResults = new HashMap<>();
- private final List<String> globalErrors = new ArrayList<>();
+ private final List<String> errors = new ArrayList<>();
private final List<String> warnings = new ArrayList<>();
+ /**
+ * Is the configuration valid?
+ * @return {@code true} if it is valid
+ */
public boolean isValid() {
- boolean valid = globalErrors.isEmpty();
+ boolean valid = errors.isEmpty();
if ( valid ) {
for(final PropertyValidationResult r : this.propertyResults.values()) {
if ( !r.isValid() ) {
@@ -42,10 +49,18 @@
return valid;
}
- public List<String> getGlobalErrors() {
- return this.globalErrors;
+ /**
+ * Return the list of errors
+ * @return A list of errors. Might be empty.
+ */
+ public List<String> getErrors() {
+ return this.errors;
}
+ /**
+ * Get a property validation result for each property of the configuration
+ * @return A map of property results keyed by property name
+ */
public Map<String, PropertyValidationResult> getPropertyResults() {
return propertyResults;
}
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 a7e505d..0a4228e 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
@@ -31,7 +31,7 @@
import org.osgi.framework.Constants;
/**
- * Validator to validate a configuration
+ * Validator to validate a configuration or factory configuration
*/
public class ConfigurationValidator {
@@ -47,6 +47,7 @@
/**
* Validate a configuration
+ *
* @param config The OSGi configuration
* @param desc The configuration description
* @param region The optional region for the configuration
@@ -56,15 +57,15 @@
final ConfigurationValidationResult result = new ConfigurationValidationResult();
if ( config.isFactoryConfiguration() ) {
if ( !(desc instanceof FactoryConfigurationDescription) ) {
- result.getGlobalErrors().add("Factory configuration cannot be validated against non factory configuration description");
+ result.getErrors().add("Factory configuration cannot be validated against non factory configuration description");
} else {
- validateProperties(desc, config, result.getPropertyResults(), region);
+ validateProperties(config, desc, result.getPropertyResults(), region);
}
} else {
if ( !(desc instanceof ConfigurationDescription) ) {
- result.getGlobalErrors().add("Configuration cannot be validated against factory configuration description");
+ result.getErrors().add("Configuration cannot be validated against factory configuration description");
} else {
- validateProperties(desc, config, result.getPropertyResults(), region);
+ validateProperties(config, desc, result.getPropertyResults(), region);
}
}
@@ -74,16 +75,25 @@
return result;
}
- void validateProperties(final ConfigurableEntity desc,
- final Configuration configuration,
+ /**
+ * Validate all properties
+ * @param configuration The OSGi configuration
+ * @param desc The configuration description
+ * @param results The map of results per property
+ * @param region The configuration region
+ */
+ void validateProperties(final Configuration configuration,
+ final ConfigurableEntity desc,
final Map<String, PropertyValidationResult> results,
final Region region) {
final Dictionary<String, Object> properties = configuration.getConfigurationProperties();
+ // validate the described properties
for(final Map.Entry<String, PropertyDescription> propEntry : desc.getPropertyDescriptions().entrySet()) {
final Object value = properties.get(propEntry.getKey());
final PropertyValidationResult result = propertyValidator.validate(value, propEntry.getValue());
results.put(propEntry.getKey(), result);
}
+ // validate additional properties
final Enumeration<String> keyEnum = properties.keys();
while ( keyEnum.hasMoreElements() ) {
final String propName = keyEnum.nextElement();
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 63cdc37..9e2cf44 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
@@ -74,7 +74,7 @@
if ( regionInfo == null ) {
final ConfigurationValidationResult cvr = new ConfigurationValidationResult();
- cvr.getGlobalErrors().add("Unable to properly validate configuration, region info cannot be determined");
+ cvr.getErrors().add("Unable to properly validate configuration, region info cannot be determined");
result.getConfigurationResults().put(config.getPid(), cvr);
} else {
if ( config.isFactoryConfiguration() ) {
@@ -84,22 +84,22 @@
result.getConfigurationResults().put(config.getPid(), r);
if ( regionInfo.region != Region.INTERNAL ) {
if ( desc.getOperations().isEmpty() ) {
- r.getGlobalErrors().add("No operations allowed for factory configuration");
+ r.getErrors().add("No operations allowed for factory configuration");
} else {
if ( regionInfo.isUpdate && !desc.getOperations().contains(Operation.UPDATE)) {
- r.getGlobalErrors().add("Updating of factory configuration is not allowed");
+ r.getErrors().add("Updating of factory configuration is not allowed");
} else if ( !regionInfo.isUpdate && !desc.getOperations().contains(Operation.CREATE)) {
- r.getGlobalErrors().add("Creation of factory configuration is not allowed");
+ r.getErrors().add("Creation of factory configuration is not allowed");
}
}
if ( desc.getInternalNames().contains(config.getName())) {
- r.getGlobalErrors().add("Factory configuration with name is not allowed");
+ r.getErrors().add("Factory configuration with name is not allowed");
}
}
} else if ( regionInfo.region != Region.INTERNAL && api.getInternalFactoryConfigurations().contains(config.getFactoryPid())) {
final ConfigurationValidationResult cvr = new ConfigurationValidationResult();
- cvr.getGlobalErrors().add("Factory configuration is not allowed");
+ cvr.getErrors().add("Factory configuration is not allowed");
result.getConfigurationResults().put(config.getPid(), cvr);
}
} else {
@@ -109,7 +109,7 @@
result.getConfigurationResults().put(config.getPid(), r);
} else if ( regionInfo.region!= Region.INTERNAL && api.getInternalConfigurations().contains(config.getPid())) {
final ConfigurationValidationResult cvr = new ConfigurationValidationResult();
- cvr.getGlobalErrors().add("Configuration is not allowed");
+ cvr.getErrors().add("Configuration is not allowed");
result.getConfigurationResults().put(config.getPid(), cvr);
}
}
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 b68ef2e..431ff59 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
@@ -19,6 +19,9 @@
import java.util.ArrayList;
import java.util.List;
+/**
+ * Validation result for a property
+ */
public class PropertyValidationResult {
private final List<String> errors = new ArrayList<>();
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 82f7a1c..370e247 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
@@ -27,18 +27,20 @@
import org.apache.sling.feature.extension.apiregions.api.config.PropertyDescription;
/**
- * Validate a configuration property
+ * Validate a configuration property or framework property
*/
public class PropertyValidator {
/**
* Validate the value against the property definition
+ * @param value The value to validate
+ * @param desc The property description
* @return A property validation result
*/
- public PropertyValidationResult validate(final Object value, final PropertyDescription prop) {
+ public PropertyValidationResult validate(final Object value, final PropertyDescription desc) {
final PropertyValidationResult result = new PropertyValidationResult();
if ( value == null ) {
- if ( prop.isRequired() ) {
+ if ( desc.isRequired() ) {
result.getErrors().add("No value provided");
}
} else {
@@ -59,19 +61,19 @@
} else {
// single value
values = null;
- validateValue(prop, value, result.getErrors());
+ validateValue(desc, value, result.getErrors());
}
if ( values != null ) {
// array or collection
for(final Object val : values) {
- validateValue(prop, val, result.getErrors());
+ validateValue(desc, val, result.getErrors());
}
- validateList(prop, values, result.getErrors());
+ validateList(desc, values, result.getErrors());
}
- if ( prop.getDeprecated() != null ) {
- result.getWarnings().add(prop.getDeprecated());
+ if ( desc.getDeprecated() != null ) {
+ result.getWarnings().add(desc.getDeprecated());
}
}
return result;
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/ConfigurationApiMergeHandlerTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/ConfigurationApiMergeHandlerTest.java
index 7c95589..e68606a 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/ConfigurationApiMergeHandlerTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/ConfigurationApiMergeHandlerTest.java
@@ -19,6 +19,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Feature;
@@ -26,6 +27,9 @@
import org.apache.sling.feature.builder.BuilderContext;
import org.apache.sling.feature.builder.FeatureBuilder;
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.Region;
import org.junit.Test;
@@ -114,7 +118,6 @@
}
@Test public void testRegionMerge() {
- // always return prototype
final BuilderContext context = new BuilderContext(id -> null);
context.addMergeExtensions(new ConfigurationApiMergeHandler());
@@ -194,4 +197,144 @@
api = ConfigurationApi.getConfigurationApi(result);
assertEquals(Region.GLOBAL, api.getRegion());
}
+
+ @Test public void testConfigurationApiMergeDifferentConfig() {
+ final BuilderContext context = new BuilderContext(id -> null);
+ context.addMergeExtensions(new ConfigurationApiMergeHandler());
+
+ final Feature featureA = new Feature(ArtifactId.parse("g:a:1"));
+ final ConfigurationApi apiA = new ConfigurationApi();
+ apiA.getConfigurationDescriptions().put("a", new ConfigurationDescription());
+ apiA.getFactoryConfigurationDescriptions().put("fa", new FactoryConfigurationDescription());
+ apiA.getFrameworkPropertyDescriptions().put("pa", new FrameworkPropertyDescription());
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+
+ final Feature featureB = new Feature(ArtifactId.parse("g:b:1"));
+ final ConfigurationApi apiB = new ConfigurationApi();
+ apiB.getConfigurationDescriptions().put("b", new ConfigurationDescription());
+ apiB.getFactoryConfigurationDescriptions().put("fb", new FactoryConfigurationDescription());
+ apiB.getFrameworkPropertyDescriptions().put("pb", new FrameworkPropertyDescription());
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+
+ final ArtifactId id = ArtifactId.parse("g:m:1");
+ Feature result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ ConfigurationApi api = ConfigurationApi.getConfigurationApi(result);
+ assertNotNull(api);
+
+ assertEquals(2, api.getConfigurationDescriptions().size());
+ assertNotNull(api.getConfigurationDescriptions().get("a"));
+ assertNotNull(api.getConfigurationDescriptions().get("b"));
+
+ assertEquals(2, api.getFactoryConfigurationDescriptions().size());
+ assertNotNull(api.getFactoryConfigurationDescriptions().get("fa"));
+ assertNotNull(api.getFactoryConfigurationDescriptions().get("fb"));
+
+ assertEquals(2, api.getFrameworkPropertyDescriptions().size());
+ assertNotNull(api.getFrameworkPropertyDescriptions().get("pa"));
+ assertNotNull(api.getFrameworkPropertyDescriptions().get("pb"));
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testConfigurationApiMergeSameConfigurations() {
+ final BuilderContext context = new BuilderContext(id -> null);
+ context.addMergeExtensions(new ConfigurationApiMergeHandler());
+
+ final Feature featureA = new Feature(ArtifactId.parse("g:a:1"));
+ final ConfigurationApi apiA = new ConfigurationApi();
+ apiA.getConfigurationDescriptions().put("a", new ConfigurationDescription());
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+
+ final Feature featureB = new Feature(ArtifactId.parse("g:b:1"));
+ final ConfigurationApi apiB = new ConfigurationApi();
+ apiB.getConfigurationDescriptions().put("a", new ConfigurationDescription());
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+
+ final ArtifactId id = ArtifactId.parse("g:m:1");
+ FeatureBuilder.assemble(id, context, featureA, featureB);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testConfigurationApiMergeSameFactoryConfigurations() {
+ final BuilderContext context = new BuilderContext(id -> null);
+ context.addMergeExtensions(new ConfigurationApiMergeHandler());
+
+ final Feature featureA = new Feature(ArtifactId.parse("g:a:1"));
+ final ConfigurationApi apiA = new ConfigurationApi();
+ apiA.getFactoryConfigurationDescriptions().put("fa", new FactoryConfigurationDescription());
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+
+ final Feature featureB = new Feature(ArtifactId.parse("g:b:1"));
+ final ConfigurationApi apiB = new ConfigurationApi();
+ apiB.getFactoryConfigurationDescriptions().put("fa", new FactoryConfigurationDescription());
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+
+ final ArtifactId id = ArtifactId.parse("g:m:1");
+ FeatureBuilder.assemble(id, context, featureA, featureB);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testConfigurationApiMergeSameFrameworkProperties() {
+ final BuilderContext context = new BuilderContext(id -> null);
+ context.addMergeExtensions(new ConfigurationApiMergeHandler());
+
+ final Feature featureA = new Feature(ArtifactId.parse("g:a:1"));
+ final ConfigurationApi apiA = new ConfigurationApi();
+ apiA.getFrameworkPropertyDescriptions().put("pa", new FrameworkPropertyDescription());
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+
+ final Feature featureB = new Feature(ArtifactId.parse("g:b:1"));
+ final ConfigurationApi apiB = new ConfigurationApi();
+ apiB.getFrameworkPropertyDescriptions().put("pa", new FrameworkPropertyDescription());
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+
+ final ArtifactId id = ArtifactId.parse("g:m:1");
+ FeatureBuilder.assemble(id, context, featureA, featureB);
+ }
+
+ @Test public void testConfigurationApiMergeInternalNames() {
+ final BuilderContext context = new BuilderContext(id -> null);
+ context.addMergeExtensions(new ConfigurationApiMergeHandler());
+
+ final Feature featureA = new Feature(ArtifactId.parse("g:a:1"));
+ final ConfigurationApi apiA = new ConfigurationApi();
+ apiA.getInternalConfigurations().add("a");
+ apiA.getInternalFactoryConfigurations().add("fa");
+ apiA.getInternalFrameworkProperties().add("pa");
+
+ apiA.getInternalConfigurations().add("c");
+ apiA.getInternalFactoryConfigurations().add("fc");
+ apiA.getInternalFrameworkProperties().add("pc");
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+
+ final Feature featureB = new Feature(ArtifactId.parse("g:b:1"));
+ final ConfigurationApi apiB = new ConfigurationApi();
+ apiB.getInternalConfigurations().add("b");
+ apiB.getInternalFactoryConfigurations().add("fb");
+ apiB.getInternalFrameworkProperties().add("pb");
+
+ apiB.getInternalConfigurations().add("c");
+ apiB.getInternalFactoryConfigurations().add("fc");
+ apiB.getInternalFrameworkProperties().add("pc");
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+
+ final ArtifactId id = ArtifactId.parse("g:m:1");
+ Feature result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ ConfigurationApi api = ConfigurationApi.getConfigurationApi(result);
+ assertNotNull(api);
+
+ assertEquals(3, api.getInternalConfigurations().size());
+ assertTrue(api.getInternalConfigurations().contains("a"));
+ assertTrue(api.getInternalConfigurations().contains("b"));
+ assertTrue(api.getInternalConfigurations().contains("c"));
+
+ assertEquals(3, api.getInternalFactoryConfigurations().size());
+ assertTrue(api.getInternalFactoryConfigurations().contains("fa"));
+ assertTrue(api.getInternalFactoryConfigurations().contains("fb"));
+ assertTrue(api.getInternalFactoryConfigurations().contains("fc"));
+
+ assertEquals(3, api.getInternalFrameworkProperties().size());
+ assertTrue(api.getInternalFrameworkProperties().contains("pa"));
+ assertTrue(api.getInternalFrameworkProperties().contains("pb"));
+ assertTrue(api.getInternalFrameworkProperties().contains("pc"));
+ }
}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDupsTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDupsTest.java
index 84f8e01..1785daa 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDupsTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDupsTest.java
@@ -20,6 +20,7 @@
import org.apache.sling.feature.Feature;
import org.apache.sling.feature.analyser.Analyser;
import org.apache.sling.feature.analyser.AnalyserResult;
+import org.apache.sling.feature.analyser.AnalyserResult.ArtifactReport;
import org.apache.sling.feature.analyser.task.AnalyserTask;
import org.apache.sling.feature.builder.ArtifactProvider;
import org.apache.sling.feature.extension.apiregions.scanner.ApiRegionsExtensionScanner;
@@ -55,13 +56,16 @@
Collections.singletonMap("warningPackages", "x.y.z"));
Analyser a = new Analyser(scanner, configs, at);
AnalyserResult res = a.analyse(f);
- assertEquals(1, res.getErrors().size());
- assertEquals(0, res.getWarnings().size());
+ assertEquals(1, res.getArtifactErrors().size());
+ assertEquals(0, res.getArtifactWarnings().size());
+ assertEquals(0, res.getGlobalWarnings().size());
+ assertEquals(0, res.getGlobalErrors().size());
- String err = res.getErrors().get(0);
- assertTrue(err.contains("a.b.c"));
- assertTrue(err.contains("g:f3:1"));
- assertTrue(err.contains("feature-export2"));
+ ArtifactReport err = res.getArtifactErrors().get(0);
+ assertEquals(ArtifactId.parse("g:exp2:1"), err.getKey());
+ assertTrue(err.getValue().contains("a.b.c"));
+ assertTrue(err.getValue().contains("g:f3:1"));
+ assertTrue(err.getValue().contains("feature-export2"));
}
@Test
@@ -80,13 +84,16 @@
Collections.singletonMap("api-regions-crossfeature-dups", cfg);
Analyser a = new Analyser(scanner, configs, at);
AnalyserResult res = a.analyse(f);
- assertEquals(0, res.getErrors().size());
- assertEquals(1, res.getWarnings().size());
+ assertEquals(0, res.getArtifactErrors().size());
+ assertEquals(1, res.getArtifactWarnings().size());
+ assertEquals(0, res.getGlobalWarnings().size());
+ assertEquals(0, res.getGlobalErrors().size());
- String err = res.getWarnings().get(0);
- assertTrue(err.contains("a.b.c"));
- assertTrue(err.contains("g:f3:1"));
- assertTrue(err.contains("feature-export2"));
+ ArtifactReport err = res.getArtifactWarnings().get(0);
+ assertEquals(ArtifactId.parse("g:exp2:1"), err.getKey());
+ assertTrue(err.getValue().contains("a.b.c"));
+ assertTrue(err.getValue().contains("g:f3:1"));
+ assertTrue(err.getValue().contains("feature-export2"));
}
@Test
@@ -104,13 +111,16 @@
Collections.singletonMap("api-regions-crossfeature-dups", cfg);
Analyser a = new Analyser(scanner, configs, at);
AnalyserResult res = a.analyse(f);
- assertEquals(0, res.getErrors().size());
- assertEquals(1, res.getWarnings().size());
+ assertEquals(0, res.getArtifactErrors().size());
+ assertEquals(1, res.getArtifactWarnings().size());
+ assertEquals(0, res.getGlobalWarnings().size());
+ assertEquals(0, res.getGlobalErrors().size());
- String err = res.getWarnings().get(0);
- assertTrue(err.contains("a.b.c"));
- assertTrue(err.contains("g:f3:1"));
- assertTrue(err.contains("feature-export2"));
+ ArtifactReport err = res.getArtifactWarnings().get(0);
+ assertEquals(ArtifactId.parse("g:exp2:1"), err.getKey());
+ assertTrue(err.getValue().contains("a.b.c"));
+ assertTrue(err.getValue().contains("g:f3:1"));
+ assertTrue(err.getValue().contains("feature-export2"));
}
@Test
@@ -128,8 +138,10 @@
Collections.singletonMap("api-regions-crossfeature-dups", cfg);
Analyser a = new Analyser(scanner, configs, at);
AnalyserResult res = a.analyse(f);
- assertEquals(0, res.getErrors().size());
- assertEquals(0, res.getWarnings().size());
+ assertEquals(0, res.getArtifactErrors().size());
+ assertEquals(0, res.getArtifactWarnings().size());
+ assertEquals(0, res.getGlobalWarnings().size());
+ assertEquals(0, res.getGlobalErrors().size());
}
@Test
@@ -143,12 +155,18 @@
AnalyserTask at = new CheckApiRegionsCrossFeatureDups();
Analyser a = new Analyser(scanner, at);
AnalyserResult res = a.analyse(f);
- assertEquals(2, res.getErrors().size());
- assertEquals(0, res.getWarnings().size());
+ assertEquals(2, res.getArtifactErrors().size());
+ assertEquals(0, res.getArtifactWarnings().size());
+ assertEquals(0, res.getGlobalWarnings().size());
+ assertEquals(0, res.getGlobalErrors().size());
- String allErrs = res.getErrors().get(0) + res.getErrors().get(1);
- assertTrue(allErrs.contains("a.b.c"));
- assertTrue(allErrs.contains("zzz.zzz"));
+ ArtifactReport err1 = res.getArtifactErrors().get(0);
+ assertEquals(ArtifactId.parse("g:extra:1"), err1.getKey());
+ assertTrue(err1.getValue().contains("zzz.zzz"));
+
+ ArtifactReport err2 = res.getArtifactErrors().get(1);
+ assertEquals(ArtifactId.parse("g:exp2:1"), err2.getKey());
+ assertTrue(err2.getValue().contains("a.b.c"));
}
@Test
@@ -165,11 +183,14 @@
Collections.singletonMap("regions", "something,global"));
Analyser a = new Analyser(scanner, configs, at);
AnalyserResult res = a.analyse(f);
- assertEquals(1, res.getErrors().size());
- assertEquals(0, res.getWarnings().size());
+ assertEquals(1, res.getArtifactErrors().size());
+ assertEquals(0, res.getArtifactWarnings().size());
+ assertEquals(0, res.getGlobalWarnings().size());
+ assertEquals(0, res.getGlobalErrors().size());
- String err = res.getErrors().get(0);
- assertTrue(err.contains("zzz.zzz"));
+ ArtifactReport err1 = res.getArtifactErrors().get(0);
+ assertEquals(ArtifactId.parse("g:extra:1"), err1.getKey());
+ assertTrue(err1.getValue().contains("zzz.zzz"));
}
@Test
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 46c1f57..762305d 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
@@ -39,7 +39,7 @@
final ConfigurationValidationResult result = validator.validate(cfg, fcd, null);
assertFalse(result.isValid());
- assertEquals(1, result.getGlobalErrors().size());
+ assertEquals(1, result.getErrors().size());
}
@Test public void testWrongDescriptionTypeForFactoryConfiguration() {
@@ -48,7 +48,7 @@
final ConfigurationValidationResult result = validator.validate(cfg, fcd, null);
assertFalse(result.isValid());
- assertEquals(1, result.getGlobalErrors().size());
+ assertEquals(1, result.getErrors().size());
}
@Test public void testDeprecated() {
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 acbffa5..ea45d0d 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
@@ -50,210 +50,207 @@
final PropertyDescription prop = new PropertyDescription();
prop.setType(PropertyType.BOOLEAN);
- final List<String> messages = new ArrayList<>();
+ PropertyValidationResult result;
+ result = validator.validate(Boolean.TRUE, prop);
+ assertTrue(result.isValid());
- validator.validateBoolean(prop, Boolean.TRUE, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(Boolean.FALSE, prop);
+ assertTrue(result.isValid());
- validator.validateBoolean(prop, Boolean.FALSE, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("TRUE", prop);
+ assertTrue(result.isValid());
- validator.validateBoolean(prop, "TRUE", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("FALSE", prop);
+ assertTrue(result.isValid());
- validator.validateBoolean(prop, "FALSE", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("yes", prop);
+ assertEquals(1, result.getErrors().size());
- validator.validateBoolean(prop, "yes", messages);
- assertEquals(1, messages.size());
- messages.clear();
-
- validator.validateBoolean(prop, 1, messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate(1, prop);
+ assertEquals(1, result.getErrors().size());
}
@Test public void testValidateByte() {
final PropertyDescription prop = new PropertyDescription();
prop.setType(PropertyType.BYTE);
- final List<String> messages = new ArrayList<>();
+ PropertyValidationResult result;
- validator.validateByte(prop, (byte)1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate((byte)1, prop);
+ assertTrue(result.isValid());
- validator.validateByte(prop, "1", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("1", prop);
+ assertTrue(result.isValid());
- validator.validateByte(prop, "yes", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("yes", prop);
+ assertEquals(1, result.getErrors().size());
- validator.validateByte(prop, 1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1, prop);
+ assertTrue(result.isValid());
}
@Test public void testValidateShort() {
final PropertyDescription prop = new PropertyDescription();
prop.setType(PropertyType.SHORT);
- final List<String> messages = new ArrayList<>();
+ PropertyValidationResult result;
- validator.validateShort(prop, (short)1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate((short)1, prop);
+ assertTrue(result.isValid());
- validator.validateShort(prop, "1", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("1", prop);
+ assertTrue(result.isValid());
- validator.validateShort(prop, "yes", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("yes", prop);
+ assertEquals(1, result.getErrors().size());
- validator.validateShort(prop, 1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1, prop);
+ assertTrue(result.isValid());
}
@Test public void testValidateInteger() {
final PropertyDescription prop = new PropertyDescription();
prop.setType(PropertyType.INTEGER);
- final List<String> messages = new ArrayList<>();
+ PropertyValidationResult result;
- validator.validateInteger(prop, 1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1, prop);
+ assertTrue(result.isValid());
- validator.validateInteger(prop, "1", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("1", prop);
+ assertTrue(result.isValid());
- validator.validateInteger(prop, "yes", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("yes", prop);
+ assertEquals(1, result.getErrors().size());
- validator.validateInteger(prop, 1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1, prop);
+ assertTrue(result.isValid());
}
@Test public void testValidateLong() {
final PropertyDescription prop = new PropertyDescription();
prop.setType(PropertyType.LONG);
- final List<String> messages = new ArrayList<>();
+ PropertyValidationResult result;
- validator.validateLong(prop, 1L, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1L, prop);
+ assertTrue(result.isValid());
- validator.validateLong(prop, "1", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("1", prop);
+ assertTrue(result.isValid());
- validator.validateLong(prop, "yes", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("yes", prop);
+ assertEquals(1, result.getErrors().size());
- validator.validateLong(prop, 1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1, prop);
+ assertTrue(result.isValid());
}
@Test public void testValidateFloat() {
final PropertyDescription prop = new PropertyDescription();
prop.setType(PropertyType.FLOAT);
- final List<String> messages = new ArrayList<>();
+ PropertyValidationResult result;
- validator.validateFloat(prop, 1.1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1.1, prop);
+ assertTrue(result.isValid());
- validator.validateFloat(prop, "1.1", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("1.1", prop);
+ assertTrue(result.isValid());
- validator.validateFloat(prop, "yes", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("yes", prop);
+ assertEquals(1, result.getErrors().size());
- validator.validateFloat(prop, 1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1, prop);
+ assertTrue(result.isValid());
}
@Test public void testValidateDouble() {
final PropertyDescription prop = new PropertyDescription();
prop.setType(PropertyType.DOUBLE);
- final List<String> messages = new ArrayList<>();
+ PropertyValidationResult result;
- validator.validateDouble(prop, 1.1d, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1.1d, prop);
+ assertTrue(result.isValid());
- validator.validateDouble(prop, "1.1", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("1.1", prop);
+ assertTrue(result.isValid());
- validator.validateDouble(prop, "yes", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("yes", prop);
+ assertEquals(1, result.getErrors().size());
- validator.validateDouble(prop, 1, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(1, prop);
+ assertTrue(result.isValid());
}
@Test public void testValidateChar() {
final PropertyDescription prop = new PropertyDescription();
prop.setType(PropertyType.CHARACTER);
- final List<String> messages = new ArrayList<>();
+ PropertyValidationResult result;
- validator.validateCharacter(prop, 'x', messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate('x', prop);
+ assertTrue(result.isValid());
- validator.validateCharacter(prop, "y", messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate("y", prop);
+ assertTrue(result.isValid());
- validator.validateCharacter(prop, "yes", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("yes", prop);
+ assertEquals(1, result.getErrors().size());
}
@Test public void testValidateUrl() {
- final List<String> messages = new ArrayList<>();
+ final PropertyDescription prop = new PropertyDescription();
+ prop.setType(PropertyType.URL);
- validator.validateURL(null, "https://sling.apache.org/documentation", messages);
- assertTrue(messages.isEmpty());
+ PropertyValidationResult result;
- validator.validateURL(null, "hello world", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("https://sling.apache.org/documentation", prop);
+ assertTrue(result.isValid());
+
+ result = validator.validate("hello world", prop);
+ assertEquals(1, result.getErrors().size());
}
@Test public void testValidateEmail() {
- final List<String> messages = new ArrayList<>();
+ final PropertyDescription prop = new PropertyDescription();
+ prop.setType(PropertyType.EMAIL);
- validator.validateEmail(null, "a@b.com", messages);
- assertTrue(messages.isEmpty());
+ PropertyValidationResult result;
- validator.validateEmail(null, "hello world", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("a@b.com", prop);
+ assertTrue(result.isValid());
+
+ result = validator.validate("hello world", prop);
+ assertEquals(1, result.getErrors().size());
}
@Test public void testValidatePassword() {
final PropertyDescription prop = new PropertyDescription();
- final List<String> messages = new ArrayList<>();
+ prop.setType(PropertyType.PASSWORD);
- validator.validatePassword(prop, null, messages);
- assertEquals(1, messages.size());
- messages.clear();
+ PropertyValidationResult result;
+
+ result = validator.validate(null, prop);
+ assertTrue(result.isValid());
prop.setVariable("secret");
- validator.validatePassword(prop, null, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(null, prop);
+ assertTrue(result.isValid());
}
@Test public void testValidatePath() {
- final List<String> messages = new ArrayList<>();
+ final PropertyDescription prop = new PropertyDescription();
+ prop.setType(PropertyType.PATH);
- validator.validatePath(null, "/a/b/c", messages);
- assertTrue(messages.isEmpty());
+ PropertyValidationResult result;
- validator.validateEmail(null, "hello world", messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate("/a/b/c", prop);
+ assertTrue(result.isValid());
+
+ result = validator.validate("hello world", prop);
+ assertEquals(1, result.getErrors().size());
}
@Test public void testValidateRange() {
@@ -352,7 +349,6 @@
}
@Test public void testValidateList() {
- final List<String> messages = new ArrayList<>();
final PropertyDescription prop = new PropertyDescription();
final List<Object> values = new ArrayList<>();
@@ -361,44 +357,83 @@
values.add("c");
// default cardinality - no excludes/includes
- validator.validateList(prop, values, messages);
- assertEquals(1, messages.size());
- messages.clear();
+ PropertyValidationResult result;
+ result = validator.validate(values, prop);
+ assertEquals(1, result.getErrors().size());
// cardinality 3 - no excludes/includes
prop.setCardinality(3);
- validator.validateList(prop, values, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(values, prop);
+ assertTrue(result.getErrors().isEmpty());
values.add("d");
- validator.validateList(prop, values, messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate(values, prop);
+ assertEquals(1, result.getErrors().size());
// excludes
prop.setExcludes(new String[] {"d", "e"});
- validator.validateList(prop, values, messages);
- assertEquals(2, messages.size()); // cardinality and exclude
- messages.clear();
+ result = validator.validate(values, prop);
+ assertEquals(2, result.getErrors().size()); // cardinality and exclude
values.remove("d");
- validator.validateList(prop, values, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(values, prop);
+ assertTrue(result.getErrors().isEmpty());
// includes
prop.setIncludes(new String[] {"b"});
- validator.validateList(prop, values, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(values, prop);
+ assertTrue(result.getErrors().isEmpty());
prop.setIncludes(new String[] {"x"});
- validator.validateList(prop, values, messages);
- assertEquals(1, messages.size());
- messages.clear();
+ result = validator.validate(values, prop);
+ assertEquals(1, result.getErrors().size());
values.add("x");
values.remove("a");
- validator.validateList(prop, values, messages);
- assertTrue(messages.isEmpty());
+ result = validator.validate(values, prop);
+ assertTrue(result.getErrors().isEmpty());
+ }
+
+ @Test public void testValidateArray() {
+ final PropertyDescription prop = new PropertyDescription();
+
+ String[] values = new String[] {"a", "b", "c"};
+
+ // default cardinality - no excludes/includes
+ PropertyValidationResult result;
+ result = validator.validate(values, prop);
+ assertEquals(1, result.getErrors().size());
+
+ // cardinality 3 - no excludes/includes
+ prop.setCardinality(3);
+ result = validator.validate(values, prop);
+ assertTrue(result.getErrors().isEmpty());
+
+ values = new String[] {"a", "b", "c", "d"};
+ result = validator.validate(values, prop);
+ assertEquals(1, result.getErrors().size());
+
+ // excludes
+ prop.setExcludes(new String[] {"d", "e"});
+ result = validator.validate(values, prop);
+ assertEquals(2, result.getErrors().size()); // cardinality and exclude
+
+ values = new String[] {"a", "b", "c"};
+ result = validator.validate(values, prop);
+ assertTrue(result.getErrors().isEmpty());
+
+ // includes
+ prop.setIncludes(new String[] {"b"});
+ result = validator.validate(values, prop);
+ assertTrue(result.getErrors().isEmpty());
+
+ prop.setIncludes(new String[] {"x"});
+ result = validator.validate(values, prop);
+ assertEquals(1, result.getErrors().size());
+
+ values = new String[] {"b", "c", "x"};
+ result = validator.validate(values, prop);
+ assertTrue(result.getErrors().isEmpty());
}
@Test public void testDeprecation() {
