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 baae44b Update javadocs and add conf validation tests
baae44b is described below
commit baae44ba617d3bd50eb35ece6ec25010b137f45b
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Thu Nov 12 06:25:45 2020 +0100
Update javadocs and add conf validation tests
---
.../apiregions/api/config/AttributeableEntity.java | 11 ++-
.../apiregions/api/config/ConfigurableEntity.java | 5 +-
.../apiregions/api/config/ConfigurationApi.java | 23 +++---
.../api/config/ConfigurationDescription.java | 3 +
.../apiregions/api/config/DescribableEntity.java | 21 +++---
.../config/FactoryConfigurationDescription.java | 15 +++-
.../api/config/FrameworkPropertyDescription.java | 2 +-
.../extension/apiregions/api/config/Operation.java | 4 +-
.../extension/apiregions/api/config/Option.java | 7 +-
.../apiregions/api/config/PropertyDescription.java | 9 ++-
.../extension/apiregions/api/config/Range.java | 9 ++-
.../validation/ConfigurationValidationResult.java | 2 +-
.../config/validation/FeatureValidationResult.java | 2 +-
.../validation/ConfigurationValidatorTest.java | 82 ++++++++++++++++++++++
14 files changed, 156 insertions(+), 39 deletions(-)
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 javax.json.JsonValue;
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 @@ public abstract class AttributeableEntity {
/**
* 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 @@ public abstract class AttributeableEntity {
/**
* 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 @@ public abstract class AttributeableEntity {
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 @@ public abstract class ConfigurableEntity extends
DescribableEntity {
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 @@ public abstract class ConfigurableEntity extends
DescribableEntity {
/**
* 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 @@ public abstract class ConfigurableEntity extends
DescribableEntity {
/**
* 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 @@ public class ConfigurationApi extends AttributeableEntity {
* 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 @@ public class ConfigurationApi extends AttributeableEntity {
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 @@ public class ConfigurationApi extends AttributeableEntity {
/**
* 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 @@ public class ConfigurationApi extends AttributeableEntity {
/**
* 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 @@ public class ConfigurationApi extends AttributeableEntity {
/**
* 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 @@ public class ConfigurationApi extends AttributeableEntity
{
/**
* 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 @@ public class ConfigurationApi extends AttributeableEntity {
/**
* 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.JsonException;
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 @@ public abstract class DescribableEntity extends
AttributeableEntity {
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 @@ public abstract class DescribableEntity extends
AttributeableEntity {
/**
* 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 @@ public abstract class DescribableEntity extends
AttributeableEntity {
/**
* Get the title
- * @return the title
+ * @return The title or {@code null}
*/
public String getTitle() {
return title;
@@ -72,13 +77,13 @@ public abstract class DescribableEntity extends
AttributeableEntity {
* 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 @@ public abstract class DescribableEntity extends
AttributeableEntity {
* 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 @@ public abstract class DescribableEntity extends
AttributeableEntity {
/**
* 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.JsonObject;
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 @@ public class FactoryConfigurationDescription extends
ConfigurableEntity {
}
/**
- * Clear the object and remove all metadata
+ * Clear the object and set the defaults
*/
public void clear() {
super.clear();
@@ -56,6 +59,7 @@ public class FactoryConfigurationDescription extends
ConfigurableEntity {
/**
* 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 @@ public class FactoryConfigurationDescription extends
ConfigurableEntity {
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 @@ public class FactoryConfigurationDescription extends
ConfigurableEntity {
}
/**
- * @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 @@ package
org.apache.sling.feature.extension.apiregions.api.config;
*/
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.JsonException;
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 @@ public class Option extends DescribableEntity {
/**
* 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 @@ public class PropertyDescription extends DescribableEntity {
/** Required? */
private boolean required;
+ /**
+ * Create a new description
+ */
public PropertyDescription() {
this.setDefaults();
}
@@ -71,7 +74,7 @@ public class PropertyDescription extends DescribableEntity {
}
/**
- * Clear the object and remove all metadata
+ * Clear the object and reset to defaults
*/
public void clear() {
super.clear();
@@ -222,7 +225,7 @@ public class PropertyDescription extends DescribableEntity {
/**
* Get the variable
- * @return the variable
+ * @return the variable or {@code null}
*/
public String getVariable() {
return variable;
@@ -302,7 +305,7 @@ public class PropertyDescription extends DescribableEntity {
/**
* 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 javax.json.JsonObjectBuilder;
import org.apache.felix.cm.json.Configurations;
+/**
+ * A numerical value range
+ */
public class Range extends AttributeableEntity {
/** The optional min value */
@@ -33,7 +36,7 @@ public class Range extends AttributeableEntity {
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 @@ public class Range extends AttributeableEntity {
/**
* Get the min value
- * @return the min
+ * @return the min or {@code null}
*/
public Number getMin() {
return min;
@@ -75,7 +78,7 @@ public class Range extends AttributeableEntity {
/**
* 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 @@ public class ConfigurationValidationResult {
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 class FeatureValidationResult {
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 @@ 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 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 @@ public class ConfigurationValidatorTest {
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());
+ }
}
