From: Chocobozzz <florian.bigard@gmail.com>
Date: Tue, 12 Sep 2017 12:17:46 +0000 (+0200)
Subject: Fix concurrency error when deleting a video
X-Git-Tag: v0.0.1-alpha~325
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=91f6f169b1110eeae6ebf5c387f4204b0d07703c;p=oweals%2Fpeertube.git

Fix concurrency error when deleting a video
---

diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts
index 7a9cd9d37..6fa84c801 100644
--- a/server/controllers/api/videos/index.ts
+++ b/server/controllers/api/videos/index.ts
@@ -45,6 +45,7 @@ import { VideoCreate, VideoUpdate } from '../../../../shared'
 import { abuseVideoRouter } from './abuse'
 import { blacklistRouter } from './blacklist'
 import { rateVideoRouter } from './rate'
+import { VideoInstance } from '../../../models/video/video-interface'
 
 const videosRouter = express.Router()
 
@@ -106,7 +107,7 @@ videosRouter.get('/:id',
 videosRouter.delete('/:id',
   authenticate,
   videosRemoveValidator,
-  removeVideo
+  removeVideoRetryWrapper
 )
 
 videosRouter.get('/search/:value',
@@ -291,7 +292,6 @@ function updateVideoRetryWrapper (req: express.Request, res: express.Response, n
 
   retryTransactionWrapper(updateVideo, options)
     .then(() => {
-      // TODO : include Location of the new video -> 201
       return res.type('json').status(204).end()
     })
     .catch(err => next(err))
@@ -396,18 +396,32 @@ function listVideos (req: express.Request, res: express.Response, next: express.
     .catch(err => next(err))
 }
 
-function removeVideo (req: express.Request, res: express.Response, next: express.NextFunction) {
-  const videoInstance = res.locals.video
+function removeVideoRetryWrapper (req: express.Request, res: express.Response, next: express.NextFunction) {
+  const options = {
+    arguments: [ req, res ],
+    errorMessage: 'Cannot remove the video with many retries.'
+  }
 
-  videoInstance.destroy()
+  retryTransactionWrapper(removeVideo, options)
     .then(() => {
-      logger.info('Video with name %s and uuid %s deleted.', videoInstance.name, videoInstance.uuid)
-      res.type('json').status(204).end()
-    })
-    .catch(err => {
-      logger.error('Errors when removed the video.', err)
-      return next(err)
+      return res.type('json').status(204).end()
     })
+    .catch(err => next(err))
+}
+
+function removeVideo (req: express.Request, res: express.Response) {
+  const videoInstance: VideoInstance = res.locals.video
+
+  return db.sequelize.transaction(t => {
+    return videoInstance.destroy({ transaction: t })
+  })
+  .then(() => {
+    logger.info('Video with name %s and uuid %s deleted.', videoInstance.name, videoInstance.uuid)
+  })
+  .catch(err => {
+    logger.error('Errors when removed the video.', err)
+    throw err
+  })
 }
 
 function searchVideos (req: express.Request, res: express.Response, next: express.NextFunction) {
diff --git a/server/lib/friends.ts b/server/lib/friends.ts
index 3f0ce3f33..ea9ddbe8d 100644
--- a/server/lib/friends.ts
+++ b/server/lib/friends.ts
@@ -80,12 +80,12 @@ function updateVideoToFriends (videoData: RemoteVideoUpdateData, transaction: Se
   return createRequest(options)
 }
 
-function removeVideoToFriends (videoParams: RemoteVideoRemoveData) {
+function removeVideoToFriends (videoParams: RemoteVideoRemoveData, transaction: Sequelize.Transaction) {
   const options = {
     type: ENDPOINT_ACTIONS.REMOVE,
     endpoint: REQUEST_ENDPOINTS.VIDEOS,
     data: videoParams,
-    transaction: null
+    transaction
   }
   return createRequest(options)
 }
diff --git a/server/models/video/video.ts b/server/models/video/video.ts
index 1134525f0..e011c3b4d 100644
--- a/server/models/video/video.ts
+++ b/server/models/video/video.ts
@@ -300,7 +300,7 @@ function associate (models) {
   })
 }
 
-function afterDestroy (video: VideoInstance) {
+function afterDestroy (video: VideoInstance, options: { transaction: Sequelize.Transaction }) {
   const tasks = []
 
   tasks.push(
@@ -314,10 +314,10 @@ function afterDestroy (video: VideoInstance) {
 
     tasks.push(
       video.removePreview(),
-      removeVideoToFriends(removeVideoToFriendsParams)
+      removeVideoToFriends(removeVideoToFriendsParams, options.transaction)
     )
 
-    // TODO: check files is populated
+    // Remove physical files and torrents
     video.VideoFiles.forEach(file => {
       video.removeFile(file),
       video.removeTorrent(file)