This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch SLING-9867
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/SLING-9867 by this push:
new 58d8095 Implement region merging and feature validation
58d8095 is described below
commit 58d80954b07e8a622972c2f80f9156a178ce09dd
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Tue Nov 17 07:28:25 2020 +0100
Implement region merging and feature validation
---
pom.xml | 2 +-
.../config/validation/ConfigurationValidator.java | 25 +-
.../config/validation/FeatureValidationResult.java | 11 +
.../api/config/validation/FeatureValidator.java | 132 ++++++-
.../api/config/validation/PropertyValidator.java | 2 +-
.../ConfigurationApiMergeHandlerTest.java | 84 +++-
.../validation/ConfigurationValidatorTest.java | 26 +-
.../config/validation/FeatureValidatorTest.java | 436 +++++++++++++++++++++
.../config/validation/PropertyValidatorTest.java | 10 +-
9 files changed, 675 insertions(+), 53 deletions(-)
diff --git a/pom.xml b/pom.xml
index 0a929c5..d8e03c0 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.13-SNAPSHOT</version>
+ <version>1.2.14</version>
<scope>provided</scope>
</dependency>
<dependency>
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 53ea26c..a7e505d 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
@@ -27,6 +27,7 @@ import
org.apache.sling.feature.extension.apiregions.api.config.ConfigurableEnti
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.PropertyDescription;
+import org.apache.sling.feature.extension.apiregions.api.config.Region;
import org.osgi.framework.Constants;
/**
@@ -46,30 +47,24 @@ public class ConfigurationValidator {
/**
* Validate a configuration
- * @param The configuration description
* @param config The OSGi configuration
+ * @param desc The configuration description
+ * @param region The optional region for the configuration
* @return The result
*/
- public ConfigurationValidationResult validate(final ConfigurableEntity
desc, final Configuration config) {
+ public ConfigurationValidationResult validate(final Configuration config,
final ConfigurableEntity desc, final Region region) {
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");
} else {
- final FactoryConfigurationDescription fDesc =
(FactoryConfigurationDescription)desc;
- if ( fDesc.getOperations().isEmpty() ) {
- result.getGlobalErrors().add("Factory configuration not
allowed");
- } else if (
fDesc.getInternalNames().contains(config.getName())) {
- result.getGlobalErrors().add("Factory configuration with
name " + config.getName() + " not allowed");
- } else {
- validateProperties(desc, config,
result.getPropertyResults());
- }
+ validateProperties(desc, config, result.getPropertyResults(),
region);
}
} else {
if ( !(desc instanceof ConfigurationDescription) ) {
result.getGlobalErrors().add("Configuration cannot be
validated against factory configuration description");
} else {
- validateProperties(desc, config, result.getPropertyResults());
+ validateProperties(desc, config, result.getPropertyResults(),
region);
}
}
@@ -81,11 +76,12 @@ public class ConfigurationValidator {
void validateProperties(final ConfigurableEntity desc,
final Configuration configuration,
- final Map<String, PropertyValidationResult> results) {
+ final Map<String, PropertyValidationResult> results,
+ final Region region) {
final Dictionary<String, Object> properties =
configuration.getConfigurationProperties();
for(final Map.Entry<String, PropertyDescription> propEntry :
desc.getPropertyDescriptions().entrySet()) {
final Object value = properties.get(propEntry.getKey());
- final PropertyValidationResult result =
propertyValidator.validate(propEntry.getValue(), value);
+ final PropertyValidationResult result =
propertyValidator.validate(value, propEntry.getValue());
results.put(propEntry.getKey(), result);
}
final Enumeration<String> keyEnum = properties.keys();
@@ -99,9 +95,8 @@ public class ConfigurationValidator {
if ( !(value instanceof Integer) ) {
result.getErrors().add("service.ranking must be of
type Integer");
}
- } else if ( !ALLOWED_PROPERTIES.contains(propName) ) {
+ } else if ( !ALLOWED_PROPERTIES.contains(propName) && region
!= Region.INTERNAL ) {
result.getErrors().add("Property is not allowed");
-
}
}
}
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 1e49dd0..09def1a 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
@@ -19,10 +19,17 @@ package
org.apache.sling.feature.extension.apiregions.api.config.validation;
import java.util.HashMap;
import java.util.Map;
+/**
+ * Validation result for a feature
+ */
public class FeatureValidationResult {
private final Map<String, ConfigurationValidationResult>
configurationResults = new HashMap<>();
+ /**
+ * Is the configuration of the feature valid?
+ * @return {@code true} if it is valid
+ */
public boolean isValid() {
boolean valid = true;
for(final ConfigurationValidationResult r :
this.configurationResults.values()) {
@@ -34,6 +41,10 @@ public class FeatureValidationResult {
return valid;
}
+ /**
+ * Get the confiugration validation results.
+ * @return The results keyed by configuration PIDs
+ */
public Map<String, ConfigurationValidationResult>
getConfigurationResults() {
return this.configurationResults;
}
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 770bc60..95df5db 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,11 +16,17 @@
*/
package org.apache.sling.feature.extension.apiregions.api.config.validation;
+import java.util.List;
+
+import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Configuration;
import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.builder.FeatureProvider;
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.Operation;
+import org.apache.sling.feature.extension.apiregions.api.config.Region;
/**
* Validator to validate a feature
@@ -29,37 +35,123 @@ public class FeatureValidator {
private final ConfigurationValidator configurationValidator = new
ConfigurationValidator();
+ private volatile FeatureProvider featureProvider;
+
+ /**
+ * Get the current feature provider
+ * @return the feature provider or {@code null}
+ */
+ public FeatureProvider getFeatureProvider() {
+ return featureProvider;
+ }
+
+ /**
+ * Set the feature provider
+ * @param featureProvider the feature provider to set
+ */
+ public void setFeatureProvider(final FeatureProvider provider) {
+ this.featureProvider = provider;
+ }
+
/**
* Validate the feature against the configuration API
- * @param api The configuration API
* @param feature The feature
+ * @param api The configuration API
* @return A {@code FeatureValidationResult}
+ * @throws IllegalArgumentException If api is {@code null}
*/
- public FeatureValidationResult validate(final ConfigurationApi api, final
Feature feature) {
+ public FeatureValidationResult validate(final Feature feature, final
ConfigurationApi api) {
final FeatureValidationResult result = new FeatureValidationResult();
+ if ( api == null ) {
+ throw new IllegalArgumentException();
+ }
for(final Configuration config : feature.getConfigurations()) {
- if ( config.isFactoryConfiguration() ) {
- final FactoryConfigurationDescription desc =
api.getFactoryConfigurationDescriptions().get(config.getFactoryPid());
- if ( desc != null ) {
- final ConfigurationValidationResult r =
configurationValidator.validate(desc, config);
- result.getConfigurationResults().put(config.getPid(), r);
- } else if (
api.getInternalFactoryConfigurations().contains(config.getFactoryPid())) {
- final ConfigurationValidationResult cvr = new
ConfigurationValidationResult();
- cvr.getGlobalErrors().add("Factory configuration is not
allowed");
- result.getConfigurationResults().put(config.getPid(), cvr);
- }
+ final RegionInfo regionInfo = getRegionInfo(feature, config);
+
+ if ( regionInfo == null ) {
+ final ConfigurationValidationResult cvr = new
ConfigurationValidationResult();
+ cvr.getGlobalErrors().add("Unable to properly validate
configuration, region info cannot be determined");
+ result.getConfigurationResults().put(config.getPid(), cvr);
} else {
- final ConfigurationDescription desc =
api.getConfigurationDescriptions().get(config.getPid());
- if ( desc != null ) {
- final ConfigurationValidationResult r =
configurationValidator.validate(desc, config);
- result.getConfigurationResults().put(config.getPid(), r);
- } else if (
api.getInternalConfigurations().contains(config.getPid())) {
- final ConfigurationValidationResult cvr = new
ConfigurationValidationResult();
- cvr.getGlobalErrors().add("Configuration is not allowed");
- result.getConfigurationResults().put(config.getPid(), cvr);
+ if ( config.isFactoryConfiguration() ) {
+ final FactoryConfigurationDescription desc =
api.getFactoryConfigurationDescriptions().get(config.getFactoryPid());
+ if ( desc != null ) {
+ final ConfigurationValidationResult r =
configurationValidator.validate(config, desc, regionInfo.region);
+ result.getConfigurationResults().put(config.getPid(),
r);
+ if ( regionInfo.region != Region.INTERNAL ) {
+ if ( desc.getOperations().isEmpty() ) {
+ r.getGlobalErrors().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");
+ } else if ( !regionInfo.isUpdate &&
!desc.getOperations().contains(Operation.CREATE)) {
+ r.getGlobalErrors().add("Creation of
factory configuration is not allowed");
+ }
+ }
+ if (
desc.getInternalNames().contains(config.getName())) {
+ r.getGlobalErrors().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");
+ result.getConfigurationResults().put(config.getPid(),
cvr);
+ }
+ } else {
+ final ConfigurationDescription desc =
api.getConfigurationDescriptions().get(config.getPid());
+ if ( desc != null ) {
+ final ConfigurationValidationResult r =
configurationValidator.validate(config, desc, regionInfo.region);
+ 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");
+ result.getConfigurationResults().put(config.getPid(),
cvr);
+ }
+ }
+ }
+ // make sure a result exists
+ result.getConfigurationResults().computeIfAbsent(config.getPid(),
id -> new ConfigurationValidationResult());
+ }
+ return result;
+ }
+
+ static final class RegionInfo {
+
+ public Region region;
+
+ public boolean isUpdate;
+ }
+
+ RegionInfo getRegionInfo(final Feature feature, final Configuration cfg) {
+ final FeatureProvider provider = this.getFeatureProvider();
+ final RegionInfo result = new RegionInfo();
+
+ final List<ArtifactId> list = cfg.getFeatureOrigins();
+ if ( !list.isEmpty() ) {
+ boolean global = false;
+ for(final ArtifactId id : list) {
+ final Feature f = provider == null ? null :
provider.provide(id);
+ if ( f == null ) {
+ return null;
+ }
+ final ConfigurationApi api =
ConfigurationApi.getConfigurationApi(f);
+ if ( api == null || api.getRegion() != Region.INTERNAL ) {
+ global = true;
+ break;
}
}
+ result.region = global ? Region.GLOBAL : Region.INTERNAL;
+ result.isUpdate = list.size() > 1;
+ } else {
+ final ConfigurationApi api =
ConfigurationApi.getConfigurationApi(feature);
+ if ( api == null || api.getRegion() == null || api.getRegion() ==
Region.GLOBAL ) {
+ result.region = Region.GLOBAL;
+ } else {
+ result.region = Region.INTERNAL;
+ }
+ result.isUpdate = false;
}
return result;
}
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 3bd72d8..82f7a1c 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
@@ -35,7 +35,7 @@ public class PropertyValidator {
* Validate the value against the property definition
* @return A property validation result
*/
- public PropertyValidationResult validate(final PropertyDescription
prop, final Object value) {
+ public PropertyValidationResult validate(final Object value, final
PropertyDescription prop) {
final PropertyValidationResult result = new
PropertyValidationResult();
if ( value == null ) {
if ( prop.isRequired() ) {
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 5856910..7c95589 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
@@ -112,4 +112,86 @@ public class ConfigurationApiMergeHandlerTest {
api = ConfigurationApi.getConfigurationApi(result);
assertEquals(Region.GLOBAL, api.getRegion());
}
- }
\ No newline at end of file
+
+ @Test public void testRegionMerge() {
+ // always return prototype
+ 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();
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+
+ final Feature featureB = new Feature(ArtifactId.parse("g:b:1"));
+ final ConfigurationApi apiB = new ConfigurationApi();
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+
+ // no region
+ final ArtifactId id = ArtifactId.parse("g:m:1");
+ Feature result = FeatureBuilder.assemble(id, context, featureA,
featureB);
+ ConfigurationApi api = ConfigurationApi.getConfigurationApi(result);
+ assertNotNull(api);
+ assertNull(api.getRegion());
+
+ // only A has region
+ apiA.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+ result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ api = ConfigurationApi.getConfigurationApi(result);
+ assertEquals(Region.GLOBAL, api.getRegion());
+
+ apiA.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+ result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ api = ConfigurationApi.getConfigurationApi(result);
+ assertEquals(Region.GLOBAL, api.getRegion());
+
+ // only B has region
+ apiA.setRegion(null);
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+ apiB.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+ result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ api = ConfigurationApi.getConfigurationApi(result);
+ assertEquals(Region.GLOBAL, api.getRegion());
+
+ apiB.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+ result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ api = ConfigurationApi.getConfigurationApi(result);
+ assertEquals(Region.GLOBAL, api.getRegion());
+
+ // both have region
+ apiA.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+ apiB.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+ result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ api = ConfigurationApi.getConfigurationApi(result);
+ assertEquals(Region.INTERNAL, api.getRegion());
+
+ apiA.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+ apiB.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+ result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ api = ConfigurationApi.getConfigurationApi(result);
+ assertEquals(Region.GLOBAL, api.getRegion());
+
+ apiA.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+ apiB.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+ result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ api = ConfigurationApi.getConfigurationApi(result);
+ assertEquals(Region.GLOBAL, api.getRegion());
+
+ apiA.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(featureA, apiA);
+ apiB.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(featureB, apiB);
+ result = FeatureBuilder.assemble(id, context, featureA, featureB);
+ api = ConfigurationApi.getConfigurationApi(result);
+ assertEquals(Region.GLOBAL, api.getRegion());
+ }
+}
\ No newline at end of file
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 8620daf..46c1f57 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
@@ -25,6 +25,7 @@ import
org.apache.sling.feature.extension.apiregions.api.config.ConfigurationDes
import
org.apache.sling.feature.extension.apiregions.api.config.FactoryConfigurationDescription;
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.Test;
import org.osgi.framework.Constants;
@@ -36,7 +37,7 @@ public class ConfigurationValidatorTest {
final Configuration cfg = new Configuration("org.apache");
final FactoryConfigurationDescription fcd = new
FactoryConfigurationDescription();
- final ConfigurationValidationResult result = validator.validate(fcd,
cfg);
+ final ConfigurationValidationResult result = validator.validate(cfg,
fcd, null);
assertFalse(result.isValid());
assertEquals(1, result.getGlobalErrors().size());
}
@@ -45,7 +46,7 @@ public class ConfigurationValidatorTest {
final Configuration cfg = new Configuration("org.apache~foo");
final ConfigurationDescription fcd = new ConfigurationDescription();
- final ConfigurationValidationResult result = validator.validate(fcd,
cfg);
+ final ConfigurationValidationResult result = validator.validate(cfg,
fcd, null);
assertFalse(result.isValid());
assertEquals(1, result.getGlobalErrors().size());
}
@@ -54,12 +55,12 @@ public class ConfigurationValidatorTest {
final Configuration cfg = new Configuration("org.apache");
final ConfigurationDescription cd = new ConfigurationDescription();
- ConfigurationValidationResult result = validator.validate(cd, cfg);
+ ConfigurationValidationResult result = validator.validate(cfg, cd,
null);
assertTrue(result.isValid());
assertTrue(result.getWarnings().isEmpty());
cd.setDeprecated("this is deprecated");
- result = validator.validate(cd, cfg);
+ result = validator.validate(cfg, cd, null);
assertTrue(result.isValid());
assertFalse(result.getWarnings().isEmpty());
assertEquals("this is deprecated", result.getWarnings().get(0));
@@ -70,11 +71,11 @@ public class ConfigurationValidatorTest {
final ConfigurationDescription cd = new ConfigurationDescription();
cfg.getProperties().put(Constants.SERVICE_RANKING, 5);
- ConfigurationValidationResult result = validator.validate(cd, cfg);
+ ConfigurationValidationResult result = validator.validate(cfg, cd,
null);
assertTrue(result.isValid());
cfg.getProperties().put(Constants.SERVICE_RANKING, "5");
- result = validator.validate(cd, cfg);
+ result = validator.validate(cfg, cd, null);
assertFalse(result.isValid());
}
@@ -84,7 +85,7 @@ public class ConfigurationValidatorTest {
cfg.getProperties().put(Constants.SERVICE_DESCRIPTION, "desc");
cfg.getProperties().put(Constants.SERVICE_VENDOR, "vendor");
- ConfigurationValidationResult result = validator.validate(cd, cfg);
+ ConfigurationValidationResult result = validator.validate(cfg, cd,
null);
assertTrue(result.isValid());
}
@@ -96,17 +97,22 @@ public class ConfigurationValidatorTest {
final PropertyDescription prop = new PropertyDescription();
cd.getPropertyDescriptions().put("a", prop);
- ConfigurationValidationResult result = validator.validate(cd, cfg);
+ ConfigurationValidationResult result = validator.validate(cfg, cd,
Region.GLOBAL);
assertTrue(result.isValid());
assertEquals(1, result.getPropertyResults().size());
assertTrue(result.getPropertyResults().get("a").isValid());
cfg.getProperties().put("b", "vendor");
- result = validator.validate(cd, cfg);
+ result = validator.validate(cfg, cd, Region.GLOBAL);
assertFalse(result.isValid());
assertEquals(2, result.getPropertyResults().size());
assertTrue(result.getPropertyResults().get("a").isValid());
assertFalse(result.getPropertyResults().get("b").isValid());
+
+ // allowed if internal
+ result = validator.validate(cfg, cd, Region.INTERNAL);
+ assertTrue(result.isValid());
+ assertEquals(2, result.getPropertyResults().size());
}
@Test public void testInvalidProperty() {
@@ -121,7 +127,7 @@ public class ConfigurationValidatorTest {
propB.setType(PropertyType.INTEGER);
cd.getPropertyDescriptions().put("b", propB);
- ConfigurationValidationResult result = validator.validate(cd, cfg);
+ ConfigurationValidationResult result = validator.validate(cfg, cd,
null);
assertFalse(result.isValid());
assertEquals(2, result.getPropertyResults().size());
assertTrue(result.getPropertyResults().get("a").isValid());
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
new file mode 100644
index 0000000..fb4fa0c
--- /dev/null
+++
b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidatorTest.java
@@ -0,0 +1,436 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+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.Arrays;
+import java.util.Collections;
+
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.Configuration;
+import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.builder.FeatureProvider;
+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.Operation;
+import
org.apache.sling.feature.extension.apiregions.api.config.PropertyDescription;
+import org.apache.sling.feature.extension.apiregions.api.config.Region;
+import org.junit.Before;
+import org.junit.Test;
+
+public class FeatureValidatorTest {
+
+ private static final String PID = "org.apache.sling";
+
+ private static final String FACTORY_PID = "org.apache.sling.factory";
+
+ private final FeatureValidator validator = new FeatureValidator();
+
+ @Before public void setup() {
+ this.validator.setFeatureProvider(null);
+ }
+
+ private Feature createFeature(final String id) {
+ final Feature f= new Feature(ArtifactId.parse(id));
+ final Configuration c = new Configuration(PID);
+ c.getProperties().put("prop", "a");
+ f.getConfigurations().add(c);
+
+ final Configuration fc = new
Configuration(FACTORY_PID.concat("~print"));
+ fc.getProperties().put("key", "value");
+ f.getConfigurations().add(fc);
+
+ return f;
+ }
+
+ private ConfigurationApi createApi() {
+ final ConfigurationApi api = new ConfigurationApi();
+
+ final ConfigurationDescription cd = new ConfigurationDescription();
+ cd.getPropertyDescriptions().put("prop", new PropertyDescription());
+
+ api.getConfigurationDescriptions().put(PID, cd);
+
+ final FactoryConfigurationDescription fd = new
FactoryConfigurationDescription();
+ fd.getPropertyDescriptions().put("key", new PropertyDescription());
+
+ api.getFactoryConfigurationDescriptions().put(FACTORY_PID, fd);
+
+ return api;
+ }
+
+ @Test public void testGetRegionInfoNoOrigin() {
+ final Feature f1 = createFeature("g:a:1");
+ final Configuration cfg = f1.getConfigurations().getConfiguration(PID);
+
+ // no api set
+ FeatureValidator.RegionInfo info = validator.getRegionInfo(f1, cfg);
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // empty region in api
+ final ConfigurationApi api = createApi();
+ ConfigurationApi.setConfigurationApi(f1, api);
+ info = validator.getRegionInfo(f1, cfg);
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // global region in api
+ api.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ info = validator.getRegionInfo(f1, cfg);
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // internal region in api
+ api.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ info = validator.getRegionInfo(f1, cfg);
+ assertEquals(Region.INTERNAL, info.region);
+ assertFalse(info.isUpdate);
+ }
+
+ @Test public void testGetRegionInfoSingleOrigin() {
+ final Feature f1 = createFeature("g:a:1");
+ final Configuration cfg = f1.getConfigurations().getConfiguration(PID);
+
+ final Feature f2 = createFeature("g:b:1");
+ cfg.setFeatureOrigins(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, cfg);
+ 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, cfg);
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // global in api
+ api2.setRegion(Region.GLOBAL);
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ info = validator.getRegionInfo(f1, cfg);
+ assertEquals(Region.GLOBAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // internal in api
+ api2.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f2, api2);
+ info = validator.getRegionInfo(f1, cfg);
+ assertEquals(Region.INTERNAL, info.region);
+ assertFalse(info.isUpdate);
+
+ // unknown id
+ this.validator.setFeatureProvider(id -> null);
+
cfg.setFeatureOrigins(Collections.singletonList(ArtifactId.parse("g:xy:1")));
+ info = validator.getRegionInfo(f1, cfg);
+ assertNull(info);
+ }
+
+ @Test public void testGetRegionInfoMultipleOrigins() {
+ final Feature f1 = createFeature("g:a:1");
+ final Configuration cfg = f1.getConfigurations().getConfiguration(PID);
+
+ final Feature f2 = createFeature("g:b:1");
+ final Feature f3 = createFeature("g:c:1");
+ cfg.setFeatureOrigins(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, cfg);
+ 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, cfg);
+ 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, cfg);
+ 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, cfg);
+ 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, cfg);
+ assertEquals(Region.GLOBAL, info.region);
+ assertTrue(info.isUpdate);
+ }
+ @Test public void testSingleValidation() {
+ final Feature f1 = createFeature("g:a:1");
+ final ConfigurationApi api = createApi();
+ ConfigurationApi.setConfigurationApi(f1, api);
+
+ FeatureValidationResult result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+ // add property
+ f1.getConfigurations().getConfiguration(PID).getProperties().put("b",
"x");
+ result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+ }
+
+ @Test public void testInternalConfiguration() {
+ 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 configurations as internal
+ api.getInternalConfigurations().add(PID);
+ api.getInternalFactoryConfigurations().add(FACTORY_PID);
+ ConfigurationApi.setConfigurationApi(f1, api);
+
+ // global region
+ result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+ assertFalse(result.getConfigurationResults().get(PID).isValid());
+
assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid());
+
+ // internal region
+ api.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ }
+
+ @Test public void testInternalFactoryNames() {
+ final Feature f1 = createFeature("g:a:1");
+
+ final Configuration fa = new Configuration(FACTORY_PID.concat("~a"));
+ fa.getProperties().put("key", "value");
+ f1.getConfigurations().add(fa);
+
+ final Configuration fb = new Configuration(FACTORY_PID.concat("~b"));
+ fb.getProperties().put("key", "value");
+ f1.getConfigurations().add(fb);
+
+ final ConfigurationApi api = createApi();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getInternalNames().add("a");
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getInternalNames().add("b");
+ ConfigurationApi.setConfigurationApi(f1, api);
+
+ FeatureValidationResult result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+
assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~a")).isValid());
+
assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~b")).isValid());
+
assertTrue(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid());
+
+ // internal region
+ api.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ }
+
+ @Test public void testFactoryConfigurationOperationsWithCreate() {
+ final Feature f1 = createFeature("g:a:1");
+ final ConfigurationApi api = createApi();
+
+ // no operation -> fail
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+ ConfigurationApi.setConfigurationApi(f1, api);
+ FeatureValidationResult result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+
assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid());
+
+ // only update -> fail
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+
assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid());
+
+ // only create -> success
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+ // update, create -> success
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE);
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+ // internal region -> always success
+ api.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ }
+
+ @Test public void testFactoryConfigurationOperationsWithUpdate() {
+ final Feature f1 = createFeature("g:a:1");
+ final ConfigurationApi api = createApi();
+
+ final Configuration cfg =
f1.getConfigurations().getConfiguration(FACTORY_PID.concat("~print"));
+
+ final Feature f2 = createFeature("g:b:1");
+ final Feature f3 = createFeature("g:c:1");
+ cfg.setFeatureOrigins(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 operation -> fail
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+ ConfigurationApi.setConfigurationApi(f1, api);
+ FeatureValidationResult result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+
assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid());
+
+ // only update -> success
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+ // only create -> fail
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertFalse(result.isValid());
+
assertFalse(result.getConfigurationResults().get(FACTORY_PID.concat("~print")).isValid());
+
+ // update, create -> success
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE);
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+ // internal region -> always success
+ api.setRegion(Region.INTERNAL);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ ConfigurationApi.setConfigurationApi(f2, api);
+ ConfigurationApi.setConfigurationApi(f3, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.UPDATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().add(Operation.CREATE);
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+
+
api.getFactoryConfigurationDescriptions().get(FACTORY_PID).getOperations().clear();
+ ConfigurationApi.setConfigurationApi(f1, api);
+ result = validator.validate(f1, api);
+ assertTrue(result.isValid());
+ }
+}
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 64cbddb..acbffa5 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
@@ -37,13 +37,13 @@ public class PropertyValidatorTest {
final PropertyDescription prop = new PropertyDescription();
// prop not required - no error
- assertTrue(validator.validate(prop, null).getErrors().isEmpty());
- assertTrue(validator.validate(prop, null).isValid());
+ assertTrue(validator.validate(null, prop).getErrors().isEmpty());
+ assertTrue(validator.validate(null, prop).isValid());
// prop required - error
prop.setRequired(true);
- assertEquals(1, validator.validate(prop, null).getErrors().size());
- assertFalse(validator.validate(prop, null).isValid());
+ assertEquals(1, validator.validate(null, prop).getErrors().size());
+ assertFalse(validator.validate(null, prop).isValid());
}
@Test public void testValidateBoolean() {
@@ -405,7 +405,7 @@ public class PropertyValidatorTest {
final PropertyDescription prop = new PropertyDescription();
prop.setDeprecated("This is deprecated");
- final PropertyValidationResult result = validator.validate(prop,
"foo");
+ final PropertyValidationResult result = validator.validate("foo",
prop);
assertTrue(result.isValid());
assertEquals(1, result.getWarnings().size());
assertEquals("This is deprecated", result.getWarnings().get(0));
