From eefb76f9865ac4403367189159a972bb27dffb01 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Wed, 15 Apr 2026 15:00:21 -0400 Subject: [PATCH 1/4] Inconsistent behaviour of dataset delete API --- .../11253-inconsistent-dataset-delete-api.md | 2 ++ .../iq/dataverse/PermissionsWrapper.java | 2 +- .../harvard/iq/dataverse/api/Datasets.java | 33 +++++-------------- .../api/datadeposit/ContainerManagerImpl.java | 9 +---- .../command/impl/DeleteDatasetCommand.java | 33 ------------------- .../command/impl/DestroyDatasetCommand.java | 4 +-- .../harvard/iq/dataverse/api/DatasetsIT.java | 26 ++++++++++++--- 7 files changed, 36 insertions(+), 73 deletions(-) create mode 100644 doc/release-notes/11253-inconsistent-dataset-delete-api.md delete mode 100644 src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetCommand.java diff --git a/doc/release-notes/11253-inconsistent-dataset-delete-api.md b/doc/release-notes/11253-inconsistent-dataset-delete-api.md new file mode 100644 index 00000000000..387edca329f --- /dev/null +++ b/doc/release-notes/11253-inconsistent-dataset-delete-api.md @@ -0,0 +1,2 @@ +## Bug Fix +When calling DELETE "/api/datasets/{id}", of a released dataset, as a superuser, the call will no longer be 'upgraded' to a dataset destroy action. The user will receive an 'unauthorized' response with the message to call the API with /destroy in order to delete the dataset. diff --git a/src/main/java/edu/harvard/iq/dataverse/PermissionsWrapper.java b/src/main/java/edu/harvard/iq/dataverse/PermissionsWrapper.java index 0af3816035a..1e418817550 100644 --- a/src/main/java/edu/harvard/iq/dataverse/PermissionsWrapper.java +++ b/src/main/java/edu/harvard/iq/dataverse/PermissionsWrapper.java @@ -255,7 +255,7 @@ public boolean canIssueUpdateDatasetCommand(DvObject dvo){ // DELETE DATASET public boolean canIssueDeleteDatasetCommand(DvObject dvo){ - return canIssueCommand(dvo, DeleteDatasetCommand.class); + return canIssueCommand(dvo, DeleteDatasetVersionCommand.class); } // PUBLISH DATASET 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 a00130de31a..d13d7069911 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -311,40 +311,25 @@ public Response exportDataset(@Context ContainerRequestContext crc, @QueryParam( @AuthRequired @Path("{id}") public Response deleteDataset(@Context ContainerRequestContext crc, @PathParam("id") String id) { - // Internally, "DeleteDatasetCommand" simply redirects to "DeleteDatasetVersionCommand" - // (and there's a comment that says "TODO: remove this command") - // do we need an exposed API call for it? - // And DeleteDatasetVersionCommand further redirects to DestroyDatasetCommand, - // if the dataset only has 1 version... In other words, the functionality - // currently provided by this API is covered between the "deleteDraftVersion" and - // "destroyDataset" API calls. - // (The logic below follows the current implementation of the underlying - // commands!) - User u = getRequestUser(crc); return response( req -> { Dataset doomed = findDatasetOrDie(id); DatasetVersion doomedVersion = doomed.getLatestVersion(); - boolean destroy = false; - if (doomed.getVersions().size() == 1) { - if (doomed.isReleased() && (!(u instanceof AuthenticatedUser) || !u.isSuperuser())) { - throw new WrappedResponse(error(Response.Status.UNAUTHORIZED, "Only superusers can delete published datasets")); - } - destroy = true; - } else { - if (!doomedVersion.isDraft()) { - throw new WrappedResponse(error(Response.Status.UNAUTHORIZED, "This is a published dataset with multiple versions. This API can only delete the latest version if it is a DRAFT")); + if (!doomedVersion.isDraft()) { + String msg = "This API can only delete the latest version if it is a DRAFT."; + if (u.isSuperuser()) { + msg += " Please use '/destroy' to delete the published version"; } + throw new WrappedResponse(error(Response.Status.UNAUTHORIZED, msg)); } - // Gather the locations of the physical files that will need to be - // deleted once the destroy command execution has been finalized: - Map deleteStorageLocations = fileService.getPhysicalFilesToDelete(doomedVersion, destroy); + // Gather the locations of the physical files that will need to be deleted + Map deleteStorageLocations = fileService.getPhysicalFilesToDelete(doomedVersion); - execCommand( new DeleteDatasetCommand(req, findDatasetOrDie(id))); + execCommand(new DeleteDatasetVersionCommand(req, doomed)); - // If we have gotten this far, the destroy command has succeeded, + // If we have gotten this far, the delete command has succeeded, // so we can finalize it by permanently deleting the physical files: // (DataFileService will double-check that the datafiles no // longer exist in the database, before attempting to delete diff --git a/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/ContainerManagerImpl.java b/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/ContainerManagerImpl.java index 4d4d1d08b51..4186db72678 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/ContainerManagerImpl.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/ContainerManagerImpl.java @@ -12,7 +12,6 @@ import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.exception.CommandExecutionException; -import edu.harvard.iq.dataverse.engine.command.impl.DeleteDatasetCommand; import edu.harvard.iq.dataverse.engine.command.impl.DeleteDatasetVersionCommand; import edu.harvard.iq.dataverse.engine.command.impl.PublishDatasetCommand; import edu.harvard.iq.dataverse.engine.command.impl.PublishDataverseCommand; @@ -214,12 +213,6 @@ public void deleteContainer(String uri, AuthCredentials authCredentials, SwordCo Dataset dataset = dataset = datasetService.findByGlobalId(globalId); if (dataset != null) { Dataverse dvThatOwnsDataset = dataset.getOwner(); - /** - * We are checking if DeleteDatasetVersionCommand can be - * called even though DeleteDatasetCommand can be called - * when a dataset hasn't been published. They should be - * equivalent in terms of a permission check. - */ DeleteDatasetVersionCommand deleteDatasetVersionCommand = new DeleteDatasetVersionCommand(dvRequest, dataset); if (!permissionService.isUserAllowedOn(user, deleteDatasetVersionCommand, dataset)) { throw new SwordError(UriRegistry.ERROR_BAD_REQUEST, "User " + user.getDisplayInfo().getTitle() + " is not authorized to modify " + dvThatOwnsDataset.getAlias()); @@ -253,7 +246,7 @@ public void deleteContainer(String uri, AuthCredentials authCredentials, SwordCo // dataset has never been published, this is just a sanity check (should always be draft) if (datasetVersionState.equals(DatasetVersion.VersionState.DRAFT)) { try { - engineSvc.submit(new DeleteDatasetCommand(dvRequest, dataset)); + engineSvc.submit(new DeleteDatasetVersionCommand(dvRequest, dataset)); logger.fine("dataset deleted"); } catch (CommandExecutionException ex) { // internal error diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetCommand.java deleted file mode 100644 index 419bf5522e9..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetCommand.java +++ /dev/null @@ -1,33 +0,0 @@ -package edu.harvard.iq.dataverse.engine.command.impl; - -import edu.harvard.iq.dataverse.Dataset; -import edu.harvard.iq.dataverse.authorization.Permission; -import edu.harvard.iq.dataverse.engine.command.AbstractVoidCommand; -import edu.harvard.iq.dataverse.engine.command.CommandContext; -import edu.harvard.iq.dataverse.engine.command.DataverseRequest; -import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; -import edu.harvard.iq.dataverse.engine.command.exception.CommandException; - -/** - * Deletes a dataset. - * - * @author michael - */ -@RequiredPermissions(Permission.DeleteDatasetDraft) -public class DeleteDatasetCommand extends AbstractVoidCommand { - - private final Dataset doomed; - - public DeleteDatasetCommand(DataverseRequest aRequest, Dataset dataset) { - super(aRequest, dataset); - this.doomed = dataset; - } - - @Override - protected void executeImpl(CommandContext ctxt) throws CommandException { - // TODO: REMOVE THIS COMMAND - // for now it just calls DeleteDatasetVersion - ctxt.engine().submit(new DeleteDatasetVersionCommand(getRequest(), doomed)); - } - -} diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DestroyDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DestroyDatasetCommand.java index 49861e084b6..e1f5afcb813 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DestroyDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DestroyDatasetCommand.java @@ -41,12 +41,12 @@ import org.apache.solr.client.solrj.SolrServerException; /** - * Same as {@link DeleteDatasetCommand}, but does not stop if the dataset is + * Same as {@link DeleteDatasetVersionCommand}, but does not stop if the dataset is * published. This command is reserved for super-users, if at all. * * @author michael */ -// Since this is used by DeleteDatasetCommand, must have at least that permission +// Since this is used by DeleteDatasetVersionCommand, must have at least that permission // (for released, user is checked for superuser) @RequiredPermissions( Permission.DeleteDatasetDraft ) public class DestroyDatasetCommand extends AbstractVoidCommand { diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java index 583a76f07a7..7859c68806c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -260,6 +260,10 @@ private void testDatasetSchemaValidationHelper(String dataverseAlias, String api @Test public void testCreateDataset() { + Response createSuperUser = UtilIT.createRandomUser(); + String superusername = UtilIT.getUsernameFromResponse(createSuperUser); + UtilIT.setSuperuserStatus(superusername, true); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperUser); Response createUser = UtilIT.createRandomUser(); createUser.prettyPrint(); @@ -339,19 +343,31 @@ public void testCreateDataset() { datasetAsJson.then().assertThat() .statusCode(OK.getStatusCode()); - // OK, let's delete this dataset as well, and then delete the dataverse... - - deleteDatasetResponse = UtilIT.deleteDatasetViaNativeApi(datasetId, apiToken); + // Now publish the dataset and try to delete it (as superuser) + Response publishResponse = UtilIT.publishDatasetViaNativeApi(datasetId, "major", apiToken); + assertEquals(200, publishResponse.getStatusCode()); + deleteDatasetResponse = UtilIT.deleteDatasetViaNativeApi(datasetId, superuserApiToken); + deleteDatasetResponse.prettyPrint(); + deleteDatasetResponse.then().assertThat() + .body("message", containsString("/destroy")) + .statusCode(UNAUTHORIZED.getStatusCode()); + // Try /destroy to get rid of the dataset + deleteDatasetResponse = UtilIT.destroyDataset(datasetId, superuserApiToken); deleteDatasetResponse.prettyPrint(); assertEquals(200, deleteDatasetResponse.getStatusCode()); - + + // Delete the dataverse Response deleteDataverseResponse = UtilIT.deleteDataverse(dataverseAlias, apiToken); deleteDataverseResponse.prettyPrint(); assertEquals(200, deleteDataverseResponse.getStatusCode()); + // Delete the Users Response deleteUserResponse = UtilIT.deleteUser(username); deleteUserResponse.prettyPrint(); assertEquals(200, deleteUserResponse.getStatusCode()); + deleteUserResponse = UtilIT.deleteUser(superusername); + deleteUserResponse.prettyPrint(); + assertEquals(200, deleteUserResponse.getStatusCode()); } @@ -3785,7 +3801,7 @@ public void testSemanticMetadataAPIs() { deleteDraftResponse.then().assertThat().statusCode(OK.getStatusCode()); //Delete the published dataset - Response deletePublishedResponse = UtilIT.deleteDatasetViaNativeApi(datasetId, apiToken); + Response deletePublishedResponse = UtilIT.destroyDataset(datasetId, apiToken); deletePublishedResponse.prettyPrint(); deleteDraftResponse.then().assertThat().statusCode(OK.getStatusCode()); From 184440c3b368e4ab59f603ff12cff9aa882244ff Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Mon, 13 Jul 2026 14:08:23 -0400 Subject: [PATCH 2/4] fix conflict merge --- src/main/java/edu/harvard/iq/dataverse/api/Datasets.java | 1 - 1 file changed, 1 deletion(-) 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 cd3b5941558..0764ad1a019 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -321,7 +321,6 @@ public Response exportDataset(@Context ContainerRequestContext crc, @Parameter(d @DELETE @AuthRequired @Path("{id}") - public Response deleteDataset(@Context ContainerRequestContext crc, @PathParam("id") String id) { @Operation(summary = "Deletes a dataset", description = "Deletes the latest draft version or destroys an unpublished single-version dataset after checking delete permissions.") public Response deleteDataset(@Context ContainerRequestContext crc, @Parameter(description = "Resource id or persistent identifier.") @PathParam("id") String id) { From 991981296a49994791eeb56825d593aa0a3abc85 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:33:03 -0400 Subject: [PATCH 3/4] add to changelog --- doc/sphinx-guides/source/api/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/sphinx-guides/source/api/changelog.rst b/doc/sphinx-guides/source/api/changelog.rst index ca36d972326..612f8fba546 100644 --- a/doc/sphinx-guides/source/api/changelog.rst +++ b/doc/sphinx-guides/source/api/changelog.rst @@ -20,6 +20,7 @@ v6.12 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 permission reindexing endpoints have been updated to use ``POST`` and require superuser access. They are now documented in the :doc:`/admin/solr-search-index` guide. +- Change to DELETE "/api/datasets/$ID" for a superuser. This api will no longer fall back to a Dataset destroy for published versions. Instead the message "Please use '/destroy' to delete the published version" will be returned to the superuser. - **/api/admin/index/perms** From 180f7aaf293e84a803d38f26ca0ea96aafde06b2 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:45:14 -0400 Subject: [PATCH 4/4] doc change --- doc/sphinx-guides/source/api/native-api.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 582891e0861..8a595c48b22 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -4057,6 +4057,8 @@ Delete Unpublished Dataset Delete the dataset whose id is passed: +.. note:: This api will only delete the DRAFT version of the dataset (along with metadata changes and deletes any files added in the draft) which results in deletion of the dataset if there are no published versions. + .. code-block:: bash export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx