You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2021/06/21 20:53:00 UTC

[sling-org-apache-sling-feature] 01/01: SLING-10513: merge extensions even if there is no target to merge with (use an empty extension in that case) to preserve the feature origin

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

pauls pushed a commit to branch SLING-10513
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature.git

commit 3bd16679883285a9231e64c594c6301183d221f5
Author: Karl Pauls <ka...@gmail.com>
AuthorDate: Mon Jun 21 22:52:40 2021 +0200

    SLING-10513: merge extensions even if there is no target to merge with (use an empty extension in that case) to preserve the feature origin
---
 .../apache/sling/feature/builder/BuilderUtil.java  | 63 ++++++++++++----------
 .../sling/feature/builder/FeatureBuilderTest.java  | 25 +++++++++
 2 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
index 880df90..a6f617d 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -592,40 +592,43 @@ class BuilderUtil {
             final String originKey) {
         switch ( target.getType() ) {
             case TEXT : // simply append
-                target.setText(target.getText() + "\n" + source.getText());
+                target.setText((target.getText() != null && !target.getText().trim().isEmpty() ? (target.getText() + "\n") : "") + source.getText());
                 break;
             case JSON : JsonStructure struct1;
-                try ( final StringReader reader = new StringReader(target.getJSON()) ) {
-                    struct1 = Json.createReader(reader).read();
-                }
-                JsonStructure struct2;
-                try ( final StringReader reader = new StringReader(source.getJSON()) ) {
-                    struct2 = Json.createReader(reader).read();
-                }
+                if (target.getJSON() != null) {
+                    try (final StringReader reader = new StringReader(target.getJSON())) {
+                        struct1 = Json.createReader(reader).read();
+                    }
+                    JsonStructure struct2;
+                    try (final StringReader reader = new StringReader(source.getJSON())) {
+                        struct2 = Json.createReader(reader).read();
+                    }
 
-                if ( struct1.getValueType() != struct2.getValueType() ) {
-                    throw new IllegalStateException("Found different JSON types for extension " + target.getName()
-                        + " : " + struct1.getValueType() + " and " + struct2.getValueType());
-                }
-                if ( struct1.getValueType() == ValueType.ARRAY ) {
-                    final JsonArrayBuilder builder = Json.createArrayBuilder();
+                    if (struct1.getValueType() != struct2.getValueType()) {
+                        throw new IllegalStateException("Found different JSON types for extension " + target.getName()
+                                + " : " + struct1.getValueType() + " and " + struct2.getValueType());
+                    }
+                    if (struct1.getValueType() == ValueType.ARRAY) {
+                        final JsonArrayBuilder builder = Json.createArrayBuilder();
 
-                    Stream.concat(
-                        ((JsonArray) struct1).stream(),
-                        ((JsonArray) struct2).stream()
-                    ).forEachOrdered(builder::add);
+                        Stream.concat(
+                                ((JsonArray) struct1).stream(),
+                                ((JsonArray) struct2).stream()
+                        ).forEachOrdered(builder::add);
 
-                    struct1 = builder.build();
+                        struct1 = builder.build();
+                    } else {
+                        // object is merge
+                        struct1 = merge((JsonObject) struct1, (JsonObject) struct2);
+                    }
+                    StringWriter buffer = new StringWriter();
+                    try (JsonWriter writer = Json.createWriter(buffer)) {
+                        writer.write(struct1);
+                    }
+                    target.setJSON(buffer.toString());
                 } else {
-                    // object is merge
-                    struct1 = merge((JsonObject)struct1, (JsonObject)struct2);
-                }
-                StringWriter buffer = new StringWriter();
-                try (JsonWriter writer = Json.createWriter(buffer))
-                {
-                    writer.write(struct1);
+                    target.setJSON(source.getJSON());
                 }
-                target.setJSON(buffer.toString());
                 break;
 
         case ARTIFACTS:
@@ -679,8 +682,10 @@ class BuilderUtil {
                     }
                 }
                 if ( !handled ) {
-                    // no merge handler, just add a copy
-                    target.getExtensions().add(ext.copy());
+                    // no merge handler, just merge with an empty target
+                    Extension current = new Extension(ext.getType(), ext.getName(), ext.getState());
+                    target.getExtensions().add(current);
+                    mergeExtensions(current, ext, source, artifactOverrides, originKey);
                 }
             }
         }
diff --git a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
index e32ae42..1a50105 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -325,6 +325,31 @@ public class FeatureBuilderTest {
         assertArrayEquals(a.getFeatureOrigins(), b.getFeatureOrigins());
     }
 
+    @Test public void testDistinctOrigionsArtifacts() {
+        Feature a = new Feature(ArtifactId.fromMvnId("g:a:1"));
+        Feature b = new Feature(ArtifactId.fromMvnId("g:b:1"));
+
+        Extension extensionA = new Extension(ExtensionType.ARTIFACTS, Extension.EXTENSION_NAME_CONTENT_PACKAGES, ExtensionState.OPTIONAL);
+        a.getExtensions().add(extensionA);
+
+        Extension extensionB = new Extension(ExtensionType.ARTIFACTS, Extension.EXTENSION_NAME_CONTENT_PACKAGES, ExtensionState.OPTIONAL);
+        b.getExtensions().add(extensionB);
+
+        extensionA.getArtifacts().add(BuilderUtilTest.createBundle("o/a/1.0.0", 10));
+        extensionB.getArtifacts().add(BuilderUtilTest.createBundle("o/b/1.0.0", 8));
+
+        Feature ab = new Feature(ArtifactId.fromMvnId("g:ab:1"));
+        Extension extensionAB = new Extension(ExtensionType.ARTIFACTS, Extension.EXTENSION_NAME_CONTENT_PACKAGES, ExtensionState.OPTIONAL);
+
+        extensionAB.getArtifacts().add(BuilderUtilTest.createBundle("o/a/1.0.0", 10, new AbstractMap.SimpleEntry<>(Artifact.KEY_FEATURE_ORIGINS, a.getId().toMvnId())));
+        extensionAB.getArtifacts().add(BuilderUtilTest.createBundle("o/b/1.0.0", 8, new AbstractMap.SimpleEntry<>(Artifact.KEY_FEATURE_ORIGINS, b.getId().toMvnId())));
+
+        ab.getExtensions().add(extensionAB);
+        Feature assembled = FeatureBuilder.assemble(ArtifactId.fromMvnId("g:ab:1"), new BuilderContext(provider), a, b);
+        assembled.getExtensions().remove(assembled.getExtensions().getByName(Extension.EXTENSION_NAME_ASSEMBLED_FEATURES));
+        equals(ab, assembled);
+    }
+
 
     @Test public void testNoIncludesNoUpgrade() throws Exception {
         final Feature base = new Feature(ArtifactId.parse("org.apache.sling/test-feature/1.1"));