Correctly cleanup files from object storage

4 files changed, 42 insertions, 19 deletions

diff --git a/server/lib/object-storage/shared/object-storage-helpers.ts b/server/lib/object-storage/shared/object-storage-helpers.ts
index 05b52f412..d13c25798 100644
--- a/server/lib/object-storage/shared/object-storage-helpers.ts
+++ b/server/lib/object-storage/shared/object-storage-helpers.ts
@@ -1,3 +1,4 @@
+import { map } from 'bluebird'
 import { createReadStream, createWriteStream, ensureDir, ReadStream } from 'fs-extra'
 import { dirname } from 'path'
 import { Readable } from 'stream'
@@ -93,6 +94,8 @@ function updatePrefixACL (options: {
     prefix,
     bucketInfo,
     commandBuilder: obj => {
+      logger.debug('Updating ACL of %s inside prefix %s in bucket %s', obj.Key, prefix, bucketInfo.BUCKET_NAME, lTags())
+
       return new PutObjectAclCommand({
         Bucket: bucketInfo.BUCKET_NAME,
         Key: obj.Key,
@@ -117,7 +120,7 @@ function removeObject (objectStorageKey: string, bucketInfo: BucketInfo) {
   return getClient().send(command)
 }
 
-function removePrefix (prefix: string, bucketInfo: BucketInfo) {
+async function removePrefix (prefix: string, bucketInfo: BucketInfo) {
   // FIXME: use bulk delete when s3ninja will support this operation
 
   logger.debug('Removing prefix %s in bucket %s', prefix, bucketInfo.BUCKET_NAME, lTags())
@@ -126,6 +129,8 @@ function removePrefix (prefix: string, bucketInfo: BucketInfo) {
     prefix,
     bucketInfo,
     commandBuilder: obj => {
+      logger.debug('Removing %s inside prefix %s in bucket %s', obj.Key, prefix, bucketInfo.BUCKET_NAME, lTags())
+
       return new DeleteObjectCommand({
         Bucket: bucketInfo.BUCKET_NAME,
         Key: obj.Key
@@ -259,7 +264,7 @@ async function applyOnPrefix (options: {
 
   const s3Client = getClient()
 
-  const commandPrefix = bucketInfo.PREFIX + prefix
+  const commandPrefix = buildKey(prefix, bucketInfo)
   const listCommand = new ListObjectsV2Command({
     Bucket: bucketInfo.BUCKET_NAME,
     Prefix: commandPrefix,
@@ -275,11 +280,11 @@ async function applyOnPrefix (options: {
     throw new Error(message)
   }
 
-  for (const object of listedObjects.Contents) {
+  await map(listedObjects.Contents, object => {
     const command = commandBuilder(object)
 
-    await s3Client.send(command)
-  }
+    return s3Client.send(command)
+  }, { concurrency: 10 })
 
   // Repeat if not all objects could be listed at once (limit of 1000?)
   if (listedObjects.IsTruncated) {
diff --git a/server/models/video/video.ts b/server/models/video/video.ts
index c568075d8..9399712b8 100644
--- a/server/models/video/video.ts
+++ b/server/models/video/video.ts
@@ -785,9 +785,8 @@ export class VideoModel extends Model<Partial<AttributesOnly<VideoModel>>> {
 
     // Do not wait video deletion because we could be in a transaction
     Promise.all(tasks)
-           .catch(err => {
-             logger.error('Some errors when removing files of video %s in before destroy hook.', instance.uuid, { err })
-           })
+      .then(() => logger.info('Removed files of video %s.', instance.url))
+      .catch(err => logger.error('Some errors when removing files of video %s in before destroy hook.', instance.uuid, { err }))
 
     return undefined
   }
diff --git a/server/tests/api/object-storage/video-static-file-privacy.ts b/server/tests/api/object-storage/video-static-file-privacy.ts
index 981bbaa16..c6d7a1a2c 100644
--- a/server/tests/api/object-storage/video-static-file-privacy.ts
+++ b/server/tests/api/object-storage/video-static-file-privacy.ts
@@ -192,16 +192,6 @@ describe('Object storage for video static file privacy', function () {
 
       await checkPublicFiles(publicVideoUUID)
     })
-
-    after(async function () {
-      this.timeout(30000)
-
-      if (privateVideoUUID) await server.videos.remove({ id: privateVideoUUID })
-      if (publicVideoUUID) await server.videos.remove({ id: publicVideoUUID })
-      if (userPrivateVideoUUID) await server.videos.remove({ id: userPrivateVideoUUID })
-
-      await waitJobs([ server ])
-    })
   })
 
   describe('Live', function () {
@@ -331,6 +321,18 @@ describe('Object storage for video static file privacy', function () {
   })
 
   after(async function () {
+    this.timeout(60000)
+
+    const { data } = await server.videos.listAllForAdmin()
+
+    for (const v of data) {
+      await server.videos.remove({ id: v.uuid })
+    }
+
+    for (const v of data) {
+      await server.servers.waitUntilLog('Removed files of video ' + v.url, 1, true)
+    }
+
     await cleanupTests([ server ])
   })
 })
diff --git a/shared/server-commands/videos/videos-command.ts b/shared/server-commands/videos/videos-command.ts
index 1d6cad351..590244484 100644
--- a/shared/server-commands/videos/videos-command.ts
+++ b/shared/server-commands/videos/videos-command.ts
@@ -4,7 +4,7 @@ import { expect } from 'chai'
 import { createReadStream, stat } from 'fs-extra'
 import got, { Response as GotResponse } from 'got'
 import validator from 'validator'
-import { buildAbsoluteFixturePath, omit, pick, wait } from '@shared/core-utils'
+import { buildAbsoluteFixturePath, getAllPrivacies, omit, pick, wait } from '@shared/core-utils'
 import { buildUUID } from '@shared/extra-utils'
 import {
   HttpStatusCode,
@@ -15,6 +15,7 @@ import {
   VideoCreateResult,
   VideoDetails,
   VideoFileMetadata,
+  VideoInclude,
   VideoPrivacy,
   VideosCommonQuery,
   VideoTranscodingCreate
@@ -234,6 +235,22 @@ export class VideosCommand extends AbstractCommand {
     })
   }
 
+  listAllForAdmin (options: OverrideCommandOptions & VideosCommonQuery = {}) {
+    const include = VideoInclude.NOT_PUBLISHED_STATE | VideoInclude.BLACKLISTED | VideoInclude.BLOCKED_OWNER
+    const nsfw = 'both'
+    const privacyOneOf = getAllPrivacies()
+
+    return this.list({
+      ...options,
+
+      include,
+      nsfw,
+      privacyOneOf,
+
+      token: this.buildCommonRequestToken({ ...options, implicitToken: true })
+    })
+  }
+
   listByAccount (options: OverrideCommandOptions & VideosCommonQuery & {
     handle: string
   }) {