From 508c1b1e9f3b26752a961e945b7fa59b72b30827 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 25 Oct 2022 11:51:20 +0200 Subject: [PATCH] Correctly cleanup files from object storage --- .../shared/object-storage-helpers.ts | 15 ++++++++----- server/models/video/video.ts | 5 ++--- .../video-static-file-privacy.ts | 22 ++++++++++--------- .../server-commands/videos/videos-command.ts | 19 +++++++++++++++- 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>> { // 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 }) {