More JsonUtil additions and forbidden Json.* calls - #12603
Merged
Conversation
pdurbin
approved these changes
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it: As #12541 looks like it works well and improves performance significantly, I went through the code looking for other Json.* methods we use and did the same conversion - add a JsonUtil method and forbid the corresponding calls to the jakarta Json. method to avoid the performance hit.
In addition, I took the opportunity to replace calls to createReader, which were exclusively used to eventually get a JsonObject or JosnArray from text with the existing JsonUtil.getJsonObject and JsonUtil.getJsonArray methods. This simplifies the code and probably fixes a few leaks (mostly in tests) where we weren't properly closing the StringReader/other class we used.
I also went ahead and created JsonUtil methods for Json.* methods we don't use and added those Json.* methods to the forbidden list as well so we don't accidentally add those over time.
Which issue(s) this PR closes:
Special notes for your reviewer: Lots of changed classes, but very formulaic. If there's concern about adding the methods we don't use (either blocking them with forbidden) and/or adding the corresponding wrapper methods in JsonUtil (which we don't use), I'm happy to delete them. I think it is worthwhile to keep them in forbidden so we don't add things that will be slow, but I'm not sure it makes sense to add wrappers just-in-case (though that's what I did to start).
Suggestions on how to test this: As with #12541, I think the existing tests should exercise all these changes pretty well, so I don't think much additional QA is needed beyond some general regression testing.
Note - unlike me, you should make sure the PR is up to date before merging - we just saw a test get added after #12541 was built that used now forbidden calls, resulting in #12602.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?: I think the release note for #12541 is broad enough - we're not listing specific methods and the new changes are all to build/create/factory methods like that PR.
Additional documentation: