You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "codeconsole (via GitHub)" <gi...@apache.org> on 2026/09/26 04:06:22 UTC

[PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

codeconsole opened a new pull request, #2949:
URL: https://github.com/apache/groovy/pull/2949

   https://issues.apache.org/jira/browse/GROOVY-12433
   
   Moves groovy-yaml, groovy-toml and groovy-csv from Jackson 2 to Jackson 3 (`tools.jackson` 3.1.6, the version Spring Boot 4.2 manages). None of them exposes a Jackson type, so callers need no changes.
   
   - Mappers are built with `configureForJackson2()` so parsing and serialization behave as before. The CSV mapper also enables `SORT_PROPERTIES_ALPHABETICALLY`, which Jackson 2's `CsvMapper` turned on itself: plain classes keep alphabetical columns and records keep declaration order, as the user guide documents.
   - `jackson-datatype-jsr310` is dropped; java.time support is built into Jackson 3 databind.
   - `XmlParser.parseAs`/`parseTextAs` use Jackson 3 when present and fall back to Jackson 2. A failed conversion now carries Jackson's exception; from dynamic Groovy it used to surface as an `InvocationTargetException` with no message. The cached mapper is now published after its method, so a concurrent first call can't see a null method.
   - The SnakeYAML alias recipe in the YAML guide now declares `org.yaml:snakeyaml`, since Jackson 3's YAML backend uses SnakeYAML Engine.
   - `tools.jackson` and `snakeyaml-engine` are signed with keys the dependency verification already trusts; two scoped trust entries are added.
   - The JSON and XML guide recipes for using Jackson directly now use the Jackson 3 artifacts.
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6255277932

   ## ✅ All tests passed ✅
   
   🏷️ Commit: a19601af1aa9aa5b8f4daac449f9ae9eb07c3d88
   ▶️ Tests:  122246 executed
   ⚪️ Checks: 33/33 completed
   
   ---
   _Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2949:
URL: https://github.com/apache/groovy/pull/2949#discussion_r4306229546


##########
subprojects/groovy-yaml/build.gradle:
##########
@@ -22,12 +22,12 @@ plugins {
 
 dependencies {
     api project(':')  // YamlBuilder extends GroovyObjectSupport...
-    implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${versions.jackson}"
-    implementation "com.fasterxml.jackson.core:jackson-databind:${versions.jackson}"
-    implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson}"
+    implementation "tools.jackson.dataformat:jackson-dataformat-yaml:${versions.jackson3}"

Review Comment:
   The backend upgrade changes YAML scalar resolution, which `configureForJackson2()` does not restore. For example, `new YamlSlurper().parseText('enabled: off').enabled` changes from Boolean `false` to the string `'off'`. That string is truthy in Groovy, so existing configuration checks can reverse their meaning. Jackson 3.1.6 explicitly covers this change in its [scalar-resolution test](https://github.com/FasterXML/jackson-dataformats-text/blob/jackson-dataformats-text-3.1.6/yaml/src/test/java/tools/jackson/dataformat/yaml/deser/ParseBooleanLikeWordsAsStringsTest.java). Preserve the previous behavior, or document the breaking input change and add regression coverage showing that callers must use `false` instead of `off`.



-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6246778084

   ## ✅ All tests passed ✅
   
   🏷️ Commit: d7e8e6c41ca42fc0de9515b381a722b6f97c090f
   ▶️ Tests:  121913 executed
   ⚪️ Checks: 33/33 completed
   
   ---
   _Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
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.



-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-5843316298

   ## [Codecov](https://app.codecov.io/gh/apache/groovy/pull/2949?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `70.68966%` with `17 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 71.6110%. Comparing base ([`ac3babd`](https://app.codecov.io/gh/apache/groovy/commit/ac3babdee109f9b0bcfab241f152028fb52b47f4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e5e2f17`](https://app.codecov.io/gh/apache/groovy/commit/e5e2f17eba8c35dcdd404ae4b30a005054b53f79?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2949?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...java/org/apache/groovy/xml/util/JacksonHelper.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-xml%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fxml%2Futil%2FJacksonHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXhtbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ3Jvb3Z5L3htbC91dGlsL0phY2tzb25IZWxwZXIuamF2YQ==) | 53.3333% | [6 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...roovy-csv/src/main/java/groovy/csv/CsvBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-csv%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Fcsv%2FCsvBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNzdi9zcmMvbWFpbi9qYXZhL2dyb292eS9jc3YvQ3N2QnVpbGRlci5qYXZh) | 0.0000% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...roovy-csv/src/main/java/groovy/csv/CsvSlurper.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-csv%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Fcsv%2FCsvSlurper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNzdi9zcmMvbWFpbi9qYXZhL2dyb292eS9jc3YvQ3N2U2x1cnBlci5qYXZh) | 71.4286% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...vy-toml/src/main/java/groovy/toml/TomlBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-toml%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Ftoml%2FTomlBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXRvbWwvc3JjL21haW4vamF2YS9ncm9vdnkvdG9tbC9Ub21sQnVpbGRlci5qYXZh) | 0.0000% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...vy-toml/src/main/java/groovy/toml/TomlSlurper.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-toml%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Ftoml%2FTomlSlurper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXRvbWwvc3JjL21haW4vamF2YS9ncm9vdnkvdG9tbC9Ub21sU2x1cnBlci5qYXZh) | 75.0000% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ava/org/apache/groovy/toml/util/TomlConverter.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-toml%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Ftoml%2Futil%2FTomlConverter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXRvbWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS90b21sL3V0aWwvVG9tbENvbnZlcnRlci5qYXZh) | 87.5000% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...vy-yaml/src/main/java/groovy/yaml/YamlBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-yaml%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Fyaml%2FYamlBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXlhbWwvc3JjL21haW4vamF2YS9ncm9vdnkveWFtbC9ZYW1sQnVpbGRlci5qYXZh) | 80.0000% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ava/org/apache/groovy/yaml/util/YamlConverter.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-yaml%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fyaml%2Futil%2FYamlConverter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXlhbWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS95YW1sL3V0aWwvWWFtbENvbnZlcnRlci5qYXZh) | 87.5000% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   [![Impacted file tree graph](https://app.codecov.io/gh/apache/groovy/pull/2949/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #2949        +/-   ##
   ==================================================
   + Coverage     71.6060%   71.6110%   +0.0051%     
   - Complexity      38060      38064         +4     
   ==================================================
     Files            1588       1588                
     Lines          137078     137092        +14     
     Branches        25507      25508         +1     
   ==================================================
   + Hits            98156      98173        +17     
   + Misses          30015      30010         -5     
   - Partials         8907       8909         +2     
   ```
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2949?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [...rg/apache/groovy/json/internal/BaseJsonParser.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-json%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fjson%2Finternal%2FBaseJsonParser.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWpzb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9qc29uL2ludGVybmFsL0Jhc2VKc29uUGFyc2VyLmphdmE=) | `75.2941% <ø> (ø)` | |
   | [...vy-yaml/src/main/java/groovy/yaml/YamlSlurper.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-yaml%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Fyaml%2FYamlSlurper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXlhbWwvc3JjL21haW4vamF2YS9ncm9vdnkveWFtbC9ZYW1sU2x1cnBlci5qYXZh) | `93.3333% <100.0000%> (+0.2299%)` | :arrow_up: |
   | [...vy-toml/src/main/java/groovy/toml/TomlBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-toml%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Ftoml%2FTomlBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXRvbWwvc3JjL21haW4vamF2YS9ncm9vdnkvdG9tbC9Ub21sQnVpbGRlci5qYXZh) | `75.0000% <0.0000%> (ø)` | |
   | [...vy-toml/src/main/java/groovy/toml/TomlSlurper.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-toml%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Ftoml%2FTomlSlurper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXRvbWwvc3JjL21haW4vamF2YS9ncm9vdnkvdG9tbC9Ub21sU2x1cnBlci5qYXZh) | `87.0968% <75.0000%> (ø)` | |
   | [...ava/org/apache/groovy/toml/util/TomlConverter.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-toml%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Ftoml%2Futil%2FTomlConverter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXRvbWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS90b21sL3V0aWwvVG9tbENvbnZlcnRlci5qYXZh) | `80.0000% <87.5000%> (+30.0000%)` | :arrow_up: |
   | [...vy-yaml/src/main/java/groovy/yaml/YamlBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-yaml%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Fyaml%2FYamlBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXlhbWwvc3JjL21haW4vamF2YS9ncm9vdnkveWFtbC9ZYW1sQnVpbGRlci5qYXZh) | `78.9474% <80.0000%> (+1.1696%)` | :arrow_up: |
   | [...ava/org/apache/groovy/yaml/util/YamlConverter.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-yaml%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fyaml%2Futil%2FYamlConverter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXlhbWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS95YW1sL3V0aWwvWWFtbENvbnZlcnRlci5qYXZh) | `81.8182% <87.5000%> (+18.1818%)` | :arrow_up: |
   | [...roovy-csv/src/main/java/groovy/csv/CsvSlurper.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-csv%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Fcsv%2FCsvSlurper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNzdi9zcmMvbWFpbi9qYXZhL2dyb292eS9jc3YvQ3N2U2x1cnBlci5qYXZh) | `83.6956% <71.4286%> (ø)` | |
   | [...roovy-csv/src/main/java/groovy/csv/CsvBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-csv%2Fsrc%2Fmain%2Fjava%2Fgroovy%2Fcsv%2FCsvBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNzdi9zcmMvbWFpbi9qYXZhL2dyb292eS9jc3YvQ3N2QnVpbGRlci5qYXZh) | `65.2174% <0.0000%> (ø)` | |
   | [...java/org/apache/groovy/xml/util/JacksonHelper.java](https://app.codecov.io/gh/apache/groovy/pull/2949?src=pr&el=tree&filepath=subprojects%2Fgroovy-xml%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fxml%2Futil%2FJacksonHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXhtbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ3Jvb3Z5L3htbC91dGlsL0phY2tzb25IZWxwZXIuamF2YQ==) | `48.6486% <53.3333%> (+0.5005%)` | :arrow_up: |
   
   ... and [11 files with indirect coverage changes](https://app.codecov.io/gh/apache/groovy/pull/2949/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   </details>
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6510233010

   ## ✅ All tests passed ✅
   
   🏷️ Commit: b6d43bc75fe3b89fbb1bcd3382579a9157291f3a
   ▶️ Tests:  123900 executed
   ⚪️ Checks: 27/27 completed
   
   ---
   _Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6506921438

   Thanks, Daniel. On treating the breaks as first-class migration items (points 3 and 5):
   
   - **`COMPATIBILITY.md`** (b6d43bc) now has a "Groovy 7 — groovy-yaml, groovy-toml and groovy-csv use Jackson 3 (GROOVY-12433)" section beside the other Groovy 7 entries. It lists each change with who is affected and what to do: YAML scalar resolution, Jackson 2 databind annotations, typed XML following the classpath, Jackson 2 and SnakeYAML no longer transitive, and parser limits. It also says what stays the same.
   - **The PR description's breaking changes** point to that section and now include the parser limits as well. The YAML, TOML, CSV and XML guides keep their notes beside typed parsing, and `YamlScalarResolutionTest` pins the scalar behaviour.
   
   On the limits (point 4), I measured both versions to get the wording right. YAML and TOML input deeper than 500 levels now fails, where Jackson 2 allowed 1000. The string-length increase only matters for TOML: neither version length-checks CSV fields, and both cap a YAML document at 3,145,728 code points. The PR description says so now.
   
   On the scalar change, thanks for weighing in on the [dev@ thread](https://lists.apache.org/thread.html/t5nf7xk17wvf1dbol40ock8thg771rz3). Jochen's point that this isn't YAML 1.2 core either is in the wording: the guide and `COMPATIBILITY.md` both say the resolver is the JSON schema, stricter than the core schema, and name the values that change.
   
   I've also merged master again. It merged cleanly, and the yaml, toml, csv, xml (including `jackson2Test`) and json tests pass.
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-5843538291

   ## ✅ All tests passed ✅
   
   🏷️ Commit: e5e2f17eba8c35dcdd404ae4b30a005054b53f79
   ▶️ Tests:  120306 executed
   ⚪️ Checks: 33/33 completed
   
   ---
   _Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2949:
URL: https://github.com/apache/groovy/pull/2949#discussion_r4209783466


##########
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:
   The newly documented Jackson 2 fallback is no longer exercised: `groovy-xml` now runs this suite only with Jackson 3, so every test takes this branch and the reflective Jackson 2 path can regress unnoticed. Please add a separate Jackson-2-only test configuration/classloader case that verifies successful conversion and unknown-property rejection without Jackson 3 present.



-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6371908116

   AI read:
   
   > Thanks, this is a nice piece of work. I checked out the branch and looked mostly at the serialization and classpath side:
   > 
   > - **No Jackson 2 needed any more.** The runtime classpaths of groovy-yaml, groovy-toml and groovy-csv contain only `tools.jackson` 3.1.6, plus `jackson-annotations` 2.21, which Jackson 3 still uses. Typed `parseAs`/`parseTextAs` works without Jackson 2 for all three, and XML falls back correctly.
   > - **Builder-built shared mappers are the right fit for Jackson 3's immutable mappers.** Nothing reconfigures a mapper after it's built, and no public API exposes one.
   > - **POGOs still serialize safely.** Jackson 3 keeps the special case that skips Groovy's `getMetaClass()` getter.
   > - **Other YAML features are unchanged.** I ran the same documents through master (Jackson 2 / SnakeYAML) and this branch (Jackson 3 / SnakeYAML Engine). Anchors and aliases, merge keys, standard, custom and class tags, complex keys and multi-document streams all behave identically. Class tags are never instantiated, before or after. The scalar resolution change you've documented is the only behavioural difference I found.
   > 
   > I'd like a few additions to the docs or release notes before merging:
   > 
   > 1. **Jackson 2 *databind* annotations are silently ignored.** `@JsonProperty`, `@JsonIgnore`, `@JsonFormat`, `@JsonCreator` and so on still work, because they live in the shared annotations jar. But `@JsonDeserialize`, `@JsonSerialize`, `@JsonNaming` and `@JsonPOJOBuilder` moved from `com.fasterxml.jackson.databind.annotation` to `tools.jackson.databind.annotation`. Types annotated with the Jackson 2 versions lose their custom (de)serializers or naming strategy in YAML/TOML/CSV `parseAs`, with no error. Annotations are the only customisation hook users have, since we don't expose a mapper. So this deserves a sentence in the YAML/TOML/CSV guides next to parseAs ("use the tools.jackson.databind.annotation versions of these"), and an entry under breaking changes in the 7.0 release notes.
   > 
   > 2. **XML typed parsing picks its Jackson version from the classpath.** Jackson 3 wins whenever it is present. An application that gains Jackson 3 transitively, for example via Spring Boot 4, moves XML `parseAs` from 2 to 3 without any Groovy change. Combined with point 1, the same type can then bind differently. I think the precedence is right, but please document it in the XML guide.
   > 
   > 3. **Jackson 2 and SnakeYAML no longer arrive transitively.** On master, groovy-yaml/csv/toml put Jackson 2 databind and `org.yaml:snakeyaml` on users' runtime classpath. Code that used either directly, including anyone following the guide's alias recipe, now needs to declare them itself. A release-note line would cover it. You've already updated the recipe itself.
   > 
   > 4. **YAML scalar resolution.** Thanks for calling this out so clearly.
   >    - **The wording undersells the change.** It's stricter than YAML 1.2's *core* schema, under which `~`, `Null`, `True` and `0x1F` are still null, boolean and integer. With this branch they're all strings. I also tried passing SnakeYAML Engine's `CoreSchema` through `YAMLFactory.builder().loadSettings(...)` and it made no difference, which confirms that Jackson fixes the resolution in its parser. So "Jackson's JSON-schema resolution" is the accurate description, rather than "YAML 1.2".
   >    - **Add a recipe for users who need YAML 1.1 typing:** load with SnakeYAML directly, alongside the alias recipe. `new Yaml().load(text)` still gives `yes` as `true`.
   >    - **This needs the team's attention.** `enabled: off` silently becoming truthy is the change most likely to bite existing users. I'm fine with it for 7.0 as a documented break, but I'd like it raised explicitly on the dev list rather than landing quietly. A prominent release-notes entry is the minimum.
   > 
   > A minor point: 3.1.6 matches Spring Boot 4.2, which seems a sensible pin, but Jackson 3.2.x is out. Was staying on 3.1 a deliberate choice?
   
   I usually get AI to pull out bits for the release notes, but feel free to create a PR on groovy-website if you want.


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "paulk-asert (via GitHub) via notifications" <no...@groovy.apache.org>.
paulk-asert commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6621680268

   Merged, thanks!


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6374567300

   Thanks for checking it so thoroughly. Four new commits:
   
   1. **Databind annotations** (3ed9138): the YAML, TOML and CSV guides now have a note beside typed parsing. The `com.fasterxml.jackson.annotation` ones work unchanged; `@JsonSerialize`, `@JsonDeserialize`, `@JsonNaming`, `@JsonPOJOBuilder` and the rest must come from `tools.jackson.databind.annotation`, because the Jackson 2 versions are silently ignored. I checked it with `@JsonNaming`: the Jackson 2 one is ignored on both read and write.
   2. **XML precedence** (93ed3dc): the XML guide now says Jackson 3 wins whenever it's on the classpath, gives Spring Boot 4 as the transitive example, and notes that each version ignores the other's databind annotations, so the same type can bind differently after the switch.
   3. **Transitive dependencies**: listed under breaking changes in the PR description.
   4. **YAML scalar resolution** (e4cb388):
      - The note now says "JSON schema" and that it's stricter than the YAML 1.2 core schema, naming `True`, `Null`, `~` and `0x1F` as values the core schema would still type.
      - A new "YAML 1.1 typing with SnakeYAML directly" section follows the alias recipe, and the scalar note links to it. Its spec test loads `off`, `yes`, `0x1F` and `~` as `false`, `true`, `31` and `null`.
      - Agreed it shouldn't land quietly. I'll start a thread on dev@, and it's the first entry under breaking changes in the PR description.
   
   **Jackson 3.2**: 3.1.6 was picked to match Spring Boot 4.2, but Boot has since moved to 3.1.7. 3.2 is also the sibling of the Jackson 2.22 that master already uses, so 873001b moves to 3.2.4. Scalar resolution is the same apart from `.inf`, `-.inf` and `.nan`, which Jackson 2 and 3.1 rejected as malformed numbers. They now parse: as `Double` through the typed API, and as the strings `'Infinity'`, `'-Infinity'` and `'NaN'` through the untyped one, which converts through JSON. The guide notes this and `YamlScalarResolutionTest` pins it. 3.2 also applies `maxStringLength` to YAML input, which 3.1 didn't, and fixes several TOML parser bugs. The yaml, toml, csv, xml and json tests all pass on it, including `jackson2Test`.
   
   **Release notes**: the PR description's breaking changes are now four entries (YAML scalars, databind annotations, XML precedence, transitive dependencies), written so they can be lifted as they are. Happy to open a groovy-website PR as well if that's easier.
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6376336634

   ## ✅ All tests passed ✅
   
   🏷️ Commit: 93ed3dc6858cfa59bf15c47b357c4e0505f8ace9
   ▶️ Tests:  123063 executed
   ⚪️ Checks: 33/33 completed
   
   ---
   _Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2949:
URL: https://github.com/apache/groovy/pull/2949#issuecomment-6400514034

   ## ✅ All tests passed ✅
   
   🏷️ Commit: eeaa3edcf65d525fb55a8a36d3416723943ce813
   ▶️ Tests:  123363 executed
   ⚪️ Checks: 33/33 completed
   
   ---
   _Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
   


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "paulk-asert (via GitHub) via notifications" <no...@groovy.apache.org>.
paulk-asert closed pull request #2949: GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3
URL: https://github.com/apache/groovy/pull/2949


-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GROOVY-12433: Move groovy-yaml, groovy-toml and groovy-csv to Jackson 3 [groovy]

Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on code in PR #2949:
URL: https://github.com/apache/groovy/pull/2949#discussion_r4308544702


##########
subprojects/groovy-yaml/build.gradle:
##########
@@ -22,12 +22,12 @@ plugins {
 
 dependencies {
     api project(':')  // YamlBuilder extends GroovyObjectSupport...
-    implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${versions.jackson}"
-    implementation "com.fasterxml.jackson.core:jackson-databind:${versions.jackson}"
-    implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson}"
+    implementation "tools.jackson.dataformat:jackson-dataformat-yaml:${versions.jackson3}"

Review Comment:
   Keeping Jackson 3's resolution, since this targets 7.0, and documenting it as a breaking change. It's wider than booleans: Jackson 3.1.6 hard-codes snakeyaml-engine's `JsonScalarResolver` in `YAMLParser`, so `True`/`FALSE`, `Null`/`~` and `0x1F`/`017` also become strings, and no feature or `LoadSettings` schema restores YAML 1.1 resolution. 3702e55 adds `YamlScalarResolutionTest` pinning the behaviour and a note in the YAML user guide; the PR description now lists it under breaking changes.



-- 
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: notifications-unsubscribe@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org