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/19 14:46:31 UTC

[sling-whiteboard] branch master updated: Extension merging

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 81cd791  Extension merging
81cd791 is described below

commit 81cd79176d7d54cf6f246c21a5e71c692d15a203
Author: David Bosschaert <da...@gmail.com>
AuthorDate: Thu Dec 19 14:46:08 2019 +0000

    Extension merging
---
 .../main/java/org/osgi/feature/MergeContext.java   |   2 +
 .../org/osgi/feature/builder/ExtensionBuilder.java |  27 +++---
 .../osgi/feature/builder/MergeContextBuilder.java  |  23 ++++-
 .../org/osgi/feature/impl/FeatureServiceImpl.java  | 102 +++++++++++++--------
 .../osgi/feature/impl/FeatureServiceImplTest.java  |  27 +++++-
 .../src/test/resources/features/test-exfeat1.json  |  23 +++--
 .../src/test/resources/features/test-exfeat2.json  |   9 +-
 7 files changed, 135 insertions(+), 78 deletions(-)

diff --git a/osgi-featuremodel/src/main/java/org/osgi/feature/MergeContext.java b/osgi-featuremodel/src/main/java/org/osgi/feature/MergeContext.java
index 61e221d..8d504e7 100644
--- a/osgi-featuremodel/src/main/java/org/osgi/feature/MergeContext.java
+++ b/osgi-featuremodel/src/main/java/org/osgi/feature/MergeContext.java
@@ -42,4 +42,6 @@ public interface MergeContext {
      * @return The configuration to use.
      */
     Configuration resolveConfigurationConflict(Configuration c1, Configuration c2);
+
+    Extension resolveExtensionConflict(Extension e1, Extension e2);
 }
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 e93fb3e..7864ad5 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
@@ -49,10 +49,11 @@ public class ExtensionBuilder {
         return this;
     }
 
