elharo commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2981126677
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode
node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String>
attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> attr : attributes.entrySet()) {
+ String key = attr.getKey();
+ if ("xmlns".equals(key)) {
+ xmlWriter.writeDefaultNamespace(attr.getValue());
+ } else if (key.startsWith("xmlns:")) {
+ xmlWriter.writeNamespace(key.substring(6), attr.getValue());
+ }
+ }
+ for (Map.Entry<String, String> attr : attributes.entrySet()) {
Review Comment:
attr --> attribute
##########
impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java:
##########
@@ -715,6 +717,198 @@ public Object toInputLocation(XMLStreamReader parser) {
}
}
+ /**
+ * Verifies that when an XmlNode has xmlns:prefix declarations and prefixed
+ * attributes, the write side produces valid XML with proper namespace
handling.
+ */
+ @Test
+ void testWriteWithNamespaceDeclarationsAndPrefixedAttributes() throws
Exception {
+ String xml = """
+ <project xmlns="http://maven.apache.org/POM/4.0.0";
xmlns:mvn="http://maven.apache.org/POM/4.0.0";>
+ <compilerArgs mvn:combine.children="append">
+ <arg>-Xlint:deprecation</arg>
+ </compilerArgs>
+ </project>
+ """;
+
+ XmlNode node = toXmlNode(xml);
+
+ // The xmlns:mvn declaration should be preserved in the parsed node
+ assertEquals("http://maven.apache.org/POM/4.0.0";,
node.attribute("xmlns:mvn"));
+
+ // Write and re-read to verify round-trip produces valid XML
+ java.io.StringWriter writer = new java.io.StringWriter();
+ XmlService.write(node, writer);
+ String output = writer.toString();
+
+ // The output should be parseable XML (no undeclared namespace prefix
errors)
+ XmlNode reRead = toXmlNode(output);
+ assertNotNull(reRead);
+ }
+
+ /**
+ * Verifies that when a prefixed attribute has no corresponding xmlns
declaration
+ * (as happens in consumer POM transformation), the prefix is stripped on
write
+ * to produce valid XML.
+ */
+ @Test
+ void testWriteStripsOrphanedPrefixOnAttributes() throws Exception {
+ // Simulate a consumer POM scenario: mvn:combine.children exists
+ // but xmlns:mvn was lost during model transformation
+ XmlNode node = XmlNode.newBuilder()
+ .name("compilerArgs")
+ .attributes(java.util.Map.of("mvn:combine.children", "append"))
+ .children(java.util.List.of(XmlNode.newBuilder()
+ .name("arg")
+ .value("-Xlint:deprecation")
+ .build()))
+ .build();
+
+ java.io.StringWriter writer = new java.io.StringWriter();
+ XmlService.write(node, writer);
+ String output = writer.toString();
+
+ // Should not contain the undeclared mvn: prefix
+ assertFalse(output.contains("mvn:combine"), "Output should not contain
orphaned mvn: prefix");
+ // Should contain the unprefixed attribute instead
+ assertTrue(output.contains("combine.children=\"append\""), "Attribute
should be written unprefixed");
+
+ // The output should be parseable XML
+ XmlNode reRead = toXmlNode(output);
+ assertNotNull(reRead);
+ assertEquals("append", reRead.attribute("combine.children"));
+ }
+
+ /**
+ * Verifies that foreign namespace prefixed attributes round-trip correctly
+ * when the xmlns declaration is on the same element as the prefixed
attribute.
+ */
+ @Test
+ void testWriteForeignNamespaceAttributeRoundTrip() throws Exception {
+ // Build a node where xmlns:custom and custom:myattr are on the same
element
+ XmlNode node = XmlNode.newBuilder()
+ .name("compilerArgs")
+ .attributes(java.util.Map.of(
+ "xmlns:custom", "http://example.com/custom";,
+ "custom:myattr", "value"))
+ .children(java.util.List.of(XmlNode.newBuilder()
+ .name("arg")
+ .value("-Xlint:deprecation")
+ .build()))
+ .build();
+
+ java.io.StringWriter writer = new java.io.StringWriter();
+ XmlService.write(node, writer);
+ String output = writer.toString();
+
+ XmlNode reRead = toXmlNode(output);
+ assertNotNull(reRead);
+ assertEquals("value", reRead.attribute("custom:myattr"));
+ assertEquals("http://example.com/custom";,
reRead.attribute("xmlns:custom"));
+ }
+
+ /**
+ * Verifies that when a prefixed attribute's xmlns declaration is on a
parent
Review Comment:
Not sure what this is trying to test but this doesn't sound right. I don't
think the prefix should be removed.
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode
node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String>
attributes) throws XMLStreamException {
Review Comment:
This looks like it can be private
##########
impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java:
##########
@@ -715,6 +717,198 @@ public Object toInputLocation(XMLStreamReader parser) {
}
}
+ /**
+ * Verifies that when an XmlNode has xmlns:prefix declarations and prefixed
+ * attributes, the write side produces valid XML with proper namespace
handling.
+ */
+ @Test
+ void testWriteWithNamespaceDeclarationsAndPrefixedAttributes() throws
Exception {
+ String xml = """
+ <project xmlns="http://maven.apache.org/POM/4.0.0";
xmlns:mvn="http://maven.apache.org/POM/4.0.0";>
+ <compilerArgs mvn:combine.children="append">
+ <arg>-Xlint:deprecation</arg>
+ </compilerArgs>
+ </project>
+ """;
+
+ XmlNode node = toXmlNode(xml);
+
+ // The xmlns:mvn declaration should be preserved in the parsed node
+ assertEquals("http://maven.apache.org/POM/4.0.0";,
node.attribute("xmlns:mvn"));
+
+ // Write and re-read to verify round-trip produces valid XML
+ java.io.StringWriter writer = new java.io.StringWriter();
+ XmlService.write(node, writer);
+ String output = writer.toString();
+
+ // The output should be parseable XML (no undeclared namespace prefix
errors)
+ XmlNode reRead = toXmlNode(output);
+ assertNotNull(reRead);
+ }
+
+ /**
+ * Verifies that when a prefixed attribute has no corresponding xmlns
declaration
+ * (as happens in consumer POM transformation), the prefix is stripped on
write
+ * to produce valid XML.
+ */
+ @Test
+ void testWriteStripsOrphanedPrefixOnAttributes() throws Exception {
+ // Simulate a consumer POM scenario: mvn:combine.children exists
+ // but xmlns:mvn was lost during model transformation
+ XmlNode node = XmlNode.newBuilder()
+ .name("compilerArgs")
+ .attributes(java.util.Map.of("mvn:combine.children", "append"))
+ .children(java.util.List.of(XmlNode.newBuilder()
+ .name("arg")
+ .value("-Xlint:deprecation")
+ .build()))
+ .build();
+
+ java.io.StringWriter writer = new java.io.StringWriter();
+ XmlService.write(node, writer);
+ String output = writer.toString();
+
+ // Should not contain the undeclared mvn: prefix
+ assertFalse(output.contains("mvn:combine"), "Output should not contain
orphaned mvn: prefix");
+ // Should contain the unprefixed attribute instead
+ assertTrue(output.contains("combine.children=\"append\""), "Attribute
should be written unprefixed");
+
+ // The output should be parseable XML
+ XmlNode reRead = toXmlNode(output);
+ assertNotNull(reRead);
+ assertEquals("append", reRead.attribute("combine.children"));
+ }
+
+ /**
+ * Verifies that foreign namespace prefixed attributes round-trip correctly
+ * when the xmlns declaration is on the same element as the prefixed
attribute.
+ */
+ @Test
+ void testWriteForeignNamespaceAttributeRoundTrip() throws Exception {
+ // Build a node where xmlns:custom and custom:myattr are on the same
element
+ XmlNode node = XmlNode.newBuilder()
+ .name("compilerArgs")
+ .attributes(java.util.Map.of(
+ "xmlns:custom", "http://example.com/custom";,
+ "custom:myattr", "value"))
+ .children(java.util.List.of(XmlNode.newBuilder()
+ .name("arg")
+ .value("-Xlint:deprecation")
+ .build()))
+ .build();
+
+ java.io.StringWriter writer = new java.io.StringWriter();
Review Comment:
I'd just import this class instead of using the fully package qualified name
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode
node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String>
attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> attr : attributes.entrySet()) {
+ String key = attr.getKey();
+ if ("xmlns".equals(key)) {
+ xmlWriter.writeDefaultNamespace(attr.getValue());
+ } else if (key.startsWith("xmlns:")) {
+ xmlWriter.writeNamespace(key.substring(6), attr.getValue());
+ }
+ }
+ for (Map.Entry<String, String> attr : attributes.entrySet()) {
+ String key = attr.getKey();
+ String value = attr.getValue();
+ if ("xmlns".equals(key) || key.startsWith("xmlns:")) {
+ continue; // already written above
+ } else if (key.startsWith("xml:")) {
Review Comment:
The xml prefix is predefined and understood by all parsers. It does not need
to be (must not be?) explicitly declared. You do need the else if case here to
keep it from being written, but it should be empty.
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode
node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String>
attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> attr : attributes.entrySet()) {
Review Comment:
Not sure about what XML library you're using here. Some do not include
namespace declarations in their attribute map. If you're using DOM I think it
depends on how the parser is configured, though I'd really need to review to
make remember exactly how that works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
