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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
