From 516df59b3bbb0218afeda595ee4966800bff4519 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 3 Aug 2018 09:43:00 +0200 Subject: Remove ability to delete video imports Users should remove the linked video instead --- server/controllers/api/videos/import.ts | 21 +--------------- server/helpers/youtube-dl.ts | 2 +- server/lib/job-queue/handlers/video-import.ts | 34 ++++++++++++++++++-------- server/middlewares/validators/video-imports.ts | 29 +++------------------- server/models/video/video.ts | 6 +++-- 5 files changed, 34 insertions(+), 58 deletions(-) (limited to 'server') diff --git a/server/controllers/api/videos/import.ts b/server/controllers/api/videos/import.ts index 680d8665f..ca7a5f9ca 100644 --- a/server/controllers/api/videos/import.ts +++ b/server/controllers/api/videos/import.ts @@ -4,8 +4,7 @@ import { asyncMiddleware, asyncRetryTransactionMiddleware, authenticate, - videoImportAddValidator, - videoImportDeleteValidator + videoImportAddValidator } from '../../../middlewares' import { CONFIG, IMAGE_MIMETYPE_EXT, PREVIEWS_SIZE, sequelizeTypescript, THUMBNAILS_SIZE } from '../../../initializers' import { getYoutubeDLInfo, YoutubeDLInfo } from '../../../helpers/youtube-dl' @@ -39,12 +38,6 @@ videoImportsRouter.post('/imports', asyncRetryTransactionMiddleware(addVideoImport) ) -videoImportsRouter.delete('/imports/:id', - authenticate, - asyncMiddleware(videoImportDeleteValidator), - asyncRetryTransactionMiddleware(deleteVideoImport) -) - // --------------------------------------------------------------------------- export { @@ -145,15 +138,3 @@ async function addVideoImport (req: express.Request, res: express.Response) { return res.json(videoImport.toFormattedJSON()) } - -async function deleteVideoImport (req: express.Request, res: express.Response) { - await sequelizeTypescript.transaction(async t => { - const videoImport = res.locals.videoImport - const video = videoImport.Video - - await videoImport.destroy({ transaction: t }) - await video.destroy({ transaction: t }) - }) - - return res.status(204).end() -} diff --git a/server/helpers/youtube-dl.ts b/server/helpers/youtube-dl.ts index 43156bb22..ff1fbf59f 100644 --- a/server/helpers/youtube-dl.ts +++ b/server/helpers/youtube-dl.ts @@ -30,7 +30,7 @@ function getYoutubeDLInfo (url: string): Promise { } function downloadYoutubeDLVideo (url: string) { - const hash = crypto.createHash('sha256').update(url).digest('base64') + const hash = crypto.createHash('sha256').update(url).digest('hex') const path = join(CONFIG.STORAGE.VIDEOS_DIR, hash + '-import.mp4') logger.info('Importing video %s', url) diff --git a/server/lib/job-queue/handlers/video-import.ts b/server/lib/job-queue/handlers/video-import.ts index 4f2faab7d..3b9d08d3b 100644 --- a/server/lib/job-queue/handlers/video-import.ts +++ b/server/lib/job-queue/handlers/video-import.ts @@ -12,6 +12,7 @@ import { doRequestAndSaveToFile } from '../../../helpers/requests' import { VideoState } from '../../../../shared' import { JobQueue } from '../index' import { federateVideoIfNeeded } from '../../activitypub' +import { VideoModel } from '../../../models/video/video' export type VideoImportPayload = { type: 'youtube-dl' @@ -26,9 +27,13 @@ async function processVideoImport (job: Bull.Job) { logger.info('Processing video import in job %d.', job.id) const videoImport = await VideoImportModel.loadAndPopulateVideo(payload.videoImportId) - if (!videoImport) throw new Error('Cannot import video %s: the video import entry does not exist anymore.') + if (!videoImport || !videoImport.Video) { + throw new Error('Cannot import video %s: the video import or video linked to this import does not exist anymore.') + } let tempVideoPath: string + let videoDestFile: string + let videoFile: VideoFileModel try { // Download video from youtubeDL tempVideoPath = await downloadYoutubeDLVideo(videoImport.targetUrl) @@ -47,11 +52,14 @@ async function processVideoImport (job: Bull.Job) { fps, videoId: videoImport.videoId } - const videoFile = new VideoFileModel(videoFileData) + videoFile = new VideoFileModel(videoFileData) + // Import if the import fails, to clean files + videoImport.Video.VideoFiles = [ videoFile ] // Move file - const destination = join(CONFIG.STORAGE.VIDEOS_DIR, videoImport.Video.getVideoFilename(videoFile)) - await renamePromise(tempVideoPath, destination) + videoDestFile = join(CONFIG.STORAGE.VIDEOS_DIR, videoImport.Video.getVideoFilename(videoFile)) + await renamePromise(tempVideoPath, videoDestFile) + tempVideoPath = null // This path is not used anymore // Process thumbnail if (payload.downloadThumbnail) { @@ -77,15 +85,21 @@ async function processVideoImport (job: Bull.Job) { await videoImport.Video.createTorrentAndSetInfoHash(videoFile) const videoImportUpdated: VideoImportModel = await sequelizeTypescript.transaction(async t => { - await videoFile.save({ transaction: t }) + // Refresh video + const video = await VideoModel.load(videoImport.videoId, t) + if (!video) throw new Error('Video linked to import ' + videoImport.videoId + ' does not exist anymore.') + videoImport.Video = video + + const videoFileCreated = await videoFile.save({ transaction: t }) + video.VideoFiles = [ videoFileCreated ] // Update video DB object - videoImport.Video.duration = duration - videoImport.Video.state = CONFIG.TRANSCODING.ENABLED ? VideoState.TO_TRANSCODE : VideoState.PUBLISHED - const videoUpdated = await videoImport.Video.save({ transaction: t }) + video.duration = duration + video.state = CONFIG.TRANSCODING.ENABLED ? VideoState.TO_TRANSCODE : VideoState.PUBLISHED + const videoUpdated = await video.save({ transaction: t }) // Now we can federate the video - await federateVideoIfNeeded(videoImport.Video, true, t) + await federateVideoIfNeeded(video, true, t) // Update video import object videoImport.state = VideoImportState.SUCCESS @@ -112,7 +126,7 @@ async function processVideoImport (job: Bull.Job) { try { if (tempVideoPath) await unlinkPromise(tempVideoPath) } catch (errUnlink) { - logger.error('Cannot cleanup files after a video import error.', { err: errUnlink }) + logger.warn('Cannot cleanup files after a video import error.', { err: errUnlink }) } videoImport.error = err.message diff --git a/server/middlewares/validators/video-imports.ts b/server/middlewares/validators/video-imports.ts index 0dedcf803..e0a552976 100644 --- a/server/middlewares/validators/video-imports.ts +++ b/server/middlewares/validators/video-imports.ts @@ -1,14 +1,12 @@ import * as express from 'express' -import { body, param } from 'express-validator/check' +import { body } from 'express-validator/check' import { isIdValid } from '../../helpers/custom-validators/misc' import { logger } from '../../helpers/logger' import { areValidationErrors } from './utils' import { getCommonVideoAttributes } from './videos' -import { isVideoImportTargetUrlValid, isVideoImportExist } from '../../helpers/custom-validators/video-imports' +import { isVideoImportTargetUrlValid } from '../../helpers/custom-validators/video-imports' import { cleanUpReqFiles } from '../../helpers/utils' -import { isVideoChannelOfAccountExist, isVideoNameValid, checkUserCanManageVideo } from '../../helpers/custom-validators/videos' -import { VideoImportModel } from '../../models/video/video-import' -import { UserRight } from '../../../shared' +import { isVideoChannelOfAccountExist, isVideoNameValid } from '../../helpers/custom-validators/videos' const videoImportAddValidator = getCommonVideoAttributes().concat([ body('targetUrl').custom(isVideoImportTargetUrlValid).withMessage('Should have a valid video import target URL'), @@ -31,29 +29,10 @@ const videoImportAddValidator = getCommonVideoAttributes().concat([ } ]) -const videoImportDeleteValidator = [ - param('id').custom(isIdValid).not().isEmpty().withMessage('Should have a valid id'), - - async (req: express.Request, res: express.Response, next: express.NextFunction) => { - logger.debug('Checking videoImportDeleteValidator parameters', { parameters: req.body }) - - if (areValidationErrors(req, res)) return - if (!await isVideoImportExist(req.params.id, res)) return - - const user = res.locals.oauth.token.User - const videoImport: VideoImportModel = res.locals.videoImport - - if (!await checkUserCanManageVideo(user, videoImport.Video, UserRight.UPDATE_ANY_VIDEO, res)) return - - return next() - } -] - // --------------------------------------------------------------------------- export { - videoImportAddValidator, - videoImportDeleteValidator + videoImportAddValidator } // --------------------------------------------------------------------------- diff --git a/server/models/video/video.ts b/server/models/video/video.ts index f32010014..67711b102 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -957,8 +957,10 @@ export class VideoModel extends Model { }) } - static load (id: number) { - return VideoModel.findById(id) + static load (id: number, t?: Sequelize.Transaction) { + const options = t ? { transaction: t } : undefined + + return VideoModel.findById(id, options) } static loadByUrlAndPopulateAccount (url: string, t?: Sequelize.Transaction) { -- cgit v1.2.3