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:13 UTC

[sling-org-apache-sling-feature-extension-apiregions] branch master updated (5a11d59 -> 55f6f35)

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

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


    from 5a11d59  [maven-release-plugin] prepare for next development iteration
     new e1a4e2e  SLING-10374 : For API behind a toggle allow to specify the version of the previous package
     new 55f6f35  SLING-10374 : For API behind a toggle allow to specify the version of the previous package

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 docs/api-regions.md                                |   3 +-
 .../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 +
 6 files changed, 177 insertions(+), 83 deletions(-)

[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

Posted by cz...@apache.org.
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"
         }
       ]
     }

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

Posted by cz...@apache.org.
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 55f6f358a673538113977219183e5ddbfd40064f
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Thu May 13 15:37:00 2021 +0200

    SLING-10374 : For API behind a toggle allow to specify the version of the previous package
---
 docs/api-regions.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/api-regions.md b/docs/api-regions.md
index 3d9a215..f288d27 100644
--- a/docs/api-regions.md
+++ b/docs/api-regions.md
@@ -96,7 +96,8 @@ While the above example enables a complete package based on a toggle, it is also
                 {
                     "name" : "org.apache.sling.api",
                     "toggle" : "NEW_SLING_API",
-                    "previous" : "org.apache.sling:org.apache.sling.api:1.1"
+                    "previous-package-version" : "1.0.3",
+                    "previous-artifact-id" : "org.apache.sling:org.apache.sling.api:1.1"
                 }
             ]
         }