You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2020/11/09 06:36:14 UTC

[sling-org-apache-sling-feature-extension-apiregions] branch SLING-9867 updated: Rename types, add initial validation for complete features and configurations

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 6fece35  Rename types, add initial validation for complete features and configurations
6fece35 is described below

commit 6fece3555e07552ddaa437a78343c5f4c4efd20c
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Mon Nov 9 07:34:04 2020 +0100

    Rename types, add initial validation for complete features and configurations
---
 .../apiregions/api/config/ConfigurableEntity.java  |  12 +-
 .../apiregions/api/config/ConfigurationApi.java    |  50 +++---
 ...guration.java => ConfigurationDescription.java} |   2 +-
 ...n.java => FactoryConfigurationDescription.java} |   2 +-
 ...erty.java => FrameworkPropertyDescription.java} |   2 +-
 .../apiregions/api/config/InternalConstants.java   |   4 +-
 .../{Property.java => PropertyDescription.java}    |   4 +-
 .../ConfigurationValidationResult.java}            |  29 +++-
 .../config/validation/ConfigurationValidator.java  |  97 +++++++++++
 .../FeatureValidationResult.java}                  |  18 +-
 .../api/config/validation/FeatureValidator.java    |  70 ++++++++
 .../PropertyValidationResult.java}                 |  27 ++-
 .../PropertyValidator.java}                        |  75 ++++----
 .../api/config/validation/package-info.java        |  23 +++
 .../api/config/ConfigurableEntityTest.java         |  14 +-
 .../api/config/ConfigurationApiTest.java           |  48 ++---
 ...va => FactoryConfigurationDescriptionTest.java} |  12 +-
 .../apiregions/api/config/PropertyTest.java        |   8 +-
 .../PropertyValidatorTest.java}                    | 193 +++++++++++----------
 19 files changed, 466 insertions(+), 224 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntity.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntity.java
