From 5b77537ce54832f47931ba47dc513be2a9197f92 Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Tue, 23 Jul 2019 12:04:15 +0200
Subject: Correctly notify on auto blacklist

---
 server/controllers/api/videos/blacklist.ts         |  3 ++-
 server/controllers/api/videos/import.ts            |  9 ++++++++-
 server/controllers/api/videos/index.ts             | 19 +++++++------------
 server/helpers/custom-validators/video-channels.ts | 17 +----------------
 server/lib/activitypub/process/process-announce.ts |  2 +-
 server/lib/activitypub/process/process-create.ts   |  4 ++--
 server/lib/activitypub/videos.ts                   | 13 ++++++++-----
 server/lib/job-queue/handlers/video-import.ts      |  4 ++--
 server/lib/job-queue/handlers/video-transcoding.ts |  4 ++--
 server/lib/notifier.ts                             |  4 ++--
 server/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 +-
 14 files changed, 46 insertions(+), 48 deletions(-)

(limited to 'server')

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<string>, 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<VideoModel> {
     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`)
-- 
cgit v1.2.3