From 943e5193905908dd1f2800d8810c635d86e3b28f Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 4 Feb 2020 15:45:41 +0100 Subject: [PATCH] Don't refresh videos when processing views It allows us to use a cache --- server/helpers/video.ts | 15 ++++-- server/lib/activitypub/audience.ts | 12 ++++- .../lib/activitypub/process/process-view.ts | 3 +- server/lib/activitypub/send/utils.ts | 4 +- server/lib/activitypub/videos.ts | 46 +++++++++++++------ .../middlewares/validators/videos/videos.ts | 3 ++ server/models/model-cache.ts | 6 ++- server/models/video/video.ts | 36 +++++++++++---- server/typings/models/video/video.ts | 2 +- 9 files changed, 94 insertions(+), 33 deletions(-) diff --git a/server/helpers/video.ts b/server/helpers/video.ts index 907564703..4fe2a60f0 100644 --- a/server/helpers/video.ts +++ b/server/helpers/video.ts @@ -38,14 +38,23 @@ function fetchVideo ( if (fetchType === 'id' || fetchType === 'none') return VideoModel.loadOnlyId(id) } -type VideoFetchByUrlType = 'all' | 'only-video' +type VideoFetchByUrlType = 'all' | 'only-video' | 'only-immutable-attributes' function fetchVideoByUrl (url: string, fetchType: 'all'): Bluebird +function fetchVideoByUrl (url: string, fetchType: 'only-immutable-attributes'): Bluebird function fetchVideoByUrl (url: string, fetchType: 'only-video'): Bluebird -function fetchVideoByUrl (url: string, fetchType: VideoFetchByUrlType): Bluebird -function fetchVideoByUrl (url: string, fetchType: VideoFetchByUrlType): Bluebird { +function fetchVideoByUrl ( + url: string, + fetchType: VideoFetchByUrlType +): Bluebird +function fetchVideoByUrl ( + url: string, + fetchType: VideoFetchByUrlType +): Bluebird { if (fetchType === 'all') return VideoModel.loadByUrlAndPopulateAccount(url) + if (fetchType === 'only-immutable-attributes') return VideoModel.loadByUrlImmutableAttributes(url) + if (fetchType === 'only-video') return VideoModel.loadByUrl(url) } diff --git a/server/lib/activitypub/audience.ts b/server/lib/activitypub/audience.ts index f2ab54cf7..39caeef7b 100644 --- a/server/lib/activitypub/audience.ts +++ b/server/lib/activitypub/audience.ts @@ -4,7 +4,15 @@ import { ACTIVITY_PUB } from '../../initializers/constants' import { ActorModel } from '../../models/activitypub/actor' import { VideoModel } from '../../models/video/video' import { VideoShareModel } from '../../models/video/video-share' -import { MActorFollowersUrl, MActorLight, MCommentOwner, MCommentOwnerVideo, MVideo, MVideoAccountLight } from '../../typings/models' +import { + MActorFollowersUrl, + MActorLight, + MCommentOwner, + MCommentOwnerVideo, + MVideo, + MVideoAccountLight, + MVideoId +} from '../../typings/models' function getRemoteVideoAudience (video: MVideoAccountLight, actorsInvolvedInVideo: MActorFollowersUrl[]): ActivityAudience { return { @@ -48,7 +56,7 @@ function getAudienceFromFollowersOf (actorsInvolvedInObject: MActorFollowersUrl[ } } -async function getActorsInvolvedInVideo (video: MVideo, t: Transaction) { +async function getActorsInvolvedInVideo (video: MVideoId, t: Transaction) { const actors: MActorLight[] = await VideoShareModel.loadActorsByShare(video.id, t) const videoAll = video as VideoModel diff --git a/server/lib/activitypub/process/process-view.ts b/server/lib/activitypub/process/process-view.ts index df29ee968..b3b6c933d 100644 --- a/server/lib/activitypub/process/process-view.ts +++ b/server/lib/activitypub/process/process-view.ts @@ -23,7 +23,8 @@ async function processCreateView (activity: ActivityView | ActivityCreate, byAct const options = { videoObject, - fetchType: 'only-video' as 'only-video' + fetchType: 'only-immutable-attributes' as 'only-immutable-attributes', + allowRefresh: false as false } const { video } = await getOrCreateVideoAndAccountAndChannel(options) diff --git a/server/lib/activitypub/send/utils.ts b/server/lib/activitypub/send/utils.ts index 0d67bb3d6..9436daf17 100644 --- a/server/lib/activitypub/send/utils.ts +++ b/server/lib/activitypub/send/utils.ts @@ -7,7 +7,7 @@ import { JobQueue } from '../../job-queue' import { getActorsInvolvedInVideo, getAudienceFromFollowersOf, getRemoteVideoAudience } from '../audience' import { getServerActor } from '../../../helpers/utils' import { afterCommitIfTransaction } from '../../../helpers/database-utils' -import { MActorWithInboxes, MActor, MActorId, MActorLight, MVideo, MVideoAccountLight } from '../../../typings/models' +import { MActorWithInboxes, MActor, MActorId, MActorLight, MVideo, MVideoAccountLight, MVideoId } from '../../../typings/models' import { ContextType } from '@server/helpers/activitypub' async function sendVideoRelatedActivity (activityBuilder: (audience: ActivityAudience) => Activity, options: { @@ -43,7 +43,7 @@ async function forwardVideoRelatedActivity ( activity: Activity, t: Transaction, followersException: MActorWithInboxes[] = [], - video: MVideo + video: MVideoId ) { // Mastodon does not add our announces in audience, so we forward to them manually const additionalActors = await getActorsInvolvedInVideo(video, t) diff --git a/server/lib/activitypub/videos.ts b/server/lib/activitypub/videos.ts index 9e43caa20..7d8296e45 100644 --- a/server/lib/activitypub/videos.ts +++ b/server/lib/activitypub/videos.ts @@ -68,7 +68,7 @@ import { MVideoAPWithoutCaption, MVideoFile, MVideoFullLight, - MVideoId, + MVideoId, MVideoImmutable, MVideoThumbnail } from '../../typings/models' import { MThumbnail } from '../../typings/models/video/thumbnail' @@ -200,24 +200,41 @@ async function syncVideoExternalAttributes (video: MVideo, fetchedVideo: VideoTo await Bluebird.map(jobPayloads, payload => JobQueue.Instance.createJobWithPromise({ type: 'activitypub-http-fetcher', payload })) } -function getOrCreateVideoAndAccountAndChannel (options: { +type GetVideoResult = Promise<{ + video: T + created: boolean + autoBlacklisted?: boolean +}> + +type GetVideoParamAll = { videoObject: { id: string } | string syncParam?: SyncParam fetchType?: 'all' allowRefresh?: boolean -}): Promise<{ video: MVideoAccountLightBlacklistAllFiles, created: boolean, autoBlacklisted?: boolean }> -function getOrCreateVideoAndAccountAndChannel (options: { +} + +type GetVideoParamImmutable = { videoObject: { id: string } | string syncParam?: SyncParam - fetchType?: VideoFetchByUrlType - allowRefresh?: boolean -}): Promise<{ video: MVideoAccountLightBlacklistAllFiles | MVideoThumbnail, created: boolean, autoBlacklisted?: boolean }> -async function getOrCreateVideoAndAccountAndChannel (options: { + fetchType: 'only-immutable-attributes' + allowRefresh: false +} + +type GetVideoParamOther = { videoObject: { id: string } | string syncParam?: SyncParam - fetchType?: VideoFetchByUrlType - allowRefresh?: boolean // true by default -}): Promise<{ video: MVideoAccountLightBlacklistAllFiles | MVideoThumbnail, created: boolean, autoBlacklisted?: boolean }> { + fetchType?: 'all' | 'only-video' + allowRefresh?: boolean +} + +function getOrCreateVideoAndAccountAndChannel (options: GetVideoParamAll): GetVideoResult +function getOrCreateVideoAndAccountAndChannel (options: GetVideoParamImmutable): GetVideoResult +function getOrCreateVideoAndAccountAndChannel ( + options: GetVideoParamOther +): GetVideoResult +async function getOrCreateVideoAndAccountAndChannel ( + options: GetVideoParamAll | GetVideoParamImmutable | GetVideoParamOther +): GetVideoResult { // Default params const syncParam = options.syncParam || { likes: true, dislikes: true, shares: true, comments: true, thumbnail: true, refreshVideo: false } const fetchType = options.fetchType || 'all' @@ -225,12 +242,13 @@ async function getOrCreateVideoAndAccountAndChannel (options: { // Get video url const videoUrl = getAPId(options.videoObject) - let videoFromDatabase = await fetchVideoByUrl(videoUrl, fetchType) + if (videoFromDatabase) { - if (videoFromDatabase.isOutdated() && allowRefresh === true) { + // If allowRefresh is true, we could not call this function using 'only-immutable-attributes' fetch type + if (allowRefresh === true && (videoFromDatabase as MVideoThumbnail).isOutdated()) { const refreshOptions = { - video: videoFromDatabase, + video: videoFromDatabase as MVideoThumbnail, fetchedType: fetchType, syncParam } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index c14184b35..a027c4840 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -160,6 +160,9 @@ const videosCustomGetValidator = ( if (areValidationErrors(req, res)) return if (!await doesVideoExist(req.params.id, res, fetchType)) return + // Controllers does not need to check video rights + if (fetchType === 'only-immutable-attributes') return next() + const video = getVideoWithAttributes(res) const videoAll = video as MVideoFullLight diff --git a/server/models/model-cache.ts b/server/models/model-cache.ts index 8afe3834f..a87f99aa2 100644 --- a/server/models/model-cache.ts +++ b/server/models/model-cache.ts @@ -6,7 +6,8 @@ type ModelCacheType = 'local-account-name' | 'local-actor-name' | 'local-actor-url' - | 'video-immutable' + | 'load-video-immutable-id' + | 'load-video-immutable-url' type DeleteKey = 'video' @@ -19,7 +20,8 @@ class ModelCache { 'local-account-name': new Map(), 'local-actor-name': new Map(), 'local-actor-url': new Map(), - 'video-immutable': new Map() + 'load-video-immutable-id': new Map(), + 'load-video-immutable-url': new Map() } private readonly deleteIds: { diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 9e02d163f..5964526a9 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -145,6 +145,7 @@ export enum ScopeNames { WITH_USER_HISTORY = 'WITH_USER_HISTORY', WITH_STREAMING_PLAYLISTS = 'WITH_STREAMING_PLAYLISTS', WITH_USER_ID = 'WITH_USER_ID', + WITH_IMMUTABLE_ATTRIBUTES = 'WITH_IMMUTABLE_ATTRIBUTES', WITH_THUMBNAILS = 'WITH_THUMBNAILS' } @@ -188,6 +189,9 @@ export type AvailableForListIDsOptions = { } @Scopes(() => ({ + [ScopeNames.WITH_IMMUTABLE_ATTRIBUTES]: { + attributes: [ 'id', 'url', 'uuid', 'remote' ] + }, [ScopeNames.FOR_API]: (options: ForAPIOptions) => { const query: FindOptions = { include: [ @@ -1476,20 +1480,16 @@ export class VideoModel extends Model { static loadImmutableAttributes (id: number | string, t?: Transaction): Bluebird { const fun = () => { - const where = buildWhereIdOrUUID(id) - const options = { - attributes: [ - 'id', 'url', 'uuid' - ], - where, + const query = { + where: buildWhereIdOrUUID(id), transaction: t } - return VideoModel.unscoped().findOne(options) + return VideoModel.scope(ScopeNames.WITH_IMMUTABLE_ATTRIBUTES).findOne(query) } return ModelCache.Instance.doCache({ - cacheType: 'video-immutable', + cacheType: 'load-video-immutable-id', key: '' + id, deleteKey: 'video', fun @@ -1559,6 +1559,26 @@ export class VideoModel extends Model { return VideoModel.scope(ScopeNames.WITH_THUMBNAILS).findOne(query) } + static loadByUrlImmutableAttributes (url: string, transaction?: Transaction): Bluebird { + const fun = () => { + const query: FindOptions = { + where: { + url + }, + transaction + } + + return VideoModel.scope(ScopeNames.WITH_IMMUTABLE_ATTRIBUTES).findOne(query) + } + + return ModelCache.Instance.doCache({ + cacheType: 'load-video-immutable-url', + key: url, + deleteKey: 'video', + fun + }) + } + static loadByUrlAndPopulateAccount (url: string, transaction?: Transaction): Bluebird { const query: FindOptions = { where: { diff --git a/server/typings/models/video/video.ts b/server/typings/models/video/video.ts index 3ebb5a762..022a9566d 100644 --- a/server/typings/models/video/video.ts +++ b/server/typings/models/video/video.ts @@ -37,7 +37,7 @@ export type MVideoId = Pick export type MVideoUrl = Pick export type MVideoUUID = Pick -export type MVideoImmutable = Pick +export type MVideoImmutable = Pick export type MVideoIdUrl = MVideoId & MVideoUrl export type MVideoFeed = Pick -- 2.25.1