openrewrite / rewrite-spring

OpenRewrite recipes for Spring projects.
Apache License 2.0
265 stars 78 forks source link

ChangeSpringPropertyKey - fails to touch coalesced yaml subproperties #581

Open nmck257 opened 3 months ago

nmck257 commented 3 months ago

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 commented 3 months ago

Hmm, I feel I've dug up some skeletons in this code, particularly looking at this case again: https://github.com/openrewrite/rewrite-spring/blob/912d49d20171b8215c39133596c486fab2f64b10/src/test/java/org/openrewrite/java/spring/ChangeSpringPropertyKeyTest.java#L226-L231

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 commented 3 months ago

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 commented 3 months ago

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