From 9a92e5267eb41e6cd5fc6bb8e29347e497e1e149 Mon Sep 17 00:00:00 2001 From: Jacob Danner Date: Thu, 13 Feb 2025 08:39:03 -0800 Subject: [PATCH] Proposed Fix for ProjectMerger.mergeMavenPlugins Issue https://github.com/spring-projects/spring-cli/issues/217 Discovered while working on a project that was trying to merge some openapi plugin tooling. The ConversionUtil code was returning strings with xml declarations and that was breaking and not working as expectd in the openrewrite recipes that modify the pom's plugins * Added a Test for ConversionUtil to demonstrate existing behavior * Added Test for expected ConversionUtil output * Updated ConversionUtil to avoid returning dependencies with default dependency content * Updated MavenModificationTests with additional validation around merged/added plugins Signed-off-by: Jacob Danner --- .../cli/util/ConversionUtils.java | 34 ++++++-- .../cli/merger/MavenModificationTests.java | 6 ++ .../cli/util/ConversionUtilsTest.java | 87 +++++++++++++++++++ 3 files changed, 119 insertions(+), 8 deletions(-) create mode 100644 src/test/java/org/springframework/cli/util/ConversionUtilsTest.java diff --git a/src/main/java/org/springframework/cli/util/ConversionUtils.java b/src/main/java/org/springframework/cli/util/ConversionUtils.java index 0c035dc6..0cc08df2 100644 --- a/src/main/java/org/springframework/cli/util/ConversionUtils.java +++ b/src/main/java/org/springframework/cli/util/ConversionUtils.java @@ -18,6 +18,8 @@ import java.io.StringWriter; import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; import javax.xml.bind.JAXB; @@ -33,17 +35,33 @@ private ConversionUtils() { } + /** + * Converts Xpp3Dom to a String without any xml declaration + * @param dom the Xpp3Dom to convert + * @return String without any Xml Declarations + */ public static String fromDomToString(Xpp3Dom dom) { String element = dom.toString(); - return element; + return element.lines().filter(l -> !l.contains(" dependencies) { StringWriter sw = new StringWriter(); + Dependencies deps = new Dependencies(dependencies); JAXB.marshal(deps, sw); String xmlString = sw.toString(); - return xmlString; + // filter out lines containing xml declarations and default elements like: + // false and jar + return xmlString.lines().filter(l -> !l.contains("false") && + !l.contains("jar")) + .collect(Collectors.joining("\n")); } /* @@ -52,18 +70,18 @@ public static String fromDependencyListToString(List dependencies) { */ public static class Dependencies { - private List dependencies; + private List dependency; public Dependencies(List dependencies) { - this.dependencies = dependencies; + this.dependency = dependencies; } - public List getDependencies() { - return dependencies; + public List getDependency() { + return dependency; } - public void setDependencies(List dependencies) { - this.dependencies = dependencies; + public void setDependency(List dependencies) { + this.dependency = dependencies; } } diff --git a/src/test/java/org/springframework/cli/merger/MavenModificationTests.java b/src/test/java/org/springframework/cli/merger/MavenModificationTests.java index f97da023..2e9a37f6 100644 --- a/src/test/java/org/springframework/cli/merger/MavenModificationTests.java +++ b/src/test/java/org/springframework/cli/merger/MavenModificationTests.java @@ -78,7 +78,11 @@ void addPluginDependency(@TempDir Path tempDir) throws Exception { pomReader.readPom(pomToMerge.toFile()), paths, mavenParser); Model mergedModel = pomReader.readPom(mergedPomPath.toFile()); + assertThat(mergedModel.getBuild().getPlugins()).hasSize(3); + List mergedPluginIds = new ArrayList<>(); for (Plugin plugin : mergedModel.getBuild().getPlugins()) { + + mergedPluginIds.add(plugin.getArtifactId()); if (plugin.getGroupId().equals("org.apache.maven.plugins") && plugin.getArtifactId().equals("maven-deploy-plugin")) { assertThat(ConversionUtils.fromDomToString((Xpp3Dom) plugin.getConfiguration())) @@ -98,6 +102,8 @@ else if (plugin.getGroupId().equals("org.springframework.boot") assertThat(dep.getArtifactId()).isEqualTo("spring-boot-thin-layout"); } } + assertThat(mergedPluginIds).containsExactlyInAnyOrder("maven-deploy-plugin", "maven-shade-plugin", + "spring-boot-maven-plugin"); } @Test diff --git a/src/test/java/org/springframework/cli/util/ConversionUtilsTest.java b/src/test/java/org/springframework/cli/util/ConversionUtilsTest.java new file mode 100644 index 00000000..cc7af325 --- /dev/null +++ b/src/test/java/org/springframework/cli/util/ConversionUtilsTest.java @@ -0,0 +1,87 @@ +package org.springframework.cli.util; + +import org.apache.maven.model.Dependency; +import org.codehaus.plexus.util.xml.Xpp3Dom; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +//@Disabled("This is a WIP Test") +class ConversionUtilsTest { + + + + @DisplayName("Characterization Test to validate how this current works, note the xml declaration AND the duplicate dependencies element") + @Disabled("This test exists to show what was actually coming out of ConversionUtil") + @Test + void existingFromDependencyListToString() { + List dependencies = new ArrayList<>(); + Dependency dependency = new Dependency(); + dependency.setGroupId("org.springframework"); + dependency.setArtifactId("spring-core"); + dependency.setVersion("5.2.0.RELEASE"); + dependencies.add(dependency); + String result = ConversionUtils.fromDependencyListToString(dependencies); + assertThat(result.replaceAll("\\s","")) + .isEqualTo("" + + "" + + "" + + "spring-core" + + "org.springframework" + + "false" + + "5.2.0.RELEASE" + + "" + + ""); + + } + + @DisplayName("After ConversionUtils is corrected, this test should pass") + @Test + void validateFromDependencyListToString() { + List dependencies = new ArrayList<>(); + Dependency dependency = new Dependency(); + dependency.setGroupId("org.springframework"); + dependency.setArtifactId("spring-core"); + dependency.setVersion("5.2.0.RELEASE"); + dependencies.add(dependency); + String result = ConversionUtils.fromDependencyListToString(dependencies); + assertThat(result.replaceAll("\\s","")) + .isEqualTo("" + + "" + + "spring-core" + + "org.springframework" + + "5.2.0.RELEASE" + + "" + + ""); + + } + + + @DisplayName("fromDomToString removes XML declaration") + @Test + void fromDomToStringRemovesXmlDeclaration() { + Xpp3Dom dom = new Xpp3Dom("root"); + Xpp3Dom child = new Xpp3Dom("child"); + child.setValue("value"); + dom.addChild(child); + String result = ConversionUtils.fromDomToString(dom); + assertThat(result).doesNotContain("").contains("").contains("value"); + } + + @DisplayName("fromDomToString handles empty Xpp3Dom") + @Test + void fromDomToStringHandlesEmptyXpp3Dom() { + Xpp3Dom dom = new Xpp3Dom("root"); + String result = ConversionUtils.fromDomToString(dom); + assertThat(result).isEqualTo(""); + } + + + + +} \ No newline at end of file