From 09979f8959425390b879bce22101a9bc061ae9a0 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 5 Mar 2019 11:30:43 +0100 Subject: [PATCH] Refactor video playlist middlewares --- server/controllers/api/video-playlist.ts | 24 +++------------ .../custom-validators/video-playlists.ts | 6 ++-- .../validators/videos/video-playlists.ts | 8 ++--- server/models/account/account.ts | 6 ++-- server/models/video/video-playlist.ts | 30 +++++++++++++++++-- server/tests/api/videos/index.ts | 1 + shared/utils/videos/videos.ts | 1 - 7 files changed, 44 insertions(+), 32 deletions(-) diff --git a/server/controllers/api/video-playlist.ts b/server/controllers/api/video-playlist.ts index 8605f3dfc..2700e8dc7 100644 --- a/server/controllers/api/video-playlist.ts +++ b/server/controllers/api/video-playlist.ts @@ -4,7 +4,8 @@ import { asyncMiddleware, asyncRetryTransactionMiddleware, authenticate, - commonVideosFiltersValidator, optionalAuthenticate, + commonVideosFiltersValidator, + optionalAuthenticate, paginationValidator, setDefaultPagination, setDefaultSort @@ -31,15 +32,8 @@ import { VideoPlaylistPrivacy } from '../../../shared/models/videos/playlist/vid import { processImage } from '../../helpers/image-utils' import { join } from 'path' import { UserModel } from '../../models/account/user' -import { - sendCreateVideoPlaylist, - sendDeleteVideoPlaylist, - sendUpdateVideoPlaylist -} from '../../lib/activitypub/send' -import { - getVideoPlaylistActivityPubUrl, - getVideoPlaylistElementActivityPubUrl -} from '../../lib/activitypub/url' +import { sendCreateVideoPlaylist, sendDeleteVideoPlaylist, sendUpdateVideoPlaylist } from '../../lib/activitypub/send' +import { getVideoPlaylistActivityPubUrl, getVideoPlaylistElementActivityPubUrl } from '../../lib/activitypub/url' import { VideoPlaylistUpdate } from '../../../shared/models/videos/playlist/video-playlist-update.model' import { VideoModel } from '../../models/video/video' import { VideoPlaylistElementModel } from '../../models/video/video-playlist-element' @@ -233,8 +227,6 @@ async function updateVideoPlaylist (req: express.Request, res: express.Response) } const playlistUpdated = await videoPlaylistInstance.save(sequelizeOptions) - // We need more attributes for the federation - playlistUpdated.OwnerAccount = await AccountModel.load(playlistUpdated.OwnerAccount.id, t) const isNewPlaylist = wasPrivatePlaylist && playlistUpdated.privacy !== VideoPlaylistPrivacy.PRIVATE @@ -305,8 +297,6 @@ async function addVideoInPlaylist (req: express.Request, res: express.Response) } } - // We need more attributes for the federation - videoPlaylist.OwnerAccount = await AccountModel.load(videoPlaylist.OwnerAccount.id, t) await sendUpdateVideoPlaylist(videoPlaylist, t) return playlistElement @@ -332,8 +322,6 @@ async function updateVideoPlaylistElement (req: express.Request, res: express.Re const element = await videoPlaylistElement.save({ transaction: t }) - // We need more attributes for the federation - videoPlaylist.OwnerAccount = await AccountModel.load(videoPlaylist.OwnerAccount.id, t) await sendUpdateVideoPlaylist(videoPlaylist, t) return element @@ -355,8 +343,6 @@ async function removeVideoFromPlaylist (req: express.Request, res: express.Respo // Decrease position of the next elements await VideoPlaylistElementModel.increasePositionOf(videoPlaylist.id, positionToDelete, null, -1, t) - // We need more attributes for the federation - videoPlaylist.OwnerAccount = await AccountModel.load(videoPlaylist.OwnerAccount.id, t) await sendUpdateVideoPlaylist(videoPlaylist, t) logger.info('Video playlist element %d of playlist %s deleted.', videoPlaylistElement.position, videoPlaylist.uuid) @@ -398,8 +384,6 @@ async function reorderVideosPlaylist (req: express.Request, res: express.Respons // Decrease positions of elements after the old position of our ordered elements (decrease) await VideoPlaylistElementModel.increasePositionOf(videoPlaylist.id, oldPosition, null, -reorderLength, t) - // We need more attributes for the federation - videoPlaylist.OwnerAccount = await AccountModel.load(videoPlaylist.OwnerAccount.id, t) await sendUpdateVideoPlaylist(videoPlaylist, t) }) diff --git a/server/helpers/custom-validators/video-playlists.ts b/server/helpers/custom-validators/video-playlists.ts index c217a39bf..e0eb423d7 100644 --- a/server/helpers/custom-validators/video-playlists.ts +++ b/server/helpers/custom-validators/video-playlists.ts @@ -26,8 +26,10 @@ function isVideoPlaylistTypeValid (value: any) { return exists(value) && VIDEO_PLAYLIST_TYPES[ value ] !== undefined } -async function isVideoPlaylistExist (id: number | string, res: express.Response) { - const videoPlaylist = await VideoPlaylistModel.loadWithAccountAndChannel(id, undefined) +async function isVideoPlaylistExist (id: number | string, res: express.Response, fetchType: 'summary' | 'all' = 'summary') { + const videoPlaylist = fetchType === 'summary' + ? await VideoPlaylistModel.loadWithAccountAndChannelSummary(id, undefined) + : await VideoPlaylistModel.loadWithAccountAndChannel(id, undefined) if (!videoPlaylist) { res.status(404) diff --git a/server/middlewares/validators/videos/video-playlists.ts b/server/middlewares/validators/videos/video-playlists.ts index 796c63748..fa26e2336 100644 --- a/server/middlewares/validators/videos/video-playlists.ts +++ b/server/middlewares/validators/videos/video-playlists.ts @@ -45,7 +45,7 @@ const videoPlaylistsUpdateValidator = getCommonPlaylistEditAttributes().concat([ if (areValidationErrors(req, res)) return cleanUpReqFiles(req) - if (!await isVideoPlaylistExist(req.params.playlistId, res)) return cleanUpReqFiles(req) + if (!await isVideoPlaylistExist(req.params.playlistId, res, 'all')) return cleanUpReqFiles(req) const videoPlaylist = res.locals.videoPlaylist @@ -153,7 +153,7 @@ const videoPlaylistsAddVideoValidator = [ if (areValidationErrors(req, res)) return - if (!await isVideoPlaylistExist(req.params.playlistId, res)) return + if (!await isVideoPlaylistExist(req.params.playlistId, res, 'all')) return if (!await isVideoExist(req.body.videoId, res, 'only-video')) return const videoPlaylist: VideoPlaylistModel = res.locals.videoPlaylist @@ -193,7 +193,7 @@ const videoPlaylistsUpdateOrRemoveVideoValidator = [ if (areValidationErrors(req, res)) return - if (!await isVideoPlaylistExist(req.params.playlistId, res)) return + if (!await isVideoPlaylistExist(req.params.playlistId, res, 'all')) return if (!await isVideoExist(req.params.videoId, res, 'id')) return const videoPlaylist: VideoPlaylistModel = res.locals.videoPlaylist @@ -261,7 +261,7 @@ const videoPlaylistsReorderVideosValidator = [ if (areValidationErrors(req, res)) return - if (!await isVideoPlaylistExist(req.params.playlistId, res)) return + if (!await isVideoPlaylistExist(req.params.playlistId, res, 'all')) return const videoPlaylist: VideoPlaylistModel = res.locals.videoPlaylist if (!checkUserCanManageVideoPlaylist(res.locals.oauth.token.User, videoPlaylist, UserRight.UPDATE_ANY_VIDEO_PLAYLIST, res)) return diff --git a/server/models/account/account.ts b/server/models/account/account.ts index 3fb766c8a..7cc40f631 100644 --- a/server/models/account/account.ts +++ b/server/models/account/account.ts @@ -10,7 +10,8 @@ import { ForeignKey, HasMany, Is, - Model, Scopes, + Model, + Scopes, Table, UpdatedAt } from 'sequelize-typescript' @@ -26,7 +27,6 @@ import { VideoCommentModel } from '../video/video-comment' import { UserModel } from './user' import { CONFIG } from '../../initializers' import { AvatarModel } from '../avatar/avatar' -import { WhereOptions } from 'sequelize' import { VideoPlaylistModel } from '../video/video-playlist' export enum ScopeNames { @@ -42,7 +42,7 @@ export enum ScopeNames { ] }) @Scopes({ - [ ScopeNames.SUMMARY ]: (whereActor?: WhereOptions) => { + [ ScopeNames.SUMMARY ]: (whereActor?: Sequelize.WhereOptions) => { return { attributes: [ 'id', 'name' ], include: [ diff --git a/server/models/video/video-playlist.ts b/server/models/video/video-playlist.ts index ce49f77ec..4d2ea0a66 100644 --- a/server/models/video/video-playlist.ts +++ b/server/models/video/video-playlist.ts @@ -47,7 +47,8 @@ enum ScopeNames { AVAILABLE_FOR_LIST = 'AVAILABLE_FOR_LIST', WITH_VIDEOS_LENGTH = 'WITH_VIDEOS_LENGTH', WITH_ACCOUNT_AND_CHANNEL_SUMMARY = 'WITH_ACCOUNT_AND_CHANNEL_SUMMARY', - WITH_ACCOUNT = 'WITH_ACCOUNT' + WITH_ACCOUNT = 'WITH_ACCOUNT', + WITH_ACCOUNT_AND_CHANNEL = 'WITH_ACCOUNT_AND_CHANNEL' } type AvailableForListOptions = { @@ -89,6 +90,18 @@ type AvailableForListOptions = { } ] }, + [ ScopeNames.WITH_ACCOUNT_AND_CHANNEL ]: { + include: [ + { + model: () => AccountModel, + required: true + }, + { + model: () => VideoChannelModel, + required: false + } + ] + }, [ ScopeNames.AVAILABLE_FOR_LIST ]: (options: AvailableForListOptions) => { // Only list local playlists OR playlists that are on an instance followed by actorId const inQueryInstanceFollow = buildServerIdsFollowedBy(options.followerActorId) @@ -317,7 +330,7 @@ export class VideoPlaylistModel extends Model { .then(e => !!e) } - static loadWithAccountAndChannel (id: number | string, transaction: Sequelize.Transaction) { + static loadWithAccountAndChannelSummary (id: number | string, transaction: Sequelize.Transaction) { const where = buildWhereIdOrUUID(id) const query = { @@ -330,6 +343,19 @@ export class VideoPlaylistModel extends Model { .findOne(query) } + static loadWithAccountAndChannel (id: number | string, transaction: Sequelize.Transaction) { + const where = buildWhereIdOrUUID(id) + + const query = { + where, + transaction + } + + return VideoPlaylistModel + .scope([ ScopeNames.WITH_ACCOUNT_AND_CHANNEL, ScopeNames.WITH_VIDEOS_LENGTH ]) + .findOne(query) + } + static loadByUrlAndPopulateAccount (url: string) { const query = { where: { diff --git a/server/tests/api/videos/index.ts b/server/tests/api/videos/index.ts index a501a80b2..4be12ad15 100644 --- a/server/tests/api/videos/index.ts +++ b/server/tests/api/videos/index.ts @@ -11,6 +11,7 @@ import './video-description' import './video-hls' import './video-imports' import './video-nsfw' +import './video-playlists' import './video-privacy' import './video-schedule-update' import './video-transcoder' diff --git a/shared/utils/videos/videos.ts b/shared/utils/videos/videos.ts index 16b5165f1..54c6bccec 100644 --- a/shared/utils/videos/videos.ts +++ b/shared/utils/videos/videos.ts @@ -608,7 +608,6 @@ async function uploadVideoAndGetId (options: { server: ServerInfo, videoName: st const videoAttrs: any = { name: options.videoName } if (options.nsfw) videoAttrs.nsfw = options.nsfw - const res = await uploadVideo(options.server.url, options.token || options.server.accessToken, videoAttrs) return { id: res.body.video.id, uuid: res.body.video.uuid }