Skip to content

Refactor Gradle recipes to use model mapping APIs - #8339

Open
KamilPatora wants to merge 5 commits into
openrewrite:mainfrom
KamilPatora:kamil/gradle-model-mapping
Open

Refactor Gradle recipes to use model mapping APIs#8339
KamilPatora wants to merge 5 commits into
openrewrite:mainfrom
KamilPatora:kamil/gradle-model-mapping

Conversation

@KamilPatora

@KamilPatora KamilPatora commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What's changed?

Refactored Gradle dependency recipes to update the Gradle model through GradleProject.mapConfiguration(s) and GradleDependencyConfiguration.mapDependencies().

Updated ChangeDependency, RemoveDependency, ChangeDependencyGroupId, ChangeDependencyArtifactId, ChangeDependencyClassifier, and ChangeManagedDependency.

What's your motivation?

Manual model updates could leave resolved or transitive dependency data stale after a recipe changed or removed a dependency. Using the existing mapping APIs centralizes model invalidation and provides consistent configuration inheritance behavior

Anything in particular you'd like reviewers to focus on?

selected mapping API is appropriate for each recipe, especially configuration inheritance and resolved dependency graph handling.

Anyone you would like to review specifically?

No specific reviewers.

Have you considered any alternatives or workarounds?

Continuing to mutate requested and resolved dependencies manually was considered, but it duplicates synchronization logic and can leave stale model data. Updating only source files would also leave the attached GradleProject inconsistent during subsequent recipe execution.

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@KamilPatora

KamilPatora commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Build passes locally, any idea what went wrong? Nothing useful is logged. Also, those tests took half of the time of full gradle test suite, it's too much i think
image
@timtebeek

edit: ChangeDependencyConcurrencyTest

@KamilPatora

Copy link
Copy Markdown
Contributor Author

What copilot said about failing ChangeDependencyConcurrencyTest:

The test combined mutually incompatible fixture settings.

BUILD_FILE declares a dependency without a version. The marker fixture accurately models it with version = null, meaning it is a managed/versionless dependency.

ChangeDependency.updateGradleModel intentionally skips version selection for a versionless dependency unless overrideManagedVersion is enabled:

!StringUtils.isBlank(gav.getVersion()) || Boolean.TRUE.equals(overrideManagedVersion)

However, the test set overrideManagedVersion = false and then waited for a metadata request made only inside that guarded branch. Thread A therefore correctly never invoked the mocked HTTP sender, never decremented aInsideMappingFunction, and the test failed at the latch assertion before exercising concurrent access.

Setting a requested version would make the latch fire, but would no longer model the versionless Gradle declaration. It also triggers Gradle dependency resolution, requiring the corresponding POM to be stubbed.

The correct fixture keeps the dependency versionless, enables overrideManagedVersion, and mocks both Maven metadata and the resulting POM request. That reaches the intended concurrent metadata-resolution path while preserving a coherent Gradle model.

@timtebeek

Copy link
Copy Markdown
Member

Thanks for the continued work here @KamilPatora ; I'm falling further and further behind in reviews, especially with the increased productivity producing changes, which has not kept pace with our ability to review, polish and merge. I'll hope to get to this soon, or convince colleagues to pitch in as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants