diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/AttributeableEntity.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/AttributeableEntity.java
index 7ca4b67..31210f8 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/AttributeableEntity.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/AttributeableEntity.java
@@ -28,13 +28,16 @@
 
 import org.apache.felix.cm.json.Configurations;
 
+/**
+ * Abstract class used by all entities which allow additional attributes to be stored.
+ */
 public abstract class AttributeableEntity {
 	
 	/** The additional attributes */
 	private final Map<String, JsonValue> attributes = new LinkedHashMap<>();
 
     /**
-     * Clear the object and remove all metadata
+     * Clear the object and reset to defaults
      */
 	public void clear() {
 		this.attributes.clear();
@@ -54,6 +57,7 @@
 	/**
 	 * Extract the metadata from the JSON object.
 	 * This method first calls {@link #clear()}
+     * 
 	 * @param jsonObj The JSON Object
 	 * @throws IOException If JSON parsing fails
 	 */
@@ -70,7 +74,7 @@
 
 	/**
 	 * Get the attributes
-	 * @return The attributes
+	 * @return Mutable map of attributes, by attribute name
 	 */
 	public Map<String, JsonValue> getAttributes() {
         return this.attributes;
@@ -139,6 +143,9 @@
 		return null;
 	}
 
+    /**
+     * Helper method to set a string value
+     */
     void setString(final JsonObjectBuilder builder, final String attributeName, final String value) {
 		if ( value != null ) {
 			builder.add(attributeName, value);
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 73513fb..6f25d4d 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
@@ -35,7 +35,7 @@
     private final Map<String, PropertyDescription> properties = new LinkedHashMap<>();
 
     /**
-     * Clear the object and remove all metadata
+     * Clear the object and reset to defaults
      */
 	public void clear() {
         super.clear();
@@ -45,6 +45,7 @@
 	/**
 	 * Extract the metadata from the JSON object.
 	 * This method first calls {@link #clear()}
+     * 
 	 * @param jsonObj The JSON Object
 	 * @throws IOException If JSON parsing fails
 	 */
@@ -66,7 +67,7 @@
 
 	/**
 	 * Get the properties
-	 * @return The properties
+	 * @return Mutable map of properties by property name
 	 */
     public Map<String, PropertyDescription> getPropertyDescriptions() {
         return this.properties;
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 558a0fe..33599d8 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
@@ -59,7 +59,7 @@
      * Get the configuration api from the extension.
      * 
      * @param ext The extension
-     * @return The configuration api or {@code null}.
+     * @return The configuration api or {@code null} if the extension is {@code null}.
      * @throws IllegalArgumentException If the extension is wrongly formatted
      */
     public static ConfigurationApi getConfigurationApi(final Extension ext) {
@@ -97,7 +97,7 @@
     private final List<String> internalFrameworkProperties = new ArrayList<>();
     
     /**
-     * Clear the object and remove all metadata
+     * Clear the object and reset to defaults
      */
     public void clear() {
         super.clear();
@@ -111,7 +111,8 @@
 
 	/**
 	 * Extract the metadata from the JSON object.
-	 * This method first calls {@link #clear()}
+	 * This method first calls {@link #clear()}.
+     * 
 	 * @param jsonObj The JSON Object
 	 * @throws IOException If JSON parsing fails
 	 */
@@ -174,7 +175,7 @@
 
     /**
      * Get the configuration descriptions
-	 * @return the configuration descriptions
+	 * @return Mutable map of configuration descriptions by pid
 	 */
 	public Map<String, ConfigurationDescription> getConfigurationDescriptions() {
 		return configurations;
@@ -182,7 +183,7 @@
 
 	/**
      * Get the factory configuration descriptions
-	 * @return the factories
+	 * @return Mutable map of factory descriptions by factory pid
 	 */
 	public Map<String, FactoryConfigurationDescription> getFactoryConfigurationDescriptions() {
 		return factories;
@@ -190,23 +191,23 @@
 
 	/**
      * Get the framework properties
-	 * @return the frameworkProperties
+	 * @return Mutable map of framework properties
 	 */
 	public Map<String, FrameworkPropertyDescription> getFrameworkPropertyDescriptions() {
 		return frameworkProperties;
 	}
 
 	/**
-     * Get the internal configuration names
-	 * @return the internalConfigurations
+     * Get the internal configuration pids
+	 * @return Mutable list of internal configuration pids
 	 */
 	public List<String> getInternalConfigurations() {
 		return internalConfigurations;
 	}
 
 	/**
-     * Get the internal factory names
-	 * @return the internalFactories
+     * Get the internal factory pids
+	 * @return Mutable list of internal factory configuration pids
 	 */
 	public List<String> getInternalFactoryConfigurations() {
 		return internalFactories;
@@ -214,7 +215,7 @@
 
 	/**
      * Get the internal framework property names
-	 * @return the internalFrameworkProperties
+	 * @return Mutable list of internal framework property names
 	 */
 	public List<String> getInternalFrameworkProperties() {
 		return internalFrameworkProperties;
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationDescription.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationDescription.java
index e9184f2..0e2c7e9 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationDescription.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurationDescription.java
@@ -16,6 +16,9 @@
  */
 package org.apache.sling.feature.extension.apiregions.api.config;
 
+/**
+ * A description of an OSGi configuration
+ */
 public class ConfigurationDescription extends ConfigurableEntity {
 
 }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/DescribableEntity.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/DescribableEntity.java
index 0f36d8a..b2617d4 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/DescribableEntity.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/DescribableEntity.java
@@ -22,6 +22,10 @@
 import javax.json.JsonObject;
 import javax.json.JsonObjectBuilder;
 
+/**
+ * Abstract class for all describable entities, having an optional title,
+ * description and deprecation info.
+ */
 public abstract class DescribableEntity extends AttributeableEntity {
 	
 	/** The title */
@@ -34,7 +38,7 @@
 	private String deprecated;
 
 	/**
-     * Clear the object and remove all metadata
+     * Clear the object and reset to defaults
      */
 	public void clear() {
 		super.clear();
@@ -46,6 +50,7 @@
 	/**
 	 * Extract the metadata from the JSON object.
 	 * This method first calls {@link #clear()}
+     * 
 	 * @param jsonObj The JSON Object
 	 * @throws IOException If JSON parsing fails
 	 */
@@ -62,7 +67,7 @@
 
 	/**
 	 * Get the title
-	 * @return the title
+	 * @return The title or {@code null}
 	 */
 	public String getTitle() {
 		return title;
@@ -72,13 +77,13 @@
 	 * Set the title
 	 * @param title the title to set
 	 */
-	public void setTitle(String title) {
+	public void setTitle(final String title) {
 		this.title = title;
 	}
 
 	/**
 	 * Get the description
-	 * @return the description
+	 * @return the description or {@code null}
 	 */
 	public String getDescription() {
 		return description;
@@ -88,13 +93,13 @@
 	 * Set the description
 	 * @param description the description to set
 	 */
-	public void setDescription(String description) {
+	public void setDescription(final String description) {
 		this.description = description;
 	}
 
 	/**
 	 * Get the deprecation text
-	 * @return the deprecated
+	 * @return the deprecation text or {@code null}
 	 */
 	public String getDeprecated() {
 		return deprecated;
@@ -102,9 +107,9 @@
 
 	/**
 	 * Set the deprecation text
-	 * @param deprecated the deprecated to set
+	 * @param deprecated the deprecation text to set
 	 */
-	public void setDeprecated(String deprecated) {
+	public void setDeprecated(final String deprecated) {
 		this.deprecated = deprecated;
 	}
 
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescription.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescription.java
index 6ce6936..9c8d7fa 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescription.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FactoryConfigurationDescription.java
@@ -29,6 +29,9 @@
 import javax.json.JsonObjectBuilder;
 import javax.json.JsonValue;
 
+/**
+ * Description of an OSGi factory configuration
+ */
 public class FactoryConfigurationDescription extends ConfigurableEntity {
     
     private final Set<Operation> operations = new HashSet<>();
@@ -45,7 +48,7 @@
     }
     
     /**
-     * Clear the object and remove all metadata
+     * Clear the object and set the defaults
      */
     public void clear() {
         super.clear();
@@ -56,6 +59,7 @@
 	/**
 	 * Extract the metadata from the JSON object.
 	 * This method first calls {@link #clear()}
+     *
 	 * @param jsonObj The JSON Object
 	 * @throws IOException If JSON parsing fails
 	 */
@@ -70,6 +74,9 @@
                     final String v = getString(innerVal).toUpperCase();
                     this.getOperations().add(Operation.valueOf(v));
                 }
+                if ( this.getOperations().isEmpty() ) {
+                    throw new IOException("Operations must not be empty");
+                }
             }
             
             val = this.getAttributes().remove(InternalConstants.KEY_INTERNAL_NAMES);
@@ -85,14 +92,16 @@
     }
 
    /**
-	 * @return the operations
+    * Get the operations
+	 * @return Mutable set of operations
 	 */
 	public Set<Operation> getOperations() {
 		return operations;
 	}
 
 	/**
-	 * @return the internalNames
+     * Get the internal factory configuration name
+	 * @return Mutable list of internal names
 	 */
 	public List<String> getInternalNames() {
 		return internalNames;
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkPropertyDescription.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkPropertyDescription.java
index 3e1e75f..df9a3b0 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkPropertyDescription.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/FrameworkPropertyDescription.java
@@ -17,7 +17,7 @@
 package org.apache.sling.feature.extension.apiregions.api.config;
 
 /**
- * A framework property
+ * A framework property description
  */
 public class FrameworkPropertyDescription extends PropertyDescription {
     
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Operation.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Operation.java
index 308143b..312c2b2 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Operation.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Operation.java
@@ -21,6 +21,6 @@
  */
 public enum Operation {
     
-    CREATE,
-    UPDATE
+    CREATE, // allowed to create a factory configuration
+    UPDATE  // allowed to update a factory configuration
 }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Option.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Option.java
index 4efeecf..41d2c23 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Option.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Option.java
@@ -22,13 +22,16 @@
 import javax.json.JsonObject;
 import javax.json.JsonObjectBuilder;
 
+/**
+ * Option for a property value
+ */
 public class Option extends DescribableEntity {
 
     /** The value for the option */
     private String value;
 
     /**
-     * Clear the object and remove all metadata
+     * Clear the object and reset to defaults
      */
     public void clear() {
         super.clear();
@@ -52,7 +55,7 @@
     
     /**
      * Get the value for the option
-  	 * @return the value
+  	 * @return the value or {@code null}
      */
     public String getValue() {
         return value;
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
index 571c9c9..4323937 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/PropertyDescription.java
@@ -60,6 +60,9 @@
 	/** Required? */
 	private boolean required;
     
+    /**
+     * Create a new description
+     */
     public PropertyDescription() {
         this.setDefaults();
     }
@@ -71,7 +74,7 @@
     }
 
     /**
-     * Clear the object and remove all metadata
+     * Clear the object and reset to defaults
      */
 	public void clear() {
         super.clear();
@@ -222,7 +225,7 @@
 
 	/**
 	 * Get the variable
-	 * @return the variable
+	 * @return the variable or {@code null}
 	 */
 	public String getVariable() {
 		return variable;
@@ -302,7 +305,7 @@
 
 	/**
 	 * Get the regex
-	 * @return the regex
+	 * @return the regex or {@code null}
 	 */
 	public String getRegex() {
 		return pattern == null ? null : pattern.pattern();
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Range.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Range.java
index 3acac02..f5daf55 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Range.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/Range.java
@@ -24,6 +24,9 @@
 
 import org.apache.felix.cm.json.Configurations;
 
+/**
+ * A numerical value range
+ */
 public class Range extends AttributeableEntity {
 
     /** The optional min value */
@@ -33,7 +36,7 @@
     private Number max;
 
     /**
-     * Clear the object and remove all metadata
+     * Clear the object and reset to defaults
      */
 	public void clear() {
         super.clear();
@@ -59,7 +62,7 @@
 
 	/**
      * Get the min value
-	 * @return the min
+	 * @return the min or {@code null}
 	 */
 	public Number getMin() {
 		return min;
@@ -75,7 +78,7 @@
 
 	/**
      * Get the max value
-	 * @return the max
+	 * @return the max or {@code null}
 	 */
 	public Number getMax() {
 		return max;
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
index 8765347..f1cb9f4 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidationResult.java
@@ -33,7 +33,7 @@
         boolean valid = globalErrors.isEmpty();
         if ( valid ) {
             for(final PropertyValidationResult r : this.propertyResults.values()) {
-                if ( r.isValid() ) {
+                if ( !r.isValid() ) {
                     valid = false;
                     break;
                 }
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 257df68..1e49dd0 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/FeatureValidationResult.java
@@ -26,7 +26,7 @@
     public boolean isValid() {
         boolean valid = true;
         for(final ConfigurationValidationResult r : this.configurationResults.values()) {
-            if ( r.isValid() ) {
+            if ( !r.isValid() ) {
                 valid = false;
                 break;
             }
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 1a451b2..8620daf 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
@@ -18,11 +18,15 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 import org.apache.sling.feature.Configuration;
 import org.apache.sling.feature.extension.apiregions.api.config.ConfigurationDescription;
 import org.apache.sling.feature.extension.apiregions.api.config.FactoryConfigurationDescription;
+import org.apache.sling.feature.extension.apiregions.api.config.PropertyDescription;
+import org.apache.sling.feature.extension.apiregions.api.config.PropertyType;
 import org.junit.Test;
+import org.osgi.framework.Constants;
 
 public class ConfigurationValidatorTest {
 
@@ -45,4 +49,82 @@
         assertFalse(result.isValid());
         assertEquals(1, result.getGlobalErrors().size());
     }
+
+    @Test public void testDeprecated() {
+        final Configuration cfg = new Configuration("org.apache");
+        final ConfigurationDescription cd = new ConfigurationDescription();
+        
+        ConfigurationValidationResult result = validator.validate(cd, cfg);
+        assertTrue(result.isValid());
+        assertTrue(result.getWarnings().isEmpty());
+
+        cd.setDeprecated("this is deprecated");
+        result = validator.validate(cd, cfg);
+        assertTrue(result.isValid());
+        assertFalse(result.getWarnings().isEmpty());
+        assertEquals("this is deprecated", result.getWarnings().get(0));
+    }
+
+    @Test public void testServiceRanking() {
+        final Configuration cfg = new Configuration("org.apache");
+        final ConfigurationDescription cd = new ConfigurationDescription();
+        cfg.getProperties().put(Constants.SERVICE_RANKING, 5); 
+
+        ConfigurationValidationResult result = validator.validate(cd, cfg);
+        assertTrue(result.isValid());
+
+        cfg.getProperties().put(Constants.SERVICE_RANKING, "5");
+        result = validator.validate(cd, cfg);
+        assertFalse(result.isValid());
+    }
+
+    @Test public void testAllowedProperties() {
+        final Configuration cfg = new Configuration("org.apache");
+        final ConfigurationDescription cd = new ConfigurationDescription();
+        cfg.getProperties().put(Constants.SERVICE_DESCRIPTION, "desc");
+        cfg.getProperties().put(Constants.SERVICE_VENDOR, "vendor");
+
+        ConfigurationValidationResult result = validator.validate(cd, cfg);
+        assertTrue(result.isValid());
+    }
+
+    @Test public void testAdditionalProperties() {
+        final Configuration cfg = new Configuration("org.apache");
+        cfg.getProperties().put("a", "desc");
+
+        final ConfigurationDescription cd = new ConfigurationDescription();
+        final PropertyDescription prop = new PropertyDescription();
+        cd.getPropertyDescriptions().put("a", prop);
+
+        ConfigurationValidationResult result = validator.validate(cd, cfg);
+        assertTrue(result.isValid());
+        assertEquals(1, result.getPropertyResults().size());
+        assertTrue(result.getPropertyResults().get("a").isValid());
+
+        cfg.getProperties().put("b", "vendor");
+        result = validator.validate(cd, cfg);
+        assertFalse(result.isValid());
+        assertEquals(2, result.getPropertyResults().size());
+        assertTrue(result.getPropertyResults().get("a").isValid());
+        assertFalse(result.getPropertyResults().get("b").isValid());
+    }
+
+    @Test public void testInvalidProperty() {
+        final Configuration cfg = new Configuration("org.apache");
+        cfg.getProperties().put("a", "desc");
+        cfg.getProperties().put("b", "vendor");
+
+        final ConfigurationDescription cd = new ConfigurationDescription();
+        final PropertyDescription propA = new PropertyDescription();
+        cd.getPropertyDescriptions().put("a", propA);
+        final PropertyDescription propB = new PropertyDescription();
+        propB.setType(PropertyType.INTEGER);
+        cd.getPropertyDescriptions().put("b", propB);
+
+        ConfigurationValidationResult result = validator.validate(cd, cfg);
+        assertFalse(result.isValid());
+        assertEquals(2, result.getPropertyResults().size());
+        assertTrue(result.getPropertyResults().get("a").isValid());
+        assertFalse(result.getPropertyResults().get("b").isValid());
+     }
 }