Add recursive option to bucket delete endpoint

Author: TimCsaky

app/src/controllers/bucket.js

@@ -3,6 +3,7 @@ const { UniqueViolationError } = require('objection');
 const { NIL: SYSTEM_USER } = require('uuid');
 
 const { DEFAULTREGION, Permissions } = require('../components/constants');
+const utils = require('../db/models/utils');
 const errorToProblem = require('../components/errorToProblem');
 const log = require('../components/log')(module.filename);
 const {
@@ -215,7 +216,20 @@ const controller = {
   async deleteBucket(req, res, next) {
     try {
       const bucketId = addDashesToUuid(req.params.bucketId);
-      await bucketService.delete(bucketId);
+      const parentBucket = await bucketService.read(bucketId);
+      let buckets = [parentBucket];
+      // if doing recursive delete
+      if (isTruthy(req.query.recursive)) {
+        // get sub-folders
+        const dbChildBuckets = await bucketService.searchChildBuckets(parentBucket);
+        buckets = buckets.concat(dbChildBuckets);
+      }
+      // do delete
+      await utils.trxWrapper(async (trx) => {
+        return await Promise.all(
+          buckets.map(bucket => bucketService.delete(bucket.bucketId, trx))
+        );
+      });
       res.status(204).end();
     } catch (e) {
       next(errorToProblem(SERVICE, e));

app/src/docs/v1.api-spec.yaml

@@ -198,16 +198,19 @@ paths:
     delete:
       summary: Deletes a bucket
       description: >-
-        Deletes the bucket record based on bucketId. This request does not
-        dispatch an S3 operation to request the deletion of the associated
-        bucket.
+        Deletes the bucket record (and child objects) from the COMS database, based on bucketId.
+        Providing the 'recursive' parameter will also delete all sub-folders.
+        This request does not dispatch an S3 operation to request the deletion of the associated
+        bucket(s) or delete any objects in object storage, it simply 'de-registers' knowledge of the bucket/folder
+        and objects from the COMS system.
       operationId: deleteBucket
       tags:
         - Bucket
       parameters:
         - $ref: "#/components/parameters/Header-S3AccessMode-Bucket"
         - $ref: "#/components/parameters/Header-S3AccessMode-Endpoint"
         - $ref: "#/components/parameters/Path-BucketId"
+        - $ref: "#/components/parameters/Query-Recursive"
       responses:
         "204":
           description: Returns no content success
@@ -285,11 +288,14 @@ paths:
       description: >-
         Synchronize the contents of an existing bucket with COMS so that it can
         track and manage the objects in it. This avoids the need to re-upload
-        preexisting files through the COMS API. This endpoint does not guarantee
+        pre-existing files through the COMS API. This endpoint does not guarantee
         immediately synchronized results. Synchronization latency will be
         affected by the remaining number of objects awaiting synchronization.
         This endpoint updates the last known successful synchronization request
         date when invoked.
+        The `recursive` option will optionally sync sub-folders.
+        Note: The current user (if using OIDC auth) will get the permissions they
+        already have on the parent applied to any NEW sub-folders created in COMS
       tags:
         - Sync
       operationId: syncBucket
@@ -2051,7 +2057,7 @@ components:
       in: query
       name: recursive
       description: >-
-        Whether or not to also sync sub-folders inside the provided bucket/folder
+        Whether or not to also run this operation for sub-folders inside the provided bucket/folder
       schema:
         type: boolean
         default: false

app/src/validators/bucket.js

@@ -40,6 +40,7 @@ const schema = {
       bucketId: type.uuidv4
     }),
     query: Joi.object({
+      recursive: type.truthy,
     })
   },
 

app/tests/unit/controllers/bucket.spec.js

@@ -3,6 +3,7 @@ const { UniqueViolationError } = require('objection');
 const { NIL: SYSTEM_USER } = require('uuid');
 
 const controller = require('../../../src/controllers/bucket');
+const moduleUtils = require('../../../src/db/models/utils');
 const {
   bucketService,
   storageService,
@@ -453,6 +454,8 @@ describe('createBucketChild', () => {
 describe('deleteBucket', () => {
   // mock service calls
   const addDashesToUuidSpy = jest.spyOn(utils, 'addDashesToUuid');
+  const readSpy = jest.spyOn(bucketService, 'read');
+  const trxWrapperSpy = jest.spyOn(moduleUtils, 'trxWrapper');
   const deleteSpy = jest.spyOn(bucketService, 'delete');
 
   const next = jest.fn();
@@ -466,13 +469,16 @@ describe('deleteBucket', () => {
     };
 
     addDashesToUuidSpy.mockReturnValue(REQUEST_BUCKET_ID);
+    readSpy.mockReturnValue({ bucketId: REQUEST_BUCKET_ID });
+    trxWrapperSpy.mockImplementation(callback => callback({}));
     deleteSpy.mockReturnValue(true);
 
     await controller.deleteBucket(req, res, next);
 
     expect(addDashesToUuidSpy).toHaveBeenCalledTimes(1);
     expect(addDashesToUuidSpy).toHaveBeenCalledWith(REQUEST_BUCKET_ID);
-    expect(deleteSpy).toHaveBeenCalledWith(REQUEST_BUCKET_ID);
+    expect(trxWrapperSpy).toHaveBeenCalledTimes(1);
+    expect(deleteSpy).toHaveBeenCalledWith(REQUEST_BUCKET_ID, {});
 
     expect(res.status).toHaveBeenCalledWith(204);
   });
@@ -486,6 +492,8 @@ describe('deleteBucket', () => {
     };
 
     addDashesToUuidSpy.mockReturnValue(REQUEST_BUCKET_ID);
+    readSpy.mockReturnValue({ bucketId: REQUEST_BUCKET_ID });
+    trxWrapperSpy.mockImplementation(callback => callback({}));
     deleteSpy.mockImplementationOnce(() => {
       throw new Problem(502, 'Unknown BucketService Error');
     });
@@ -494,7 +502,8 @@ describe('deleteBucket', () => {
 
     expect(addDashesToUuidSpy).toHaveBeenCalledTimes(1);
     expect(addDashesToUuidSpy).toHaveBeenCalledWith(REQUEST_BUCKET_ID);
-    expect(deleteSpy).toHaveBeenCalledWith(REQUEST_BUCKET_ID);
+    expect(trxWrapperSpy).toHaveBeenCalledTimes(1);
+    expect(deleteSpy).toHaveBeenCalledWith(REQUEST_BUCKET_ID, {});
 
     expect(next).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledWith(