Video blacklist refractoring

12 files changed, 161 insertions, 190 deletions

diff --git a/server/controllers/api/blacklist.ts b/server/controllers/api/blacklist.ts
deleted file mode 100644
index 9b2d8017e..000000000
--- a/server/controllers/api/blacklist.ts
+++ /dev/null
@@ -1,60 +0,0 @@
-import * as express from 'express'
-
-import { database } from '../../initializers'
-import { getFormattedObjects } from '../../helpers'
-import { BlacklistedVideo } from '../../../shared'
-import { BlacklistedVideoInstance } from '../../models'
-
-import {
-  removeVideoFromBlacklist
-} from '../../lib'
-import {
-  authenticate,
-  ensureIsAdmin,
-  paginationValidator,
-  blacklistSortValidator,
-  setBlacklistSort,
-  setPagination,
-  blacklistRemoveValidator
-} from '../../middlewares'
-
-const blacklistRouter = express.Router()
-
-blacklistRouter.get('/',
-  authenticate,
-  ensureIsAdmin,
-  paginationValidator,
-  blacklistSortValidator,
-  setBlacklistSort,
-  setPagination,
-  listBlacklist
-)
-
-blacklistRouter.delete('/:id',
-  authenticate,
-  ensureIsAdmin,
-  blacklistRemoveValidator,
-  removeVideoFromBlacklistController
-)
-
-// ---------------------------------------------------------------------------
-
-export {
-  blacklistRouter
-}
-
-// ---------------------------------------------------------------------------
-
-function listBlacklist (req: express.Request, res: express.Response, next: express.NextFunction) {
-  database.BlacklistedVideo.listForApi(req.query.start, req.query.count, req.query.sort)
-    .then(resultList => res.json(getFormattedObjects<BlacklistedVideo, BlacklistedVideoInstance>(resultList.data, resultList.total)))
-    .catch(err => next(err))
-}
-
-function removeVideoFromBlacklistController (req: express.Request, res: express.Response, next: express.NextFunction) {
-  const entry = res.locals.blacklistEntryToRemove as BlacklistedVideoInstance
-
-  removeVideoFromBlacklist(entry)
-    .then(() => res.sendStatus(204))
-    .catch(err => next(err))
-}
diff --git a/server/controllers/api/index.ts b/server/controllers/api/index.ts
index fdc887915..a9205b33c 100644
--- a/server/controllers/api/index.ts
+++ b/server/controllers/api/index.ts
@@ -9,7 +9,6 @@ import { remoteRouter } from './remote'
 import { requestSchedulerRouter } from './request-schedulers'
 import { usersRouter } from './users'
 import { videosRouter } from './videos'
-import { blacklistRouter } from './blacklist'
 
 const apiRouter = express.Router()
 
@@ -20,7 +19,6 @@ apiRouter.use('/remote', remoteRouter)
 apiRouter.use('/request-schedulers', requestSchedulerRouter)
 apiRouter.use('/users', usersRouter)
 apiRouter.use('/videos', videosRouter)
-apiRouter.use('/blacklist', blacklistRouter)
 apiRouter.use('/ping', pong)
 apiRouter.use('/*', badRequest)
 
diff --git a/server/controllers/api/videos/blacklist.ts b/server/controllers/api/videos/blacklist.ts
index d8f2068ec..66311598e 100644
--- a/server/controllers/api/videos/blacklist.ts
+++ b/server/controllers/api/videos/blacklist.ts
@@ -1,22 +1,46 @@
 import * as express from 'express'
 
-import { database as db } from '../../../initializers/database'
-import { logger } from '../../../helpers'
+import { database as db } from '../../../initializers'
+import { logger, getFormattedObjects } from '../../../helpers'
 import {
   authenticate,
   ensureIsAdmin,
-  videosBlacklistValidator
+  videosBlacklistAddValidator,
+  videosBlacklistRemoveValidator,
+  paginationValidator,
+  blacklistSortValidator,
+  setBlacklistSort,
+  setPagination
 } from '../../../middlewares'
+import { BlacklistedVideoInstance } from '../../../models'
+import { BlacklistedVideo } from '../../../../shared'
 
 const blacklistRouter = express.Router()
 
-blacklistRouter.post('/:id/blacklist',
+blacklistRouter.post('/:videoId/blacklist',
   authenticate,
   ensureIsAdmin,
-  videosBlacklistValidator,
+  videosBlacklistAddValidator,
   addVideoToBlacklist
 )
 
+blacklistRouter.get('/blacklist',
+  authenticate,
+  ensureIsAdmin,
+  paginationValidator,
+  blacklistSortValidator,
+  setBlacklistSort,
+  setPagination,
+  listBlacklist
+)
+
+blacklistRouter.delete('/:videoId/blacklist',
+  authenticate,
+  ensureIsAdmin,
+  videosBlacklistRemoveValidator,
+  removeVideoFromBlacklistController
+)
+
 // ---------------------------------------------------------------------------
 
 export {
@@ -39,3 +63,23 @@ function addVideoToBlacklist (req: express.Request, res: express.Response, next:
       return next(err)
     })
 }
+
+function listBlacklist (req: express.Request, res: express.Response, next: express.NextFunction) {
+  db.BlacklistedVideo.listForApi(req.query.start, req.query.count, req.query.sort)
+    .then(resultList => res.json(getFormattedObjects<BlacklistedVideo, BlacklistedVideoInstance>(resultList.data, resultList.total)))
+    .catch(err => next(err))
+}
+
+function removeVideoFromBlacklistController (req: express.Request, res: express.Response, next: express.NextFunction) {
+  const blacklistedVideo = res.locals.blacklistedVideo as BlacklistedVideoInstance
+
+  blacklistedVideo.destroy()
+    .then(() => {
+      logger.info('Video %s removed from blacklist.', res.locals.video.uuid)
+      res.sendStatus(204)
+    })
+    .catch(err => {
+      logger.error('Some error while removing video %s from blacklist.', res.locals.video.uuid, err)
+      next(err)
+    })
+}
diff --git a/server/helpers/custom-validators/videos.ts b/server/helpers/custom-validators/videos.ts
index a31aca019..05d1dc607 100644
--- a/server/helpers/custom-validators/videos.ts
+++ b/server/helpers/custom-validators/videos.ts
@@ -1,5 +1,7 @@
 import { values } from 'lodash'
 import * as validator from 'validator'
+import * as Promise from 'bluebird'
+import * as express from 'express'
 import 'express-validator'
 import 'multer'
 
@@ -8,10 +10,13 @@ import {
   VIDEO_CATEGORIES,
   VIDEO_LICENCES,
   VIDEO_LANGUAGES,
-  VIDEO_RATE_TYPES
+  VIDEO_RATE_TYPES,
+  database as db
 } from '../../initializers'
 import { isUserUsernameValid } from './users'
 import { isArray, exists } from './misc'
+import { VideoInstance } from '../../models'
+import { logger } from '../../helpers'
 import { VideoRateType } from '../../../shared'
 
 const VIDEOS_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.VIDEOS
@@ -138,6 +143,30 @@ function isVideoFileInfoHashValid (value: string) {
   return exists(value) && validator.isLength(value, VIDEOS_CONSTRAINTS_FIELDS.INFO_HASH)
 }
 
