Repository: decorators-squad/eo-yaml
Pull request #436 (2021-02-08 14:04:57)

#420 Recursive YamlMapping Merge.
- can override conflicting keys
- when the conflicting keys values are sequences and the override conflicts flag is set, they will be merged into one sequence.

Pr for #420


Patch:
diff --git a/src/main/java/com/amihaiemil/eoyaml/extensions/MergedYamlMapping.java b/src/main/java/com/amihaiemil/eoyaml/extensions/MergedYamlMapping.java
index 1533419..77f09b9 100644
--- a/src/main/java/com/amihaiemil/eoyaml/extensions/MergedYamlMapping.java
+++ b/src/main/java/com/amihaiemil/eoyaml/extensions/MergedYamlMapping.java
@@ -142,8 +142,9 @@ public final class MergedYamlMapping extends BaseYamlMapping {
      * @param changed Changed mapping.
      * @param overrideConflicts Should conflicting keys be overriden or not?
      * @return Merged mapping.
+     * @checkstyle HiddenField (500 lines)
      */
-    private static YamlMapping merge(
+    private YamlMapping merge(
         final YamlMapping original,
         final YamlMapping changed,
         final boolean overrideConflicts
@@ -154,26 +155,102 @@ public final class MergedYamlMapping extends BaseYamlMapping {
         } else if (changed == null || changed.keys().isEmpty()) {
             merged = original;
         } else {
-            YamlMappingBuilder builder = Yaml.createYamlMappingBuilder();
-            final Set<YamlNode> changedKeys = changed.keys();
-            for (final YamlNode key : original.keys()) {
-                if(changedKeys.contains(key)) {
-                    if(overrideConflicts) {
-                        builder = builder.add(key, changed.value(key));
+            merged = this.recursiveMerge(original, changed, overrideConflicts);
+        }
+        return merged;
+    }
+
+    /**
+     * Recursively merge to mappings.
+     * @param original Original mapping.
+     * @param changed Changed mapping.
+     * @param overrideConflicts Should conflicting keys be overridden or not?
+     * @return Merged mapping.
+     * @checkstyle CyclomaticComplexity (200 lines)
+     * @checkstyle ExecutableStatementCount (200 lines)
+     */
+    private YamlMapping recursiveMerge(
+        final YamlMapping original,
+        final YamlMapping changed,
+        final boolean overrideConflicts
+    ) {
+        YamlMappingBuilder originalBuilder = this
+            .yamlMappingBuilderFrom(original);
+        final Set<YamlNode> changedKeys = changed.keys();
+        for(final YamlNode key : changedKeys) {
+            final YamlNode originalValue = original.value(key);
+            final YamlNode changedValue = changed.value(key);
+            if (changedValue instanceof YamlMapping
+                && originalValue instanceof YamlMapping) {
+                originalBuilder = originalBuilder.add(
+                    key,
+                    this.recursiveMerge(
+                        (YamlMapping) originalValue,
+                        (YamlMapping) changedValue,
+                        overrideConflicts
+                    )
+                );
+            } else if(overrideConflicts
+                && changedValue instanceof YamlSequence
+                && originalValue instanceof YamlSequence){
+                final YamlSequence originalSeq = (YamlSequence) originalValue;
+                final YamlSequence changedSeq = (YamlSequence) changedValue;
+                YamlSequenceBuilder originalSeqBuilder = this
+                    .yamlSequenceBuilderFrom(originalSeq);
+                for (final YamlNode node : changedSeq.values()) {
+                    if (!originalSeq.values().contains(node)) {
+                        originalSeqBuilder = originalSeqBuilder.add(node);
+                    }
+                }
+                originalBuilder = originalBuilder.add(
+                    key,
+                    originalSeqBuilder.build(originalSeq.comment().value())
+                );
+            } else {
+                final YamlNode newValue;
+                if (originalValue != null) {
+                    if (overrideConflicts) {
+                        newValue = changedValue;
                     } else {
-                        builder = builder.add(key, original.value(key));
+                        newValue = originalValue;
                     }
-                    changedKeys.remove(key);
                 } else {
-                    builder = builder.add(key, original.value(key));
+                    newValue = changedValue;
                 }
+                originalBuilder = originalBuilder.add(key, newValue);
             }
-            for(final YamlNode key : changedKeys) {
-                builder = builder.add(key, changed.value(key));
-            }
-            merged = builder.build(original.comment().value());
         }
-        return merged;
+        return originalBuilder.build(original.comment().value());
+    }
+
+    /**
+     * Create a {@link YamlSequenceBuilder} from a {@link YamlSequence} source.
+     * @param source YamlMapping source.
+     * @return Builder of YamlSequence.
+     */
+    private YamlSequenceBuilder yamlSequenceBuilderFrom(
+        final YamlSequence source
+    ) {
+        YamlSequenceBuilder builder = Yaml.createYamlSequenceBuilder();
+        for (final YamlNode node : source.values()) {
+            builder = builder.add(node);
+        }
+        return builder;
     }
 
+    /**
+     * Create a {@link YamlMappingBuilder} from a {@link YamlMapping} source.
+     * @param source YamlMapping source.
+     * @return Builder of YamlMapping.
+     */
+    private YamlMappingBuilder yamlMappingBuilderFrom(
+        final YamlMapping source
+    ) {
+        YamlMappingBuilder builder = Yaml
+            .createYamlMappingBuilder();
+        for (final YamlNode key : source.keys()) {
+            builder = builder.add(key, source.value(key));
+        }
+        return builder;
+    }
 }


Test patch:
diff --git a/src/test/java/com/amihaiemil/eoyaml/extensions/MergedYamlMappingTest.java b/src/test/java/com/amihaiemil/eoyaml/extensions/MergedYamlMappingTest.java
index 7f2eb64..8ef5221 100644
--- a/src/test/java/com/amihaiemil/eoyaml/extensions/MergedYamlMappingTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/extensions/MergedYamlMappingTest.java
@@ -33,6 +33,8 @@ import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
 import org.junit.Test;
 
+import java.io.IOException;
+
 /**
  * Unit tests for {@link MergedYamlMapping}.
  * @author Mihai Andronache (amihaiemil.com)
@@ -302,4 +304,220 @@ public final class MergedYamlMappingTest {
             Matchers.equalTo(expected.toString())
         );
     }
+
+    /**
+     * Merges two simple mappings. Uses the request issue
+     * <a href="https://github.com/decorators-squad/eo-yaml/issues/420#issuecomment-731776176">example</a>
+     * as test case.
+     * @throws IOException If something goes wrong.
+     */
+    @Test
+    public void mergesSimpleMappings() throws IOException {
+        final YamlMapping original =  Yaml.createYamlInput(
+            new StringBuilder()
+                .append("Parameters:").append(System.lineSeparator())
+                .append("  Param1:").append(System.lineSeparator())
+                .append("    Type: x")
+                .toString()
+        ).readYamlMapping();
+        final YamlMapping changed =Yaml.createYamlInput(
+            new StringBuilder()
+                .append("Parameters:").append(System.lineSeparator())
+                .append("  Param2:").append(System.lineSeparator())
+                .append("    Type: y")
+                .toString()
+        ).readYamlMapping();
+        final YamlMapping merged = new MergedYamlMapping(
+            original, changed
+        );
+        final StringBuilder expected = new StringBuilder();
+        expected
+            .append("Parameters:").append(System.lineSeparator())
+            .append("  Param1:").append(System.lineSeparator())
+            .append("    Type: x").append(System.lineSeparator())
+            .append("  Param2:").append(System.lineSeparator())
+            .append("    Type: y");
+        MatcherAssert.assertThat(
+            merged.toString(),
+            Matchers.equalTo(expected.toString())
+        );
+    }
+
+    /**
+     * Merges two complex mappings with conflict override.
+     * @throws IOException If something goes wrong.
+     */
+    @Test
+    public void mergesComplexMappingsWithOverride() throws IOException {
+        final YamlMapping original = Yaml.createYamlInput(
+            new StringBuilder()
+                .append("Parameters:").append(System.lineSeparator())
+                .append("  Param1:").append(System.lineSeparator())
+                .append("    Type:").append(System.lineSeparator())
+                .append("      Prop1: x").append(System.lineSeparator())
+                .append("      Prop2:").append(System.lineSeparator())
+                .append("        -").append(System.lineSeparator())
+                .append("          foo: 1").append(System.lineSeparator())
+                .append("          bar: 2").append(System.lineSeparator())
+                .append("        - x").append(System.lineSeparator())
+                .append("        - y").append(System.lineSeparator())
+                .toString()
+        ).readYamlMapping();
+        final YamlMapping changed = Yaml.createYamlInput(
+            new StringBuilder()
+                .append("Parameters:").append(System.lineSeparator())
+                .append("  Param1:").append(System.lineSeparator())
+                .append("    Type:").append(System.lineSeparator())
+                .append("      Prop1: changedX").append(System.lineSeparator())
+                .append("      Prop2:").append(System.lineSeparator())
+                .append("        - x").append(System.lineSeparator())
+                .append("        - addedZ").append(System.lineSeparator())
+                .append("  Param2:").append(System.lineSeparator())
+                .append("    Type: y")
+                .toString()
+        ).readYamlMapping();
+        final YamlMapping merged = new MergedYamlMapping(
+            original, changed, true
+        );
+        final StringBuilder expected = new StringBuilder();
+        expected
+            .append("Parameters:").append(System.lineSeparator())
+            .append("  Param1:").append(System.lineSeparator())
+            .append("    Type:").append(System.lineSeparator())
+            .append("      Prop1: changedX").append(System.lineSeparator())
+            .append("      Prop2:").append(System.lineSeparator())
+            .append("        -").append(System.lineSeparator())
+            .append("          foo: 1").append(System.lineSeparator())
+            .append("          bar: 2").append(System.lineSeparator())
+            .append("        - x").append(System.lineSeparator())
+            .append("        - y").append(System.lineSeparator())
+            .append("        - addedZ").append(System.lineSeparator())
+            .append("  Param2:").append(System.lineSeparator())
+            .append("    Type: y");
+        MatcherAssert.assertThat(
+            merged.toString(),
+            Matchers.equalTo(expected.toString())
+        );
+    }
+
+    /**
+     * Merges two complex mappings with no conflict override.
+     * @throws IOException If something goes wrong.
+     */
+    @Test
+    public void mergesComplexMappingsWithNoOverride() throws IOException {
+        final YamlMapping original = Yaml.createYamlInput(
+            new StringBuilder()
+                .append("Parameters:").append(System.lineSeparator())
+                .append("  Param1:").append(System.lineSeparator())
+                .append("    Type:").append(System.lineSeparator())
+                .append("      Prop1: x").append(System.lineSeparator())
+                .append("      Prop2:").append(System.lineSeparator())
+                .append("        -").append(System.lineSeparator())
+                .append("          foo: 1").append(System.lineSeparator())
+                .append("          bar: 2").append(System.lineSeparator())
+                .append("        - x").append(System.lineSeparator())
+                .append("        - y").append(System.lineSeparator())
+                .toString()
+        ).readYamlMapping();
+        final YamlMapping changed = Yaml.createYamlInput(
+            new StringBuilder()
+                .append("Parameters:").append(System.lineSeparator())
+                .append("  Param1:").append(System.lineSeparator())
+                .append("    Type:").append(System.lineSeparator())
+                .append("      Prop1: changedX").append(System.lineSeparator())
+                .append("      Prop2:").append(System.lineSeparator())
+                .append("        - x").append(System.lineSeparator())
+                .append("        - addedZ").append(System.lineSeparator())
+                .append("  Param2:").append(System.lineSeparator())
+                .append("    Type: y")
+                .toString()
+        ).readYamlMapping();
+        final YamlMapping merged = new MergedYamlMapping(
+            original, changed
+        );
+        final StringBuilder expected = new StringBuilder();
+        expected
+            .append("Parameters:").append(System.lineSeparator())
+            .append("  Param1:").append(System.lineSeparator())
+            .append("    Type:").append(System.lineSeparator())
+            .append("      Prop1: x").append(System.lineSeparator())
+            .append("      Prop2:").append(System.lineSeparator())
+            .append("        -").append(System.lineSeparator())
+            .append("          foo: 1").append(System.lineSeparator())
+            .append("          bar: 2").append(System.lineSeparator())
+            .append("        - x").append(System.lineSeparator())
+            .append("        - y").append(System.lineSeparator())
+            .append("  Param2:").append(System.lineSeparator())
+            .append("    Type: y");
+        MatcherAssert.assertThat(
+            merged.toString(),
+            Matchers.equalTo(expected.toString())
+        );
+    }
+
+    /**
+     * It should remove key "Prop3" from Mapping by replacing it with null.
+     * @throws IOException If something goes wrong.
+     */
+    @Test
+    public void shouldRemoveKeyFromComplexMapping() throws IOException{
+        final YamlMapping original = Yaml.createYamlInput(
+            new StringBuilder()
+                .append("Parameters:").append(System.lineSeparator())
+                .append("  Param1:").append(System.lineSeparator())
+                .append("    Type:").append(System.lineSeparator())
+                .append("      Prop1: x").append(System.lineSeparator())
+                .append("      Prop2:").append(System.lineSeparator())
+                .append("        -").append(System.lineSeparator())
+                .append("          foo: 1").append(System.lineSeparator())
+                .append("          bar: 2").append(System.lineSeparator())
+                .append("        - x").append(System.lineSeparator())
+                .append("        - y").append(System.lineSeparator())
+                .append("      Prop3:").append(System.lineSeparator())
+                .append("        foo: 1").append(System.lineSeparator())
+                .append("        bar: 2").append(System.lineSeparator())
+                .append("  Param2:").append(System.lineSeparator())
+                .append("    Type: y")
+                .toString()
+        ).readYamlMapping();
+
+        final YamlMapping keyRemover = Yaml
+            .createYamlMappingBuilder()
+            .add("Parameters", Yaml
+                .createYamlMappingBuilder()
+                .add("Param1", Yaml
+                    .createYamlMappingBuilder()
+                    .add("Type", Yaml
+                        .createYamlMappingBuilder()
+                        .add("Prop3", (String) null)
+                        .build())
+                    .build())
+                .build()
+            ).build();
+
+        final YamlMapping merged = new MergedYamlMapping(
+            original, keyRemover, true
+        );
+
+        final StringBuilder expected = new StringBuilder();
+        expected
+            .append("Parameters:").append(System.lineSeparator())
+            .append("  Param1:").append(System.lineSeparator())
+            .append("    Type:").append(System.lineSeparator())
+            .append("      Prop1: x").append(System.lineSeparator())
+            .append("      Prop2:").append(System.lineSeparator())
+            .append("        -").append(System.lineSeparator())
+            .append("          foo: 1").append(System.lineSeparator())
+            .append("          bar: 2").append(System.lineSeparator())
+            .append("        - x").append(System.lineSeparator())
+            .append("        - y").append(System.lineSeparator())
+            .append("      Prop3: null").append(System.lineSeparator())
+            .append("  Param2:").append(System.lineSeparator())
+            .append("    Type: y");
+        MatcherAssert.assertThat(
+            merged.toString(),
+            Matchers.equalTo(expected.toString())
+        );
+    }
 }