You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by da...@apache.org on 2019/12/17 14:19:57 UTC
[sling-whiteboard] branch master updated: Add configuration merge
handling
This is an automated email from the ASF dual-hosted git repository.
davidb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
The following commit(s) were added to refs/heads/master by this push:
new f9a7874 Add configuration merge handling
f9a7874 is described below
commit f9a7874e73e1a306f6650a6c682281c230a7805b
Author: David Bosschaert <da...@gmail.com>
AuthorDate: Tue Dec 17 14:19:37 2019 +0000
Add configuration merge handling
---
.../src/main/java/org/osgi/feature/Feature.java | 2 +-
.../org/osgi/feature/builder/BundleBuilder.java | 2 +-
.../osgi/feature/builder/ConfigurationBuilder.java | 43 +++++++-
.../org/osgi/feature/builder/ExtensionBuilder.java | 21 ++++
.../org/osgi/feature/builder/FeatureBuilder.java | 51 +++++++--
.../osgi/feature/builder/MergeContextBuilder.java | 29 ++++--
.../org/osgi/feature/impl/FeatureServiceImpl.java | 116 ++++++++++++++++++---
.../osgi/feature/impl/FeatureServiceImplTest.java | 31 +++++-
.../src/test/resources/features/test-feature2.json | 5 +-
9 files changed, 258 insertions(+), 42 deletions(-)
diff --git a/osgi-featuremodel/src/main/java/org/osgi/feature/Feature.java b/osgi-featuremodel/src/main/java/org/osgi/feature/Feature.java
index 520d7b7..f041817 100644
--- a/osgi-featuremodel/src/main/java/org/osgi/feature/Feature.java
+++ b/osgi-featuremodel/src/main/java/org/osgi/feature/Feature.java
@@ -36,7 +36,7 @@ public interface Feature extends Artifact {
List<Bundle> getBundles();
- List<Configuration> getConfigurations();
+ Map<String, Configuration> getConfigurations();
Map<String, String> getVariables();
diff --git a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/BundleBuilder.java b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/BundleBuilder.java
index 3531d3d..0466f14 100644
--- a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/BundleBuilder.java
+++ b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/BundleBuilder.java
@@ -83,7 +83,7 @@ public class BundleBuilder {
@Override
public String toString() {
- return "BundleImpl [metadata=" + metadata + ", getID()=" + getID() + "]";
+ return "BundleImpl [getID()=" + getID() + "]";
}
}
}
diff --git a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/ConfigurationBuilder.java b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/ConfigurationBuilder.java
index 40bac8c..dd8aebe 100644
--- a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/ConfigurationBuilder.java
+++ b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/ConfigurationBuilder.java
@@ -21,6 +21,7 @@ import org.osgi.feature.Configuration;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.Objects;
public class ConfigurationBuilder {
private final String p;
@@ -35,16 +36,29 @@ public class ConfigurationBuilder {
public ConfigurationBuilder(String factoryPid, String name) {
this.p = factoryPid;
- this.name = null;
+ this.name = name;
+ }
+
+ public ConfigurationBuilder(Configuration c) {
+ if (c.getFactoryPid() == null) {
+ p = c.getPid();
+ name = null;
+ } else {
+ // TODO
+ p = null;
+ name = null;
+ }
+
+ addValues(c.getValues());
}
- public ConfigurationBuilder addConfiguration(String key, Object value) {
+ public ConfigurationBuilder addValue(String key, Object value) {
// TODO can do some validation on the configuration
this.values.put(key, value);
return this;
}
- public ConfigurationBuilder addConfiguration(Map<String, Object> cfg) {
+ public ConfigurationBuilder addValues(Map<String, Object> cfg) {
// TODO can do some validation on the configuration
this.values.putAll(cfg);
return this;
@@ -54,7 +68,7 @@ public class ConfigurationBuilder {
if (name == null) {
return new ConfigurationImpl(p, null, values);
} else {
- return new ConfigurationImpl(p, p + "~" + name, values);
+ return new ConfigurationImpl(p + "~" + name, p, values);
}
}
@@ -81,5 +95,26 @@ public class ConfigurationBuilder {
public Map<String, Object> getValues() {
return values;
}
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(factoryPid, pid, values);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (!(obj instanceof ConfigurationImpl))
+ return false;
+ ConfigurationImpl other = (ConfigurationImpl) obj;
+ return Objects.equals(factoryPid, other.factoryPid) && Objects.equals(pid, other.pid)
+ && Objects.equals(values, other.values);
+ }
+
+ @Override
+ public String toString() {
+ return "ConfigurationImpl [pid=" + pid + "]";
+ }
}
}
diff --git a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/ExtensionBuilder.java b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/ExtensionBuilder.java
index e4be602..a2ef848 100644
--- a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/ExtensionBuilder.java
+++ b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/ExtensionBuilder.java
@@ -25,6 +25,7 @@ import java.io.IOException;
import java.io.StringReader;
import java.util.ArrayList;
import java.util.List;
+import java.util.Objects;
public class ExtensionBuilder {
private final String name;
@@ -140,5 +141,25 @@ public class ExtensionBuilder {
return res;
}
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(content, name, type);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (!(obj instanceof ExtensionImpl))
+ return false;
+ ExtensionImpl other = (ExtensionImpl) obj;
+ return Objects.equals(content, other.content) && Objects.equals(name, other.name) && type == other.type;
+ }
+
+ @Override
+ public String toString() {
+ return "ExtensionImpl [name=" + name + ", type=" + type + "]";
+ }
}
}
diff --git a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/FeatureBuilder.java b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/FeatureBuilder.java
index b37c76f..f2b0e77 100644
--- a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/FeatureBuilder.java
+++ b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/FeatureBuilder.java
@@ -27,6 +27,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
public class FeatureBuilder {
private final ArtifactID id;
@@ -40,7 +41,7 @@ public class FeatureBuilder {
private boolean isFinal;
private final List<Bundle> bundles = new ArrayList<>();
- private final List<Configuration> configurations = new ArrayList<>();
+ private final Map<String,Configuration> configurations = new HashMap<>();
private final Map<String,String> variables = new HashMap<>();
public FeatureBuilder(ArtifactID id) {
@@ -88,7 +89,9 @@ public class FeatureBuilder {
}
public FeatureBuilder addConfigurations(Configuration ... configs) {
- this.configurations.addAll(Arrays.asList(configs));
+ for (Configuration cfg : configs) {
+ this.configurations.put(cfg.getPid(), cfg);
+ }
return this;
}
@@ -97,7 +100,7 @@ public class FeatureBuilder {
return this;
}
- public FeatureBuilder addVariables(Map<String, String> variables) {
+ public FeatureBuilder addVariables(Map<String,String> variables) {
this.variables.putAll(variables);
return this;
}
@@ -118,11 +121,11 @@ public class FeatureBuilder {
private final boolean isFinal;
private final List<Bundle> bundles;
- private final List<Configuration> configurations;
- private final Map<String, String> variables;
+ private final Map<String,Configuration> configurations;
+ private final Map<String,String> variables;
private FeatureImpl(ArtifactID id, String aTitle, String desc, String vnd, String lic, String loc,
- boolean comp, boolean fin, List<Bundle> bs, List<Configuration> cs, Map<String,String> vars) {
+ boolean comp, boolean fin, List<Bundle> bs, Map<String,Configuration> cs, Map<String,String> vars) {
super(id);
title = aTitle;
@@ -134,7 +137,7 @@ public class FeatureBuilder {
isFinal = fin;
bundles = Collections.unmodifiableList(bs);
- configurations = Collections.unmodifiableList(cs);
+ configurations = Collections.unmodifiableMap(cs);
variables = Collections.unmodifiableMap(vars);
}
@@ -179,13 +182,43 @@ public class FeatureBuilder {
}
@Override
- public List<Configuration> getConfigurations() {
+ public Map<String,Configuration> getConfigurations() {
return configurations;
}
@Override
- public Map<String, String> getVariables() {
+ public Map<String,String> getVariables() {
return variables;
}
+
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = super.hashCode();
+ result = prime * result + Objects.hash(bundles, complete, configurations, description, isFinal, license, location,
+ title, variables, vendor);
+ return result;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (!super.equals(obj))
+ return false;
+ if (!(obj instanceof FeatureImpl))
+ return false;
+ FeatureImpl other = (FeatureImpl) obj;
+ return Objects.equals(bundles, other.bundles) && complete == other.complete
+ && Objects.equals(configurations, other.configurations) && Objects.equals(description, other.description)
+ && isFinal == other.isFinal && Objects.equals(license, other.license)
+ && Objects.equals(location, other.location) && Objects.equals(title, other.title)
+ && Objects.equals(variables, other.variables) && Objects.equals(vendor, other.vendor);
+ }
+
+ @Override
+ public String toString() {
+ return "FeatureImpl [getID()=" + getID() + "]";
+ }
}
}
diff --git a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/MergeContextBuilder.java b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/MergeContextBuilder.java
index f558230..14f96c6 100644
--- a/osgi-featuremodel/src/main/java/org/osgi/feature/builder/MergeContextBuilder.java
+++ b/osgi-featuremodel/src/main/java/org/osgi/feature/builder/MergeContextBuilder.java
@@ -24,34 +24,41 @@ import java.util.List;
import java.util.function.BiFunction;
public class MergeContextBuilder {
- private BiFunction<Bundle, Bundle, List<Bundle>> bundleResolver;
+ private BiFunction<Bundle, Bundle, List<Bundle>> bundleHandler;
+ private BiFunction<Configuration, Configuration, Configuration> configHandler;
- public MergeContextBuilder setBundleConflictResolver(BiFunction<Bundle, Bundle, List<Bundle>> bf) {
- bundleResolver = bf;
+ public MergeContextBuilder bundleConflictHandler(BiFunction<Bundle, Bundle, List<Bundle>> bh) {
+ bundleHandler = bh;
+ return this;
+ }
+
+ public MergeContextBuilder configConflictHandler(BiFunction<Configuration, Configuration, Configuration> ch) {
+ configHandler = ch;
return this;
}
public MergeContext build() {
- return new MergeContextImpl(bundleResolver);
+ return new MergeContextImpl(bundleHandler, configHandler);
}
private static class MergeContextImpl implements MergeContext {
- private BiFunction<Bundle, Bundle, List<Bundle>> bundleResolver;
+ private BiFunction<Bundle, Bundle, List<Bundle>> bundleHandler;
+ private BiFunction<Configuration, Configuration, Configuration> configHandler;
- private MergeContextImpl(BiFunction<Bundle, Bundle, List<Bundle>> bundleResolver) {
- this.bundleResolver = bundleResolver;
+ private MergeContextImpl(BiFunction<Bundle, Bundle, List<Bundle>> bundleHandler,
+ BiFunction<Configuration, Configuration, Configuration> configHandler) {
+ this.bundleHandler = bundleHandler;
+ this.configHandler = configHandler;
}
@Override
public List<Bundle> resolveBundleConflict(Bundle b1, Bundle b2) {
- return bundleResolver.apply(b1, b2);
+ return bundleHandler.apply(b1, b2);
}
@Override
public Configuration resolveConfigurationConflict(Configuration c1, Configuration c2) {
- // TODO Auto-generated method stub
- return null;
+ return configHandler.apply(c1, c2);
}
-
}
}
diff --git a/osgi-featuremodel/src/main/java/org/osgi/feature/impl/FeatureServiceImpl.java b/osgi-featuremodel/src/main/java/org/osgi/feature/impl/FeatureServiceImpl.java
index c30f308..cbd4645 100644
--- a/osgi-featuremodel/src/main/java/org/osgi/feature/impl/FeatureServiceImpl.java
+++ b/osgi-featuremodel/src/main/java/org/osgi/feature/impl/FeatureServiceImpl.java
@@ -18,16 +18,19 @@ package org.osgi.feature.impl;
import org.osgi.feature.ArtifactID;
import org.osgi.feature.Bundle;
+import org.osgi.feature.Configuration;
import org.osgi.feature.Feature;
import org.osgi.feature.FeatureService;
import org.osgi.feature.MergeContext;
import org.osgi.feature.builder.BundleBuilder;
+import org.osgi.feature.builder.ConfigurationBuilder;
import org.osgi.feature.builder.FeatureBuilder;
import java.io.IOException;
import java.io.Reader;
import java.io.Writer;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -58,6 +61,7 @@ public class FeatureServiceImpl implements FeatureService {
builder.setFinal(json.getBoolean("final", false));
builder.addBundles(getBundles(json));
+ builder.addConfigurations(getConfigurations(json));
return builder.build();
}
@@ -70,7 +74,7 @@ public class FeatureServiceImpl implements FeatureService {
if (val.getValueType() == JsonValue.ValueType.OBJECT) {
JsonObject jo = val.asJsonObject();
String bid = jo.getString("id");
- BundleBuilder bbuilder = new BundleBuilder(ArtifactID.fromMavenID(bid));
+ BundleBuilder builder = new BundleBuilder(ArtifactID.fromMavenID(bid));
for (Map.Entry<String, JsonValue> entry : jo.entrySet()) {
if (entry.getKey().equals("id"))
@@ -89,15 +93,67 @@ public class FeatureServiceImpl implements FeatureService {
default:
v = value.toString();
}
- bbuilder.addMetadata(entry.getKey(), v);
+ builder.addMetadata(entry.getKey(), v);
}
- bundles.add(bbuilder.build());
+ bundles.add(builder.build());
}
}
return bundles.toArray(new Bundle[0]);
}
+ private Configuration[] getConfigurations(JsonObject json) {
+ List<Configuration> configs = new ArrayList<>();
+
+ JsonObject jo = json.getJsonObject("configurations");
+ for (Map.Entry<String, JsonValue> entry : jo.entrySet()) {
+
+ String p = entry.getKey();
+ String factoryPid = null;
+ int idx = p.indexOf('~');
+ if (idx > 0) {
+ factoryPid = p.substring(0, idx);
+ p = p.substring(idx + 1);
+ }
+
+ ConfigurationBuilder builder;
+ if (factoryPid == null) {
+ builder = new ConfigurationBuilder(p);
+ } else {
+ builder = new ConfigurationBuilder(factoryPid, p);
+ }
+
+ JsonObject values = entry.getValue().asJsonObject();
+ for (Map.Entry<String, JsonValue> value : values.entrySet()) {
+ JsonValue val = value.getValue();
+
+ Object v;
+ switch (val.getValueType()) {
+ case TRUE:
+ v = true;
+ break;
+ case FALSE:
+ v = false;
+ break;
+ case NUMBER:
+ v = ((JsonNumber) val).longValueExact();
+ break;
+ case STRING:
+ v = ((JsonString) val).getString();
+ break;
+ default:
+ v = val.toString();
+
+ // TODO object types, arrays, and requested type conversions
+ }
+ builder.addValue(value.getKey(), v);
+ }
+ configs.add(builder.build());
+ }
+
+ return configs.toArray(new Configuration[] {});
+ }
+
@Override
public void writeFeature(Feature feature, Writer jsonWriter) throws IOException {
// TODO Auto-generated method stub
@@ -112,24 +168,26 @@ public class FeatureServiceImpl implements FeatureService {
copyAttrs(f1, fb);
copyAttrs(f2, fb);
- List<Bundle> bundles = resolveBundles(f1, f2, ctx);
- fb.addBundles(bundles.toArray(new Bundle[0]));
+ fb.addBundles(resolveBundles(f1, f2, ctx));
+ fb.addConfigurations(resolveConfigs(f1, f2, ctx));
return fb.build();
}
- private List<Bundle> resolveBundles(Feature f1, Feature f2, MergeContext ctx) {
+ private Bundle[] resolveBundles(Feature f1, Feature f2, MergeContext ctx) {
List<Bundle> bundles = new ArrayList<>(f1.getBundles());
List<Bundle> addedBundles = new ArrayList<>();
for (Bundle b : f2.getBundles()) {
ArtifactID bID = b.getID();
+ boolean found = false;
for (Iterator<Bundle> it = bundles.iterator(); it.hasNext(); ) {
Bundle orgb = it.next();
ArtifactID orgID = orgb.getID();
if (bID.getGroupId().equals(orgID.getGroupId()) &&
bID.getArtifactId().equals(orgID.getArtifactId())) {
+ found = true;
List<Bundle> res = new ArrayList<>(ctx.resolveBundleConflict(b, orgb));
if (res.contains(orgb)) {
res.remove(orgb);
@@ -139,19 +197,51 @@ public class FeatureServiceImpl implements FeatureService {
addedBundles.addAll(res);
}
}
+ if (!found) {
+ addedBundles.add(b);
+ }
}
bundles.addAll(addedBundles);
- return bundles;
+ return bundles.toArray(new Bundle[] {});
}
- private void copyAttrs(Feature f, FeatureBuilder fb) {
- fb.setTitle(f.getTitle());
- fb.setDescription(f.getDescription());
- fb.setVendor(f.getVendor());
- fb.setLicense(f.getLicense());
- fb.setLocation(f.getLocation());
+ private Configuration[] resolveConfigs(Feature f1, Feature f2, MergeContext ctx) {
+ Map<String,Configuration> configs = new HashMap<>(f1.getConfigurations());
+ Map<String,Configuration> addConfigs = new HashMap<>();
+
+ for (Map.Entry<String,Configuration> cfgEntry : f2.getConfigurations().entrySet()) {
+ String pid = cfgEntry.getKey();
+ Configuration newCfg = cfgEntry.getValue();
+ Configuration orgCfg = configs.get(pid);
+ if (orgCfg != null) {
+ Configuration resCfg = ctx.resolveConfigurationConflict(orgCfg, newCfg);
+ if (!resCfg.equals(orgCfg)) {
+ configs.remove(pid);
+ addConfigs.put(pid, resCfg);
+ }
+ } else {
+ addConfigs.put(pid, newCfg);
+ }
+ }
+ configs.putAll(addConfigs);
+ return configs.values().toArray(new Configuration[] {});
}
+ private void copyAttrs(Feature f, FeatureBuilder fb) {
+ if (f.getTitle() != null)
+ fb.setTitle(f.getTitle());
+
+ if (f.getDescription() != null)
+ fb.setDescription(f.getDescription());
+
+ if (f.getVendor() != null)
+ fb.setVendor(f.getVendor());
+ if (f.getLicense() != null)
+ fb.setLicense(f.getLicense());
+
+ if (f.getLocation() != null)
+ fb.setLocation(f.getLocation());
+ }
}
diff --git a/osgi-featuremodel/src/test/java/org/osgi/feature/impl/FeatureServiceImplTest.java b/osgi-featuremodel/src/test/java/org/osgi/feature/impl/FeatureServiceImplTest.java
index a18a132..79c922d 100644
--- a/osgi-featuremodel/src/test/java/org/osgi/feature/impl/FeatureServiceImplTest.java
+++ b/osgi-featuremodel/src/test/java/org/osgi/feature/impl/FeatureServiceImplTest.java
@@ -19,10 +19,12 @@ package org.osgi.feature.impl;
import org.junit.Test;
import org.osgi.feature.ArtifactID;
import org.osgi.feature.Bundle;
+import org.osgi.feature.Configuration;
import org.osgi.feature.Feature;
import org.osgi.feature.FeatureService;
import org.osgi.feature.MergeContext;
import org.osgi.feature.builder.BundleBuilder;
+import org.osgi.feature.builder.ConfigurationBuilder;
import org.osgi.feature.builder.MergeContextBuilder;
import java.io.IOException;
@@ -30,7 +32,10 @@ import java.io.InputStreamReader;
import java.io.Reader;
import java.net.URL;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
@@ -82,16 +87,38 @@ public class FeatureServiceImplTest {
}
MergeContext ctx = new MergeContextBuilder()
- .setBundleConflictResolver((b1, b2) -> Arrays.asList(b1, b2))
+ .bundleConflictHandler((b1, b2) -> Arrays.asList(b1, b2))
+ .configConflictHandler((c1, c2) -> new ConfigurationBuilder(c1)
+ .addValues(c2.getValues()).build())
.build();
+
+
ArtifactID tid = new ArtifactID("foo", "bar", "1.2.3");
Feature f3 = fs.mergeFeatures(tid, f1, f2, ctx);
assertEquals(tid, f3.getID());
List<Bundle> bundles = f3.getBundles();
- assertEquals(4, bundles.size());
+ assertEquals(5, bundles.size());
assertTrue(bundles.contains(new BundleBuilder(new ArtifactID("org.slf4j", "slf4j-api", "1.7.29")).build()));
assertTrue(bundles.contains(new BundleBuilder(new ArtifactID("org.slf4j", "slf4j-api", "1.7.30")).build()));
+ assertTrue(bundles.contains(new BundleBuilder(new ArtifactID("org.slf4j", "slf4j-nop", "1.7.30")).build()));
+
+ Map<String, Configuration> configs = f3.getConfigurations();
+ assertEquals(2, configs.size());
+
+ Configuration cfg1 = configs.get("my.factory.pid~name");
+ assertEquals("my.factory.pid~name", cfg1.getPid());
+ assertEquals("my.factory.pid", cfg1.getFactoryPid());
+ assertEquals(Collections.singletonMap("a.value", "yeah"), cfg1.getValues());
+
+ Configuration cfg2 = configs.get("my.pid");
+ assertEquals("my.pid", cfg2.getPid());
+ assertNull(cfg2.getFactoryPid());
+ Map<String,Object> expected = new HashMap<>();
+ expected.put("foo", 5L);
+ expected.put("bar", "toast");
+ expected.put("number:Integer", 7L); // this is wrong TODO
+ assertEquals(expected, cfg2.getValues());
}
}
diff --git a/osgi-featuremodel/src/test/resources/features/test-feature2.json b/osgi-featuremodel/src/test/resources/features/test-feature2.json
index 3c3400c..2747d21 100644
--- a/osgi-featuremodel/src/test/resources/features/test-feature2.json
+++ b/osgi-featuremodel/src/test/resources/features/test-feature2.json
@@ -5,7 +5,10 @@
"bundles" :[
{
"id" : "org.slf4j:slf4j-api:1.7.30"
- }
+ },
+ {
+ "id" : "org.slf4j:slf4j-nop:1.7.30"
+ }
],
"configurations" : {
"my.pid" : {