Skip to content

[#10939] Enable addLocationInformation in DefaultModelXmlFactory base on InputLocationFormatter presence #10940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[#10939] Enable addLocationInformation in DefaultModelXmlFactory base on InputLocationFormatter presence #10940

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
acbc713
[#10939] Enable addLocationInformation in DefaultModelXmlFactory base…
Giovds Jul 18, 2025
4773734
[#10939] Replace assertThat with assertEquals in DefaultModelXmlFacto…
Giovds Jul 20, 2025
d9a3399
[#10939] Refactor location tracking logic to depend on stringFormatte…
Giovds Jul 20, 2025
d521474
[#10939] Add non-formatter constructor to reflect original behaviour
Giovds Jul 20, 2025
db06137
[#10939] Apply Spotless
Giovds Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ public void write(XmlWriterRequest<Model> request) throws XmlWriterException {
}
try {
MavenStaxWriter w = new MavenStaxWriter();
w.setAddLocationInformation(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use an else block for this; what's the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MavenStaxWriter is initialized with true therefore I explicitly set it to false. I personally prefer to keep nesting brackets to a minimum, as this makes it less verbose and easier to understand what's happening. The result would be the same?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, no need to set it to true below. Use an else block to set it to false

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make it depend on the implementation of the MavenStaxWriter. If it changes the default this will affect our class.

I was thinking the same as @gnodet:

I wonder if this should be moved further down in the MavenStaxWriter and remove the addLocationInformation completely in that case.

If we use the proposed solution of moving this behaviour to the MavenStaxWriter this might solve both the need to set the value, and other XML factory implementations.

This is a generic generated class (throughwriter-stax.vm), so all ZZZStaxWriter classes (which are about 8 classes) will use this behaviour from then on. I'm not entirely sure about that change, but probably will be the better option as it will solve the issues for other DefaultZZZXmlFactory classes as well that use one of the ZZZStaxWriter's.

if (inputLocationFormatter != null) {
w.setAddLocationInformation(true);
w.setStringFormatter((Function) inputLocationFormatter);
}
if (writer != null) {
Expand Down
99 changes: 95 additions & 4 deletions impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultModelXmlFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,17 @@
package org.apache.maven.impl;

import java.io.StringReader;
import java.io.StringWriter;

import org.apache.maven.api.model.InputLocation;
import org.apache.maven.api.model.Model;
import org.apache.maven.api.services.xml.XmlReaderException;
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.services.xml.XmlWriterRequest;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand All @@ -40,7 +44,7 @@ void setUp() {
}

@Test
void testValidNamespaceWithModelVersion400() throws Exception {
void testValidNamespaceWithModelVersion400() {
String xml =
"""
<project xmlns="http://maven.apache.org/POM/4.0.0">
Expand All @@ -56,7 +60,7 @@ void testValidNamespaceWithModelVersion400() throws Exception {
}

@Test
void testValidNamespaceWithModelVersion410() throws Exception {
void testValidNamespaceWithModelVersion410() {
String xml =
"""
<project xmlns="http://maven.apache.org/POM/4.1.0">
Expand Down Expand Up @@ -88,7 +92,7 @@ void testInvalidNamespaceWithModelVersion410() {
}

@Test
void testNoNamespaceWithModelVersion400() throws Exception {
void testNoNamespaceWithModelVersion400() {
String xml =
"""
<project>
Expand All @@ -109,7 +113,7 @@ void testNullRequest() {
}

@Test
void testMalformedModelVersion() throws Exception {
void testMalformedModelVersion() {
String xml =
"""
<project xmlns="http://maven.apache.org/POM/4.0.0">
Expand All @@ -122,4 +126,91 @@ void testMalformedModelVersion() throws Exception {
Model model = factory.read(request);
assertEquals("invalid.version", model.getModelVersion());
}

@Test
void testWriteModelWithoutInputLocationTracking() {
String model =
"""
<project xmlns="http://maven.apache.org/POM/4.0.0">
<modelVersion>4.0.0</modelVersion>
<groupId>org.example</groupId>
<artifactId>example-artifact</artifactId>
<version>1.0.0</version>
<name>Example Project</name>
<properties>
<example.property>value</example.property>
</properties>
</project>""";

String expected =
"""
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>org.example</groupId>
<artifactId>example-artifact</artifactId>
<version>1.0.0</version>
<name>Example Project</name>
<properties>
<example.property>value</example.property>
</properties>
</project>""";

final StringWriter writer = new StringWriter();
XmlWriterRequest<Model> request = XmlWriterRequest.<Model>builder()
.content(factory.read(XmlReaderRequest.builder()
.reader(new StringReader(model))
.build()))
.writer(writer)
.build();

factory.write(request);

final String processedPomAsString = writer.toString();
assertThat(processedPomAsString).isEqualTo(expected);
}

@Test
void testWriteModelWithInputLocationTracking() {
String model =
"""
<project xmlns="http://maven.apache.org/POM/4.0.0">
<modelVersion>4.0.0</modelVersion>
<groupId>org.example</groupId>
<artifactId>example-artifact</artifactId>
<version>1.0.0</version>
<name>Example Project</name>
<properties>
<example.property>value</example.property>
</properties>
</project>""";

String expected =
"""
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion><!--some formatter: n/a @ 2:3-->
<groupId>org.example</groupId><!--some formatter: n/a @ 3:5-->
<artifactId>example-artifact</artifactId><!--some formatter: n/a @ 4:5-->
<version>1.0.0</version><!--some formatter: n/a @ 5:5-->
<name>Example Project</name><!--some formatter: n/a @ 6:5-->
<properties>
<example.property>value</example.property><!--some formatter: n/a @ 8:32-->
</properties>
</project>""";

final StringWriter writer = new StringWriter();
XmlWriterRequest<Model> request = XmlWriterRequest.<Model>builder()
.content(factory.read(XmlReaderRequest.builder()
.reader(new StringReader(model))
.build()))
.writer(writer)
.inputLocationFormatter((x) -> "some formatter: " + (InputLocation) x)
.build();

factory.write(request);

final String processedPomAsString = writer.toString();
assertThat(processedPomAsString).isEqualTo(expected);
}
}
Loading