MergeYaml creates duplicate properties in yaml with several documents (separated by "---")

[luisrov]

I am using

• OpenRewrite v2.0.1
• Maven/Gradle plugin v5.2.2
• rewrite-module v8.1.2

I`m using the Maven plugin on my multi-module project.

<plugin>
	<groupId>org.openrewrite.maven</groupId>
	<artifactId>rewrite-maven-plugin</artifactId>
	<version>5.2.2</version>
	<configuration>
		<activeRecipes>
			<recipe>myrecipe</recipe><!-- my recipe invoke MergeYaml -->
		</activeRecipes>
	</configuration>
</plugin>

The org.openrewrite.yaml.MergeYaml recipe creates duplicate properties in yaml files containing multiple documents separated with "---".

Below a unit tests to test the bug:

package com.mycompany.app.yaml

import com.mycompany.app.utils.SourcesTest
import org.junit.jupiter.api.Test
import org.openrewrite.Recipe
import org.openrewrite.yaml.MergeYaml
import org.openrewrite.yaml.YamlParser
import org.openrewrite.yaml.tree.Yaml

class BugMergeYamlTest : SourcesTest<Yaml.Documents> {

    override val parser: YamlParser
        get() = YamlParser()

    override val recipe: Recipe
        get() = MergeYaml("$", """
            app:
              core:
                key2: value02
        """.trimIndent(), false, null)

    @Test
    fun replacePropTest() = assertChanged(
            before = """
            app:
              app.key: app
            ---
            com:
              key1: value1
              key2: value2
              key3: value3
            app:
              core:
                key1: value01
        """.trimIndent(),
            after = """
            app:
              app.key: app
            ---
            com:
              key1: value1
              key2: value2
              key3: value3
            app:
              core:
                key1: value01
                key2: value02
        """
    )
}

What did you expect to see?

app:
  app.key: app
---
com:
  key1: value1
  key2: value2
  key3: value3
app:
  core:
    key1: value01
    key2: value02

What did you see instead?

app:
  app.key: app
  core:
    key2: value02
---
com:
  key1: value1
  key2: value2
  key3: value3
app:
  core:
    key1: value01
    key2: value02

Shouldn't the recipe have an attribute to indicate which document should be modified? Previously it had the fileMatcher attribute, but in this case it would not work either, since both documents are in the same file.

Thank you and greetings.

[timtebeek]

Thanks for the detailed report and provided test @luisrov ! I don't think we've encountered this before; and I'm not sure how we would handle it best. We write our recipes such that they should be safe to apply, and not specific to a particular target file. As such I don't think specifying a numbered document index would be a generic enough approach here. Do you have any other suggestions?

• We are revisiting MergeYaml in no coalesced keys + key separator #3470
  So it might help to keep an eye out there as well.

[adrilo]

Another way to solve this would be to add a generic document selector to the recipe.
If you always want to modify the first document, then use documentSelector: "document-index(0)". If you want to change documents with a given node, then use documentSelector: "has($.foo.bar)".

Obviously, the selector syntax is to be determined, but a similar approach could solve the problem while being generic

[timtebeek]

This issue appears to be fixed. The merge logic in visitDocument() now processes each document independently, and commit 60b7a7f (fixing #6101) improved multi-document YAML handling. A confirmation test (mergeExistingKeyInMultiDocumentYaml) verifying the exact scenario from this issue passes cleanly on the current codebase.