Inconsistent behaviour of dataset delete API

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. 

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.
 - The datafile/{fileId}/metadata/ddi (see :ref:`data-variable-metadata-access`) now returns 403/Forbidden rather than 400/Bad Request when the caller can't access the file due to permissions, embargo, or retention period (consistent with other file access APIs)
 
   - **/api/admin/index/perms**

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

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

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

@@ -338,26 +338,21 @@
         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<Long, String> deleteStorageLocations = fileService.getPhysicalFilesToDelete(doomedVersion, destroy);
+            // Gather the locations of the physical files that will need to be deleted
+            Map<Long, String> 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
@@ -1997,7 +1992,7 @@
 
         // dateAvailable is within limits
         if (minRetentionDateTime != null){
             if (dateUnavailable.isBefore(minRetentionDateTime.minusDays(1))){
                 return error(Status.BAD_REQUEST, "Date unavailable can not be earlier than MinRetentionDurationInMonths: "+minRetentionDurationInMonths + " from now");
             }
         }

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

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 {

src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java

@@ -259,6 +259,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();
@@ -338,19 +342,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());
 
     }
 
@@ -3782,7 +3798,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());