Fix nested message object in API responses

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

@@ -0,0 +1,27 @@
+### API Response Format Fix for `message` Field
+
+The `message` field in API responses from certain endpoints was incorrectly returned as a nested object (`{"message": {"message": "..."}}`) instead of a plain string (`{"message": "..."}`).
+
+This has been fixed. The following endpoints now return the `message` field as a string, consistent with all other API responses:
+
+- `POST /api/datasets/{id}/add` (when uploading duplicate files)
+- `PUT /api/admin/settings`
+- `PUT /api/dataverses/{id}`
+- `PUT /api/dataverses/{id}/inputLevels`
+- `POST /api/admin/savedsearches`
+- `PUT /api/harvest/clients/{nickName}`
+- `PUT /api/harvest/server/oaisets/{specname}`
+
+**Note:** If you have integrations that implemented workarounds for the nested `message` object, you may need to update your code to expect a plain string instead.
+If you need time to update your integrations, you can temporarily revert to the legacy behavior by setting this JVM option:
+
+```
+dataverse.legacy.api-response-message-style=true
+```
+
+This flag will be removed in a future version.
+
+**Note:** As of this version, there is also an experimental opt-in feature that will align API responses on about 230 more occasions.
+In these responses, the message is embedded into the "data" field as a nested object.
+If you want to test your integrations and clients, please enable the `dataverse.feature.unify-api-response-message-style` feature flag.
+In a future version of Dataverse, this now experimental style is going to become the supported default.

doc/sphinx-guides/source/api/changelog.rst

@@ -21,6 +21,16 @@ v6.11
 
 v6.10
 -----
+
+- Several API endpoints that return both a ``message`` and ``data`` field were incorrectly returning the message as a nested object (``{"message":{"message":"..."}}``).
+  This has been fixed so that the message is now a plain string (``{"message":"..."}``).
+  If you have integrations that depend on the old behavior, you can temporarily revert by setting ``dataverse.legacy.api-response-message-style=true``.
+  This flag will be removed in a future version.
+  Affected endpoints: ``POST /api/datasets/{id}/add`` (duplicate file warning), ``PUT /api/admin/settings``, ``PUT /api/dataverses/{id}``, ``PUT /api/dataverses/{id}/inputLevels``, ``POST /api/admin/savedsearches``, ``PUT /api/harvest/clients/{nickName}``, ``PUT /api/harvest/server/oaisets/{specname}``.
+  See `#12096 <https://github.com/IQSS/dataverse/issues/12096>`_.
+- Most API endpoints that return a success notification but no actual data have it embedded into ``data``:  ``{"data":{"message":"..."}}``.
+  For now, this style will remain the supported default. In a future version of Dataverse the ``message`` will always be a separate top field: ``{"data":{},"message":"..."}``.
+  Integrators and client vendors are welcome to opt-in to the new style and test thoroughly by enabling :ref:`dataverse.feature.unify-api-response-message-style`.
 - The following GET APIs will now return ``400`` if a required Guestbook Response is not supplied. A Guestbook Response can be passed to these APIs in the JSON body using a POST call. See the notes under :ref:`basic-file-access` and :ref:`download-by-dataset-by-version` for details.
 
   - **/api/access/datafile/{fileId:.+}**

doc/sphinx-guides/source/installation/config.rst

@@ -3430,6 +3430,31 @@ Can also be set via any `supported MicroProfile Config API source`_, e.g. the en
    This setting will be ignored unless the :ref:`dataverse.api.blocked.policy` is set to ``unblock-key``.  Otherwise the deprecated :ref:`:BlockedApiKey` will be used
 
 
