ramu11 commented on PR #25684:
URL: https://github.com/apache/camel/pull/25684#issuecomment-5419297348
> Well-structured new TOON data format that follows Camel conventions with
good tests and documentation. CI is green and the implementation is clean.
>
> **Two observations worth discussing:**
>
> 1. **Unmanaged Jackson 3.x transitive dependencies** — `jtoon:2.0.2`
brings in `tools.jackson.core:jackson-databind:3.2.1` and
`tools.jackson.module:jackson-module-blackbird:3.2.1` as runtime-scoped
transitives. These are not managed by Camel's parent POM. While the versions
currently match Camel's `jackson3-version=3.2.1`, future version bumps could
cause inconsistency. Consider importing `tools.jackson:jackson-bom` in
`camel-toon`'s `<dependencyManagement>` (as `camel-jackson3` does) to keep
Jackson 3.x versions consistent.
> 2. **String bodies treated as JSON documents** — `marshal()` routes all
`String` bodies through `JToon.encodeJson()`, so a plain Java `String` like
`"hello"` cannot be marshalled as a TOON string scalar (it throws
`IllegalArgumentException`). This is an intentional design choice (documented
in the code and validated by tests), but worth noting in the user-facing
documentation so users know they need to pass JSON strings or typed Java
objects.
>
> ### 📋 PR Metadata
> Aspect Current Suggested
> Milestone _(none)_ `4.23.0`
> _This review was generated by an AI agent and may contain inaccuracies.
Please verify all suggestions before applying._
>
> _Claude Code on behalf of @gnodet_
Thanks for the observations.
Jackson 3.x dependencies: Addressed by importing tools.jackson:jackson-bom
in camel-toon using Camel's existing ${jackson3-version}. This ensures JToon's
Jackson 3.x transitive dependencies remain aligned with Camel's managed
version. We verified the dependency tree now resolves the Jackson 3.x artifacts
to 3.2.2.
String bodies: This behavior is intentional. String bodies are treated as
JSON input and passed to JToon.encodeJson(), so a plain string such as "hello"
is not accepted as a TOON scalar. We have clarified this behavior in the
user-facing documentation and added/retained test coverage for invalid JSON
strings. We prefer to keep this behavior for the current Preview implementation
rather than introduce an additional option.
--
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]
