Update javadocs and add conf validation tests
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());
+ }
}
