From 89cd12756035a146bbcc4db78cd3cd64f2f3d88d Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 19 Jul 2019 17:30:41 +0200 Subject: [PATCH] Add hook filters tests --- server.ts | 3 +- server/controllers/activitypub/client.ts | 2 +- server/controllers/api/videos/comment.ts | 10 +- server/controllers/api/videos/index.ts | 10 +- server/lib/plugins/hooks.ts | 19 +++- server/lib/plugins/plugin-manager.ts | 10 +- server/lib/video-blacklist.ts | 5 +- .../middlewares/validators/videos/videos.ts | 5 +- server/models/video/video.ts | 7 +- server/tests/cli/plugins.ts | 8 +- .../fixtures/peertube-plugin-test-two/main.js | 21 +++++ .../peertube-plugin-test-two/package.json | 19 ++++ .../fixtures/peertube-plugin-test/main.js | 62 +++++++++---- server/tests/plugins/action-hooks.ts | 91 +++++++++++++++++-- server/tests/plugins/filter-hooks.ts | 69 ++++++++++++-- shared/core-utils/plugins/hooks.ts | 35 ++++--- shared/extra-utils/server/plugins.ts | 7 +- shared/extra-utils/server/servers.ts | 3 +- 18 files changed, 306 insertions(+), 80 deletions(-) create mode 100644 server/tests/fixtures/peertube-plugin-test-two/main.js create mode 100644 server/tests/fixtures/peertube-plugin-test-two/package.json diff --git a/server.ts b/server.ts index b75c78b07..abfeeed2e 100644 --- a/server.ts +++ b/server.ts @@ -114,6 +114,7 @@ import { isHTTPSignatureDigestValid } from './server/helpers/peertube-crypto' import { PeerTubeSocket } from './server/lib/peertube-socket' import { updateStreamingPlaylistsInfohashesIfNeeded } from './server/lib/hls' import { PluginsCheckScheduler } from './server/lib/schedulers/plugins-check-scheduler' +import { Hooks } from './server/lib/plugins/hooks' // ----------- Command line ----------- @@ -269,7 +270,7 @@ async function startApplication () { logger.info('Server listening on %s:%d', hostname, port) logger.info('Web server: %s', WEBSERVER.URL) - PluginManager.Instance.runHook('action:application.listening') + Hooks.runAction('action:application.listening') }) process.on('exit', () => { diff --git a/server/controllers/activitypub/client.ts b/server/controllers/activitypub/client.ts index d36d10de1..11504b354 100644 --- a/server/controllers/activitypub/client.ts +++ b/server/controllers/activitypub/client.ts @@ -208,7 +208,7 @@ function getAccountVideoRate (rateType: VideoRateType) { async function videoController (req: express.Request, res: express.Response) { // We need more attributes - const video = await VideoModel.loadForGetAPI(res.locals.video.id) + const video = await VideoModel.loadForGetAPI({ id: res.locals.video.id }) if (video.url.startsWith(WEBSERVER.URL) === false) return res.redirect(video.url) diff --git a/server/controllers/api/videos/comment.ts b/server/controllers/api/videos/comment.ts index a95392543..feda71bdd 100644 --- a/server/controllers/api/videos/comment.ts +++ b/server/controllers/api/videos/comment.ts @@ -85,8 +85,9 @@ async function listVideoThreads (req: express.Request, res: express.Response) { user: user }, 'filter:api.video-threads.list.params') - resultList = await Hooks.wrapPromise( - VideoCommentModel.listThreadsForApi(apiOptions), + resultList = await Hooks.wrapPromiseFun( + VideoCommentModel.listThreadsForApi, + apiOptions, 'filter:api.video-threads.list.result' ) } else { @@ -112,8 +113,9 @@ async function listVideoThreadComments (req: express.Request, res: express.Respo user: user }, 'filter:api.video-thread-comments.list.params') - resultList = await Hooks.wrapPromise( - VideoCommentModel.listThreadCommentsForApi(apiOptions), + resultList = await Hooks.wrapPromiseFun( + VideoCommentModel.listThreadCommentsForApi, + apiOptions, 'filter:api.video-thread-comments.list.result' ) } else { diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts index a3b1dde29..11e468df2 100644 --- a/server/controllers/api/videos/index.ts +++ b/server/controllers/api/videos/index.ts @@ -436,8 +436,9 @@ async function getVideo (req: express.Request, res: express.Response) { // We need more attributes const userId: number = res.locals.oauth ? res.locals.oauth.token.User.id : null - const video = await Hooks.wrapPromise( - VideoModel.loadForGetAPI(res.locals.video.id, undefined, userId), + const video = await Hooks.wrapPromiseFun( + VideoModel.loadForGetAPI, + { id: res.locals.video.id, userId }, 'filter:api.video.get.result' ) @@ -502,8 +503,9 @@ async function listVideos (req: express.Request, res: express.Response) { user: res.locals.oauth ? res.locals.oauth.token.User : undefined }, 'filter:api.videos.list.params') - const resultList = await Hooks.wrapPromise( - VideoModel.listForApi(apiOptions), + const resultList = await Hooks.wrapPromiseFun( + VideoModel.listForApi, + apiOptions, 'filter:api.videos.list.result' ) diff --git a/server/lib/plugins/hooks.ts b/server/lib/plugins/hooks.ts index 7bb907e6a..b694d4118 100644 --- a/server/lib/plugins/hooks.ts +++ b/server/lib/plugins/hooks.ts @@ -3,16 +3,25 @@ import { PluginManager } from './plugin-manager' import { logger } from '../../helpers/logger' import * as Bluebird from 'bluebird' +type PromiseFunction = (params: U) => Promise | Bluebird +type RawFunction = (params: U) => T + // Helpers to run hooks const Hooks = { - wrapObject: (obj: T, hookName: U) => { - return PluginManager.Instance.runHook(hookName, obj) as Promise + wrapObject: (result: T, hookName: U) => { + return PluginManager.Instance.runHook(hookName, result) as Promise + }, + + wrapPromiseFun: async (fun: PromiseFunction, params: U, hookName: V) => { + const result = await fun(params) + + return PluginManager.Instance.runHook(hookName, result, params) }, - wrapPromise: async (fun: Promise | Bluebird, hookName: U) => { - const result = await fun + wrapFun: async (fun: RawFunction, params: U, hookName: V) => { + const result = fun(params) - return PluginManager.Instance.runHook(hookName, result) + return PluginManager.Instance.runHook(hookName, result, params) }, runAction: (hookName: U, params?: T) => { diff --git a/server/lib/plugins/plugin-manager.ts b/server/lib/plugins/plugin-manager.ts index 9afda97ea..6485a47c5 100644 --- a/server/lib/plugins/plugin-manager.ts +++ b/server/lib/plugins/plugin-manager.ts @@ -98,15 +98,15 @@ export class PluginManager implements ServerHook { // ###################### Hooks ###################### - async runHook (hookName: ServerHookName, param?: any) { - let result = param - - if (!this.hooks[hookName]) return result + async runHook (hookName: ServerHookName, result?: T, params?: any): Promise { + if (!this.hooks[hookName]) return Promise.resolve(result) const hookType = getHookType(hookName) for (const hook of this.hooks[hookName]) { - result = await internalRunHook(hook.handler, hookType, param, err => { + logger.debug('Running hook %s of plugin %s.', hookName, hook.npmName) + + result = await internalRunHook(hook.handler, hookType, result, params, err => { logger.error('Cannot run hook %s of plugin %s.', hookName, hook.pluginName, { err }) }) } diff --git a/server/lib/video-blacklist.ts b/server/lib/video-blacklist.ts index 32b1a28fa..9bc996f5a 100644 --- a/server/lib/video-blacklist.ts +++ b/server/lib/video-blacklist.ts @@ -9,8 +9,9 @@ import { UserAdminFlag } from '../../shared/models/users/user-flag.model' import { Hooks } from './plugins/hooks' async function autoBlacklistVideoIfNeeded (video: VideoModel, user?: UserModel, transaction?: Transaction) { - const doAutoBlacklist = await Hooks.wrapPromise( - autoBlacklistNeeded({ video, user }), + const doAutoBlacklist = await Hooks.wrapPromiseFun( + autoBlacklistNeeded, + { video, user }, 'filter:video.auto-blacklist.result' ) diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index cb2c071ba..5593ede64 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -444,8 +444,9 @@ async function isVideoAccepted (req: express.Request, res: express.Response, vid videoFile, user: res.locals.oauth.token.User } - const acceptedResult = await Hooks.wrapObject( - isLocalVideoAccepted(acceptParameters), + const acceptedResult = await Hooks.wrapFun( + isLocalVideoAccepted, + acceptParameters, 'filter:api.video.upload.accept.result' ) diff --git a/server/models/video/video.ts b/server/models/video/video.ts index ec3d5ddb0..443aec9c2 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1472,7 +1472,12 @@ export class VideoModel extends Model { .findOne(options) } - static loadForGetAPI (id: number | string, t?: Transaction, userId?: number) { + static loadForGetAPI (parameters: { + id: number | string, + t?: Transaction, + userId?: number + }) { + const { id, t, userId } = parameters const where = buildWhereIdOrUUID(id) const options = { diff --git a/server/tests/cli/plugins.ts b/server/tests/cli/plugins.ts index d7bf8a690..a5257d671 100644 --- a/server/tests/cli/plugins.ts +++ b/server/tests/cli/plugins.ts @@ -6,13 +6,13 @@ import { execCLI, flushAndRunServer, getConfig, - getEnvCli, killallServers, + getEnvCli, + getPluginTestPath, + killallServers, reRunServer, - root, ServerInfo, setAccessTokensToServers } from '../../../shared/extra-utils' -import { join } from 'path' import { ServerConfig } from '../../../shared/models/server' import { expect } from 'chai' @@ -29,7 +29,7 @@ describe('Test plugin scripts', function () { it('Should install a plugin from stateless CLI', async function () { this.timeout(60000) - const packagePath = join(root(), 'server', 'tests', 'fixtures', 'peertube-plugin-test') + const packagePath = getPluginTestPath() const env = getEnvCli(server) await execCLI(`${env} npm run plugin:install -- --plugin-path ${packagePath}`) diff --git a/server/tests/fixtures/peertube-plugin-test-two/main.js b/server/tests/fixtures/peertube-plugin-test-two/main.js new file mode 100644 index 000000000..71c11b2ba --- /dev/null +++ b/server/tests/fixtures/peertube-plugin-test-two/main.js @@ -0,0 +1,21 @@ +async function register ({ registerHook, registerSetting, settingsManager, storageManager, peertubeHelpers }) { + registerHook({ + target: 'filter:api.videos.list.params', + handler: obj => addToCount(obj) + }) +} + +async function unregister () { + return +} + +module.exports = { + register, + unregister +} + +// ############################################################################ + +function addToCount (obj) { + return Object.assign({}, obj, { count: obj.count + 1 }) +} diff --git a/server/tests/fixtures/peertube-plugin-test-two/package.json b/server/tests/fixtures/peertube-plugin-test-two/package.json new file mode 100644 index 000000000..52ebb5ac1 --- /dev/null +++ b/server/tests/fixtures/peertube-plugin-test-two/package.json @@ -0,0 +1,19 @@ +{ + "name": "peertube-plugin-test-two", + "version": "0.0.1", + "description": "Plugin test 2", + "engine": { + "peertube": ">=1.3.0" + }, + "keywords": [ + "peertube", + "plugin" + ], + "homepage": "https://github.com/Chocobozzz/PeerTube", + "author": "Chocobozzz", + "bugs": "https://github.com/Chocobozzz/PeerTube/issues", + "library": "./main.js", + "staticDirs": {}, + "css": [], + "clientScripts": [] +} diff --git a/server/tests/fixtures/peertube-plugin-test/main.js b/server/tests/fixtures/peertube-plugin-test/main.js index fae0ef948..c5317ab41 100644 --- a/server/tests/fixtures/peertube-plugin-test/main.js +++ b/server/tests/fixtures/peertube-plugin-test/main.js @@ -1,23 +1,52 @@ -async function register ({ registerHook, registerSetting, settingsManager, storageManager }) { - const defaultAdmin = 'PeerTube admin' +async function register ({ registerHook, registerSetting, settingsManager, storageManager, peertubeHelpers }) { + const actionHooks = [ + 'action:application.listening', + + 'action:api.video.updated', + 'action:api.video.deleted', + 'action:api.video.uploaded', + 'action:api.video.viewed', + + 'action:api.video-thread.created', + 'action:api.video-comment-reply.created', + 'action:api.video-comment.deleted' + ] + + for (const h of actionHooks) { + registerHook({ + target: h, + handler: () => peertubeHelpers.logger.debug('Run hook %s.', h) + }) + } registerHook({ - target: 'action:application.listening', - handler: () => displayHelloWorld(settingsManager, defaultAdmin) + target: 'filter:api.videos.list.params', + handler: obj => addToCount(obj) }) - registerSetting({ - name: 'admin-name', - label: 'Admin name', - type: 'input', - default: defaultAdmin + registerHook({ + target: 'filter:api.videos.list.result', + handler: obj => ({ data: obj.data, total: obj.total + 1 }) }) - const value = await storageManager.getData('toto') - console.log(value) - console.log(value.coucou) + registerHook({ + target: 'filter:api.video.get.result', + handler: video => { + video.name += ' <3' - await storageManager.storeData('toto', { coucou: 'hello' + new Date() }) + return video + } + }) + + registerHook({ + target: 'filter:api.video.upload.accept.result', + handler: ({ accepted }, { videoBody }) => { + if (accepted !== false) return { accepted: true } + if (videoBody.name.indexOf('bad word') !== -1) return { accepted: false, errorMessage: 'bad word '} + + return { accepted: true } + } + }) } async function unregister () { @@ -31,9 +60,6 @@ module.exports = { // ############################################################################ -async function displayHelloWorld (settingsManager, defaultAdmin) { - let value = await settingsManager.getSetting('admin-name') - if (!value) value = defaultAdmin - - console.log('hello world ' + value) +function addToCount (obj) { + return Object.assign({}, obj, { count: obj.count + 1 }) } diff --git a/server/tests/plugins/action-hooks.ts b/server/tests/plugins/action-hooks.ts index 93dc57d09..2a941148a 100644 --- a/server/tests/plugins/action-hooks.ts +++ b/server/tests/plugins/action-hooks.ts @@ -2,26 +2,101 @@ import * as chai from 'chai' import 'mocha' -import { cleanupTests, flushAndRunServer, ServerInfo } from '../../../shared/extra-utils/server/servers' -import { setAccessTokensToServers } from '../../../shared/extra-utils' +import { + cleanupTests, + flushAndRunMultipleServers, + flushAndRunServer, killallServers, reRunServer, + ServerInfo, + waitUntilLog +} from '../../../shared/extra-utils/server/servers' +import { + addVideoCommentReply, + addVideoCommentThread, deleteVideoComment, + getPluginTestPath, + installPlugin, removeVideo, + setAccessTokensToServers, + updateVideo, + uploadVideo, + viewVideo +} from '../../../shared/extra-utils' const expect = chai.expect describe('Test plugin action hooks', function () { - let server: ServerInfo + let servers: ServerInfo[] + let videoUUID: string + let threadId: number + + function checkHook (hook: string) { + return waitUntilLog(servers[0], 'Run hook ' + hook) + } before(async function () { this.timeout(30000) - server = await flushAndRunServer(1) - await setAccessTokensToServers([ server ]) + servers = await flushAndRunMultipleServers(2) + await setAccessTokensToServers(servers) + + await installPlugin({ + url: servers[0].url, + accessToken: servers[0].accessToken, + path: getPluginTestPath() + }) + + await killallServers([ servers[0] ]) + + await reRunServer(servers[0]) }) - it('Should execute ', async function () { - // empty + it('Should run action:application.listening', async function () { + await checkHook('action:application.listening') + }) + + it('Should run action:api.video.uploaded', async function () { + const res = await uploadVideo(servers[0].url, servers[0].accessToken, { name: 'video' }) + videoUUID = res.body.video.uuid + + await checkHook('action:api.video.uploaded') + }) + + it('Should run action:api.video.updated', async function () { + await updateVideo(servers[0].url, servers[0].accessToken, videoUUID, { name: 'video updated' }) + + await checkHook('action:api.video.updated') + }) + + it('Should run action:api.video.viewed', async function () { + await viewVideo(servers[0].url, videoUUID) + + await checkHook('action:api.video.viewed') + }) + + it('Should run action:api.video-thread.created', async function () { + const res = await addVideoCommentThread(servers[0].url, servers[0].accessToken, videoUUID, 'thread') + threadId = res.body.comment.id + + await checkHook('action:api.video-thread.created') + }) + + it('Should run action:api.video-comment-reply.created', async function () { + await addVideoCommentReply(servers[0].url, servers[0].accessToken, videoUUID, threadId, 'reply') + + await checkHook('action:api.video-comment-reply.created') + }) + + it('Should run action:api.video-comment.deleted', async function () { + await deleteVideoComment(servers[0].url, servers[0].accessToken, videoUUID, threadId) + + await checkHook('action:api.video-comment.deleted') + }) + + it('Should run action:api.video.deleted', async function () { + await removeVideo(servers[0].url, servers[0].accessToken, videoUUID) + + await checkHook('action:api.video.deleted') }) after(async function () { - await cleanupTests([ server ]) + await cleanupTests(servers) }) }) diff --git a/server/tests/plugins/filter-hooks.ts b/server/tests/plugins/filter-hooks.ts index 500728712..4fc2c437b 100644 --- a/server/tests/plugins/filter-hooks.ts +++ b/server/tests/plugins/filter-hooks.ts @@ -2,26 +2,79 @@ import * as chai from 'chai' import 'mocha' -import { cleanupTests, flushAndRunServer, ServerInfo } from '../../../shared/extra-utils/server/servers' -import { setAccessTokensToServers } from '../../../shared/extra-utils' +import { + cleanupTests, + flushAndRunMultipleServers, + flushAndRunServer, killallServers, reRunServer, + ServerInfo, + waitUntilLog +} from '../../../shared/extra-utils/server/servers' +import { + addVideoCommentReply, + addVideoCommentThread, deleteVideoComment, + getPluginTestPath, getVideosList, + installPlugin, removeVideo, + setAccessTokensToServers, + updateVideo, + uploadVideo, + viewVideo, + getVideosListPagination, getVideo +} from '../../../shared/extra-utils' const expect = chai.expect describe('Test plugin filter hooks', function () { - let server: ServerInfo + let servers: ServerInfo[] + let videoUUID: string + let threadId: number before(async function () { this.timeout(30000) - server = await flushAndRunServer(1) - await setAccessTokensToServers([ server ]) + servers = await flushAndRunMultipleServers(2) + await setAccessTokensToServers(servers) + + await installPlugin({ + url: servers[0].url, + accessToken: servers[0].accessToken, + path: getPluginTestPath() + }) + + await installPlugin({ + url: servers[0].url, + accessToken: servers[0].accessToken, + path: getPluginTestPath('-two') + }) + + for (let i = 0; i < 10; i++) { + await uploadVideo(servers[0].url, servers[0].accessToken, { name: 'default video ' + i }) + } + + const res = await getVideosList(servers[0].url) + videoUUID = res.body.data[0].uuid }) - it('Should execute ', async function () { - // empty + it('Should run filter:api.videos.list.params hook', async function () { + const res = await getVideosListPagination(servers[0].url, 0, 2) + + // 2 plugins do +1 to the count parameter + expect(res.body.data).to.have.lengthOf(4) + }) + + it('Should run filter:api.videos.list.result', async function () { + const res = await getVideosListPagination(servers[0].url, 0, 0) + + // Plugin do +1 to the total result + expect(res.body.total).to.equal(11) + }) + + it('Should run filter:api.video.get.result', async function () { + const res = await getVideo(servers[0].url, videoUUID) + + expect(res.body.name).to.contain('<3') }) after(async function () { - await cleanupTests([ server ]) + await cleanupTests(servers) }) }) diff --git a/shared/core-utils/plugins/hooks.ts b/shared/core-utils/plugins/hooks.ts index 047c04f7b..60f4130f5 100644 --- a/shared/core-utils/plugins/hooks.ts +++ b/shared/core-utils/plugins/hooks.ts @@ -8,25 +8,30 @@ function getHookType (hookName: string) { return HookType.STATIC } -async function internalRunHook (handler: Function, hookType: HookType, param: any, onError: (err: Error) => void) { - let result = param - +async function internalRunHook (handler: Function, hookType: HookType, result: T, params: any, onError: (err: Error) => void) { try { - const p = handler(result) + if (hookType === HookType.FILTER) { + const p = handler(result, params) + + if (isPromise(p)) result = await p + else result = p + + return result + } - switch (hookType) { - case HookType.FILTER: - if (isPromise(p)) result = await p - else result = p - break + // Action/static hooks do not have result value + const p = handler(params) + + if (hookType === HookType.STATIC) { + if (isPromise(p)) await p + + return undefined + } - case HookType.STATIC: - if (isPromise(p)) await p - break + if (hookType === HookType.ACTION) { + if (isCatchable(p)) p.catch(err => onError(err)) - case HookType.ACTION: - if (isCatchable(p)) p.catch(err => onError(err)) - break + return undefined } } catch (err) { onError(err) diff --git a/shared/extra-utils/server/plugins.ts b/shared/extra-utils/server/plugins.ts index 7a5c5344b..2302208a8 100644 --- a/shared/extra-utils/server/plugins.ts +++ b/shared/extra-utils/server/plugins.ts @@ -201,6 +201,10 @@ function getPluginPackageJSON (server: ServerInfo, npmName: string) { return readJSON(path) } +function getPluginTestPath (suffix = '') { + return join(root(), 'server', 'tests', 'fixtures', 'peertube-plugin-test' + suffix) +} + export { listPlugins, listAvailablePlugins, @@ -213,5 +217,6 @@ export { getPluginRegisteredSettings, getPackageJSONPath, updatePluginPackageJSON, - getPluginPackageJSON + getPluginPackageJSON, + getPluginTestPath } diff --git a/shared/extra-utils/server/servers.ts b/shared/extra-utils/server/servers.ts index 9167ebe5b..40cf7f0f3 100644 --- a/shared/extra-utils/server/servers.ts +++ b/shared/extra-utils/server/servers.ts @@ -171,7 +171,8 @@ async function runServer (server: ServerInfo, configOverrideArg?: any, args = [] thumbnails: `test${server.internalServerNumber}/thumbnails/`, torrents: `test${server.internalServerNumber}/torrents/`, captions: `test${server.internalServerNumber}/captions/`, - cache: `test${server.internalServerNumber}/cache/` + cache: `test${server.internalServerNumber}/cache/`, + plugins: `test${server.internalServerNumber}/plugins/` }, admin: { email: `admin${server.internalServerNumber}@example.com` -- 2.25.1