[
https://issues.apache.org/jira/browse/GROOVY-11927?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18073029#comment-18073029
]
ASF GitHub Bot commented on GROOVY-11927:
-----------------------------------------
Copilot commented on code in PR #2465:
URL: https://github.com/apache/groovy/pull/2465#discussion_r3070419290
##########
subprojects/groovy-xml/src/main/java/groovy/xml/XmlParser.java:
##########
@@ -293,6 +294,104 @@ public Node parseText(String text) throws IOException,
SAXException {
return parse(new StringReader(text));
}
+ /**
+ * Parse the content of the specified XML text into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ * Supports {@code @JsonProperty} and {@code @JsonFormat} annotations.
+ *
+ * @param type the target type
+ * @param text the XML text to parse
+ * @param <T> the target type
+ * @return a typed object
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseTextAs(Class<T> type, String text) {
+ return parseAs(type, new StringReader(text));
+ }
+
+ /**
+ * Parse XML from a reader into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ *
+ * @param type the target type
+ * @param reader the reader of XML
+ * @param <T> the target type
+ * @return a typed object
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseAs(Class<T> type, Reader reader) {
+ try {
+ Node root = parse(reader);
+ return JacksonHelper.convertMapToType(root.toMap(), type);
+ } catch (IOException | SAXException e) {
+ throw new XmlRuntimeException(e);
+ }
+ }
+
+ /**
+ * Parse XML from an input stream into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ *
+ * @param type the target type
+ * @param stream the input stream of XML
+ * @param <T> the target type
+ * @return a typed object
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseAs(Class<T> type, InputStream stream) {
+ try {
+ Node root = parse(stream);
+ return JacksonHelper.convertMapToType(root.toMap(), type);
+ } catch (IOException | SAXException e) {
+ throw new XmlRuntimeException(e);
+ }
+ }
+
+ /**
+ * Parse XML from a file into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ *
+ * @param type the target type
+ * @param file the XML file
+ * @param <T> the target type
+ * @return a typed object
+ * @throws IOException if the file cannot be read
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseAs(Class<T> type, File file) throws IOException {
+ try {
+ Node root = parse(file);
Review Comment:
parseAs(File) currently delegates to parse(file), which opens a new
InputStream via Files.newInputStream(...) without closing it. Since
parseAs(File) owns the resource, it should ensure the stream is closed (e.g.,
try-with-resources around Files.newInputStream + parsing via InputSource with
systemId) to avoid file-descriptor leaks.
```suggestion
try (InputStream stream = Files.newInputStream(file.toPath())) {
InputSource inputSource = new InputSource(stream);
inputSource.setSystemId(file.toURI().toString());
Node root = parse(inputSource);
```
##########
subprojects/groovy-xml/src/main/java/groovy/xml/XmlParser.java:
##########
@@ -293,6 +294,104 @@ public Node parseText(String text) throws IOException,
SAXException {
return parse(new StringReader(text));
}
+ /**
+ * Parse the content of the specified XML text into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ * Supports {@code @JsonProperty} and {@code @JsonFormat} annotations.
+ *
+ * @param type the target type
+ * @param text the XML text to parse
+ * @param <T> the target type
+ * @return a typed object
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseTextAs(Class<T> type, String text) {
+ return parseAs(type, new StringReader(text));
+ }
+
+ /**
+ * Parse XML from a reader into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ *
+ * @param type the target type
+ * @param reader the reader of XML
+ * @param <T> the target type
+ * @return a typed object
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseAs(Class<T> type, Reader reader) {
+ try {
+ Node root = parse(reader);
+ return JacksonHelper.convertMapToType(root.toMap(), type);
+ } catch (IOException | SAXException e) {
+ throw new XmlRuntimeException(e);
+ }
+ }
+
+ /**
+ * Parse XML from an input stream into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ *
+ * @param type the target type
+ * @param stream the input stream of XML
+ * @param <T> the target type
+ * @return a typed object
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseAs(Class<T> type, InputStream stream) {
+ try {
+ Node root = parse(stream);
+ return JacksonHelper.convertMapToType(root.toMap(), type);
+ } catch (IOException | SAXException e) {
+ throw new XmlRuntimeException(e);
+ }
+ }
+
+ /**
+ * Parse XML from a file into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ *
+ * @param type the target type
+ * @param file the XML file
+ * @param <T> the target type
+ * @return a typed object
+ * @throws IOException if the file cannot be read
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseAs(Class<T> type, File file) throws IOException {
+ try {
+ Node root = parse(file);
+ return JacksonHelper.convertMapToType(root.toMap(), type);
+ } catch (SAXException e) {
+ throw new XmlRuntimeException(e);
+ }
+ }
+
+ /**
+ * Parse XML from a path into a typed object.
+ * Requires jackson-databind on the classpath for type conversion.
+ *
+ * @param type the target type
+ * @param path the path to the XML file
+ * @param <T> the target type
+ * @return a typed object
+ * @throws IOException if the file cannot be read
+ * @throws XmlRuntimeException if parsing or conversion fails, or
jackson-databind is absent
+ * @since 6.0.0
+ */
+ public <T> T parseAs(Class<T> type, Path path) throws IOException {
+ try {
+ Node root = parse(path);
Review Comment:
parseAs(Path) currently delegates to parse(path), which opens a new
InputStream via Files.newInputStream(...) without closing it. Since
parseAs(Path) owns the resource, it should ensure the stream is closed (e.g.,
try-with-resources around Files.newInputStream + parsing via InputSource with
systemId) to avoid file-descriptor leaks.
```suggestion
try (InputStream stream = Files.newInputStream(path)) {
InputSource inputSource = new InputSource(stream);
inputSource.setSystemId(path.toUri().toString());
Node root = parse(inputSource);
```
##########
subprojects/groovy-xml/src/test/groovy/groovy/xml/XmlParserTypedTest.groovy:
##########
@@ -0,0 +1,153 @@
+/*
+ * 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 groovy.xml
+
+import groovy.util.Node
Review Comment:
Unused import: groovy.util.Node isn't referenced in this test class.
```suggestion
```
##########
subprojects/groovy-xml/src/main/java/org/apache/groovy/xml/util/JacksonHelper.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.groovy.xml.util;
+
+import groovy.xml.XmlRuntimeException;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+
+/**
+ * Internal helper for optional Jackson databinding support.
+ * Uses reflection to avoid a compile-time dependency on jackson-databind.
+ */
+public class JacksonHelper {
+
+ private static final String OBJECT_MAPPER_CLASS =
"com.fasterxml.jackson.databind.ObjectMapper";
+
+ /**
+ * Converts a Map to a typed object using Jackson's
ObjectMapper.convertValue.
+ * Requires jackson-databind on the classpath.
+ *
+ * @param map the source map
+ * @param type the target type
+ * @param <T> the target type
+ * @return the converted object
+ * @throws XmlRuntimeException if jackson-databind is not available or
conversion fails
+ */
+ public static <T> T convertMapToType(Map<String, Object> map, Class<T>
type) {
+ Class<?> omClass = loadOptionalClass(OBJECT_MAPPER_CLASS);
+ if (omClass == null) {
+ throw new XmlRuntimeException(
+ "Typed XML parsing requires jackson-databind on the
classpath. "
+ + "Add com.fasterxml.jackson.core:jackson-databind
to your dependencies.");
+ }
+ try {
+ Object mapper = omClass.getDeclaredConstructor().newInstance();
+ Method convertValue = omClass.getMethod("convertValue",
Object.class, Class.class);
+ return type.cast(convertValue.invoke(mapper, map, type));
Review Comment:
JacksonHelper.convertMapToType() creates a new ObjectMapper instance and
reflectively looks up convertValue on every call. ObjectMapper is thread-safe
after construction and reflective lookups are relatively expensive, so this
should be cached (e.g., lazily initialize and reuse a singleton mapper and
Method/handle) to avoid unnecessary overhead during repeated typed parsing.
##########
src/main/java/groovy/util/Node.java:
##########
@@ -870,6 +871,90 @@ public BigInteger toBigInteger() {
return StringGroovyMethods.toBigInteger((CharSequence)text());
}
+ /**
+ * The key used for text content when a node has both attributes and text.
+ *
+ * @since 6.0.0
+ */
+ public static final String TEXT_KEY = "_text";
+
+ /**
+ * Converts this Node tree into a nested {@code Map<String, Object>}.
+ * <p>
+ * Attributes and child elements are merged into a single map keyed by
name.
+ * If a child element name collides with an attribute name, the child
element wins.
+ * Leaf child elements (text only, no attributes) become String values.
+ * Non-leaf child elements and leaf elements with attributes become nested
Maps (recursive).
+ * Repeated same-name sibling elements become Lists.
+ * Elements with both attributes and text content use the {@value
#TEXT_KEY} key for the text.
+ *
+ * @return a Map representation of this Node
+ * @since 6.0.0
+ */
+ @SuppressWarnings("unchecked")
+ public Map<String, Object> toMap() {
+ Map<String, Object> map = new LinkedHashMap<>();
+
+ // copy attributes, tracking which keys came from attributes
+ java.util.Set<String> attributeKeys = new java.util.HashSet<>();
+ for (Map.Entry<Object, String> entry : ((Map<Object, String>)
attributes()).entrySet()) {
+ String key = nameAsString(entry.getKey());
+ map.put(key, entry.getValue());
+ attributeKeys.add(key);
+ }
Review Comment:
Node.toMap() currently casts attributes() to Map<Object, String> and
iterates Map.Entry<Object, String>. Since Node attributes are untyped (and can
be non-String for programmatically-built Nodes), this can throw
ClassCastException at runtime and also prevents preserving non-String attribute
values. Iterate over Map.Entry<Object, Object> (or raw Entry) and put the value
as Object to match the declared Map<String, Object> return type.
##########
subprojects/groovy-xml/src/test/groovy/groovy/xml/XmlParserTypedTest.groovy:
##########
@@ -0,0 +1,153 @@
+/*
+ * 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 groovy.xml
+
+import groovy.util.Node
+import org.junit.jupiter.api.Test
+
+class XmlParserTypedTest {
+
+ static class ServerConfig {
+ String host
+ int port
+ boolean debug
+ }
+
+ static class AppConfig {
+ String name
+ ServerConfig server
+ }
+
+ static class ClusterConfig {
+ String name
+ List<String> alias
+ }
+
Review Comment:
ClusterConfig is declared but never used in this test class; consider
removing it to keep the test focused on the covered scenarios.
```suggestion
```
> Add toMap() and typed parsing support to XmlParser
> --------------------------------------------------
>
> Key: GROOVY-11927
> URL: https://issues.apache.org/jira/browse/GROOVY-11927
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Add {{Node.toMap()}} for converting XML node trees to nested Maps, and
> {{XmlParser.parseTextAs()}} / {{parseAs()}} for typed XML parsing via Jackson
> databinding.
> Three levels of support:
> * `node.toMap()` — zero-dependency conversion to `Map<String, Object>`,
> usable standalone
> * `node as MyType` — coercion via `toMap()` + Groovy's named-param
> constructor (works for String-typed properties, no additional deps)
> * `parser.parseTextAs(Type, xml)` — full typed conversion (String→int,
> boolean, etc.) plus `@JsonProperty`/`@JsonFormat` support; requires
> `jackson-databind` at runtime, throws clear error if absent
> Also adds {{XmlRuntimeException}} and an internal {{JacksonHelper}} utility
> using reflective access to avoid a compile-time Jackson dependency on
> {{groovy-xml}}.
> {code:groovy}
> static class ServerConfig {
> String host
> int port
> boolean debug
> }
> def config = new XmlParser().parseTextAs(ServerConfig, '''
> <server>
> <host>localhost</host>
> <port>8080</port>
> <debug>true</debug>
> </server>'''.stripIndent())
> assert config instanceof ServerConfig
> assert config.host == 'localhost'
> assert config.port == 8080
> assert config.debug == true
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
