You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2019/08/21 16:00:20 UTC
[sling-org-apache-sling-feature] 01/01: SLING-8647: Provide
policies for configuration merging
This is an automated email from the ASF dual-hosted git repository.
pauls pushed a commit to branch issues/SLING-8647
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature.git
commit 7eb6d35969bbbb1723edd10aa8530b0c16e77baa
Author: Karl Pauls <ka...@gmail.com>
AuthorDate: Wed Aug 21 18:00:06 2019 +0200
SLING-8647: Provide policies for configuration merging
---
.../sling/feature/builder/BuilderContext.java | 28 +++
.../apache/sling/feature/builder/BuilderUtil.java | 90 +++++++++-
.../sling/feature/builder/FeatureBuilder.java | 8 +-
.../apache/sling/feature/builder/package-info.java | 2 +-
.../sling/feature/builder/BuilderUtilTest.java | 189 ++++++++++++++++++++-
.../sling/feature/builder/FeatureBuilderTest.java | 37 ++++
6 files changed, 340 insertions(+), 14 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
index a515c94..a9df0d4 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
@@ -64,6 +64,18 @@ public class BuilderContext {
/** Used in override rule to match all coordinates */
public static final String COORDINATE_MATCH_ALL = "*";
+ /** Used to handle configuration merging - fail the merge when there is a clash for a PID - this is the default */
+ public static final String CONFIG_FAIL_ON_CLASH = "CLASH";
+
+ /** Used to handle configuration merging - fail the merge only when there is a clash on a property level */
+ public static final String CONFIG_FAIL_ON_PROPERTY_CLASH = "PROPERTY_CLASH";
+
+ /** Used to handle configuration merging - use the latest configuration, but don't merge */
+ public static final String CONFIG_USE_LATEST = "USE_LATEST";
+
+ /** Used to handle configuration merging - merge the latest configuration in, latest props might override previous values */
+ public static final String CONFIG_MERGE_LATEST = "MERGE_LATEST";
+
/** Configuration key for configuration for all handlers */
static final String CONFIGURATION_ALL_HANDLERS_KEY = "all";
@@ -79,6 +91,7 @@ public class BuilderContext {
private final List<ArtifactId> artifactsOverrides = new ArrayList<>();
private final Map<String,String> variables = new HashMap<>();
private final Map<String,String> frameworkProperties = new HashMap<>();
+ private final Map<String,String> configOverrides = new HashMap<>();
/**
@@ -156,6 +169,17 @@ public class BuilderContext {
}
/**
+ * Add merge policies for configuration clashes.
+ *
+ * @param overrides The overrides
+ * @return The builder context
+ */
+ public BuilderContext addConfigsOverrides(final Map<String, String> overrides) {
+ this.configOverrides.putAll(overrides);
+ return this;
+ }
+
+ /**
* Add merge extensions
*
* @param extensions A list of merge extensions.
@@ -211,6 +235,10 @@ public class BuilderContext {
return this.artifactsOverrides;
}
+ Map<String, String> getConfigOverrides() {
+ return this.configOverrides;
+ }
+
Map<String,String> getVariablesOverrides() {
return this.variables;
}
diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
index 4c8a65c..c4c98f8 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -219,7 +219,7 @@ class BuilderUtil {
}
final List<Artifact> result = new ArrayList<>();
- for (ArtifactId prefix : commonPrefixes) {
+ outer: for (ArtifactId prefix : commonPrefixes) {
for (final ArtifactId override : artifactOverrides) {
if (match(prefix, override)) {
String rule = override.getVersion();
@@ -247,6 +247,7 @@ class BuilderUtil {
result.add(selectStartOrder(fromTarget, fromSource, new Artifact(override)));
}
}
+ break outer;
}
}
}
@@ -310,6 +311,45 @@ class BuilderUtil {
return matchCount == 4;
}
+ private static boolean match(final Configuration config, String override) {
+ boolean result;
+ if (override.equals("*")) {
+ result = true;
+ }
+ else if (Configuration.isFactoryConfiguration(config.getPid())) {
+ if (Configuration.isFactoryConfiguration(override)) {
+ String configPID = Configuration.getFactoryPid(config.getPid());
+ String overridePID = Configuration.getFactoryPid(override);
+ if (match(configPID, overridePID)) {
+ String configName = Configuration.getName(config.getPid());
+ String overrideName = Configuration.getName(override);
+ result = match(configName, overrideName);
+ }
+ else {
+ result = false;
+ }
+ }
+ else {
+ result = false;
+ }
+ }
+ else {
+ result = match(config.getPid(), override);
+ }
+ return result;
+ }
+
+ private static boolean match(String value, String override) {
+ boolean result;
+ if (override.endsWith("*")) {
+ override = override.substring(0, override.length() - 1);
+ result = value.startsWith(override);
+ }
+ else {
+ result = value.equals(override);
+ }
+ return result;
+ }
private static Set<ArtifactId> getCommonArtifactIds(Artifact a1, Artifact a2) {
final Set<ArtifactId> result = new HashSet<>();
for (final ArtifactId id : a1.getAliases(true)) {
@@ -335,19 +375,51 @@ class BuilderUtil {
}
// configurations - merge / override
- static void mergeConfigurations(final Configurations target, final Configurations source) {
+ static void mergeConfigurations(final Configurations target, final Configurations source, final Map<String, String> overrides) {
for(final Configuration cfg : source) {
boolean found = false;
- for(final Configuration current : target) {
+ for(int c = 0; c < target.size();c++) {
+ final Configuration current = target.get(c);
+
if ( current.compareTo(cfg) == 0 ) {
found = true;
- // merge / override properties
- final Enumeration<String> i = cfg.getProperties().keys();
- while ( i.hasMoreElements() ) {
- final String key = i.nextElement();
- current.getProperties().put(key, cfg.getProperties().get(key));
+
+ boolean handled = false;
+ outer:
+ for (Map.Entry<String, String> override : overrides.entrySet()) {
+ if (match(cfg, override.getKey())) {
+ if (BuilderContext.CONFIG_USE_LATEST.equals(override.getValue())) {
+ int idx = target.indexOf(current);
+ target.remove(current);
+ target.add(idx, cfg.copy(cfg.getPid()));
+ handled = true;
+ }
+ else if (BuilderContext.CONFIG_FAIL_ON_PROPERTY_CLASH.equals(override.getValue())){
+ for (Enumeration<String> i = cfg.getProperties().keys(); i.hasMoreElements(); ) {
+ final String key = i.nextElement();
+ if (current.getProperties().get(key) != null) {
+ break outer;
+ }
+ else {
+ current.getProperties().put(key, cfg.getProperties().get(key));
+ }
+ }
+ handled = true;
+ }
+ else if (BuilderContext.CONFIG_MERGE_LATEST.equals(override.getValue())) {
+ for (Enumeration<String> i = cfg.getProperties().keys(); i.hasMoreElements(); ) {
+ final String key = i.nextElement();
+ current.getProperties().put(key, cfg.getProperties().get(key));
+ }
+ handled = true;
+ }
+ break outer;
+ }
+ }
+ if (!handled) {
+ throw new IllegalStateException("Configuration override rule required to select between configurations for " +
+ cfg.getPid());
}
- break;
}
}
if ( !found ) {
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 7dc6a42..83de762 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -202,7 +202,7 @@ public abstract class FeatureBuilder {
targetIsComplete = false;
}
- merge(target, assembled, context, context.getArtifactOverrides(), null);
+ merge(target, assembled, context, context.getArtifactOverrides(), context.getConfigOverrides(),null);
}
// check complete flag
@@ -324,11 +324,12 @@ public abstract class FeatureBuilder {
processPrototype(prototypeFeature, i);
// and now merge the prototype feature into the result. No overrides should be needed since the result is empty before
- merge(result, prototypeFeature, context, Collections.emptyList(), TRACKING_KEY);
+ merge(result, prototypeFeature, context, Collections.emptyList(), Collections.EMPTY_MAP, TRACKING_KEY);
// and merge the current feature over the prototype feature into the result
merge(result, feature, context, Collections.singletonList(
ArtifactId.parse(BuilderUtil.CATCHALL_OVERRIDE + BuilderContext.VERSION_OVERRIDE_ALL)),
+ Collections.singletonMap("*", BuilderContext.CONFIG_MERGE_LATEST),
TRACKING_KEY);
for (Artifact a : result.getBundles()) {
@@ -354,10 +355,11 @@ public abstract class FeatureBuilder {
final Feature source,
final BuilderContext context,
final List<ArtifactId> artifactOverrides,
+ final Map<String, String> configOverrides,
final String originKey) {
BuilderUtil.mergeVariables(target.getVariables(), source.getVariables(), context);
BuilderUtil.mergeArtifacts(target.getBundles(), source.getBundles(), source, artifactOverrides, originKey);
- BuilderUtil.mergeConfigurations(target.getConfigurations(), source.getConfigurations());
+ BuilderUtil.mergeConfigurations(target.getConfigurations(), source.getConfigurations(), configOverrides);
BuilderUtil.mergeFrameworkProperties(target.getFrameworkProperties(), source.getFrameworkProperties(), context);
BuilderUtil.mergeRequirements(target.getRequirements(), source.getRequirements());
BuilderUtil.mergeCapabilities(target.getCapabilities(), source.getCapabilities());
diff --git a/src/main/java/org/apache/sling/feature/builder/package-info.java b/src/main/java/org/apache/sling/feature/builder/package-info.java
index d8c51dc..3310a48 100644
--- a/src/main/java/org/apache/sling/feature/builder/package-info.java
+++ b/src/main/java/org/apache/sling/feature/builder/package-info.java
@@ -17,7 +17,7 @@
* under the License.
*/
-@org.osgi.annotation.versioning.Version("1.1.0")
+@org.osgi.annotation.versioning.Version("1.2.0")
package org.apache.sling.feature.builder;
diff --git a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
index d8721b5..09d9bac 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -17,6 +17,8 @@
package org.apache.sling.feature.builder;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -29,6 +31,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Properties;
import javax.json.Json;
import javax.json.JsonArray;
@@ -43,6 +46,8 @@ import javax.json.stream.JsonGenerator;
import org.apache.sling.feature.Artifact;
import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Bundles;
+import org.apache.sling.feature.Configuration;
+import org.apache.sling.feature.Configurations;
import org.apache.sling.feature.Extension;
import org.apache.sling.feature.ExtensionState;
import org.apache.sling.feature.ExtensionType;
@@ -482,7 +487,7 @@ public class BuilderUtilTest {
Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("gid:aid:2"), ArtifactId.parse("gid:aid:3"));
- assertEquals(Arrays.asList(a2, new Artifact(ArtifactId.fromMvnId("gid:aid:3"))),
+ assertEquals(Arrays.asList(a2),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
}
@@ -561,6 +566,188 @@ public class BuilderUtilTest {
assertEquals(0, cfg.size());
}
+ @Test public void testMergeConfigurations() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration bar = new Configuration("bar");
+ bar.getProperties().put("barKey", "valueBAR");
+ source.add(bar);
+ BuilderUtil.mergeConfigurations(target, source, Collections.EMPTY_MAP);
+ assertEquals(2, target.size());
+ assertEquals(target.getConfiguration("foo").getProperties(), foo.getProperties());
+ assertEquals(target.getConfiguration("bar").getProperties(), bar.getProperties());
+ }
+
+ @Test public void testMergeConfigurationsCLASH() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ target.add(new Configuration("foo"));
+ source.add(new Configuration("foo"));
+ try {
+ BuilderUtil.mergeConfigurations(target, source, Collections.EMPTY_MAP);
+ fail();
+ } catch (IllegalStateException ex) {
+
+ }
+ }
+
+ @Test public void testMergeConfigurationsCLASHPROPERTY() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration foo2 = new Configuration("foo");
+ foo2.getProperties().put("barKey", "valueBAR");
+ source.add(foo2);
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("fo*", BuilderContext.CONFIG_FAIL_ON_PROPERTY_CLASH));
+
+ assertEquals("valueFOO", target.getConfiguration("foo").getProperties().get("fooKey"));
+ assertEquals("valueBAR", target.getConfiguration("foo").getProperties().get("barKey"));
+
+ try {
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("fo*", BuilderContext.CONFIG_FAIL_ON_PROPERTY_CLASH));
+ fail();
+ } catch (IllegalStateException ex) {
+
+ }
+ }
+
+ @Test public void testMergeConfigurationsUSELATEST() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration foo2 = new Configuration("foo");
+ foo2.getProperties().put("barKey", "valueBAR");
+ source.add(foo2);
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("fo*", BuilderContext.CONFIG_USE_LATEST));
+
+ assertEquals("valueBAR", target.getConfiguration("foo").getProperties().get("barKey"));
+ assertNull(target.getConfiguration("foo").getProperties().get("fooKey"));
+ }
+
+ @Test public void testMergeConfigurationsMERGELATEST() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration foo2 = new Configuration("foo");
+ foo2.getProperties().put("fooKey", "valueBAR");
+ source.add(foo2);
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("fo*", BuilderContext.CONFIG_MERGE_LATEST));
+
+ assertEquals("valueBAR", target.getConfiguration("foo").getProperties().get("fooKey"));
+ }
+
+ @Test public void testMergeConfigurationsFactory() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo~foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration bar = new Configuration("bar~bar");
+ bar.getProperties().put("barKey", "valueBAR");
+ source.add(bar);
+ BuilderUtil.mergeConfigurations(target, source, Collections.EMPTY_MAP);
+ assertEquals(2, target.size());
+ assertEquals(target.getConfiguration("foo~foo").getProperties(), foo.getProperties());
+ assertEquals(target.getConfiguration("bar~bar").getProperties(), bar.getProperties());
+ }
+
+ @Test public void testMergeConfigurationsCLASHFactory() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ target.add(new Configuration("foo~foo"));
+ source.add(new Configuration("foo~foo"));
+ try {
+ BuilderUtil.mergeConfigurations(target, source, Collections.EMPTY_MAP);
+ fail();
+ } catch (IllegalStateException ex) {
+
+ }
+ }
+
+ @Test public void testMergeConfigurationsCLASHPROPERTYFactory() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo~foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration foo2 = new Configuration("foo~foo");
+ foo2.getProperties().put("barKey", "valueBAR");
+ source.add(foo2);
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("fo*~f*", BuilderContext.CONFIG_FAIL_ON_PROPERTY_CLASH));
+
+ assertEquals("valueFOO", target.getConfiguration("foo~foo").getProperties().get("fooKey"));
+ assertEquals("valueBAR", target.getConfiguration("foo~foo").getProperties().get("barKey"));
+
+ try {
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("fo*~fo*", BuilderContext.CONFIG_FAIL_ON_PROPERTY_CLASH));
+ fail();
+ } catch (IllegalStateException ex) {
+
+ }
+ }
+
+ @Test public void testMergeConfigurationsUSELATESTFactory() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo~foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration foo2 = new Configuration("foo~foo");
+ foo2.getProperties().put("barKey", "valueBAR");
+ source.add(foo2);
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("fo*~f*", BuilderContext.CONFIG_USE_LATEST));
+
+ assertEquals("valueBAR", target.getConfiguration("foo~foo").getProperties().get("barKey"));
+ assertNull(target.getConfiguration("foo~foo").getProperties().get("fooKey"));
+ }
+
+ @Test public void testMergeConfigurationsMERGELATESTFactory() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo~foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration foo2 = new Configuration("foo~foo");
+ foo2.getProperties().put("fooKey", "valueBAR");
+ source.add(foo2);
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("foo~foo", BuilderContext.CONFIG_MERGE_LATEST));
+
+ assertEquals("valueBAR", target.getConfiguration("foo~foo").getProperties().get("fooKey"));
+ }
+
+ @Test public void testMergeConfigurationsMixed() {
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configuration foo = new Configuration("foo~foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration foo4 = new Configuration("foo~foo");
+ foo.getProperties().put("fooKey", "valueFOO4");
+ source.add(foo);
+ Configuration foo2 = new Configuration("foo");
+ foo2.getProperties().put("fooKey", "valueBAR");
+ source.add(foo2);
+ Configuration foo3 = new Configuration("foo");
+ foo2.getProperties().put("fooKey", "valueBAR2");
+ source.add(foo3);
+ Map<String, String> overrides = new HashMap<>();
+ overrides.put("foo", BuilderContext.CONFIG_MERGE_LATEST);
+ overrides.put("foo~foo", BuilderContext.CONFIG_USE_LATEST);
+ BuilderUtil.mergeConfigurations(target, source, overrides);
+
+ assertEquals("valueFOO4", target.getConfiguration("foo~foo").getProperties().get("fooKey"));
+ assertEquals("valueBAR2", target.getConfiguration("foo").getProperties().get("fooKey"));
+ }
+
@SafeVarargs
public static Artifact createBundle(final String id, final int startOrder, Map.Entry<String, String> ... metadata) {
final Artifact a = new Artifact(ArtifactId.parse(id));
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 4ba78ba..53ed1ce 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -924,6 +924,43 @@ public class FeatureBuilderTest {
}
}
+ @Test
+ public void testConfigurationMerge() {
+ Feature a = new Feature(ArtifactId.fromMvnId("g:a:1.0.0"));
+ a.getConfigurations().add(new Configuration("foo"));
+ Feature b = new Feature(ArtifactId.fromMvnId("g:b:1.0.0"));
+ b.getConfigurations().add(new Configuration("bar"));
+
+ Feature c = FeatureBuilder.assemble(ArtifactId.fromMvnId("g:c:1.0.0"), new BuilderContext(provider), a, b);
+ c.getExtensions().clear();
+
+ Feature test = new Feature(ArtifactId.fromMvnId("g:c:1.0.0"));
+ test.getConfigurations().add(new Configuration("foo"));
+ test.getConfigurations().add(new Configuration("bar"));
+
+ assertEquals(test, c);
+
+ b.getConfigurations().add(new Configuration("foo"));
+
+ try
+ {
+ FeatureBuilder.assemble(ArtifactId.fromMvnId("g:c:1.0.0"), new BuilderContext(provider), a, b);
+ fail();
+ } catch (IllegalStateException ex) {
+
+ }
+
+ b.getConfigurations().get(0).getProperties().put("foo", "bar");
+
+ c = FeatureBuilder.assemble(ArtifactId.fromMvnId("g:c:1.0.0"), new BuilderContext(provider).addConfigsOverrides(Collections.singletonMap("*", BuilderContext.CONFIG_USE_LATEST)), a, b);
+
+ test.getConfigurations().add(b.getConfigurations().get(0));
+
+ c.getExtensions().clear();
+
+ assertEquals(test, c);
+ }
+
private static class MatchingRequirementImpl extends RequirementImpl implements MatchingRequirement {
public MatchingRequirementImpl(Resource res, String ns, Map<String, String> dirs, Map<String, Object> attrs) {