From 35bf0c83c80f59ca79f4b84fac8700f17adeb22d Mon Sep 17 00:00:00 2001
From: Chocobozzz <florian.bigard@gmail.com>
Date: Tue, 10 Oct 2017 10:02:18 +0200
Subject: Video blacklist refractoring
---
server/controllers/api/blacklist.ts | 60 ---------------------
server/controllers/api/index.ts | 2 -
server/controllers/api/videos/blacklist.ts | 54 +++++++++++++++++--
server/helpers/custom-validators/videos.ts | 34 +++++++++++-
server/lib/blacklist.ts | 20 -------
server/lib/index.ts | 1 -
server/middlewares/validators/blacklist.ts | 35 -------------
server/middlewares/validators/index.ts | 2 +-
server/middlewares/validators/video-blacklist.ts | 67 ++++++++++++++++++++++++
server/middlewares/validators/videos.ts | 58 ++------------------
server/tests/api/check-params/video-blacklist.ts | 12 ++---
server/tests/utils/video-blacklist.ts | 6 +--
12 files changed, 161 insertions(+), 190 deletions(-)
delete mode 100644 server/controllers/api/blacklist.ts
delete mode 100644 server/lib/blacklist.ts
delete mode 100644 server/middlewares/validators/blacklist.ts
create mode 100644 server/middlewares/validators/video-blacklist.ts
(limited to 'server')
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)
--
cgit v1.2.3