You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2021/05/13 13:37:14 UTC

[sling-org-apache-sling-feature-extension-apiregions] 01/02: SLING-10374 : For API behind a toggle allow to specify the version of the previous package

This is an automated email from the ASF dual-hosted git repository.

cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-extension-apiregions.git

commit e1a4e2e28de1659de6badaacdb7e545d27bfcfa8
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Thu May 13 15:35:46 2021 +0200

    SLING-10374 : For API behind a toggle allow to specify the version of the previous package
---
 .../extension/apiregions/api/ApiExport.java        | 162 ++++++++++++++++++++-
 .../extension/apiregions/api/ApiRegions.java       |  74 +---------
 .../extension/apiregions/api/package-info.java     |   2 +-
 .../extension/apiregions/api/ApiRegionsTest.java   |  13 +-
 src/test/resources/json/apis-toggles.json          |   6 +
 5 files changed, 175 insertions(+), 82 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiExport.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiExport.java
index 8aab4cb..818fbde 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiExport.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiExport.java
@@ -45,11 +45,25 @@ public class ApiExport implements Comparable<ApiExport> {
 
     private static final String MEMBERS_KEY = "members";
 
+    private static final String NAME_KEY = "name";
+
+    private static final String TOGGLE_KEY = "toggle";
+
+    private static final String PREVIOUS_KEY = "previous";
+
+    private static final String PREVIOUS_ARTIFACT_ID_KEY = "previous-artifact-id";
+
+    private static final String PREVIOUS_PACKAGE_VERSION_KEY = "previous-package-version";
+
     private final String name;
 
     private String toggle;
 
-    private ArtifactId previous;
+    /** If the package is behind a toggle, this is the previous artifact containing the package not behind a toggle */
+    private ArtifactId previousArtifactId;
+
+    /** If the package is behind a toggle, this is the previous version of the package not behind a toggle */
+    private String previousPackageVersion;
 
     private final Map<String, String> properties = new HashMap<>();
 
@@ -95,21 +109,61 @@ public class ApiExport implements Comparable<ApiExport> {
     }
 
     /**
+     * Get the previous version of this package
+     * @return The previous version of this package or {@code null}
+     * @since 1.2.0
+     */
+    public String getPreviousPackageVersion() {
+        return this.previousPackageVersion;
+    }
+
+    /**
+     * Set the previous version of this package
+     * @param version The previous version of this package 
+     * @since 1.2.0
+     */
+    public void setPreviousPackageVersion(final String version) {
+        this.previousPackageVersion = version;
+    }
+
+    /**
+     * Get the previous artifact id containing the previous version
+     *
+     * @return The previous artifact id or {@code null}
+     * @since 1.2.0
+     */
+    public ArtifactId getPreviousArtifactId() {
+        return previousArtifactId;
+    }
+
+    /**
+     * Set the previous artifact id
+     *
+     * @param previous Previus artifact id
+     * @since 1.2.0
+     */
+    public void setPreviousArtifactId(final ArtifactId previous) {
+        this.previousArtifactId = previous;
+    }
+
+    /**
      * Get the previous version of this api
      *
      * @return The previous version or {@code null}
+     * @deprecated Use {@link #getPreviousArtifactId()}
      */
     public ArtifactId getPrevious() {
-        return previous;
+        return this.getPreviousArtifactId();
     }
 
     /**
      * Set the previous version
      *
      * @param previous Previus version
+     * @deprecated Use {@link #setPreviousArtifactId(ArtifactId)}
      */
-    public void setPrevious(ArtifactId previous) {
-        this.previous = previous;
+    public void setPrevious(final ArtifactId previous) {
+        this.setPreviousArtifactId(previous);
     }
 
     /**
@@ -224,6 +278,97 @@ public class ApiExport implements Comparable<ApiExport> {
         return null;
     }
 
+    JsonValue toJSONValue() {
+        final JsonValue depValue = this.deprecationToJSON();
+        if (this.getToggle() == null 
+            && this.getPreviousPackageVersion() == null 
+            && this.getPreviousArtifactId() == null
+            && this.getProperties().isEmpty() 
+            && depValue == null ) {
+           return Json.createValue(this.getName());
+        }
+        final JsonObjectBuilder expBuilder = Json.createObjectBuilder();
+        expBuilder.add(NAME_KEY, this.getName());
+        if (this.getToggle() != null) {
+            expBuilder.add(TOGGLE_KEY, this.getToggle());
+        }
+        if (this.getPreviousPackageVersion() != null) {
+            expBuilder.add(PREVIOUS_PACKAGE_VERSION_KEY, this.getPreviousPackageVersion());
+        }
+        if (this.getPreviousArtifactId() != null) {
+            expBuilder.add(PREVIOUS_ARTIFACT_ID_KEY, this.getPreviousArtifactId().toMvnId());
+        }
+
+        if ( depValue != null ) {
+            expBuilder.add(DEPRECATED_KEY, depValue);
+        }
+
+        for (final Map.Entry<String, String> entry : this.getProperties().entrySet()) {
+            expBuilder.add(entry.getKey(), entry.getValue());
+        }
+        return expBuilder.build();
+    }
+
+    static ApiExport fromJson(final ApiRegion region, final JsonValue val) throws IOException {
+        if (val.getValueType() == ValueType.STRING) {
+            final String name = ((JsonString) val).getString();
+            if (!name.startsWith("#")) {
+                final ApiExport export = new ApiExport(name);
+                if (!region.add(export)) {
+                    throw new IOException("Export " + export.getName()
+                            + " is defined twice in region " + region.getName());
+                }
+                return export;
+            }
+            return null;
+        } else if (val.getValueType() == ValueType.OBJECT) {
+            final JsonObject expObj = (JsonObject) val;
+            final ApiExport export = new ApiExport(expObj.getString(NAME_KEY));
+            if (!region.add(export)) {
+                throw new IOException("Export " + export.getName() + " is defined twice in region "
+                        + region.getName());
+            }
+
+            boolean setPreviousArtifact = false;
+            for (final String key : expObj.keySet()) {
+                if (NAME_KEY.equals(key)) {
+                    continue; // already set
+
+                } else if (TOGGLE_KEY.equals(key)) {
+                    export.setToggle(expObj.getString(key));
+
+                } else if (PREVIOUS_PACKAGE_VERSION_KEY.equals(key)) {
+                    export.setPreviousPackageVersion(expObj.getString(key));
+                } else if (PREVIOUS_KEY.equals(key)) {
+                    if ( setPreviousArtifact ) {
+                        throw new IOException("Export " + export.getName() + " is defining previous artifact id twice in region "
+                                + region.getName());
+                    }
+                    export.setPreviousArtifactId(ArtifactId.parse(expObj.getString(key)));
+                    setPreviousArtifact = true;
+                } else if (PREVIOUS_ARTIFACT_ID_KEY.equals(key)) {
+                    if ( setPreviousArtifact ) {
+                        throw new IOException("Export " + export.getName() + " is defining previous artifact id twice in region "
+                                + region.getName());
+                    }
+                    export.setPreviousArtifactId(ArtifactId.parse(expObj.getString(key)));
+                    setPreviousArtifact = true;
+
+                } else if ( DEPRECATED_KEY.equals(key)) {
+                    final JsonValue dValue = expObj.get(DEPRECATED_KEY);
+                    export.parseDeprecation(dValue);
+
+                    // everything else is stored as a string property
+                } else {
+                    export.getProperties().put(key, expObj.getString(key));
+                }
+            }
+            return export;
+        } else {
+            throw new IOException("Region " + region.getName() + " has wrong type for package export : " + val.getValueType().name());
+        }
+    }
+
     @Override
     public int compareTo(final ApiExport o) {
         return this.name.compareTo(o.name);
@@ -231,13 +376,13 @@ public class ApiExport implements Comparable<ApiExport> {
 
     @Override
     public String toString() {
-        return "ApiExport [name=" + name + ", toggle=" + toggle + ", previous=" + previous + ", properties="
-                + properties + "]";
+        return "ApiExport [name=" + name + ", toggle=" + toggle + ", previousPackageVersion=" + previousPackageVersion
+                + ", previousArtifactId=" + previousArtifactId + ", properties=" + properties + "]";
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(deprecation, name, previous, properties, toggle);
+        return Objects.hash(deprecation, name, previousPackageVersion, previousArtifactId, properties, toggle);
     }
 
     @Override
@@ -253,7 +398,8 @@ public class ApiExport implements Comparable<ApiExport> {
         }
         ApiExport other = (ApiExport) obj;
         return Objects.equals(deprecation, other.deprecation) && Objects.equals(name, other.name)
-                && Objects.equals(previous, other.previous) && Objects.equals(properties, other.properties)
+                && Objects.equals(previousArtifactId, other.previousArtifactId)
+                && Objects.equals(previousPackageVersion, other.previousPackageVersion) && Objects.equals(properties, other.properties)
                 && Objects.equals(toggle, other.toggle);
     }
 }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
index 492f9cc..c40b98c 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
@@ -61,12 +61,6 @@ public class ApiRegions {
 
     private static final String EXPORTS_KEY = "exports";
 
-    private static final String TOGGLE_KEY = "toggle";
-
-    private static final String PREVIOUS_KEY = "previous";
-
-    private static final String DEPRECATED_KEY = "deprecated";
-
     private final List<ApiRegion> regions = new ArrayList<>();
 
     /**
@@ -195,32 +189,7 @@ public class ApiRegions {
             if (!region.listExports().isEmpty()) {
                 final JsonArrayBuilder expArrayBuilder = Json.createArrayBuilder();
                 for (final ApiExport exp : region.listExports()) {
-                    if (exp.getToggle() == null 
-                        && exp.getPrevious() == null 
-                        && exp.getProperties().isEmpty() 
-                        && exp.getDeprecation().getPackageInfo() == null
-                        && exp.getDeprecation().getMemberInfos().isEmpty() ) {
-                        expArrayBuilder.add(exp.getName());
-                    } else {
-                        final JsonObjectBuilder expBuilder = Json.createObjectBuilder();
-                        expBuilder.add(NAME_KEY, exp.getName());
-                        if (exp.getToggle() != null) {
-                            expBuilder.add(TOGGLE_KEY, exp.getToggle());
-                        }
-                        if (exp.getPrevious() != null) {
-                            expBuilder.add(PREVIOUS_KEY, exp.getPrevious().toMvnId());
-                        }
-
-                        final JsonValue depValue = exp.deprecationToJSON();
-                        if ( depValue != null ) {
-                            expBuilder.add(DEPRECATED_KEY, depValue);
-                        }
-
-                        for (final Map.Entry<String, String> entry : exp.getProperties().entrySet()) {
-                            expBuilder.add(entry.getKey(), entry.getValue());
-                        }
-                        expArrayBuilder.add(expBuilder);
-                    }
+                    expArrayBuilder.add(exp.toJSONValue());
                 }
 
                 regionBuilder.add(EXPORTS_KEY, expArrayBuilder);
@@ -295,46 +264,7 @@ public class ApiRegions {
                         continue; // already set
                     } else if (entry.getKey().equals(EXPORTS_KEY)) {
                         for (final JsonValue e : (JsonArray) entry.getValue()) {
-                            if (e.getValueType() == ValueType.STRING) {
-                                final String name = ((JsonString) e).getString();
-                                if (!name.startsWith("#")) {
-                                    final ApiExport export = new ApiExport(name);
-                                    if (!region.add(export)) {
-                                        throw new IOException("Export " + export.getName()
-                                                + " is defined twice in region " + region.getName());
-                                    }
-                                }
-                            } else if (e.getValueType() == ValueType.OBJECT) {
-                                final JsonObject expObj = (JsonObject) e;
-                                final ApiExport export = new ApiExport(expObj.getString(NAME_KEY));
-                                if (!region.add(export)) {
-                                    throw new IOException("Export " + export.getName() + " is defined twice in region "
-                                            + region.getName());
-                                }
-
-                                for (final String key : expObj.keySet()) {
-                                    if (NAME_KEY.equals(key)) {
-                                        continue; // already set
-
-                                    } else if (TOGGLE_KEY.equals(key)) {
-                                        export.setToggle(expObj.getString(key));
-
-                                    } else if (PREVIOUS_KEY.equals(key)) {
-                                        export.setPrevious(ArtifactId.parse(expObj.getString(key)));
-
-
-                                    } else if ( DEPRECATED_KEY.equals(key)) {
-                                        final JsonValue dValue = expObj.get(DEPRECATED_KEY);
-                                        export.parseDeprecation(dValue);
-
-                                        // everything else is stored as a string property
-                                    } else {
-                                        export.getProperties().put(key, expObj.getString(key));
-                                    }
-                                }
-                            } else {
-                                throw new IOException("Region " + region.getName() + " has wrong type for " + EXPORTS_KEY + " : " + e.getValueType().name());
-                            }
+                            ApiExport.fromJson(region, e);
                         }
                     } else if (entry.getKey().equals(Artifact.KEY_FEATURE_ORIGINS)) {
                         final Set<ArtifactId> origins = new LinkedHashSet<>();
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/package-info.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/package-info.java
index 5c7d69a..95c1c76 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/package-info.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/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.extension.apiregions.api;
 
 
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/ApiRegionsTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/ApiRegionsTest.java
index 58ce56a..fcc88b5 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/ApiRegionsTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/ApiRegionsTest.java
@@ -200,18 +200,29 @@ public class ApiRegionsTest {
         final ApiRegion global = regions.listRegions().get(0);
         assertEquals("global", global.getName());
 
-        assertEquals(2, global.listExports().size());
+        assertEquals(3, global.listExports().size());
 
         final Iterator<ApiExport> iter = global.listExports().iterator();
         final ApiExport exp1 = iter.next();
         assertEquals("org.apache.sling.global", exp1.getName());
         assertEquals("sling_enabled", exp1.getToggle());
         assertNull(exp1.getPrevious());
+        assertNull(exp1.getPreviousArtifactId());
+        assertNull(exp1.getPreviousPackageVersion());
 
         final ApiExport exp2 = iter.next();
         assertEquals("org.apache.felix.global", exp2.getName());
         assertEquals("global_enabled", exp2.getToggle());
         assertEquals(ArtifactId.parse("org.apache.felix:api:1.1"), exp2.getPrevious());
+        assertEquals(ArtifactId.parse("org.apache.felix:api:1.1"), exp2.getPreviousArtifactId());
+        assertNull(exp2.getPreviousPackageVersion());
+
+        final ApiExport exp3 = iter.next();
+        assertEquals("org.apache.felix.global.sub", exp3.getName());
+        assertEquals("global_enabled", exp3.getToggle());
+        assertEquals(ArtifactId.parse("org.apache.felix:api:1.1"), exp3.getPrevious());
+        assertEquals(ArtifactId.parse("org.apache.felix:api:1.1"), exp3.getPreviousArtifactId());
+        assertEquals("1.1", exp3.getPreviousPackageVersion());
 
         // create json and parse
         final ApiRegions regions2 = ApiRegions.parse(regions.toJSONArray());
diff --git a/src/test/resources/json/apis-toggles.json b/src/test/resources/json/apis-toggles.json
index 40baf62..4c48926 100644
--- a/src/test/resources/json/apis-toggles.json
+++ b/src/test/resources/json/apis-toggles.json
@@ -10,6 +10,12 @@
             "name" : "org.apache.felix.global",
             "toggle" : "global_enabled",
             "previous" : "org.apache.felix:api:1.1"
+        },
+        {
+            "name" : "org.apache.felix.global.sub",
+            "toggle" : "global_enabled",
+            "previous-package-version" : "1.1",
+            "previous-artifact-id" : "org.apache.felix:api:1.1"
         }
       ]
     }