From 91f6f169b1110eeae6ebf5c387f4204b0d07703c Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 12 Sep 2017 14:17:46 +0200 Subject: Fix concurrency error when deleting a video --- server/controllers/api/videos/index.ts | 36 +++++++++++++++++++++++----------- server/lib/friends.ts | 4 ++-- server/models/video/video.ts | 6 +++--- 3 files changed, 30 insertions(+), 16 deletions(-) (limited to 'server') 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) -- cgit v1.2.3