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/29 16:12:20 UTC

[sling-org-apache-sling-feature-extension-apiregions] branch master updated: Refactor extension to not use 'org-feature' any more.

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-org-apache-sling-feature-extension-apiregions.git


The following commit(s) were added to refs/heads/master by this push:
     new 3164daf  Refactor extension to not use 'org-feature' any more.
3164daf is described below

commit 3164dafe0392b1f1f6a1daccf32f6d06e9b19a5b
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Thu Nov 29 16:11:51 2018 +0000

    Refactor extension to not use 'org-feature' any more.
---
 .../apiregions/APIRegionMergeHandler.java          | 115 ++++++++++++++-------
 .../extension/apiregions/AbstractHandler.java      |   1 -
 .../apiregions/BundleArtifactFeatureHandler.java   |  11 +-
 .../apiregions/APIRegionMergeHandlerTest.java      |  30 +++---
 .../BundleArtifactFeatureHandlerTest.java          |  25 ++---
 5 files changed, 104 insertions(+), 78 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java b/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java
index 2343c8a..17f032f 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java
@@ -24,13 +24,13 @@ import org.apache.sling.feature.builder.MergeHandler;
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.util.ArrayList;
+import java.util.Collection;
 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;
@@ -39,9 +39,7 @@ import javax.json.stream.JsonGenerator;
 
 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
@@ -59,6 +57,32 @@ public class APIRegionMergeHandler implements MergeHandler {
         JsonReader srcJR = Json.createReader(new StringReader(sourceEx.getJSON()));
         JsonArray srcJA = srcJR.readArray();
 
+        Map<String, Map<String, Object>> srcRegions = new LinkedHashMap<>();
+        for (int i=0; i < srcJA.size(); i++) {
+            String regionName = null;
+            Map<String, Object> region = new LinkedHashMap<>();
+            JsonObject jo = srcJA.getJsonObject(i);
+            for (Map.Entry<String, JsonValue> entry : jo.entrySet()) {
+                Object val;
+                switch (entry.getKey()) {
+                case EXPORTS_KEY:
+                    val = readJsonArray((JsonArray) entry.getValue());
+                    break;
+                default:
+                    val = ((JsonString) entry.getValue()).getString();
+                    if (NAME_KEY.equals(entry.getKey())) {
+                        regionName = val.toString();
+                    }
+                    break;
+                }
+                region.put(entry.getKey(), val);
+            }
+            if (regionName == null) {
+                throw new IllegalStateException("No region name specified: " + sourceEx.getJSON());
+            }
+            srcRegions.put(regionName, region);
+        }
+
         JsonArray tgtJA;
         if (targetEx != null) {
             JsonReader tgtJR = Json.createReader(new StringReader(targetEx.getJSON()));
@@ -74,46 +98,52 @@ public class APIRegionMergeHandler implements MergeHandler {
         JsonGenerator gen = Json.createGenerator(sw);
         gen.writeStartArray();
         for (JsonValue jv : tgtJA) {
-            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());
+            if (jv instanceof JsonObject) {
+                JsonObject jo = (JsonObject) jv;
+                Map<String, Object> srcRegion = srcRegions.remove(jo.getString(NAME_KEY));
+                if (srcRegion != null) {
+                    gen.writeStartObject();
+                    for (Map.Entry<String, JsonValue> entry : jo.entrySet()) {
+                        Object sp = srcRegion.get(entry.getKey());
+                        if (EXPORTS_KEY.equals(entry.getKey()) && sp instanceof List) {
+                            List<String> tgtPkgs = readJsonArray((JsonArray) entry.getValue());
+                            @SuppressWarnings("unchecked")
+                            List<String> srcPkgs = (List<String>) sp;
+                            for (String srcPkg : srcPkgs) {
+                                if (!tgtPkgs.contains(srcPkg)) {
+                                    tgtPkgs.add(srcPkg);
+                                }
+                            }
+                            gen.writeStartArray(entry.getKey());
+                            for (String p : tgtPkgs) {
+                                gen.write(p);
+                            }
+                            gen.writeEnd();
+                        } else {
+                            gen.write(entry.getKey(), entry.getValue());
                         }
                     }
+                    gen.writeEnd();
+                } else {
+                    gen.write(jv);
                 }
+            }
+        }
 
-                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);
+        // If there are any remaining regions in the src extension, process them now
+        for (Map<String, Object> region : srcRegions.values()) {
+            gen.writeStartObject();
+            for (Map.Entry<String, Object> entry : region.entrySet()) {
+                if (entry.getValue() instanceof Collection) {
+                    gen.writeStartArray(entry.getKey());
+                    for (Object o : (Collection<?>) entry.getValue()) {
+                        gen.write(o.toString());
                     }
-                    inheritedPackages.put(name, localExports);
-
-                    JsonArrayBuilder eab = Json.createArrayBuilder();
-                    exports.stream().forEach(e -> eab.add(e));
-                    gen.write(EXPORTS_KEY, eab.build());
-                    exportsWritten = true;
+                    gen.writeEnd();
+                } else {
+                    gen.write(entry.getKey(), entry.getValue().toString());
                 }
             }
-            for (Map.Entry<String, JsonValue> entry : jo.entrySet()) {
-                if (EXPORTS_KEY.equals(entry.getKey()) && exportsWritten)
-                    continue;
-                gen.write(entry.getKey(), entry.getValue());
-            }
             gen.writeEnd();
         }
 
@@ -122,4 +152,17 @@ public class APIRegionMergeHandler implements MergeHandler {
 
         targetEx.setJSON(sw.toString());
     }
+
+    private static List<String> readJsonArray(JsonArray jsonArray) {
+        List<String> l = new ArrayList<>();
+        for (JsonValue jv : jsonArray) {
+            if (jv instanceof JsonString) {
+                String pkg = ((JsonString) jv).getString();
+                if (!pkg.startsWith("#")) { // ignore comment lines
+                    l.add(pkg);
+                }
+            }
+        }
+        return l;
+    }
 }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java b/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java
