From 627621c1e8d37c33f7b3dd59f4c8907b12c630bc Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 18 Sep 2018 12:00:49 +0200 Subject: [PATCH] Optimize SQL requests of watch page API endpoints --- scripts/create-import-video-file-job.ts | 2 +- scripts/create-transcoding-job.ts | 2 +- scripts/prune-storage.ts | 2 +- server/controllers/api/users/me.ts | 2 +- server/controllers/api/videos/comment.ts | 2 +- server/helpers/custom-validators/videos.ts | 14 ++-- server/lib/cache/videos-caption-cache.ts | 2 +- server/lib/cache/videos-preview-cache.ts | 4 +- server/lib/client-html.ts | 6 +- server/lib/job-queue/handlers/video-file.ts | 8 +- server/lib/job-queue/handlers/video-import.ts | 2 +- server/middlewares/validators/users.ts | 2 +- .../middlewares/validators/video-captions.ts | 2 +- .../middlewares/validators/video-comments.ts | 4 +- server/models/video/video.ts | 74 ++++++++++--------- 15 files changed, 68 insertions(+), 60 deletions(-) diff --git a/scripts/create-import-video-file-job.ts b/scripts/create-import-video-file-job.ts index 2b636014a..c8c6c6429 100644 --- a/scripts/create-import-video-file-job.ts +++ b/scripts/create-import-video-file-job.ts @@ -25,7 +25,7 @@ run() async function run () { await initDatabaseModels(true) - const video = await VideoModel.loadByUUID(program['video']) + const video = await VideoModel.loadByUUIDWithFile(program['video']) if (!video) throw new Error('Video not found.') if (video.isOwned() === false) throw new Error('Cannot import files of a non owned video.') diff --git a/scripts/create-transcoding-job.ts b/scripts/create-transcoding-job.ts index 3ea30f98e..7e5b687bb 100755 --- a/scripts/create-transcoding-job.ts +++ b/scripts/create-transcoding-job.ts @@ -28,7 +28,7 @@ run() async function run () { await initDatabaseModels(true) - const video = await VideoModel.loadByUUID(program['video']) + const video = await VideoModel.loadByUUIDWithFile(program['video']) if (!video) throw new Error('Video not found.') const dataInput = { diff --git a/scripts/prune-storage.ts b/scripts/prune-storage.ts index 572283868..b00f20934 100755 --- a/scripts/prune-storage.ts +++ b/scripts/prune-storage.ts @@ -56,7 +56,7 @@ async function pruneDirectory (directory: string) { const uuid = getUUIDFromFilename(file) let video: VideoModel - if (uuid) video = await VideoModel.loadByUUID(uuid) + if (uuid) video = await VideoModel.loadByUUIDWithFile(uuid) if (!uuid || !video) toDelete.push(join(directory, file)) } diff --git a/server/controllers/api/users/me.ts b/server/controllers/api/users/me.ts index e886d4b2a..113563c39 100644 --- a/server/controllers/api/users/me.ts +++ b/server/controllers/api/users/me.ts @@ -293,7 +293,7 @@ async function getUserVideoQuotaUsed (req: express.Request, res: express.Respons } async function getUserVideoRating (req: express.Request, res: express.Response, next: express.NextFunction) { - const videoId = +req.params.videoId + const videoId = res.locals.video.id const accountId = +res.locals.oauth.token.User.Account.id const ratingObj = await AccountVideoRateModel.load(accountId, videoId, null) diff --git a/server/controllers/api/videos/comment.ts b/server/controllers/api/videos/comment.ts index e35247829..8d0692b2b 100644 --- a/server/controllers/api/videos/comment.ts +++ b/server/controllers/api/videos/comment.ts @@ -86,7 +86,7 @@ async function listVideoThreadComments (req: express.Request, res: express.Respo let resultList: ResultList if (video.commentsEnabled === true) { - resultList = await VideoCommentModel.listThreadCommentsForApi(res.locals.video.id, res.locals.videoCommentThread.id) + resultList = await VideoCommentModel.listThreadCommentsForApi(video.id, res.locals.videoCommentThread.id) } else { resultList = { total: 0, diff --git a/server/helpers/custom-validators/videos.ts b/server/helpers/custom-validators/videos.ts index edafba6e2..dd207c787 100644 --- a/server/helpers/custom-validators/videos.ts +++ b/server/helpers/custom-validators/videos.ts @@ -152,13 +152,15 @@ function checkUserCanManageVideo (user: UserModel, video: VideoModel, right: Use return true } -async function isVideoExist (id: string, res: Response) { +async function isVideoExist (id: string, res: Response, fetchType: 'all' | 'only-video' | 'id' | 'none' = 'all') { let video: VideoModel | null - if (validator.isInt(id)) { - video = await VideoModel.loadAndPopulateAccountAndServerAndTags(+id) - } else { // UUID - video = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(id) + if (fetchType === 'all') { + video = await VideoModel.loadAndPopulateAccountAndServerAndTags(id) + } else if (fetchType === 'only-video') { + video = await VideoModel.load(id) + } else if (fetchType === 'id' || fetchType === 'none') { + video = await VideoModel.loadOnlyId(id) } if (video === null) { @@ -169,7 +171,7 @@ async function isVideoExist (id: string, res: Response) { return false } - res.locals.video = video + if (fetchType !== 'none') res.locals.video = video return true } diff --git a/server/lib/cache/videos-caption-cache.ts b/server/lib/cache/videos-caption-cache.ts index 380d42b2c..f240affbc 100644 --- a/server/lib/cache/videos-caption-cache.ts +++ b/server/lib/cache/videos-caption-cache.ts @@ -38,7 +38,7 @@ class VideosCaptionCache extends AbstractVideoStaticFileCache { if (videoCaption.isOwned()) throw new Error('Cannot load remote caption of owned video.') // Used to fetch the path - const video = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(videoId) + const video = await VideoModel.loadAndPopulateAccountAndServerAndTags(videoId) if (!video) return undefined const remoteStaticPath = videoCaption.getCaptionStaticPath() diff --git a/server/lib/cache/videos-preview-cache.ts b/server/lib/cache/videos-preview-cache.ts index 22b6d9cb0..a5d6f5b62 100644 --- a/server/lib/cache/videos-preview-cache.ts +++ b/server/lib/cache/videos-preview-cache.ts @@ -16,7 +16,7 @@ class VideosPreviewCache extends AbstractVideoStaticFileCache { } async getFilePath (videoUUID: string) { - const video = await VideoModel.loadByUUID(videoUUID) + const video = await VideoModel.loadByUUIDWithFile(videoUUID) if (!video) return undefined if (video.isOwned()) return join(CONFIG.STORAGE.PREVIEWS_DIR, video.getPreviewName()) @@ -25,7 +25,7 @@ class VideosPreviewCache extends AbstractVideoStaticFileCache { } protected async loadRemoteFile (key: string) { - const video = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(key) + const video = await VideoModel.loadAndPopulateAccountAndServerAndTags(key) if (!video) return undefined if (video.isOwned()) throw new Error('Cannot load remote preview of owned video.') diff --git a/server/lib/client-html.ts b/server/lib/client-html.ts index b1088c096..fc013e0c3 100644 --- a/server/lib/client-html.ts +++ b/server/lib/client-html.ts @@ -39,10 +39,8 @@ export class ClientHtml { let videoPromise: Bluebird // Let Angular application handle errors - if (validator.isUUID(videoId, 4)) { - videoPromise = VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(videoId) - } else if (validator.isInt(videoId)) { - videoPromise = VideoModel.loadAndPopulateAccountAndServerAndTags(+videoId) + if (validator.isInt(videoId) || validator.isUUID(videoId, 4)) { + videoPromise = VideoModel.loadAndPopulateAccountAndServerAndTags(videoId) } else { return ClientHtml.getIndexHTML(req, res) } diff --git a/server/lib/job-queue/handlers/video-file.ts b/server/lib/job-queue/handlers/video-file.ts index 2c9ca8e12..1463c93fc 100644 --- a/server/lib/job-queue/handlers/video-file.ts +++ b/server/lib/job-queue/handlers/video-file.ts @@ -26,7 +26,7 @@ async function processVideoFileImport (job: Bull.Job) { const payload = job.data as VideoFileImportPayload logger.info('Processing video file import in job %d.', job.id) - const video = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(payload.videoUUID) + const video = await VideoModel.loadAndPopulateAccountAndServerAndTags(payload.videoUUID) // No video, maybe deleted? if (!video) { logger.info('Do not process job %d, video does not exist.', job.id) @@ -43,7 +43,7 @@ async function processVideoFile (job: Bull.Job) { const payload = job.data as VideoFilePayload logger.info('Processing video file in job %d.', job.id) - const video = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(payload.videoUUID) + const video = await VideoModel.loadAndPopulateAccountAndServerAndTags(payload.videoUUID) // No video, maybe deleted? if (!video) { logger.info('Do not process job %d, video does not exist.', job.id) @@ -69,7 +69,7 @@ async function onVideoFileTranscoderOrImportSuccess (video: VideoModel) { return sequelizeTypescript.transaction(async t => { // Maybe the video changed in database, refresh it - let videoDatabase = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(video.uuid, t) + let videoDatabase = await VideoModel.loadAndPopulateAccountAndServerAndTags(video.uuid, t) // Video does not exist anymore if (!videoDatabase) return undefined @@ -99,7 +99,7 @@ async function onVideoFileOptimizerSuccess (video: VideoModel, isNewVideo: boole return sequelizeTypescript.transaction(async t => { // Maybe the video changed in database, refresh it - const videoDatabase = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(video.uuid, t) + const videoDatabase = await VideoModel.loadAndPopulateAccountAndServerAndTags(video.uuid, t) // Video does not exist anymore if (!videoDatabase) return undefined diff --git a/server/lib/job-queue/handlers/video-import.ts b/server/lib/job-queue/handlers/video-import.ts index ebcb2090c..9e14e57e6 100644 --- a/server/lib/job-queue/handlers/video-import.ts +++ b/server/lib/job-queue/handlers/video-import.ts @@ -183,7 +183,7 @@ async function processFile (downloader: () => Promise, videoImport: Vide const videoUpdated = await video.save({ transaction: t }) // Now we can federate the video (reload from database, we need more attributes) - const videoForFederation = await VideoModel.loadByUUIDAndPopulateAccountAndServerAndTags(video.uuid, t) + const videoForFederation = await VideoModel.loadAndPopulateAccountAndServerAndTags(video.uuid, t) await federateVideoIfNeeded(videoForFederation, true, t) // Update video import object diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts index d13c50c84..d3ba1ae23 100644 --- a/server/middlewares/validators/users.ts +++ b/server/middlewares/validators/users.ts @@ -172,7 +172,7 @@ const usersVideoRatingValidator = [ logger.debug('Checking usersVideoRating parameters', { parameters: req.params }) if (areValidationErrors(req, res)) return - if (!await isVideoExist(req.params.videoId, res)) return + if (!await isVideoExist(req.params.videoId, res, 'id')) return return next() } diff --git a/server/middlewares/validators/video-captions.ts b/server/middlewares/validators/video-captions.ts index 4f393ea84..51ffd7f3c 100644 --- a/server/middlewares/validators/video-captions.ts +++ b/server/middlewares/validators/video-captions.ts @@ -58,7 +58,7 @@ const listVideoCaptionsValidator = [ logger.debug('Checking listVideoCaptions parameters', { parameters: req.params }) if (areValidationErrors(req, res)) return - if (!await isVideoExist(req.params.videoId, res)) return + if (!await isVideoExist(req.params.videoId, res, 'id')) return return next() } diff --git a/server/middlewares/validators/video-comments.ts b/server/middlewares/validators/video-comments.ts index 227bc1fca..4b15eed23 100644 --- a/server/middlewares/validators/video-comments.ts +++ b/server/middlewares/validators/video-comments.ts @@ -17,7 +17,7 @@ const listVideoCommentThreadsValidator = [ logger.debug('Checking listVideoCommentThreads parameters.', { parameters: req.params }) if (areValidationErrors(req, res)) return - if (!await isVideoExist(req.params.videoId, res)) return + if (!await isVideoExist(req.params.videoId, res, 'only-video')) return return next() } @@ -31,7 +31,7 @@ const listVideoThreadCommentsValidator = [ logger.debug('Checking listVideoThreadComments parameters.', { parameters: req.params }) if (areValidationErrors(req, res)) return - if (!await isVideoExist(req.params.videoId, res)) return + if (!await isVideoExist(req.params.videoId, res, 'only-video')) return if (!await isVideoCommentThreadExist(req.params.threadId, res.locals.video, res)) return return next() diff --git a/server/models/video/video.ts b/server/models/video/video.ts index ce856aed2..c7cd2890c 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -91,6 +91,7 @@ import { videoModelToFormattedDetailsJSON, videoModelToFormattedJSON } from './video-format-utils' +import * as validator from 'validator' // FIXME: Define indexes here because there is an issue with TS and Sequelize.literal when called directly in the annotation const indexes: Sequelize.DefineIndexesOptions[] = [ @@ -466,6 +467,7 @@ type AvailableForListIDsOptions = { required: false, include: [ { + attributes: [ 'fileUrl' ], model: () => VideoRedundancyModel.unscoped(), required: false } @@ -1062,44 +1064,34 @@ export class VideoModel extends Model { return VideoModel.getAvailableForApi(query, queryOptions) } - static load (id: number, t?: Sequelize.Transaction) { - return VideoModel.findById(id, { transaction: t }) - } - - static loadWithFile (id: number, t?: Sequelize.Transaction, logging?: boolean) { - return VideoModel.scope(ScopeNames.WITH_FILES) - .findById(id, { transaction: t, logging }) - } - - static loadByUrlAndPopulateAccount (url: string, t?: Sequelize.Transaction) { - const query: IFindOptions = { - where: { - url - } + static load (id: number | string, t?: Sequelize.Transaction) { + const where = VideoModel.buildWhereIdOrUUID(id) + const options = { + where, + transaction: t } - if (t !== undefined) query.transaction = t - - return VideoModel.scope([ ScopeNames.WITH_ACCOUNT_DETAILS, ScopeNames.WITH_FILES ]).findOne(query) + return VideoModel.findOne(options) } - static loadAndPopulateAccountAndServerAndTags (id: number) { + static loadOnlyId (id: number | string, t?: Sequelize.Transaction) { + const where = VideoModel.buildWhereIdOrUUID(id) + const options = { - order: [ [ 'Tags', 'name', 'ASC' ] ] + attributes: [ 'id' ], + where, + transaction: t } - return VideoModel - .scope([ - ScopeNames.WITH_TAGS, - ScopeNames.WITH_BLACKLISTED, - ScopeNames.WITH_FILES, - ScopeNames.WITH_ACCOUNT_DETAILS, - ScopeNames.WITH_SCHEDULED_UPDATE - ]) - .findById(id, options) + return VideoModel.findOne(options) + } + + static loadWithFile (id: number, t?: Sequelize.Transaction, logging?: boolean) { + return VideoModel.scope(ScopeNames.WITH_FILES) + .findById(id, { transaction: t, logging }) } - static loadByUUID (uuid: string) { + static loadByUUIDWithFile (uuid: string) { const options = { where: { uuid @@ -1111,12 +1103,24 @@ export class VideoModel extends Model { .findOne(options) } - static loadByUUIDAndPopulateAccountAndServerAndTags (uuid: string, t?: Sequelize.Transaction) { + static loadByUrlAndPopulateAccount (url: string, t?: Sequelize.Transaction) { + const query: IFindOptions = { + where: { + url + } + } + + if (t !== undefined) query.transaction = t + + return VideoModel.scope([ ScopeNames.WITH_ACCOUNT_DETAILS, ScopeNames.WITH_FILES ]).findOne(query) + } + + static loadAndPopulateAccountAndServerAndTags (id: number | string, t?: Sequelize.Transaction) { + const where = VideoModel.buildWhereIdOrUUID(id) + const options = { order: [ [ 'Tags', 'name', 'ASC' ] ], - where: { - uuid - }, + where, transaction: t } @@ -1277,6 +1281,10 @@ export class VideoModel extends Model { return VIDEO_STATES[ id ] || 'Unknown' } + static buildWhereIdOrUUID (id: number | string) { + return validator.isInt('' + id) ? { id } : { uuid: id } + } + getOriginalFile () { if (Array.isArray(this.VideoFiles) === false) return undefined -- 2.25.1