jamesfredley commented on PR #15958:
URL: https://github.com/apache/grails-core/pull/15958#issuecomment-4997825702
Just to align on the goal - I think we actually want the same end state
("adopt upstream"), and the facade is the non-breaking route to it rather than
an alternative.
Per the pre-release review (Codebase 4.1): `grails-web-common` carries a
custom JSON stack - `JSONObject.java` (~1,191 lines), `JSONArray`, and a
JavaCC-generated parser (`JsonParserTokenManager.java`, ~1,038 lines) - that
predates `groovy-json`/Jackson and is a standing maintenance and
security-surface cost (custom JSON parsers are where JSON CVEs live).
The recommended starting point there is exactly to move onto upstream
`groovy-json` **internally** while keeping the public `org.grails.web.json.*`
type surface as a **deprecated facade**:
- `groovy-json` is already a BOM dependency, so this adds **zero new deps**.
- The reason for a facade instead of a hard swap is backward compatibility:
applications, plugins, and `grails-converters` (`JSON.java`) reference
`org.grails.web.json.JSONObject` / `JSONArray` directly. Deleting that public
API outright is a breaking change and isn't safe in 8.x.
- So the facade *is* adopting upstream - it reimplements the internals on
`groovy-json` and validates against the existing converter test suites in
`grails-test-suite-web` - while deferring removal of the vendored public API to
9.0.
If your objection is specifically to keeping the vendored **public API**
around at all, that's the 9.0 removal step this facade sets up. Would it help
if I framed the PR explicitly as "reimplement internals on groovy-json;
deprecate the public facade for 9.0 removal" so the upstream-adoption intent is
unambiguous?
--
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]
