UpgradeDependencyVersion does not update dependency version whose value is defined via a property from the parent POM

[nguyenhoan]

What version of OpenRewrite are you using?

I am using

• Maven plugin v5.28.0
• rewrite-spring v5.7.0

How are you running OpenRewrite?

mvn -U org.openrewrite.maven:rewrite-maven-plugin:5.28.0:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-spring:5.7.0 -Drewrite.activeRecipes=org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_2

I am using the Maven plugin, and my project is a two-module project.

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

Root/Parent

<?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/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>org.example</groupId>
    <artifactId>test</artifactId>
    <version>1.0-SNAPSHOT</version>
    <packaging>pom</packaging>

    <modules>
        <module>test-spring-boot-1</module>
        <module>test-spring-boot-2</module>
    </modules>

    <properties>
        <jdk.version>1.8</jdk.version>
        <maven.compiler.source>1.8</maven.compiler.source>
        <maven.compiler.target>1.8</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <maven.compiler.encoding>UTF-8</maven.compiler.encoding>
        <spring-boot.version>2.1.1.RELEASE</spring-boot.version>
    </properties>

</project>

Module 1

<?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/xsd/maven-4.0.0.xsd">
    <parent>
        <groupId>org.example</groupId>
        <artifactId>test</artifactId>
        <version>1.0-SNAPSHOT</version>
    </parent>
    <modelVersion>4.0.0</modelVersion>

    <artifactId>test-spring-boot-1</artifactId>
    <version>1.0-SNAPSHOT</version>

    <dependencies>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot</artifactId>
            <version>${spring-boot.version}</version>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-autoconfigure</artifactId>
            <version>${spring-boot.version}</version>
        </dependency>
    </dependencies>

</project>

Module 2

<?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/xsd/maven-4.0.0.xsd">
    <parent>
        <groupId>org.example</groupId>
        <artifactId>test</artifactId>
        <version>1.0-SNAPSHOT</version>
    </parent>
    <modelVersion>4.0.0</modelVersion>

    <artifactId>test-spring-boot-2</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <spring-boot.version>2.1.1.RELEASE</spring-boot.version>
    </properties>

    <dependencies>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot</artifactId>
            <version>${spring-boot.version}</version>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-autoconfigure</artifactId>
            <version>${spring-boot.version}</version>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-actuator</artifactId>
            <version>${spring-boot.version}</version>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-configuration-processor</artifactId>
            <version>${spring-boot.version}</version>
        </dependency>
    </dependencies>

</project>

What did you expect to see?

The property spring-boot.version in root/parent POM is updated to 3.2.5.

        <spring-boot.version>3.2.5</spring-boot.version>

What did you see instead?

The property spring-boot.version in root/parent POM is unchanged: 2.1.1.RELEASE. Therefore, Spring Boot in module 1 was not upgraded.

        <spring-boot.version>2.1.1.RELEASE</spring-boot.version>

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

Are you interested in contributing a fix to OpenRewrite?

[nguyenhoan]

@timtebeek could you please take a look and let me know if there’s a plan to support this?
Thanks!

[timtebeek]

Hi @nguyenhoan ; Thanks for the detailed report. Are changes made to the root pom when there is no module 2, just module 1? Or do both modules have to exist before the recipe only applies changes to module 2?

The relevant parts of the recipe are likely here

rewrite/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java

Lines 219 to 223 in ea566dc

	Optional<Xml.Tag> version = t.getChild("version");
	if (version.isPresent()) {
	String requestedVersion = d.getRequested().getVersion();
	if (requestedVersion != null && requestedVersion.startsWith("${") && !implicitlyDefinedVersionProperties.contains(requestedVersion)) {
	doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false).getVisitor());

We'd maybe need to revise the logic a bit if we want to (additionally) bump properties that are in a parent that's part of the same project. Your examples above converted into a unit test could be good step in that direction.

[nguyenhoan]

Thanks for your response!

Are changes made to the root pom when there is no module 2, just module 1? Or do both modules have to exist before the recipe only applies changes to module 2?

Module 2 does not have to be there to reproduce the issue.

Your examples above converted into a unit test could be good step in that direction.

Can you share an example of a test case using parent and child pom.xml files?

[timtebeek]

Can you share an example of a test case using parent and child pom.xml files?

Sure! Here's an example that uses a parent pom.xml and then a child project that uses that parent.

rewrite/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java

