From 3d9653d441e90f46be502765b449aff251dcb5e6 Mon Sep 17 00:00:00 2001 From: t-huebner-id <161744488+t-huebner-id@users.noreply.github.com> Date: Thu, 16 Jan 2025 21:15:07 +0100 Subject: [PATCH] =?UTF-8?q?Fixed=20missing=20version=20on=20multiple=20dep?= =?UTF-8?q?endency=20management=20sections=20in=20m=E2=80=A6=20(#4888)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fixed missing version on multiple dependency management sections in multi-module projects and fixed ignored classifiers when choosing snapshot timestamp * Formatted code, add unit test * Added issue annotation to test * Formatted * Minor polish * Further polish * Generalize comment on continue * Test for snapshot downloading issue * Minor polish --------- Co-authored-by: Tobias Hübner Co-authored-by: Tim te Beek --- .../maven/internal/MavenPomDownloader.java | 76 ++++-- .../openrewrite/maven/tree/MavenMetadata.java | 2 + .../openrewrite/maven/tree/ResolvedPom.java | 4 +- .../internal/MavenPomDownloaderTest.java | 246 +++++++++++++----- .../maven/tree/ResolvedPomTest.java | 140 +++++++++- 5 files changed, 378 insertions(+), 90 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java index 26e59770b35..175cdb7a9a0 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java @@ -219,7 +219,7 @@ private List getAncestryWithinProject(Pom projectPom, Map projec .normalize(); Pom parentPom = projectPoms.get(parentPath); return parentPom != null && parentPom.getGav().getGroupId().equals(parent.getGav().getGroupId()) && - parentPom.getGav().getArtifactId().equals(parent.getGav().getArtifactId()) ? parentPom : null; + parentPom.getGav().getArtifactId().equals(parent.getGav().getArtifactId()) ? parentPom : null; } public MavenMetadata downloadMetadata(GroupArtifact groupArtifact, @Nullable ResolvedPom containingPom, List repositories) throws MavenDownloadingException { @@ -259,9 +259,9 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv try { String scheme = URI.create(repo.getUri()).getScheme(); String baseUri = repo.getUri() + (repo.getUri().endsWith("/") ? "" : "/") + - requireNonNull(gav.getGroupId()).replace('.', '/') + '/' + - gav.getArtifactId() + '/' + - (gav.getVersion() == null ? "" : gav.getVersion() + '/'); + requireNonNull(gav.getGroupId()).replace('.', '/') + '/' + + gav.getArtifactId() + '/' + + (gav.getVersion() == null ? "" : gav.getVersion() + '/'); if ("file".equals(scheme)) { // A maven repository can be expressed as a URI with a file scheme @@ -356,8 +356,8 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv String scheme = URI.create(repo.getUri()).getScheme(); String uri = repo.getUri() + (repo.getUri().endsWith("/") ? "" : "/") + - requireNonNull(gav.getGroupId()).replace('.', '/') + '/' + - gav.getArtifactId() + '/'; + requireNonNull(gav.getGroupId()).replace('.', '/') + '/' + + gav.getArtifactId() + '/'; try { MavenMetadata.Versioning versioning; @@ -496,7 +496,7 @@ public Pom download(GroupArtifactVersion gav, // The requested gav might itself have an unresolved placeholder in the version, so also check raw values for (Pom projectPom : projectPoms.values()) { if (!projectPom.getGroupId().equals(gav.getGroupId()) || - !projectPom.getArtifactId().equals(gav.getArtifactId())) { + !projectPom.getArtifactId().equals(gav.getArtifactId())) { continue; } @@ -512,7 +512,7 @@ public Pom download(GroupArtifactVersion gav, } if (containingPom != null && containingPom.getRequested().getSourcePath() != null && - !StringUtils.isBlank(relativePath) && !relativePath.contains(":")) { + !StringUtils.isBlank(relativePath) && !relativePath.contains(":")) { Path folderContainingPom = containingPom.getRequested().getSourcePath().getParent(); if (folderContainingPom != null) { Pom maybeLocalPom = projectPoms.get(folderContainingPom.resolve(Paths.get(relativePath, "pom.xml")) @@ -521,9 +521,9 @@ public Pom download(GroupArtifactVersion gav, // So double check that the GAV coordinates match so that we don't get a relative path from a remote // pom like ".." or "../.." which coincidentally _happens_ to have led to an unrelated pom on the local filesystem if (maybeLocalPom != null && - gav.getGroupId().equals(maybeLocalPom.getGroupId()) && - gav.getArtifactId().equals(maybeLocalPom.getArtifactId()) && - gav.getVersion().equals(maybeLocalPom.getVersion())) { + gav.getGroupId().equals(maybeLocalPom.getGroupId()) && + gav.getArtifactId().equals(maybeLocalPom.getArtifactId()) && + gav.getVersion().equals(maybeLocalPom.getVersion())) { return maybeLocalPom; } } @@ -552,10 +552,10 @@ public Pom download(GroupArtifactVersion gav, if (result == null) { URI uri = URI.create(repo.getUri() + (repo.getUri().endsWith("/") ? "" : "/") + - requireNonNull(gav.getGroupId()).replace('.', '/') + '/' + - gav.getArtifactId() + '/' + - gav.getVersion() + '/' + - gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".pom"); + requireNonNull(gav.getGroupId()).replace('.', '/') + '/' + + gav.getArtifactId() + '/' + + gav.getVersion() + '/' + + gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".pom"); uris.add(uri.toString()); if ("file".equals(uri.getScheme())) { Path inputPath = Paths.get(gav.getGroupId(), gav.getArtifactId(), gav.getVersion()); @@ -688,9 +688,9 @@ private GroupArtifactVersion handleSnapshotTimestampVersion(GroupArtifactVersion if (gav.getVersion() != null && gav.getVersion().endsWith("-SNAPSHOT")) { for (ResolvedGroupArtifactVersion pinnedSnapshotVersion : new MavenExecutionContextView(ctx).getPinnedSnapshotVersions()) { if (pinnedSnapshotVersion.getDatedSnapshotVersion() != null && - pinnedSnapshotVersion.getGroupId().equals(gav.getGroupId()) && - pinnedSnapshotVersion.getArtifactId().equals(gav.getArtifactId()) && - pinnedSnapshotVersion.getVersion().equals(gav.getVersion())) { + pinnedSnapshotVersion.getGroupId().equals(gav.getGroupId()) && + pinnedSnapshotVersion.getArtifactId().equals(gav.getArtifactId()) && + pinnedSnapshotVersion.getVersion().equals(gav.getVersion())) { return pinnedSnapshotVersion.getDatedSnapshotVersion(); } } @@ -702,6 +702,23 @@ private GroupArtifactVersion handleSnapshotTimestampVersion(GroupArtifactVersion //This can happen if the artifact only exists in the local maven cache. In this case, just return the original return gav.getVersion(); } + + // Find the newest with a matching classifier - the latest snapshot may not have artifacts for all classifiers. + List snapshotVersions = mavenMetadata.getVersioning().getSnapshotVersions(); + if (snapshotVersions != null) { + // Try to get requested classifier (this is unfortunately not present in 'gav' structure) + String classifier = getClassifierFromContainingPom(gav, containingPom); + MavenMetadata.SnapshotVersion snapshotVersion = snapshotVersions.stream() + .sorted(Comparator.comparing(MavenMetadata.SnapshotVersion::getUpdated).reversed()) + .filter(s -> Objects.equals(s.getClassifier(), classifier)) + .findFirst() + .orElse(null); + if (snapshotVersion != null) { + return snapshotVersion.getValue(); + } + } + + // Replace SNAPSHOT suffix with timestamp and build number MavenMetadata.Snapshot snapshot = mavenMetadata.getVersioning().getSnapshot(); if (snapshot != null && snapshot.getTimestamp() != null) { return gav.getVersion().replaceFirst("SNAPSHOT$", snapshot.getTimestamp() + "-" + snapshot.getBuildNumber()); @@ -1054,4 +1071,27 @@ private static boolean hasPomFile(Path dir, Path versionPath, GroupArtifactVersi String artifactPomFile = gav.getArtifactId() + "-" + versionPath.getFileName() + ".pom"; return Files.exists(dir.resolve(versionPath.resolve(artifactPomFile))); } + + /** + * Retrieves the classifier of a dependency from a provided resolved POM, if it exists. + * + * @param gav The group, artifact, and version information of the dependency to locate. + * @param containingPom The resolved POM that potentially contains the dependency information. + * If null, the method will return null. + * @return The classifier of the dependency within the provided POM, or null if no matching + * dependency or classifier is found. + */ + private static @Nullable String getClassifierFromContainingPom(GroupArtifactVersion gav, @Nullable ResolvedPom containingPom) { + if (containingPom != null) { + for (Dependency dep : containingPom.getRequestedDependencies()) { + if (Objects.equals(dep.getGroupId(), gav.getGroupId())) { + if (Objects.equals(dep.getArtifactId(), gav.getArtifactId())) { + return dep.getClassifier(); + } + } + } + } + return null; + } + } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenMetadata.java b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenMetadata.java index 2ece965a7f5..8d94d2dd036 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenMetadata.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenMetadata.java @@ -101,5 +101,7 @@ public static class SnapshotVersion { String extension; String value; String updated; + @Nullable + String classifier; } } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java index 7f5a0d343e1..7b9b00721d3 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java @@ -320,11 +320,13 @@ public String getPackaging() { public @Nullable String getManagedVersion(@Nullable String groupId, String artifactId, @Nullable String type, @Nullable String classifier) { for (ResolvedManagedDependency dm : dependencyManagement) { + if (dm.getVersion() == null) { + continue; // Unclear why this happens; just ignore those entries, because a valid version is requested + } if (dm.matches(groupId, artifactId, type, classifier)) { return getValue(dm.getVersion()); } } - return null; } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java index 4c19fc87f33..b641f835b64 100755 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java @@ -33,11 +33,11 @@ import org.openrewrite.*; import org.openrewrite.ipc.http.HttpSender; import org.openrewrite.ipc.http.HttpUrlConnectionSender; -import org.openrewrite.maven.http.OkHttpSender; import org.openrewrite.maven.MavenDownloadingException; import org.openrewrite.maven.MavenExecutionContextView; import org.openrewrite.maven.MavenParser; import org.openrewrite.maven.MavenSettings; +import org.openrewrite.maven.http.OkHttpSender; import org.openrewrite.maven.tree.*; import javax.net.ssl.SSLSocketFactory; @@ -314,7 +314,7 @@ void useHttpWhenHttpsFails() throws IOException { @Test @Disabled - void dontFetchSnapshotsFromReleaseRepos() { + void dontFetchSnapshotsFromReleaseRepos() throws Exception { try (MockWebServer snapshotRepo = new MockWebServer(); MockWebServer releaseRepo = new MockWebServer()) { snapshotRepo.setDispatcher(new Dispatcher() { @@ -322,39 +322,39 @@ void dontFetchSnapshotsFromReleaseRepos() { public MockResponse dispatch(RecordedRequest request) { MockResponse response = new MockResponse().setResponseCode(200); if (request.getPath() != null && request.getPath().contains("maven-metadata")) { + //language=xml response.setBody( - //language=xml """ - - org.springframework.cloud - spring-cloud-dataflow-build - 2.10.0-SNAPSHOT - - - 20220201.001946 - 85 - - 20220201001950 - - - pom - 2.10.0-20220201.001946-85 - 20220201001946 - - - - + + org.springframework.cloud + spring-cloud-dataflow-build + 2.10.0-SNAPSHOT + + + 20220201.001946 + 85 + + 20220201001950 + + + pom + 2.10.0-20220201.001946-85 + 20220201001946 + + + + """ ); } else if (request.getPath() != null && request.getPath().endsWith(".pom")) { + //language=xml response.setBody( - //language=xml """ - - org.springframework.cloud - spring-cloud-dataflow-build - 2.10.0-SNAPSHOT - + + org.springframework.cloud + spring-cloud-dataflow-build + 2.10.0-SNAPSHOT + """ ); } @@ -380,45 +380,173 @@ public MockResponse dispatch(RecordedRequest request) { MavenParser.builder().build().parse(ctx, //language=xml """ - - 4.0.0 + + 4.0.0 - org.openrewrite.test - foo - 0.1.0-SNAPSHOT + org.openrewrite.test + foo + 0.1.0-SNAPSHOT - - - snapshot - - true - - http://%s:%d - - - release - - false - - http://%s:%d - - + + + snapshot + + true + + http://%s:%d + + + release + + false + + http://%s:%d + + - - - org.springframework.cloud - spring-cloud-dataflow-build - 2.10.0-SNAPSHOT - - - + + + org.springframework.cloud + spring-cloud-dataflow-build + 2.10.0-SNAPSHOT + + + """.formatted(snapshotRepo.getHostName(), snapshotRepo.getPort(), releaseRepo.getHostName(), releaseRepo.getPort()) ); assertThat(snapshotRepo.getRequestCount()).isGreaterThan(1); assertThat(metadataPaths.get()).isEqualTo(0); - } catch (IOException e) { - throw new RuntimeException(e); + } + } + + @Issue("https://github.com/openrewrite/rewrite-maven-plugin/issues/862") + @Test + void fetchSnapshotWithCorrectClassifier() throws Exception { + try (MockWebServer snapshotServer = new MockWebServer()) { + snapshotServer.setDispatcher(new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest request) { + MockResponse response = new MockResponse().setResponseCode(200); + if (request.getPath() != null && request.getPath().contains("maven-metadata")) { + //language=xml + response.setBody( + """ + + com.some + an-artifact + 10.5.0-SNAPSHOT + + + 20250113.114247 + 36 + + 20250113114247 + + + javadoc + jar + 10.5.0-20250113.114247-36 + 20250113114247 + + + tests + jar + 10.5.0-20250113.114244-35 + 20250113114244 + + + sources + jar + 10.5.0-20250113.114242-34 + 20250113114242 + + + jar + 10.5.0-20250113.114227-33 + 20250113114227 + + + pom + 10.5.0-20250113.114227-33 + 20250113114227 + + + + + """ + ); + } else if (request.getPath() != null && request.getPath().endsWith(".pom")) { + //language=xml + response.setBody( + """ + + com.some + an-artifact + 10.5.0-SNAPSHOT + + """ + ); + } + return response; + } + }); + + snapshotServer.start(); + + //language=xml + MavenParser.builder().build().parse(ctx, + """ + + 4.0.0 + org.openrewrite.test + foo + 0.1.0-SNAPSHOT + + + snapshot + + true + + http://%s:%d + + + + + com.some + an-artifact + 10.5.0-SNAPSHOT + + + + """.formatted(snapshotServer.getHostName(), snapshotServer.getPort()) + ); + + MavenRepository snapshotRepo = MavenRepository.builder() + .id("id") + .uri("http://%s:%d/maven/".formatted(snapshotServer.getHostName(), snapshotServer.getPort())) + .build(); + + var gav = new GroupArtifactVersion("com.some", "an-artifact", "10.5.0-SNAPSHOT"); + var mavenPomDownloader = new MavenPomDownloader(emptyMap(), ctx); + + var pomPath = Paths.get("pom.xml"); + var pom = Pom.builder() + .sourcePath(pomPath) + .repository(snapshotRepo) + .properties(singletonMap("REPO_URL", snapshotRepo.getUri())) + .gav(new ResolvedGroupArtifactVersion( + "${REPO_URL}", gav.getGroupId(), gav.getArtifactId(), gav.getVersion(), null)) + .build(); + var resolvedPom = ResolvedPom.builder() + .requested(pom) + .properties(singletonMap("REPO_URL", snapshotRepo.getUri())) + .repositories(singletonList(snapshotRepo)) + .build(); + + // Ensure that classifier 'javadoc', 'tests' and 'sources' are not used + var downloadedPom = mavenPomDownloader.download(gav, null, resolvedPom, List.of(snapshotRepo)); + assertThat(downloadedPom).returns("10.5.0-20250113.114227-33", Pom::getDatedSnapshotVersion); } } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java index b1c0e12f1bd..ea52800af59 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java @@ -367,19 +367,135 @@ public void downloadError(GroupArtifactVersion gav, List attemptedUris, assertThat(downloadErrorArgs).hasSize(1); } - private static void createJarFile(Path localRepository1) throws IOException { - Path localJar = localRepository1.resolve("com/some/some-artifact/1/some-artifact-1.jar"); - assertThat(localJar.getParent().toFile().mkdirs()).isTrue(); - Files.writeString(localJar, "some content not to be empty"); - } + } - private static MavenRepository createMavenRepository(Path localRepository, String name) { - return MavenRepository.builder() - .id(name) - .uri(localRepository.toUri().toString()) - .snapshots(false) - .knownToExist(true) - .build(); + @Nested + class DependencyManagement { + + @Issue("https://github.com/openrewrite/rewrite-maven-plugin/issues/862") + @Test + void resolveVersionFromParentDependencyManagement(@TempDir Path localRepository) throws IOException { + MavenRepository mavenLocal = createMavenRepository(localRepository, "local"); + createJarFile(localRepository); + createJarFile(localRepository, "org.openrewrite.test", "lib", "1.0"); + + List> downloadErrorArgs = new ArrayList<>(); + MavenExecutionContextView ctx = MavenExecutionContextView.view(new InMemoryExecutionContext(Throwable::printStackTrace)); + ctx.setRepositories(List.of(mavenLocal)); + ctx.setResolutionListener(new ResolutionEventListener() { + @Override + public void downloadError(GroupArtifactVersion gav, List attemptedUris, @Nullable Pom containing) { + List list = new ArrayList<>(); + list.add(gav); + list.add(attemptedUris); + list.add(containing); + downloadErrorArgs.add(list); + } + }); + + String father = """ + + 4.0.0 + org.example + father + 1.0.0-SNAPSHOT + pom + + childA + childB + + + + + com.some + some-artifact + 1 + + + + + """; + String childA = """ + + 4.0.0 + + org.example + father + 1.0.0-SNAPSHOT + + childA + + + + org.openrewrite.test + lib + 1.0 + pom + import + + + + + + com.some + some-artifact + + + + """; + String childB = """ + + 4.0.0 + + org.example + father + 1.0.0-SNAPSHOT + + childB + + + + com.some + some-artifact + compile + + + + + """; + + rewriteRun( + spec -> spec.executionContext(ctx), + pomXml(father, spec -> spec.path("pom.xml")), + pomXml(childA, spec -> spec.path("childA/pom.xml")), + pomXml(childB, spec -> spec.path("childB/pom.xml").afterRecipe(doc -> { + ResolvedPom pom = doc.getMarkers().findFirst(MavenResolutionResult.class).get().getPom(); + String version = pom.getManagedVersion("com.some", "some-artifact", null, null); + // Assert that version is not null! + assertThat(version).isEqualTo("1"); + }) + ) + ); } } + + private static void createJarFile(Path localRepository1) throws IOException { + createJarFile(localRepository1, "com/some", "some-artifact", "1"); + } + + private static void createJarFile(Path localRepository, String groupId, String artifactId, String version) throws IOException { + Path localJar = localRepository.resolve("%s/%s/%s/%s-%s.jar".formatted( + groupId.replace('.', '/'), artifactId, version, artifactId, version)); + assertThat(localJar.getParent().toFile().mkdirs()).isTrue(); + Files.writeString(localJar, "some content not to be empty"); + } + + private static MavenRepository createMavenRepository(Path localRepository, String name) { + return MavenRepository.builder() + .id(name) + .uri(localRepository.toUri().toString()) + .snapshots(false) + .knownToExist(true) + .build(); + } }