Skip to content

Commit

Permalink
Fix failing test and infinity loop in MavenPomDownloader (#4609)
Browse files Browse the repository at this point in the history 
* fixed the issue with the infinity loop and made it simpler

* Remove final modifier from local variables

* Clearly name each pom xml variable in tests

* Further simplifications to fix MavenParserTests

---------

Co-authored-by: Tim te Beek <[email protected]>
  • Loading branch information
@marcel-gepardec @timtebeek
marcel-gepardec and timtebeek authored Oct 27, 2024
1 parent e168652 commit 4ee1955
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 46 deletions.
50 changes: 22 additions & 28 deletions rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,10 @@ byte[] sendRequest(HttpSender.Request request) throws IOException, HttpSenderRes

private Map<GroupArtifactVersion, Pom> projectPomsByGav(Map<Path, Pom> projectPoms) {
Map<GroupArtifactVersion, Pom> result = new HashMap<>();
for (final Pom projectPom : projectPoms.values()) {
final List<Pom> ancestryWithinProject = getAncestryWithinProject(projectPom, projectPoms);
final Map<String, String> mergedProperties = mergeProperties(ancestryWithinProject);
final GroupArtifactVersion gav = new GroupArtifactVersion(
for (Pom projectPom : projectPoms.values()) {
List<Pom> ancestryWithinProject = getAncestryWithinProject(projectPom, projectPoms);
Map<String, String> mergedProperties = mergeProperties(ancestryWithinProject);
GroupArtifactVersion gav = new GroupArtifactVersion(
projectPom.getGroupId(),
projectPom.getArtifactId(),
ResolvedPom.placeholderHelper.replacePlaceholders(projectPom.getVersion(), mergedProperties::get)
Expand All @@ -187,8 +187,8 @@ private Map<GroupArtifactVersion, Pom> projectPomsByGav(Map<Path, Pom> projectPo

private Map<String, String> mergeProperties(final List<Pom> pomAncestry) {
Map<String, String> mergedProperties = new HashMap<>();
for (final Pom pom : pomAncestry) {
for (final Map.Entry<String, String> property : pom.getProperties().entrySet()) {
for (Pom pom : pomAncestry) {
for (Map.Entry<String, String> property : pom.getProperties().entrySet()) {
mergedProperties.putIfAbsent(property.getKey(), property.getValue());
}
}
Expand All @@ -205,7 +205,7 @@ private List<Pom> getAncestryWithinProject(Pom projectPom, Map<Path, Pom> projec
}

private @Nullable Pom getParentWithinProject(Pom projectPom, Map<Path, Pom> projectPoms) {
final Parent parent = projectPom.getParent();
Parent parent = projectPom.getParent();
if (parent == null || projectPom.getSourcePath() == null) {
return null;
}
Expand Down Expand Up @@ -488,32 +488,26 @@ public Pom download(GroupArtifactVersion gav,

// The pom being examined might be from a remote repository or a local filesystem.
// First try to match the requested download with one of the project POMs.
final Pom projectPomWithResolvedVersion = projectPomsByGav.get(gav);
Pom projectPomWithResolvedVersion = projectPomsByGav.get(gav);
if (projectPomWithResolvedVersion != null) {
return projectPomWithResolvedVersion;
}

// The requested gav might itself have an unresolved placeholder in the version, so also check raw values
for (Pom projectPom : projectPoms.values()) {
if (gav.getGroupId().equals(projectPom.getGroupId()) &&
gav.getArtifactId().equals(projectPom.getArtifactId())){
if (gav.getVersion().equals(projectPom.getVersion()) || projectPom.getVersion().equals(projectPom.getValue(gav.getVersion()))) {
return projectPom;
}
Parent parent = projectPom.getParent();
if (parent != null){
for (Pom project : projectPoms.values()) {
if (parent.getGroupId().equals(project.getGroupId()) && parent.getArtifactId().equals(project.getArtifactId())){
if (projectPom.getVersion().equals(project.getValue(gav.getVersion()))){
return projectPom;
}
parent = project.getParent();
if (parent == null){
break;
}
}
}
}
if (!projectPom.getGroupId().equals(gav.getGroupId()) ||
!projectPom.getArtifactId().equals(gav.getArtifactId())) {
continue;
}

if (projectPom.getVersion().equals(gav.getVersion())) {
return projectPom;
}

Map<String, String> mergedProperties = mergeProperties(getAncestryWithinProject(projectPom, projectPoms));
String versionWithReplacements = ResolvedPom.placeholderHelper.replacePlaceholders(gav.getVersion(), mergedProperties::get);
if (projectPom.getVersion().equals(versionWithReplacements)) {
return projectPom;
}
}

Expand Down Expand Up @@ -649,7 +643,7 @@ public Pom download(GroupArtifactVersion gav,
private GroupArtifactVersion handleSnapshotTimestampVersion(GroupArtifactVersion gav) {
Matcher m = SNAPSHOT_TIMESTAMP.matcher(requireNonNull(gav.getVersion()));
if (m.matches()) {
final String baseVersion;
String baseVersion;
if (m.group(1) != null) {
baseVersion = m.group(1) + SNAPSHOT;
} else {
Expand Down
36 changes: 18 additions & 18 deletions rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,9 +624,9 @@ private static GroupArtifactVersion createArtifact(Path repository) throws IOExc
void shouldNotThrowExceptionForModulesInModulesWithRightProperty() {
var gav = new GroupArtifactVersion("test", "test2", "${test}");

Path pomPath = Paths.get("test/test2/pom.xml");
Path testTest2PomXml = Paths.get("test/test2/pom.xml");
Pom pom = Pom.builder()
.sourcePath(pomPath)
.sourcePath(testTest2PomXml)
.repository(MAVEN_CENTRAL)
.properties(singletonMap("REPO_URL", MAVEN_CENTRAL.getUri()))
.parent(new Parent(new GroupArtifactVersion("test", "test", "${test}"), "../pom.xml"))
Expand All @@ -640,9 +640,9 @@ void shouldNotThrowExceptionForModulesInModulesWithRightProperty() {
.repositories(singletonList(MAVEN_CENTRAL))
.build();

Path pomPath2 = Paths.get("test/pom.xml");
Path testPomXml = Paths.get("test/pom.xml");
Pom pom2 = Pom.builder()
.sourcePath(pomPath)
.sourcePath(testPomXml)
.repository(MAVEN_CENTRAL)
.properties(singletonMap("REPO_URL", MAVEN_CENTRAL.getUri()))
.parent(new Parent(new GroupArtifactVersion("test", "root-test", "${test}"), "../pom.xml"))
Expand All @@ -651,9 +651,9 @@ void shouldNotThrowExceptionForModulesInModulesWithRightProperty() {
.build();


Path parentPomPath = Paths.get("pom.xml");
Path rootPomXml = Paths.get("pom.xml");
Pom parentPom = Pom.builder()
.sourcePath(parentPomPath)
.sourcePath(rootPomXml)
.repository(MAVEN_CENTRAL)
.properties(singletonMap("test", "7.0.0"))
.parent(null)
Expand All @@ -662,9 +662,9 @@ void shouldNotThrowExceptionForModulesInModulesWithRightProperty() {
.build();

Map<Path, Pom> pomsByPath = new HashMap<>();
pomsByPath.put(parentPomPath, parentPom);
pomsByPath.put(pomPath, pom);
pomsByPath.put(pomPath2, pom2);
pomsByPath.put(rootPomXml, parentPom);
pomsByPath.put(testTest2PomXml, pom);
pomsByPath.put(testPomXml, pom2);

String httpUrl = "http://%s.com".formatted(UUID.randomUUID());
MavenRepository nonexistentRepo = new MavenRepository("repo", httpUrl, null, null, false, null, null, null, null);
Expand All @@ -678,9 +678,9 @@ void shouldNotThrowExceptionForModulesInModulesWithRightProperty() {
void shouldThrowExceptionForModulesInModulesWithNoRightProperty() {
var gav = new GroupArtifactVersion("test", "test2", "${test}");

Path pomPath = Paths.get("test/test2/pom.xml");
Path testTest2PomXml = Paths.get("test/test2/pom.xml");
Pom pom = Pom.builder()
.sourcePath(pomPath)
.sourcePath(testTest2PomXml)
.repository(MAVEN_CENTRAL)
.properties(singletonMap("REPO_URL", MAVEN_CENTRAL.getUri()))
.parent(new Parent(new GroupArtifactVersion("test", "test", "${test}"), "../pom.xml"))
Expand All @@ -694,9 +694,9 @@ void shouldThrowExceptionForModulesInModulesWithNoRightProperty() {
.repositories(singletonList(MAVEN_CENTRAL))
.build();

Path pomPath2 = Paths.get("test/pom.xml");
Path testPomXml = Paths.get("test/pom.xml");
Pom pom2 = Pom.builder()
.sourcePath(pomPath)
.sourcePath(testPomXml)
.repository(MAVEN_CENTRAL)
.properties(singletonMap("REPO_URL", MAVEN_CENTRAL.getUri()))
.parent(new Parent(new GroupArtifactVersion("test", "root-test", "${test}"), "../pom.xml"))
Expand All @@ -705,9 +705,9 @@ void shouldThrowExceptionForModulesInModulesWithNoRightProperty() {
.build();


Path parentPomPath = Paths.get("pom.xml");
Path rootPomXml = Paths.get("pom.xml");
Pom parentPom = Pom.builder()
.sourcePath(parentPomPath)
.sourcePath(rootPomXml)
.repository(MAVEN_CENTRAL)
.properties(singletonMap("tt", "7.0.0"))
.parent(null)
Expand All @@ -716,9 +716,9 @@ void shouldThrowExceptionForModulesInModulesWithNoRightProperty() {
.build();

Map<Path, Pom> pomsByPath = new HashMap<>();
pomsByPath.put(parentPomPath, parentPom);
pomsByPath.put(pomPath, pom);
pomsByPath.put(pomPath2, pom2);
pomsByPath.put(rootPomXml, parentPom);
pomsByPath.put(testTest2PomXml, pom);
pomsByPath.put(testPomXml, pom2);

String httpUrl = "http://%s.com".formatted(UUID.randomUUID());
MavenRepository nonexistentRepo = new MavenRepository("repo", httpUrl, null, null, false, null, null, null, null);
Expand Down

0 comments on commit 4ee1955

Please sign in to comment.