Fix nested message object in API responses #12097

ErykKul · 2026-01-16T20:50:38Z

What this PR does / why we need it:

Fixes the ok(String msg, JsonObjectBuilder bld) method in AbstractApiBean.java which incorrectly wraps the message in a nested object:

// Before (incorrect):
{"status":"OK","message":{"message":"actual message"},"data":{...}}

// After (correct):
{"status":"OK","message":"actual message","data":{...}}

The bug was introduced in commit f311312c34 on May 12, 2020, as part of #4813.

Which issue(s) this PR closes:

Closes API returns malformed JSON with nested message object instead of string in ok(String, JsonObjectBuilder) responses #12096
Closes Dataverse API responses inconsistent in JSON structure #11667

Special notes for your reviewer:

This is a one-line fix in AbstractApiBean.java line 1000:

// From:
.add("message", Json.createObjectBuilder().add("message",msg))
// To:
.add("message", msg)

The following 7 API endpoints are affected:

File	Line	API Endpoint
`Datasets.java`	3087	`POST /api/datasets/{id}/add` (duplicate file warning)
`Admin.java`	233	`PUT /api/admin/settings`
`Dataverses.java`	754	`PUT /api/dataverses/{id}`
`Dataverses.java`	788	`PUT /api/dataverses/{id}/inputLevels`
`SavedSearches.java`	161	`POST /api/admin/savedsearches`
`HarvestingClients.java`	309	`PUT /api/harvest/clients/{nickName}`
`HarvestingServer.java`	228	`PUT /api/harvest/server/oaisets/{specname}`

Suggestions on how to test this:

Upload a file to a dataset via API
Upload the same file again (duplicate)
Check that the response message field is a string, not a nested object:

curl -X POST "http://localhost:8080/api/datasets/:persistentId/add?persistentId=doi:..." \
  -H "X-Dataverse-key: $API_TOKEN" \
  -F "[email protected]"
# Upload again to trigger duplicate warning
curl -X POST "http://localhost:8080/api/datasets/:persistentId/add?persistentId=doi:..." \
  -H "X-Dataverse-key: $API_TOKEN" \
  -F "[email protected]"

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

Yes, included in this PR: doc/release-notes/12096-fix-ok-message-nested-object.md

Additional documentation:

N/A

coveralls · 2026-01-16T21:02:07Z

coverage: 24.354% (+0.03%) from 24.323%
when pulling 47dece5 on 12096-fix-ok-message-nested-object
into 86796a1 on develop.

pdurbin

A couple quick comments.

Also, @poikilotherm noticed this double message bug, which lead to this API v2 proposal: https://docs.google.com/document/d/1XtXPF_PZCuhPbm4HCu28yRge_4_--ah604NhigPdjPk/edit?usp=sharing . And these related issues:

doc/release-notes/12096-fix-ok-message-nested-object.md

src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java

ErykKul · 2026-01-17T00:24:47Z

@pdurbin Thanks for the quick review and the context about the API v2 proposal!

I was a bit hesitant to make this change since it could break existing API consumers that have implemented workarounds for the nested message object. However, I think it's worth fixing. The current behavior is clearly a bug, and the affected endpoints are relatively niche (the bug was actually discovered through duplicate file uploads, while regular file additions work fine).

That said, I can see the argument for keeping this for API v2 if you think the breaking change risk is too high. Let me know your thoughts - happy to go either way.

I've fixed the integration tests and added the changelog entry as requested. The docs build should pass now too (had a small RST syntax issue with the reference).

poikilotherm

While the commits look good overall, I am reluctant to give this a pass as-is.
I discovered more problems with inconsistencies in #11667 and as @pdurbin pointed out, this led to the v2 API proposal.

I agree with @ErykKul that changing this is very useful. And for 6 of the 7 affected endpoints I am agreeing that changing them may cause minor inconveniences for administrators (fine, just deal with it).

Yet with the Dataset endpoint to add files warning about duplicates and which are duplicated - this may seriously affect some integration with Dataverse out there. This may actually be a breaking change causing trouble for users. IMHO we shouldn't do this.

Also, if we fix that, let's do it all in one sweep and fix any other inconsistencies as well.