Lines 265 to 329 in d6417f2

	void overrideManagedDependency() {
	rewriteRun(
	spec -> spec.recipe(new UpgradeDependencyVersion("com.google.guava", "guava", "14.0", "", true, null)),
	pomXml(
	"""
	<project>
	<groupId>com.mycompany</groupId>
	<artifactId>my-parent</artifactId>
	<version>1</version>
	<dependencyManagement>
	<dependencies>
	<dependency>
	<groupId>com.google.guava</groupId>
	<artifactId>guava</artifactId>
	<version>13.0</version>
	</dependency>
	</dependencies>
	</dependencyManagement>
	</project>
	""",
	SourceSpec::skip
	),
	mavenProject("my-child",
	pomXml(
	"""
	<project>
	<parent>
	<groupId>com.mycompany</groupId>
	<artifactId>my-parent</artifactId>
	<version>1</version>
	</parent>
	<groupId>com.mycompany</groupId>
	<artifactId>my-child</artifactId>
	<version>1</version>
	<dependencies>
	<dependency>
	<groupId>com.google.guava</groupId>
	<artifactId>guava</artifactId>
	</dependency>
	</dependencies>
	</project>
	""",
	"""
	<project>
	<parent>
	<groupId>com.mycompany</groupId>
	<artifactId>my-parent</artifactId>
	<version>1</version>
	</parent>
	<groupId>com.mycompany</groupId>
	<artifactId>my-child</artifactId>
	<version>1</version>
	<dependencies>
	<dependency>
	<groupId>com.google.guava</groupId>
	<artifactId>guava</artifactId>
	<version>14.0</version>
	</dependency>
	</dependencies>
	</project>
	"""
	)
	)
	);
	}

Notice that you'll want to remove the SourceSpec::skip on the parent if you want to see changes there. Thanks for offering to help!

[nguyenhoan]

Thanks for the pointers!
Here is the unit test for that test class

    void upgradeVersionDefinedViaPropertyInLocalParent() {
        rewriteRun(
          spec -> spec.recipe(new UpgradeDependencyVersion("com.google.guava", "guava", "14.0", "", false, null)),
          pomXml(
            """
              <project>
                  <groupId>com.mycompany</groupId>
                  <artifactId>my-parent</artifactId>
                  <version>1</version>
                  <properties>
                    <guava.version>13.0</guava.version>
                  </properties>
              </project>
              """,
            """
              <project>
                  <groupId>com.mycompany</groupId>
                  <artifactId>my-parent</artifactId>
                  <version>1</version>
                  <properties>
                    <guava.version>14.0</guava.version>
                  </properties>
              </project>
              """
          ),
          mavenProject("my-child",
            pomXml(
              """
                <project>
                    <parent>
                        <groupId>com.mycompany</groupId>
                        <artifactId>my-parent</artifactId>
                        <version>1</version>
                    </parent>
                    <groupId>com.mycompany</groupId>
                    <artifactId>my-child</artifactId>
                    <version>1</version>
                    <dependencies>
                        <dependency>
                            <groupId>com.google.guava</groupId>
                            <artifactId>guava</artifactId>
                            <version>${guava.version}</version>
                        </dependency>
                    </dependencies>
                </project>
                """,
              """
                    <project>
                        <parent>
                            <groupId>com.mycompany</groupId>
                            <artifactId>my-parent</artifactId>
                            <version>1</version>
                        </parent>
                        <groupId>com.mycompany</groupId>
                        <artifactId>my-child</artifactId>
                        <version>1</version>
                        <dependencies>
                            <dependency>
                                <groupId>com.google.guava</groupId>
                                <artifactId>guava</artifactId>
                                <version>${guava.version}</version>
                            </dependency>
                        </dependencies>
                    </project>
                """
            )
          )
        );
    }

[timtebeek]

Thanks for reducing this down to a unit test! As an implementation note UpgradeDependencyVersion already extends ScanningRecipe, so we should be able to work across pom.xml files within the same multi-module-project to bump such version properties, provided we correctly account for that in the recipe.

rewrite/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java

Line 51 in 74faca0

public class UpgradeDependencyVersion extends ScanningRecipe<Set<GroupArtifact>> {

[nguyenhoan]

Thanks for the note!

Would you be able to help with the implementation?

I could figure out how to check if a version refers a property not defined in the same POM but I am stuck in finding the way to modify across files.
Do you have any example of such recipe/use case?

[timtebeek]

Sure! Although it might be best to join our Slack to get faster feedback on any questions, also because I'll be traveling for the next two weeks.

In general I can say that Scanning Recipes can collect information across files in an Accumulator, which is then accessible when making changes in a second pass over the files. In this case you'll likely want to keep track of properties and their use, such that when it comes to making changes you can update them in the appropriate file.

[nguyenhoan]

That makes sense. The problem is that UpgradeDependencyVersion extends ScanningRecipe<Set<GroupArtifact>> whose parameter type is Set<GroupArtifact> instead of a custom, extensible type.

public class GroupArtifact implements Serializable {
    String groupId;
    String artifactId;
}

This UpgradeDependencyVersion is used by org.openrewrite.java.dependencies.UpgradeDependencyVersion in rewrite-java-dependencies, so changing the parameter type to a custom type, say UpgradeDependencyVersion.Accumulator would break the rewrite-java-dependencies.

What would be your suggestion?

[timtebeek]

Ah thanks for pointing that out and being cautious to break things downstream. I think in this case it's fine as long as we update both in tandem, as we release all modules in tandem every two weeks or so. You'll have noticed the model for Gradle is already different for Maven, so we should be ok to introduce a custom object here too if we need more than just the Set<GroupArtifact>.

[nguyenhoan]

Sounds good. I will try to make progress from my side in my local workspace.

I am not sure how the build would work when I create two separate PRs for the two repos though.

[timtebeek]

This repository can go first; the change in rewrite-java-dependencies is most likely relatively easy to do comparatively.

[nguyenhoan]

PR to the first repo rewrite.
#4214