Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MergeYaml should avoid adding query with no result as key at end of file #4297

Merged
merged 10 commits into from
Jul 2, 2024

Conversation

adastraperangusta
Copy link
Contributor

@adastraperangusta adastraperangusta commented Jun 30, 2024

In case the jsonpath is a query which yelds no result, avoid merging the yaml snippet at end of file with the query as a key.

What's changed?

In case the jsonpath has no match in a given yaml file, the MergeYaml recipe appends the yaml snippet at the end of the file under the jsonpath used as a key.
When this key is a JsonPath query using function, this creates an invalid key.

What's your motivation?

Applying MergeYaml on yaml files which don't match the query adds an extra block which is not needed.
Checking this type of key seems easier than using "preconditions"

Any additional context

Maybe the MergeYaml recipe should have an optional "mergeIfKeyNotFound" property set to true by default.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

in case the jsonpath is a query which yelds no result,
avoid merging the yaml at end of file with the query as a key
@timtebeek timtebeek changed the title [MergeYaml recipe] avoid adding query with no result as key at end of file MergeYaml should avoid adding query with no result as key at end of file Jul 1, 2024
@timtebeek timtebeek added the bug Something isn't working label Jul 1, 2024
@timtebeek timtebeek self-requested a review July 1, 2024 08:43
ZhyliaievD and others added 9 commits July 2, 2024 08:40
)

* Fix StackOverflowError in case of circular import deps

Check if import dependency was already resolved before

* Format test, remove unnecessary paths and add issue link

* Replace Streams with for loop as per convention

To limit object allocations

* Swap argument order for consistency

* Verify merged managed dependency version

---------

Co-authored-by: Daniil Zhyliaiev <dzhyliaiev@playtika.com>
Co-authored-by: Tim te Beek <tim@moderne.io>
…ontexts where services.gradle.org is unavailable
…lectors, when services.gradle.org cannot be reached
Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the immediate fix @adastraperangusta ! Nice to see a test that was previously disabled now passes too.

We expect to do a release either today or tomorrow, but until then you can use our snapshot versions.

@timtebeek timtebeek merged commit 897f701 into openrewrite:main Jul 2, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants