Skip to content

More JsonUtil additions and forbidden Json.* calls - #12603

Merged
pdurbin merged 6 commits into
IQSS:developfrom
QualitativeDataRepository:moreJsonUtil
Aug 18, 2026
Merged

More JsonUtil additions and forbidden Json.* calls#12603
pdurbin merged 6 commits into
IQSS:developfrom
QualitativeDataRepository:moreJsonUtil

Conversation

@qqmyers

@qqmyers qqmyers commented Aug 11, 2026

Copy link
Copy Markdown
Member

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:

  • 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:

…ethods that call provider. internally.

(cherry picked from commit eead50e)
(cherry picked from commit fe171bc)
@qqmyers qqmyers moved this to Ready for Triage in IQSS Dataverse Project Aug 11, 2026
@qqmyers qqmyers added the Size: 3 A percentage of a sprint. 2.1 hours. label Aug 11, 2026
@coveralls

Copy link
Copy Markdown

Coverage Status

No base build to compare — QualitativeDataRepository:moreJsonUtil into IQSS:develop

@github-project-automation github-project-automation Bot moved this from Ready for Triage to Ready for QA ⏩ in IQSS Dataverse Project Aug 18, 2026
@pdurbin
pdurbin merged commit fb387b8 into IQSS:develop Aug 18, 2026
17 checks passed
@github-project-automation github-project-automation Bot moved this from Ready for QA ⏩ to Merged 🚀 in IQSS Dataverse Project Aug 18, 2026
@pdurbin pdurbin added this to the 6.12 milestone Aug 18, 2026
@pdurbin pdurbin moved this from Merged 🚀 to Done 🧹 in IQSS Dataverse Project Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: 3 A percentage of a sprint. 2.1 hours.

Projects

Status: Done 🧹

Development

Successfully merging this pull request may close these issues.

3 participants