index 0589a67..a67ceb6 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java
@@ -36,7 +36,6 @@ class AbstractHandler {
 
     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.";
     private static final String FILE_STORAGE_DIR_KEY = "fileStorage";
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java b/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java
index b963d9f..a8b9052 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java
@@ -58,10 +58,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_KEY);
-            if (fid == null)
-                fid = feature.getId().toMvnId().trim();
+            String fid = feature.getId().toMvnId().trim();
 
             String m = map.getProperty(id);
             if (m != null) {
@@ -89,11 +86,7 @@ public class BundleArtifactFeatureHandler extends AbstractHandler implements Pos
         for (JsonValue jv : ja) {
             if (jv instanceof JsonObject) {
                 JsonObject jo = (JsonObject) jv;
-                String fid = null;
-                if (jo.containsKey(ORG_FEATURE_KEY))
-                    fid = jo.getString(ORG_FEATURE_KEY);
-                if (fid == null)
-                    fid = feature.getId().toMvnId();
+                String fid = feature.getId().toMvnId();
 
                 Set<String> regionSet = new HashSet<>();
                 String regions = frMap.getProperty(fid);
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java
index 9fd6860..488df88 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java
@@ -54,28 +54,25 @@ public class APIRegionMergeHandlerTest {
                 + "\"exports\": [\"a.b.c\",\"d.e.f\"]},"
                 + "{\"name\":\"internal\","
                 + "\"exports\":[\"xyz\"],"
-                + "\"org-feature\":\"some:feature:1\"}]");
+                + "\"some-key\":\"some-val\"}]");
 
         Extension srEx = new Extension(ExtensionType.JSON, "api-regions", false);
         srEx.setJSON("[{\"name\":\"global\","
                 + "\"exports\": [\"test\"]},"
                 + "{\"name\":\"something\","
                 + "\"exports\": [\"a.ha\"],"
-                + "\"org-feature\": \"different:feature:1\"}]");
+                + "\"my-key\": \"my-val\"}]");
 
         armh.merge(null, tf, sf, tgEx, srEx);
 
         String expectedJSON = "[{\"name\":\"global\","
-                + "\"exports\": [\"a.b.c\",\"d.e.f\"]},"
+                + "\"exports\": [\"a.b.c\",\"d.e.f\", \"test\"]},"
                 + "{\"name\":\"internal\","
                 + "\"exports\":[\"xyz\"],"
-                + "\"org-feature\":\"some:feature:1\"},"
-                + "{\"name\":\"global\","
-                + "\"org-feature\":\"y:s:2\","
-                + "\"exports\": [\"test\"]},"
+                + "\"some-key\":\"some-val\"},"
                 + "{\"name\":\"something\","
                 + "\"exports\": [\"a.ha\"],"
-                + "\"org-feature\": \"different:feature:1\"}]";
+                + "\"my-key\": \"my-val\"}]";
         JsonReader er = Json.createReader(new StringReader(expectedJSON));
         JsonReader ar = Json.createReader(new StringReader(tgEx.getJSON()));
         JsonArray ea = er.readArray();
@@ -84,6 +81,7 @@ public class APIRegionMergeHandlerTest {
         assertEquals(ea, aa);
     }
 
+
     @Test
     public void testRegionExportsInheritance() throws Exception {
         APIRegionMergeHandler armh = new APIRegionMergeHandler();
@@ -95,24 +93,20 @@ public class APIRegionMergeHandlerTest {
         srEx.setJSON("[{\"name\":\"global\","
                 + "\"exports\": [\"a.b.c\",\"d.e.f\"]},"
                 + "{\"name\":\"deprecated\","
-                + "\"exports\":[\"klm\",\"qrs\"]},"
+                + "\"exports\":[\"klm\",\"#ignored\",\"qrs\"]},"
                 + "{\"name\":\"internal\","
                 + "\"exports\":[\"xyz\"]},"
                 + "{\"name\":\"forbidden\","
-                + "\"exports\":[\"abc\",\"klm\"]},"
-                + "{\"name\":\"internal\","
-                + "\"exports\":[\"test\"],"
-                + "\"org-feature\":\"an.other:feature:123\"}]");
+                + "\"exports\":[\"abc\",\"klm\"]}]");
 
         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\"]}]";
+        String expectedJSON = "[{\"name\":\"global\",\"exports\":[\"a.b.c\",\"d.e.f\"]},"
+                + "{\"name\":\"deprecated\",\"exports\":[\"klm\",\"qrs\"]},"
+                + "{\"name\":\"internal\",\"exports\":[\"xyz\"]},"
+                + "{\"name\":\"forbidden\",\"exports\":[\"abc\",\"klm\"]}]";
         JsonReader er = Json.createReader(new StringReader(expectedJSON));
         JsonReader ar = Json.createReader(new StringReader(tgEx.getJSON()));
         JsonArray ea = er.readArray();
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandlerTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandlerTest.java
index f000a4c..9ff48de 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandlerTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandlerTest.java
@@ -16,15 +16,6 @@
  */
 package org.apache.sling.feature.extension.apiregions;
 
-import static org.junit.Assert.assertEquals;
-
-import java.io.FileReader;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Properties;
-
 import org.apache.sling.feature.Artifact;
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Extension;
@@ -34,6 +25,15 @@ import org.apache.sling.feature.builder.ArtifactProvider;
 import org.apache.sling.feature.builder.HandlerContext;
 import org.junit.Test;
 
+import java.io.FileReader;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
 public class BundleArtifactFeatureHandlerTest {
     @Test
     public void testBundleToFeatureMap() throws Exception {
@@ -43,7 +43,6 @@ public class BundleArtifactFeatureHandlerTest {
         Artifact b1 = new Artifact(ArtifactId.fromMvnId("org.sling:b1:1"));
         Artifact b2 = new Artifact(ArtifactId.fromMvnId("org.sling:b2:1"));
         Artifact b3a = new Artifact(ArtifactId.fromMvnId("org.sling:b3:1"));
-        b3a.getMetadata().put("org-feature", "some.other:feature:123");
         Artifact b3b = new Artifact(ArtifactId.fromMvnId("org.sling:b3:1"));
         f.getBundles().addAll(Arrays.asList(b1, b2, b3a, b3b));
 
@@ -58,7 +57,7 @@ public class BundleArtifactFeatureHandlerTest {
         Properties expected = new Properties();
         expected.put("org.sling:b1:1", "org.sling:something:1.2.3:slingosgifeature:myclassifier");
         expected.put("org.sling:b2:1", "org.sling:something:1.2.3:slingosgifeature:myclassifier");
-        expected.put("org.sling:b3:1", "some.other:feature:123,org.sling:something:1.2.3:slingosgifeature:myclassifier");
+        expected.put("org.sling:b3:1", "org.sling:something:1.2.3:slingosgifeature:myclassifier");
         assertEquals(expected, actual);
     }
 
@@ -73,8 +72,7 @@ public class BundleArtifactFeatureHandlerTest {
                 + "{\"name\":\"internal\","
                 + "\"exports\":[\"xyz\"]},"
                 + "{\"name\":\"global\","
-                + "\"exports\":[\"test\"],"
-                + "\"org-feature\":\"an.other:feature:123\"}]");
+                + "\"exports\":[\"test\"]}]");
 
         bafh.postProcess(new TestHandlerContextImpl(), f, ex);
 
@@ -83,7 +81,6 @@ public class BundleArtifactFeatureHandlerTest {
         actual.load(new FileReader(p));
 
         Properties expected = new Properties();
-        expected.put("an.other:feature:123", "global");
         expected.put("org.sling:something:1.2.3", "internal,global");
 
         String[] al = ((String) actual.remove("org.sling:something:1.2.3")).split(",");