Skip to content

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

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

#12362-factory-method-for-JSONbuilders#1
TillJan wants to merge 24 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
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
@coveralls

coveralls commented Jul 17, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 15

Coverage decreased (-0.002%) to 24.972%

Details

  • Coverage decreased (-0.002%) from the base build.
  • Patch coverage: Could not be determined — this PR's diff is too large for GitHub to return (406 error at GitHub).
  • 4294 coverage regressions across 63 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

4294 previously-covered lines in 63 files lost coverage.

Top 10 Files by Coverage Loss Lines Losing Coverage Coverage
src/main/java/edu/harvard/iq/dataverse/api/Admin.java 449 0.0%
src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java 258 2.53%
src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java 257 25.29%
src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java 239 1.45%
src/main/java/edu/harvard/iq/dataverse/DatasetVersionDifference.java 217 52.99%
src/main/java/edu/harvard/iq/dataverse/search/SolrSearchResult.java 213 21.93%
src/main/java/edu/harvard/iq/dataverse/DataFile.java 187 33.84%
src/main/java/edu/harvard/iq/dataverse/metrics/MetricsServiceBean.java 172 0.0%
src/main/java/edu/harvard/iq/dataverse/DatasetFieldServiceBean.java 143 30.05%
src/main/java/edu/harvard/iq/dataverse/mydata/RoleTagRetriever.java 139 0.0%

Coverage Stats

Coverage Status
Relevant Lines: 95429
Covered Lines: 23831
Line Coverage: 24.97%
Coverage Strength: 0.25 hits per line

💛 - Coveralls

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
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

Test Results

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit b8dc9c7. ± Comparison against base commit df71344.

♻️ This comment has been updated with latest results.

poikilotherm and others added 19 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.
…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
* 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
…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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance: inefficient usage of JSON providers/builders

6 participants