From f9d0a37d98474333f67b6a78a5f8a12ff2575e1c Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Tue, 3 Dec 2024 08:42:56 -0300 Subject: [PATCH 1/6] Patched an issue with formatting empty elements --- .../plugins/maven/operator/FormatCommand.java | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java index 511ceebc6..4b89666b8 100644 --- a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java +++ b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java @@ -9,6 +9,7 @@ import java.net.URISyntaxException; import java.nio.charset.Charset; import java.util.*; +import java.util.function.UnaryOperator; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -137,22 +138,22 @@ private BitSet elementBitSet(byte[] doc) throws XMLStreamException { * A Slight variation on writeAsUnicode from stax which writes as a regex string so we could * rewrite its output */ - private String writeAsRegex(StartElement element) { + private String writeAsRegex(final StartElement element, final List orderedAttributes) { StringWriter writer = new StringWriter(); writer.write("<"); writer.write(Pattern.quote(element.getName().getLocalPart())); - Iterator attrIter = element.getAttributes(); - while (attrIter.hasNext()) { - Attribute attr = (Attribute) attrIter.next(); - + for (var attr : orderedAttributes) { writer.write("\\s+"); writer.write(Pattern.quote(attr.getName().getLocalPart())); - writer.write("=[\\\"\']"); + writer.write("\\s*"); + writer.write("="); + writer.write("\\s*"); + writer.write("[\\\"']"); writer.write(Pattern.quote(attr.getValue())); - writer.write("[\\\"\']"); + writer.write("[\\\"']"); } writer.write("\\s*\\/>"); @@ -335,7 +336,25 @@ private void parseXmlAndCharset(POMDocument pomFile) throws XMLStreamException, new IntRange( realElementStart, realElementStart + 1 + trimmedOriginalContent.length()); - String contentRe = writeAsRegex(getLastStartElement(prevEvents)); + var element = getLastStartElement(prevEvents); + + // order the attributes by the original ordering + // attributes names are unique, we can just order them by the index of the name + + // Remove attribute contents, just in case some they contain the name of an attribute + // TODO should we trim the element name beforehand? + String contentRemoved = untrimmedOriginalContent.replaceAll("[\\\"'].*[\\\"']", ""); + + var it = element.getAttributes(); + var orderedAttributes = + Stream.iterate(it, Iterator::hasNext, UnaryOperator.identity()) + .map(Iterator::next) + .map(a -> new Pair<>(a, contentRemoved.indexOf(a.getName().getLocalPart()))) + .sorted(Comparator.comparing(p -> p.getSecond())) + .map(p -> p.getFirst()) + .collect(Collectors.toList()); + + String contentRe = writeAsRegex(element, orderedAttributes); Regex modifiedContentRE = new Regex(contentRe); @@ -397,7 +416,7 @@ private StartElement getLastStartElement(List prevEvents) { *

this is important so we can mix and match offsets and apply formatting accordingly * * @param xmlDocumentString Rendered POM Document Contents (string-formatted) - * @return map of (index, matchData object) reverse ordered + * @return map of (index, matchData object) reve/rse ordered */ private LinkedHashMap findSingleElementMatchesFrom(String xmlDocumentString) { Sequence allFoundMatchesSequence = @@ -494,6 +513,8 @@ private byte[] serializePomFile(POMDocument pom) throws XMLStreamException { // Let's find out the original empty elements from the original pom and store into a stack List elementsToReplace = getElementsToReplace(originalElementMap, pom); + // DOM parsers don't guarantee attribute ordering, extract the original ordering for the regex + // Lets to the replacements backwards on the existing, current pom Map emptyElements = getEmptyElements(targetElementMap, xmlRepresentation); From ea2a494d0e9421f3f1ac40b638f21c1dac885598 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Tue, 3 Dec 2024 09:22:53 -0300 Subject: [PATCH 2/6] Fixed bug with tests and added a new one --- .../maven/operator/AbstractTestBase.java | 2 +- .../maven/operator/POMOperatorTest.java | 12 ++ .../operator/pom-trimmed-roller-result.xml | 113 ++++++++++++++++++ .../maven/operator/pom-trimmed-roller.xml | 98 +++++++++++++++ 4 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller-result.xml create mode 100644 plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller.xml diff --git a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java index da091ba81..17558ca09 100644 --- a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java +++ b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java @@ -59,7 +59,7 @@ protected ProjectModel performAndAssertPomOperation( String testName, ProjectModel context, final OperationType operationType) throws Exception { String resultFile = "pom-" + testName + "-result.xml"; - URL resource = AbstractTestBase.class.getClass().getResource(resultFile); + URL resource = AbstractTestBase.class.getResource(resultFile); if (resource != null) { Document outcome = new SAXReader().read(resource); diff --git a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java index a49b90793..d8b87293e 100644 --- a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java +++ b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java @@ -581,6 +581,18 @@ void modify_adds_dependency_to_file_with_tabs() "\n\t\t\n\t\t\torg.dom4j\n\t\t\tdom4j\n\t\t\n"); } + @Test + void modify_adds_dependency_to_pom_with_empty_elements_with_multiple_attributes() throws Exception { + Dependency dependencyToUpgrade = + new Dependency("io.github.pixee", "java-security-toolkit", "1.0.2", null, null, null); + + performAndAssertModifyPomOperation( + "trimmed-roller", + ProjectModelFactory.load(POMOperatorTest.class.getResource("pom-trimmed-roller.xml")) + .withDependency(dependencyToUpgrade) + .withUseProperties(true)); + } + /** * Tests a scenario with an empty element from a customer's POM file and validates the resultant * POM. diff --git a/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller-result.xml b/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller-result.xml new file mode 100644 index 000000000..9f346c3b7 --- /dev/null +++ b/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller-result.xml @@ -0,0 +1,113 @@ + + + + + + + org.instancio + instancio-junit + 5.0.1 + test + + + io.github.pixee + java-security-toolkit + + + + + + roller + + + + + maven-antrun-plugin + 3.1.0 + + + ant-contrib + ant-contrib + ${maven-antrun.version} + + + ant + ant + + + + + + org.apache.velocity + velocity + 1.7 + + + + + + gen-db-scripts + generate-resources + + run + + + + + + + + + + + + + + + + + + + + + + + + + + + io.github.pixee + java-security-toolkit + ${versions.java-security-toolkit} + + + + + 1.0.2 + + diff --git a/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller.xml b/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller.xml new file mode 100644 index 000000000..faa5fc696 --- /dev/null +++ b/plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller.xml @@ -0,0 +1,98 @@ + + + + + + + org.instancio + instancio-junit + 5.0.1 + test + + + + + + roller + + + + + maven-antrun-plugin + 3.1.0 + + + ant-contrib + ant-contrib + ${maven-antrun.version} + + + ant + ant + + + + + + org.apache.velocity + velocity + 1.7 + + + + + + gen-db-scripts + generate-resources + + run + + + + + + + + + + + + + + + + + + + + + + + + + From a9dd2af3ea2c206a827b119e95d585c33bbc07de Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Tue, 3 Dec 2024 09:36:47 -0300 Subject: [PATCH 3/6] Spotless --- .../maven/operator/POMOperatorTest.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java index d8b87293e..4c8888ed8 100644 --- a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java +++ b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java @@ -581,17 +581,18 @@ void modify_adds_dependency_to_file_with_tabs() "\n\t\t\n\t\t\torg.dom4j\n\t\t\tdom4j\n\t\t\n"); } - @Test - void modify_adds_dependency_to_pom_with_empty_elements_with_multiple_attributes() throws Exception { - Dependency dependencyToUpgrade = - new Dependency("io.github.pixee", "java-security-toolkit", "1.0.2", null, null, null); + @Test + void modify_adds_dependency_to_pom_with_empty_elements_with_multiple_attributes() + throws Exception { + Dependency dependencyToUpgrade = + new Dependency("io.github.pixee", "java-security-toolkit", "1.0.2", null, null, null); - performAndAssertModifyPomOperation( - "trimmed-roller", - ProjectModelFactory.load(POMOperatorTest.class.getResource("pom-trimmed-roller.xml")) - .withDependency(dependencyToUpgrade) - .withUseProperties(true)); - } + performAndAssertModifyPomOperation( + "trimmed-roller", + ProjectModelFactory.load(POMOperatorTest.class.getResource("pom-trimmed-roller.xml")) + .withDependency(dependencyToUpgrade) + .withUseProperties(true)); + } /** * Tests a scenario with an empty element from a customer's POM file and validates the resultant From 05237eb66d6723567bd6109db609385568f019e8 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:12:00 -0300 Subject: [PATCH 4/6] Tests POMOperatorTest will now run sequentially --- .../io/codemodder/plugins/maven/operator/POMOperatorTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java index 4c8888ed8..35a712ddc 100644 --- a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java +++ b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java @@ -18,12 +18,15 @@ import org.dom4j.DocumentException; import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmlunit.diff.ComparisonType; import org.xmlunit.diff.Diff; import org.xmlunit.diff.Difference; +@Execution(ExecutionMode.SAME_THREAD) final class POMOperatorTest extends AbstractTestBase { private static final Logger LOGGER = LoggerFactory.getLogger(POMOperatorTest.class); From 6bf51f36ca013888ce53e8b4741bc456cb94a54c Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Tue, 3 Dec 2024 13:14:20 -0300 Subject: [PATCH 5/6] Fixed concurrency issue --- .../plugins/maven/operator/CommandChain.java | 54 +++++++++++++------ .../maven/operator/POMOperatorTest.java | 3 -- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java index 9ae936c10..dc03a16ed 100644 --- a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java +++ b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java @@ -16,18 +16,7 @@ class CommandChain { private static final Logger LOGGER = LoggerFactory.getLogger(CommandChain.class); /** Internal ArrayList of the Commands */ - private List commandList; - - private static List COMMON_COMMANDS = - List.of( - // Validation commands - CheckDependencyPresent.getInstance(), - CheckParentPackaging.getInstance(), - // Format commands - new FormatCommand(), - new DiscardFormatCommand(), - // Multipom command - new CompositeDependencyManagement()); + private final List commandList; private CommandChain(List commands) { this.commandList = commands; @@ -93,7 +82,17 @@ public boolean execute(ProjectModel c) * @return A pre-configured Chain for modifying a POM. */ public static CommandChain modifyDependency() { - final List modifyCommands = new ArrayList<>(COMMON_COMMANDS); + final List modifyCommands = + new ArrayList<>( + List.of( + // Validation commands + CheckDependencyPresent.getInstance(), + CheckParentPackaging.getInstance(), + // Format commands + new FormatCommand(), + new DiscardFormatCommand(), + // Multipom command + new CompositeDependencyManagement())); modifyCommands.addAll( List.of( SimpleUpgrade.getInstance(), @@ -109,7 +108,17 @@ public static CommandChain modifyDependency() { * @return A pre-configured Chain. */ public static CommandChain insertDependency() { - final List insertCommands = new ArrayList<>(COMMON_COMMANDS); + final List insertCommands = + new ArrayList<>( + List.of( + // Validation commands + CheckDependencyPresent.getInstance(), + CheckParentPackaging.getInstance(), + // Format commands + new FormatCommand(), + new DiscardFormatCommand(), + // Multipom command + new CompositeDependencyManagement())); insertCommands.add(new SimpleInsert(true)); return new CommandChain(insertCommands); } @@ -120,7 +129,17 @@ public static CommandChain insertDependency() { * @return A pre-configured Chain. */ public static CommandChain updateDependency() { - final List insertCommands = new ArrayList<>(COMMON_COMMANDS); + final List insertCommands = + new ArrayList<>( + List.of( + // Validation commands + CheckDependencyPresent.getInstance(), + CheckParentPackaging.getInstance(), + // Format commands + new FormatCommand(), + new DiscardFormatCommand(), + // Multipom command + new CompositeDependencyManagement())); insertCommands.addAll( List.of(SimpleUpgrade.getInstance(), SimpleDependencyManagement.getInstance())); @@ -169,7 +188,8 @@ public static CommandChain createForDependencyQuery(QueryType queryType) { return filterByQueryType( AVAILABLE_DEPENDENCY_QUERY_COMMANDS, queryType, - Arrays.asList(CheckLocalRepositoryDirCommand.CheckParentDirCommand.getInstance()), + Collections.singletonList( + CheckLocalRepositoryDirCommand.CheckParentDirCommand.getInstance()), it -> it == queryType); } @@ -196,7 +216,7 @@ public static CommandChain createForVersionQuery(QueryType queryType) { * and report issues creating */ static final List> AVAILABLE_DEPENDENCY_QUERY_COMMANDS = - new ArrayList<>(Arrays.asList(new Pair<>(QueryType.SAFE, "QueryByParsing"))); + new ArrayList<>(List.of(new Pair<>(QueryType.SAFE, "QueryByParsing"))); /** List of Commands for Version Query */ private static final List> AVAILABLE_QUERY_VERSION_COMMANDS = diff --git a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java index 35a712ddc..4c8888ed8 100644 --- a/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java +++ b/plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java @@ -18,15 +18,12 @@ import org.dom4j.DocumentException; import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.parallel.Execution; -import org.junit.jupiter.api.parallel.ExecutionMode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmlunit.diff.ComparisonType; import org.xmlunit.diff.Diff; import org.xmlunit.diff.Difference; -@Execution(ExecutionMode.SAME_THREAD) final class POMOperatorTest extends AbstractTestBase { private static final Logger LOGGER = LoggerFactory.getLogger(POMOperatorTest.class); From 4129d3ceb4cee253f9ae7b0c6f8bc7744673d14c Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Tue, 3 Dec 2024 13:36:14 -0300 Subject: [PATCH 6/6] Typo --- .../io/codemodder/plugins/maven/operator/FormatCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java index 4b89666b8..4ada1c5a7 100644 --- a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java +++ b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java @@ -416,7 +416,7 @@ private StartElement getLastStartElement(List prevEvents) { *

this is important so we can mix and match offsets and apply formatting accordingly * * @param xmlDocumentString Rendered POM Document Contents (string-formatted) - * @return map of (index, matchData object) reve/rse ordered + * @return map of (index, matchData object) reverse ordered */ private LinkedHashMap findSingleElementMatchesFrom(String xmlDocumentString) { Sequence allFoundMatchesSequence =