Skip to content

Speed up ModuleHasDependency and RepositoryHasDependency by removing DependencyInsight#176

Merged
timtebeek merged 3 commits into
mainfrom
fix/module-repository-has-dependency-direct-lookup
May 21, 2026
Merged

Speed up ModuleHasDependency and RepositoryHasDependency by removing DependencyInsight#176
timtebeek merged 3 commits into
mainfrom
fix/module-repository-has-dependency-direct-lookup

Conversation

@steve-aom-elliott
Copy link
Copy Markdown
Contributor

@steve-aom-elliott steve-aom-elliott commented Apr 22, 2026

What

Both org.openrewrite.java.dependencies.search.ModuleHasDependency and RepositoryHasDependency ran a fresh DependencyInsight visitor against every source file during scanning, just to answer a binary yes/no question about whether the module contains a given GAV.

Replace the visitor with a direct check against:

  • MavenResolutionResult.findDependencies(groupId, artifactId, scope) for Maven modules, and

  • GradleProject's configurations + ResolvedDependency.findDependency walk for Gradle modules.

  • This mirrors the pattern Sam applied to the single-build-system ModuleHasDependency classes in openrewrite/rewrite (commit 919c9f5 / PR #6664) — these cross-build-system versions weren't updated alongside.

Why

These recipes are used as preconditions in many declarative migration recipes — in the Spring Boot 4 recipe tree alone, ModuleHasDependency is a precondition in 11+ places across mockito.yml, spring-boot-20.yml, spring-boot-25.yml, and spring-cloud-2025.yml.

Allocating a DependencyInsight and running its Gradle + Maven visitors per source file is wasteful when all we need is a single boolean per module. With Sam's fix in place for the single-build-system versions, the cross-build-system wrappers became the remaining hot path.

How

  • ModuleHasDependency#hasDependency(Tree) and RepositoryHasDependency#hasDependency(Tree) now:
    1. Look up MavenResolutionResult on the tree's markers. If present, use findDependencies(g, a, scope) with version-comparator filtering.
    2. Otherwise look up GradleProject and walk GradleDependencyConfiguration#getDirectResolved(), calling ResolvedDependency#findDependency(g, a) to catch matches in the transitive tree.
  • ModuleHasDependency's scanner additionally short-circuits the dependency lookup once the JavaProject is already in the accumulator, avoiding redundant scans across the many source files in a module that has already matched. RepositoryHasDependency retains its existing whole-repo short-circuit via AtomicBoolean.
  • No option signatures or behavior visible to callers changed.

Incidentally fixed: arg-order bug in RepositoryHasDependency

The previous RepositoryHasDependency constructed new DependencyInsight(groupIdPattern, artifactIdPattern, scope, version), but DependencyInsight's signature is (groupId, artifactId, version, scope) — so scope and version were silently swapped. ModuleHasDependency already had the correct order. The direct-lookup rewrite passes both values to the right places, so this bug is fixed by construction.

Tests

  • Full rewrite-java-dependencies:test suite passes.
  • ModuleHasDependencyTest and RepositoryHasDependencyTest cover Maven-only, Gradle-only, mixed multi-module, direct/transitive, and version-glob cases.

Follow-ups (not in this PR)

  • hasDependency(Tree) is byte-for-byte identical in both recipes; worth extracting to a shared package-private helper to prevent drift if scope handling, transitive walking, or version semantics ever change.
  • Add a RepositoryHasDependencyTest case that sets both scope and version to lock in the arg-order fix above.
  • Minor: Semver.validate(version, null).getValue() is re-computed on every hasDependency call though version is a recipe field; could be memoized once per recipe instance.

…DependencyInsight

Both recipes ran a fresh DependencyInsight visitor against every source file during
scanning, which performs exhaustive Gradle + Maven dependency analysis just to answer
a binary yes/no question about whether the module contains a given GAV.

Replace the visitor with a direct check against MavenResolutionResult.findDependencies
for Maven modules, and against GradleProject's configurations + ResolvedDependency.findDependency
for Gradle modules. This mirrors the pattern Sam applied to the single-build-system versions
of ModuleHasDependency in openrewrite/rewrite (commit 919c9f5 / PR #6664).

These recipes are used as preconditions in many declarative migration recipes.
Avoiding the DependencyInsight allocation + traversal per source file substantially
reduces scanner time on repositories with many Java files.
@github-project-automation github-project-automation Bot moved this from In Progress to Ready to Review in OpenRewrite Apr 22, 2026
timtebeek added 2 commits May 21, 2026 09:10
…match

Avoids re-running the per-tree dependency lookup for every source file in a
module after the JavaProject has already been added to the accumulator.
@timtebeek timtebeek merged commit c68d8f7 into main May 21, 2026
1 check passed
@timtebeek timtebeek deleted the fix/module-repository-has-dependency-direct-lookup branch May 21, 2026 07:39
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants