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 2018/11/06 15:19:42 UTC

[sling-whiteboard] branch master updated: Support inheriting exported packages from regions defined earlier in the feature

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 8b9cd1b  Support inheriting exported packages from regions defined earlier in the feature
8b9cd1b is described below

commit 8b9cd1b396447c406cc3412ad503b13974d4508f
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Tue Nov 6 15:12:07 2018 +0000

    Support inheriting exported packages from regions defined earlier in the feature
    
    When exported packages are defined in a region, these are inherited by
    subsequent regions in the feature file. This is with the exception of
    the global region, which is 'special'.
    So if you have:
    api-regions:JSON|false": [
      {"name": "global",
       "exports": ["com.global"]},
      {"name": "deprecated",
       "exports": ["some.package"]},
      {"name": "internal",
       "exports": ["dont.touch"]}]
    Then the actual packages in the regions are as follows:
    global: com.global
    deprecated: some.package
    internal: dont.touch, some.package
---
 .../apiregions/APIRegionMergeHandler.java          | 50 +++++++++++++++++++---
 .../extension/apiregions/AbstractHandler.java      |  7 +++
 .../apiregions/BundleArtifactFeatureHandler.java   | 12 +++---
 .../extension/apiregions/BundleMappingHandler.java |  2 +-
 .../apiregions/APIRegionMergeHandlerTest.java      | 49 +++++++++++++++++++--
 5 files changed, 105 insertions(+), 15 deletions(-)

diff --git a/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java b/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java
index fbd695f..2343c8a 100644
--- a/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java
+++ b/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java
@@ -23,27 +23,37 @@ import org.apache.sling.feature.builder.MergeHandler;
 
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 
 import javax.json.Json;
 import javax.json.JsonArray;
+import javax.json.JsonArrayBuilder;
 import javax.json.JsonObject;
 import javax.json.JsonReader;
+import javax.json.JsonString;
 import javax.json.JsonValue;
 import javax.json.stream.JsonGenerator;
 