There may be some middle ground here if we make this opt-in. Using a feature flag marking it as experimental, we can leave a choice to the admin. They can test with their integrations and see what happens and return to the default at any time. (This reminds me of mentioning https://openfeature.dev somewhere - making feature flags activate based on more criteria than mere configuration...)
A way forward may be to make this experimental for now, then deprecate it in a future version, then drop it. This way this may not even need an API v2 (only regarding this message - the mileage for the other inconsistencies may vary!)

Adds API_MESSAGE_FIELD_LEGACY feature flag that allows admins to revert to the old (buggy) nested message format if they have integrations that depend on it. The fix is now the default behavior. Set dataverse.feature.api-message-field-legacy=true to use the legacy format. This flag will be removed in a future version.

ErykKul · 2026-02-09T10:13:58Z

Hi @pdurbin, @poikilotherm,

I've addressed the feedback in this PR:

Merged the latest develop into this branch.
Added a feature flag dataverse.feature.api-message-field-legacy as @poikilotherm suggested. The fix is now the default behavior, and admins who discover issues with their integrations can set this flag to true to revert to the previous (buggy) format. This way we get the fix out without risking breakage for anyone.
Updated the release notes and API changelog accordingly.

I ran the full test suite locally (unit tests + integration tests) and everything passes. However, I'm unable to access the Jenkins server from here — it seems to be restricted to internal networks. @pdurbin, if Jenkins fails again on this latest commit, could you share the relevant log output or a screenshot? That would help me figure out what's going on. Thanks!

ErykKul · 2026-02-10T20:50:45Z

Note: rename to something like dataverse.legacy.api-message-wrap

Replaced `this.error(...)` with `error(...)` for consistency with recent style updates in error handling.

…` and remove duplicates Consolidated shared API constants—such as header names, status codes, and response messages—from `AbstractApiBean` into the dedicated `ApiConstants` class. Replaced direct usages with references to `ApiConstants` for improved maintainability and consistency.

…n structure Unified the multiple `ok()` overloads into a single primary method accepting `JsonValue`s for `data and` `message` fields, plus `totalCount` Long field, reducing duplication and ensuring consistent JSON response formatting across API endpoints.

…se creation Use feature flag `API_MESSAGE_FIELD_LEGACY` to preserve the old `{data:{message:"..."}}` response format if needed in an installation.

…tion Consolidated `ok()` logic by delegating to the unified `ok(JsonValue, JsonValue, Long)` method, removing redundant response construction. Keeping the support to create the wrapped legacy {message:{message:value}} style.

…ttings` Moved the legacy API message response style toggle from `FeatureFlags` to `JvmSettings`, using `LEGACY_API_RESPONSE_MESSAGE_STYLE`. Updated `AbstractApiBean` to use the new setting, and removed the deprecated `API_MESSAGE_FIELD_LEGACY` entry from `FeatureFlags`.

src/main/java/edu/harvard/iq/dataverse/api/BatchImport.java

            batchService.processFilePath(fileDir, parentIdtf, dataverseRequest, owner, importType, createDV);

        } catch (ImportException e) {
-            return this.error(Response.Status.BAD_REQUEST, "Import Exception, " + e.getMessage());


In general, to fix information exposure through error messages, do not send raw exception messages to the client. Instead, log the detailed exception server-side (including stack trace and message) and return a generic, non-sensitive error message to the user.

For this specific case, the best minimal fix is to change the catch (ImportException | IOException e) block in postImport so that it no longer uses e.getMessage() in the HTTP response. Instead, it should return a generic error like "Error importing dataset" (or similar). If the error(...) helper logs the full error or if there is another logging mechanism in this class, that logging can continue to use e or e.getMessage(); however, since the snippet shows no existing logging, we will only adjust the message sent to the client and keep behavior otherwise identical.

Concretely:

In src/main/java/edu/harvard/iq/dataverse/api/BatchImport.java, in the postImport method, update line 92 from return error(Response.Status.BAD_REQUEST, e.getMessage()); to return error(Response.Status.BAD_REQUEST, "Error importing dataset");.

This change keeps the HTTP status code and flow intact while ensuring the client does not receive internal error details.

No new imports or helper methods are required for this minimal fix.

The most often used style of embedding the message into the data field is not standardized, but only opt-in. As this is a breaking change, we soften the blow by allowing integrations, libraries, clients, and anyone else to test thoroughly before the alignment is made permanent at a later point.

…anipulation Introduces `@FeatureFlag`, `FeatureFlagExtension`, and `FeatureFlagBroker` to enable reliable, serial test execution with configurable feature flags via system properties, backed by `@LocalFeatureFlags` for local JVM testing. Also adds `getScopedKey()` to `FeatureFlags`, as necessary for retrieval in the extension.

Adds `testUnifiedMessageStyle()` to verify `ok(String)` respects the `UNIFY_API_RESPONSE_MESSAGE_STYLE` feature flag and returns message at top level. Also updates test class to use `@LocalFeatureFlags` and modern test style (package-private methods).

…d format and feature flag opt-in #11667

poikilotherm · 2026-02-11T21:43:29Z

I've been a busy 🐝 Can someone else from the core team give this another review please? Thx!

…naming

github-actions · 2026-02-11T22:15:44Z

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:12096-fix-ok-message-nested-object

ghcr.io/gdcc/configbaker:12096-fix-ok-message-nested-object

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

Fix nested message object in API responses #12096

7911c63

github-actions bot added the Type: Bug a defect label Jan 16, 2026

ErykKul changed the title ~~Fix nested message object in API responses #12096~~ Fix nested message object in API responses Jan 16, 2026

This comment has been minimized.

Sign in to view

pdurbin added this to IQSS Dataverse Project Jan 16, 2026

pdurbin moved this to Ready for Triage in IQSS Dataverse Project Jan 16, 2026

pdurbin reviewed Jan 16, 2026

View reviewed changes

doc/release-notes/12096-fix-ok-message-nested-object.md Outdated Show resolved Hide resolved

src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java Outdated Show resolved Hide resolved

ErykKul added 2 commits January 17, 2026 01:13

Fix integration tests and add API changelog entry

2e43bd7

Fix RST reference syntax in changelog

0a9ce32

This comment has been minimized.

Sign in to view

scolapasta moved this from Ready for Triage to Ready for Review ⏩ in IQSS Dataverse Project Jan 20, 2026

pdurbin self-assigned this Jan 20, 2026

pdurbin moved this from Ready for Review ⏩ to In Review 🔎 in IQSS Dataverse Project Jan 20, 2026

pdurbin assigned ErykKul Jan 20, 2026

pdurbin requested a review from poikilotherm January 20, 2026 16:13

poikilotherm requested changes Jan 20, 2026

View reviewed changes

pdurbin assigned poikilotherm Jan 20, 2026

cmbz added FY26 Sprint 15 FY26 Sprint 15 (2026-01-14 - 2026-01-28) FY26 Sprint 16 FY26 Sprint 16 (2026-01-28 - 2026-02-11) labels Jan 29, 2026

pdurbin added the Waiting label Feb 4, 2026

ErykKul added 2 commits February 9, 2026 10:24

Merge develop into 12096-fix-ok-message-nested-object

0ba746a

This comment has been minimized.

Sign in to view

pdurbin removed the Waiting label Feb 10, 2026

pdurbin unassigned ErykKul Feb 10, 2026

pdurbin removed their assignment Feb 10, 2026

poikilotherm added 6 commits February 11, 2026 01:42

refactor(api): remove redundant this. qualifier in BatchImport.java

18f42aa

Replaced `this.error(...)` with `error(...)` for consistency with recent style updates in error handling.

refactor(api): add legacy message field support for ok(String) respon…

15acb22

…se creation Use feature flag `API_MESSAGE_FIELD_LEGACY` to preserve the old `{data:{message:"..."}}` response format if needed in an installation.

github-advanced-security bot found potential problems Feb 11, 2026

View reviewed changes

poikilotherm added 4 commits February 11, 2026 19:30

fix(api): ensure JsonObjectBuilder is built before adding to response

6403498

This comment has been minimized.

Sign in to view

docs(api,configuration): document message field style with new unifie…

f58b469

…d format and feature flag opt-in #11667

This comment has been minimized.

Sign in to view

fix(api): remove trailing space from MESSAGE_FIELD constant

8f80e9c

This comment has been minimized.

Sign in to view

poikilotherm moved this from In Review 🔎 to Ready for Review ⏩ in IQSS Dataverse Project Feb 11, 2026

poikilotherm removed their assignment Feb 11, 2026

docs(release-notes): update API message field changes and unify flag …

47dece5

…naming

cmbz added the FY26 Sprint 17 FY26 Sprint 17 (2026-02-11 - 2026-02-25) label Feb 11, 2026

@@ -89,7 +89,7 @@
                         JsonObjectBuilder status = importService.doImport(dataverseRequest, owner, body, filename, ImportType.NEW, cleanupLog);
                         return this.ok(status);
                     } catch (ImportException | IOException e) {
-                        return error(Response.Status.BAD_REQUEST, e.getMessage());
+                        return error(Response.Status.BAD_REQUEST, "Error importing dataset");
                     }
                 }

Fix nested message object in API responses #12097

Are you sure you want to change the base?

Fix nested message object in API responses #12097

Conversation

ErykKul commented Jan 16, 2026 • edited by poikilotherm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

pdurbin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ErykKul commented Jan 17, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

poikilotherm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

ErykKul commented Feb 9, 2026

Uh oh!

This comment has been minimized.

ErykKul commented Feb 10, 2026

Uh oh!

Check warning

Uh oh!

Copilot Autofix

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

poikilotherm commented Feb 11, 2026

Uh oh!

github-actions bot commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ErykKul commented Jan 16, 2026 •

edited by poikilotherm

Loading

coveralls commented Jan 16, 2026 •

edited

Loading

poikilotherm left a comment •

edited

Loading