MergeYaml recipe fileMatcher property is present in 7.x version but removed in 8.x versions
MergeYaml recipe fileMatcher is present in 7.x version but removed in 8.x version causing the recipe changes to apply to all yml files in project and not only to specific files.
I am using
- OpenRewrite recipe bom V2.3.1
- Maven compiler plugin v3.8.0
- rewrite-yaml v8.7.2 and 8.11.1
How are you running OpenRewrite?
Are you using the Maven plugin, Gradle plugin, Moderne CLI, Moderne.io or something else? Maven plugin Is your project a single module or a multi-module project? Single Module Can you share your configuration so that we can rule out any configuration issues?
I am using the Maven plugin, and my project is a single module project.
<plugin>
<groupId>org.openrewrite.maven</groupId>
<artifactId>rewrite-maven-plugin</artifactId>
<version>3.8.0</version>
</plugin>
<dependency>
<groupId>org.openrewrite</groupId>
<artifactId>rewrite-yaml</artifactId>
<scope>compile</scope>
</dependency>
org.openrewrite.yaml.MergeYaml:
key: $
yaml: >
tasks:
preBuild:
- closure: startServer
acceptTheirs: false
fileMatcher: '**/jules.yml'
objectIdentifyingProperty: name
What is the smallest, simplest way to reproduce the problem?
using the declarative approach of the above mentioned recipe in creating our own recipe and compile and run the jar on our projects to will apply the yaml changes to all the yml files in the project structure not only to jules.yml.
What did you expect to see?
We are expecting to revert the changes done to MergeYaml recipe to include back fileMatcher property and apply changes only to those files. Like in above example only to jules.yml in all directories of project.
What did you see instead?
Right now after 8.x versions as the fileMatcher property is removed we see the changes getting applied to all the yml files in the project which is not expected.
What is expected to be done to fix it?
Earlier the code for the 7.11.0 and above version is
@Override
protected TreeVisitor<?, ExecutionContext> getSingleSourceApplicableTest() {
return new HasSourcePath<>(fileMatcher);
}
We need similar code to check which files to have the changes applied.
Are you interested in contributing a fix to OpenRewrite?
No
Hi @vijay-chavakula ! We have indeed removed file matchers from individual recipes are recommend that folks use Preconditions instead:
- https://docs.openrewrite.org/authoring-recipes/recipe-conventions-and-best-practices#use-preconditions
- https://docs.openrewrite.org/reference/yaml-format-reference#preconditions
Preconditions allow you to limit where recipes are applied, similar to how you used fileMatcher before. For that you can use FindSourceFiles as a precondition:
- https://docs.openrewrite.org/recipes/core/findsourcefiles
By using preconditions we no longer have to add fileMatcher options to some of the 1300+ recipes that we maintain, which helps simplify our individual recipe implementations. I hope that context and link to the docs helps!
I'll close this issue as I don't think there's anything to do on our side at the moment, but you can still reply and ask follow up questions below. If it turns out there's something to be done still we can reopen the issue.
hi @timtebeek, I tried in declarative approach for Preconditions.
type: specs.openrewrite.org/v1beta/recipe
name: net.example.recipe.yaml.AddingCassandraEnv
displayName: sample file
description: "sample file"
preconditions:
- org.openrewrite.FindSourceFiles:
filePattern: "**/application.yml"
recipeList:
- org.openrewrite.yaml.MergeYaml:
key: $
yaml: >
node: java
acceptTheirs: false
objectIdentifyingProperty: value
What does above precodition do?
It checks if application.yml is found in project and then if found then it appends the yaml content to all the yml files and not to application.yml file.
@timtebeek can you kindly reopen this one please.
That's surprising and disappointing to hear @vijay-chavakula ; are you able to reproduce that issue with a unit test in MergeYamlTest using org.openrewrite.test.RecipeSpec#recipeFromYaml to add the precondition? That might help us to work on a fix.
We used only declarative approach for this as shown above so there is no unit test for the same, but yes it is reproducible, @timtebeek, can i contribute for this fix? can you guide me what are the steps to contribute for the fix for this issue.
You can start with a unit test similar to this one https://github.com/openrewrite/rewrite/blob/b537267807c1af2948c39c12805741d9fc689c26/rewrite-java-test/src/test/java/org/openrewrite/java/search/HasJavaVersionTest.java#L63-L79
But then place it in MergeYamlTest and use the yaml recipe configuration that you shared above. If you put that in a draft pull request we can step through what's needed to fix your issue.
@timtebeek i have opened a PR for the fix of this issue https://github.com/openrewrite/rewrite/pull/3866 kindly help to review and merge.
@vijay-chavakula Maybe you can use the following test case as a basis to demonstrate the issue.
@Test
void mergeYamlAppliedWhenFileMatchesGivenPattern() {
rewriteRun(
spec -> spec
.recipeFromYaml(
"""
---
type: specs.openrewrite.org/v1beta/recipe
name: test.recipe
displayName: Test Recipe
preconditions:
- org.openrewrite.FindSourceFiles:
filePattern: "**/jules.yml"
recipeList:
- org.openrewrite.yaml.MergeYaml:
key: "$"
yaml: >
node: java
acceptTheirs: false
""",
"test.recipe"
),
yaml(
"""
service: db
host: localhost
""",
"""
service: db
host: localhost
node: java
""",
spec -> spec.path("src/main/resources/jules.yml")
),
yaml(
"""
service: db
host: localhost
""",
spec -> spec.path("src/main/resources/application.yml")
)
);
}
I cannot contribute to this issue, can someone please pick this issue. @knutwannheden above testcase i copied in my local and it failed.
I just ran the above unit test again after pasting it into https://github.com/openrewrite/rewrite/blob/5b1ee6eb9715940665e89f7afcf5aea61720e0aa/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java#L27
That test (now) passes, or perhaps already passed back in January. I'm leaning towards closing this issue until we're able to reproduce the issue. There have been quite some fixes in the past six months that might already show different behavior for you as well. Hope that's the case, but let us know if not; preferably with steps to reproduce.