Skip to content

Commit 66585e1

Browse files
sambsnydsambsnyd
authored
Fix issue where BOMs/platforms defined in Gradle Module Metadata could end up erroneously listed as requested dependencies in our maven model (#6488)
* Fix issue where BOMs/platforms defined in Gradle Module Metadata could end up erroneously listed as requested dependencies in our maven model Fixes #6487 * Fix javadoc failures due to not being able to find lombok generated class definition * Maybe this will make the CI failure go away?
1 parent 03ac2d1 commit 66585e1

File tree

8 files changed

+167
-53
lines changed

8 files changed

+167
-53
lines changed

‎rewrite-gradle/src/test/java/org/openrewrite/gradle/RemoveRedundantSecurityResolutionRulesKotlinTest.java‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void removeRedundantCveRule() {
4545
buildGradleKts(
4646
"""
4747
plugins {
48-
java
48+
id("java")
4949
}
5050
repositories { mavenCentral() }
5151
configurations.all {
@@ -63,7 +63,7 @@ void removeRedundantCveRule() {
6363
""",
6464
"""
6565
plugins {
66-
java
66+
id("java")
6767
}
6868
repositories { mavenCentral() }
6969
dependencies {
@@ -87,7 +87,7 @@ void keepRuleWhenManagedVersionIsLower() {
8787
buildGradleKts(
8888
"""
8989
plugins {
90-
java
90+
id("java")
9191
}
9292
repositories { mavenCentral() }
9393
configurations.all {
@@ -113,7 +113,7 @@ void keepRuleWithoutSecurityReason() {
113113
buildGradleKts(
114114
"""
115115
plugins {
116-
java
116+
id("java")
117117
}
118118
repositories { mavenCentral() }
119119
configurations.all {
@@ -140,7 +140,7 @@ void removeRuleWithCustomPattern() {
140140
buildGradleKts(
141141
"""
142142
plugins {
143-
java
143+
id("java")
144144
}
145145
repositories { mavenCentral() }
146146
configurations.all {
@@ -158,7 +158,7 @@ void removeRuleWithCustomPattern() {
158158
""",
159159
"""
160160
plugins {
161-
java
161+
id("java")
162162
}
163163
repositories { mavenCentral() }
164164
dependencies {
@@ -177,7 +177,7 @@ void removeMultipleRulesFromElseIfChain() {
177177
buildGradleKts(
178178
"""
179179
plugins {
180-
java
180+
id("java")
181181
}
182182
repositories { mavenCentral() }
183183
configurations.all {
@@ -199,7 +199,7 @@ void removeMultipleRulesFromElseIfChain() {
199199
""",
200200
"""
201201
plugins {
202-
java
202+
id("java")
203203
}
204204
repositories { mavenCentral() }
205205
dependencies {
@@ -229,7 +229,7 @@ void keepBuildscriptRuleWhenNoPlatformInBuildscript() {
229229
}
230230
}
231231
plugins {
232-
java
232+
id("java")
233233
}
234234
repositories { mavenCentral() }
235235
dependencies {
@@ -268,7 +268,7 @@ void removeBuildscriptRuleWhenPlatformInBuildscript() {
268268
}
269269
}
270270
plugins {
271-
java
271+
id("java")
272272
}
273273
repositories { mavenCentral() }
274274
dependencies {
@@ -284,7 +284,7 @@ void removeBuildscriptRuleWhenPlatformInBuildscript() {
284284
}
285285
}
286286
plugins {
287-
java
287+
id("java")
288288
}
289289
repositories { mavenCentral() }
290290
dependencies {
@@ -301,7 +301,7 @@ void keepRuleWithoutBecauseClause() {
301301
buildGradleKts(
302302
"""
303303
plugins {
304-
java
304+
id("java")
305305
}
306306
repositories { mavenCentral() }
307307
configurations.all {
@@ -332,7 +332,7 @@ void removeRuleWhenDirectDependencyProvidesNewerVersion() {
332332
buildGradleKts(
333333
"""
334334
plugins {
335-
java
335+
id("java")
336336
}
337337
repositories { mavenCentral() }
338338
configurations.all {
@@ -349,7 +349,7 @@ void removeRuleWhenDirectDependencyProvidesNewerVersion() {
349349
""",
350350
"""
351351
plugins {
352-
java
352+
id("java")
353353
}
354354
repositories { mavenCentral() }
355355
dependencies {
@@ -366,7 +366,7 @@ void removeRuleButKeepCustomConfiguration() {
366366
buildGradleKts(
367367
"""
368368
plugins {
369-
java
369+
id("java")
370370
}
371371
repositories { mavenCentral() }
372372
configurations {
@@ -387,7 +387,7 @@ void removeRuleButKeepCustomConfiguration() {
387387
""",
388388
"""
389389
plugins {
390-
java
390+
id("java")
391391
}
392392
repositories { mavenCentral() }
393393
configurations {
@@ -412,7 +412,7 @@ void keepConfigurationsBlockWithoutResolutionStrategy() {
412412
buildGradleKts(
413413
"""
414414
plugins {
415-
java
415+
id("java")
416416
}
417417
repositories { mavenCentral() }
418418
configurations {
@@ -438,7 +438,7 @@ void removeRedundantRuleOnSpecificConfiguration() {
438438
buildGradleKts(
439439
"""
440440
plugins {
441-
java
441+
id("java")
442442
}
443443
repositories { mavenCentral() }
444444
configurations.named("compileClasspath") {
@@ -456,7 +456,7 @@ void removeRedundantRuleOnSpecificConfiguration() {
456456
""",
457457
"""
458458
plugins {
459-
java
459+
id("java")
460460
}
461461
repositories { mavenCentral() }
462462
dependencies {
@@ -474,7 +474,7 @@ void removeRedundantRuleOnNestedConfiguration() {
474474
buildGradleKts(
475475
"""
476476
plugins {
477-
java
477+
id("java")
478478
}
479479
repositories { mavenCentral() }
480480
configurations {
@@ -496,7 +496,7 @@ void removeRedundantRuleOnNestedConfiguration() {
496496
""",
497497
"""
498498
plugins {
499-
java
499+
id("java")
500500
}
501501
repositories { mavenCentral() }
502502
dependencies {

‎rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv
300300
if (derivedMeta != null) {
301301
Counter.builder("rewrite.maven.derived.metadata")
302302
.tag("repositoryUri", repo.getUri())
303-
.tag("group", gav.getGroupId())
303+
.tag("group", gav.getGroupId() == null ? "" : gav.getGroupId())
304304
.tag("artifact", gav.getArtifactId())
305305
.register(Metrics.globalRegistry)
306306
.increment();
@@ -636,13 +636,23 @@ public Pom download(GroupArtifactVersion gav,
636636
String line;
637637
while ((line = reader.readLine()) != null) {
638638
if (line.contains("published-with-gradle-metadata")) {
639-
// as some artifacts do not have the bom as dependency listed, we need to add it by fetching the gradle metadata module.
639+
// Some artifacts do not have the bom as dependency listed, we need to add it by fetching the gradle metadata module.
640+
// It isn't strictly correct to do this from a maven standpoint, but helps with emulating Gradle dependency resolution
640641
pomResponseBody = requestAsAuthenticatedOrAnonymous(repo, uri.toString().replaceFirst(".pom$", ".module"));
641642
RawGradleModule module = RawGradleModule.parse(new ByteArrayInputStream(pomResponseBody));
642-
for (Dependency dependency : module.getDependencies("apiElements", "platform")) {
643-
rawPom.getDependencies().getDependencies().add(
644-
new RawPom.Dependency(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion(), null, "bom", null, null, null)
645-
);
643+
for (Dependency dependency : module.getDependencies("apiElements", "platform", "enforcedPlatform")) {
644+
String category = dependency.getAttributes().get("org.gradle.category");
645+
// Platform and enforcedPlatform dependencies are BOMs that should be in dependencyManagement,
646+
// not regular dependencies - they only provide version constraints
647+
if ("platform".equalsIgnoreCase(category) || "enforcedPlatform".equalsIgnoreCase(category)) {
648+
if (rawPom.getDependencyManagement() == null) {
649+
rawPom.setDependencyManagement(new RawPom.DependencyManagement(new RawPom.Dependencies()));
650+
}
651+
assert rawPom.getDependencyManagement().getDependencies() != null;
652+
rawPom.getDependencyManagement().getDependencies().getDependencies().add(
653+
new RawPom.Dependency(dependency.getGroupId() == null ? "" : dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion(), "import", "pom", null, null, null)
654+
);
655+
}
646656
}
647657
break;
648658
}
@@ -999,7 +1009,7 @@ public boolean isSuccess() {
9991009
enum Reachability {
10001010
SUCCESS,
10011011
ERROR,
1002-
UNREACHABLE;
1012+
UNREACHABLE
10031013
}
10041014

10051015
private ReachabilityResult reachable(HttpSender.Request.Builder request) {

‎rewrite-maven/src/main/java/org/openrewrite/maven/internal/RawGradleModule.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.util.Objects;
3636

3737
import static java.util.Collections.emptyList;
38+
import static java.util.Collections.singletonMap;
3839
import static java.util.stream.Collectors.toList;
3940
import static org.openrewrite.internal.ObjectMappers.propertyBasedMapper;
4041

@@ -125,10 +126,14 @@ public List<org.openrewrite.maven.tree.Dependency> getDependencies(String varian
125126
.filter(Objects::nonNull)
126127
.flatMap(Collection::stream)
127128
.filter(d -> categories.length == 0 || (d.getAttributes() != null && d.getAttributes().getCategory() != null && Arrays.stream(categories).anyMatch(cat -> d.getAttributes().getCategory().equalsIgnoreCase(cat))))
128-
.map(Dependency::asGav)
129-
.map(gav -> org.openrewrite.maven.tree.Dependency.builder()
130-
.gav(gav)
131-
.build())
129+
.map(dep -> {
130+
org.openrewrite.maven.tree.Dependency.DependencyBuilder builder = org.openrewrite.maven.tree.Dependency.builder().gav(dep.asGav());
131+
if (dep.getAttributes() != null && dep.getAttributes().getCategory() != null) {
132+
//noinspection NullableProblems,ResultOfMethodCallIgnored
133+
builder.attributes(singletonMap("org.gradle.category", dep.getAttributes().getCategory()));
134+
}
135+
return builder.build();
136+
})
132137
.collect(toList());
133138
}
134139
}

‎rewrite-maven/src/main/java/org/openrewrite/maven/internal/RawPom.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import lombok.*;
2424
import lombok.experimental.FieldDefaults;
2525
import lombok.experimental.NonFinal;
26-
import org.jspecify.annotations.NonNull;
2726
import org.jspecify.annotations.Nullable;
2827
import org.openrewrite.internal.ListUtils;
2928
import org.openrewrite.internal.StringUtils;
@@ -101,6 +100,8 @@ public class RawPom {
101100
Dependencies dependencies;
102101

103102
@Nullable
103+
@NonFinal
104+
@Setter(AccessLevel.PACKAGE)
104105
DependencyManagement dependencyManagement;
105106

106107
@Nullable
@@ -498,7 +499,6 @@ private List<org.openrewrite.maven.tree.Profile> mapProfiles(@Nullable Profiles
498499
return profiles;
499500
}
500501

501-
@NonNull
502502
private List<MavenRepository> mapRepositories(@Nullable RawRepositories rawRepositories) {
503503
List<MavenRepository> pomRepositories = emptyList();
504504
if (rawRepositories != null) {
@@ -517,7 +517,6 @@ private List<MavenRepository> mapRepositories(@Nullable RawRepositories rawRepos
517517
return pomRepositories;
518518
}
519519

520-
@NonNull
521520
private List<MavenRepository> mapPluginRepositories(@Nullable RawPluginRepositories rawRepositories) {
522521
List<MavenRepository> pomRepositories = emptyList();
523522
if (rawRepositories != null) {

‎rewrite-maven/src/test/java/org/openrewrite/maven/MavenParserTest.java‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.openrewrite.maven.internal.MavenParsingException;
4040
import org.openrewrite.maven.tree.*;
4141
import org.openrewrite.test.RewriteTest;
42-
import org.openrewrite.test.SourceSpecs;
4342
import org.openrewrite.test.TypeValidation;
4443
import org.openrewrite.tree.ParseError;
4544

@@ -1557,7 +1556,6 @@ void parseNotInProfileActivation() {
15571556
@Issue("https://github.com/openrewrite/rewrite/issues/1427")
15581557
@Test
15591558
void parseEmptyActivationTag() {
1560-
//noinspection DataFlowIssue
15611559
rewriteRun(
15621560
pomXml(
15631561
"""
@@ -1613,7 +1611,6 @@ void parseEmptyValueActivationTag() {
16131611
assertThat(activation).isNotNull();
16141612
assertThat(activation.getActiveByDefault()).isNull();
16151613
assertThat(activation.getJdk()).isNull();
1616-
//noinspection DataFlowIssue
16171614
assertThat(activation.getProperty()).isNull();
16181615
})
16191616
)
@@ -3118,7 +3115,7 @@ void mergePluginConfigListAppendOverride() {
31183115
@Test
31193116
void escapedA() {
31203117
rewriteRun(
3121-
spec -> spec.recipe(new AddManagedDependency("ch.qos.logback", "logback-classic", "1.4.14", null, null, null, null, null, null, null)),
3118+
spec -> spec.recipe(new AddManagedDependency("ch.qos.logback", "logback-classic", "1.4.14", null, null, null, null, null, null, null, null)),
31223119
//language=xml
31233120
pomXml(
31243121
"""
@@ -3631,7 +3628,7 @@ void systemPropertyTakesPrecedence() {
36313628
<version>1.0-SNAPSHOT</version>
36323629
<packaging>pom</packaging>
36333630
<name>parent</name>
3634-
<url>http://www.example.com</url>
3631+
<url>https://www.example.com</url>
36353632
<properties>
36363633
<hatversion>SYSTEM_PROPERTY_SHOULD_OVERRIDE_THIS</hatversion>
36373634
</properties>
@@ -4893,4 +4890,41 @@ void groupIdAndArtifactIdAsProperties() {
48934890
)
48944891
);
48954892
}
4893+
4894+
@Issue("https://github.com/openrewrite/rewrite/issues/6487")
4895+
@Test
4896+
void bomsShouldNotAppearInRuntimeDependencies() {
4897+
// jackson-core takes no direct dependencies
4898+
// Its parent pom is com.fasterxml.jackson:jackson-base
4899+
// jackson-base's parent pom is jackson-bom
4900+
// Like most boms/parents jackson-bom contains only dependencyManagement entries
4901+
rewriteRun(
4902+
pomXml(
4903+
"""
4904+
<project>
4905+
<modelVersion>4.0.0</modelVersion>
4906+
<groupId>com.example</groupId>
4907+
<artifactId>test-app</artifactId>
4908+
<version>1.0.0</version>
4909+
4910+
<dependencies>
4911+
<dependency>
4912+
<groupId>com.fasterxml.jackson.core</groupId>
4913+
<artifactId>jackson-core</artifactId>
4914+
<version>2.17.2</version>
4915+
</dependency>
4916+
</dependencies>
4917+
</project>
4918+
""",
4919+
spec -> spec.afterRecipe(pom -> {
4920+
MavenResolutionResult mrr = pom.getMarkers().findFirst(MavenResolutionResult.class).orElseThrow();
4921+
List<ResolvedDependency> runtimeDeps = mrr.getDependencies().get(Scope.Runtime);
4922+
assertThat(runtimeDeps)
4923+
.filteredOn(dep -> "jackson-bom".equals(dep.getArtifactId()))
4924+
.as("jackson-bom is jackson-core's parent pom, it is not a runtime dependency")
4925+
.isEmpty();
4926+
})
4927+
)
4928+
);
4929+
}
48964930
}

0 commit comments

Comments
 (0)