index b7edfe6..73513fb 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntity.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntity.java
@@ -32,7 +32,7 @@ import javax.json.JsonValue;
 public abstract class ConfigurableEntity extends DescribableEntity {
 	
 	/** The properties */
-    private final Map<String, Property> properties = new LinkedHashMap<>();
+    private final Map<String, PropertyDescription> properties = new LinkedHashMap<>();
 
     /**
      * Clear the object and remove all metadata
@@ -54,9 +54,9 @@ public abstract class ConfigurableEntity extends DescribableEntity {
             final JsonValue val = this.getAttributes().remove(InternalConstants.KEY_PROPERTIES);
             if ( val != null ) {
                 for(final Map.Entry<String, JsonValue> innerEntry : val.asJsonObject().entrySet()) {
-					final Property prop = new Property();
+					final PropertyDescription prop = new PropertyDescription();
 					prop.fromJSONObject(innerEntry.getValue().asJsonObject());
-                    this.getProperties().put(innerEntry.getKey(), prop);
+                    this.getPropertyDescriptions().put(innerEntry.getKey(), prop);
                 }
             }            
         } catch (final JsonException | IllegalArgumentException e) {
@@ -68,7 +68,7 @@ public abstract class ConfigurableEntity extends DescribableEntity {
 	 * Get the properties
 	 * @return The properties
 	 */
-    public Map<String, Property> getProperties() {
+    public Map<String, PropertyDescription> getPropertyDescriptions() {
         return this.properties;
     }
 
@@ -81,9 +81,9 @@ public abstract class ConfigurableEntity extends DescribableEntity {
 	JsonObjectBuilder createJson() throws IOException {
 		final JsonObjectBuilder objBuilder = super.createJson();
 
-		if ( !this.getProperties().isEmpty() ) {
+		if ( !this.getPropertyDescriptions().isEmpty() ) {
 			final JsonObjectBuilder propBuilder = Json.createObjectBuilder();
-			for(final Map.Entry<String, Property> entry : this.getProperties().entrySet()) {
+			for(final Map.Entry<String, PropertyDescription> entry : this.getPropertyDescriptions().entrySet()) {
 				propBuilder.add(entry.getKey(), entry.getValue().createJson());
 			}
 			objBuilder.add(InternalConstants.KEY_PROPERTIES, propBuilder);
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 c97040a..558a0fe 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
@@ -79,13 +79,13 @@ public class ConfigurationApi extends AttributeableEntity {
     }
    
     /** The map of configurations */
- 	private final Map<String, Configuration> configurations = new LinkedHashMap<>();
+ 	private final Map<String, ConfigurationDescription> configurations = new LinkedHashMap<>();
 
     /** The map of factory configurations */
-    private final Map<String, FactoryConfiguration> factories = new LinkedHashMap<>();
+    private final Map<String, FactoryConfigurationDescription> factories = new LinkedHashMap<>();
 
     /** The map of framework properties */
-    private final Map<String, FrameworkProperty> frameworkProperties = new LinkedHashMap<>();
+    private final Map<String, FrameworkPropertyDescription> frameworkProperties = new LinkedHashMap<>();
 
     /** The list of internal configuration names */
     private final List<String> internalConfigurations = new ArrayList<>();
@@ -122,27 +122,27 @@ public class ConfigurationApi extends AttributeableEntity {
             val = this.getAttributes().remove(InternalConstants.KEY_CONFIGURATIONS);
             if ( val != null ) {
                 for(final Map.Entry<String, JsonValue> innerEntry : val.asJsonObject().entrySet()) {
-                    final Configuration cfg = new Configuration();
+                    final ConfigurationDescription cfg = new ConfigurationDescription();
                     cfg.fromJSONObject(innerEntry.getValue().asJsonObject());
-                    this.getConfigurations().put(innerEntry.getKey(), cfg);
+                    this.getConfigurationDescriptions().put(innerEntry.getKey(), cfg);
                 }
             }
             
             val = this.getAttributes().remove(InternalConstants.KEY_FACTORIES);
             if ( val != null ) {
                 for(final Map.Entry<String, JsonValue> innerEntry : val.asJsonObject().entrySet()) {
-                    final FactoryConfiguration cfg = new FactoryConfiguration();
+                    final FactoryConfigurationDescription cfg = new FactoryConfigurationDescription();
                     cfg.fromJSONObject(innerEntry.getValue().asJsonObject());
-                    this.getFactories().put(innerEntry.getKey(), cfg);
+                    this.getFactoryConfigurationDescriptions().put(innerEntry.getKey(), cfg);
                 }
             }
 
             val = this.getAttributes().remove(InternalConstants.KEY_FWK_PROPERTIES);
             if ( val != null ) {
                 for(final Map.Entry<String, JsonValue> innerEntry : val.asJsonObject().entrySet()) {
-                    final FrameworkProperty cfg = new FrameworkProperty();
+                    final FrameworkPropertyDescription cfg = new FrameworkPropertyDescription();
                     cfg.fromJSONObject(innerEntry.getValue().asJsonObject());
-                    this.getFrameworkProperties().put(innerEntry.getKey(), cfg);
+                    this.getFrameworkPropertyDescriptions().put(innerEntry.getKey(), cfg);
                 }
             }
 
@@ -156,7 +156,7 @@ public class ConfigurationApi extends AttributeableEntity {
             val = this.getAttributes().remove(InternalConstants.KEY_INTERNAL_FACTORIES);
             if ( val != null ) {
                 for(final JsonValue innerVal : val.asJsonArray()) {
-                    this.getInternalFactories().add(getString(innerVal));
+                    this.getInternalFactoryConfigurations().add(getString(innerVal));
                 }
             }
 
@@ -173,18 +173,18 @@ public class ConfigurationApi extends AttributeableEntity {
     }
 
     /**
-     * Get the configurations
-	 * @return the configurations
+     * Get the configuration descriptions
+	 * @return the configuration descriptions
 	 */
-	public Map<String, Configuration> getConfigurations() {
+	public Map<String, ConfigurationDescription> getConfigurationDescriptions() {
 		return configurations;
 	}
 
 	/**
-     * Get the factory configurations
+     * Get the factory configuration descriptions
 	 * @return the factories
 	 */
-	public Map<String, FactoryConfiguration> getFactories() {
+	public Map<String, FactoryConfigurationDescription> getFactoryConfigurationDescriptions() {
 		return factories;
 	}
 
@@ -192,7 +192,7 @@ public class ConfigurationApi extends AttributeableEntity {
      * Get the framework properties
 	 * @return the frameworkProperties
 	 */
-	public Map<String, FrameworkProperty> getFrameworkProperties() {
+	public Map<String, FrameworkPropertyDescription> getFrameworkPropertyDescriptions() {
 		return frameworkProperties;
 	}
 
@@ -208,7 +208,7 @@ public class ConfigurationApi extends AttributeableEntity {
      * Get the internal factory names
 	 * @return the internalFactories
 	 */
-	public List<String> getInternalFactories() {
+	public List<String> getInternalFactoryConfigurations() {
 		return internalFactories;
 	}
 
@@ -228,23 +228,23 @@ public class ConfigurationApi extends AttributeableEntity {
      */
     JsonObjectBuilder createJson() throws IOException {
 		final JsonObjectBuilder objBuilder = super.createJson();
-        if ( !this.getConfigurations().isEmpty() ) {
+        if ( !this.getConfigurationDescriptions().isEmpty() ) {
             final JsonObjectBuilder propBuilder = Json.createObjectBuilder();
-            for(final Map.Entry<String, Configuration> entry : this.getConfigurations().entrySet()) {
+            for(final Map.Entry<String, ConfigurationDescription> entry : this.getConfigurationDescriptions().entrySet()) {
                 propBuilder.add(entry.getKey(), entry.getValue().createJson());
             }
             objBuilder.add(InternalConstants.KEY_CONFIGURATIONS, propBuilder);
         }
-        if ( !this.getFactories().isEmpty() ) {
+        if ( !this.getFactoryConfigurationDescriptions().isEmpty() ) {
             final JsonObjectBuilder propBuilder = Json.createObjectBuilder();
-            for(final Map.Entry<String, FactoryConfiguration> entry : this.getFactories().entrySet()) {
+            for(final Map.Entry<String, FactoryConfigurationDescription> entry : this.getFactoryConfigurationDescriptions().entrySet()) {
                 propBuilder.add(entry.getKey(), entry.getValue().createJson());
             }
             objBuilder.add(InternalConstants.KEY_FACTORIES, propBuilder);
         }
-        if ( !this.getFrameworkProperties().isEmpty() ) {
+        if ( !this.getFrameworkPropertyDescriptions().isEmpty() ) {
             final JsonObjectBuilder propBuilder = Json.createObjectBuilder();
-            for(final Map.Entry<String, FrameworkProperty> entry : this.getFrameworkProperties().entrySet()) {
+            for(final Map.Entry<String, FrameworkPropertyDescription> entry : this.getFrameworkPropertyDescriptions().entrySet()) {
                 propBuilder.add(entry.getKey(), entry.getValue().createJson());
             }
             objBuilder.add(InternalConstants.KEY_FWK_PROPERTIES, propBuilder);
@@ -256,9 +256,9 @@ public class ConfigurationApi extends AttributeableEntity {
             }
 			objBuilder.add(InternalConstants.KEY_INTERNAL_CONFIGURATIONS, arrayBuilder);
 		}
-		if ( !this.getInternalFactories().isEmpty() ) {
+		if ( !this.getInternalFactoryConfigurations().isEmpty() ) {
             final JsonArrayBuilder arrayBuilder = Json.createArrayBuilder();
-            for(final String n : this.getInternalFactories()) {
+            for(final String n : this.getInternalFactoryConfigurations()) {
                 arrayBuilder.add(n);
             }
 			objBuilder.add(InternalConstants.KEY_INTERNAL_FACTORIES, arrayBuilder);
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationDescription.java
similarity index 92%
copy from src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java
copy to src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationDescription.java
index 5f95df2..e9184f2 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationDescription.java
@@ -16,6 +16,6 @@
  */
 package org.apache.sling.feature.extension.apiregions.api.config;
 
-public class Configuration extends ConfigurableEntity {
+public class ConfigurationDescription extends ConfigurableEntity {
 
 }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfiguration.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescription.java
similarity index 98%
rename from src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfiguration.java
rename to src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescription.java
index ea41555..4991d2e 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfiguration.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescription.java
@@ -29,7 +29,7 @@ import javax.json.JsonObject;
 import javax.json.JsonObjectBuilder;
 import javax.json.JsonValue;
 
-public class FactoryConfiguration extends ConfigurableEntity {
+public class FactoryConfigurationDescription extends ConfigurableEntity {
     
     private final Set<Operation> operations = new HashSet<>();
 
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkProperty.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkPropertyDescription.java
similarity index 92%
copy from src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkProperty.java
copy to src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkPropertyDescription.java
index 320448b..3e1e75f 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkProperty.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkPropertyDescription.java
@@ -19,6 +19,6 @@ package org.apache.sling.feature.extension.apiregions.api.config;
 /**
  * A framework property
  */
-public class FrameworkProperty extends Property {
+public class FrameworkPropertyDescription extends PropertyDescription {
     
 }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/InternalConstants.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/InternalConstants.java
index 5ba22ec..65922cd 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/InternalConstants.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/InternalConstants.java
@@ -31,13 +31,13 @@ abstract class InternalConstants {
 
     static final String KEY_CONFIGURATIONS = "configurations";
 
-    static final String KEY_FACTORIES = "factories";
+    static final String KEY_FACTORIES = "factory-configurations";
     
     static final String KEY_FWK_PROPERTIES = "framework-properties";
 
     static final String KEY_INTERNAL_CONFIGURATIONS = "internal-configurations";
 
-    static final String KEY_INTERNAL_FACTORIES = "internal-factories";
+    static final String KEY_INTERNAL_FACTORIES = "internal-factory-configurations";
     
     static final String KEY_INTERNAL_FWK_PROPERTIES = "internal-framework-properties";
 	
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Property.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
similarity index 98%
rename from src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Property.java
rename to src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
index bebccfd..3ae8a5c 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Property.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
@@ -31,7 +31,7 @@ import javax.json.JsonValue;
 /**
  * Instances of this class represent a single configuration property
  */
-public class Property extends DescribableEntity {
+public class PropertyDescription extends DescribableEntity {
 	
 	/** The property type */
 	private PropertyType type;
@@ -60,7 +60,7 @@ public class Property extends DescribableEntity {
 	/** Required? */
 	private boolean required;
     
-    public Property() {
+    public PropertyDescription() {
         this.setDefaults();
     }
 
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkProperty.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
similarity index 58%
rename from src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkProperty.java
rename to src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
index 320448b..4adf24d 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkProperty.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
@@ -14,11 +14,28 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.sling.feature.extension.apiregions.api.config;
+package org.apache.sling.feature.extension.apiregions.api.config.validation;
 
-/**
- * A framework property
- */
-public class FrameworkProperty extends Property {
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class ConfigurationValidationResult {
+
+    private final Map<String, PropertyValidationResult> propertyErrors = new HashMap<>();
+
+    private final List<String> globalErrors = new ArrayList<>();
+    
+    public boolean isValid() {
+        return propertyErrors.isEmpty() && globalErrors.isEmpty();
+    }
+
+    public List<String> getGlobalErrors() {
+        return this.globalErrors;
+    }
     
-}
+    public Map<String, PropertyValidationResult> getPropertyErrors() {
+        return propertyErrors;
+    }
+}
\ No newline at end of file
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java
new file mode 100644
index 0000000..ab5aa6b
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java
@@ -0,0 +1,97 @@
+/*
+ * 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 java.util.Arrays;
+import java.util.Dictionary;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.sling.feature.Configuration;
+import org.apache.sling.feature.extension.apiregions.api.config.ConfigurableEntity;
+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.osgi.framework.Constants;
+
+/**
+ * Validate values
+ */
+public class ConfigurationValidator {
+    
+    public static final List<String> ALLOWED_PROPERTIES = Arrays.asList(Constants.SERVICE_DESCRIPTION,
+        Constants.SERVICE_VENDOR,
+        Constants.SERVICE_RANKING);
+
+
+    private final PropertyValidator propertyValidator = new PropertyValidator();
+
+    /**
+     * Validate a configuration
+     */
+    public ConfigurationValidationResult validate(final ConfigurableEntity desc, final Configuration config) {
+        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.getPropertyErrors());
+                }
+            }
+        } else {
+            if ( !(desc instanceof ConfigurationDescription) ) {
+                result.getGlobalErrors().add("Configuration cannot be validated against factory configuration description");
+            } else {
+                validateProperties(desc, config, result.getPropertyErrors());
+            }
+        }
+
+        return result;
+    }
+
+    void validateProperties(final ConfigurableEntity desc, 
+            final Configuration configuration, 
+            final Map<String, PropertyValidationResult> errors) {
+        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);
+            if ( !result.isValid()) {
+                errors.put(propEntry.getKey(), result);
+            }
+        }
+        final Enumeration<String> keyEnum = properties.keys();
+        while ( keyEnum.hasMoreElements() ) {
+            final String propName = keyEnum.nextElement();
+            if ( !desc.getPropertyDescriptions().containsKey(propName) && !ALLOWED_PROPERTIES.contains(propName)) {
+                errors.computeIfAbsent(propName, key -> new PropertyValidationResult()).getErrors().add("Property is not allowed");
+            } else if ( Constants.SERVICE_RANKING.equals(propName) ) {
+                final Object value = properties.get(propName);
+                if ( !(value instanceof Integer) ) {
+                    errors.computeIfAbsent(propName, key -> new PropertyValidationResult()).getErrors().add("service.ranking must be of type Integer");
+                }
+            }
+        }
+    }
+}
\ No newline at end of file
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java
similarity index 67%
copy from src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java
copy to src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java
index 5f95df2..81b03c4 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java
@@ -14,8 +14,20 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.sling.feature.extension.apiregions.api.config;
+package org.apache.sling.feature.extension.apiregions.api.config.validation;
 
-public class Configuration extends ConfigurableEntity {
+import java.util.HashMap;
+import java.util.Map;
 
-}
+public class FeatureValidationResult {
+
+    private final Map<String, ConfigurationValidationResult> configurationErrors = new HashMap<>();
+
+    public boolean isValid() {
+        return configurationErrors.isEmpty();
+    }
+
+    public Map<String, ConfigurationValidationResult> getConfigurationErrors() {
+        return this.configurationErrors;
+    }
+}
\ 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
new file mode 100644
index 0000000..229587b
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidator.java
@@ -0,0 +1,70 @@
+/*
+ * 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 org.apache.sling.feature.Configuration;
+import org.apache.sling.feature.Feature;
+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;
+
+/**
+ * Validator to validate a feature
+ */
+public class FeatureValidator {
+    
+    private final ConfigurationValidator configurationValidator = new ConfigurationValidator();
+
+    /**
+     * Validate the feature against the configuration API
+     * @param api The configuration API
+     * @param feature The feature
+     * @return A {@code FeatureValidationResult}
+     */
+    public FeatureValidationResult validate(final ConfigurationApi api, final Feature feature) {
+        final FeatureValidationResult result = new FeatureValidationResult();
+
+        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);
+                    if ( !r.isValid()) {
+                        result.getConfigurationErrors().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.getConfigurationErrors().put(config.getPid(), cvr);
+                }
+            } else {
+                final ConfigurationDescription desc = api.getConfigurationDescriptions().get(config.getPid());
+                if ( desc != null ) {
+                    final ConfigurationValidationResult r = configurationValidator.validate(desc, config);
+                    if ( !r.isValid()) {
+                        result.getConfigurationErrors().put(config.getPid(), r);
+                    }
+                } else if ( api.getInternalConfigurations().contains(config.getPid())) {
+                    final ConfigurationValidationResult cvr = new ConfigurationValidationResult();
+                    cvr.getGlobalErrors().add("Configuration is not allowed");
+                    result.getConfigurationErrors().put(config.getPid(), cvr);
+                }
+            }
+        }
+        return result;
+    }
+}
\ No newline at end of file
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidationResult.java
similarity index 59%
rename from src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java
rename to src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidationResult.java
index 5f95df2..a048cc8 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Configuration.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidationResult.java
@@ -14,8 +14,29 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.sling.feature.extension.apiregions.api.config;
+package org.apache.sling.feature.extension.apiregions.api.config.validation;
 
-public class Configuration extends ConfigurableEntity {
+import java.util.ArrayList;
+import java.util.List;
 
-}
+public class PropertyValidationResult {
+
+    private final List<String> errors = new ArrayList<>();
+
+    /**
+     * Is the property value valid?
+     * @return {@code true} if the value is valid
+     */
+	public boolean isValid() {
+        return errors.isEmpty();
+    }
+
+    /**
+     * If {@link #isValid()} returns {@code false} this returns
+     * a list of human readable errors.
+     * @return A list of errors - empty if {@link #isValid()} returns {@code true}
+     */
+	public List<String> getErrors() {
+        return errors;
+    }
+}
\ No newline at end of file
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Validator.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidator.java
similarity index 83%
rename from src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Validator.java
rename to src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidator.java
index 1a21b21..b609cc2 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Validator.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidator.java
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.sling.feature.extension.apiregions.api.config;
+package org.apache.sling.feature.extension.apiregions.api.config.validation;
 
 import java.lang.reflect.Array;
 import java.net.MalformedURLException;
@@ -23,26 +23,24 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
+import org.apache.sling.feature.extension.apiregions.api.config.Option;
+import org.apache.sling.feature.extension.apiregions.api.config.PropertyDescription;
+
 /**
  * Validate values
  */
-public class Validator {
-	
-	public interface ValidationResult {
-
-		boolean isValid();
-
-		List<String> getErrors();
-	}
-
+public class PropertyValidator {
+    
 	/**
 	 * Validate the value against the property definition
-	 * @return {@code null} if the value is valid, a human readable validation error message otherwise
+	 * @return A property validation result
 	 */
-	public static ValidationResult validate(final Property prop, final Object value) {
-		final List<String> messages = new ArrayList<>();
+	public PropertyValidationResult validate(final PropertyDescription prop, final Object value) {
+		final PropertyValidationResult result = new PropertyValidationResult();
 		if ( value == null ) {
-			messages.add("No value provided");
+            if ( prop.isRequired() ) {
+                result.getErrors().add("No value provided");
+            }
 		} else {
 			final List<Object> values;
 			if ( value.getClass().isArray() ) {
@@ -61,30 +59,21 @@ public class Validator {
 			} else {
 				// single value
 				values = null;
-				validateValue(prop, value, messages);
+				validateValue(prop, value, result.getErrors());
 			}
 
 			if ( values != null ) {
                 // array or collection
                 for(final Object val : values) {
-                    validateValue(prop, val, messages);
+                    validateValue(prop, val, result.getErrors());
                 }
-                validateList(prop, values, messages);
+                validateList(prop, values, result.getErrors());
 			}
 		}
-		return new ValidationResult(){
-			
-			public boolean isValid() {
-				return messages.isEmpty();
-			}
-
-			public List<String> getErrors() {
-				return messages;
-			}	
-		};
+		return result;
 	}
 
-    static void validateList(final Property prop, final List<Object> values, final List<String> messages) {
+    void validateList(final PropertyDescription prop, final List<Object> values, final List<String> messages) {
         if ( prop.getCardinality() > 0 && values.size() > prop.getCardinality() ) {
             messages.add("Array/collection contains too many elements, only " + prop.getCardinality() + " allowed");
         }
@@ -118,7 +107,7 @@ public class Validator {
         }
     }
 
-	static void validateValue(final Property prop, final Object value, final List<String> messages) {
+	void validateValue(final PropertyDescription prop, final Object value, final List<String> messages) {
 		if ( value != null ) {
 			switch ( prop.getType() ) {
 				case BOOLEAN : validateBoolean(prop, value, messages);
@@ -154,7 +143,7 @@ public class Validator {
 		}
 	}
 	
-	static void validateBoolean(final Property prop, final Object value, final List<String> messages) {
+	void validateBoolean(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( ! (value instanceof Boolean) ) {
 			if ( value instanceof String ) {
 				final String v = (String)value;
@@ -167,7 +156,7 @@ public class Validator {
 		}
 	}
 
-	static void validateByte(final Property prop, final Object value, final List<String> messages) {
+	void validateByte(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( ! (value instanceof Byte) ) {
 			if ( value instanceof String ) {
 				final String v = (String)value;
@@ -186,7 +175,7 @@ public class Validator {
 		}
 	}
 
-	static void validateShort(final Property prop, final Object value, final List<String> messages) {
+	void validateShort(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( ! (value instanceof Short) ) {
 			if ( value instanceof String ) {
 				final String v = (String)value;
@@ -205,7 +194,7 @@ public class Validator {
 		}
 	}
 
-	static void validateInteger(final Property prop, final Object value, final List<String> messages) {
+	void validateInteger(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( ! (value instanceof Integer) ) {
 			if ( value instanceof String ) {
 				final String v = (String)value;
@@ -224,7 +213,7 @@ public class Validator {
 		}
 	}
 
-	static void validateLong(final Property prop, final Object value, final List<String> messages) {
+	void validateLong(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( ! (value instanceof Long) ) {
 			if ( value instanceof String ) {
 				final String v = (String)value;
@@ -243,7 +232,7 @@ public class Validator {
 		}
 	}
 
-	static void validateFloat(final Property prop, final Object value, final List<String> messages) {
+	void validateFloat(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( ! (value instanceof Float) ) {
 			if ( value instanceof String ) {
 				final String v = (String)value;
@@ -262,7 +251,7 @@ public class Validator {
 		}
 	}
 
-	static void validateDouble(final Property prop, final Object value, final List<String> messages) {
+	void validateDouble(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( ! (value instanceof Double) ) {
 			if ( value instanceof String ) {
 				final String v = (String)value;
@@ -281,7 +270,7 @@ public class Validator {
 		}
 	}
 
-	static void validateCharacter(final Property prop, final Object value, final List<String> messages) {
+	void validateCharacter(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( ! (value instanceof Character) ) {
 			if ( value instanceof String ) {
 				final String v = (String)value;
@@ -294,7 +283,7 @@ public class Validator {
 		}
 	}
 
-	static void validateURL(final Property prop, final Object value, final List<String> messages) {
+	void validateURL(final PropertyDescription prop, final Object value, final List<String> messages) {
 		final String val = value.toString();
 		try {
 			new URL(val);
@@ -303,7 +292,7 @@ public class Validator {
 		}
 	}
 
-	static void validateEmail(final Property prop, final Object value, final List<String> messages) {
+	void validateEmail(final PropertyDescription prop, final Object value, final List<String> messages) {
 		final String val = value.toString();
 		// poor man's validation (should probably use InternetAddress)
 		if ( !val.contains("@") ) {
@@ -311,13 +300,13 @@ public class Validator {
 		}
 	}
 
-	static void validatePassword(final Property prop, final Object value, final List<String> messages) {
+	void validatePassword(final PropertyDescription prop, final Object value, final List<String> messages) {
 		if ( prop.getVariable() == null ) {
 			messages.add("Value for a password must use a variable");
 		}
 	}
 
-	static void validateRange(final Property prop, final Number value, final List<String> messages) {
+	void validateRange(final PropertyDescription prop, final Number value, final List<String> messages) {
 	    if ( prop.getRange() != null ) {
             if ( prop.getRange().getMin() != null ) {
                 if ( value instanceof Float || value instanceof Double ) {
@@ -348,7 +337,7 @@ public class Validator {
 		}
 	}
 
-    static void validateRegex(final Property prop, final Object value, final List<String> messages) {
+    void validateRegex(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( prop.getRegexPattern() != null ) {
             if ( !prop.getRegexPattern().matcher(value.toString()).matches() ) {
                 messages.add("Value " + value + " does not match regex " + prop.getRegex());
@@ -356,7 +345,7 @@ public class Validator {
         }
     }
 
-    static void validateOptions(final Property prop, final Object value, final List<String> messages) {
+    void validateOptions(final PropertyDescription prop, final Object value, final List<String> messages) {
         if ( prop.getOptions() != null ) {
             boolean found = false;
             for(final Option opt : prop.getOptions()) {
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/package-info.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/package-info.java
new file mode 100644
index 0000000..247f925
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/package-info.java
@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+@org.osgi.annotation.versioning.Version("1.0.0")
+package org.apache.sling.feature.extension.apiregions.api.config.validation;
+
+
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntityTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntityTest.java
index 79efb74..caf06d4 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntityTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntityTest.java
@@ -42,13 +42,13 @@ public class ConfigurableEntityTest {
         entity.setDeprecated("d");
         entity.setTitle("t");
         entity.setDescription("x");
-        entity.getProperties().put("a", new Property());
+        entity.getPropertyDescriptions().put("a", new PropertyDescription());
         entity.clear();
         assertTrue(entity.getAttributes().isEmpty());
         assertNull(entity.getDeprecated());
         assertNull(entity.getTitle());
         assertNull(entity.getDescription());
-        assertTrue(entity.getProperties().isEmpty());
+        assertTrue(entity.getPropertyDescriptions().isEmpty());
     }
 
     @Test public void testFromJSONObject() throws IOException {
@@ -57,15 +57,15 @@ public class ConfigurableEntityTest {
 
         final CE entity = new CE();
         entity.fromJSONObject(ext.getJSONStructure().asJsonObject());
-        assertEquals(2, entity.getProperties().size());
-        assertNotNull(entity.getProperties().get("a"));
-        assertNotNull(entity.getProperties().get("b"));
+        assertEquals(2, entity.getPropertyDescriptions().size());
+        assertNotNull(entity.getPropertyDescriptions().get("a"));
+        assertNotNull(entity.getPropertyDescriptions().get("b"));
     }
 
     @Test public void testToJSONObject() throws IOException {
         final CE entity = new CE();
-        entity.getProperties().put("a", new Property());
-        entity.getProperties().put("b", new Property());
+        entity.getPropertyDescriptions().put("a", new PropertyDescription());
+        entity.getPropertyDescriptions().put("b", new PropertyDescription());
 
         final Extension ext = new Extension(ExtensionType.JSON, "a", ExtensionState.OPTIONAL);
         ext.setJSON("{ \"properties\" : { \"a\" : {\"type\":\"STRING\"}, \"b\" : {\"type\":\"STRING\"}}}");
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationApiTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationApiTest.java
index cdb2f94..8568c24 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationApiTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationApiTest.java
@@ -54,63 +54,63 @@ public class ConfigurationApiTest {
     @Test public void testClear() {
         final ConfigurationApi entity = new ConfigurationApi();
         entity.getAttributes().put("a", Json.createValue(5));
-        entity.getConfigurations().put("pid", new Configuration());
-        entity.getFactories().put("factory", new FactoryConfiguration());
-        entity.getFrameworkProperties().put("prop", new FrameworkProperty());
+        entity.getConfigurationDescriptions().put("pid", new ConfigurationDescription());
+        entity.getFactoryConfigurationDescriptions().put("factory", new FactoryConfigurationDescription());
+        entity.getFrameworkPropertyDescriptions().put("prop", new FrameworkPropertyDescription());
         entity.getInternalConfigurations().add("ipid");
-        entity.getInternalFactories().add("ifactory");
+        entity.getInternalFactoryConfigurations().add("ifactory");
         entity.getInternalFrameworkProperties().add("iprop");
         entity.clear();
         assertTrue(entity.getAttributes().isEmpty());
-        assertTrue(entity.getConfigurations().isEmpty());
-        assertTrue(entity.getFactories().isEmpty());
-        assertTrue(entity.getFrameworkProperties().isEmpty());
+        assertTrue(entity.getConfigurationDescriptions().isEmpty());
+        assertTrue(entity.getFactoryConfigurationDescriptions().isEmpty());
+        assertTrue(entity.getFrameworkPropertyDescriptions().isEmpty());
         assertTrue(entity.getInternalConfigurations().isEmpty());
-        assertTrue(entity.getInternalFactories().isEmpty());
+        assertTrue(entity.getInternalFactoryConfigurations().isEmpty());
         assertTrue(entity.getInternalFrameworkProperties().isEmpty());
     }
 
     @Test public void testFromJSONObject() throws IOException {
         final Extension ext = new Extension(ExtensionType.JSON, "a", ExtensionState.OPTIONAL);
         ext.setJSON("{ \"a\" : 5, \"configurations\" : { \"pid\": {}}, " +
-            "\"factories\" : { \"factory\" : {}}," +
+            "\"factory-configurations\" : { \"factory\" : {}}," +
             "\"framework-properties\" : { \"prop\" : { \"type\" : \"STRING\"}}," +
             "\"internal-configurations\" : [\"ipid\"],"+
-            "\"internal-factories\" : [\"ifactory\"],"+
+            "\"internal-factory-configurations\" : [\"ifactory\"],"+
             "\"internal-framework-properties\" : [\"iprop\"]}");
 
         final ConfigurationApi entity = new ConfigurationApi();
         entity.fromJSONObject(ext.getJSONStructure().asJsonObject());
-        assertEquals(1, entity.getConfigurations().size());
-        assertEquals(1, entity.getFactories().size());
-        assertEquals(1, entity.getFrameworkProperties().size());
+        assertEquals(1, entity.getConfigurationDescriptions().size());
+        assertEquals(1, entity.getFactoryConfigurationDescriptions().size());
+        assertEquals(1, entity.getFrameworkPropertyDescriptions().size());
         assertEquals(1, entity.getInternalConfigurations().size());
-        assertEquals(1, entity.getInternalFactories().size());
+        assertEquals(1, entity.getInternalFactoryConfigurations().size());
         assertEquals(1, entity.getInternalFrameworkProperties().size());
-        assertTrue(entity.getConfigurations().containsKey("pid"));
-        assertTrue(entity.getFactories().containsKey("factory"));
-        assertTrue(entity.getFrameworkProperties().containsKey("prop"));
+        assertTrue(entity.getConfigurationDescriptions().containsKey("pid"));
+        assertTrue(entity.getFactoryConfigurationDescriptions().containsKey("factory"));
+        assertTrue(entity.getFrameworkPropertyDescriptions().containsKey("prop"));
         assertTrue(entity.getInternalConfigurations().contains("ipid"));
-        assertTrue(entity.getInternalFactories().contains("ifactory"));
+        assertTrue(entity.getInternalFactoryConfigurations().contains("ifactory"));
         assertTrue(entity.getInternalFrameworkProperties().contains("iprop"));
     }
 
     @Test public void testToJSONObject() throws IOException {
         final ConfigurationApi entity = new ConfigurationApi();
         entity.getAttributes().put("a", Json.createValue(5));
-        entity.getConfigurations().put("pid", new Configuration());
-        entity.getFactories().put("factory", new FactoryConfiguration());
-        entity.getFrameworkProperties().put("prop", new FrameworkProperty());
+        entity.getConfigurationDescriptions().put("pid", new ConfigurationDescription());
+        entity.getFactoryConfigurationDescriptions().put("factory", new FactoryConfigurationDescription());
+        entity.getFrameworkPropertyDescriptions().put("prop", new FrameworkPropertyDescription());
         entity.getInternalConfigurations().add("ipid");
-        entity.getInternalFactories().add("ifactory");
+        entity.getInternalFactoryConfigurations().add("ifactory");
         entity.getInternalFrameworkProperties().add("iprop");
 
         final Extension ext = new Extension(ExtensionType.JSON, "a", ExtensionState.OPTIONAL);
         ext.setJSON("{ \"a\" : 5, \"configurations\" : { \"pid\": {}}, " +
-            "\"factories\" : { \"factory\" : {}}," +
+            "\"factory-configurations\" : { \"factory\" : {}}," +
             "\"framework-properties\" : { \"prop\" : { \"type\" : \"STRING\"}}," +
             "\"internal-configurations\" : [\"ipid\"],"+
-            "\"internal-factories\" : [\"ifactory\"],"+
+            "\"internal-factory-configurations\" : [\"ifactory\"],"+
             "\"internal-framework-properties\" : [\"iprop\"]}");
 
         assertEquals(ext.getJSONStructure().asJsonObject(), entity.toJSONObject());
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescriptionTest.java
similarity index 86%
rename from src/test/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationTest.java
rename to src/test/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescriptionTest.java
index 37f09bf..fb0f2b6 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescriptionTest.java
@@ -29,15 +29,15 @@ import org.apache.sling.feature.ExtensionState;
 import org.apache.sling.feature.ExtensionType;
 import org.junit.Test;
 
-public class FactoryConfigurationTest {
+public class FactoryConfigurationDescriptionTest {
 
     @Test public void testClear() {
-        final FactoryConfiguration entity = new FactoryConfiguration();
+        final FactoryConfigurationDescription entity = new FactoryConfigurationDescription();
         entity.getAttributes().put("a", Json.createValue(5));
         entity.setDeprecated("d");
         entity.setTitle("t");
         entity.setDescription("x");
-        entity.getProperties().put("a", new Property());
+        entity.getPropertyDescriptions().put("a", new PropertyDescription());
         entity.getOperations().add(Operation.CREATE);
         entity.getInternalNames().add("internal");
         entity.clear();
@@ -45,7 +45,7 @@ public class FactoryConfigurationTest {
         assertNull(entity.getDeprecated());
         assertNull(entity.getTitle());
         assertNull(entity.getDescription());
-        assertTrue(entity.getProperties().isEmpty());
+        assertTrue(entity.getPropertyDescriptions().isEmpty());
         assertTrue(entity.getOperations().isEmpty());
         assertTrue(entity.getInternalNames().isEmpty());
     }
@@ -54,7 +54,7 @@ public class FactoryConfigurationTest {
         final Extension ext = new Extension(ExtensionType.JSON, "a", ExtensionState.OPTIONAL);
         ext.setJSON("{ \"internal-names\" : [ \"a\", \"b\"], \"operations\" : [\"create\"]}");
 
-        final FactoryConfiguration entity = new FactoryConfiguration();
+        final FactoryConfigurationDescription entity = new FactoryConfigurationDescription();
         entity.fromJSONObject(ext.getJSONStructure().asJsonObject());
         assertEquals(2, entity.getInternalNames().size());
         assertTrue(entity.getInternalNames().contains("a"));
@@ -64,7 +64,7 @@ public class FactoryConfigurationTest {
     }
 
     @Test public void testToJSONObject() throws IOException {
-        final FactoryConfiguration entity = new FactoryConfiguration();
+        final FactoryConfigurationDescription entity = new FactoryConfigurationDescription();
         entity.getInternalNames().add("a");
         entity.getInternalNames().add("b");
         entity.getOperations().add(Operation.UPDATE);
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyTest.java
index b007391..e895023 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyTest.java
@@ -36,7 +36,7 @@ import org.junit.Test;
 public class PropertyTest {
 
     @Test public void testClear() {
-        final Property entity = new Property();
+        final PropertyDescription entity = new PropertyDescription();
         entity.getAttributes().put("a", Json.createValue(5));
         entity.setDeprecated("d");
         entity.setTitle("t");
@@ -49,7 +49,7 @@ public class PropertyTest {
         entity.setRegex(".");
         entity.setRequired(true);
         entity.setVariable("var");
-        entity.setType(PropertyType.BYTE);
+        entity.setType(PropertyType.BYTE);        
         entity.clear();
         assertTrue(entity.getAttributes().isEmpty());
         assertNull(entity.getDeprecated());
@@ -72,7 +72,7 @@ public class PropertyTest {
         ext.setJSON("{ \"type\" : \"BYTE\", \"cardinality\": 5, \"required\" : true, \"variable\" : \"var\"," +
         "\"range\" : {}, \"includes\" : [\"in\"], \"excludes\" : [\"ex\"] , \"options\": [{}], \"regex\": \".\"}");
 
-        final Property entity = new Property();
+        final PropertyDescription entity = new PropertyDescription();
         entity.fromJSONObject(ext.getJSONStructure().asJsonObject());
 
         assertEquals(5, entity.getCardinality());
@@ -103,7 +103,7 @@ public class PropertyTest {
    }
 
     @Test public void testToJSONObject() throws IOException {
-        final Property entity = new Property();
+        final PropertyDescription entity = new PropertyDescription();
         entity.setCardinality(5);
         entity.setExcludes(new String[] {"ex"});
         entity.setIncludes(new String[] {"in"});
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ValidatorTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidatorTest.java
similarity index 56%
rename from src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ValidatorTest.java
rename to src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidatorTest.java
index 1bfff16..4ce08aa 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/ValidatorTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/validation/PropertyValidatorTest.java
@@ -14,185 +14,198 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.sling.feature.extension.apiregions.api.config;
+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.assertTrue;
 
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.sling.feature.extension.apiregions.api.config.Option;
+import org.apache.sling.feature.extension.apiregions.api.config.PropertyDescription;
+import org.apache.sling.feature.extension.apiregions.api.config.PropertyType;
+import org.apache.sling.feature.extension.apiregions.api.config.Range;
 import org.junit.Test;
 
-public class ValidatorTest {
+public class PropertyValidatorTest {
     
-    @Test public void testValidateValueWithNull() {
-        final List<String> messages = new ArrayList<>();
-        Validator.validateValue(null, null, messages);
-        assertEquals(1, messages.size());
-        messages.clear();
+    private final PropertyValidator validator = new PropertyValidator();
+    
+    @Test public void testValidateWithNull() {
+        final PropertyDescription prop = new PropertyDescription();
+
+        // prop not required - no error
+        assertTrue(validator.validate(prop, null).getErrors().isEmpty());
+        assertTrue(validator.validate(prop, null).isValid());
+
+        // prop required - error
+        prop.setRequired(true);
+        assertEquals(1, validator.validate(prop, null).getErrors().size());
+        assertFalse(validator.validate(prop, null).isValid());
     }
 
     @Test public void testValidateBoolean() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         prop.setType(PropertyType.BOOLEAN);
 
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateBoolean(prop, Boolean.TRUE, messages);
+        validator.validateBoolean(prop, Boolean.TRUE, messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateBoolean(prop, Boolean.FALSE, messages);
+        validator.validateBoolean(prop, Boolean.FALSE, messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateBoolean(prop, "TRUE", messages);
+        validator.validateBoolean(prop, "TRUE", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateBoolean(prop, "FALSE", messages);
+        validator.validateBoolean(prop, "FALSE", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateBoolean(prop, "yes", messages);
+        validator.validateBoolean(prop, "yes", messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateBoolean(prop, 1, messages);
+        validator.validateBoolean(prop, 1, messages);
         assertEquals(1, messages.size());
         messages.clear();
     }
 
     @Test public void testValidateByte() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         prop.setType(PropertyType.BYTE);
 
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateByte(prop, (byte)1, messages);
+        validator.validateByte(prop, (byte)1, messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateByte(prop, "1", messages);
+        validator.validateByte(prop, "1", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateByte(prop, "yes", messages);
+        validator.validateByte(prop, "yes", messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateByte(prop, 1, messages);
+        validator.validateByte(prop, 1, messages);
         assertTrue(messages.isEmpty());
     }
 
     @Test public void testValidateShort() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         prop.setType(PropertyType.SHORT);
 
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateShort(prop, (short)1, messages);
+        validator.validateShort(prop, (short)1, messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateShort(prop, "1", messages);
+        validator.validateShort(prop, "1", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateShort(prop, "yes", messages);
+        validator.validateShort(prop, "yes", messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateShort(prop, 1, messages);
+        validator.validateShort(prop, 1, messages);
         assertTrue(messages.isEmpty());
     }
 
     @Test public void testValidateInteger() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         prop.setType(PropertyType.INTEGER);
 
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateInteger(prop, 1, messages);
+        validator.validateInteger(prop, 1, messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateInteger(prop, "1", messages);
+        validator.validateInteger(prop, "1", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateInteger(prop, "yes", messages);
+        validator.validateInteger(prop, "yes", messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateInteger(prop, 1, messages);
+        validator.validateInteger(prop, 1, messages);
         assertTrue(messages.isEmpty());
     }
 
     @Test public void testValidateLong() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         prop.setType(PropertyType.LONG);
 
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateLong(prop, 1L, messages);
+        validator.validateLong(prop, 1L, messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateLong(prop, "1", messages);
+        validator.validateLong(prop, "1", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateLong(prop, "yes", messages);
+        validator.validateLong(prop, "yes", messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateLong(prop, 1, messages);
+        validator.validateLong(prop, 1, messages);
         assertTrue(messages.isEmpty());
     }
 
     @Test public void testValidateFloat() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         prop.setType(PropertyType.FLOAT);
 
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateFloat(prop, 1.1, messages);
+        validator.validateFloat(prop, 1.1, messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateFloat(prop, "1.1", messages);
+        validator.validateFloat(prop, "1.1", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateFloat(prop, "yes", messages);
+        validator.validateFloat(prop, "yes", messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateFloat(prop, 1, messages);
+        validator.validateFloat(prop, 1, messages);
         assertTrue(messages.isEmpty());
     }
 
     @Test public void testValidateDouble() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         prop.setType(PropertyType.DOUBLE);
 
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateDouble(prop, 1.1d, messages);
+        validator.validateDouble(prop, 1.1d, messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateDouble(prop, "1.1", messages);
+        validator.validateDouble(prop, "1.1", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateDouble(prop, "yes", messages);
+        validator.validateDouble(prop, "yes", messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateDouble(prop, 1, messages);
+        validator.validateDouble(prop, 1, messages);
         assertTrue(messages.isEmpty());
     }
 
     @Test public void testValidateChar() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         prop.setType(PropertyType.CHARACTER);
 
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateCharacter(prop, 'x', messages);
+        validator.validateCharacter(prop, 'x', messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateCharacter(prop, "y", messages);
+        validator.validateCharacter(prop, "y", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateCharacter(prop, "yes", messages);
+        validator.validateCharacter(prop, "yes", messages);
         assertEquals(1, messages.size());
         messages.clear();
     }
@@ -200,10 +213,10 @@ public class ValidatorTest {
     @Test public void testValidateUrl() {
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateURL(null, "https://sling.apache.org/documentation", messages);
+        validator.validateURL(null, "https://sling.apache.org/documentation", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateURL(null, "hello world", messages);
+        validator.validateURL(null, "hello world", messages);
         assertEquals(1, messages.size());
         messages.clear();
     }
@@ -211,102 +224,102 @@ public class ValidatorTest {
     @Test public void testValidateEmail() {
         final List<String> messages = new ArrayList<>();
 
-        Validator.validateEmail(null, "a@b.com", messages);
+        validator.validateEmail(null, "a@b.com", messages);
         assertTrue(messages.isEmpty());
 
-        Validator.validateEmail(null, "hello world", messages);
+        validator.validateEmail(null, "hello world", messages);
         assertEquals(1, messages.size());
         messages.clear();
     }
 
     @Test public void testValidatePassword() {
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
         final List<String> messages = new ArrayList<>();
 
-        Validator.validatePassword(prop, null, messages);
+        validator.validatePassword(prop, null, messages);
         assertEquals(1, messages.size());
         messages.clear();
 
         prop.setVariable("secret");
-        Validator.validatePassword(prop, null, messages);
+        validator.validatePassword(prop, null, messages);
         assertTrue(messages.isEmpty());
     }
 
     @Test public void testValidateRange() {
         final List<String> messages = new ArrayList<>();
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
 
         // no range set
-        Validator.validateRange(prop, 2, messages);
+        validator.validateRange(prop, 2, messages);
         assertTrue(messages.isEmpty());
 
         // empty range set
         prop.setRange(new Range());
-        Validator.validateRange(prop, 2, messages);
+        validator.validateRange(prop, 2, messages);
         assertTrue(messages.isEmpty());
 
         // min set
         prop.getRange().setMin(5);
-        Validator.validateRange(prop, 5, messages);
+        validator.validateRange(prop, 5, messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRange(prop, 6, messages);
+        validator.validateRange(prop, 6, messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRange(prop, 4, messages);
+        validator.validateRange(prop, 4, messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateRange(prop, 5.0, messages);
+        validator.validateRange(prop, 5.0, messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRange(prop, 6.0, messages);
+        validator.validateRange(prop, 6.0, messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRange(prop, 4.0, messages);
+        validator.validateRange(prop, 4.0, messages);
         assertEquals(1, messages.size());
         messages.clear();
 
         // max set
         prop.getRange().setMax(6);
-        Validator.validateRange(prop, 5, messages);
+        validator.validateRange(prop, 5, messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRange(prop, 6, messages);
+        validator.validateRange(prop, 6, messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRange(prop, 7, messages);
+        validator.validateRange(prop, 7, messages);
         assertEquals(1, messages.size());
         messages.clear();
 
-        Validator.validateRange(prop, 5.0, messages);
+        validator.validateRange(prop, 5.0, messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRange(prop, 6.0, messages);
+        validator.validateRange(prop, 6.0, messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRange(prop, 7.0, messages);
+        validator.validateRange(prop, 7.0, messages);
         assertEquals(1, messages.size());
         messages.clear();
     }   
     
     @Test public void testValidateRegex() {
         final List<String> messages = new ArrayList<>();
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
 
         // no regex
-        Validator.validateRegex(prop, "hello world", messages);
-        Validator.validateRegex(prop, "world", messages);
+        validator.validateRegex(prop, "hello world", messages);
+        validator.validateRegex(prop, "world", messages);
         assertTrue(messages.isEmpty());
 
         // regex
         prop.setRegex("h(.*)");
-        Validator.validateRegex(prop, "hello world", messages);
+        validator.validateRegex(prop, "hello world", messages);
         assertTrue(messages.isEmpty());
-        Validator.validateRegex(prop, "world", messages);
+        validator.validateRegex(prop, "world", messages);
         assertEquals(1, messages.size());
         messages.clear();
     }
 
     @Test public void testValidateOptions() {
         final List<String> messages = new ArrayList<>();
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
 
         // no options
-        Validator.validateOptions(prop, "foo", messages);
-        Validator.validateOptions(prop, "bar", messages);
+        validator.validateOptions(prop, "foo", messages);
+        validator.validateOptions(prop, "bar", messages);
         assertTrue(messages.isEmpty());
 
         // options
@@ -318,18 +331,18 @@ public class ValidatorTest {
         options.add(o1);
         options.add(o2);
         prop.setOptions(options);
-        Validator.validateOptions(prop, "foo", messages);
+        validator.validateOptions(prop, "foo", messages);
         assertTrue(messages.isEmpty());
-        Validator.validateOptions(prop, "bar", messages);
+        validator.validateOptions(prop, "bar", messages);
         assertEquals(1, messages.size());
         messages.clear();
-        Validator.validateOptions(prop, 7, messages);
+        validator.validateOptions(prop, 7, messages);
         assertTrue(messages.isEmpty());
     }
     
     @Test public void testValidateList() {
         final List<String> messages = new ArrayList<>();
-        final Property prop = new Property();
+        final PropertyDescription prop = new PropertyDescription();
 
         final List<Object> values = new ArrayList<>();
         values.add("a");
@@ -337,43 +350,43 @@ public class ValidatorTest {
         values.add("c");
 
         // default cardinality - no excludes/includes
-        Validator.validateList(prop, values, messages);
+        validator.validateList(prop, values, messages);
         assertEquals(1, messages.size());
         messages.clear();
 
         // cardinality 3 - no excludes/includes
         prop.setCardinality(3);
-        Validator.validateList(prop, values, messages);
+        validator.validateList(prop, values, messages);
         assertTrue(messages.isEmpty());
 
         values.add("d");
-        Validator.validateList(prop, values, messages);
+        validator.validateList(prop, values, messages);
         assertEquals(1, messages.size());
         messages.clear();
 
         // excludes
         prop.setExcludes(new String[] {"d", "e"});
-        Validator.validateList(prop, values, messages);
+        validator.validateList(prop, values, messages);
         assertEquals(2, messages.size()); // cardinality and exclude
         messages.clear();
 
         values.remove("d");
-        Validator.validateList(prop, values, messages);
+        validator.validateList(prop, values, messages);
         assertTrue(messages.isEmpty());
 
         // includes
         prop.setIncludes(new String[] {"b"});
-        Validator.validateList(prop, values, messages);
+        validator.validateList(prop, values, messages);
         assertTrue(messages.isEmpty());
 
         prop.setIncludes(new String[] {"x"});
-        Validator.validateList(prop, values, messages);
+        validator.validateList(prop, values, messages);
         assertEquals(1, messages.size());
         messages.clear();
 
         values.add("x");
         values.remove("a");
-        Validator.validateList(prop, values, messages);
+        validator.validateList(prop, values, messages);
         assertTrue(messages.isEmpty());
     }
 }