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

ChangeSpringPropertyKey - fails to touch coalesced yaml subproperties #581

Open
nmck257 opened this issue Aug 27, 2024 · 3 comments
Open
Labels
bug Something isn't working

Comments

@nmck257
Copy link
Collaborator

nmck257 commented Aug 27, 2024

What version of OpenRewrite are you using?

I am using main

How are you running OpenRewrite?

unit tests

What is the smallest, simplest way to reproduce the problem?

edited this existing test case to include a third file of coalesced yaml properties:

    @Test
    void subproperties() {
        rewriteRun(
          spec -> spec.recipe(new ChangeSpringPropertyKey("spring.resources", "spring.web.resources", null)),
          //language=properties
          properties(
            """
              spring.resources.chain.strategy.content.enabled= true
              spring.resources.chain.strategy.content.paths= /foo/**, /bar/**
              """,
            """
              spring.web.resources.chain.strategy.content.enabled= true
              spring.web.resources.chain.strategy.content.paths= /foo/**, /bar/**
              """
          ),
          //language=yaml
          yaml(
            """
              spring:
                resources:
                  chain:
                    strategy:
                      content:
                        enabled: true
                        paths:
                          - /foo/**
                          - /bar/**
                """,
            """
              spring:
                web.resources:
                  chain:
                    strategy:
                      content:
                        enabled: true
                        paths:
                          - /foo/**
                          - /bar/**
                """
          ),
          //language=yaml
          yaml(
            """
              spring.resouces.chain.strategy.content.enabled: true
              spring.resouces.chain.strategy.content.paths:
                - /foo/**
                - /bar/**
                """,
            """
              spring.web.resouces.chain.strategy.content.enabled: true
              spring.web.resouces.chain.strategy.content.paths:
                - /foo/**
                - /bar/**
                """
          )
        );
    }

What did you expect to see?

success

What did you see instead?

failure; no changes to that new third file

What is the full stack trace of any errors you encountered?

n/a

Are you interested in contributing a fix to OpenRewrite?

I've got some free time this week and will probably play with it, yeah

@nmck257
Copy link
Collaborator Author

nmck257 commented Aug 27, 2024

Hmm, I feel I've dug up some skeletons in this code, particularly looking at this case again:

@Test
@Disabled
@Issue("https://github.com/openrewrite/rewrite-spring/issues/436")
void loggingFileSubpropertiesYaml() {
rewriteRun(
spec -> spec.recipeFromResource("/META-INF/rewrite/spring-boot-22-properties.yml", "org.openrewrite.java.spring.boot2.SpringBootProperties_2_2"),

The except: [ .+ ] trick works for properties files, but not yaml files, because ChangeSpringPropertyKey handles except as a regex for properties but as glob for yaml. A tidier solution might be just adding a subproperties: true|false arg; I think that might more-directly handle cases where spring took an existing property key (with scalar value) and repurposed it to accept a mapping value in future versions. Though that change would span across a few repos since yaml/ChangePropertyKey is in rewrite, not rewrite-spring.

I also recognize that I was the one who added the except construct in the first place, and am wondering if I just fully over-engineered it for flexibility that isn't actually needed :)

Have we considered moving yaml/ChangePropertyKey into rewrite-spring, since it targets that dot-separated syntax which is specific to Spring? Or, I forget -- do other frameworks like quarkus or micronaut borrow that syntax / recipes?

@timtebeek
Copy link
Contributor

There's indeed a bit of unfortunate "here be dragons" to that ChangePropertyKey. From a quick search it appears that rewrite-spring is unique in using the yaml ChangePropertyKey. The properties variant is used by Quarkus and Micronaut as well.

@nmck257
Copy link
Collaborator Author

nmck257 commented Aug 27, 2024

hm, I see in those search results rewrite-docs pages describing Quarkus recipes which use that yaml variant?
and clicking through there, I found this?
https://github.com/quarkusio/quarkus-updates/blob/2415504af33e5a79ed038bc0674b12fd6365898f/recipes/src/main/resources/quarkus-updates/core/3.3.yaml#L24

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
Status: Backlog
Development

No branches or pull requests

2 participants