Add checks in MongoClientInterface

- We should ensure we consider properly the returned flags from
  MongoDB.
- The coverage of MongoClientInterface is increased as well, to
  also test the new logic.

Issue: ARSN-488

Author: williamlardier

lib/storage/metadata/mongoclient/MongoClientInterface.ts

@@ -417,7 +417,11 @@ class MongoClientInterface {
         }, payload, {
             upsert: true,
         })
-            .then(() => {
+            .then(result => {
+                if (result.modifiedCount === 0 && result.upsertedCount === 0) {
+                    log.debug('createBucket: failed to create bucket', { bucketName });
+                    return cb(errors.InternalError);
+                }
                 // caching bucket vFormat
                 this.bucketVFormatCache.add(bucketName, payload.$set.vFormat);
                 // NOTE: We do not need to create a collection for
@@ -583,7 +587,13 @@ class MongoClientInterface {
         }, {
             upsert: true,
         })
-            .then(() => cb(null))
+            .then(result => {
+                if (result.modifiedCount === 0 && result.upsertedCount === 0) {
+                    log.debug('putBucketAttributes: failed to update bucket', { bucketName });
+                    return cb(errors.InternalError);
+                }
+                return cb(null);
+            })
             .catch(err => {
                 log.error(
                     'putBucketAttributes: error putting bucket attributes',
@@ -623,7 +633,13 @@ class MongoClientInterface {
             },
         }, {
             upsert: true,
-        }).then(() => cb(null)).catch(err => {
+        }).then(result => {
+            if (result.modifiedCount === 0 && result.upsertedCount === 0) {
+                log.error('putBucketAttributesCapabilities: failed to update bucket', { bucketName });
+                return cb(errors.InternalError);
+            }
+            return cb(null);
+        }).catch(err => {
             log.error(
                 'putBucketAttributesCapabilities: error putting bucket attributes',
                 { error: err.message });
@@ -657,7 +673,14 @@ class MongoClientInterface {
             $unset: {
                 [updateString]: '',
             },
-        }).then(() => cb(null)).catch(err => {
+        }).then(result => {
+            if (result.modifiedCount === 0) {
+                log.debug('deleteBucketAttributesCapability: bucket not found or capability not present',
+                    { bucketName });
+                return cb(errors.NoSuchBucket);
+            }
+            return cb(null);
+        }).catch(err => {
             if (err) {
                 log.error(
                     'deleteBucketAttributesCapability: error deleting bucket attributes',
@@ -1967,7 +1990,19 @@ class MongoClientInterface {
         if (params && params.doesNotNeedOpogUpdate) {
             // If flag is true, directly delete object
             return collection.deleteOne(deleteFilter)
-                .then(() => cb(null, undefined))
+                .then(result => {
+                    // In case of race conditions (e.g. two concurrent deletes), or invalid state
+                    // (e.g. object is already deleted), the result.deletedCount will be 0
+                    // without error, and we need to catch it because the API might continue and
+                    // delete the actual data, leading to an invalid state, where MongoDB references
+                    // and object not in the data layers anymore.
+                    if (!result || result?.deletedCount != 1) {
+                        log.debug('internalDeleteObject: object not found or already deleted',
+                            { bucket: bucketName, object: key });
+                        return cb(errors.NoSuchKey);
+                    }
+                    return cb(null, undefined);
+                })
                 .catch(err => {
                     log.error('internalDeleteObject: error deleting object',
                         { bucket: bucketName, object: key, error: err.message });
@@ -2030,7 +2065,19 @@ class MongoClientInterface {
                         filter: updateDeleteFilter,
                     },
                 },
-            ], { ordered: true }).then(() => next(null)).catch(err => next(err)),
+            ], { ordered: true }).then(result => {
+                if (!result || !result.ok) {
+                    log.debug('internalDeleteObject: bulk operation failed', 
+                        { bucket: bucketName, object: key });
+                    return next(errors.InternalError);
+                }
+                if (result.deletedCount === 0) {
+                    log.debug('internalDeleteObject: object not deleted', 
+                        { bucket: bucketName, object: key });
+                    return next(errors.NoSuchKey);
+                }
+                return next(null);
+            }).catch(err => next(err)),
         ], (err, res) => {
             if (err) {
                 if (err instanceof ArsenalError && err.is.NoSuchKey) {
@@ -2396,7 +2443,13 @@ class MongoClientInterface {
         i.insertOne(<InfostoreDocument>{
             _id: __UUID,
             value: uuid,
-        }, {}).then(() => cb(null)) // FIXME: shoud we check for result.ok === 1 ?
+        }, {}).then(result => {
+            if (!result || !result.acknowledged) {
+                log.debug('writeUUIDIfNotExists: insertion failed');
+                return cb(errors.InternalError);
+            }
+            return cb(null);
+        })
             .catch(err => {
                 if (err.code === 11000) {
                     // duplicate key error
@@ -3124,7 +3177,13 @@ class MongoClientInterface {
     putBucketIndexes(bucketName: string, indexSpecs, log: werelogs.Logger, cb: ArsenalCallback<void>) {
         const c = this.getCollection<ObjectMetastoreDocument>(bucketName);
         const indexes = MongoUtils.indexFormatObjectToMongoArray(indexSpecs);
-        c.createIndexes(indexes).then(() => cb(null)).catch(err => {
+        c.createIndexes(indexes).then(result => {
+            if (!result) {
+                log.debug('putBucketIndexes: failed creating bucket indexes');
+                return cb(errors.InternalError);
+            }
+            return cb(null);
+        }).catch(err => {
             if (err.codeName === 'NamespaceNotFound') {
                 return cb(errors.NoSuchBucket);
             }
@@ -3147,7 +3206,13 @@ class MongoClientInterface {
     deleteBucketIndexes(bucketName: string, indexSpecs: { name: string }[], log: werelogs.Logger, cb: ArsenalCallback<void>) {
         const c = this.getCollection<ObjectMetastoreDocument>(bucketName);
         async.each(indexSpecs,
-            (spec, next) => c.dropIndex(spec.name).then(() => next()).catch(err => next(err)),
+            (spec, next) => c.dropIndex(spec.name).then(result => {
+                if (!result || !result.ok) {
+                    log.debug('deleteBucketIndexes: failed to drop index', { indexName: spec.name });
+                    return next(new Error('Failed to drop index'));
+                }
+                return next();
+            }).catch(err => next(err)),
             err => {
                 if (err) {
                     if (err instanceof MongoServerError && err.codeName === 'NamespaceNotFound') {

lib/storage/metadata/mongoclient/utils.ts

@@ -222,6 +222,10 @@ function indexFormatMongoArrayToObject(mongoIndexArray: MongoIndex[]): Index[] {
 }
 
 function indexFormatObjectToMongoArray(indexObj: Index[]): MongoIndex[] {
+    if (!indexObj || !Array.isArray(indexObj)) {
+        return [];
+    }
+    
     return indexObj.map(idx => {
         const key = new Map();
         idx.keys.forEach(k => key.set(k.key, k.order));