Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix AddManagedDependency when using properties on the GAV #3992

Merged
merged 3 commits into from
Feb 11, 2024
Merged

Fix AddManagedDependency when using properties on the GAV #3992

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a1492b
Failing test
ammachado Feb 11, 2024
6052172
AddManagedDependency fix
ammachado Feb 11, 2024
6e812ef
Fixes from review
ammachado Feb 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions rewrite-core/src/main/java/org/openrewrite/internal/PropertyPlaceholderHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ public PropertyPlaceholderHelper(String placeholderPrefix, String placeholderSuf
this.valueSeparator = valueSeparator;
}

public boolean hasPlaceholders(@Nullable String value) {
if (value == null) {
return false;
}
int startIndex = value.indexOf(placeholderPrefix);
return startIndex > -1 && value.indexOf(placeholderSuffix, startIndex) > startIndex;
}

public String replacePlaceholders(String value, final Properties properties) {
return replacePlaceholders(value, properties::getProperty);
}
Expand Down
18 changes: 13 additions & 5 deletions rewrite-maven/src/main/java/org/openrewrite/maven/AddManagedDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.openrewrite.maven.tree.MavenMetadata;
import org.openrewrite.maven.tree.MavenResolutionResult;
import org.openrewrite.maven.tree.ResolvedDependency;
import org.openrewrite.maven.tree.ResolvedPom;
import org.openrewrite.semver.LatestRelease;
import org.openrewrite.semver.Semver;
import org.openrewrite.semver.VersionComparator;
Expand Down Expand Up @@ -198,12 +199,19 @@ public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
Xml maven = super.visitDocument(document, ctx);

if (!Boolean.TRUE.equals(addToRootPom) || acc.rootPoms.contains(document)) {
Validated<VersionComparator> versionValidation = Semver.validate(version, versionPattern);
ResolvedPom pom = getResolutionResult().getPom();
String convertedVersion = pom.getValue(version);
Validated<VersionComparator> versionValidation = Semver.validate(convertedVersion, versionPattern);
if (versionValidation.isValid()) {
VersionComparator versionComparator = requireNonNull(versionValidation.getValue());
try {
String versionToUse = findVersionToUse(versionComparator, ctx);
if (!Objects.equals(versionToUse, existingManagedDependencyVersion())) {
String versionToUse = findVersionToUse(versionComparator, pom, ctx);
String existingManagedDependencyVersion = existingManagedDependencyVersion();
if (!Objects.equals(versionToUse, pom.getValue(existingManagedDependencyVersion))) {
if (ResolvedPom.placeholderHelper.hasPlaceholders(version) && Objects.equals(convertedVersion, versionToUse)) {
// revert back to the original version if the version has a placeholder
versionToUse = version;
}
doAfterVisit(new AddManagedDependencyVisitor(groupId, artifactId,
versionToUse, scope, type, classifier));
maybeUpdateModel();
Expand Down Expand Up @@ -235,8 +243,8 @@ private String existingManagedDependencyVersion() {
}

@Nullable
private String findVersionToUse(VersionComparator versionComparator, ExecutionContext ctx) throws MavenDownloadingException {
MavenMetadata mavenMetadata = metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, ctx));
private String findVersionToUse(VersionComparator versionComparator, ResolvedPom containingPom, ExecutionContext ctx) throws MavenDownloadingException {
MavenMetadata mavenMetadata = metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, containingPom, ctx));
LatestRelease latest = new LatestRelease(versionPattern);
return mavenMetadata.getVersioning().getVersions().stream()
.filter(v -> versionComparator.isValid(null, v))
Expand Down
6 changes: 5 additions & 1 deletion rewrite-maven/src/main/java/org/openrewrite/maven/MavenVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,11 @@ public Collection<ResolvedDependency> findDependencies(Predicate<ResolvedDepende
}

public MavenMetadata downloadMetadata(String groupId, String artifactId, ExecutionContext ctx) throws MavenDownloadingException {
return downloadMetadata(groupId, artifactId, null, ctx);
}

public MavenMetadata downloadMetadata(String groupId, String artifactId, @Nullable ResolvedPom containingPom, ExecutionContext ctx) throws MavenDownloadingException {
return new MavenPomDownloader(emptyMap(), ctx, getResolutionResult().getMavenSettings(), getResolutionResult().getActiveProfiles())
.downloadMetadata(new GroupArtifact(groupId, artifactId), null, getResolutionResult().getPom().getRepositories());
.downloadMetadata(new GroupArtifact(groupId, artifactId), containingPom, getResolutionResult().getPom().getRepositories());
}
}
7 changes: 7 additions & 0 deletions rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ private Pom getParentWithinProject(Pom projectPom, Map<Path, Pom> projectPoms) {
}

public MavenMetadata downloadMetadata(GroupArtifact groupArtifact, @Nullable ResolvedPom containingPom, List<MavenRepository> repositories) throws MavenDownloadingException {
if (containingPom != null) {
groupArtifact = containingPom.getValues(groupArtifact);
}
ammachado marked this conversation as resolved.
Show resolved Hide resolved
return downloadMetadata(new GroupArtifactVersion(groupArtifact.getGroupId(), groupArtifact.getArtifactId(), null),
containingPom,
repositories);
Expand All @@ -218,6 +221,10 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv
throw new MavenDownloadingException("Missing group id.", null, gav);
}

if (containingPom != null) {
gav = containingPom.getValues(gav);
}

ctx.getResolutionListener().downloadMetadata(gav);

Timer.Sample sample = Timer.start();
Expand Down
46 changes: 46 additions & 0 deletions rewrite-maven/src/test/java/org/openrewrite/maven/AddManagedDependencyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,50 @@ void addedToRootPom() {
)
);
}

@DocumentExample
@Test
void propertiesAsGAVCoordinates() {
rewriteRun(
spec -> spec.recipe(new AddManagedDependency("${quarkus.platform.group-id}", "${quarkus.platform.artifact-id}",
"${quarkus.platform.version}", "import", "pom", null,null, null, null, null)),
pomXml(
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>core</artifactId>
<version>1</version>
<properties>
<quarkus.platform.artifact-id>quarkus-bom</quarkus.platform.artifact-id>
<quarkus.platform.group-id>io.quarkus.platform</quarkus.platform.group-id>
<quarkus.platform.version>3.2.3.Final</quarkus.platform.version>
</properties>
</project>
""",
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>core</artifactId>
<version>1</version>
<properties>
<quarkus.platform.artifact-id>quarkus-bom</quarkus.platform.artifact-id>
<quarkus.platform.group-id>io.quarkus.platform</quarkus.platform.group-id>
<quarkus.platform.version>3.2.3.Final</quarkus.platform.version>
</properties>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>${quarkus.platform.group-id}</groupId>
<artifactId>${quarkus.platform.artifact-id}</artifactId>
<version>${quarkus.platform.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>
</project>
"""
)
);
}
}