From 9901c8d6908a43ab4594406446eac770ee21176c Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 25 Jul 2023 15:17:58 +0200 Subject: [PATCH] Add video file update hook tests --- .../videos/shared/video-validators.ts | 8 ++--- .../fixtures/peertube-plugin-test/main.js | 12 ++++++++ server/tests/plugins/action-hooks.ts | 8 +++++ server/tests/plugins/filter-hooks.ts | 19 +++++++++++- .../server-commands/videos/videos-command.ts | 30 +++++++++++++------ 5 files changed, 61 insertions(+), 16 deletions(-) diff --git a/server/middlewares/validators/videos/shared/video-validators.ts b/server/middlewares/validators/videos/shared/video-validators.ts index 72536011d..95e4fef11 100644 --- a/server/middlewares/validators/videos/shared/video-validators.ts +++ b/server/middlewares/validators/videos/shared/video-validators.ts @@ -45,7 +45,7 @@ export async function isVideoFileAccepted (options: { videoFile: express.VideoUploadFile hook: Extract }) { - const { req, res, videoFile } = options + const { req, res, videoFile, hook } = options // Check we accept this video const acceptParameters = { @@ -53,11 +53,7 @@ export async function isVideoFileAccepted (options: { videoFile, user: res.locals.oauth.token.User } - const acceptedResult = await Hooks.wrapFun( - isLocalVideoFileAccepted, - acceptParameters, - 'filter:api.video.upload.accept.result' - ) + const acceptedResult = await Hooks.wrapFun(isLocalVideoFileAccepted, acceptParameters, hook) if (!acceptedResult || acceptedResult.accepted !== true) { logger.info('Refused local video file.', { acceptedResult, acceptParameters }) diff --git a/server/tests/fixtures/peertube-plugin-test/main.js b/server/tests/fixtures/peertube-plugin-test/main.js index 17032f6d9..e16bf0ca3 100644 --- a/server/tests/fixtures/peertube-plugin-test/main.js +++ b/server/tests/fixtures/peertube-plugin-test/main.js @@ -9,6 +9,8 @@ async function register ({ registerHook, registerSetting, settingsManager, stora 'action:api.video.uploaded', 'action:api.video.viewed', + 'action:api.video.file-updated', + 'action:api.video-channel.created', 'action:api.video-channel.updated', 'action:api.video-channel.deleted', @@ -160,6 +162,16 @@ async function register ({ registerHook, registerSetting, settingsManager, stora }) } + registerHook({ + target: 'filter:api.video.update-file.accept.result', + handler: ({ accepted }, { videoFile }) => { + if (!accepted) return { accepted: false } + if (videoFile.filename.includes('webm')) return { accepted: false, errorMessage: 'no webm' } + + return { accepted: true } + } + }) + registerHook({ target: 'filter:api.video.pre-import-url.accept.result', handler: ({ accepted }, { videoImportBody }) => { diff --git a/server/tests/plugins/action-hooks.ts b/server/tests/plugins/action-hooks.ts index 34b4e1891..773be0d76 100644 --- a/server/tests/plugins/action-hooks.ts +++ b/server/tests/plugins/action-hooks.ts @@ -40,6 +40,8 @@ describe('Test plugin action hooks', function () { } }) + await servers[0].config.enableFileUpdate() + await doubleFollow(servers[0], servers[1]) }) @@ -70,6 +72,12 @@ describe('Test plugin action hooks', function () { await checkHook('action:api.video.viewed') }) + it('Should run action:api.video.file-updated', async function () { + await servers[0].videos.replaceSourceFile({ videoId: videoUUID, fixture: 'video_short.mp4' }) + + await checkHook('action:api.video.file-updated') + }) + it('Should run action:api.video.deleted', async function () { await servers[0].videos.remove({ id: videoUUID }) diff --git a/server/tests/plugins/filter-hooks.ts b/server/tests/plugins/filter-hooks.ts index a75a8c8fa..8382b400f 100644 --- a/server/tests/plugins/filter-hooks.ts +++ b/server/tests/plugins/filter-hooks.ts @@ -64,6 +64,11 @@ describe('Test plugin filter hooks', function () { newConfig: { live: { enabled: true }, signup: { enabled: true }, + videoFile: { + update: { + enabled: true + } + }, import: { videos: { http: { enabled: true }, @@ -178,7 +183,19 @@ describe('Test plugin filter hooks', function () { describe('Video/live/import accept', function () { it('Should run filter:api.video.upload.accept.result', async function () { - await servers[0].videos.upload({ attributes: { name: 'video with bad word' }, expectedStatus: HttpStatusCode.FORBIDDEN_403 }) + const options = { attributes: { name: 'video with bad word' }, expectedStatus: HttpStatusCode.FORBIDDEN_403 } + await servers[0].videos.upload({ mode: 'legacy', ...options }) + await servers[0].videos.upload({ mode: 'resumable', ...options }) + }) + + it('Should run filter:api.video.update-file.accept.result', async function () { + const res = await servers[0].videos.replaceSourceFile({ + videoId: videoUUID, + fixture: 'video_short1.webm', + completedExpectedStatus: HttpStatusCode.FORBIDDEN_403 + }) + + expect((res as any)?.error).to.equal('no webm') }) it('Should run filter:api.live-video.create.accept.result', async function () { diff --git a/shared/server-commands/videos/videos-command.ts b/shared/server-commands/videos/videos-command.ts index 3fdbc348a..a58f1c545 100644 --- a/shared/server-commands/videos/videos-command.ts +++ b/shared/server-commands/videos/videos-command.ts @@ -394,6 +394,7 @@ export class VideosCommand extends AbstractCommand { attributes?: VideoEdit mode?: 'legacy' | 'resumable' // default legacy waitTorrentGeneration?: boolean // default true + completedExpectedStatus?: HttpStatusCode } = {}) { const { mode = 'legacy', waitTorrentGeneration = true } = options let defaultChannelId = 1 @@ -461,8 +462,9 @@ export class VideosCommand extends AbstractCommand { async buildResumeUpload (options: OverrideCommandOptions & { path: string attributes: { fixture?: string } & { [id: string]: any } + completedExpectedStatus?: HttpStatusCode // When the upload is finished }): Promise { - const { path, attributes, expectedStatus = HttpStatusCode.OK_200 } = options + const { path, attributes, expectedStatus = HttpStatusCode.OK_200, completedExpectedStatus } = options let size = 0 let videoFilePath: string @@ -503,7 +505,8 @@ export class VideosCommand extends AbstractCommand { path, pathUploadId, videoFilePath, - size + size, + expectedStatus: completedExpectedStatus }) if (result.statusCode === HttpStatusCode.OK_200) { @@ -600,12 +603,14 @@ export class VideosCommand extends AbstractCommand { try { readable.pause() + const byterangeStart = start + chunk.length - 1 + const headers = { 'Authorization': 'Bearer ' + token, 'Content-Type': 'application/octet-stream', 'Content-Range': contentRangeBuilder ? contentRangeBuilder(start, chunk) - : `bytes ${start}-${start + chunk.length - 1}/${size}`, + : `bytes ${start}-${byterangeStart}/${size}`, 'Content-Length': contentLength ? contentLength + '' : chunk.length + '' } @@ -625,13 +630,19 @@ export class VideosCommand extends AbstractCommand { start += chunk.length - if (res.statusCode === expectedStatus) { - return resolve(res) - } + // Last request, check final status + if (byterangeStart + 1 === size) { + if (res.statusCode === expectedStatus) { + return resolve(res) + } - if (res.statusCode !== HttpStatusCode.PERMANENT_REDIRECT_308) { - readable.off('data', onData) - return reject(new Error('Incorrect transient behaviour sending intermediary chunks')) + if (res.statusCode !== HttpStatusCode.PERMANENT_REDIRECT_308) { + readable.off('data', onData) + + // eslint-disable-next-line max-len + const message = `Incorrect transient behaviour sending intermediary chunks. Status code is ${res.statusCode} instead of ${expectedStatus}` + return reject(new Error(message)) + } } readable.resume() @@ -694,6 +705,7 @@ export class VideosCommand extends AbstractCommand { replaceSourceFile (options: OverrideCommandOptions & { videoId: number | string fixture: string + completedExpectedStatus?: HttpStatusCode }) { return this.buildResumeUpload({ ...options,