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 2018/11/19 07:00:16 UTC
[sling-org-apache-sling-feature] branch master updated: SLING-8119
: Improve configuration handling
This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature.git
The following commit(s) were added to refs/heads/master by this push:
new 300a7f4 SLING-8119 : Improve configuration handling
300a7f4 is described below
commit 300a7f4f60a64e97ddd2dbf874732412c2154e71
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Mon Nov 19 07:59:37 2018 +0100
SLING-8119 : Improve configuration handling
---
.../org/apache/sling/feature/Configuration.java | 123 ++++++++++-----------
.../org/apache/sling/feature/Configurations.java | 19 +---
.../java/org/apache/sling/feature/Feature.java | 16 +--
.../sling/feature/builder/FeatureBuilder.java | 14 +--
.../sling/feature/builder/FeatureBuilderTest.java | 41 ++++---
5 files changed, 91 insertions(+), 122 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/Configuration.java b/src/main/java/org/apache/sling/feature/Configuration.java
index 64c42d5..82b128a 100644
--- a/src/main/java/org/apache/sling/feature/Configuration.java
+++ b/src/main/java/org/apache/sling/feature/Configuration.java
@@ -19,6 +19,7 @@ package org.apache.sling.feature;
import java.util.Collection;
import java.util.Dictionary;
import java.util.Enumeration;
+import java.util.Hashtable;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
@@ -39,17 +40,24 @@ public class Configuration
implements Comparable<Configuration> {
/**
- * This optional configuration property stores the artifact id (mvn id) of the bundle
- * this configuration belongs to.
+ * Prefix for instructions for the configurator.
*/
- public static final String PROP_ARTIFACT_ID = "service.bundleLocation";
+ public static final String CONFIGURATOR_PREFIX = ":configurator:";
+
+ /**
+ * Prefix for special properties which are not configuration properties.
+ */
+ public static final String PROP_PREFIX = CONFIGURATOR_PREFIX + "feature:";
+
+ /**
+ * This optional configuration property stores the artifact id (mvn id) of the
+ * bundle this configuration belongs to.
+ */
+ public static final String PROP_ARTIFACT_ID = PROP_PREFIX + "service.bundleLocation";
/** The pid or name for factory pids. */
private final String pid;
- /** The factory pid. */
- private final String factoryPid;
-
/** The properties. */
private final Dictionary<String, Object> properties = new OrderedDictionary();
@@ -63,101 +71,88 @@ public class Configuration
throw new IllegalArgumentException("pid must not be null");
}
this.pid = pid;
- this.factoryPid = null;
- }
-
- /**
- * Create a new factor configuration
- * @param factoryPid The factory pid
- * @param name The name of the factory pid
- * @throws IllegalArgumentException If factoryPid or name is {@code null}
- */
- public Configuration(final String factoryPid, final String name) {
- if ( factoryPid == null || name == null ) {
- throw new IllegalArgumentException("factoryPid and/or name must not be null");
- }
- this.pid = name;
- this.factoryPid = factoryPid;
- }
-
- private int compareString(final String a, final String b) {
- if ( a == null ) {
- if ( b == null ) {
- return 0;
- }
- return -1;
- }
- if ( b == null ) {
- return 1;
- }
- return a.compareTo(b);
}
@Override
public int compareTo(final Configuration o) {
- int result = compareString(this.factoryPid, o.factoryPid);
- if ( result == 0 ) {
- result = compareString(this.pid, o.pid);
- }
- return result;
+ return this.pid.compareTo(o.pid);
}
-
/**
* Get the pid.
- * If this is a factory configuration, it returns {@code null}
- * @return The pid or {@code null}
+ *
+ * @return The pid
*/
public String getPid() {
- if ( this.isFactoryConfiguration() ) {
- return null;
- }
return this.pid;
}
/**
- * Return the factory pid
- * @return The factory pid or {@code null}.
+ * Check whether the pid is a factory pid
+ *
+ * @param pid The pid
+ * @return {@code true} if it's a factory pid
*/
- public String getFactoryPid() {
- return this.factoryPid;
+ public static boolean isFactoryConfiguration(final String pid) {
+ return pid.contains("~");
}
/**
- * Return the name for a factory configuration.
- * @return The name or {@code null}.
+ * Return the factory pid of a pid if it's a factory configuration
+ *
+ * @param pid The pid
+ * @return The factory pid or {@code null}.
*/
- public String getName() {
- if ( this.isFactoryConfiguration() ) {
- return this.pid;
+ public static String getFactoryPid(final String pid) {
+ final int pos = pid.indexOf('~');
+ if (pos != -1) {
+ return pid.substring(0, pos);
}
return null;
}
/**
- * Check whether this is a factory configuration
- * @return {@code true} if it is a factory configuration
+ * Return the name for a factory configuration.
+ *
+ * @param pid The pid
+ * @return The name or {@code null}.
*/
- public boolean isFactoryConfiguration() {
- return this.factoryPid != null;
+ public static String getName(final String pid) {
+ final int pos = pid.indexOf('~');
+ if (pos != -1) {
+ return pid.substring(pos + 1);
+ }
+ return null;
}
/**
* Get all properties of the configuration.
+ *
* @return The properties
*/
public Dictionary<String, Object> getProperties() {
return this.properties;
}
+ /**
+ * Get the configuration properties of the configuration.
+ *
+ * @return The properties
+ */
+ public Dictionary<String, Object> getConfigurationProperties() {
+ final Dictionary<String, Object> p = new Hashtable<>();
+ final Enumeration<String> keys = this.properties.keys();
+ while (keys.hasMoreElements()) {
+ final String key = keys.nextElement();
+ if (!key.startsWith(CONFIGURATOR_PREFIX)) {
+ p.put(key, this.properties.get(key));
+ }
+ }
+ return p;
+ }
+
@Override
public String toString() {
- if ( this.isFactoryConfiguration() ) {
- return "Factory Configuration [factoryPid=" + factoryPid
- + ", name=" + pid
- + ", properties=" + properties
- + "]";
- }
return "Configuration [pid=" + pid
+ ", properties=" + properties
+ "]";
diff --git a/src/main/java/org/apache/sling/feature/Configurations.java b/src/main/java/org/apache/sling/feature/Configurations.java
index 2286f4d..b8337f8 100644
--- a/src/main/java/org/apache/sling/feature/Configurations.java
+++ b/src/main/java/org/apache/sling/feature/Configurations.java
@@ -34,24 +34,7 @@ public class Configurations extends ArrayList<Configuration> {
*/
public Configuration getConfiguration(final String pid) {
for(final Configuration cfg : this) {
- if ( !cfg.isFactoryConfiguration() && pid.equals(cfg.getPid())) {
- return cfg;
- }
- }
- return null;
- }
-
- /**
- * Get the factory configuration
- * @param factoryPid The factoryPid of the configuration
- * @param name The name of the configuration
- * @return The factory configuration or {@code null}
- */
- public Configuration getFactoryConfiguration(final String factoryPid, final String name) {
- for(final Configuration cfg : this) {
- if ( cfg.isFactoryConfiguration()
- && factoryPid.equals(cfg.getFactoryPid())
- && name.equals(cfg.getName())) {
+ if (pid.equals(cfg.getPid())) {
return cfg;
}
}
diff --git a/src/main/java/org/apache/sling/feature/Feature.java b/src/main/java/org/apache/sling/feature/Feature.java
index 9b0b9c6..2fc8430 100644
--- a/src/main/java/org/apache/sling/feature/Feature.java
+++ b/src/main/java/org/apache/sling/feature/Feature.java
@@ -16,17 +16,17 @@
*/
package org.apache.sling.feature;
-import org.apache.felix.utils.resource.CapabilityImpl;
-import org.apache.felix.utils.resource.RequirementImpl;
-import org.osgi.resource.Capability;
-import org.osgi.resource.Requirement;
-
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import org.apache.felix.utils.resource.CapabilityImpl;
+import org.apache.felix.utils.resource.RequirementImpl;
+import org.osgi.resource.Capability;
+import org.osgi.resource.Requirement;
+
/**
* A feature consists of
* <ul>
@@ -330,7 +330,9 @@ public class Feature implements Comparable<Feature> {
}
/**
- * Create a copy of the feature with a different id
+ * Create a copy of the feature with a different id For contained items like
+ * bundles, artifacts and configurations a copy is created as well.
+ *
* @param id The new id
* @return The copy of the feature with the new id
*/
@@ -360,7 +362,7 @@ public class Feature implements Comparable<Feature> {
// configurations
for(final Configuration cfg : this.getConfigurations()) {
- final Configuration c = cfg.isFactoryConfiguration() ? new Configuration(cfg.getFactoryPid(), cfg.getName()) : new Configuration(cfg.getPid());
+ final Configuration c = new Configuration(cfg.getPid());
final Enumeration<String> keyEnum = cfg.getProperties().keys();
while ( keyEnum.hasMoreElements() ) {
final String key = keyEnum.nextElement();
diff --git a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
index 0c9bdf7..ddb5c2a 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -401,20 +401,10 @@ public abstract class FeatureBuilder {
// process configuration removals
for (final String c : include.getConfigurationRemovals()) {
final int attrPos = c.indexOf('@');
- final String val = (attrPos == -1 ? c : c.substring(0, attrPos));
+ final String pid = (attrPos == -1 ? c : c.substring(0, attrPos));
final String attr = (attrPos == -1 ? null : c.substring(attrPos + 1));
- final int sepPos = val.indexOf('~');
- Configuration found = null;
- if ( sepPos == -1 ) {
- found = feature.getConfigurations().getConfiguration(val);
-
- } else {
- final String factoryPid = val.substring(0, sepPos);
- final String name = val.substring(sepPos + 1);
-
- found = feature.getConfigurations().getFactoryConfiguration(factoryPid, name);
- }
+ final Configuration found = feature.getConfigurations().getConfiguration(pid);
if ( found != null ) {
if ( attr == null ) {
feature.getConfigurations().remove(found);
diff --git a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
index dd22ac6..fb3faa2 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -16,6 +16,23 @@
*/
package org.apache.sling.feature.builder;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.AbstractMap;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
import org.apache.felix.utils.resource.CapabilityImpl;
import org.apache.felix.utils.resource.RequirementImpl;
import org.apache.sling.feature.Artifact;
@@ -30,23 +47,6 @@ import org.junit.Test;
import org.osgi.resource.Capability;
import org.osgi.resource.Requirement;
-import java.util.AbstractMap;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
public class FeatureBuilderTest {
private static final Map<String, Feature> FEATURES = new HashMap<>();
@@ -139,8 +139,7 @@ public class FeatureBuilderTest {
// configurations
assertEquals(expected.getConfigurations().size(), actuals.getConfigurations().size());
for(final Configuration cfg : expected.getConfigurations()) {
- final Configuration found = (cfg.isFactoryConfiguration() ? actuals.getConfigurations().getFactoryConfiguration(cfg.getFactoryPid(), cfg.getName())
- : actuals.getConfigurations().getConfiguration(cfg.getPid()));
+ final Configuration found = actuals.getConfigurations().getConfiguration(cfg.getPid());
assertNotNull("Configuration " + cfg, found);
assertEquals("Configuration " + cfg, cfg.getProperties(), found.getProperties());
}
@@ -241,7 +240,7 @@ public class FeatureBuilderTest {
co1.getProperties().put("number", 7);
base.getConfigurations().add(co1);
- final Configuration co2 = new Configuration("my.factory.pid", "name");
+ final Configuration co2 = new Configuration("my.factory.pid~name");
co2.getProperties().put("a.value", "yeah");
base.getConfigurations().add(co2);
@@ -294,7 +293,7 @@ public class FeatureBuilderTest {
co1.getProperties().put("number", 7);
base.getConfigurations().add(co1);
- final Configuration co2 = new Configuration("my.factory.pid", "name");
+ final Configuration co2 = new Configuration("my.factory.pid~name");
co2.getProperties().put("a.value", "yeah");
base.getConfigurations().add(co2);