openrewrite / rewrite-spring

OpenRewrite recipes for Spring projects.
Apache License 2.0
249 stars 71 forks source link

SpringBootProperties_2_4 - Profile Group config gets corrupted if specific yml indenting #189

Closed nmck257 closed 2 years ago

nmck257 commented 2 years ago

org.openrewrite.java.spring.boot2.SpringBootProperties_2_4 contains the following change:

  - org.openrewrite.properties.ChangePropertyKey:
      oldPropertyKey: spring.profiles
      newPropertyKey: spring.config.activate.on-profile
  - org.openrewrite.yaml.ChangePropertyKey:
      oldPropertyKey: spring.profiles
      newPropertyKey: spring.config.activate.on-profile

...which works as intended w/ Spring's changes to profile-specific documents in 2.4.

Spring Boot 2.4 also introduces Profile Groups, with the following syntax:

spring.profiles.group.prod=proddb,prodmq,prodmetrics

This works fine in most cases, but the recipe above will unintentionally break usages of that new syntax if it's in a yml file with indented hierarchy:

import org.junit.jupiter.api.Test
import org.openrewrite.Recipe
import org.openrewrite.test.RecipeSpec
import org.openrewrite.test.RewriteTest
import org.openrewrite.yaml.ChangePropertyKey

class MyTest : RewriteTest {
    override fun defaults(spec: RecipeSpec) {
        spec.recipe(ChangePropertyKey("spring.profiles", "spring.config.activate.on-profile", null, null))
    }

    @Test
    fun `does not alter spring profile group config on single-line`() = rewriteRun(yaml(
        """
            spring.profiles.group.prod: proddb,prodmq,prodmetrics
        """
    )) // success

    @Test
    fun `does not alter spring profile group config on multi-line`() = rewriteRun(yaml(
        """
            spring:
              profiles:
                group:
                  prod: proddb,prodmq,prodmetrics
        """
    )) // failure -- actual: spring.config.activate.on-profile.group.prod: proddb,prodmq,prodmetrics
}

Repos migrating into 2.4 for the first time will be fine, but existing post-2.4 repos who use this feature and receive this recipe will break (and given how UpgradeSpringBoot recipes are chained together, that's not unlikely to occur).

nmck257 commented 2 years ago

(revised with a test case to demonstrate)

nmck257 commented 2 years ago

@tkvangorder - actually, I think there might be two issues here:

  1. as written originally, this spring recipe calls for a change which would corrupt profile group config
  2. if the yaml recipes are meant to be savvy to dot-separated syntax, then you would actually expect the first test case to fail as well: in other words, ChangePropertyKey("spring.profiles", //... should affect a yml line like spring.profiles.group.prod: proddb,prodmq,prodmetrics

Should I split off a separate issue (on rewrite) for that second item? And in terms of a solution, do you think it's appropriate for ChangePropertyKey to always behave with that dot-separation awareness, or should it be a configurable boolean?

nmck257 commented 2 years ago

Possible duplicate between this and #132

nmck257 commented 2 years ago

Closed by #212