ModuleHasDependency precondition fails to detect dependency in submodule

[wleese]

What version of OpenRewrite are you using?

• OpenRewrite 8.70.2
• Maven plugin 6.26.0

How are you running OpenRewrite?

Maven plugin

Can you share your configuration so that we can rule out any configuration issues?

Running from the commandline:

mvn org.openrewrite.maven:rewrite-maven-plugin:run \
    -Drewrite.configLocation=src/main/resources/test.yaml \
    -Drewrite.activeRecipes=com.xxx.Flyway

With test.yaml being:

---
type: specs.openrewrite.org/v1beta/recipe
name: com.xxx.Flyway
displayName: Perform Flyway migrations
description: Perform Flyway migrations.
preconditions:
- org.openrewrite.java.dependencies.search.ModuleHasDependency:
    groupIdPattern: org.flywaydb
    artifactIdPattern: flyway-database-*
recipeList:
- org.openrewrite.java.dependencies.AddDependency:
    groupId: org.springframework.boot
    artifactId: spring-boot-starter-flyway
    version: 4.0.x

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

See above.

What did you expect to see?

In my multi-module project, I expect the dependency org.flywaydb:flyway-database-postgresql to trigger adding a new dependency org.springframework.boot:spring-boot-starter-flyway

What did you see instead?