+function checkVideoExists (id: string, res: express.Response, callback: () => void) {
+  let promise: Promise<VideoInstance>
+  if (validator.isInt(id)) {
+    promise = db.Video.loadAndPopulateAuthorAndPodAndTags(+id)
+  } else { // UUID
+    promise = db.Video.loadByUUIDAndPopulateAuthorAndPodAndTags(id)
+  }
+
+  promise.then(video => {
+    if (!video) {
+      return res.status(404)
+                .json({ error: 'Video not found' })
+                .end()
+    }
+
+    res.locals.video = video
+    callback()
+  })
+  .catch(err => {
+    logger.error('Error in video request validator.', err)
+    return res.sendStatus(500)
+  })
+}
+
 // ---------------------------------------------------------------------------
 
 export {
@@ -166,5 +195,6 @@ export {
   isVideoDislikesValid,
   isVideoEventCountValid,
   isVideoFileSizeValid,
-  isVideoFileResolutionValid
+  isVideoFileResolutionValid,
+  checkVideoExists
 }
diff --git a/server/lib/blacklist.ts b/server/lib/blacklist.ts
deleted file mode 100644
index dcf8aa03c..000000000
--- a/server/lib/blacklist.ts
+++ /dev/null
@@ -1,20 +0,0 @@
-import { logger } from '../helpers'
-import { BlacklistedVideoInstance } from '../models'
-
-function removeVideoFromBlacklist (entry: BlacklistedVideoInstance) {
-  return entry.destroy()
-    .then(() => {
-      logger.info('Video removed from the blacklist')
-    })
-    .catch(err => {
-      logger.error('Some error while removing video from the blacklist.', err)
-    })
-}
-
-// ---------------------------------------------------------------------------
-
-export {
-  removeVideoFromBlacklist
-}
-
-// ---------------------------------------------------------------------------
diff --git a/server/lib/index.ts b/server/lib/index.ts
index df781f29f..8628da4dd 100644
--- a/server/lib/index.ts
+++ b/server/lib/index.ts
@@ -3,4 +3,3 @@ export * from './jobs'
 export * from './request'
 export * from './friends'
 export * from './oauth-model'
-export * from './blacklist'
diff --git a/server/middlewares/validators/blacklist.ts b/server/middlewares/validators/blacklist.ts
deleted file mode 100644
index fe8fa40a4..000000000
--- a/server/middlewares/validators/blacklist.ts
+++ /dev/null
@@ -1,35 +0,0 @@
-import { param } from 'express-validator/check'
-import * as express from 'express'
-
-import { database as db } from '../../initializers/database'
-import { checkErrors } from './utils'
-import { logger } from '../../helpers'
-
-const blacklistRemoveValidator = [
-  param('id').isNumeric().not().isEmpty().withMessage('Should have a valid id'),
-
-  (req: express.Request, res: express.Response, next: express.NextFunction) => {
-    logger.debug('Checking blacklistRemove parameters.', { parameters: req.params })
-
-    checkErrors(req, res, () => {
-      db.BlacklistedVideo.loadById(req.params.id)
-        .then(entry => {
-          if (!entry) return res.status(404).send('Blacklisted video not found')
-
-          res.locals.blacklistEntryToRemove = entry
-
-          next()
-        })
-        .catch(err => {
-          logger.error('Error in blacklistRemove request validator', { error: err })
-          return res.sendStatus(500)
-        })
-    })
-  }
-]
-
-// ---------------------------------------------------------------------------
-
-export {
-  blacklistRemoveValidator
-}
diff --git a/server/middlewares/validators/index.ts b/server/middlewares/validators/index.ts
index a6198e22c..418fa5f1d 100644
--- a/server/middlewares/validators/index.ts
+++ b/server/middlewares/validators/index.ts
@@ -4,4 +4,4 @@ export * from './pods'
 export * from './sort'
 export * from './users'
 export * from './videos'
-export * from './blacklist'
+export * from './video-blacklist'
diff --git a/server/middlewares/validators/video-blacklist.ts b/server/middlewares/validators/video-blacklist.ts
new file mode 100644
index 000000000..30c6d4bd9
--- /dev/null
+++ b/server/middlewares/validators/video-blacklist.ts
@@ -0,0 +1,67 @@
+import { param } from 'express-validator/check'
+import * as express from 'express'
+
+import { database as db } from '../../initializers/database'
+import { checkErrors } from './utils'
+import { logger, isVideoIdOrUUIDValid, checkVideoExists } from '../../helpers'
+
+const videosBlacklistRemoveValidator = [
+  param('videoId').custom(isVideoIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid videoId'),
+
+  (req: express.Request, res: express.Response, next: express.NextFunction) => {
+    logger.debug('Checking blacklistRemove parameters.', { parameters: req.params })
+
+    checkErrors(req, res, () => {
+      checkVideoExists(req.params.videoId, res, () => {
+        checkVideoIsBlacklisted(req, res, next)
+      })
+    })
+  }
+]
+
+const videosBlacklistAddValidator = [
+  param('videoId').custom(isVideoIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid videoId'),
+
+  (req: express.Request, res: express.Response, next: express.NextFunction) => {
+    logger.debug('Checking videosBlacklist parameters', { parameters: req.params })
+
+    checkErrors(req, res, () => {
+      checkVideoExists(req.params.videoId, res, () => {
+        checkVideoIsBlacklistable(req, res, next)
+      })
+    })
+  }
+]
+
+// ---------------------------------------------------------------------------
+
+export {
+  videosBlacklistAddValidator,
+  videosBlacklistRemoveValidator
+}
+// ---------------------------------------------------------------------------
+
+function checkVideoIsBlacklistable (req: express.Request, res: express.Response, callback: () => void) {
+  if (res.locals.video.isOwned() === true) {
+    return res.status(403)
+              .json({ error: 'Cannot blacklist a local video' })
+              .end()
+  }
+
+  callback()
+}
+
+function checkVideoIsBlacklisted (req: express.Request, res: express.Response, callback: () => void) {
+  db.BlacklistedVideo.loadByVideoId(res.locals.video.id)
+    .then(blacklistedVideo => {
+      if (!blacklistedVideo) return res.status(404).send('Blacklisted video not found')
+
+      res.locals.blacklistedVideo = blacklistedVideo
+
+      callback()
+    })
+    .catch(err => {
+      logger.error('Error in blacklistRemove request validator', { error: err })
+      return res.sendStatus(500)
+    })
+}
diff --git a/server/middlewares/validators/videos.ts b/server/middlewares/validators/videos.ts
index 5f213f974..deed07524 100644
--- a/server/middlewares/validators/videos.ts
+++ b/server/middlewares/validators/videos.ts
@@ -1,7 +1,5 @@
 import { body, param, query } from 'express-validator/check'
 import * as express from 'express'
-import * as Promise from 'bluebird'
-import * as validator from 'validator'
 
 import { database as db } from '../../initializers/database'
 import { checkErrors } from './utils'
@@ -20,9 +18,9 @@ import {
   isVideoIdOrUUIDValid,
   isVideoAbuseReasonValid,
   isVideoRatingTypeValid,
-  getDurationFromVideoFile
+  getDurationFromVideoFile,
+  checkVideoExists
 } from '../../helpers'
-import { VideoInstance } from '../../models'
 
 const videosAddValidator = [
   body('videofile').custom((value, { req }) => isVideoFile(req.files)).withMessage('Should have a valid file'),
@@ -186,20 +184,6 @@ const videoRateValidator = [
   }
 ]
 
-const videosBlacklistValidator = [
-  param('id').custom(isVideoIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid id'),
-
-  (req: express.Request, res: express.Response, next: express.NextFunction) => {
-    logger.debug('Checking videosBlacklist parameters', { parameters: req.params })
-
-    checkErrors(req, res, () => {
-      checkVideoExists(req.params.id, res, () => {
-        checkVideoIsBlacklistable(req, res, next)
-      })
-    })
-  }
-]
-
 // ---------------------------------------------------------------------------
 
 export {
@@ -211,37 +195,11 @@ export {
 
   videoAbuseReportValidator,
 
-  videoRateValidator,
-
-  videosBlacklistValidator
+  videoRateValidator
 }
 
 // ---------------------------------------------------------------------------
 
-function checkVideoExists (id: string, res: express.Response, callback: () => void) {
-  let promise: Promise<VideoInstance>
-  if (validator.isInt(id)) {
-    promise = db.Video.loadAndPopulateAuthorAndPodAndTags(+id)
-  } else { // UUID
-    promise = db.Video.loadByUUIDAndPopulateAuthorAndPodAndTags(id)
-  }
-
-  promise.then(video => {
-    if (!video) {
-      return res.status(404)
-                .json({ error: 'Video not found' })
-                .end()
-    }
-
-    res.locals.video = video
-    callback()
-  })
-  .catch(err => {
-    logger.error('Error in video request validator.', err)
-    return res.sendStatus(500)
-  })
-}
-
 function checkUserCanDeleteVideo (userId: number, res: express.Response, callback: () => void) {
   // Retrieve the user who did the request
   db.User.loadById(userId)
@@ -269,13 +227,3 @@ function checkUserCanDeleteVideo (userId: number, res: express.Response, callbac
       return res.sendStatus(500)
     })
 }
-
-function checkVideoIsBlacklistable (req: express.Request, res: express.Response, callback: () => void) {
-  if (res.locals.video.isOwned() === true) {
-    return res.status(403)
-              .json({ error: 'Cannot blacklist a local video' })
-              .end()
-  }
-
-  callback()
-}
diff --git a/server/tests/api/check-params/video-blacklist.ts b/server/tests/api/check-params/video-blacklist.ts
index 80e6f8011..eb16b3af0 100644
--- a/server/tests/api/check-params/video-blacklist.ts
+++ b/server/tests/api/check-params/video-blacklist.ts
@@ -81,10 +81,10 @@ describe('Test video blacklist API validators', function () {
   })
 
   describe('When removing a video in blacklist', function () {
-    const basePath = '/api/v1/blacklist/'
+    const basePath = '/api/v1/videos/'
 
     it('Should fail with a non authenticated user', async function () {
-      const path = basePath + server.video.id
+      const path = basePath + server.video.id + '/blacklist'
 
       await request(server.url)
               .delete(path)
@@ -94,7 +94,7 @@ describe('Test video blacklist API validators', function () {
     })
 
     it('Should fail with a non admin user', async function () {
-      const path = basePath + server.video.id
+      const path = basePath + server.video.id + '/blacklist'
 
       await request(server.url)
               .delete(path)
@@ -104,7 +104,7 @@ describe('Test video blacklist API validators', function () {
     })
 
     it('Should fail with an incorrect id', async function () {
-      const path = basePath + 'foobar'
+      const path = basePath + 'foobar/blacklist'
 
       await request(server.url)
               .delete(path)
@@ -115,7 +115,7 @@ describe('Test video blacklist API validators', function () {
 
     it('Should fail with a not blacklisted video', async function () {
       // The video was not added to the blacklist so it should fail
-      const path = basePath + server.video.id
+      const path = basePath + server.video.id + '/blacklist'
 
       await request(server.url)
               .delete(path)
@@ -126,7 +126,7 @@ describe('Test video blacklist API validators', function () {
   })
 
   describe('When listing videos in blacklist', function () {
-    const basePath = '/api/v1/blacklist/'
+    const basePath = '/api/v1/videos/blacklist/'
 
     it('Should fail with a non authenticated user', async function () {
       const path = basePath
diff --git a/server/tests/utils/video-blacklist.ts b/server/tests/utils/video-blacklist.ts
index 5729d13d8..3a499f46a 100644
--- a/server/tests/utils/video-blacklist.ts
+++ b/server/tests/utils/video-blacklist.ts
@@ -11,7 +11,7 @@ function addVideoToBlacklist (url: string, token: string, videoId: number, speci
 }
 
 function removeVideoFromBlacklist (url: string, token: string, videoId: number, specialStatus = 204) {
-  const path = '/api/v1/blacklist/' + videoId
+  const path = '/api/v1/videos/' + videoId + '/blacklist'
 
   return request(url)
           .delete(path)
@@ -21,7 +21,7 @@ function removeVideoFromBlacklist (url: string, token: string, videoId: number,
 }
 
 function getBlacklistedVideosList (url: string, token: string, specialStatus = 200) {
-  const path = '/api/v1/blacklist/'
+  const path = '/api/v1/videos/blacklist/'
 
   return request(url)
           .get(path)
@@ -33,7 +33,7 @@ function getBlacklistedVideosList (url: string, token: string, specialStatus = 2
 }
 
 function getSortedBlacklistedVideosList (url: string, token: string, sort: string, specialStatus = 200) {
-  const path = '/api/v1/blacklist/'
+  const path = '/api/v1/videos/blacklist/'
 
   return request(url)
           .get(path)