-    public ExtensionBuilder addJSON(String json) {
+    public ExtensionBuilder setJSON(String json) {
         if (type != Type.JSON)
             throw new IllegalStateException("Cannot add text to extension of type " + type);
 
+        content.setLength(0); // Clear any previous value
         content.append(json);
         return this;
     }
@@ -63,16 +64,7 @@ public class ExtensionBuilder {
     }
 
     public ExtensionBuilder addArtifact(String groupId, String artifactId, String version) {
-        if (type != Type.ARTIFACTS)
-            throw new IllegalStateException("Cannot add artifacts to extension of type " + type);
-
-        content.append(groupId);
-        content.append(':');
-        content.append(artifactId);
-        content.append(':');
-        content.append(version);
-        content.append('\n');
-        return this;
+        return addArtifact(groupId, artifactId, version, null, null);
     }
 
     public ExtensionBuilder addArtifact(String groupId, String artifactId, String version, String at, String classifier) {
@@ -84,10 +76,15 @@ public class ExtensionBuilder {
         content.append(artifactId);
         content.append(':');
         content.append(version);
-        content.append(':');
-        content.append(at);
-        content.append(':');
-        content.append(classifier);
+
+        if (at != null) {
+            content.append(':');
+            content.append(at);
+            if (classifier != null) {
+                content.append(':');
+                content.append(classifier);
+            }
+        }
         content.append('\n');
         return this;
     }
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 14f96c6..100d01b 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
@@ -18,6 +18,7 @@ package org.osgi.feature.builder;
 
 import org.osgi.feature.Bundle;
 import org.osgi.feature.Configuration;
+import org.osgi.feature.Extension;
 import org.osgi.feature.MergeContext;
 
 import java.util.List;
@@ -26,6 +27,7 @@ import java.util.function.BiFunction;
 public class MergeContextBuilder {
     private BiFunction<Bundle, Bundle, List<Bundle>> bundleHandler;
     private BiFunction<Configuration, Configuration, Configuration> configHandler;
+    private BiFunction<Extension, Extension, Extension> extensionHandler;
 
     public MergeContextBuilder bundleConflictHandler(BiFunction<Bundle, Bundle, List<Bundle>> bh) {
         bundleHandler = bh;
@@ -37,18 +39,26 @@ public class MergeContextBuilder {
         return this;
     }
 
+    public MergeContextBuilder extensionConflictHandler(BiFunction<Extension, Extension, Extension> eh) {
+        extensionHandler = eh;
+        return this;
+    }
+
     public MergeContext build() {
-        return new MergeContextImpl(bundleHandler, configHandler);
+        return new MergeContextImpl(bundleHandler, configHandler, extensionHandler);
     }
 
     private static class MergeContextImpl implements MergeContext {
-        private BiFunction<Bundle, Bundle, List<Bundle>> bundleHandler;
-        private BiFunction<Configuration, Configuration, Configuration> configHandler;
+        private final BiFunction<Bundle, Bundle, List<Bundle>> bundleHandler;
+        private final BiFunction<Configuration, Configuration, Configuration> configHandler;
+        private final BiFunction<Extension, Extension, Extension> extensionHandler;
 
         private MergeContextImpl(BiFunction<Bundle, Bundle, List<Bundle>> bundleHandler,
-                BiFunction<Configuration, Configuration, Configuration> configHandler) {
+                BiFunction<Configuration, Configuration, Configuration> configHandler,
+                BiFunction<Extension, Extension, Extension> extensionHandler) {
             this.bundleHandler = bundleHandler;
             this.configHandler = configHandler;
+            this.extensionHandler = extensionHandler;
         }
 
         @Override
@@ -60,5 +70,10 @@ public class MergeContextBuilder {
         public Configuration resolveConfigurationConflict(Configuration c1, Configuration c2) {
             return configHandler.apply(c1, c2);
         }
+
+        @Override
+        public Extension resolveExtensionConflict(Extension e1, Extension e2) {
+            return extensionHandler.apply(e1, e2);
+        }
     }
 }
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 b2d23d2..6bbc926 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
@@ -164,49 +164,47 @@ public class FeatureServiceImpl implements FeatureService {
     }
 
     private Extension[] getExtensions(JsonObject json) {
-        JsonArray ja = json.getJsonArray("extensions");
-        if (ja == null)
+        JsonObject jo = json.getJsonObject("extensions");
+        if (jo == null)
             return new Extension[] {};
 
         List<Extension> extensions = new ArrayList<>();
 
-        for (JsonValue ex : ja) {
-            for (Map.Entry<String,JsonValue> entry : ex.asJsonObject().entrySet()) {
-                JsonObject exData = entry.getValue().asJsonObject();
-                Extension.Type type;
-                if (exData.containsKey("text")) {
-                    type = Extension.Type.TEXT;
-                } else if (exData.containsKey("artifacts")) {
-                    type = Extension.Type.ARTIFACTS;
-                } else if (exData.containsKey("json")) {
-                    type = Extension.Type.JSON;
-                } else {
-                    throw new IllegalStateException("Invalid extension: " + entry);
-                }
-                String k = exData.getString("kind", "optional");
-                Extension.Kind kind = Extension.Kind.valueOf(k.toUpperCase());
-
-                ExtensionBuilder builder = new ExtensionBuilder(entry.getKey(), type, kind);
-
-                switch (type) {
-                case TEXT:
-                    builder.addText(exData.getString("text"));
-                    break;
-                case ARTIFACTS:
-                    JsonArray ja2 = exData.getJsonArray("artifacts");
-                    for (JsonValue jv : ja2) {
-                        if (jv.getValueType() == JsonValue.ValueType.STRING) {
-                            String id = ((JsonString) jv).getString();
-                            builder.addArtifact(ArtifactID.fromMavenID(id));
-                        }
+        for (Map.Entry<String,JsonValue> entry : jo.entrySet()) {
+            JsonObject exData = entry.getValue().asJsonObject();
+            Extension.Type type;
+            if (exData.containsKey("text")) {
+                type = Extension.Type.TEXT;
+            } else if (exData.containsKey("artifacts")) {
+                type = Extension.Type.ARTIFACTS;
+            } else if (exData.containsKey("json")) {
+                type = Extension.Type.JSON;
+            } else {
+                throw new IllegalStateException("Invalid extension: " + entry);
+            }
+            String k = exData.getString("kind", "optional");
+            Extension.Kind kind = Extension.Kind.valueOf(k.toUpperCase());
+
+            ExtensionBuilder builder = new ExtensionBuilder(entry.getKey(), type, kind);
+
+            switch (type) {
+            case TEXT:
+                builder.addText(exData.getString("text"));
+                break;
+            case ARTIFACTS:
+                JsonArray ja2 = exData.getJsonArray("artifacts");
+                for (JsonValue jv : ja2) {
+                    if (jv.getValueType() == JsonValue.ValueType.STRING) {
+                        String id = ((JsonString) jv).getString();
+                        builder.addArtifact(ArtifactID.fromMavenID(id));
                     }
-                    break;
-                case JSON:
-                    exData.getJsonObject("json").toString();
-                    break;
                 }
-                extensions.add(builder.build());
+                break;
+            case JSON:
+                builder.setJSON(exData.getJsonObject("json").toString());
+                break;
             }
+            extensions.add(builder.build());
         }
 
         return extensions.toArray(new Extension[] {});
@@ -226,13 +224,14 @@ public class FeatureServiceImpl implements FeatureService {
         copyAttrs(f1, fb);
         copyAttrs(f2, fb);
 
-        fb.addBundles(resolveBundles(f1, f2, ctx));
-        fb.addConfigurations(resolveConfigs(f1, f2, ctx));
+        fb.addBundles(mergeBundles(f1, f2, ctx));
+        fb.addConfigurations(mergeConfigs(f1, f2, ctx));
+        fb.addExtensions(mergeExtensions(f1, f2, ctx));
 
         return fb.build();
     }
 
-    private Bundle[] resolveBundles(Feature f1, Feature f2, MergeContext ctx) {
+    private Bundle[] mergeBundles(Feature f1, Feature f2, MergeContext ctx) {
         List<Bundle> bundles = new ArrayList<>(f1.getBundles());
         List<Bundle> addedBundles = new ArrayList<>();
 
@@ -263,7 +262,7 @@ public class FeatureServiceImpl implements FeatureService {
         return bundles.toArray(new Bundle[] {});
     }
 
-    private Configuration[] resolveConfigs(Feature f1, Feature f2, MergeContext ctx) {
+    private Configuration[] mergeConfigs(Feature f1, Feature f2, MergeContext ctx) {
         Map<String,Configuration> configs = new HashMap<>(f1.getConfigurations());
         Map<String,Configuration> addConfigs = new HashMap<>();
 
@@ -286,6 +285,29 @@ public class FeatureServiceImpl implements FeatureService {
         return configs.values().toArray(new Configuration[] {});
     }
 
+    private Extension[] mergeExtensions(Feature f1, Feature f2, MergeContext ctx) {
+        Map<String,Extension> extensions = new HashMap<>(f1.getExtensions());
+        Map<String,Extension> addExtensions = new HashMap<>();
+
+        for (Map.Entry<String,Extension> exEntry : f2.getExtensions().entrySet()) {
+            String key = exEntry.getKey();
+            Extension newEx = exEntry.getValue();
+            Extension orgEx = extensions.get(key);
+            if (orgEx != null) {
+                Extension resEx = ctx.resolveExtensionConflict(orgEx, newEx);
+                if (!resEx.equals(orgEx)) {
+                    extensions.remove(key);
+                    addExtensions.put(key, resEx);
+                }
+            } else {
+                addExtensions.put(key, newEx);
+            }
+        }
+
+        extensions.putAll(addExtensions);
+        return extensions.values().toArray(new Extension[] {});
+    }
+
     private void copyAttrs(Feature f, FeatureBuilder fb) {
         if (f.getTitle() != null)
             fb.setTitle(f.getTitle());
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 bce29e3..0c28e78 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
@@ -26,6 +26,7 @@ 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.ExtensionBuilder;
 import org.osgi.feature.builder.MergeContextBuilder;
 
 import java.io.IOException;
@@ -139,13 +140,35 @@ public class FeatureServiceImplTest {
             f2 = fs.readFeature(r);
         }
 
-        MergeContext ctx = new MergeContextBuilder().build();
+        MergeContext ctx = new MergeContextBuilder()
+                .extensionConflictHandler((e1, e2) ->
+                    new ExtensionBuilder(e1.getName(), e1.getType(), e1.getKind())
+                        .addText(e1.getText())
+                        .addText(e2.getText())
+                        .build())
+                .build();
 
         ArtifactID tid = new ArtifactID("g", "a", "1.2.3");
         Feature f3 = fs.mergeFeatures(tid, f1, f2, ctx);
 
         Map<String, Extension> extensions = f3.getExtensions();
         assertEquals(3, extensions.size());
-        assertEquals("ABCDEF", extensions.get("my-text-ex").getText());
+        Extension txtEx = extensions.get("my-text-ex");
+        assertEquals("ABCDEF", txtEx.getText());
+        assertEquals(Extension.Kind.OPTIONAL, txtEx.getKind());
+        assertEquals(Extension.Type.TEXT, txtEx.getType());
+
+        Extension artEx = extensions.get("my-art-ex");
+        assertEquals(Extension.Kind.MANDATORY, artEx.getKind());
+        assertEquals(Extension.Type.ARTIFACTS, artEx.getType());
+        List<ArtifactID> artifacts = artEx.getArtifacts();
+        assertEquals(2, artifacts.size());
+        assertTrue(artifacts.contains(new ArtifactID("g", "a", "1")));
+        assertTrue(artifacts.contains(new ArtifactID("g", "a", "2")));
+
+        Extension jsonEx = extensions.get("my-json-ex");
+        assertEquals(Extension.Kind.TRANSIENT, jsonEx.getKind());
+        assertEquals(Extension.Type.JSON, jsonEx.getType());
+        assertEquals("{\"foo\":[1,2,3]}", jsonEx.getJSON());
     }
 }
diff --git a/osgi-featuremodel/src/test/resources/features/test-exfeat1.json b/osgi-featuremodel/src/test/resources/features/test-exfeat1.json
index 4d22abd..6213d87 100644
--- a/osgi-featuremodel/src/test/resources/features/test-exfeat1.json
+++ b/osgi-featuremodel/src/test/resources/features/test-exfeat1.json
@@ -1,18 +1,17 @@
 {
     "id" : "org.apache.sling:test-extension-feature:1.0.0",
-    "extensions" : [
-        { "my-text-ex": {
-          "kind": "optional",
-          "text": "ABC" }
+    "extensions" : {
+        "my-text-ex": {
+            "kind": "optional",
+            "text": "ABC" 
         },
-        { "my-art-ex": {
-          "artifacts": ["g:a:1", "g:a:2"],
-          "kind": "mandatory" 
-          } 
+        "my-art-ex": {
+            "artifacts": ["g:a:1", "g:a:2"],
+            "kind": "mandatory" 
         },
-        { "my-json-ex": {
-          "kind": "transient", 
-          "json": {"foo": [1, 2, 3] }}
+        "my-json-ex": {
+           "kind": "transient", 
+            "json": {"foo": [1, 2, 3] }
         }
-    ]
+    }
 }
\ No newline at end of file
diff --git a/osgi-featuremodel/src/test/resources/features/test-exfeat2.json b/osgi-featuremodel/src/test/resources/features/test-exfeat2.json
index 7db40bb..5fa93e4 100644
--- a/osgi-featuremodel/src/test/resources/features/test-exfeat2.json
+++ b/osgi-featuremodel/src/test/resources/features/test-exfeat2.json
@@ -1,10 +1,9 @@
 {
     "id" : "org.apache.sling:test-extension-feature2:1.0.0",
-    "extensions" : [
-        { "my-text-ex": 
-          {
+    "extensions" : {
+        "my-text-ex": 
+        {
             "text": "DEF"
-          }
         }
-    ]
+    }
 }
\ No newline at end of file