From 5b77537ce54832f47931ba47dc513be2a9197f92 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 23 Jul 2019 12:04:15 +0200 Subject: [PATCH] Correctly notify on auto blacklist --- scripts/optimize-old-videos.ts | 22 +++++++++++++------ server/controllers/api/videos/blacklist.ts | 3 ++- server/controllers/api/videos/import.ts | 9 +++++++- server/controllers/api/videos/index.ts | 19 ++++++---------- .../custom-validators/video-channels.ts | 17 +------------- .../activitypub/process/process-announce.ts | 2 +- .../lib/activitypub/process/process-create.ts | 4 ++-- server/lib/activitypub/videos.ts | 13 ++++++----- server/lib/job-queue/handlers/video-import.ts | 4 ++-- .../job-queue/handlers/video-transcoding.ts | 4 ++-- server/lib/notifier.ts | 4 ++-- .../lib/schedulers/update-videos-scheduler.ts | 2 +- server/lib/video-blacklist.ts | 7 ++++-- server/models/video/video.ts | 4 ++++ server/tests/cli/optimize-old-videos.ts | 2 +- 15 files changed, 61 insertions(+), 55 deletions(-) diff --git a/scripts/optimize-old-videos.ts b/scripts/optimize-old-videos.ts index a1d5345a1..2c80f16bc 100644 --- a/scripts/optimize-old-videos.ts +++ b/scripts/optimize-old-videos.ts @@ -32,6 +32,7 @@ async function run () { for (const video of localVideos) { currentVideoId = video.id + for (const file of video.VideoFiles) { currentFile = join(CONFIG.STORAGE.VIDEOS_DIR, video.getVideoFilename(file)) @@ -44,22 +45,29 @@ async function run () { const maxBitrate = getMaxBitrate(resolution.videoFileResolution, fps, VIDEO_TRANSCODING_FPS) const isMaxBitrateExceeded = videoBitrate > maxBitrate if (isMaxBitrateExceeded) { - console.log('Optimizing video file %s with bitrate %s kbps (max: %s kbps)', - basename(currentFile), videoBitrate / 1000, maxBitrate / 1000) + console.log( + 'Optimizing video file %s with bitrate %s kbps (max: %s kbps)', + basename(currentFile), videoBitrate / 1000, maxBitrate / 1000 + ) + const backupFile = `${currentFile}_backup` await copy(currentFile, backupFile) + await optimizeVideofile(video, file) + const originalDuration = await getDurationFromVideoFile(backupFile) const newDuration = await getDurationFromVideoFile(currentFile) + if (originalDuration === newDuration) { console.log('Finished optimizing %s', basename(currentFile)) await remove(backupFile) - } else { - console.log('Failed to optimize %s, restoring original', basename(currentFile)) - move(backupFile, currentFile, { overwrite: true }) - await video.createTorrentAndSetInfoHash(file) - await file.save() + return } + + console.log('Failed to optimize %s, restoring original', basename(currentFile)) + await move(backupFile, currentFile, { overwrite: true }) + await video.createTorrentAndSetInfoHash(file) + await file.save() } } } diff --git a/server/controllers/api/videos/blacklist.ts b/server/controllers/api/videos/blacklist.ts index 0ec518e0d..9ff494def 100644 --- a/server/controllers/api/videos/blacklist.ts +++ b/server/controllers/api/videos/blacklist.ts @@ -115,6 +115,7 @@ async function removeVideoFromBlacklistController (req: express.Request, res: ex const videoBlacklistType = videoBlacklist.type await videoBlacklist.destroy({ transaction: t }) + video.VideoBlacklist = undefined // Re federate the video if (unfederated === true) { @@ -131,7 +132,7 @@ async function removeVideoFromBlacklistController (req: express.Request, res: ex // Delete on object so new video notifications will send delete video.VideoBlacklist - Notifier.Instance.notifyOnNewVideo(video) + Notifier.Instance.notifyOnNewVideoIfNeeded(video) } logger.info('Video %s removed from blacklist.', res.locals.video.uuid) diff --git a/server/controllers/api/videos/import.ts b/server/controllers/api/videos/import.ts index 47c6f122c..1f08fe20a 100644 --- a/server/controllers/api/videos/import.ts +++ b/server/controllers/api/videos/import.ts @@ -245,7 +245,14 @@ function insertIntoDB (parameters: { if (thumbnailModel) await videoCreated.addAndSaveThumbnail(thumbnailModel, t) if (previewModel) await videoCreated.addAndSaveThumbnail(previewModel, t) - await autoBlacklistVideoIfNeeded({ video, user, isRemote: false, isNew: true, transaction: t }) + await autoBlacklistVideoIfNeeded({ + video, + user, + notify: false, + isRemote: false, + isNew: true, + transaction: t + }) // Set tags to the video if (tags) { diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts index 6a79a16c7..973bf1123 100644 --- a/server/controllers/api/videos/index.ts +++ b/server/controllers/api/videos/index.ts @@ -235,7 +235,7 @@ async function addVideo (req: express.Request, res: express.Response) { // Create the torrent file await video.createTorrentAndSetInfoHash(videoFile) - const { videoCreated, videoWasAutoBlacklisted } = await sequelizeTypescript.transaction(async t => { + const { videoCreated } = await sequelizeTypescript.transaction(async t => { const sequelizeOptions = { transaction: t } const videoCreated = await video.save(sequelizeOptions) @@ -268,23 +268,22 @@ async function addVideo (req: express.Request, res: express.Response) { }, { transaction: t }) } - const videoWasAutoBlacklisted = await autoBlacklistVideoIfNeeded({ + await autoBlacklistVideoIfNeeded({ video, user: res.locals.oauth.token.User, isRemote: false, isNew: true, transaction: t }) - if (!videoWasAutoBlacklisted) await federateVideoIfNeeded(video, true, t) + await federateVideoIfNeeded(video, true, t) auditLogger.create(getAuditIdFromRes(res), new VideoAuditView(videoCreated.toFormattedDetailsJSON())) logger.info('Video with name %s and uuid %s created.', videoInfo.name, videoCreated.uuid) - return { videoCreated, videoWasAutoBlacklisted } + return { videoCreated } }) - if (videoWasAutoBlacklisted) Notifier.Instance.notifyOnVideoAutoBlacklist(videoCreated) - else Notifier.Instance.notifyOnNewVideo(videoCreated) + Notifier.Instance.notifyOnNewVideoIfNeeded(videoCreated) if (video.state === VideoState.TO_TRANSCODE) { // Put uuid because we don't have id auto incremented for now @@ -413,11 +412,7 @@ async function updateVideo (req: express.Request, res: express.Response) { }) const isNewVideo = wasPrivateVideo && videoInstanceUpdated.privacy !== VideoPrivacy.PRIVATE - - // Don't send update if the video was unfederated - if (!videoInstanceUpdated.VideoBlacklist || videoInstanceUpdated.VideoBlacklist.unfederated === false) { - await federateVideoIfNeeded(videoInstanceUpdated, isNewVideo, t) - } + await federateVideoIfNeeded(videoInstanceUpdated, isNewVideo, t) auditLogger.update( getAuditIdFromRes(res), @@ -430,7 +425,7 @@ async function updateVideo (req: express.Request, res: express.Response) { }) if (wasUnlistedVideo || wasPrivateVideo) { - Notifier.Instance.notifyOnNewVideo(videoInstanceUpdated) + Notifier.Instance.notifyOnNewVideoIfNeeded(videoInstanceUpdated) } Hooks.runAction('action:api.video.updated', { video: videoInstanceUpdated }) diff --git a/server/helpers/custom-validators/video-channels.ts b/server/helpers/custom-validators/video-channels.ts index 0471f6ec4..f55f0c8ef 100644 --- a/server/helpers/custom-validators/video-channels.ts +++ b/server/helpers/custom-validators/video-channels.ts @@ -1,9 +1,7 @@ -import * as express from 'express' import 'express-validator' import 'multer' import * as validator from 'validator' import { CONSTRAINTS_FIELDS } from '../../initializers/constants' -import { VideoChannelModel } from '../../models/video/video-channel' import { exists } from './misc' const VIDEO_CHANNELS_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.VIDEO_CHANNELS @@ -25,18 +23,5 @@ function isVideoChannelSupportValid (value: string) { export { isVideoChannelDescriptionValid, isVideoChannelNameValid, - isVideoChannelSupportValid, -} - -function processVideoChannelExist (videoChannel: VideoChannelModel, res: express.Response) { - if (!videoChannel) { - res.status(404) - .json({ error: 'Video channel not found' }) - .end() - - return false - } - - res.locals.videoChannel = videoChannel - return true + isVideoChannelSupportValid } diff --git a/server/lib/activitypub/process/process-announce.ts b/server/lib/activitypub/process/process-announce.ts index bbf1bd3a8..1fe347506 100644 --- a/server/lib/activitypub/process/process-announce.ts +++ b/server/lib/activitypub/process/process-announce.ts @@ -63,5 +63,5 @@ async function processVideoShare (actorAnnouncer: ActorModel, activity: Activity return undefined }) - if (videoCreated) Notifier.Instance.notifyOnNewVideo(video) + if (videoCreated) Notifier.Instance.notifyOnNewVideoIfNeeded(video) } diff --git a/server/lib/activitypub/process/process-create.ts b/server/lib/activitypub/process/process-create.ts index b9f584aa5..1e893cdeb 100644 --- a/server/lib/activitypub/process/process-create.ts +++ b/server/lib/activitypub/process/process-create.ts @@ -48,9 +48,9 @@ export { async function processCreateVideo (activity: ActivityCreate) { const videoToCreateData = activity.object as VideoTorrentObject - const { video, created, autoBlacklisted } = await getOrCreateVideoAndAccountAndChannel({ videoObject: videoToCreateData }) + const { video, created } = await getOrCreateVideoAndAccountAndChannel({ videoObject: videoToCreateData }) - if (created && !autoBlacklisted) Notifier.Instance.notifyOnNewVideo(video) + if (created) Notifier.Instance.notifyOnNewVideoIfNeeded(video) return video } diff --git a/server/lib/activitypub/videos.ts b/server/lib/activitypub/videos.ts index 67b433165..d7bc3d650 100644 --- a/server/lib/activitypub/videos.ts +++ b/server/lib/activitypub/videos.ts @@ -58,8 +58,12 @@ import { Hooks } from '../plugins/hooks' import { autoBlacklistVideoIfNeeded } from '../video-blacklist' async function federateVideoIfNeeded (video: VideoModel, isNewVideo: boolean, transaction?: sequelize.Transaction) { - // If the video is not private and is published, we federate it - if (video.privacy !== VideoPrivacy.PRIVATE && video.state === VideoState.PUBLISHED) { + if ( + // Check this is not a blacklisted video, or unfederated blacklisted video + (video.isBlacklisted() === false || (isNewVideo === false && video.VideoBlacklist.unfederated === false)) && + // Check the video is public/unlisted and published + video.privacy !== VideoPrivacy.PRIVATE && video.state === VideoState.PUBLISHED + ) { // Fetch more attributes that we will need to serialize in AP object if (isArray(video.VideoCaptions) === false) { video.VideoCaptions = await video.$get('VideoCaptions', { @@ -354,7 +358,7 @@ async function updateVideoFromAP (options: { } }) - const autoBlacklisted = await autoBlacklistVideoIfNeeded({ + await autoBlacklistVideoIfNeeded({ video, user: undefined, isRemote: true, @@ -362,8 +366,7 @@ async function updateVideoFromAP (options: { transaction: undefined }) - if (autoBlacklisted) Notifier.Instance.notifyOnVideoAutoBlacklist(video) - else if (!wasPrivateVideo || wasUnlistedVideo) Notifier.Instance.notifyOnNewVideo(video) // Notify our users? + if (wasPrivateVideo || wasUnlistedVideo) Notifier.Instance.notifyOnNewVideoIfNeeded(video) // Notify our users? logger.info('Remote video with uuid %s updated', videoObject.uuid) } catch (err) { diff --git a/server/lib/job-queue/handlers/video-import.ts b/server/lib/job-queue/handlers/video-import.ts index 50e159245..13b741180 100644 --- a/server/lib/job-queue/handlers/video-import.ts +++ b/server/lib/job-queue/handlers/video-import.ts @@ -199,10 +199,10 @@ async function processFile (downloader: () => Promise, videoImport: Vide Notifier.Instance.notifyOnFinishedVideoImport(videoImportUpdated, true) - if (videoImportUpdated.Video.VideoBlacklist) { + if (videoImportUpdated.Video.isBlacklisted()) { Notifier.Instance.notifyOnVideoAutoBlacklist(videoImportUpdated.Video) } else { - Notifier.Instance.notifyOnNewVideo(videoImportUpdated.Video) + Notifier.Instance.notifyOnNewVideoIfNeeded(videoImportUpdated.Video) } // Create transcoding jobs? diff --git a/server/lib/job-queue/handlers/video-transcoding.ts b/server/lib/job-queue/handlers/video-transcoding.ts index e9b84ecd6..981daf9a1 100644 --- a/server/lib/job-queue/handlers/video-transcoding.ts +++ b/server/lib/job-queue/handlers/video-transcoding.ts @@ -112,7 +112,7 @@ async function publishNewResolutionIfNeeded (video: VideoModel, payload?: NewRes }) if (videoPublished) { - Notifier.Instance.notifyOnNewVideo(videoDatabase) + Notifier.Instance.notifyOnNewVideoIfNeeded(videoDatabase) Notifier.Instance.notifyOnVideoPublishedAfterTranscoding(videoDatabase) } @@ -172,7 +172,7 @@ async function onVideoFileOptimizerSuccess (videoArg: VideoModel, payload: Optim return { videoDatabase, videoPublished } }) - if (payload.isNewVideo) Notifier.Instance.notifyOnNewVideo(videoDatabase) + if (payload.isNewVideo) Notifier.Instance.notifyOnNewVideoIfNeeded(videoDatabase) if (videoPublished) Notifier.Instance.notifyOnVideoPublishedAfterTranscoding(videoDatabase) const hlsPayload = Object.assign({}, payload, { resolution: videoDatabase.getOriginalFile().resolution }) diff --git a/server/lib/notifier.ts b/server/lib/notifier.ts index c1e63fa8f..a7dfb0979 100644 --- a/server/lib/notifier.ts +++ b/server/lib/notifier.ts @@ -22,9 +22,9 @@ class Notifier { private constructor () {} - notifyOnNewVideo (video: VideoModel): void { + notifyOnNewVideoIfNeeded (video: VideoModel): void { // Only notify on public and published videos which are not blacklisted - if (video.privacy !== VideoPrivacy.PUBLIC || video.state !== VideoState.PUBLISHED || video.VideoBlacklist) return + if (video.privacy !== VideoPrivacy.PUBLIC || video.state !== VideoState.PUBLISHED || video.isBlacklisted()) return this.notifySubscribersOfNewVideo(video) .catch(err => logger.error('Cannot notify subscribers of new video %s.', video.url, { err })) diff --git a/server/lib/schedulers/update-videos-scheduler.ts b/server/lib/schedulers/update-videos-scheduler.ts index 80080a132..5b673b913 100644 --- a/server/lib/schedulers/update-videos-scheduler.ts +++ b/server/lib/schedulers/update-videos-scheduler.ts @@ -57,7 +57,7 @@ export class UpdateVideosScheduler extends AbstractScheduler { }) for (const v of publishedVideos) { - Notifier.Instance.notifyOnNewVideo(v) + Notifier.Instance.notifyOnNewVideoIfNeeded(v) Notifier.Instance.notifyOnVideoPublishedAfterScheduledUpdate(v) } } diff --git a/server/lib/video-blacklist.ts b/server/lib/video-blacklist.ts index 1dd7139f7..bdaecd8e2 100644 --- a/server/lib/video-blacklist.ts +++ b/server/lib/video-blacklist.ts @@ -7,15 +7,17 @@ import { VideoModel } from '../models/video/video' import { logger } from '../helpers/logger' import { UserAdminFlag } from '../../shared/models/users/user-flag.model' import { Hooks } from './plugins/hooks' +import { Notifier } from './notifier' async function autoBlacklistVideoIfNeeded (parameters: { video: VideoModel, user?: UserModel, isRemote: boolean, isNew: boolean, + notify?: boolean, transaction?: Transaction }) { - const { video, user, isRemote, isNew, transaction } = parameters + const { video, user, isRemote, isNew, notify = true, transaction } = parameters const doAutoBlacklist = await Hooks.wrapPromiseFun( autoBlacklistNeeded, { video, user, isRemote, isNew }, @@ -37,9 +39,10 @@ async function autoBlacklistVideoIfNeeded (parameters: { defaults: videoBlacklistToCreate, transaction }) - video.VideoBlacklist = videoBlacklist + if (notify) Notifier.Instance.notifyOnVideoAutoBlacklist(video) + logger.info('Video %s auto-blacklisted.', video.uuid) return true diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 443aec9c2..c7f2658ed 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1715,6 +1715,10 @@ export class VideoModel extends Model { return VIDEO_STATES[ id ] || 'Unknown' } + isBlacklisted () { + return !!this.VideoBlacklist + } + getOriginalFile () { if (Array.isArray(this.VideoFiles) === false) return undefined diff --git a/server/tests/cli/optimize-old-videos.ts b/server/tests/cli/optimize-old-videos.ts index 3822fca42..fa82f962c 100644 --- a/server/tests/cli/optimize-old-videos.ts +++ b/server/tests/cli/optimize-old-videos.ts @@ -75,7 +75,7 @@ describe('Test optimize old videos', function () { }) it('Should run optimize script', async function () { - this.timeout(120000) + this.timeout(200000) const env = getEnvCli(servers[0]) await execCLI(`${env} npm run optimize-old-videos`) -- 2.25.1