Skip to content

#12362-factory-method-for-JSONbuilders - #1

Open
TillJan wants to merge 46 commits into
developfrom
#12362-factory-method-for-JSONbuilders
Open

#12362-factory-method-for-JSONbuilders#1
TillJan wants to merge 46 commits into
developfrom
#12362-factory-method-for-JSONbuilders

Conversation

@TillJan

@TillJan TillJan commented Jul 17, 2026

Copy link
Copy Markdown
Owner

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:

TillJan pushed a commit that referenced this pull request Jul 17, 2026
get `podman compose up` working
@TillJan
TillJan force-pushed the #12362-factory-method-for-JSONbuilders branch from bff606e to 04f565f Compare July 17, 2026 12:13
@coveralls

coveralls commented Jul 17, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 25

Coverage increased (+0.03%) to 25.004%

Details

  • Coverage increased (+0.03%) from the base build.
  • Patch coverage: 530 uncovered changes across 81 files (183 of 713 lines covered, 25.67%).
  • 6 coverage regressions across 3 files.

Uncovered Changes

Top 10 Files by Coverage Impact Changed Covered %
src/main/java/edu/harvard/iq/dataverse/api/Datasets.java 71 2 2.82%
src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java 66 25 37.88%
src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetUserTracesCommand.java 35 0 0.0%
src/main/java/edu/harvard/iq/dataverse/api/Admin.java 34 0 0.0%
src/main/java/edu/harvard/iq/dataverse/util/bagit/OREMap.java 28 0 0.0%
src/main/java/edu/harvard/iq/dataverse/api/Index.java 23 0 0.0%
src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java 20 0 0.0%
src/main/java/edu/harvard/iq/dataverse/search/IndexBatchServiceBean.java 19 0 0.0%
src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java 17 0 0.0%
src/main/java/edu/harvard/iq/dataverse/search/SolrSearchResult.java 16 1 6.25%
Total (96 files) 713 183 25.67%

Coverage Regressions

6 previously-covered lines in 3 files lost coverage.

File Lines Losing Coverage Coverage
src/main/java/edu/harvard/iq/dataverse/dataaccess/DataAccess.java 4 47.7%
src/main/java/edu/harvard/iq/dataverse/search/savedsearch/SavedSearchServiceBean.java 1 0.0%
src/main/java/edu/harvard/iq/dataverse/util/json/JsonParser.java 1 65.1%

Coverage Stats

Coverage Status
Relevant Lines: 95468
Covered Lines: 23871
Line Coverage: 25.0%
Coverage Strength: 0.25 hits per line

💛 - Coveralls

@TillJan
TillJan force-pushed the #12362-factory-method-for-JSONbuilders branch from 04f565f to ac66224 Compare July 17, 2026 12:45
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

Test Results

403 tests  +403   388 ✅ +388   33m 47s ⏱️ + 33m 47s
 55 suites + 55    15 💤 + 15 
 55 files   + 55     0 ❌ ±  0 

Results for commit 6b2954a. ± Comparison against base commit df71344.

♻️ This comment has been updated with latest results.

poikilotherm and others added 23 commits July 21, 2026 19:22
…ions

- Implemented SonarQube job in Maven CI workflow for enhanced code quality analysis.
- Updated `pom.xml` and parent POM with SonarQube configuration properties and plugin definitions.
- Refined JaCoCo configuration to support integrated coverage reporting.
- Adjusted artifact handling to include binary class files required by SonarQube.
- Set `fetch-depth` to 0 for full Git history to improve Sonar analysis.
- Updated JaCoCo XML report path in `pom.xml` for accurate coverage reporting (Sonar needs the full path to the XML file).
* fix required textarea in gbr

* release note
…cense

# Conflicts:
#	doc/sphinx-guides/source/installation/config.rst
* retention null pointer exception

* review
Bumps [org.postgresql:postgresql](https://github.com/pgjdbc/pgjdbc) from 42.7.11 to 42.7.12.
- [Release notes](https://github.com/pgjdbc/pgjdbc/releases)
- [Changelog](https://github.com/pgjdbc/pgjdbc/blob/master/CHANGELOG.md)
- [Commits](pgjdbc/pgjdbc@REL42.7.11...REL42.7.12)

---
updated-dependencies:
- dependency-name: org.postgresql:postgresql
  dependency-version: 42.7.12
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
- about fixed NPE for draft file preview
- typo in another file name
…verse-parent/org.postgresql-postgresql-42.7.12

chore(deps): bump org.postgresql:postgresql from 42.7.11 to 42.7.12 in /modules/dataverse-parent
Bumps [org.omnifaces:omnifaces](https://github.com/omnifaces/omnifaces) from 4.7.5 to 4.7.12.
- [Commits](https://github.com/omnifaces/omnifaces/commits)

---
updated-dependencies:
- dependency-name: org.omnifaces:omnifaces
  dependency-version: 4.7.12
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
…s-omnifaces-4.7.12

chore(deps): bump org.omnifaces:omnifaces from 4.7.5 to 4.7.12
Add SonarQube analysis and coverage reporting to GitHub Actions
Co-authored-by: Philip Durbin <philip_durbin@harvard.edu>
@poikilotherm
poikilotherm force-pushed the #12362-factory-method-for-JSONbuilders branch from c1d1d91 to 5842cfa Compare July 31, 2026 13:04
poikilotherm and others added 19 commits July 31, 2026 15:40
- Introduced a new job to verify the availability of secrets required for SonarQube analysis.
- Updated job dependencies and conditions to ensure SonarQube job only runs when secrets are available.
- Removed the unreliable org name check, as it won't work for PR runs
…orks

Limit SonarQube job to upstream repository
…/IQSS/11161-defaultLicense

Allow Creating dataset with default license via API regardless of allow custom terms setting
use JsonUtil.createObjectBuilder and JsonUtil.createArrayBuilder to reuse JsonUtil.builderFactory and avoid inefficient usage of JSON providers/builders.
refactored the start import to specific imports
replaced all Json.createObjectBuilder() and Json.createArrayBuilder() calls with the matching JsonUtil methods.
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.
…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.
@pdurbin
pdurbin force-pushed the #12362-factory-method-for-JSONbuilders branch from 9b41ec9 to 9ddbf5c Compare August 7, 2026 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance: inefficient usage of JSON providers/builders

7 participants