-public class APIRegionMergeHandler implements MergeHandler {
+import static org.apache.sling.feature.extension.apiregions.AbstractHandler.API_REGIONS_NAME;
+import static org.apache.sling.feature.extension.apiregions.AbstractHandler.EXPORTS_KEY;
+import static org.apache.sling.feature.extension.apiregions.AbstractHandler.GLOBAL_NAME;
+import static org.apache.sling.feature.extension.apiregions.AbstractHandler.NAME_KEY;
+import static org.apache.sling.feature.extension.apiregions.AbstractHandler.ORG_FEATURE_KEY;
 
+public class APIRegionMergeHandler implements MergeHandler {
     @Override
     public boolean canMerge(Extension extension) {
-        return "api-regions".equals(extension.getName());
+        return API_REGIONS_NAME.equals(extension.getName());
     }
 
     @Override
     public void merge(HandlerContext context, Feature target, Feature source, Extension targetEx, Extension sourceEx) {
-        if (!sourceEx.getName().equals("api-regions"))
+        if (!sourceEx.getName().equals(API_REGIONS_NAME))
             return;
-        if (targetEx != null && !targetEx.getName().equals("api-regions"))
+        if (targetEx != null && !targetEx.getName().equals(API_REGIONS_NAME))
             return;
 
         JsonReader srcJR = Json.createReader(new StringReader(sourceEx.getJSON()));
@@ -67,13 +77,43 @@ public class APIRegionMergeHandler implements MergeHandler {
             gen.write(jv);
         }
 
+        Map<String, List<String>> inheritedPackages = new LinkedHashMap<>(); // keep the insertion order
         for (int i=0; i < srcJA.size(); i++) {
             gen.writeStartObject();
             JsonObject jo = srcJA.getJsonObject(i);
+            boolean exportsWritten = false;
+            if (!jo.containsKey(ORG_FEATURE_KEY)) {
+                gen.write(ORG_FEATURE_KEY, source.getId().toMvnId());
+
+                List<String> exports = new ArrayList<>();
+                if (jo.containsKey(EXPORTS_KEY)) {
+                    JsonArray ja = jo.getJsonArray(EXPORTS_KEY);
+                    for (JsonValue jv : ja) {
+                        if (jv instanceof JsonString) {
+                            exports.add(((JsonString) jv).getString());
+                        }
+                    }
+                }
+
+                String name = jo.getString(NAME_KEY);
+                if (!GLOBAL_NAME.equals(name)) {
+                    ArrayList<String> localExports = new ArrayList<>(exports);
+                    for (Map.Entry<String, List<String>> entry : inheritedPackages.entrySet()) {
+                        entry.getValue().stream().filter(p -> !exports.contains(p)).forEach(exports::add);
+                    }
+                    inheritedPackages.put(name, localExports);
+
+                    JsonArrayBuilder eab = Json.createArrayBuilder();
+                    exports.stream().forEach(e -> eab.add(e));
+                    gen.write(EXPORTS_KEY, eab.build());
+                    exportsWritten = true;
+                }
+            }
             for (Map.Entry<String, JsonValue> entry : jo.entrySet()) {
+                if (EXPORTS_KEY.equals(entry.getKey()) && exportsWritten)
+                    continue;
                 gen.write(entry.getKey(), entry.getValue());
             }
-            gen.write("org-feature", source.getId().toMvnId());
             gen.writeEnd();
         }
 
diff --git a/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java b/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java
index 32321a2..bc19632 100644
--- a/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java
+++ b/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java
@@ -29,6 +29,13 @@ import java.util.Date;
 import java.util.Properties;
 
 class AbstractHandler {
+    static final String API_REGIONS_NAME = "api-regions";
+    static final String GLOBAL_NAME = "global";
+
+    static final String NAME_KEY = "name";
+    static final String EXPORTS_KEY = "exports";
+    static final String ORG_FEATURE_KEY = "org-feature";
+
     private static final String FILE_PREFIX = "apiregions.";
 
     protected File getDataFile(String name) throws IOException {
diff --git a/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java b/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java
index 0b04c4f..014315c 100644
--- a/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java
+++ b/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java
@@ -41,7 +41,7 @@ import javax.json.JsonValue;
 public class BundleArtifactFeatureHandler extends AbstractHandler implements PostProcessHandler {
     @Override
     public void postProcess(HandlerContext context, Feature feature, Extension extension) {
-        if (!"api-regions".equals(extension.getName()))
+        if (!API_REGIONS_NAME.equals(extension.getName()))
             return;
 
         try {
@@ -59,7 +59,7 @@ public class BundleArtifactFeatureHandler extends AbstractHandler implements Pos
         for (Artifact b : feature.getBundles()) {
             String id = b.getId().toMvnId().trim();
 
-            String fid = b.getMetadata().get("org-feature");
+            String fid = b.getMetadata().get(ORG_FEATURE_KEY);
             if (fid == null)
                 fid = feature.getId().toMvnId().trim();
 
@@ -90,8 +90,8 @@ public class BundleArtifactFeatureHandler extends AbstractHandler implements Pos
             if (jv instanceof JsonObject) {
                 JsonObject jo = (JsonObject) jv;
                 String fid = null;
-                if (jo.containsKey("org-feature"))
-                    fid = jo.getString("org-feature");
+                if (jo.containsKey(ORG_FEATURE_KEY))
+                    fid = jo.getString(ORG_FEATURE_KEY);
                 if (fid == null)
                     fid = feature.getId().toMvnId();
 
@@ -100,7 +100,7 @@ public class BundleArtifactFeatureHandler extends AbstractHandler implements Pos
                 if (regions != null) {
                     regionSet.addAll(Arrays.asList(regions.split(",")));
                 }
-                String region = jo.getString("name");
+                String region = jo.getString(NAME_KEY);
                 regionSet.add(region);
 
                 frMap.put(fid, regionSet.stream().collect(Collectors.joining(",")));
@@ -110,7 +110,7 @@ public class BundleArtifactFeatureHandler extends AbstractHandler implements Pos
                 if (packages != null) {
                     packageSet.addAll(Arrays.asList(packages.split(",")));
                 }
-                JsonArray eja = jo.getJsonArray("exports");
+                JsonArray eja = jo.getJsonArray(EXPORTS_KEY);
                 for (int i=0; i < eja.size(); i++) {
                     packageSet.add(eja.getString(i));
                 }
diff --git a/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/BundleMappingHandler.java b/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/BundleMappingHandler.java
index 38896e1..00a9c0a 100644
--- a/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/BundleMappingHandler.java
+++ b/featuremodel/feature-extension-apiregions/src/main/java/org/apache/sling/feature/extension/apiregions/BundleMappingHandler.java
@@ -31,7 +31,7 @@ import java.util.jar.JarFile;
 public class BundleMappingHandler extends AbstractHandler implements PostProcessHandler {
     @Override
     public void postProcess(HandlerContext context, Feature feature, Extension extension) {
-        if (!"api-regions".equals(extension.getName()))
+        if (!API_REGIONS_NAME.equals(extension.getName()))
             return;
 
         try {
diff --git a/featuremodel/feature-extension-apiregions/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java b/featuremodel/feature-extension-apiregions/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java
index db9bcd0..9fd6860 100644
--- a/featuremodel/feature-extension-apiregions/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java
+++ b/featuremodel/feature-extension-apiregions/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java
@@ -43,7 +43,7 @@ public class APIRegionMergeHandlerTest {
     }
 
     @Test
-    public void testAPIRegionMergeHandler() {
+    public void testAPIRegionMerging() {
         APIRegionMergeHandler armh = new APIRegionMergeHandler();
 
         Feature tf = new Feature(ArtifactId.fromMvnId("x:t:1"));
@@ -58,7 +58,10 @@ public class APIRegionMergeHandlerTest {
 
         Extension srEx = new Extension(ExtensionType.JSON, "api-regions", false);
         srEx.setJSON("[{\"name\":\"global\","
-                + "\"exports\": [\"test\"]}]");
+                + "\"exports\": [\"test\"]},"
+                + "{\"name\":\"something\","
+                + "\"exports\": [\"a.ha\"],"
+                + "\"org-feature\": \"different:feature:1\"}]");
 
         armh.merge(null, tf, sf, tgEx, srEx);
 
@@ -69,7 +72,47 @@ public class APIRegionMergeHandlerTest {
                 + "\"org-feature\":\"some:feature:1\"},"
                 + "{\"name\":\"global\","
                 + "\"org-feature\":\"y:s:2\","
-                + "\"exports\": [\"test\"]}]";
+                + "\"exports\": [\"test\"]},"
+                + "{\"name\":\"something\","
+                + "\"exports\": [\"a.ha\"],"
+                + "\"org-feature\": \"different:feature:1\"}]";
+        JsonReader er = Json.createReader(new StringReader(expectedJSON));
+        JsonReader ar = Json.createReader(new StringReader(tgEx.getJSON()));
+        JsonArray ea = er.readArray();
+        JsonArray aa = ar.readArray();
+
+        assertEquals(ea, aa);
+    }
+
+    @Test
+    public void testRegionExportsInheritance() throws Exception {
+        APIRegionMergeHandler armh = new APIRegionMergeHandler();
+
+        Feature tf = new Feature(ArtifactId.fromMvnId("x:t:1"));
+        Feature sf = new Feature(ArtifactId.fromMvnId("y:s:2"));
+
+        Extension srEx = new Extension(ExtensionType.JSON, "api-regions", false);
+        srEx.setJSON("[{\"name\":\"global\","
+                + "\"exports\": [\"a.b.c\",\"d.e.f\"]},"
+                + "{\"name\":\"deprecated\","
+                + "\"exports\":[\"klm\",\"qrs\"]},"
+                + "{\"name\":\"internal\","
+                + "\"exports\":[\"xyz\"]},"
+                + "{\"name\":\"forbidden\","
+                + "\"exports\":[\"abc\",\"klm\"]},"
+                + "{\"name\":\"internal\","
+                + "\"exports\":[\"test\"],"
+                + "\"org-feature\":\"an.other:feature:123\"}]");
+
+        armh.merge(null, tf, sf, null, srEx);
+
+        Extension tgEx = tf.getExtensions().iterator().next();
+
+        String expectedJSON = "[{\"name\":\"global\",\"org-feature\":\"y:s:2\",\"exports\":[\"a.b.c\",\"d.e.f\"]},"
+                + "{\"name\":\"deprecated\",\"org-feature\":\"y:s:2\",\"exports\":[\"klm\",\"qrs\"]},"
+                + "{\"name\":\"internal\",\"org-feature\":\"y:s:2\",\"exports\":[\"xyz\",\"klm\",\"qrs\"]},"
+                + "{\"name\":\"forbidden\",\"org-feature\":\"y:s:2\",\"exports\":[\"abc\",\"klm\",\"qrs\",\"xyz\"]},"
+                + "{\"name\":\"internal\",\"org-feature\":\"an.other:feature:123\",\"exports\":[\"test\"]}]";
         JsonReader er = Json.createReader(new StringReader(expectedJSON));
         JsonReader ar = Json.createReader(new StringReader(tgEx.getJSON()));
         JsonArray ea = er.readArray();