[ 
https://issues.apache.org/jira/browse/GROOVY-12433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18128539#comment-18128539
 ] 

ASF GitHub Bot commented on GROOVY-12433:
-----------------------------------------

codeconsole commented on code in PR #2949:
URL: https://github.com/apache/groovy/pull/2949#discussion_r4308554932


##########
subprojects/groovy-xml/src/main/java/org/apache/groovy/xml/util/JacksonHelper.java:
##########
@@ -55,22 +59,39 @@ public static <T> T convertMapToType(Map<String, Object> 
map, Class<T> type) {
             return type.cast(cachedConvertValue.invoke(cachedMapper, map, 
type));
         } catch (XmlRuntimeException e) {
             throw e;
+        } catch (InvocationTargetException e) {
+            // report Jackson's own exception rather than the reflective 
wrapper
+            throw new XmlRuntimeException("Failed to convert XML map to " + 
type.getName(), e.getCause());
         } catch (Exception e) {
             throw new XmlRuntimeException("Failed to convert XML map to " + 
type.getName(), e);
         }
     }
 
     private static synchronized void initMapper() {
         if (cachedMapper != null) return;
-        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.");
-        }
+        Object mapper;
         try {
-            cachedMapper = omClass.getDeclaredConstructor().newInstance();
-            cachedConvertValue = omClass.getMethod("convertValue", 
Object.class, Class.class);
+            Class<?> jsonMapperClass = 
loadOptionalClass(JACKSON3_JSON_MAPPER_CLASS);
+            if (jsonMapperClass != null) {
+                // Jackson 2's defaults (for example failing on an unknown 
property), so a typed
+                // parse behaves the same whichever Jackson version the 
application carries
+                Object builder = 
jsonMapperClass.getMethod("builderWithJackson2Defaults").invoke(null);
+                mapper = builder.getClass().getMethod("build").invoke(builder);
+            } else {
+                Class<?> objectMapperClass = 
loadOptionalClass(JACKSON2_OBJECT_MAPPER_CLASS);

Review Comment:
   Added in a19601a: a `jackson2Test` task in groovy-xml, part of `check`, 
reruns `XmlParserTypedTest` on the test runtime classpath with Jackson 3 
swapped for Jackson 2.22.3 (the two classpaths differ only in the Jackson 
artifacts). That covers successful conversion and unknown-element rejection 
through the reflective fallback. `testRunsAgainstTheExpectedJackson` asserts 
which Jackson each run sees, so the task fails instead of silently retesting 
Jackson 3 if the swap breaks; removing the exclusion makes it fail.





> Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3
> ---------------------------------------------------------
>
>                 Key: GROOVY-12433
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12433
>             Project: Groovy
>          Issue Type: Improvement
>          Components: groovy-csv, groovy-toml, groovy-yaml, XML Processing
>            Reporter: Scott Murphy Heiberg
>            Priority: Major
>             Fix For: 7.0.0-alpha-1
>
>
> groovy-yaml, groovy-toml and groovy-csv are built on Jackson 2 
> ({{com.fasterxml.jackson}} 2.22.x). Spring Boot 4 uses Jackson 3 
> ({{tools.jackson}}) by default, so an application on Spring Boot 4 that uses 
> one of these modules carries both major versions and has to manage the 
> Jackson 2 one itself. Jackson 2 and 3 use different packages and coordinates, 
> so having Jackson 3 on the classpath does nothing for these modules.
> None of the three modules exposes a Jackson type in its public API, so they 
> can move to Jackson 3 without source changes for callers.
> Proposal:
> * Depend on {{tools.jackson}} 3.1.6, the version Spring Boot 4.2 manages. 
> {{jackson-datatype-jsr310}} is no longer needed; java.time support is built 
> into Jackson 3 databind.
> * Build every mapper with {{configureForJackson2()}} so databinding keeps 
> Jackson 2's defaults: properties in declaration order, unknown properties 
> rejected, nulls for primitives and trailing tokens accepted. The CSV mapper 
> also keeps {{SORT_PROPERTIES_ALPHABETICALLY}}, which Jackson 2's 
> {{CsvMapper}} enabled itself and the user guide documents.
> * Parser limits follow Jackson 3's defaults: nesting depth 500 (was 1000) and 
> string length 100,000,000 (was 20,000,000). Jackson lowered the depth after 
> fuzzers reached a StackOverflowError below 1000 (FasterXML/jackson-core#1233) 
> and raised the string length for legitimate large payloads 
> (FasterXML/jackson-core#1538). {{JsonSlurper}} and {{XmlParser}} keep their 
> 1000-level caps.
> * {{XmlParser.parseAs}} and {{parseTextAs}} load jackson-databind 
> reflectively; use Jackson 3 when present and fall back to Jackson 2.
> * Jackson 3's YAML backend uses SnakeYAML Engine rather than SnakeYAML, so 
> the SnakeYAML alias recipe from GROOVY-12358 needs {{org.yaml:snakeyaml}} 
> declared explicitly, as the user guide already anticipates.
> Jackson 3 still uses {{com.fasterxml.jackson.annotation}}, so 
> {{@JsonProperty}}, {{@JsonFormat}} and the other annotations in user code 
> keep working.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to