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

Apply add dependency correctly, taking aggregating poms into account #4590

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Apply add dependency correctly, taking aggregating poms into account #4590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d82abba
WIP adding a mechanism to detect aggregating poms and only adding dep…
Laurens-W Oct 18, 2024
1ec3ca8
Test case with correct expectations
Laurens-W Oct 18, 2024
99b3781
Remove empty comment
Laurens-W Oct 18, 2024
030e874
Format
Laurens-W Oct 18, 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
20 changes: 15 additions & 5 deletions rewrite-maven/src/main/java/org/openrewrite/maven/AddDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@
import org.openrewrite.semver.Semver;
import org.openrewrite.xml.tree.Xml;

import java.util.*;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;

import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -180,17 +183,17 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
acc.usingType = true;
JavaProject javaProject = sourceFile.getMarkers().findFirst(JavaProject.class).orElse(null);
JavaSourceSet javaSourceSet = sourceFile.getMarkers().findFirst(JavaSourceSet.class).orElse(null);
if(javaProject != null && javaSourceSet != null) {
if (javaProject != null && javaSourceSet != null) {
acc.scopeByProject.compute(javaProject, (jp, scope) -> "compile".equals(scope) ?
scope /* a `compile` scope dependency will also be available in test source set */ :
"test".equals(javaSourceSet.getName()) ? "test" : "compile"
);
}
}
} else if(tree instanceof Xml.Document) {
} else if (tree instanceof Xml.Document) {
Xml.Document doc = (Xml.Document) tree;
MavenResolutionResult mrr = doc.getMarkers().findFirst(MavenResolutionResult.class).orElse(null);
if(mrr == null) {
if (mrr == null) {
return sourceFile;
}
acc.pomsDefinedInCurrentRepository.add(mrr.getPom().getGav());
Expand Down Expand Up @@ -235,7 +238,14 @@ public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
}
}