No change was made in my multi-module project where the dependency is in a submodule (that is, not the parent pom.xml.
Removing the precondition and the change WAS made.
Moving the dependency from the submodule to the parent pom.xml and the change WAS made.

Note that I also tried using org.openrewrite.maven.search.FindDependency and I experience exactly the same behavior.

I also tried putting this in a unit test, but the test works and does not reflect what I'm seeing when running from the commandline in my actual project:

    @Test
    void preconditionMultiModuleFindDependency() {
        rewriteRun(spec -> spec.recipeFromYaml("""
            ---
            type: specs.openrewrite.org/v1beta/recipe
            name: org.openrewrite.PreconditionTest
            description: Test.
            preconditions:
            - org.openrewrite.maven.search.FindDependency:
                groupId: jakarta.activation
                artifactId: jakarta.activation-api
            recipeList:
            - org.openrewrite.java.dependencies.AddDependency:
                groupId: org.springframework.boot
                artifactId: spring-boot-starter-flyway
                version: 4.0.x
            """, "org.openrewrite.PreconditionTest"),
          pomXml(
            """
              <project>
                <modelVersion>4.0.0</modelVersion>
                <groupId>org.sample</groupId>
                <artifactId>sample</artifactId>
                <version>1.0.0</version>
                <modules>
                  <module>sample-module</module>
                </modules>
              </project>
              """
            ,
            sourceSpecs -> sourceSpecs.path("pom.xml")),
          pomXml(
            """
              <project>
                <modelVersion>4.0.0</modelVersion>
                <groupId>org.sample</groupId>
                <artifactId>sample-module</artifactId>
                <version>1.0.0</version>
                <dependencies>
                  <dependency>
                    <groupId>jakarta.activation</groupId>
                    <artifactId>jakarta.activation-api</artifactId>
                    <version>2.1.2</version>
                  </dependency>
                </dependencies>
              </project>
              """,
            """
              <project>
                <modelVersion>4.0.0</modelVersion>
                <groupId>org.sample</groupId>
                <artifactId>sample-module</artifactId>
                <version>1.0.0</version>
                <dependencies>
                  <dependency>
                    <groupId>jakarta.activation</groupId>
                    <artifactId>jakarta.activation-api</artifactId>
                    <version>2.1.2</version>
                  </dependency>
                  <dependency>
                    <groupId>org.springframework.boot</groupId>
                    <artifactId>spring-boot-starter-flyway</artifactId>
                    <version>4.0.1</version>
                  </dependency>
                </dependencies>
              </project>
              """
            ,
            sourceSpecs -> sourceSpecs.path("sample-module/pom.xml"))
        );
    }

If this doesn't immediately ring a bell then I'll create a demo project.

[timtebeek]

Thanks for the clear report! Frustrating indeed that it does not reproduce with a unit test. I've changed your test slightly but still see it pass:

    @Issue("https://github.com/openrewrite/rewrite/issues/6468")
    @Test
    void preconditionMultiModuleFindDependency() {
        rewriteRun(spec -> spec.recipeFromYaml("""
            type: specs.openrewrite.org/v1beta/recipe
            name: org.openrewrite.PreconditionTest
            description: Test.
            preconditions:
            - org.openrewrite.maven.search.FindDependency:
                groupId: jakarta.activation
                artifactId: jakarta.activation-api
            recipeList:
            - org.openrewrite.maven.AddDependency:
                groupId: org.springframework.boot
                artifactId: spring-boot-starter-flyway
                version: 4.0.x
            """, "org.openrewrite.PreconditionTest"),
          mavenProject("parent",
            pomXml(
              """
                <project>
                  <modelVersion>4.0.0</modelVersion>
                  <groupId>org.sample</groupId>
                  <artifactId>sample</artifactId>
                  <version>1.0.0</version>
                  <modules>
                    <module>sample-module</module>
                  </modules>
                </project>
                """,
              sourceSpecs -> sourceSpecs.path("pom.xml")),
            mavenProject("sample-module",
              pomXml(
                """
                  <project>
                    <modelVersion>4.0.0</modelVersion>
                    <groupId>org.sample</groupId>
                    <artifactId>sample-module</artifactId>
                    <version>1.0.0</version>
                    <dependencies>
                      <dependency>
                        <groupId>jakarta.activation</groupId>
                        <artifactId>jakarta.activation-api</artifactId>
                        <version>2.1.2</version>
                      </dependency>
                    </dependencies>
                  </project>
                  """,
                """
                  <project>
                    <modelVersion>4.0.0</modelVersion>
                    <groupId>org.sample</groupId>
                    <artifactId>sample-module</artifactId>
                    <version>1.0.0</version>
                    <dependencies>
                      <dependency>
                        <groupId>jakarta.activation</groupId>
                        <artifactId>jakarta.activation-api</artifactId>
                        <version>2.1.2</version>
                      </dependency>
                      <dependency>
                        <groupId>org.springframework.boot</groupId>
                        <artifactId>spring-boot-starter-flyway</artifactId>
                        <version>4.0.1</version>
                      </dependency>
                    </dependencies>
                  </project>
                  """
                ,
                sourceSpecs -> sourceSpecs.path("sample-module/pom.xml"))
            )
          )
        );
    }

[wleese]

Demo repo: https://github.com/wleese/repo-openrewrite-issue-6468. Run ./test.sh to reproduce.

I noticed that the multi-module unit test I initially provided wasn't using the root pom.xml as the parent, which did not reflect my situation properly.

This unit test (with pom.xml getting packaging:pom and sample/module/pom.xml getting a parent) now fails with the error Recipe was expected to make a change but made no changes.:

@Test
    void preconditionMultiModuleFindDependency() {
        rewriteRun(spec -> spec.recipeFromYaml("""
            ---
            type: specs.openrewrite.org/v1beta/recipe
            name: org.openrewrite.PreconditionTest
            description: Test.
            preconditions:
            - org.openrewrite.maven.search.FindDependency:
                groupId: jakarta.activation
                artifactId: jakarta.activation-api
            recipeList:
            - org.openrewrite.java.dependencies.AddDependency:
                groupId: org.springframework.boot
                artifactId: spring-boot-starter-flyway
                version: 4.0.x
            """, "org.openrewrite.PreconditionTest"),
          pomXml(
            """
              <project>
                <modelVersion>4.0.0</modelVersion>
                <groupId>org.sample</groupId>
                <artifactId>sample</artifactId>
                <version>1.0.0</version>
                <packaging>pom</packaging>
                <modules>
                  <module>sample-module</module>
                </modules>
              </project>
              """
            ,
            sourceSpecs -> sourceSpecs.path("pom.xml")),
          pomXml(
            """
              <project>
                <modelVersion>4.0.0</modelVersion>
                <parent>
                    <artifactId>sample</artifactId>
                    <version>1.0.0</version>
                    <groupId>org.sample</groupId>
                </parent>
                <artifactId>sample-module</artifactId>
                <dependencies>
                  <dependency>
                    <groupId>jakarta.activation</groupId>
                    <artifactId>jakarta.activation-api</artifactId>
                    <version>2.1.2</version>
                  </dependency>
                </dependencies>
              </project>
              """,
            """
              <project>
                <modelVersion>4.0.0</modelVersion>
                <groupId>org.sample</groupId>
                <artifactId>sample-module</artifactId>
                <version>1.0.0</version>
                <dependencies>
                  <dependency>
                    <groupId>jakarta.activation</groupId>
                    <artifactId>jakarta.activation-api</artifactId>
                    <version>2.1.2</version>
                  </dependency>
                  <dependency>
                    <groupId>org.springframework.boot</groupId>
                    <artifactId>spring-boot-starter-flyway</artifactId>
                    <version>4.0.1</version>
                  </dependency>
                </dependencies>
              </project>
              """
            ,
            sourceSpecs -> sourceSpecs.path("sample-module/pom.xml"))
        );
    }

Debugging a bit, it would seem that BellwetherDecoratedScanningRecipe.getVisitor properly returns AddDependency.
However in RecipeRunCycles.editSources:

                            after = recipeRunStats.recordEdit(recipe, () -> {
                                if (visitor.isAcceptable(source, ctx)) {
                                    // propagate shared root cursor
                                    //noinspection DataFlowIssue
                                    return (SourceFile) visitor.visit(source, ctx, rootCursor);

That return doesn't modify (add the dependency to) the SourceFile, whereas the same unit test where sample-module does not have sample as a parent module, the modification does happen.

The problem seems to occur in the AddDependency class itself, where:

                if (onlyIfUsing == null && isSubprojectOfParentInRepository(acc)) {
                    return maven;
                }

.. will return without applying the requested changes.
On brief examination I could not determine why this check is in place.

Just in case, I do want to note that in our company, we have hundreds of applications that use this kind of parent-child multi-module Maven setup.