From 3c5aa0ee1a8049e300af549eca5adc7b2ff1fe80 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Fri, 10 Feb 2023 18:05:12 +0100 Subject: [PATCH 01/24] Support for deleting files using native API --- doc/release-notes/3913-delete-file-endpoint | 1 + doc/sphinx-guides/source/api/native-api.rst | 37 +++++++++++ .../edu/harvard/iq/dataverse/api/Files.java | 64 +++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 doc/release-notes/3913-delete-file-endpoint diff --git a/doc/release-notes/3913-delete-file-endpoint b/doc/release-notes/3913-delete-file-endpoint new file mode 100644 index 00000000000..8c748582f2e --- /dev/null +++ b/doc/release-notes/3913-delete-file-endpoint @@ -0,0 +1 @@ +Support for deleting files using native API. \ No newline at end of file diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 3cd469e3883..0375873f840 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2444,6 +2444,43 @@ The fully expanded example above (without environment variables) looks like this -F 'jsonData={"description":"My description.","categories":["Data"],"forceReplace":false}' \ "https://demo.dataverse.org/api/files/:persistentId/replace?persistentId=doi:10.5072/FK2/AAA000" +Deleting Files +~~~~~~~~~~~~~~ + +Delete an existing file where ``ID`` is the database id of the file to delete or ``PERSISTENT_ID`` is the persistent id (DOI or Handle) of the file. + +A curl example using an ``ID`` + +.. code-block:: bash + + export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + export SERVER_URL=https://demo.dataverse.org + export ID=24 + + curl -H "X-Dataverse-key:$API_TOKEN" -X DELETE $SERVER_URL/api/files/$ID + +The fully expanded example above (without environment variables) looks like this: + +.. code-block:: bash + + curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X DELETE https://demo.dataverse.org/api/files/24 + +A curl example using a ``PERSISTENT_ID`` + +.. code-block:: bash + + export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + export SERVER_URL=https://demo.dataverse.org + export PERSISTENT_ID=doi:10.5072/FK2/AAA000 + + curl -H "X-Dataverse-key:$API_TOKEN" -X DELETE "$SERVER_URL/api/files/:persistentId?persistentId=$PERSISTENT_ID" + +The fully expanded example above (without environment variables) looks like this: + +.. code-block:: bash + + curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X DELETE "https://demo.dataverse.org/api/files/:persistentId?persistentId=doi:10.5072/FK2/AAA000" + Getting File Metadata ~~~~~~~~~~~~~~~~~~~~~ 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 965d56d355e..c3fb7b01095 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -46,17 +46,22 @@ import static edu.harvard.iq.dataverse.util.json.JsonPrinter.json; import edu.harvard.iq.dataverse.util.json.JsonUtil; import edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder; + +import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; import javax.ejb.EJB; +import javax.ejb.EJBException; import javax.inject.Inject; import javax.json.Json; import javax.servlet.http.HttpServletResponse; import javax.ws.rs.Consumes; +import javax.ws.rs.DELETE; import javax.ws.rs.GET; import javax.ws.rs.POST; import javax.ws.rs.PUT; @@ -317,6 +322,65 @@ public Response replaceFileInDataset( } } // end: replaceFileInDataset + + /** + * Delete an Existing File + * + * @param id file ID or peristent ID + */ + @DELETE + @Path("{id}") + public Response deleteFileInDataset(@PathParam("id") String fileIdOrPersistentId){ + + if (!systemConfig.isHTTPUpload()) { + return error(Response.Status.SERVICE_UNAVAILABLE, BundleUtil.getStringFromBundle("file.api.httpDisabled")); + } + // (1) Get the user from the API key + User authUser; + try { + authUser = findUserOrDie(); + } catch (AbstractApiBean.WrappedResponse ex) { + return error(Response.Status.FORBIDDEN, BundleUtil.getStringFromBundle("file.addreplace.error.auth")); + } + + msg("DELETE!"); + + // (2) Delete + boolean deletePhysicalFile = false; + try { + DataFile dataFile = findDataFileOrDie(fileIdOrPersistentId); + List fileToDelete = dataFile.getFileMetadatas(); + Dataset dataset = dataFile.getOwner(); + DatasetVersion v = dataset.getOrCreateEditVersion(); + DataverseRequest dvRequest2 = createDataverseRequest(authUser); + dvRequest2.getUser(); + deletePhysicalFile = !dataFile.isReleased(); + + UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest2, fileToDelete, v); + update_cmd.setValidateLenient(true); + + try { + commandEngine.submit(update_cmd); + } catch (CommandException ex) { + return error(BAD_REQUEST, "Delete failed for file ID: " + fileIdOrPersistentId); + } catch (EJBException ex) { + return error(BAD_REQUEST, "Delete failed for file ID: " + fileIdOrPersistentId); + } + + if (deletePhysicalFile) { + try { + fileService.finalizeFileDelete(dataFile.getId(), fileService.getPhysicalFileToDelete(dataFile)); + } catch (IOException ioex) { + logger.warning("Failed to delete the physical file associated with the deleted datafile id=" + + dataFile.getId() + ", storage location: " + fileService.getPhysicalFileToDelete(dataFile)); + } + } + } catch (WrappedResponse ex) { + return error(BAD_REQUEST, "There was no file found for ID: " + fileIdOrPersistentId); + } + + return ok(deletePhysicalFile); + } //Much of this code is taken from the replace command, //simplified as we aren't actually switching files From 4d30879709c796f84ea2d3976854abdaf8eb3537 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Fri, 10 Feb 2023 19:17:01 +0100 Subject: [PATCH 02/24] more informative error messages --- src/main/java/edu/harvard/iq/dataverse/api/Files.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 c3fb7b01095..0a86f0e51d9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -362,9 +362,9 @@ public Response deleteFileInDataset(@PathParam("id") String fileIdOrPersistentId try { commandEngine.submit(update_cmd); } catch (CommandException ex) { - return error(BAD_REQUEST, "Delete failed for file ID: " + fileIdOrPersistentId); + return error(BAD_REQUEST, "Delete failed for file ID: " + fileIdOrPersistentId + ": " + ex.getMessage()); } catch (EJBException ex) { - return error(BAD_REQUEST, "Delete failed for file ID: " + fileIdOrPersistentId); + return error(BAD_REQUEST, "Delete failed for file ID: " + fileIdOrPersistentId + ": " + ex.getMessage()); } if (deletePhysicalFile) { From 8f125ddc483251d6f8351f8272feee1818b86c0d Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Fri, 10 Feb 2023 19:33:48 +0100 Subject: [PATCH 03/24] removed unused getUser() call --- src/main/java/edu/harvard/iq/dataverse/api/Files.java | 1 - 1 file changed, 1 deletion(-) 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 0a86f0e51d9..0a33efbec29 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -353,7 +353,6 @@ public Response deleteFileInDataset(@PathParam("id") String fileIdOrPersistentId Dataset dataset = dataFile.getOwner(); DatasetVersion v = dataset.getOrCreateEditVersion(); DataverseRequest dvRequest2 = createDataverseRequest(authUser); - dvRequest2.getUser(); deletePhysicalFile = !dataFile.isReleased(); UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest2, fileToDelete, v); From a4304e203238396911335a59fd9ea3d205cbf9a4 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 10:47:57 +0100 Subject: [PATCH 04/24] added AuthRequired annotation and replaced get user method in delete dataset --- .../java/edu/harvard/iq/dataverse/api/Files.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) 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 af1fc85cd77..c5a9643005a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -323,21 +323,15 @@ public Response replaceFileInDataset( * @param id file ID or peristent ID */ @DELETE + @AuthRequired @Path("{id}") - public Response deleteFileInDataset(@PathParam("id") String fileIdOrPersistentId){ + public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathParam("id") String fileIdOrPersistentId){ if (!systemConfig.isHTTPUpload()) { return error(Response.Status.SERVICE_UNAVAILABLE, BundleUtil.getStringFromBundle("file.api.httpDisabled")); } // (1) Get the user from the API key - User authUser; - try { - authUser = findUserOrDie(); - } catch (AbstractApiBean.WrappedResponse ex) { - return error(Response.Status.FORBIDDEN, BundleUtil.getStringFromBundle("file.addreplace.error.auth")); - } - - msg("DELETE!"); + User authUser = getRequestUser(crc); // (2) Delete boolean deletePhysicalFile = false; From 78e1ff84f2155d6cb8aca16cef545b1273d89e43 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 10:59:44 +0100 Subject: [PATCH 05/24] rename: dvRequest2 -> dvRequest --- src/main/java/edu/harvard/iq/dataverse/api/Files.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 c5a9643005a..aec7698fa47 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -340,10 +340,10 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP List fileToDelete = dataFile.getFileMetadatas(); Dataset dataset = dataFile.getOwner(); DatasetVersion v = dataset.getOrCreateEditVersion(); - DataverseRequest dvRequest2 = createDataverseRequest(authUser); + DataverseRequest dvRequest = createDataverseRequest(authUser); deletePhysicalFile = !dataFile.isReleased(); - UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest2, fileToDelete, v); + UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, fileToDelete, v); update_cmd.setValidateLenient(true); try { From 75c898628e32551752a70345205f9e72c73d8b20 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 11:03:48 +0100 Subject: [PATCH 06/24] moved DataverseRequest creation --- src/main/java/edu/harvard/iq/dataverse/api/Files.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 aec7698fa47..5d4b935c94f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -330,8 +330,9 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP if (!systemConfig.isHTTPUpload()) { return error(Response.Status.SERVICE_UNAVAILABLE, BundleUtil.getStringFromBundle("file.api.httpDisabled")); } - // (1) Get the user from the API key + // (1) Get the user from the API key and create request User authUser = getRequestUser(crc); + DataverseRequest dvRequest = createDataverseRequest(authUser); // (2) Delete boolean deletePhysicalFile = false; @@ -340,7 +341,6 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP List fileToDelete = dataFile.getFileMetadatas(); Dataset dataset = dataFile.getOwner(); DatasetVersion v = dataset.getOrCreateEditVersion(); - DataverseRequest dvRequest = createDataverseRequest(authUser); deletePhysicalFile = !dataFile.isReleased(); UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, fileToDelete, v); From 222b05220aa7b3a5478a2b57c66270dd04fb2190 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 11:07:11 +0100 Subject: [PATCH 07/24] removed the http upload enable check from the delete method --- src/main/java/edu/harvard/iq/dataverse/api/Files.java | 4 ---- 1 file changed, 4 deletions(-) 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 5d4b935c94f..8891eaa5271 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -326,10 +326,6 @@ public Response replaceFileInDataset( @AuthRequired @Path("{id}") public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathParam("id") String fileIdOrPersistentId){ - - if (!systemConfig.isHTTPUpload()) { - return error(Response.Status.SERVICE_UNAVAILABLE, BundleUtil.getStringFromBundle("file.api.httpDisabled")); - } // (1) Get the user from the API key and create request User authUser = getRequestUser(crc); DataverseRequest dvRequest = createDataverseRequest(authUser); From 8846a6a41205e0999ab40595fa922e49c258adca Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 11:15:10 +0100 Subject: [PATCH 08/24] replaced deprecated isNumber by isCreatable --- .../edu/harvard/iq/dataverse/api/UtilIT.java | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index 4262b709f58..5bc77bd61b6 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -795,7 +795,7 @@ static Response replaceFile(String fileIdOrPersistentId, String pathToFile, Json static Response replaceFile(String fileIdOrPersistentId, String pathToFile, String jsonAsString, String apiToken) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } @@ -812,7 +812,7 @@ static Response replaceFile(String fileIdOrPersistentId, String pathToFile, Stri static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } @@ -937,7 +937,7 @@ static Response downloadFiles(String datasetIdOrPersistentId, String datasetVers static Response downloadFiles(String datasetIdOrPersistentId, String datasetVersion, DownloadFormat format, String apiToken) { String idInPath = datasetIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(datasetIdOrPersistentId)) { + if (!NumberUtils.isCreatable(datasetIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + datasetIdOrPersistentId; } @@ -970,7 +970,7 @@ static Response subset(String fileId, String variables, String apiToken) { static Response getFileMetadata(String fileIdOrPersistentId, String optionalFormat, String apiToken) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "&persistentId=" + fileIdOrPersistentId; } @@ -987,7 +987,7 @@ static Response getFileMetadata(String fileIdOrPersistentId, String optionalForm static Response getFileMetadata(String fileIdOrPersistentId, String optionalFormat) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } @@ -1508,7 +1508,7 @@ static Response migrateBuiltinToOAuth(String data, String apiToken) { static Response restrictFile(String fileIdOrPersistentId, boolean restrict, String apiToken) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } @@ -1522,7 +1522,7 @@ static Response restrictFile(String fileIdOrPersistentId, boolean restrict, Stri static Response allowAccessRequests(String datasetIdOrPersistentId, boolean allowRequests, String apiToken) { String idInPath = datasetIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(datasetIdOrPersistentId)) { + if (!NumberUtils.isCreatable(datasetIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + datasetIdOrPersistentId; } @@ -1538,7 +1538,7 @@ static Response requestFileAccess(String fileIdOrPersistentId, String apiToken) System.out.print ("Reuest file acceess + apiToken: " + apiToken); String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } @@ -1556,7 +1556,7 @@ static Response requestFileAccess(String fileIdOrPersistentId, String apiToken) static Response grantFileAccess(String fileIdOrPersistentId, String identifier, String apiToken) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } @@ -1572,7 +1572,7 @@ static Response grantFileAccess(String fileIdOrPersistentId, String identifier, static Response getAccessRequestList(String fileIdOrPersistentId, String apiToken) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } @@ -1588,7 +1588,7 @@ static Response getAccessRequestList(String fileIdOrPersistentId, String apiToke static Response rejectFileAccessRequest(String fileIdOrPersistentId, String identifier, String apiToken) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(fileIdOrPersistentId)) { + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } @@ -1620,7 +1620,7 @@ static Response moveDataset(String idOrPersistentIdOfDatasetToMove, String desti } String idInPath = idOrPersistentIdOfDatasetToMove; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentIdOfDatasetToMove)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfDatasetToMove)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentIdOfDatasetToMove; } @@ -1738,7 +1738,7 @@ static Response getDatasetVersions(String idOrPersistentId, String apiToken) { logger.info("Getting Dataset Versions"); String idInPath = idOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentId)) { + if (!NumberUtils.isCreatable(idOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentId; } @@ -1755,7 +1755,7 @@ static Response getProvJson(String idOrPersistentId, String apiToken) { logger.info("Getting Provenance JSON"); String idInPath = idOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentId)) { + if (!NumberUtils.isCreatable(idOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentId; } @@ -1771,7 +1771,7 @@ static Response getProvFreeForm(String idOrPersistentId, String apiToken) { logger.info("Getting Provenance Free Form"); String idInPath = idOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentId)) { + if (!NumberUtils.isCreatable(idOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentId; } @@ -1787,7 +1787,7 @@ static Response uploadProvJson(String idOrPersistentId, JsonObject jsonObject, S logger.info("Uploading Provenance JSON"); String idInPath = idOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentId)) { + if (!NumberUtils.isCreatable(idOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentId; } @@ -1809,7 +1809,7 @@ static Response deleteProvJson(String idOrPersistentId, String apiToken) { //TODO: Repeated code, refactor String idInPath = idOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentId)) { + if (!NumberUtils.isCreatable(idOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentId; } @@ -1825,7 +1825,7 @@ static Response uploadProvFreeForm(String idOrPersistentId, JsonObject jsonObjec logger.info("Uploading Provenance Free Form"); String idInPath = idOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentId)) { + if (!NumberUtils.isCreatable(idOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentId; } @@ -1846,7 +1846,7 @@ static Response uploadProvFreeForm(String idOrPersistentId, JsonObject jsonObjec // //TODO: Repeated code, refactor // String idInPath = idOrPersistentId; // Assume it's a number. // String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. -// if (!NumberUtils.isNumber(idOrPersistentId)) { +// if (!NumberUtils.isCreatable(idOrPersistentId)) { // idInPath = ":persistentId"; // optionalQueryParam = "?persistentId=" + idOrPersistentId; // } @@ -2126,7 +2126,7 @@ static Response getRsyncScript(String datasetPersistentId, String apiToken) { static Response dataCaptureModuleChecksumValidation(String datasetPersistentId, JsonObject jsonObject, String apiToken) { String persistentIdInPath = datasetPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If datasetPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(datasetPersistentId)) { + if (!NumberUtils.isCreatable(datasetPersistentId)) { persistentIdInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + datasetPersistentId; } @@ -2173,7 +2173,7 @@ static Response deleteExternalTool(long externalToolid) { static Response getExternalToolsForDataset(String idOrPersistentIdOfDataset, String type, String apiToken) { String idInPath = idOrPersistentIdOfDataset; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentIdOfDataset)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfDataset)) { idInPath = ":persistentId"; optionalQueryParam = "&persistentId=" + idOrPersistentIdOfDataset; } @@ -2188,7 +2188,7 @@ static Response getExternalToolsForDataset(String idOrPersistentIdOfDataset, Str static Response getExternalToolsForFile(String idOrPersistentIdOfFile, String type, String apiToken) { String idInPath = idOrPersistentIdOfFile; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentIdOfFile)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfFile)) { idInPath = ":persistentId"; optionalQueryParam = "&persistentId=" + idOrPersistentIdOfFile; } @@ -2564,7 +2564,7 @@ static Response checkDatasetLocks(long datasetId, String lockType, String apiTok static Response checkDatasetLocks(String idOrPersistentId, String lockType, String apiToken) { String idInPath = idOrPersistentId; // Assume it's a number. String queryParams = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentId)) { + if (!NumberUtils.isCreatable(idOrPersistentId)) { idInPath = ":persistentId"; queryParams = "?persistentId=" + idOrPersistentId; } @@ -2733,7 +2733,7 @@ static Response makeDataCountGetMetricForDataset(String idOrPersistentIdOfDatase System.out.println("metric: " + metric); String idInPath = idOrPersistentIdOfDataset; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentIdOfDataset)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfDataset)) { idInPath = ":persistentId"; optionalQueryParam = "&persistentId=" + idOrPersistentIdOfDataset; } @@ -2749,7 +2749,7 @@ static Response makeDataCountGetMetricForDataset(String idOrPersistentIdOfDatase System.out.println("metric: " + metric); String idInPath = idOrPersistentIdOfDataset; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentIdOfDataset)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfDataset)) { idInPath = ":persistentId"; optionalQueryParam = "&persistentId=" + idOrPersistentIdOfDataset; } @@ -2765,7 +2765,7 @@ static Response makeDataCountGetMetricForDataset(String idOrPersistentIdOfDatase System.out.println("metric: " + metric); String idInPath = idOrPersistentIdOfDataset; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentIdOfDataset)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfDataset)) { idInPath = ":persistentId"; optionalQueryParam = "&persistentId=" + idOrPersistentIdOfDataset; } @@ -2781,7 +2781,7 @@ static Response makeDataCountAddUsageMetricsFromSushiReport(String idOrPersisten String idInPath = idOrPersistentIdOfDataset; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentIdOfDataset)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfDataset)) { idInPath = ":persistentId"; optionalQueryParam = "&persistentId=" + idOrPersistentIdOfDataset; } @@ -2794,7 +2794,7 @@ static Response makeDataCountUpdateCitationsForDataset(String idOrPersistentIdOf String idInPath = idOrPersistentIdOfDataset; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isNumber(idOrPersistentIdOfDataset)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfDataset)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentIdOfDataset; } @@ -3076,7 +3076,7 @@ static Response getDatasetVersionArchivalStatus(Integer datasetId, String versio static Response archiveDataset(String idOrPersistentIdOfDataset, String version, String apiToken) { String idInPath = idOrPersistentIdOfDataset; String optionalQueryParam = ""; - if (!NumberUtils.isNumber(idOrPersistentIdOfDataset)) { + if (!NumberUtils.isCreatable(idOrPersistentIdOfDataset)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + idOrPersistentIdOfDataset; } From 657f3942c8d38f79c707386628d578375cc4314e Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 12:06:28 +0100 Subject: [PATCH 09/24] delete file integration test --- .../edu/harvard/iq/dataverse/api/FilesIT.java | 37 +++++++++++++++++++ .../edu/harvard/iq/dataverse/api/UtilIT.java | 12 ++++++ tests/integration-tests.txt | 2 +- 3 files changed, 50 insertions(+), 1 deletion(-) 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 dd9ecf26228..d7fe9fb3357 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1896,4 +1896,41 @@ public void testAddFileToDatasetSkipTabIngest() throws IOException, InterruptedE } + @Test + public void testDeleteFile() { + msgt("testDeleteFile"); + // Create user + String apiToken = createUserGetToken(); + + // Create Dataverse + String dataverseAlias = createDataverseGetAlias(apiToken); + + // Create Dataset + Integer datasetId = createDatasetGetId(dataverseAlias, apiToken); + + // Upload file + String pathToFile = "src/main/webapp/resources/images/dataverseproject.png"; + JsonObjectBuilder json = Json.createObjectBuilder() + .add("description", "my description") + .add("directoryLabel", "data/subdir1") + .add("categories", Json.createArrayBuilder().add("Data")); + Response uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile, json.build(), apiToken); + uploadResponse.then().assertThat().statusCode(OK.getStatusCode()); + Integer fileId = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id"); + + // Check file uploaded + Response downloadResponse = UtilIT.downloadFile(fileId, null, null, null, apiToken); + downloadResponse.then().assertThat().statusCode(OK.getStatusCode()); + + //-------------// + // Delete file // + //-------------// + + Response deleteResponse = UtilIT.deleteFile(datasetId.toString(), fileId, apiToken); + deleteResponse.then().assertThat().statusCode(OK.getStatusCode()); + + // Check file deleted + Response downloadResponse2 = UtilIT.downloadFile(fileId, null, null, null, apiToken); + downloadResponse2.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); + } } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index 5bc77bd61b6..b27d16df044 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -808,6 +808,18 @@ static Response replaceFile(String fileIdOrPersistentId, String pathToFile, Stri return requestSpecification .post("/api/files/" + idInPath + "/replace" + optionalQueryParam); } + + static Response deleteFile(String fileIdOrPersistentId, Integer fileId, String apiToken) { + String idInPath = fileIdOrPersistentId; // Assume it's a number. + String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. + if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { + idInPath = ":persistentId"; + optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; + } + return given() + .header(API_TOKEN_HTTP_HEADER, apiToken) + .delete("/api/files/" + idInPath + "/id" + fileId + optionalQueryParam); + } static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken) { String idInPath = fileIdOrPersistentId; // Assume it's a number. diff --git a/tests/integration-tests.txt b/tests/integration-tests.txt index 1e9110be2de..cf026db8f6e 100644 --- a/tests/integration-tests.txt +++ b/tests/integration-tests.txt @@ -1 +1 @@ -DataversesIT,DatasetsIT,SwordIT,AdminIT,BuiltinUsersIT,UsersIT,UtilIT,ConfirmEmailIT,FileMetadataIT,FilesIT,SearchIT,InReviewWorkflowIT,HarvestingServerIT,HarvestingClientsIT,MoveIT,MakeDataCountApiIT,FileTypeDetectionIT,EditDDIIT,ExternalToolsIT,AccessIT,DuplicateFilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DeactivateUsersIT,AuxiliaryFilesIT,InvalidCharactersIT,LicensesIT,NotificationsIT,BagIT,MetadataBlocksIT,NetcdfIT +FilesIT#testDeleteFile From d8e3393bb3a72d9a6e81be9f128333fe4c712fdd Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 12:07:30 +0100 Subject: [PATCH 10/24] restored integration tests list --- tests/integration-tests.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration-tests.txt b/tests/integration-tests.txt index cf026db8f6e..dcc8971b256 100644 --- a/tests/integration-tests.txt +++ b/tests/integration-tests.txt @@ -1 +1 @@ -FilesIT#testDeleteFile +FilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DeactivateUsersIT,AuxiliaryFilesIT,InvalidCharactersIT,LicensesIT,NotificationsIT,BagIT,MetadataBlocksIT,NetcdfIT From 200910e4a629f5c1607576e59aa4f65e0fcdb502 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 12:08:36 +0100 Subject: [PATCH 11/24] restored integration tests list --- tests/integration-tests.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration-tests.txt b/tests/integration-tests.txt index dcc8971b256..1e9110be2de 100644 --- a/tests/integration-tests.txt +++ b/tests/integration-tests.txt @@ -1 +1 @@ -FilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DeactivateUsersIT,AuxiliaryFilesIT,InvalidCharactersIT,LicensesIT,NotificationsIT,BagIT,MetadataBlocksIT,NetcdfIT +DataversesIT,DatasetsIT,SwordIT,AdminIT,BuiltinUsersIT,UsersIT,UtilIT,ConfirmEmailIT,FileMetadataIT,FilesIT,SearchIT,InReviewWorkflowIT,HarvestingServerIT,HarvestingClientsIT,MoveIT,MakeDataCountApiIT,FileTypeDetectionIT,EditDDIIT,ExternalToolsIT,AccessIT,DuplicateFilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DeactivateUsersIT,AuxiliaryFilesIT,InvalidCharactersIT,LicensesIT,NotificationsIT,BagIT,MetadataBlocksIT,NetcdfIT From e49bea062bf047bb406b769849ae4ff36e1813cd Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Mon, 13 Mar 2023 13:06:32 +0100 Subject: [PATCH 12/24] integration test fix --- .../java/edu/harvard/iq/dataverse/api/FilesIT.java | 2 +- src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) 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 d7fe9fb3357..a0da2c95f71 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1926,7 +1926,7 @@ public void testDeleteFile() { // Delete file // //-------------// - Response deleteResponse = UtilIT.deleteFile(datasetId.toString(), fileId, apiToken); + Response deleteResponse = UtilIT.deleteFileApi(fileId, apiToken); deleteResponse.then().assertThat().statusCode(OK.getStatusCode()); // Check file deleted diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index b27d16df044..642480cf11c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -809,16 +809,10 @@ static Response replaceFile(String fileIdOrPersistentId, String pathToFile, Stri .post("/api/files/" + idInPath + "/replace" + optionalQueryParam); } - static Response deleteFile(String fileIdOrPersistentId, Integer fileId, String apiToken) { - String idInPath = fileIdOrPersistentId; // Assume it's a number. - String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. - if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { - idInPath = ":persistentId"; - optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; - } + static Response deleteFileApi(Integer fileId, String apiToken) { return given() .header(API_TOKEN_HTTP_HEADER, apiToken) - .delete("/api/files/" + idInPath + "/id" + fileId + optionalQueryParam); + .delete("/api/files/" + fileId); } static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken) { From 5b73690e0981d36aa22e588af76621b15f466ee8 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Tue, 14 Mar 2023 14:17:06 +0100 Subject: [PATCH 13/24] test fix --- src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a0da2c95f71..de540c39b6b 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1931,6 +1931,6 @@ public void testDeleteFile() { // Check file deleted Response downloadResponse2 = UtilIT.downloadFile(fileId, null, null, null, apiToken); - downloadResponse2.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); + downloadResponse2.then().assertThat().statusCode(NOT_FOUND.getStatusCode()); } } From 8a016c90e1bdb71b71e665a28c1c2f3ac4dc181f Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 15 Mar 2023 12:24:10 -0400 Subject: [PATCH 14/24] clean up error handling, add more tests #3913 --- .../edu/harvard/iq/dataverse/api/Files.java | 8 +- .../edu/harvard/iq/dataverse/api/FilesIT.java | 91 ++++++++++++++----- 2 files changed, 74 insertions(+), 25 deletions(-) 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 8891eaa5271..a81f97bb1b7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -345,9 +345,9 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP try { commandEngine.submit(update_cmd); } catch (CommandException ex) { - return error(BAD_REQUEST, "Delete failed for file ID: " + fileIdOrPersistentId + ": " + ex.getMessage()); + return error(BAD_REQUEST, "Delete failed for file ID " + fileIdOrPersistentId + " (CommandException): " + ex.getMessage()); } catch (EJBException ex) { - return error(BAD_REQUEST, "Delete failed for file ID: " + fileIdOrPersistentId + ": " + ex.getMessage()); + return error(BAD_REQUEST, "Delete failed for file ID " + fileIdOrPersistentId + "(EJBException): " + ex.getMessage()); } if (deletePhysicalFile) { @@ -358,8 +358,8 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP + dataFile.getId() + ", storage location: " + fileService.getPhysicalFileToDelete(dataFile)); } } - } catch (WrappedResponse ex) { - return error(BAD_REQUEST, "There was no file found for ID: " + fileIdOrPersistentId); + } catch (WrappedResponse wr) { + return wr.getResponse(); } return ok(deletePhysicalFile); 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 de540c39b6b..dc5db77178e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1899,38 +1899,87 @@ public void testAddFileToDatasetSkipTabIngest() throws IOException, InterruptedE @Test public void testDeleteFile() { msgt("testDeleteFile"); - // Create user + // Create user String apiToken = createUserGetToken(); + // Create user with no permission + String apiTokenNoPerms = createUserGetToken(); + // Create Dataverse String dataverseAlias = createDataverseGetAlias(apiToken); // Create Dataset - Integer datasetId = createDatasetGetId(dataverseAlias, apiToken); + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); - // Upload file - String pathToFile = "src/main/webapp/resources/images/dataverseproject.png"; - JsonObjectBuilder json = Json.createObjectBuilder() - .add("description", "my description") - .add("directoryLabel", "data/subdir1") - .add("categories", Json.createArrayBuilder().add("Data")); - Response uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile, json.build(), apiToken); - uploadResponse.then().assertThat().statusCode(OK.getStatusCode()); - Integer fileId = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id"); + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + String datasetPid = JsonPath.from(createDataset.asString()).getString("data.persistentId"); + + // Upload file 1 + String pathToFile1 = "src/main/webapp/resources/images/dataverseproject.png"; + JsonObjectBuilder json1 = Json.createObjectBuilder() + .add("description", "my description1") + .add("directoryLabel", "data/subdir1") + .add("categories", Json.createArrayBuilder().add("Data")); + Response uploadResponse1 = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile1, json1.build(), apiToken); + uploadResponse1.then().assertThat().statusCode(OK.getStatusCode()); + + Integer fileId1 = JsonPath.from(uploadResponse1.body().asString()).getInt("data.files[0].dataFile.id"); // Check file uploaded - Response downloadResponse = UtilIT.downloadFile(fileId, null, null, null, apiToken); - downloadResponse.then().assertThat().statusCode(OK.getStatusCode()); + Response downloadResponse1 = UtilIT.downloadFile(fileId1, null, null, null, apiToken); + downloadResponse1.then().assertThat().statusCode(OK.getStatusCode()); - //-------------// - // Delete file // - //-------------// + //--------------// + // Delete file 1// + //--------------// + Response deleteResponseFail = UtilIT.deleteFileApi(fileId1, apiTokenNoPerms); + deleteResponseFail.prettyPrint(); + deleteResponseFail.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); + + Response deleteResponse1 = UtilIT.deleteFileApi(fileId1, apiToken); + deleteResponse1.then().assertThat().statusCode(OK.getStatusCode()); + + // Check file 1 deleted for good because it was in a draft + Response downloadResponse1notFound = UtilIT.downloadFile(fileId1, null, null, null, apiToken); + downloadResponse1notFound.then().assertThat().statusCode(NOT_FOUND.getStatusCode()); + + // Upload file 2 + String pathToFile2 = "src/main/webapp/resources/images/cc0.png"; + JsonObjectBuilder json2 = Json.createObjectBuilder() + .add("description", "my description2") + .add("directoryLabel", "data/subdir1") + .add("categories", Json.createArrayBuilder().add("Data")); + Response uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile2, json2.build(), apiToken); + uploadResponse.then().assertThat().statusCode(OK.getStatusCode()); + + Integer fileId2 = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id"); + + // Publish collection and dataset + UtilIT.publishDataverseViaNativeApi(dataverseAlias, apiToken).then().assertThat().statusCode(OK.getStatusCode()); + UtilIT.publishDatasetViaNativeApi(datasetId, "major", apiToken).then().assertThat().statusCode(OK.getStatusCode()); + + Response deleteResponse2 = UtilIT.deleteFileApi(fileId2, apiToken); + deleteResponse2.then().assertThat().statusCode(OK.getStatusCode()); + + // Check file 2 deleted from post v1.0 draft + Response postv1draft = UtilIT.getDatasetVersion(datasetPid, ":draft", apiToken); + postv1draft.prettyPrint(); + postv1draft.then().assertThat() + .body("data.files[0]", equalTo(null)) + .statusCode(OK.getStatusCode()); + + // Check file 2 still in v1.0 + Response v1 = UtilIT.getDatasetVersion(datasetPid, "1.0", apiToken); + v1.prettyPrint(); + v1.then().assertThat() + .body("data.files[0].dataFile.filename", equalTo("cc0.png")) + .statusCode(OK.getStatusCode()); - Response deleteResponse = UtilIT.deleteFileApi(fileId, apiToken); - deleteResponse.then().assertThat().statusCode(OK.getStatusCode()); + // Check file 2 still downloadable (published in in v1.0) + Response downloadResponse2 = UtilIT.downloadFile(fileId2, null, null, null, apiToken); + downloadResponse2.then().assertThat().statusCode(OK.getStatusCode()); - // Check file deleted - Response downloadResponse2 = UtilIT.downloadFile(fileId, null, null, null, apiToken); - downloadResponse2.then().assertThat().statusCode(NOT_FOUND.getStatusCode()); } } From 30061acba40370fad76407040e99bbfa151e3fac Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 15 Mar 2023 12:44:11 -0400 Subject: [PATCH 15/24] document rules around deleting files #3913 --- doc/sphinx-guides/source/api/native-api.rst | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 0375873f840..e3da6e8b3cd 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2447,7 +2447,13 @@ The fully expanded example above (without environment variables) looks like this Deleting Files ~~~~~~~~~~~~~~ -Delete an existing file where ``ID`` is the database id of the file to delete or ``PERSISTENT_ID`` is the persistent id (DOI or Handle) of the file. +Delete an existing file where ``ID`` is the database id of the file to delete or ``PERSISTENT_ID`` is the persistent id (DOI or Handle, if it exists) of the file. + +Note that the behavior of deleting files depends on if the dataset has ever been published or not. + +- If the dataset has never been published, the file will be deleted forever. +- If the dataset has published, the file is deleted from the draft (and future published versions). +- If the dataset has published, the deleted file can still be downloaded because it was part of a published version. A curl example using an ``ID`` From 69120b8fcd06a49310adebed831260bdc9049b0b Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Thu, 23 Mar 2023 17:49:53 +0100 Subject: [PATCH 16/24] extended integration test to include one more draft ds scenario --- .../edu/harvard/iq/dataverse/api/FilesIT.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) 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 dc5db77178e..7cc94841190 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1951,10 +1951,21 @@ public void testDeleteFile() { .add("description", "my description2") .add("directoryLabel", "data/subdir1") .add("categories", Json.createArrayBuilder().add("Data")); - Response uploadResponse = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile2, json2.build(), apiToken); - uploadResponse.then().assertThat().statusCode(OK.getStatusCode()); + Response uploadResponse2 = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile2, json2.build(), apiToken); + uploadResponse2.then().assertThat().statusCode(OK.getStatusCode()); - Integer fileId2 = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id"); + Integer fileId2 = JsonPath.from(uploadResponse2.body().asString()).getInt("data.files[0].dataFile.id"); + + // Upload file 3 + String pathToFile3 = "src/main/webapp/resources/images/cc0.png"; + JsonObjectBuilder json3 = Json.createObjectBuilder() + .add("description", "my description3") + .add("directoryLabel", "data/subdir1") + .add("categories", Json.createArrayBuilder().add("Data")); + Response uploadResponse3 = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile3, json3.build(), apiToken); + uploadResponse3.then().assertThat().statusCode(OK.getStatusCode()); + + Integer fileId3 = JsonPath.from(uploadResponse3.body().asString()).getInt("data.files[0].dataFile.id"); // Publish collection and dataset UtilIT.publishDataverseViaNativeApi(dataverseAlias, apiToken).then().assertThat().statusCode(OK.getStatusCode()); @@ -1981,5 +1992,8 @@ public void testDeleteFile() { Response downloadResponse2 = UtilIT.downloadFile(fileId2, null, null, null, apiToken); downloadResponse2.then().assertThat().statusCode(OK.getStatusCode()); + // Delete file 3, the current version is still draft + Response deleteResponse3 = UtilIT.deleteFileApi(fileId3, apiToken); + deleteResponse3.then().assertThat().statusCode(OK.getStatusCode()); } } From 947a163d2da7cbad5b14616da55087e9621e793d Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Fri, 24 Mar 2023 15:14:20 +0100 Subject: [PATCH 17/24] investigating failing test --- src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 7cc94841190..8ce49057195 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1993,7 +1993,7 @@ public void testDeleteFile() { downloadResponse2.then().assertThat().statusCode(OK.getStatusCode()); // Delete file 3, the current version is still draft - Response deleteResponse3 = UtilIT.deleteFileApi(fileId3, apiToken); - deleteResponse3.then().assertThat().statusCode(OK.getStatusCode()); + //Response deleteResponse3 = UtilIT.deleteFileApi(fileId3, apiToken); + //deleteResponse3.then().assertThat().statusCode(OK.getStatusCode()); } } From 49e2df3290d905d899085816cc5b3b098999c159 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Fri, 24 Mar 2023 15:14:55 +0100 Subject: [PATCH 18/24] added deletion of the third file --- src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 8ce49057195..7cc94841190 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1993,7 +1993,7 @@ public void testDeleteFile() { downloadResponse2.then().assertThat().statusCode(OK.getStatusCode()); // Delete file 3, the current version is still draft - //Response deleteResponse3 = UtilIT.deleteFileApi(fileId3, apiToken); - //deleteResponse3.then().assertThat().statusCode(OK.getStatusCode()); + Response deleteResponse3 = UtilIT.deleteFileApi(fileId3, apiToken); + deleteResponse3.then().assertThat().statusCode(OK.getStatusCode()); } } From 242d62c17ffd0b0f8cdc659973eb2b8399f1e836 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Fri, 24 Mar 2023 15:35:43 +0100 Subject: [PATCH 19/24] fix for trying to delete already deleted metadata in file delete api call --- .../edu/harvard/iq/dataverse/api/Files.java | 5 ++--- .../edu/harvard/iq/dataverse/api/FilesIT.java | 20 +++++++++++++++---- 2 files changed, 18 insertions(+), 7 deletions(-) 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 a81f97bb1b7..182e37a193e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -52,7 +52,6 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -334,12 +333,12 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP boolean deletePhysicalFile = false; try { DataFile dataFile = findDataFileOrDie(fileIdOrPersistentId); - List fileToDelete = dataFile.getFileMetadatas(); + FileMetadata fileToDelete = dataFile.getLatestFileMetadata(); Dataset dataset = dataFile.getOwner(); DatasetVersion v = dataset.getOrCreateEditVersion(); deletePhysicalFile = !dataFile.isReleased(); - UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, fileToDelete, v); + UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, Arrays.asList(fileToDelete), v); update_cmd.setValidateLenient(true); try { 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 7cc94841190..c6efe9457d4 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1931,9 +1931,7 @@ public void testDeleteFile() { Response downloadResponse1 = UtilIT.downloadFile(fileId1, null, null, null, apiToken); downloadResponse1.then().assertThat().statusCode(OK.getStatusCode()); - //--------------// - // Delete file 1// - //--------------// + // Delete file 1 Response deleteResponseFail = UtilIT.deleteFileApi(fileId1, apiTokenNoPerms); deleteResponseFail.prettyPrint(); deleteResponseFail.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); @@ -1957,7 +1955,7 @@ public void testDeleteFile() { Integer fileId2 = JsonPath.from(uploadResponse2.body().asString()).getInt("data.files[0].dataFile.id"); // Upload file 3 - String pathToFile3 = "src/main/webapp/resources/images/cc0.png"; + String pathToFile3 = "src/main/webapp/resources/images/orcid_16x16.png"; JsonObjectBuilder json3 = Json.createObjectBuilder() .add("description", "my description3") .add("directoryLabel", "data/subdir1") @@ -1992,8 +1990,22 @@ public void testDeleteFile() { Response downloadResponse2 = UtilIT.downloadFile(fileId2, null, null, null, apiToken); downloadResponse2.then().assertThat().statusCode(OK.getStatusCode()); + // Check file 3 still in post v1.0 draft + Response postv1draft2 = UtilIT.getDatasetVersion(datasetPid, "1.0", apiToken); + postv1draft2.prettyPrint(); + postv1draft2.then().assertThat() + .body("data.files[0].dataFile.filename", equalTo("orcid_16x16.png")) + .statusCode(OK.getStatusCode()); + // Delete file 3, the current version is still draft Response deleteResponse3 = UtilIT.deleteFileApi(fileId3, apiToken); deleteResponse3.then().assertThat().statusCode(OK.getStatusCode()); + + // Check file 3 deleted from post v1.0 draft + Response postv1draft3 = UtilIT.getDatasetVersion(datasetPid, ":draft", apiToken); + postv1draft3.prettyPrint(); + postv1draft3.then().assertThat() + .body("data.files[0]", equalTo(null)) + .statusCode(OK.getStatusCode()); } } From 08d0fc2b83406d75ceba438730449fbc8a538481 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Tue, 28 Mar 2023 15:05:11 +0200 Subject: [PATCH 20/24] test fix --- src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c6efe9457d4..fd8d48079a7 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1976,7 +1976,7 @@ public void testDeleteFile() { Response postv1draft = UtilIT.getDatasetVersion(datasetPid, ":draft", apiToken); postv1draft.prettyPrint(); postv1draft.then().assertThat() - .body("data.files[0]", equalTo(null)) + .body("data.files.size()", is(1)) .statusCode(OK.getStatusCode()); // Check file 2 still in v1.0 From 91da97a2fe6a598a9caa07b9c6d13d4d79469701 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Tue, 28 Mar 2023 15:06:35 +0200 Subject: [PATCH 21/24] test fix --- src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 fd8d48079a7..8a26c266496 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1976,7 +1976,7 @@ public void testDeleteFile() { Response postv1draft = UtilIT.getDatasetVersion(datasetPid, ":draft", apiToken); postv1draft.prettyPrint(); postv1draft.then().assertThat() - .body("data.files.size()", is(1)) + .body("data.files.size()", equalTo(1)) .statusCode(OK.getStatusCode()); // Check file 2 still in v1.0 From 2d682a4e24d857d3b064ef717107df177d0b6a49 Mon Sep 17 00:00:00 2001 From: Eryk Kulikowski Date: Tue, 28 Mar 2023 17:04:36 +0200 Subject: [PATCH 22/24] test fix --- src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 8a26c266496..ad92f8967ca 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -64,7 +64,7 @@ private String createUserGetToken(){ String username = UtilIT.getUsernameFromResponse(createUser); String apiToken = UtilIT.getApiTokenFromResponse(createUser); - + System.out.println(apiToken); return apiToken; } @@ -1991,7 +1991,7 @@ public void testDeleteFile() { downloadResponse2.then().assertThat().statusCode(OK.getStatusCode()); // Check file 3 still in post v1.0 draft - Response postv1draft2 = UtilIT.getDatasetVersion(datasetPid, "1.0", apiToken); + Response postv1draft2 = UtilIT.getDatasetVersion(datasetPid, ":draft", apiToken); postv1draft2.prettyPrint(); postv1draft2.then().assertThat() .body("data.files[0].dataFile.filename", equalTo("orcid_16x16.png")) From 7fdee06d70fa354d69ef7fb5715b52302521261d Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 28 Mar 2023 12:54:56 -0400 Subject: [PATCH 23/24] add a way to check files based on name #3913 --- .../java/edu/harvard/iq/dataverse/api/FilesIT.java | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 ad92f8967ca..ed4d255ab74 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -8,6 +8,7 @@ import org.junit.Test; import org.junit.BeforeClass; import com.jayway.restassured.path.json.JsonPath; +import static com.jayway.restassured.path.json.JsonPath.with; import com.jayway.restassured.path.xml.XmlPath; import static edu.harvard.iq.dataverse.api.AccessIT.apiToken; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; @@ -23,6 +24,7 @@ import java.text.MessageFormat; import java.util.Arrays; import java.util.Collections; +import java.util.Map; import java.util.ResourceBundle; import javax.json.Json; import javax.json.JsonObjectBuilder; @@ -1985,6 +1987,10 @@ public void testDeleteFile() { v1.then().assertThat() .body("data.files[0].dataFile.filename", equalTo("cc0.png")) .statusCode(OK.getStatusCode()); + + Map v1files1 = with(v1.body().asString()).param("fileToFind", "cc0.png") + .getJsonObject("data.files.find { files -> files.label == fileToFind }"); + assertEquals("cc0.png", v1files1.get("label")); // Check file 2 still downloadable (published in in v1.0) Response downloadResponse2 = UtilIT.downloadFile(fileId2, null, null, null, apiToken); @@ -1996,6 +2002,10 @@ public void testDeleteFile() { postv1draft2.then().assertThat() .body("data.files[0].dataFile.filename", equalTo("orcid_16x16.png")) .statusCode(OK.getStatusCode()); + + Map v1files2 = with(postv1draft2.body().asString()).param("fileToFind", "orcid_16x16.png") + .getJsonObject("data.files.find { files -> files.label == fileToFind }"); + assertEquals("orcid_16x16.png", v1files2.get("label")); // Delete file 3, the current version is still draft Response deleteResponse3 = UtilIT.deleteFileApi(fileId3, apiToken); From 5ec540d48f129f4818adc7e43cd831709244b6db Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 28 Mar 2023 12:55:22 -0400 Subject: [PATCH 24/24] add link to release note #3913 --- doc/release-notes/3913-delete-file-endpoint | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/release-notes/3913-delete-file-endpoint b/doc/release-notes/3913-delete-file-endpoint index 8c748582f2e..b8b0c2f1ec5 100644 --- a/doc/release-notes/3913-delete-file-endpoint +++ b/doc/release-notes/3913-delete-file-endpoint @@ -1 +1 @@ -Support for deleting files using native API. \ No newline at end of file +Support for deleting files using native API: http://preview.guides.gdcc.io/en/develop/api/native-api.html#deleting-files