ChangeParentPom fails to update child module MavenResolutionResults

[ryan-hudson]

When running the Spring 4.0 Upgrade recipe, I noticed some unintended behavior when running on multi-module maven projects.

The test case below demonstrates the problem. To describe the test case:

• The project has two modules, sample-child and sample-parent
• sample-child declares sample-parent as its parent, and sample-parent declares spring-boot-starter-parent as its parent
• The Spring upgrade recipe first calls UpgradeParentVersion for the Spring parent, and then calls various other dependency-management recipes which rely on the project using the newer Spring 4.0 version
• But, the child pom does not know that its parent now uses Spring 4.0, and so those subsequent dependency change recipes start misbehaving
• Specifically, spring-boot-starter-webmvc did not exist prior to Spring Boot 4.0, and the child pom which now attempts to use that dependency is given a "version not found" warning comment by OpenRewrite

I've put some thought into how this might be improved, and came up with these observations:

• UpdateMavenModel only ever updates a single pom, specifically the sample-parent in this case containing the Spring parent tag. However, these updates are also relevant to any dependent module resolution results, so the resolution result markers on those dependent modules also need to be updated on a parent update.
• Even if you were to apply UpdateMavenModel on all poms after any pom change, dependent modules would still fail to resolve properly as they will attempt to resolve their local module parent poms from the original version of those poms and not the newly updated version.

Suggested Solution

Since we need to communicate updated model information between multiple source files, I think the only option is to store these updated models within the execution context. These seems best suited to happen within UpdateMavenModel, maybe somewhere around L142-145.
Child module resolutions would then utilize those newer models when retrieving projectPoms. Something like:

try {
    MavenResolutionResult updated = updateResult(ctx, resolutionResult.withPom(resolutionResult.getPom().withRequested(requested)),
            mavenExecutionContext.getProjectPoms()); // <-------
    mavenExeuctionContext.putUpdatedProjectPom(updated); // <-----
    return document.withMarkers(document.getMarkers().computeByType(getResolutionResult(),
            (original, ignored) -> updated));
} catch (MavenDownloadingExceptions e) {
    return e.warn(document);
}

Challenges

• This solution would be contingent on visiting poms in hierarchical order (parents before children), which I thiiiink might already be happening, but I could be wrong.
• When we then visit the child module, we would need to force a model update to retrieve these new parent changes before doing any other scheduled recipe runs.

Example Test Case

