From 926c3f2b377196af1e8c5725aa54b8077cb79061 Mon Sep 17 00:00:00 2001
From: q_h <q_h@inbox.ru>
Date: Thu, 29 Jun 2023 11:38:37 +0300
Subject: [PATCH] Fix the cleanup after a failed upload (#5840)

* Fix the cleanup after a failed upload

* Update tests

* Update tests
---
 .../middlewares/validators/videos/videos.ts   |  4 ++--
 server/tests/api/videos/resumable-upload.ts   | 24 ++++++++++++-------
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts
index 9034772c0..b829e4eb4 100644
--- a/server/middlewares/validators/videos/videos.ts
+++ b/server/middlewares/validators/videos/videos.ts
@@ -2,6 +2,7 @@ import express from 'express'
 import { body, header, param, query, ValidationChain } from 'express-validator'
 import { isTestInstance } from '@server/helpers/core-utils'
 import { getResumableUploadPath } from '@server/helpers/upload'
+import { uploadx } from '@server/lib/uploadx'
 import { Redis } from '@server/lib/redis'
 import { getServerActor } from '@server/models/application/application'
 import { ExpressPromiseHandler } from '@server/types/express-handler'
@@ -40,7 +41,6 @@ import {
 } from '../../../helpers/custom-validators/videos'
 import { cleanUpReqFiles } from '../../../helpers/express-utils'
 import { logger } from '../../../helpers/logger'
-import { deleteFileAndCatch } from '../../../helpers/utils'
 import { getVideoWithAttributes } from '../../../helpers/video'
 import { CONFIG } from '../../../initializers/config'
 import { CONSTRAINTS_FIELDS, OVERVIEWS } from '../../../initializers/constants'
@@ -115,7 +115,7 @@ const videosAddResumableValidator = [
     const user = res.locals.oauth.token.User
     const body: express.CustomUploadXFile<express.UploadXFileMetadata> = req.body
     const file = { ...body, duration: undefined, path: getResumableUploadPath(body.name), filename: body.metadata.filename }
-    const cleanup = () => deleteFileAndCatch(file.path)
+    const cleanup = () => uploadx.storage.delete(file).catch(err => logger.error('Cannot delete the file %s', file.name, { err }))
 
     const uploadId = req.query.upload_id
     const sessionExists = await Redis.Instance.doesUploadSessionExist(uploadId)
diff --git a/server/tests/api/videos/resumable-upload.ts b/server/tests/api/videos/resumable-upload.ts
index 9de622281..91eb61833 100644
--- a/server/tests/api/videos/resumable-upload.ts
+++ b/server/tests/api/videos/resumable-upload.ts
@@ -93,10 +93,10 @@ describe('Test resumable upload', function () {
     expect((await stat(filePath)).size).to.equal(expectedSize)
   }
 
-  async function countResumableUploads () {
+  async function countResumableUploads (wait?: number) {
     const subPath = join('tmp', 'resumable-uploads')
     const filePath = server.servers.buildDirectory(subPath)
-
+    await new Promise(resolve => setTimeout(resolve, wait))
     const files = await readdir(filePath)
     return files.length
   }
@@ -122,14 +122,20 @@ describe('Test resumable upload', function () {
 
   describe('Directory cleaning', function () {
 
-    // FIXME: https://github.com/kukhariev/node-uploadx/pull/524/files#r852989382
-    // it('Should correctly delete files after an upload', async function () {
-    //   const uploadId = await prepareUpload()
-    //   await sendChunks({ pathUploadId: uploadId })
-    //   await server.videos.endResumableUpload({ pathUploadId: uploadId })
+    it('Should correctly delete files after an upload', async function () {
+      const uploadId = await prepareUpload()
+      await sendChunks({ pathUploadId: uploadId })
+      await server.videos.endResumableUpload({ pathUploadId: uploadId })
 
-    //   expect(await countResumableUploads()).to.equal(0)
-    // })
+      expect(await countResumableUploads()).to.equal(0)
+    })
+
+    it('Should correctly delete corrupt files', async function () {
+      const uploadId = await prepareUpload({ size: 8 * 1024 })
+      await sendChunks({ pathUploadId: uploadId, size: 8 * 1024, expectedStatus: HttpStatusCode.UNPROCESSABLE_ENTITY_422 })
+
+      expect(await countResumableUploads(2000)).to.equal(0)
+    })
 
     it('Should not delete files after an unfinished upload', async function () {
       await prepareUpload()