+.. _dataverse.legacy.api-response-message-style:
+
+dataverse.legacy.api-response-message-style
++++++++++++++++++++++++++++++++++++++++++++
+
+Opt-out of no longer nesting an object in the "message" field, carrying the actual notification in its "message" field.
+Enabling this will re-activate the legacy message style using ``{"message":{"message":"..."}}``, instead of the aligned format ``{"message": "..."}``.
+
+This option is provided as a temporary workaround for integrations that may have implemented
+workarounds for the buggy behavior. The following endpoints are affected:
+
+- ``POST /api/datasets/{id}/add`` (just the duplicate file warning)
+- ``PUT /api/admin/settings``
+- ``PUT /api/dataverses/{id}``
+- ``PUT /api/dataverses/{id}/inputLevels``
+- ``POST /api/admin/savedsearches``
+- ``PUT /api/harvest/clients/{nickName}``
+- ``PUT /api/harvest/server/oaisets/{specname}``
+
+Please update your integrations to expect the corrected message format and deactivate this setting.
+In a future version of Dataverse, the legacy format is expected to be removed completely.
+See also :ref:`dataverse.feature.unify-api-response-message-style`.
+
+Can also be set via any `supported MicroProfile Config API source`_, e.g. the environment variable ``DATAVERSE_LEGACY_API_RESPONSE_MESSAGE_STYLE``.
+
 .. _dataverse.ui.show-validity-label-when-published:
 
 dataverse.ui.show-validity-label-when-published
@@ -4022,6 +4047,19 @@ dataverse.feature.api-bearer-auth-use-oauth-user-on-id-match
 
 Allows the use of an OAuth user account (GitHub, Google, or ORCID) when an identity match is found during API bearer authentication. This feature enables automatic association of an incoming IdP identity with an existing OAuth user account, bypassing the need for additional user registration steps. This feature only works when the feature flag ``api-bearer-auth`` is also enabled. **Caution: Enabling this flag could result in impersonation risks if (and only if) used with a misconfigured IdP.**
 
+.. _dataverse.feature.unify-api-response-message-style:
+
+dataverse.feature.unify-api-response-message-style
+++++++++++++++++++++++++++++++++++++++++++++++++++
+
+When activated, the "message" in API responses will no longer be nested in the "data" field.
+For any response carrying a notification, these will be found within a top-level "message" field of the JSON returned.
+This affects about 230 endpoints and is likely to break existing integrations and clients.
+It is mandatory to test instance clients and integrations thoroughly and it is not recommended to be used in production.
+In a future Dataverse version, the (currently) experimental response message style will be made the only supported one.
+
+See also :ref:`dataverse.legacy.api-response-message-style`.
+
 .. _dataverse.feature.avoid-expensive-solr-join:
 
 dataverse.feature.avoid-expensive-solr-join

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

@@ -32,6 +32,7 @@
 import edu.harvard.iq.dataverse.metrics.MetricsServiceBean;
 import edu.harvard.iq.dataverse.search.savedsearch.SavedSearchServiceBean;
 import edu.harvard.iq.dataverse.settings.FeatureFlags;
+import edu.harvard.iq.dataverse.settings.JvmSettings;
 import edu.harvard.iq.dataverse.settings.SettingsServiceBean;
 import edu.harvard.iq.dataverse.util.BundleUtil;
 import edu.harvard.iq.dataverse.util.DateUtil;