@Test
fun `should correctly update child module with webmvc version`() {
    rewriteRun({ spec -> spec.recipeFromYaml(
        """
        type: specs.openrewrite.org/v1beta/recipe
        name: SampleRecipe
        displayName: Sample for testcase
        description: Sample for testcase.
        recipeList:
          - org.openrewrite.maven.UpgradeParentVersion:
              groupId: org.springframework.boot
              artifactId: spring-boot-starter-parent
              newVersion: 4.0.x
          - org.openrewrite.java.dependencies.ChangeDependency:
              oldGroupId: org.springframework.boot
              oldArtifactId: spring-boot-starter-web
              newArtifactId: spring-boot-starter-webmvc
        """, "SampleRecipe"
    )},
        Assertions.mavenProject(
            "sample",
            pomXml(
                """
                <?xml version="1.0" encoding="UTF-8"?>
                <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">

                  <modelVersion>4.0.0</modelVersion>
                  <groupId>com.sample</groupId>
                  <artifactId>sample-parent</artifactId>
                  <version>0.0.0-SNAPSHOT</version>
                  <packaging>pom</packaging>
                  <name>sample</name>
                  
                  <parent>
                    <groupId>org.springframework.boot</groupId>
                    <artifactId>spring-boot-starter-parent</artifactId>
                    <version>3.5.0</version>
                    <relativePath/>
                  </parent>

                  <modules>
                    <module>sample-child</module>
                  </modules>

                  <dependencies>
                    <dependency>
                      <groupId>org.springframework.boot</groupId>
                      <artifactId>spring-boot-starter-web</artifactId>
                    </dependency>
                  </dependencies>
                </project>
            """.trimIndent(), """
            <?xml version="1.0" encoding="UTF-8"?>
            <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                     xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">

              <modelVersion>4.0.0</modelVersion>
              <groupId>com.sample</groupId>
              <artifactId>sample-parent</artifactId>
              <version>0.0.0-SNAPSHOT</version>
              <packaging>pom</packaging>
              <name>sample</name>
              
              <parent>
                <groupId>org.springframework.boot</groupId>
                <artifactId>spring-boot-starter-parent</artifactId>
                <version>4.0.1</version>
                <relativePath/>
              </parent>

              <modules>
                <module>sample-child</module>
              </modules>

              <dependencies>
                <dependency>
                  <groupId>org.springframework.boot</groupId>
                  <artifactId>spring-boot-starter-webmvc</artifactId>
                </dependency>
              </dependencies>
            </project>
            """.trimIndent()) { spec -> spec.path("pom.xml") },
            pomXml(
                """
                <?xml version="1.0" encoding="UTF-8" standalone="no"?>
                <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">

                  <modelVersion>4.0.0</modelVersion>
                  <artifactId>sample-child</artifactId>
                  <packaging>jar</packaging>
                  <name>sample-child</name>

                  <parent>
                    <groupId>com.sample</groupId>
                    <artifactId>sample-parent</artifactId>
                    <version>0.0.0-SNAPSHOT</version>
                    <relativePath>../pom.xml</relativePath>
                  </parent>

                  <dependencies>
                    <dependency>
                      <groupId>org.springframework.boot</groupId>
                      <artifactId>spring-boot-starter-web</artifactId>
                    </dependency>
                  </dependencies>
                </project>
                """.trimIndent(),
                """
                <?xml version="1.0" encoding="UTF-8" standalone="no"?>
                <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">

                  <modelVersion>4.0.0</modelVersion>
                  <artifactId>sample-child</artifactId>
                  <packaging>jar</packaging>
                  <name>sample-child</name>

                  <parent>
                    <groupId>com.sample</groupId>
                    <artifactId>sample-parent</artifactId>
                    <version>0.0.0-SNAPSHOT</version>
                    <relativePath>../pom.xml</relativePath>
                  </parent>

                  <dependencies>
                    <dependency>
                      <groupId>org.springframework.boot</groupId>
                      <artifactId>spring-boot-starter-webmvc</artifactId>
                    </dependency>
                  </dependencies>
                </project>
                """.trimIndent()
            ) { spec -> spec.path("sample-child/pom.xml") }
        )
    )
}

Test Output

org.opentest4j.AssertionFailedError: [Unexpected result in "sample\sample-child\pom.xml":
diff --git a/sample/sample-child/pom.xml b/sample/sample-child/pom.xml
index e359ef6..0316799 100644
--- a/sample/sample-child/pom.xml
+++ b/sample/sample-child/pom.xml
@@ -14,7 +14,7 @@ 
   </parent>
 
   <dependencies>
-    <dependency>
+    <!--~~(No version provided for direct dependency org.springframework.boot:spring-boot-starter-webmvc:compile)~~>--><dependency>
       <groupId>org.springframework.boot</groupId>
       <artifactId>spring-boot-starter-webmvc</artifactId>
     </dependency>
] 

[timtebeek]

Hey thanks for formulating the issue clearly, and helping think through the options. Indeed this is a challenging one to tackle because of the nature of those cascading updates without shared state. We have some guards in place as well against storing information in the context, as we've wanted to avoid such cross-file-contamination of the context. Let me tag @sambsnyd as he likely has some ideas here as well.

[tschut]

Hey, great issue. I ran into the exact same thing this week and was about to create a unit test and open an issue for it, luckily I saw this first :)

In our specific scenario the following happens:

1. Recipe updates the parent pom, which includes

• version management for new dependencies
• updated dependencies, which in turn bring in new transitive dependencies

2. Recipe starts adding dependencies through org.openrewrite.java.dependencies.AddDependency, not specifying a specific version and including acceptTransitive: true
3. We see 2 problems in the added dependencies:

• versions are added for dependencies which also have version management in the new parent pom
• dependencies are added even though they are also included transitively thanks to the new versions in the parent pom

At first we tried adding org.openrewrite.maven.RemoveRedundantDependencyVersions, but since the internal project model isn't updated, this indeed doesn't work.