From 9f7bd3cb2122f6f81babfef3ca0d8cfba423abcd Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 26 Oct 2016 15:41:09 -0400 Subject: [PATCH] API consistency: OK, response only has "data" #1612 Whenever the native API returns an OK response, it returns a JSON object with the key "data" and lots of detail within. "data" is the only key returned so I'm moving "message" to be within the "data" object to be consistent with the rest of the native API. I'm also removing the `okResponseGsonObject` method from the AbstractApiBean for consistency. We don't want it to be cluttered with Gson, Jackson or other methods. --- .../iq/dataverse/api/AbstractApiBean.java | 19 -------- .../harvard/iq/dataverse/api/Datasets.java | 4 +- .../edu/harvard/iq/dataverse/api/Files.java | 4 +- .../datasetutility/AddReplaceFileHelper.java | 16 ++++++- .../edu/harvard/iq/dataverse/api/FilesIT.java | 48 +++++++++---------- 5 files changed, 41 insertions(+), 50 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 8396e85c40b..531123ed301 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -499,26 +499,7 @@ protected Response ok( String msg ) { .type(MediaType.APPLICATION_JSON) .build(); } - - - /** - * Added to accommodate a JSON String generated from gson - * - * @param gsonObject - * @return - */ - protected Response okResponseGsonObject(String msg, com.google.gson.JsonObject gsonObject){ - - if (gsonObject == null){ - throw new NullPointerException("gsonObject cannot be null"); - } - gsonObject.addProperty("status", "OK"); - gsonObject.addProperty("message", msg); - - return Response.ok(gsonObject.toString(), MediaType.APPLICATION_JSON).build(); - } - /** * Returns an OK response (HTTP 200, status:OK) with the passed value * in the data field. diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index 921624053fc..ef85fd0e6fe 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -661,9 +661,7 @@ public Response addFileToDataset(@PathParam("id") String idSupplied, String successMsg = ResourceBundle.getBundle("Bundle").getString("file.addreplace.success.add"); try { //msgt("as String: " + addFileHelper.getSuccessResult()); - - return okResponseGsonObject(successMsg, - addFileHelper.getSuccessResultAsGsonObject()); + return ok(AddReplaceFileHelper.convertGsonObjectToJsonObjectBuilder(successMsg, addFileHelper.getSuccessResultAsGsonObject())); //"Look at that! You added a file! (hey hey, it may have worked)"); } catch (NoFilesException ex) { Logger.getLogger(Files.class.getName()).log(Level.SEVERE, null, ex); diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index fd40bf7b30d..6fe786a9250 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -207,8 +207,8 @@ public Response replaceFileInDataset( try { msgt("as String: " + addFileHelper.getSuccessResult()); - return okResponseGsonObject(successMsg, - addFileHelper.getSuccessResultAsGsonObject()); + return ok(AddReplaceFileHelper.convertGsonObjectToJsonObjectBuilder(successMsg, addFileHelper.getSuccessResultAsGsonObject())); + //"Look at that! You added a file! (hey hey, it may have worked)"); } catch (NoFilesException ex) { Logger.getLogger(Files.class.getName()).log(Level.SEVERE, null, ex); diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java b/src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java index 9d02ffa227b..dc51bd0fc69 100644 --- a/src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java +++ b/src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java @@ -25,6 +25,7 @@ import edu.harvard.iq.dataverse.ingest.IngestServiceBean; import java.io.IOException; import java.io.InputStream; +import java.io.StringReader; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -34,6 +35,9 @@ import java.util.logging.Level; import java.util.logging.Logger; import javax.ejb.EJBException; +import javax.json.Json; +import javax.json.JsonObjectBuilder; +import javax.json.JsonReader; import javax.validation.ConstraintViolation; import javax.ws.rs.core.Response; @@ -1468,8 +1472,16 @@ public JsonObject getSuccessResultAsGsonObject() throws NoFilesException{ //return newlyAddedFile.asGsonObject(false); } - - + + public static JsonObjectBuilder convertGsonObjectToJsonObjectBuilder(String successMsg, JsonObject gsonObject) { + JsonObjectBuilder jsonObjectBuilder = Json.createObjectBuilder(); + jsonObjectBuilder.add("message", successMsg); + JsonReader jsonReader = Json.createReader(new StringReader(gsonObject.toString())); + javax.json.JsonObject object = jsonReader.readObject(); + jsonObjectBuilder.add("files", object.get("files")); + return jsonObjectBuilder; + } + /** * Currently this is a placeholder if we decide to send * user notifications. diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index 95dc9f6f659..5358426bc01 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -91,10 +91,10 @@ public void test_001_AddFileGood() { addResponse.then().assertThat() - .body("message", equalTo(successMsg)) + .body("data.message", equalTo(successMsg)) .body("status", equalTo(AbstractApiBean.STATUS_OK)) - .body("files[0].contentType", equalTo("image/png")) - .body("files[0].filename", equalTo("dataverseproject.png")) + .body("data.files[0].contentType", equalTo("image/png")) + .body("data.files[0].filename", equalTo("dataverseproject.png")) .statusCode(OK.getStatusCode()); @@ -240,13 +240,13 @@ public void test_006_ReplaceFileGood() { String successMsgAdd = ResourceBundle.getBundle("Bundle").getString("file.addreplace.success.add"); addResponse.then().assertThat() - .body("message", equalTo(successMsgAdd)) - .body("files[0].contentType", equalTo("image/png")) - .body("files[0].filename", equalTo("dataverseproject.png")) + .body("data.message", equalTo(successMsgAdd)) + .body("data.files[0].contentType", equalTo("image/png")) + .body("data.files[0].filename", equalTo("dataverseproject.png")) .statusCode(OK.getStatusCode()); - long origFileId = JsonPath.from(addResponse.body().asString()).getLong("files[0].id"); + long origFileId = JsonPath.from(addResponse.body().asString()).getLong("data.files[0].id"); msg("Orig file id: " + origFileId); assertNotNull(origFileId); // If checkOut fails, display message @@ -296,14 +296,14 @@ public void test_006_ReplaceFileGood() { replaceResp.then().assertThat() .statusCode(OK.getStatusCode()) - .body("message", equalTo(successMsg2)) - .body("files[0].filename", equalTo("cc0.png")) + .body("data.message", equalTo(successMsg2)) + .body("data.files[0].filename", equalTo("cc0.png")) //.body("data.rootDataFileId", equalTo(origFileId)) ; - long rootDataFileId = JsonPath.from(replaceResp.body().asString()).getLong("files[0].rootDataFileId"); - long previousDataFileId = JsonPath.from(replaceResp.body().asString()).getLong("files[0].previousDataFileId"); - long newDataFileId = JsonPath.from(replaceResp.body().asString()).getLong("files[0].id"); + long rootDataFileId = JsonPath.from(replaceResp.body().asString()).getLong("data.files[0].rootDataFileId"); + long previousDataFileId = JsonPath.from(replaceResp.body().asString()).getLong("data.files[0].previousDataFileId"); + long newDataFileId = JsonPath.from(replaceResp.body().asString()).getLong("data.files[0].id"); assertEquals(origFileId, previousDataFileId); assertEquals(rootDataFileId, previousDataFileId); @@ -330,12 +330,12 @@ public void test_006_ReplaceFileGood() { replaceResp2.then().assertThat() .statusCode(OK.getStatusCode()) .body("status", equalTo(AbstractApiBean.STATUS_OK)) - .body("message", equalTo(successMsg2)) - .body("files[0].filename", equalTo("favicondataverse.png")) + .body("data.message", equalTo(successMsg2)) + .body("data.files[0].filename", equalTo("favicondataverse.png")) ; - long rootDataFileId2 = JsonPath.from(replaceResp2.body().asString()).getLong("files[0].rootDataFileId"); - long previousDataFileId2 = JsonPath.from(replaceResp2.body().asString()).getLong("files[0].previousDataFileId"); + long rootDataFileId2 = JsonPath.from(replaceResp2.body().asString()).getLong("data.files[0].rootDataFileId"); + long previousDataFileId2 = JsonPath.from(replaceResp2.body().asString()).getLong("data.files[0].previousDataFileId"); msgt("newDataFileId: " + newDataFileId); msgt("previousDataFileId2: " + previousDataFileId2); @@ -369,13 +369,13 @@ public void test_007_ReplaceFileUnpublishedAndBadIds() { String successMsgAdd = ResourceBundle.getBundle("Bundle").getString("file.addreplace.success.add"); addResponse.then().assertThat() - .body("message", equalTo(successMsgAdd)) - .body("files[0].contentType", equalTo("image/png")) - .body("files[0].filename", equalTo("dataverseproject.png")) + .body("data.message", equalTo(successMsgAdd)) + .body("data.files[0].contentType", equalTo("image/png")) + .body("data.files[0].filename", equalTo("dataverseproject.png")) .statusCode(OK.getStatusCode()); - long origFileId = JsonPath.from(addResponse.body().asString()).getLong("files[0].id"); + long origFileId = JsonPath.from(addResponse.body().asString()).getLong("data.files[0].id"); msg("Orig file id: " + origFileId); assertNotNull(origFileId); // If checkOut fails, display message @@ -452,13 +452,13 @@ public void test_008_ReplaceFileAlreadyDeleted() { String successMsgAdd = ResourceBundle.getBundle("Bundle").getString("file.addreplace.success.add"); addResponse.then().assertThat() - .body("message", equalTo(successMsgAdd)) - .body("files[0].contentType", equalTo("image/png")) - .body("files[0].filename", equalTo("dataverseproject.png")) + .body("data.message", equalTo(successMsgAdd)) + .body("data.files[0].contentType", equalTo("image/png")) + .body("data.files[0].filename", equalTo("dataverseproject.png")) .statusCode(OK.getStatusCode()); - long origFileId = JsonPath.from(addResponse.body().asString()).getLong("files[0].id"); + long origFileId = JsonPath.from(addResponse.body().asString()).getLong("data.files[0].id"); msg("Orig file id: " + origFileId); assertNotNull(origFileId); // If checkOut fails, display message