Add hook filters tests

18 files changed, 306 insertions, 80 deletions

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 <U, T> = (params: U) => Promise<T> | Bluebird<T>
+type RawFunction <U, T> = (params: U) => T
+
 // Helpers to run hooks
 const Hooks = {
-  wrapObject: <T, U extends ServerFilterHookName>(obj: T, hookName: U) => {
-    return PluginManager.Instance.runHook(hookName, obj) as Promise<T>
+  wrapObject: <T, U extends ServerFilterHookName>(result: T, hookName: U) => {
+    return PluginManager.Instance.runHook(hookName, result) as Promise<T>
+  },
+
+  wrapPromiseFun: async <U, T, V extends ServerFilterHookName>(fun: PromiseFunction<U, T>, params: U, hookName: V) => {
+    const result = await fun(params)
+
+    return PluginManager.Instance.runHook(hookName, result, params)
   },
 
-  wrapPromise: async <T, U extends ServerFilterHookName>(fun: Promise<T> | Bluebird<T>, hookName: U) => {
-    const result = await fun
+  wrapFun: async <U, T, V extends ServerFilterHookName>(fun: RawFunction<U, T>, params: U, hookName: V) => {
+    const result = fun(params)
 
-    return PluginManager.Instance.runHook(hookName, result)
+    return PluginManager.Instance.runHook(hookName, result, params)
   },
 
   runAction: <T, U extends ServerActionHookName>(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 <T> (hookName: ServerHookName, result?: T, params?: any): Promise<T> {
+    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<VideoModel> {
       .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 <T>(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`