From 7e58389089d816db6bebdb8f76cab3d58e94c8cc Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 16 Jun 2023 08:54:47 +0200 Subject: [PATCH 1/3] [MNG-7632] Regression: combine.children breaks when combining executions --- .../maven/internal/xml/XmlNodeImpl.java | 10 +++---- .../maven/internal/xml/XmlNodeImplTest.java | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeImpl.java b/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeImpl.java index 7dbfc8344e51..dd507d6b6469 100644 --- a/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeImpl.java +++ b/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeImpl.java @@ -225,14 +225,14 @@ public static XmlNode merge(XmlNode dominant, XmlNode recessive, Boolean childMe String value = dominant.getValue(); Object location = dominant.getInputLocation(); - Map attrs = null; + Map attrs = dominant.getAttributes(); List children = null; for (Map.Entry attr : recessive.getAttributes().entrySet()) { String key = attr.getKey(); - if (isEmpty(dominant.getAttribute(key))) { - if (attrs == null) { - attrs = new HashMap<>(); + if (isEmpty(attrs.get(key))) { + if (attrs == dominant.getAttributes()) { + attrs = new HashMap<>(attrs); } attrs.put(key, attr.getValue()); } @@ -243,7 +243,7 @@ public static XmlNode merge(XmlNode dominant, XmlNode recessive, Boolean childMe if (childMergeOverride != null) { mergeChildren = childMergeOverride; } else { - String childMergeMode = dominant.getAttribute(CHILDREN_COMBINATION_MODE_ATTRIBUTE); + String childMergeMode = attrs.get(CHILDREN_COMBINATION_MODE_ATTRIBUTE); if (CHILDREN_COMBINATION_APPEND.equals(childMergeMode)) { mergeChildren = false; } diff --git a/maven-xml-impl/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java b/maven-xml-impl/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java index 88618ab5b154..c098683bac46 100644 --- a/maven-xml-impl/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java +++ b/maven-xml-impl/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java @@ -610,6 +610,36 @@ void testShouldRemoveDoNotRemoveTagWhenSwappedInputDOMs() throws Exception { assertEquals(recessiveConfig.toString(), result.toString()); } + @Test + void testMergeCombineChildrenAppendOnRecessive() throws XmlPullParserException, IOException { + String dominant = "\n" + " \n" + + " org.apache.shiro.crypto.CipherService\n" + + " org.apache.shiro.crypto.cipher.CipherService\n" + + " \n" + + ""; + String recessive = "\n" + + " \n" + + " javax.faces\n" + + " jakarta.faces\n" + + " \n" + + ""; + String expected = "\n" + + " \n" + + " javax.faces\n" + + " jakarta.faces\n" + + " \n" + + " \n" + + " org.apache.shiro.crypto.CipherService\n" + + " org.apache.shiro.crypto.cipher.CipherService\n" + + " \n" + + ""; + + XmlNodeImpl d = XmlNodeBuilder.build(new StringReader(dominant)); + XmlNodeImpl r = XmlNodeBuilder.build(new StringReader(recessive)); + XmlNode m = d.merge(r); + assertEquals(expected, m.toString()); + } + private static List getChildren(XmlNode node, String name) { return node.getChildren().stream().filter(n -> n.getName().equals(name)).collect(Collectors.toList()); } From eac012299e6cc9f1adc302af8c8ae05bb95d76f7 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 16 Jun 2023 09:06:31 +0200 Subject: [PATCH 2/3] Fix test on windows --- .../java/org/apache/maven/internal/xml/XmlNodeImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maven-xml-impl/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java b/maven-xml-impl/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java index c098683bac46..5fd8080a82f2 100644 --- a/maven-xml-impl/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java +++ b/maven-xml-impl/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java @@ -637,7 +637,7 @@ void testMergeCombineChildrenAppendOnRecessive() throws XmlPullParserException, XmlNodeImpl d = XmlNodeBuilder.build(new StringReader(dominant)); XmlNodeImpl r = XmlNodeBuilder.build(new StringReader(recessive)); XmlNode m = d.merge(r); - assertEquals(expected, m.toString()); + assertEquals(expected, m.toString().replaceAll("\r\n", "\n")); } private static List getChildren(XmlNode node, String name) { From 172f59e4e3d6188d60e5209bcc162dea09c32534 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 16 Jun 2023 10:29:11 +0200 Subject: [PATCH 3/3] Fix the check and do not recreate a map --- .../java/org/apache/maven/internal/xml/XmlNodeImpl.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeImpl.java b/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeImpl.java index dd507d6b6469..f40511279166 100644 --- a/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeImpl.java +++ b/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeImpl.java @@ -344,14 +344,7 @@ public static XmlNode merge(XmlNode dominant, XmlNode recessive, Boolean childMe } } - if (value != null || attrs != null || children != null) { - if (attrs != null) { - Map nattrs = attrs; - attrs = new HashMap<>(dominant.getAttributes()); - attrs.putAll(nattrs); - } else { - attrs = dominant.getAttributes(); - } + if (value != null || attrs != dominant.getAttributes() || children != null) { if (children == null) { children = dominant.getChildren(); }