Skip to content

Introduce factory method for Json-P Builders - #12541

Open
TillJan wants to merge 13 commits into
IQSS:developfrom
TillJan:#12362-factory-method-for-JSONbuilders
Open

Introduce factory method for Json-P Builders#12541
TillJan wants to merge 13 commits into
IQSS:developfrom
TillJan:#12362-factory-method-for-JSONbuilders

Conversation

@TillJan

@TillJan TillJan commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
The PR adds methods in the JsonUtil for creating a JsonObjectBuilder and a JsonArrayBuilder. JsonUtil caches a JsonBuilderFactory which is shared by both methods.

All usage of Json.createArrayBuilder() and Json.createObjectBuilder has been replaced by the new methods.

This avoids repeatedly loading a Jakarta JSON-P implementation through the ServiceLoader which caused inefficiency.
Which issue(s) this PR closes:

Special notes for your reviewer:
The JsonBuilderFactory is shared and only created one time, while the methods are returning new Builder everytime.
Suggestions on how to test this:
run existing tests
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
no
Is there a release notes update needed for this change?:
no
Additional documentation:

@poikilotherm poikilotherm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good start!

Comment thread src/main/java/edu/harvard/iq/dataverse/util/json/JsonUtil.java Outdated
t.jansen and others added 2 commits July 17, 2026 13:44
use JsonUtil.createObjectBuilder and JsonUtil.createArrayBuilder to reuse JsonUtil.builderFactory and avoid inefficient usage of JSON providers/builders.
@TillJan
TillJan force-pushed the #12362-factory-method-for-JSONbuilders branch from bff606e to 04f565f Compare July 17, 2026 12:13
t.jansen added 2 commits July 17, 2026 14:39
refactored the start import to specific imports
replaced all Json.createObjectBuilder() and Json.createArrayBuilder() calls with the matching JsonUtil methods.
@TillJan
TillJan force-pushed the #12362-factory-method-for-JSONbuilders branch from 04f565f to ac66224 Compare July 17, 2026 12:45
poikilotherm and others added 5 commits July 17, 2026 16:25
For now, enable less invasive bundled checks.
In addition, adding first rules to disallow usage of JSON-P Json.createX() API.
Disallow more methods for object and array builders using copy-style arguments.
…sonProvider.

replaced all remaining Json.createObjectBuilder() and Json.createArrayBuilder() calls with the matching JsonUtil methods.
use JsonUtil.createValue to reuse JsonUtil.provider and avoid usage of Json.createValue.
Replaced all Json.createValue to JsonUtil.createValue.
…ch fixes the forbiddenapis signature parsing failure.
@poikilotherm poikilotherm changed the title #12362 factory method for jso nbuilders Introduce factory method for Json-P Builders Jul 21, 2026
@poikilotherm

Copy link
Copy Markdown
Contributor

@TillJan you're on to present your work during the next tech hour (2026-07-28).

@poikilotherm poikilotherm moved this to Ready for Triage in IQSS Dataverse Project Jul 21, 2026
@poikilotherm poikilotherm moved this from Ready for Triage to Needs Sizing 📏 in IQSS Dataverse Project Jul 21, 2026
@poikilotherm
poikilotherm marked this pull request as ready for review July 21, 2026 17:08
@poikilotherm

poikilotherm commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

@TillJan we should specifically take another look at the NullSafeJsonBuilder and make sure it's not using "bad" JSON-P API calls we'd like to avoid.

Also, as discussed during tech hour, lets move the forbiddenapi.txt to the dataverse-parent module to have a less cluttered project root.

@pdurbin pdurbin added Size: 3 A percentage of a sprint. 2.1 hours. and removed Size: 0.5 A percentage of a sprint. 0.35 hours labels Jul 28, 2026
@pdurbin pdurbin moved this from Needs Sizing 📏 to In Progress 💻 in IQSS Dataverse Project Jul 28, 2026
@cmbz cmbz added FY27 Sprint 2 FY27 Sprint 2 (2026-07-15 - 2026-07-29) FY27 Sprint 3 FY27 Sprint 3 (2026-07-29 - 2026-08-12) labels Jul 29, 2026
t.jansen and others added 3 commits July 31, 2026 14:07
…is plugin usage

While using the source code analysis config files from a Config JAR is a great idea in general, it will not work in our codebase.

As we do not use a parent POM at the root of our project, we cannot make the Maven Reactor pull in the dataverse-sca-config module during a build.

We'd need every developer to switch to using `mvn -f modules/dataverse-parent` all the time, which seems unlikely. Bad design choices in the past, not easy to resolve.

While we could push a built version to Maven Central, that would not help when modifying the signature file.

At the same time, using there is no reliable way to have an absolute path injected into a Maven property. We were using project.basedir within the parent POM before. This will be resolved at build time and always points to the directory where the current POM is running from. The reusability of this centralized plugin definition is rather limited, as it would fail to lookup the file in a different module.

The only way out of this:
1. Move the plugin into the dataverse module build chain.
2. This way, the resolved path is always correct, as it is model-bound, not half-parent-bound.
3. To still satisfy the request to avoid clutter in the project root, create a src/maven folder, ready to collect all these configurations.
Cleaning up the root directory, a single directory will now contain any configs related to Maven plugins. This will (mostly) be for Static Code Analysis plugins.
@poikilotherm
poikilotherm force-pushed the #12362-factory-method-for-JSONbuilders branch from c1d1d91 to 5842cfa Compare July 31, 2026 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Code Infrastructure formerly "Feature: Code Infrastructure" Feature: Performance & Stability FY27 Sprint 2 FY27 Sprint 2 (2026-07-15 - 2026-07-29) FY27 Sprint 3 FY27 Sprint 3 (2026-07-29 - 2026-08-12) Size: 3 A percentage of a sprint. 2.1 hours.

Projects

Status: In Progress 💻

Development

Successfully merging this pull request may close these issues.

Performance: inefficient usage of JSON providers/builders

4 participants