if(onlyIfUsing == null && getResolutionResult().getParent() != null && acc.pomsDefinedInCurrentRepository.contains(getResolutionResult().getParent().getPom().getGav())) {
if (onlyIfUsing == null && getResolutionResult().getParent() != null && acc.pomsDefinedInCurrentRepository.contains(getResolutionResult().getParent().getPom().getGav())) {
return maven;
}

if (!getResolutionResult().getPom().getModules().isEmpty()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't fully got this logic sorted, will continue on Monday

&& (getResolutionResult().getModules().isEmpty()
|| getResolutionResult().getModules().stream().map(MavenResolutionResult::getPom).map(ResolvedPom::getGav).map(ResolvedGroupArtifactVersion::getArtifactId).anyMatch(art -> getResolutionResult().getPom().getModules().contains(art))
)) {
return maven;
}

Expand Down
27 changes: 22 additions & 5 deletions rewrite-maven/src/main/java/org/openrewrite/maven/internal/RawPom.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ public class RawPom {
@Nullable
Profiles profiles;

@Nullable
Modules modules;

public static RawPom parse(InputStream inputStream, @Nullable String snapshotVersion) {
try {
RawPom pom = MavenXmlMapper.readMapper().readValue(inputStream, RawPom.class);
Expand Down Expand Up @@ -217,6 +220,19 @@ public Profiles(@JacksonXmlProperty(localName = "profile") List<Profile> profile
}
}

@Getter
public static class Modules {
private final List<String> modules;

public Modules() {
this.modules = emptyList();
}

public Modules(@JacksonXmlProperty(localName = "module") List<String> modules) {
this.modules = modules;
}
}

@FieldDefaults(level = AccessLevel.PRIVATE)
@Data
public static class Build {
Expand Down Expand Up @@ -346,9 +362,9 @@ public static class Profile {
}

public @Nullable String getVersion() {
if(version == null) {
if(currentVersion == null) {
if(parent == null) {
if (version == null) {
if (currentVersion == null) {
if (parent == null) {
return null;
} else {
return parent.getVersion();
Expand Down Expand Up @@ -382,8 +398,9 @@ public Pom toPom(@Nullable Path inputPath, @Nullable MavenRepository repo) {
.packaging(packaging)
.properties(getProperties() == null ? emptyMap() : getProperties())
.licenses(mapLicenses(getLicenses()))
.profiles(mapProfiles(getProfiles()));
if(StringUtils.isBlank(pomVersion)) {
.profiles(mapProfiles(getProfiles()))
.modules(getModules() == null ? emptyList() : getModules().getModules());
if (StringUtils.isBlank(pomVersion)) {
builder.dependencies(mapRequestedDependencies(getDependencies()))
.dependencyManagement(mapDependencyManagement(getDependencyManagement()))
.repositories(mapRepositories(getRepositories()))
Expand Down
16 changes: 16 additions & 0 deletions rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenResolutionResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,22 @@ public boolean parentPomIsProjectPom() {
.anyMatch(gav -> gav.equals(parentGav));
}

/**
* Often recipes operating on multi-module projects will prefer to make changes to the parent pom rather than in multiple child poms.
* But if the parent isn't in the same repository as the child, the recipe would need to be applied to the child poms.
*
* @return true when the parent pom of the current pom is present in the same repository as the current pom
*/
public boolean isAggregatingPom() {
if (getParent() == null) {
return false;
}
ResolvedGroupArtifactVersion parentGav = getParent().getPom().getGav();
return getProjectPoms().values().stream()
.map(Pom::getGav)
.anyMatch(gav -> gav.equals(parentGav));
}

private Map<Path, Pom> getProjectPomsRecursive(Map<Path, Pom> projectPoms) {
projectPoms.put(requireNonNull(pom.getRequested().getSourcePath()), pom.getRequested());
if (parent != null) {
Expand Down
6 changes: 5 additions & 1 deletion rewrite-maven/src/main/java/org/openrewrite/maven/tree/Pom.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ public static int getModelVersion() {
@Builder.Default
List<Plugin> pluginManagement = emptyList();

@Builder.Default
List<String> modules = emptyList();

public String getGroupId() {
return gav.getGroupId();
}
Expand Down Expand Up @@ -184,7 +187,8 @@ public ResolvedPom resolve(Iterable<String> activeProfiles,
repositories,
dependencies,
plugins,
pluginManagement)
pluginManagement,
modules)
.resolve(ctx, downloader);
}

Expand Down
12 changes: 9 additions & 3 deletions rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,11 @@ public class ResolvedPom {
Iterable<String> activeProfiles;

public ResolvedPom(Pom requested, Iterable<String> activeProfiles) {
this(requested, activeProfiles, emptyMap(), emptyList(), null, emptyList(), emptyList(), emptyList(), emptyList());
this(requested, activeProfiles, emptyMap(), emptyList(), null, emptyList(), emptyList(), emptyList(), emptyList(), emptyList());
}

@JsonCreator
ResolvedPom(Pom requested, Iterable<String> activeProfiles, Map<String, String> properties, List<ResolvedManagedDependency> dependencyManagement, @Nullable List<MavenRepository> initialRepositories, List<MavenRepository> repositories, List<Dependency> requestedDependencies, List<Plugin> plugins, List<Plugin> pluginManagement) {
ResolvedPom(Pom requested, Iterable<String> activeProfiles, Map<String, String> properties, List<ResolvedManagedDependency> dependencyManagement, @Nullable List<MavenRepository> initialRepositories, List<MavenRepository> repositories, List<Dependency> requestedDependencies, List<Plugin> plugins, List<Plugin> pluginManagement, List<String> modules) {
this.requested = requested;
this.activeProfiles = activeProfiles;
this.properties = properties;
Expand All @@ -83,6 +83,7 @@ public ResolvedPom(Pom requested, Iterable<String> activeProfiles) {
this.requestedDependencies = requestedDependencies;
this.plugins = plugins;
this.pluginManagement = pluginManagement;
this.modules = modules;
}

@NonFinal
Expand Down Expand Up @@ -113,6 +114,10 @@ public ResolvedPom(Pom requested, Iterable<String> activeProfiles) {
@Builder.Default
List<Plugin> pluginManagement = emptyList();

@NonFinal
@Builder.Default
List<String> modules = emptyList();


/**
* Deduplicate dependencies and dependency management dependencies
Expand Down Expand Up @@ -172,6 +177,7 @@ public ResolvedPom resolve(ExecutionContext ctx, MavenPomDownloader downloader)
emptyList(),
emptyList(),
emptyList(),
emptyList(),
emptyList()
).resolver(ctx, downloader).resolve();

Expand Down Expand Up @@ -926,7 +932,7 @@ public List<ResolvedDependency> resolveDependencies(Scope scope, Map<GroupArtifa
ResolvedPom resolvedPom = cache.getResolvedDependencyPom(dPom.getGav());
if (resolvedPom == null) {
resolvedPom = new ResolvedPom(dPom, getActiveProfiles(), emptyMap(),
emptyList(), initialRepositories, emptyList(), emptyList(), emptyList(), emptyList());
emptyList(), initialRepositories, emptyList(), emptyList(), emptyList(), emptyList(), emptyList());
resolvedPom.resolver(ctx, downloader).resolveParentsRecursively(dPom);
cache.putResolvedDependencyPom(dPom.getGav(), resolvedPom);
}
Expand Down
164 changes: 161 additions & 3 deletions rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1055,9 +1055,6 @@ void preferRootPom() {
)
),
mavenProject("project1",
srcMainJava(
java(usingGuavaIntMath)
),
pomXml(
"""
<project>
Expand Down Expand Up @@ -1464,6 +1461,167 @@ class Foo {}
);
}

@Test
void addDependencyToParentPomWhenAggregatingPomIsNotParent() {
rewriteRun(
spec -> spec.recipe(new AddDependency(
"org.hamcrest",
"hamcrest-junit",
"2.0.0.0",
null,
"test",
null,
null,
null,
null,
null,
null,
true)),
mavenProject("my-app-aggregate",
pomXml(
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-aggregate</artifactId>
<version>1</version>
<modules>
<module>project-parent</module>
<module>project-child</module>
</modules>
</project>
""")),
mavenProject("project-parent",
pomXml(
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-parent</artifactId>
<version>1</version>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.1.5</version>
</parent>
</project>
""",
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-parent</artifactId>
<version>1</version>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.1.5</version>
</parent>
<dependencies>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-junit</artifactId>
<version>2.0.0.0</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
""")
),
mavenProject("my-app-child",
pomXml(
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-child</artifactId>
<version>1</version>
<parent>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-parent</artifactId>
<version>1</version>
</parent>
</project>
"""))
);
}

@Test
void addDependencyToAggregatingPomWhenAggregatingPomIsParent() {
rewriteRun(
spec -> spec.recipe(new AddDependency(
"org.hamcrest",
"hamcrest-junit",
"2.0.0.0",
null,
"test",
null,
null,
null,
null,
null,
null,
true)),
mavenProject("my-app-aggregate",
pomXml(
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-aggregate</artifactId>
<version>1</version>
<modules>
<module>project-parent</module>
<module>project-child</module>
</modules>
</project>
""",
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-aggregate</artifactId>
<version>1</version>
<modules>
<module>project-parent</module>
<module>project-child</module>
</modules>
<dependencies>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-junit</artifactId>
<version>2.0.0.0</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
""")),
mavenProject("project-parent",
pomXml(
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-parent</artifactId>
<version>1</version>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.1.5</version>
</parent>
</project>
""")
),
mavenProject("my-app-child",
pomXml(
"""
<project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-child</artifactId>
<version>1</version>
<parent>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app-aggregate</artifactId>
<version>1</version>
</parent>
</project>
"""))
);
}

private AddDependency addDependency(@SuppressWarnings("SameParameterValue") String gav) {
return addDependency(gav, null, null, null);
}
Expand Down
Loading