[
https://issues.apache.org/jira/browse/MNG-7830?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17738086#comment-17738086
]
ASF GitHub Bot commented on MNG-7830:
-------------------------------------
elharo commented on code in PR #1185:
URL: https://github.com/apache/maven/pull/1185#discussion_r1245091437
##########
maven-model-builder/src/main/java/org/apache/maven/model/io/DefaultModelReader.java:
##########
@@ -120,23 +120,29 @@ private Model read(Reader reader, Path pomFile,
Map<String, ?> options) throws I
} else {
return readModel(transformingParser, strict);
}
- } catch (XmlPullParserException e) {
- throw new ModelParseException(e.getMessage(), e.getLineNumber(),
e.getColumnNumber(), e);
+ } catch (XMLStreamException e) {
+ Location location = e.getLocation();
+ throw new ModelParseException(
+ e.getMessage(),
+ location != null ? location.getLineNumber() : -1,
+ location != null ? location.getColumnNumber() : -1,
+ e);
} catch (IOException e) {
throw e;
} catch (Exception e) {
Review Comment:
This catch block might not be needed any more
##########
maven-model/src/test/java/org/apache/maven/model/v4/StaxTest.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.model.v4;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Objects;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.model.InputSource;
+import org.apache.maven.api.model.Model;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+class StaxTest {
+
+ @Test
+ @Disabled
+ void readAll() throws IOException {
+ Path userHome = Paths.get(System.getProperty("user.home"));
+ Stream<Path> poms = Files.walk(userHome.resolve(".m2/repository/"))
+ .filter(p -> p.getFileName().toString().endsWith(".pom") &&
Files.isRegularFile(p));
+ poms.forEach(pom -> {
+ Model stax = null, xpp3 = null;
+ try (InputStream is = Files.newInputStream(pom)) {
+ xpp3 = new MavenXpp3ReaderEx().read(is, false, new
InputSource("id", pom.toString()));
+ } catch (Exception e) {
+ System.err.println("Unable to parse using xpp3: " + pom + " ("
+ e.getMessage() + ")");
Review Comment:
The test should just fail here, or perhaps skip
##########
maven-model/src/test/java/org/apache/maven/model/v4/StaxTest.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.model.v4;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Objects;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.model.InputSource;
+import org.apache.maven.api.model.Model;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+class StaxTest {
+
+ @Test
+ @Disabled
+ void readAll() throws IOException {
+ Path userHome = Paths.get(System.getProperty("user.home"));
+ Stream<Path> poms = Files.walk(userHome.resolve(".m2/repository/"))
+ .filter(p -> p.getFileName().toString().endsWith(".pom") &&
Files.isRegularFile(p));
+ poms.forEach(pom -> {
+ Model stax = null, xpp3 = null;
+ try (InputStream is = Files.newInputStream(pom)) {
+ xpp3 = new MavenXpp3ReaderEx().read(is, false, new
InputSource("id", pom.toString()));
+ } catch (Exception e) {
+ System.err.println("Unable to parse using xpp3: " + pom + " ("
+ e.getMessage() + ")");
+ }
+ try (InputStream is = Files.newInputStream(pom)) {
+ stax = new MavenStaxReader().read(is, false, new
InputSource("id", pom.toString()));
+ } catch (Exception e) {
+ System.err.println("Unable to parse using stax: " + pom + " ("
+ e.getMessage() + ")");
+ }
+ if (stax != null && xpp3 != null) {
+ // fix encoding variations
+ if (stax.getModelEncoding() != null) {
+ stax = Model.newBuilder(stax, true)
+ .modelEncoding(
+
Charset.forName(stax.getModelEncoding()).toString())
+ .build();
+ }
+ if (xpp3.getModelEncoding() != null) {
+ xpp3 = Model.newBuilder(xpp3, true)
+ .modelEncoding(
+
Charset.forName(xpp3.getModelEncoding()).toString())
+ .build();
+ }
+ // compare models
+ try {
+ MavenXpp3Writer w = new MavenXpp3Writer();
+ StringWriter sw1 = new StringWriter();
+ StringWriter sw2 = new StringWriter();
+ w.write(sw1, xpp3);
+ w.write(sw2, stax);
+ String s1 = sw1.toString();
+ String s2 = sw2.toString();
+ if (!Objects.equals(s1, s2)) {
+ int idx = 0;
+ while (idx < s1.length() && idx < s2.length()) {
+ if (s1.charAt(idx) != s2.charAt(idx)) {
+ break;
+ }
+ idx++;
+ }
+ int nb = 25;
+ System.err.println("Mismatch reading: " + pom + " (at:
" + idx
+ + ", xpp3: '" + getSubstring(s1, idx, nb) + "'"
+ + " vs stax: '" + getSubstring(s2, idx, nb)
+ + "')");
+ }
+ } catch (IOException e) {
+ throw new RuntimeException("Error writing", e);
Review Comment:
no need to convert the exception; just bubble the IOException
##########
maven-model/src/test/java/org/apache/maven/model/v4/StaxTest.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.model.v4;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Objects;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.model.InputSource;
+import org.apache.maven.api.model.Model;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+class StaxTest {
Review Comment:
Looks like this might just be a tool during development. Do you want to
commit it to the repo?
##########
maven-model/src/test/java/org/apache/maven/model/v4/StaxTest.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.model.v4;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Objects;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.model.InputSource;
+import org.apache.maven.api.model.Model;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+class StaxTest {
+
+ @Test
+ @Disabled
Review Comment:
why disabled?
##########
maven-model-transform/pom.xml:
##########
@@ -32,6 +32,14 @@ under the License.
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-xml</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.codehaus.woodstox</groupId>
+ <artifactId>stax2-api</artifactId>
Review Comment:
Do we need stax2 or could we get away with the stax version in the JDK?
##########
maven-model-transform/src/main/java/org/apache/maven/model/transform/stax/XmlUtils.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.model.transform.stax;
+
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamConstants;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.stream.XMLStreamWriter;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+import com.ctc.wstx.api.WstxOutputProperties;
+
+public class XmlUtils {
+
+ public static InputStream writeDocument(XMLStreamReader parser) throws
XMLStreamException {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ writeDocument(parser, baos);
+ return new ByteArrayInputStream(baos.toByteArray());
+ }
+
+ public static void writeDocument(XMLStreamReader parser, OutputStream
output) throws XMLStreamException {
+ XMLOutputFactory factory = new com.ctc.wstx.stax.WstxOutputFactory();
+ factory.setProperty(XMLOutputFactory.IS_REPAIRING_NAMESPACES, false);
+
factory.setProperty(WstxOutputProperties.P_USE_DOUBLE_QUOTES_IN_XML_DECL, true);
+ XMLStreamWriter serializer = factory.createXMLStreamWriter(output,
parser.getCharacterEncodingScheme());
+ copy(parser, serializer);
+ }
+
+ private static String normalize(String input) {
+ if (input != null) {
+ return input.replace("\r\n", "\n");
+ }
+ return input;
+ }
+
+ /**
+ * Copies the reader to the writer. The start and end document methods must
+ * be handled on the writer manually.
+ */
+ public static void copy(XMLStreamReader reader, XMLStreamWriter writer)
throws XMLStreamException {
+ copy(reader, writer, false, false);
+ }
+
+ public static void copy(XMLStreamReader reader, XMLStreamWriter writer,
boolean fragment)
+ throws XMLStreamException {
+ copy(reader, writer, fragment, false);
+ }
+
+ public static void copy(XMLStreamReader reader, XMLStreamWriter writer,
boolean fragment, boolean isThreshold)
+ throws XMLStreamException {
+ // number of elements read in
+ int read = 0;
+ int elementCount = 0;
+ final Deque<Integer> countStack = new ArrayDeque<>();
+ int event = reader.getEventType();
+
+ while (true) {
+ switch (event) {
+ case XMLStreamConstants.START_ELEMENT:
+ read++;
+ if (isThreshold) {
+ elementCount++;
+ countStack.push(elementCount);
+ elementCount = 0;
+ }
+ writeStartElement(reader, writer);
+ break;
+ case XMLStreamConstants.END_ELEMENT:
+ if (read > 0) {
+ writer.writeEndElement();
+ }
+ read--;
+ if (read < 0 && fragment) {
+ return;
+ }
+ if (isThreshold && !countStack.isEmpty()) {
+ elementCount = countStack.pop();
+ }
+ break;
+ case XMLStreamConstants.CHARACTERS:
+ case XMLStreamConstants.SPACE:
+ String s = normalize(reader.getText());
+ if (s != null) {
+ writer.writeCharacters(normalize(s));
+ }
+ break;
+ case XMLStreamConstants.ENTITY_REFERENCE:
+ writer.writeEntityRef(reader.getLocalName());
+ break;
+ case XMLStreamConstants.COMMENT:
+ writer.writeComment(normalize(reader.getText()));
+ break;
+ case XMLStreamConstants.CDATA:
+ writer.writeCData(normalize(reader.getText()));
+ break;
+ case XMLStreamConstants.START_DOCUMENT:
+ if (reader.getVersion() != null) {
+
writer.writeStartDocument(reader.getCharacterEncodingScheme(),
reader.getVersion());
+ }
+ break;
+ case XMLStreamConstants.END_DOCUMENT:
+ writer.writeEndDocument();
+ return;
+ case XMLStreamConstants.ATTRIBUTE:
Review Comment:
break
##########
maven-model-transform/src/main/java/org/apache/maven/model/transform/stax/XmlUtils.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.model.transform.stax;
+
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamConstants;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.stream.XMLStreamWriter;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+import com.ctc.wstx.api.WstxOutputProperties;
+
+public class XmlUtils {
+
+ public static InputStream writeDocument(XMLStreamReader parser) throws
XMLStreamException {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ writeDocument(parser, baos);
+ return new ByteArrayInputStream(baos.toByteArray());
+ }
+
+ public static void writeDocument(XMLStreamReader parser, OutputStream
output) throws XMLStreamException {
+ XMLOutputFactory factory = new com.ctc.wstx.stax.WstxOutputFactory();
+ factory.setProperty(XMLOutputFactory.IS_REPAIRING_NAMESPACES, false);
+
factory.setProperty(WstxOutputProperties.P_USE_DOUBLE_QUOTES_IN_XML_DECL, true);
+ XMLStreamWriter serializer = factory.createXMLStreamWriter(output,
parser.getCharacterEncodingScheme());
+ copy(parser, serializer);
+ }
+
+ private static String normalize(String input) {
+ if (input != null) {
+ return input.replace("\r\n", "\n");
+ }
+ return input;
+ }
+
+ /**
+ * Copies the reader to the writer. The start and end document methods must
+ * be handled on the writer manually.
+ */
+ public static void copy(XMLStreamReader reader, XMLStreamWriter writer)
throws XMLStreamException {
+ copy(reader, writer, false, false);
+ }
+
+ public static void copy(XMLStreamReader reader, XMLStreamWriter writer,
boolean fragment)
+ throws XMLStreamException {
+ copy(reader, writer, fragment, false);
+ }
+
+ public static void copy(XMLStreamReader reader, XMLStreamWriter writer,
boolean fragment, boolean isThreshold)
+ throws XMLStreamException {
+ // number of elements read in
+ int read = 0;
+ int elementCount = 0;
+ final Deque<Integer> countStack = new ArrayDeque<>();
+ int event = reader.getEventType();
+
+ while (true) {
+ switch (event) {
+ case XMLStreamConstants.START_ELEMENT:
+ read++;
+ if (isThreshold) {
+ elementCount++;
+ countStack.push(elementCount);
+ elementCount = 0;
+ }
+ writeStartElement(reader, writer);
+ break;
+ case XMLStreamConstants.END_ELEMENT:
+ if (read > 0) {
+ writer.writeEndElement();
+ }
+ read--;
+ if (read < 0 && fragment) {
+ return;
+ }
+ if (isThreshold && !countStack.isEmpty()) {
+ elementCount = countStack.pop();
+ }
+ break;
+ case XMLStreamConstants.CHARACTERS:
Review Comment:
switch fall-through is error-prone during maintenance
> Switch from plexus-xml to stax / woodstox
> -----------------------------------------
>
> Key: MNG-7830
> URL: https://issues.apache.org/jira/browse/MNG-7830
> Project: Maven
> Issue Type: Dependency upgrade
> Reporter: Guillaume Nodet
> Assignee: Guillaume Nodet
> Priority: Major
> Fix For: 4.0.0-alpha-8
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
