Skip to content

Commit c0d0f67

Browse files
timtebeektimtebeek
authored
Skip AddMockitoJavaAgentToMavenSurefirePlugin when agent already configured in build/plugins (#1166)
* Skip AddMockitoJavaAgentToMavenSurefirePlugin when agent already configured in build/plugins (#1164) * Minimize comments * Simplify plugin matching; tolerate implied plugin groupId
1 parent 12386c8 commit c0d0f67

2 files changed

Lines changed: 176 additions & 8 deletions

File tree

‎src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import lombok.Getter;
1919
import lombok.RequiredArgsConstructor;
2020
import org.intellij.lang.annotations.Language;
21+
import org.jspecify.annotations.Nullable;
2122
import org.openrewrite.ExecutionContext;
2223
import org.openrewrite.Preconditions;
2324
import org.openrewrite.Recipe;
@@ -49,6 +50,7 @@ public class AddMockitoJavaAgentToMavenSurefirePlugin extends Recipe {
4950

5051
private static final String MAVEN_PLUGINS_GROUP_ID = "org.apache.maven.plugins";
5152
private static final String MAVEN_SUREFIRE_PLUGIN_ARTIFACT_ID = "maven-surefire-plugin";
53+
private static final String MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID = "maven-dependency-plugin";
5254

5355
@Language("xpath")
5456
private static final String MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER = "/project/build/plugins/plugin[artifactId='maven-dependency-plugin']/executions/execution";
@@ -85,28 +87,46 @@ private Xml.Tag buildConfigurationTag(String argLineJavaAgentParam, boolean hasE
8587

8688
private void maybeAddMavenDependencyPluginWithPropertiesGoal() {
8789
Optional<Plugin> mavenDependencyPlugin = getResolutionResult().getPom().getPlugins().stream()
88-
.filter(plugin -> "org.apache.maven.plugins".equals(plugin.getGroupId()) &&
89-
"maven-dependency-plugin".equals(plugin.getArtifactId())).findFirst();
90+
.filter(plugin -> isMavenPlugin(plugin, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID)).findFirst();
9091

9192
if (!mavenDependencyPlugin.isPresent()) {
92-
doAfterVisit(new AddPlugin("org.apache.maven.plugins", "maven-dependency-plugin", null, null, null,
93+
doAfterVisit(new AddPlugin(MAVEN_PLUGINS_GROUP_ID, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID, null, null, null,
9394
"<executions>" + MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG + "</executions>", "**/pom.xml").getVisitor());
9495
} else if (mavenDependencyPlugin.get().getExecutions().isEmpty()) {
95-
doAfterVisit(new ChangePluginExecutions("org.apache.maven.plugins", "maven-dependency-plugin", MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG).getVisitor());
96+
doAfterVisit(new ChangePluginExecutions(MAVEN_PLUGINS_GROUP_ID, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID, MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG).getVisitor());
9697
} else if (mavenDependencyPlugin.get().getExecutions().stream().noneMatch(execution -> execution.getGoals() != null)) {
9798
doAfterVisit(new AddOrUpdateChildTag(MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER, "<goals>" + MAVEN_DEPENDENCY_PLUGIN_PROPERTIES_GOAL + "</goals>", false).getVisitor());
98-
} else if (mavenDependencyPlugin.get().getExecutions().stream().noneMatch(execution -> execution.getGoals() != null && execution.getGoals().contains("properties"))) {
99+
} else if (!hasPropertiesGoal(mavenDependencyPlugin.get())) {
99100
doAfterVisit(new AppendChildTagToParentVisitor(
100101
new XPathMatcher(MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER + "/goals"),
101102
Xml.Tag.build(MAVEN_DEPENDENCY_PLUGIN_PROPERTIES_GOAL)));
102103
}
103104
}
104105

106+
private @Nullable String surefireArgLineWithAgent(List<Plugin> plugins) {
107+
String agentArgument = getArgLineJavaAgentArgument();
108+
return plugins.stream()
109+
.filter(plugin -> isMavenPlugin(plugin, MAVEN_SUREFIRE_PLUGIN_ARTIFACT_ID))
110+
.map(plugin -> plugin.getConfigurationStringValue("argLine"))
111+
.filter(argLine -> argLine != null && argLine.contains(agentArgument))
112+
.findFirst().orElse(null);
113+
}
114+
115+
private boolean mavenDependencyPluginHasPropertiesGoal() {
116+
return getResolutionResult().getPom().getPlugins().stream()
117+
.anyMatch(plugin -> isMavenPlugin(plugin, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID) && hasPropertiesGoal(plugin));
118+
}
119+
105120
@Override
106121
public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {
107-
if (getResolutionResult().getPom().getPluginManagement().stream().anyMatch(
108-
plugin -> "org.apache.maven.plugins".equals(plugin.getGroupId()) && "maven-surefire-plugin"
109-
.equals(plugin.getArtifactId()) && plugin.getConfigurationStringValue("argLine") != null && plugin.getConfigurationStringValue("argLine").contains(getArgLineJavaAgentArgument()))) {
122+
if (surefireArgLineWithAgent(getResolutionResult().getPom().getPluginManagement()) != null) {
123+
return document;
124+
}
125+
// Nothing to add when the surefire agent, the properties goal, and any `@{argLine}` property
126+
// are already present, whether declared here or inherited from a parent's `build/plugins` (#1164).
127+
String pluginArgLine = surefireArgLineWithAgent(getResolutionResult().getPom().getPlugins());
128+
if (pluginArgLine != null && mavenDependencyPluginHasPropertiesGoal() &&
129+
(!pluginArgLine.contains("@{argLine}") || getResolutionResult().getPom().getProperties().containsKey("argLine"))) {
110130
return document;
111131
}
112132

@@ -168,6 +188,20 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
168188
});
169189
}
170190

191+
/**
192+
* Matches a plugin under the {@code org.apache.maven.plugins} group, tolerating an implied (omitted) groupId,
193+
* which Maven defaults to that group for {@code maven-*-plugin} declarations.
194+
*/
195+
private static boolean isMavenPlugin(Plugin plugin, String artifactId) {
196+
return artifactId.equals(plugin.getArtifactId()) &&
197+
(plugin.getGroupId() == null || MAVEN_PLUGINS_GROUP_ID.equals(plugin.getGroupId()));
198+
}
199+
200+
private static boolean hasPropertiesGoal(Plugin plugin) {
201+
return plugin.getExecutions().stream()
202+
.anyMatch(execution -> execution.getGoals() != null && execution.getGoals().contains("properties"));
203+
}
204+
171205
@RequiredArgsConstructor
172206
private static class AppendChildTagToParentVisitor extends XmlIsoVisitor<ExecutionContext> {
173207
private final XPathMatcher parentXPathMatcher;

‎src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,6 +1372,140 @@ void updatesIndividualPomsWhenParentPomManagesSurefirePluginWithoutAgentConfigur
13721372
);
13731373
}
13741374

1375+
@Test
1376+
void makesNoChangesWhenParentPomDeclaresSurefirePluginWithAgentConfiguration() {
1377+
rewriteRun(
1378+
mavenProject("test-project",
1379+
pomXml(
1380+
"""
1381+
<project>
1382+
<modelVersion>4.0.0</modelVersion>
1383+
<groupId>org.sample</groupId>
1384+
<artifactId>test</artifactId>
1385+
<version>1.0</version>
1386+
<packaging>pom</packaging>
1387+
1388+
<modules>
1389+
<module>test-module1</module>
1390+
</modules>
1391+
1392+
<parent>
1393+
<groupId>org.springframework.boot</groupId>
1394+
<artifactId>spring-boot-starter-parent</artifactId>
1395+
<version>3.5.4</version>
1396+
<relativePath/>
1397+
</parent>
1398+
1399+
<dependencies>
1400+
<dependency>
1401+
<groupId>org.springframework.boot</groupId>
1402+
<artifactId>spring-boot-starter-test</artifactId>
1403+
<scope>test</scope>
1404+
</dependency>
1405+
</dependencies>
1406+
<build>
1407+
<plugins>
1408+
<plugin>
1409+
<groupId>org.apache.maven.plugins</groupId>
1410+
<artifactId>maven-dependency-plugin</artifactId>
1411+
<executions>
1412+
<execution>
1413+
<goals>
1414+
<goal>properties</goal>
1415+
</goals>
1416+
</execution>
1417+
</executions>
1418+
</plugin>
1419+
<plugin>
1420+
<groupId>org.apache.maven.plugins</groupId>
1421+
<artifactId>maven-surefire-plugin</artifactId>
1422+
<configuration>
1423+
<!--suppress MavenModelInspection -->
1424+
<argLine>-Xmx204m -javaagent:${org.mockito:mockito-core:jar}</argLine>
1425+
</configuration>
1426+
</plugin>
1427+
</plugins>
1428+
</build>
1429+
</project>
1430+
""",
1431+
spec -> spec.path("pom.xml")
1432+
),
1433+
pomXml(
1434+
"""
1435+
<project>
1436+
<modelVersion>4.0.0</modelVersion>
1437+
<groupId>org.sample</groupId>
1438+
<artifactId>test-module1</artifactId>
1439+
<version>1.0</version>
1440+
1441+
<parent>
1442+
<groupId>org.sample</groupId>
1443+
<artifactId>test</artifactId>
1444+
<version>1.0</version>
1445+
<relativePath>../pom.xml</relativePath>
1446+
</parent>
1447+
</project>
1448+
""",
1449+
spec -> spec.path("test-module1/pom.xml")
1450+
)
1451+
)
1452+
);
1453+
}
1454+
1455+
@Test
1456+
void makesNoChangesWhenAgentConfiguredOnPluginsWithImpliedGroupId() {
1457+
rewriteRun(
1458+
mavenProject("test-project",
1459+
pomXml(
1460+
"""
1461+
<project>
1462+
<modelVersion>4.0.0</modelVersion>
1463+
<groupId>org.sample</groupId>
1464+
<artifactId>test</artifactId>
1465+
<version>1.0</version>
1466+
1467+
<parent>
1468+
<groupId>org.springframework.boot</groupId>
1469+
<artifactId>spring-boot-starter-parent</artifactId>
1470+
<version>3.5.4</version>
1471+
<relativePath/>
1472+
</parent>
1473+
1474+
<dependencies>
1475+
<dependency>
1476+
<groupId>org.springframework.boot</groupId>
1477+
<artifactId>spring-boot-starter-test</artifactId>
1478+
<scope>test</scope>
1479+
</dependency>
1480+
</dependencies>
1481+
<build>
1482+
<plugins>
1483+
<plugin>
1484+
<artifactId>maven-dependency-plugin</artifactId>
1485+
<executions>
1486+
<execution>
1487+
<goals>
1488+
<goal>properties</goal>
1489+
</goals>
1490+
</execution>
1491+
</executions>
1492+
</plugin>
1493+
<plugin>
1494+
<artifactId>maven-surefire-plugin</artifactId>
1495+
<configuration>
1496+
<!--suppress MavenModelInspection -->
1497+
<argLine>-Xmx204m -javaagent:${org.mockito:mockito-core:jar}</argLine>
1498+
</configuration>
1499+
</plugin>
1500+
</plugins>
1501+
</build>
1502+
</project>
1503+
"""
1504+
)
1505+
)
1506+
);
1507+
}
1508+
13751509
@Test
13761510
void augmentsSurefirePluginDeclaredInPluginManagement() {
13771511
rewriteRun(

0 commit comments

Comments
 (0)