♻️ Migrate bucketGet and objectGetLegalHold to async/await

Issue: CLDSRV-823

Author: DarkIsDude

lib/api/bucketGet.js

@@ -1,3 +1,4 @@
+const { promisify } = require('util');
 const querystring = require('querystring');
 const { errors, errorInstances, versioning, s3middleware } = require('arsenal');
 const constants = require('../../constants');
@@ -236,8 +237,7 @@ function processMasterVersions(bucketName, listParams, list) {
     return xml.join('');
 }
 
-function handleResult(listParams, requestMaxKeys, encoding, authInfo,
-    bucketName, list, corsHeaders, log, callback) {
+function handleResult(listParams, requestMaxKeys, encoding, authInfo, bucketName, list, log) {
     // eslint-disable-next-line no-param-reassign
     listParams.maxKeys = requestMaxKeys;
     // eslint-disable-next-line no-param-reassign
@@ -250,7 +250,7 @@ function handleResult(listParams, requestMaxKeys, encoding, authInfo,
     }
     pushMetric('listBucket', log, { authInfo, bucket: bucketName });
     monitoring.promMetrics('GET', bucketName, '200', 'listBucket');
-    return callback(null, res, corsHeaders);
+    return res;
 }
 
 /**
@@ -259,11 +259,9 @@ function handleResult(listParams, requestMaxKeys, encoding, authInfo,
  *                               requester's info
  * @param  {object} request - http request object
  * @param  {function} log - Werelogs request logger
- * @param  {function} callback - callback to respond to http request
- *  with either error code or xml response body
- * @return {undefined}
+ * @return {Promise<object>} - object containing xml and additionalResHeaders
  */
-function bucketGet(authInfo, request, log, callback) {
+async function bucketGet(authInfo, request, log) {
     const params = request.query;
     const bucketName = request.bucketName;
     const v2 = params['list-type'];
@@ -277,9 +275,9 @@ function bucketGet(authInfo, request, log, callback) {
     }
 
     if (v2 !== undefined && Number.parseInt(v2, 10) !== 2) {
-        return callback(errorInstances.InvalidArgument.customizeDescription('Invalid ' +
-            'List Type specified in Request'));
+        throw errorInstances.InvalidArgument.customizeDescription('Invalid List Type specified in Request');
     }
+
     if (v2) {
         log.addDefaultFields({ action: 'ListObjectsV2', });
         if (request.serverAccessLog) {
@@ -297,18 +295,14 @@ function bucketGet(authInfo, request, log, callback) {
     const encoding = params['encoding-type'];
     if (encoding !== undefined && encoding !== 'url') {
         monitoring.promMetrics('GET', bucketName, 400, 'listBucket');
-        return callback(errorInstances.InvalidArgument.customizeDescription('Invalid ' +
-            'Encoding Method specified in Request'));
+        throw errorInstances.InvalidArgument.customizeDescription('Invalid Encoding Method specified in Request');
     }
 
     const requestMaxKeys = params['max-keys'] ? Number.parseInt(params['max-keys'], 10) : 1000;
     if (Number.isNaN(requestMaxKeys) || requestMaxKeys < 0) {
         monitoring.promMetrics('GET', bucketName, 400, 'listBucket');
-        return callback(errors.InvalidArgument);
+        throw errors.InvalidArgument;
     }
-    // AWS only returns 1000 keys even if max keys are greater.
-    // Max keys stated in response xml can be greater than actual
-    // keys returned.
     const actualMaxKeys = Math.min(constants.listingHardLimit, requestMaxKeys);
 
     const metadataValParams = {
@@ -336,51 +330,67 @@ function bucketGet(authInfo, request, log, callback) {
         listParams.marker = params.marker;
     }
 
-    standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
-        const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket);
+    let error;
+    let bucket;
 
-        if (err) {
-            log.debug('error processing request', { error: err });
-            monitoring.promMetrics('GET', bucketName, err.code, 'listBucket');
-            return callback(err, null, corsHeaders);
-        }
+    try {
+        const standardMetadataValidateBucketPromised = promisify(standardMetadataValidateBucket);
+        bucket = await standardMetadataValidateBucketPromised(metadataValParams, request.actionImplicitDenies, log);
+    } catch (err) {
+        error = err;
+    }
 
-        if (params.versions !== undefined) {
-            listParams.listingType = 'DelimiterVersions';
-            delete listParams.marker;
-            listParams.keyMarker = params['key-marker'];
-            listParams.versionIdMarker = params['version-id-marker'] ?
-                versionIdUtils.decode(params['version-id-marker']) :
-                undefined;
-        }
+    const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket);
 
-        if (!requestMaxKeys) {
-            const emptyList = {
-                CommonPrefixes: [],
-                Contents: [],
-                Versions: [],
-                IsTruncated: false,
-            };
-            return handleResult(listParams, requestMaxKeys, encoding, authInfo,
-                bucketName, emptyList, corsHeaders, log, callback);
-        }
+    if (error) {
+        log.debug('error processing request', { error });
+        monitoring.promMetrics('GET', bucketName, error.code, 'listBucket');
+        error.additionalResHeaders = corsHeaders;
+        throw error;
+    }
+    if (params.versions !== undefined) {
+        listParams.listingType = 'DelimiterVersions';
+        delete listParams.marker;
+        listParams.keyMarker = params['key-marker'];
+        listParams.versionIdMarker = params['version-id-marker'] ?
+            versionIdUtils.decode(params['version-id-marker']) :
+            undefined;
+    }
+    if (!requestMaxKeys) {
+        const emptyList = {
+            CommonPrefixes: [],
+            Contents: [],
+            Versions: [],
+            IsTruncated: false,
+        };
+        const res = handleResult(listParams, requestMaxKeys, encoding, authInfo, bucketName, emptyList, log);
+        return [res, corsHeaders];
+    }
 
-        return services.getObjectListing(bucketName, listParams, log, (err, list) => {
-            if (err) {
-                log.debug('error processing request', { error: err });
-                monitoring.promMetrics('GET', bucketName, err.code, 'listBucket');
-                return callback(err, null, corsHeaders);
-            }
+    let list;
+    try {
+        list = await promisify(services.getObjectListing)(bucketName, listParams, log);
+    } catch (err) {
+        log.debug('error processing request', { error: err });
+        monitoring.promMetrics('GET', bucketName, err.code, 'listBucket');
 
-            return handleResult(listParams, requestMaxKeys, encoding, authInfo,
-                bucketName, list, corsHeaders, log, callback);
-        });
-    });
-    return undefined;
+        err.additionalResHeaders = corsHeaders;
+        throw err;
+    }
+
+    const res = handleResult(listParams, requestMaxKeys, encoding, authInfo, bucketName, list, log);
+    return [res, corsHeaders];
 }
 
 module.exports = {
     processVersions,
     processMasterVersions,
-    bucketGet,
+    bucketGet: (...args) => {
+        const callback = args.at(-1);
+        const argsWithoutCallback = args.slice(0, -1);
+
+        bucketGet(...argsWithoutCallback)
+            .then(result => callback(null, ...result))
+            .catch(err => callback(err, null, err.additionalResHeaders));
+    },
 };

lib/api/objectGetLegalHold.js

@@ -1,4 +1,3 @@
-const async = require('async');
 const { errors, errorInstances, s3middleware } = require('arsenal');
 
 const { decodeVersionId, getVersionIdResHeader }
@@ -10,23 +9,30 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders');
 
 const { convertToXml } = s3middleware.objectLegalHold;
 
+const standardMetadataValidateBucketAndObjPromised = (metadataValParams, actionImplicitDenies, log) =>
+    new Promise((resolve, reject) => {
+        standardMetadataValidateBucketAndObj(metadataValParams, actionImplicitDenies, log, (err, bucket, objectMD) => {
+            if (err) {return reject(err);}
+            return resolve({ bucket, objectMD });
+        });
+    });
+
 /**
  * Returns legal hold status of object
  * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info
  * @param {object} request - http request object
  * @param {object} log - Werelogs logger
- * @param {function} callback - callback to server
- * @return {undefined}
+ * @return {Promise<object>} - object containing xml and additionalResHeaders
  */
-function objectGetLegalHold(authInfo, request, log, callback) {
+async function objectGetLegalHold(authInfo, request, log) {
     log.debug('processing request', { method: 'objectGetLegalHold' });
 
     const { bucketName, objectKey, query } = request;
 
     const decodedVidResult = decodeVersionId(query);
     if (decodedVidResult instanceof Error) {
         log.trace('invalid versionId query', { versionId: query.versionId, error: decodedVidResult });
-        return process.nextTick(() => callback(decodedVidResult));
+        throw decodedVidResult;
     }
     const versionId = decodedVidResult;
 
@@ -40,60 +46,68 @@ function objectGetLegalHold(authInfo, request, log, callback) {
         request,
     };
 
-    return async.waterfall([
-        next => standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log,
-            (err, bucket, objectMD) => {
-                if (err) {
-                    log.trace('request authorization failed', { method: 'objectGetLegalHold', error: err });
-                    return next(err);
-                }
-                if (!objectMD) {
-                    const err = versionId ? errors.NoSuchVersion : errors.NoSuchKey;
-                    log.trace('error no object metadata found', { method: 'objectGetLegalHold', error: err });
-                    return next(err, bucket);
-                }
-                if (objectMD.isDeleteMarker) {
-                    if (versionId) {
-                        log.trace('requested version is delete marker', { method: 'objectGetLegalHold' });
-                        // FIXME we should return a `x-amz-delete-marker: true` header, see S3C-7592
-                        return next(errors.MethodNotAllowed);
-                    }
-                    log.trace('most recent version is delete marker', { method: 'objectGetLegalHold' });
-                    // FIXME we should return a `x-amz-delete-marker: true` header, see S3C-7592
-                    return next(errors.NoSuchKey);
-                }
-                if (!bucket.isObjectLockEnabled()) {
-                    log.trace('object lock not enabled on bucket', { method: 'objectGetRetention' });
-                    return next(errorInstances.InvalidRequest.customizeDescription(
-                        'Bucket is missing Object Lock Configuration'));
-                }
-                return next(null, bucket, objectMD);
-            }),
-        (bucket, objectMD, next) => {
-            const { legalHold } = objectMD;
-            const xml = convertToXml(legalHold);
-            if (xml === '') {
-                return next(errors.NoSuchObjectLockConfiguration);
-            }
-            return next(null, bucket, xml, objectMD);
-        },
-    ], (err, bucket, xml, objectMD) => {
-        const additionalResHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket);
-        if (err) {
-            log.trace('error processing request', { error: err, method: 'objectGetLegalHold' });
-        } else {
-            pushMetric('getObjectLegalHold', log, {
-                authInfo,
-                bucket: bucketName,
-                keys: [objectKey],
-                versionId: objectMD ? objectMD.versionId : undefined,
-                location: objectMD ? objectMD.dataStoreName : undefined,
-            });
-            const verCfg = bucket.getVersioningConfiguration();
-            additionalResHeaders['x-amz-version-id'] = getVersionIdResHeader(verCfg, objectMD);
+    let bucket, objectMD;
+
+    try {
+        ({ bucket, objectMD } = await standardMetadataValidateBucketAndObjPromised(
+            metadataValParams,
+            request.actionImplicitDenies,
+            log,
+        ));
+    } catch (err) {
+        log.trace('request authorization failed', { method: 'objectGetLegalHold', error: err });
+        throw err;
+    }
+
+    if (!objectMD) {
+        const err = versionId ? errors.NoSuchVersion : errors.NoSuchKey;
+        log.trace('error no object metadata found', { method: 'objectGetLegalHold', error: err });
+        throw err;
+    }
+
+    if (objectMD.isDeleteMarker) {
+        if (versionId) {
+            log.trace('requested version is delete marker', { method: 'objectGetLegalHold' });
+            // FIXME we should return a `x-amz-delete-marker: true` header, see S3C-7592
+            throw errors.MethodNotAllowed;
         }
-        return callback(err, xml, additionalResHeaders);
+
+        log.trace('most recent version is delete marker', { method: 'objectGetLegalHold' });
+        // FIXME we should return a `x-amz-delete-marker: true` header, see S3C-7592
+        throw errors.NoSuchKey;
+    }
+
+    if (!bucket.isObjectLockEnabled()) {
+        log.trace('object lock not enabled on bucket', { method: 'objectGetRetention' });
+        throw errorInstances.InvalidRequest.customizeDescription('Bucket is missing Object Lock Configuration');
+    }
+
+    const { legalHold } = objectMD;
+    const xml = convertToXml(legalHold);
+    if (xml === '') {
+        throw errors.NoSuchObjectLockConfiguration;
+    }
+
+    const additionalResHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket);
+
+    pushMetric('getObjectLegalHold', log, {
+        authInfo,
+        bucket: bucketName,
+        keys: [objectKey],
+        versionId: objectMD ? objectMD.versionId : undefined,
+        location: objectMD ? objectMD.dataStoreName : undefined,
     });
+    const verCfg = bucket.getVersioningConfiguration();
+    additionalResHeaders['x-amz-version-id'] = getVersionIdResHeader(verCfg, objectMD);
+
+    return [xml, additionalResHeaders];
 }
 
-module.exports = objectGetLegalHold;
+module.exports = (...args) => {
+    const callback = args.at(-1);
+    const argsWithoutCallback = args.slice(0, -1);
+
+    objectGetLegalHold(...argsWithoutCallback)
+        .then(result => callback(null, ...result))
+        .catch(err => callback(err, null, err.additionalResHeaders));
+};