@@ -310,7 +311,7 @@ protected String getRequestParameter( String key ) {
     }
 
     protected String getRequestApiKey() {
-        String headerParamApiKey = httpRequest.getHeader(DATAVERSE_KEY_HEADER_NAME);
+        String headerParamApiKey = httpRequest.getHeader(ApiConstants.DATAVERSE_KEY_HEADER_NAME);
         String queryParamApiKey = httpRequest.getParameter("key");
 
         return headerParamApiKey!=null ? headerParamApiKey : queryParamApiKey;
@@ -435,11 +436,11 @@ protected Dataset findDatasetOrDie(String id, boolean deep) throws WrappedRespon
             }
         } else {
             String persistentId = id;
-            if (id.equals(PERSISTENT_ID_KEY)) {
-                persistentId = getRequestParameter(PERSISTENT_ID_KEY.substring(1));
+            if (id.equals(ApiConstants.PERSISTENT_ID_KEY)) {
+                persistentId = getRequestParameter(ApiConstants.PERSISTENT_ID_KEY.substring(1));
                 if (persistentId == null) {
                     throw new WrappedResponse(
-                            badRequest(BundleUtil.getStringFromBundle("find.dataset.error.dataset_id_is_null", Collections.singletonList(PERSISTENT_ID_KEY.substring(1)))));
+                            badRequest(BundleUtil.getStringFromBundle("find.dataset.error.dataset_id_is_null", Collections.singletonList(ApiConstants.PERSISTENT_ID_KEY.substring(1)))));
                 }
             }
             GlobalId globalId;
@@ -460,7 +461,7 @@ protected Dataset findDatasetOrDie(String id, boolean deep) throws WrappedRespon
                     fprLogService.logEntry(entry);
                 }
                 throw new WrappedResponse(
-                        notFound(BundleUtil.getStringFromBundle("find.dataset.error.dataset_id_is_null", Collections.singletonList(PERSISTENT_ID_KEY.substring(1)))));
+                        notFound(BundleUtil.getStringFromBundle("find.dataset.error.dataset_id_is_null", Collections.singletonList(ApiConstants.PERSISTENT_ID_KEY.substring(1)))));
             }
         }
         if (deep) {
@@ -542,11 +543,11 @@ protected void validateInternalTimestampIsNotOutdated(DvObject dvObject, String
 
     protected DataFile findDataFileOrDie(String id) throws WrappedResponse {
         DataFile datafile;
-        if (id.equals(PERSISTENT_ID_KEY)) {
-            String persistentId = getRequestParameter(PERSISTENT_ID_KEY.substring(1));
+        if (id.equals(ApiConstants.PERSISTENT_ID_KEY)) {
+            String persistentId = getRequestParameter(ApiConstants.PERSISTENT_ID_KEY.substring(1));
             if (persistentId == null) {
                 throw new WrappedResponse(
-                        badRequest(BundleUtil.getStringFromBundle("find.dataset.error.dataset_id_is_null", Collections.singletonList(PERSISTENT_ID_KEY.substring(1)))));
+                        badRequest(BundleUtil.getStringFromBundle("find.dataset.error.dataset_id_is_null", Collections.singletonList(ApiConstants.PERSISTENT_ID_KEY.substring(1)))));
             }
             datafile = fileService.findByGlobalId(persistentId);
             if (datafile == null) {
@@ -574,8 +575,8 @@ protected DataFile findDataFileOrDie(String id) throws WrappedResponse {
 
     protected DataverseRole findRoleOrDie(String id) throws WrappedResponse {
         DataverseRole role;
-        if (id.equals(ALIAS_KEY)) {
-            String alias = getRequestParameter(ALIAS_KEY.substring(1));
+        if (id.equals(ApiConstants.ALIAS_KEY)) {
+            String alias = getRequestParameter(ApiConstants.ALIAS_KEY.substring(1));
             try {
                 return em.createNamedQuery("DataverseRole.findDataverseRoleByAlias", DataverseRole.class)
                         .setParameter("alias", alias)
@@ -607,11 +608,11 @@ protected DatasetLinkingDataverse findDatasetLinkingDataverseOrDie(String datase
         DatasetLinkingDataverse dsld;
         Dataverse linkingDataverse = findDataverseOrDie(linkingDataverseId);
 
-        if (datasetId.equals(PERSISTENT_ID_KEY)) {
-            String persistentId = getRequestParameter(PERSISTENT_ID_KEY.substring(1));
+        if (datasetId.equals(ApiConstants.PERSISTENT_ID_KEY)) {
+            String persistentId = getRequestParameter(ApiConstants.PERSISTENT_ID_KEY.substring(1));
             if (persistentId == null) {
                 throw new WrappedResponse(
-                        badRequest(BundleUtil.getStringFromBundle("find.dataset.error.dataset_id_is_null", Collections.singletonList(PERSISTENT_ID_KEY.substring(1)))));
+                        badRequest(BundleUtil.getStringFromBundle("find.dataset.error.dataset_id_is_null", Collections.singletonList(ApiConstants.PERSISTENT_ID_KEY.substring(1)))));
             }
 
             Dataset dataset = datasetSvc.findByGlobalId(persistentId);
@@ -983,71 +984,67 @@ private Response handleDataverseRequestHandlerException(Exception ex) {
      *  HTTP Response methods *
     \* ====================== */
 
-    protected Response ok( JsonArrayBuilder bld ) {
-        return Response.ok(Json.createObjectBuilder()
-            .add("status", ApiConstants.STATUS_OK)
-            .add("data", bld).build())
-            .type(MediaType.APPLICATION_JSON).build();
+    protected Response ok(JsonValue value, JsonValue message, Long totalCount) {
+        return Response.status(Response.Status.OK)
+            .entity(NullSafeJsonBuilder.jsonObjectBuilder()
+                .add(ApiConstants.STATUS_FIELD, ApiConstants.STATUS_OK)
+                .add(ApiConstants.MESSAGE_FIELD, message)
+                .add(ApiConstants.TOTAL_COUNT_FIELD, totalCount)
+                .add(ApiConstants.DATA_FIELD, value)
+                .build())
+            .type(MediaType.APPLICATION_JSON)
+            .build();
+
+    }
+
+    protected Response ok(JsonArrayBuilder bld) {
+        return ok(bld.build(), null, null);
     }
 
-    protected Response ok( JsonArrayBuilder bld , long totalCount) {
-        return Response.ok(Json.createObjectBuilder()
-                        .add("status", ApiConstants.STATUS_OK)
-                        .add("totalCount", totalCount)
-                        .add("data", bld).build())
-                .type(MediaType.APPLICATION_JSON).build();
+    protected Response ok(JsonArrayBuilder bld, long totalCount) {
+        return ok(bld.build(), null, totalCount);
     }
 
-    protected Response ok( JsonArray ja ) {
-        return Response.ok(Json.createObjectBuilder()
-            .add("status", ApiConstants.STATUS_OK)
-            .add("data", ja).build())
-            .type(MediaType.APPLICATION_JSON).build();
+    protected Response ok(JsonArray ja) {
+        return ok(ja, null, null);
     }
 
-    protected Response ok( JsonObjectBuilder bld ) {
-        return Response.ok( Json.createObjectBuilder()
-            .add("status", ApiConstants.STATUS_OK)
-            .add("data", bld).build() )
-            .type(MediaType.APPLICATION_JSON)
-            .build();
+    protected Response ok(JsonObjectBuilder bld) {
+        return ok(bld.build(), null, null);
     }
 
-    protected Response ok( JsonObject jo ) {
-        return Response.ok( Json.createObjectBuilder()
-                .add("status", ApiConstants.STATUS_OK)
-                .add("data", jo).build() )
-                .type(MediaType.APPLICATION_JSON)
-                .build();    
+    protected Response ok(JsonObject jo) {
+        return ok(jo, null, null);
     }
 
-    protected Response ok( String msg ) {
-        return Response.ok().entity(Json.createObjectBuilder()
-            .add("status", ApiConstants.STATUS_OK)
-            .add("data", Json.createObjectBuilder().add("message",msg)).build() )
-            .type(MediaType.APPLICATION_JSON)
-            .build();
+    protected Response ok(String msg) {
+        // An instance may opt out of using the old {data:{message:"$msg"}} way.
+        // This is a highly used response builder, which is why this is an experimental opt-in change!
+        // TODO: This will be removed in a future version.
+        if (FeatureFlags.UNIFY_API_RESPONSE_MESSAGE_STYLE.enabled()) {
+            return ok(null, Json.createValue(msg), null);
+        } else {
+            return ok(Json.createObjectBuilder().add("message", msg).build(), null, null);
+        }
     }
 
-    protected Response ok( String msg, JsonObjectBuilder bld  ) {
-        return Response.ok().entity(Json.createObjectBuilder()
-            .add("status", ApiConstants.STATUS_OK)
-            .add("message", Json.createObjectBuilder().add("message",msg))     
-            .add("data", bld).build())      
-            .type(MediaType.APPLICATION_JSON)
-            .build();
+    protected Response ok(String msg, JsonObjectBuilder bld) {
+        // Legacy mode returns message as nested object for backward compatibility with integrations that worked around the bug.
+        // This is a scarcely used way to build a response, mostly relevant to admins, which is why we make it opt-out.
+        // TODO: This will be removed in a future version.
+        if (JvmSettings.LEGACY_API_RESPONSE_MESSAGE_STYLE.lookupOptional(Boolean.class).orElse(false)) {
+            return ok(bld.build(), Json.createObjectBuilder().add(ApiConstants.MESSAGE_FIELD, msg).build(), null);
+        } else {
+            return ok(bld.build(), Json.createValue(msg), null);
+        }
     }
 
     protected Response ok( boolean value ) {
-        return Response.ok().entity(Json.createObjectBuilder()
-            .add("status", ApiConstants.STATUS_OK)
-            .add("data", value).build() ).build();
+        return ok(value ? JsonValue.TRUE : JsonValue.FALSE, null, null);
     }
 
     protected Response ok(long value) {
-        return Response.ok().entity(Json.createObjectBuilder()
-                .add("status", ApiConstants.STATUS_OK)
-                .add("data", value).build()).build();
+        return ok(Json.createValue(value), null, null);
     }
 
     /**
@@ -1072,24 +1069,24 @@ protected Response ok(InputStream inputStream) {
     protected Response created( String uri, JsonObjectBuilder bld ) {
         return Response.created( URI.create(uri) )
                 .entity( Json.createObjectBuilder()
-                .add("status", "OK")
-                .add("data", bld).build())
+                .add(ApiConstants.STATUS_FIELD, ApiConstants.STATUS_OK)
+                .add(ApiConstants.DATA_FIELD, bld).build())
                 .type(MediaType.APPLICATION_JSON)
                 .build();
     }
 
     protected Response accepted(JsonObjectBuilder bld) {
         return Response.accepted()
                 .entity(Json.createObjectBuilder()
-                        .add("status", STATUS_WF_IN_PROGRESS)
-                        .add("data",bld).build()
+                        .add(ApiConstants.STATUS_FIELD, ApiConstants.STATUS_WF_IN_PROGRESS)
+                        .add(ApiConstants.DATA_FIELD, bld).build()
                 ).build();
     }
 
     protected Response accepted() {
         return Response.accepted()
                 .entity(Json.createObjectBuilder()
-                        .add("status", STATUS_WF_IN_PROGRESS).build()
+                        .add(ApiConstants.STATUS_FIELD, ApiConstants.STATUS_WF_IN_PROGRESS).build()
                 ).build();
     }
 
@@ -1104,8 +1101,8 @@ protected Response badRequest( String msg ) {
     protected Response badRequest(String msg, Map<String, String> fieldErrors) {
         return Response.status(Status.BAD_REQUEST)
                 .entity(NullSafeJsonBuilder.jsonObjectBuilder()
-                        .add("status", ApiConstants.STATUS_ERROR)
-                        .add("message", msg)
+                        .add(ApiConstants.STATUS_FIELD, ApiConstants.STATUS_ERROR)
+                        .add(ApiConstants.MESSAGE_FIELD, msg)
                         .add("fieldErrors", Json.createObjectBuilder(fieldErrors).build())
                         .build()
                 )
@@ -1138,7 +1135,7 @@ protected Response conflict( String msg ) {
     }
 
     protected Response authenticatedUserRequired() {
-        return error(Status.UNAUTHORIZED, RESPONSE_MESSAGE_AUTHENTICATED_USER_REQUIRED);
+        return error(Status.UNAUTHORIZED, ApiConstants.RESPONSE_MESSAGE_AUTHENTICATED_USER_REQUIRED);
     }
 
     protected Response permissionError( PermissionException pe ) {
@@ -1163,9 +1160,9 @@ protected Response unauthorized( String message ) {
 
     protected static Response error( Status sts, String msg ) {
         return Response.status(sts)
-                .entity( NullSafeJsonBuilder.jsonObjectBuilder()
-                        .add("status", ApiConstants.STATUS_ERROR)
-                        .add( "message", msg ).build()
+                .entity(NullSafeJsonBuilder.jsonObjectBuilder()
+                        .add(ApiConstants.STATUS_FIELD, ApiConstants.STATUS_ERROR)
+                        .add(ApiConstants.MESSAGE_FIELD, msg ).build()
                 ).type(MediaType.APPLICATION_JSON_TYPE).build();
     }
 }