From d87452277407ef6ab3a368d4b0434b8b80eb7a64 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 12 Jul 2023 11:09:29 +0200 Subject: [PATCH] Avoid update remote runner error --- server/controllers/api/runners/jobs-files.ts | 10 +-- server/controllers/api/runners/jobs.ts | 14 ++-- server/middlewares/validators/runners/jobs.ts | 74 ++++++++++--------- server/tests/api/check-params/runners.ts | 8 +- 4 files changed, 57 insertions(+), 49 deletions(-) diff --git a/server/controllers/api/runners/jobs-files.ts b/server/controllers/api/runners/jobs-files.ts index cb4eff570..d28f43701 100644 --- a/server/controllers/api/runners/jobs-files.ts +++ b/server/controllers/api/runners/jobs-files.ts @@ -4,12 +4,12 @@ import { proxifyHLS, proxifyWebVideoFile } from '@server/lib/object-storage' import { VideoPathManager } from '@server/lib/video-path-manager' import { getStudioTaskFilePath } from '@server/lib/video-studio' import { apiRateLimiter, asyncMiddleware } from '@server/middlewares' -import { jobOfRunnerGetValidator } from '@server/middlewares/validators/runners' +import { jobOfRunnerGetValidatorFactory } from '@server/middlewares/validators/runners' import { runnerJobGetVideoStudioTaskFileValidator, runnerJobGetVideoTranscodingFileValidator } from '@server/middlewares/validators/runners/job-files' -import { VideoStorage } from '@shared/models' +import { RunnerJobState, VideoStorage } from '@shared/models' const lTags = loggerTagsFactory('api', 'runner') @@ -17,21 +17,21 @@ const runnerJobFilesRouter = express.Router() runnerJobFilesRouter.post('/jobs/:jobUUID/files/videos/:videoId/max-quality', apiRateLimiter, - asyncMiddleware(jobOfRunnerGetValidator), + asyncMiddleware(jobOfRunnerGetValidatorFactory([ RunnerJobState.PROCESSING ])), asyncMiddleware(runnerJobGetVideoTranscodingFileValidator), asyncMiddleware(getMaxQualityVideoFile) ) runnerJobFilesRouter.post('/jobs/:jobUUID/files/videos/:videoId/previews/max-quality', apiRateLimiter, - asyncMiddleware(jobOfRunnerGetValidator), + asyncMiddleware(jobOfRunnerGetValidatorFactory([ RunnerJobState.PROCESSING ])), asyncMiddleware(runnerJobGetVideoTranscodingFileValidator), getMaxQualityVideoPreview ) runnerJobFilesRouter.post('/jobs/:jobUUID/files/videos/:videoId/studio/task-files/:filename', apiRateLimiter, - asyncMiddleware(jobOfRunnerGetValidator), + asyncMiddleware(jobOfRunnerGetValidatorFactory([ RunnerJobState.PROCESSING ])), asyncMiddleware(runnerJobGetVideoTranscodingFileValidator), runnerJobGetVideoStudioTaskFileValidator, getVideoStudioTaskFile diff --git a/server/controllers/api/runners/jobs.ts b/server/controllers/api/runners/jobs.ts index 5d687e689..be5911b53 100644 --- a/server/controllers/api/runners/jobs.ts +++ b/server/controllers/api/runners/jobs.ts @@ -22,7 +22,7 @@ import { cancelRunnerJobValidator, errorRunnerJobValidator, getRunnerFromTokenValidator, - jobOfRunnerGetValidator, + jobOfRunnerGetValidatorFactory, runnerJobGetValidator, successRunnerJobValidator, updateRunnerJobValidator @@ -85,7 +85,7 @@ runnerJobsRouter.post('/jobs/:jobUUID/accept', runnerJobsRouter.post('/jobs/:jobUUID/abort', apiRateLimiter, - asyncMiddleware(jobOfRunnerGetValidator), + asyncMiddleware(jobOfRunnerGetValidatorFactory([ RunnerJobState.PROCESSING ])), abortRunnerJobValidator, asyncMiddleware(abortRunnerJob) ) @@ -93,13 +93,13 @@ runnerJobsRouter.post('/jobs/:jobUUID/abort', runnerJobsRouter.post('/jobs/:jobUUID/update', runnerJobUpdateVideoFiles, apiRateLimiter, // Has to be after multer middleware to parse runner token - asyncMiddleware(jobOfRunnerGetValidator), + asyncMiddleware(jobOfRunnerGetValidatorFactory([ RunnerJobState.PROCESSING, RunnerJobState.COMPLETING, RunnerJobState.COMPLETED ])), updateRunnerJobValidator, asyncMiddleware(updateRunnerJobController) ) runnerJobsRouter.post('/jobs/:jobUUID/error', - asyncMiddleware(jobOfRunnerGetValidator), + asyncMiddleware(jobOfRunnerGetValidatorFactory([ RunnerJobState.PROCESSING ])), errorRunnerJobValidator, asyncMiddleware(errorRunnerJob) ) @@ -107,7 +107,7 @@ runnerJobsRouter.post('/jobs/:jobUUID/error', runnerJobsRouter.post('/jobs/:jobUUID/success', postRunnerJobSuccessVideoFiles, apiRateLimiter, // Has to be after multer middleware to parse runner token - asyncMiddleware(jobOfRunnerGetValidator), + asyncMiddleware(jobOfRunnerGetValidatorFactory([ RunnerJobState.PROCESSING ])), successRunnerJobValidator, asyncMiddleware(postRunnerJobSuccess) ) @@ -272,6 +272,10 @@ async function updateRunnerJobController (req: express.Request, res: express.Res const runner = runnerJob.Runner const body: RunnerJobUpdateBody = req.body + if (runnerJob.state === RunnerJobState.COMPLETING || runnerJob.state === RunnerJobState.COMPLETED) { + return res.sendStatus(HttpStatusCode.NO_CONTENT_204) + } + const payloadBuilder = jobUpdateBuilders[runnerJob.type] const updatePayload = payloadBuilder ? payloadBuilder(body.payload, req.files as UploadFiles) diff --git a/server/middlewares/validators/runners/jobs.ts b/server/middlewares/validators/runners/jobs.ts index 75cc0bdbb..384b209ba 100644 --- a/server/middlewares/validators/runners/jobs.ts +++ b/server/middlewares/validators/runners/jobs.ts @@ -159,44 +159,46 @@ export const runnerJobGetValidator = [ } ] -export const jobOfRunnerGetValidator = [ - param('jobUUID').custom(isUUIDValid), +export function jobOfRunnerGetValidatorFactory (allowedStates: RunnerJobState[]) { + return [ + param('jobUUID').custom(isUUIDValid), - body('runnerToken').custom(isRunnerTokenValid), - body('jobToken').custom(isRunnerJobTokenValid), + body('runnerToken').custom(isRunnerTokenValid), + body('jobToken').custom(isRunnerJobTokenValid), - async (req: express.Request, res: express.Response, next: express.NextFunction) => { - if (areValidationErrors(req, res, { tags })) return cleanUpReqFiles(req) + async (req: express.Request, res: express.Response, next: express.NextFunction) => { + if (areValidationErrors(req, res, { tags })) return cleanUpReqFiles(req) - const runnerJob = await RunnerJobModel.loadByRunnerAndJobTokensWithRunner({ - uuid: req.params.jobUUID, - runnerToken: req.body.runnerToken, - jobToken: req.body.jobToken - }) - - if (!runnerJob) { - cleanUpReqFiles(req) - - return res.fail({ - status: HttpStatusCode.NOT_FOUND_404, - message: 'Unknown runner job', - tags + const runnerJob = await RunnerJobModel.loadByRunnerAndJobTokensWithRunner({ + uuid: req.params.jobUUID, + runnerToken: req.body.runnerToken, + jobToken: req.body.jobToken }) + + if (!runnerJob) { + cleanUpReqFiles(req) + + return res.fail({ + status: HttpStatusCode.NOT_FOUND_404, + message: 'Unknown runner job', + tags + }) + } + + if (!allowedStates.includes(runnerJob.state)) { + cleanUpReqFiles(req) + + return res.fail({ + status: HttpStatusCode.BAD_REQUEST_400, + type: ServerErrorCode.RUNNER_JOB_NOT_IN_PROCESSING_STATE, + message: 'Job is not in "processing" state', + tags + }) + } + + res.locals.runnerJob = runnerJob + + return next() } - - if (runnerJob.state !== RunnerJobState.PROCESSING) { - cleanUpReqFiles(req) - - return res.fail({ - status: HttpStatusCode.BAD_REQUEST_400, - type: ServerErrorCode.RUNNER_JOB_NOT_IN_PROCESSING_STATE, - message: 'Job is not in "processing" state', - tags - }) - } - - res.locals.runnerJob = runnerJob - - return next() - } -] + ] +} diff --git a/server/tests/api/check-params/runners.ts b/server/tests/api/check-params/runners.ts index 7d70c412e..9112ff716 100644 --- a/server/tests/api/check-params/runners.ts +++ b/server/tests/api/check-params/runners.ts @@ -41,6 +41,7 @@ describe('Test managing runners', function () { let completedJobToken: string let completedJobUUID: string + let cancelledJobToken: string let cancelledJobUUID: string before(async function () { @@ -86,6 +87,7 @@ describe('Test managing runners', function () { { const { job } = await server.runnerJobs.autoAccept({ runnerToken }) + cancelledJobToken = job.jobToken cancelledJobUUID = job.uuid await server.runnerJobs.cancelByAdmin({ jobUUID: cancelledJobUUID }) } @@ -639,10 +641,10 @@ describe('Test managing runners', function () { it('Should fail with a job not in processing state', async function () { await server.runnerJobs.update({ - jobUUID: completedJobUUID, - jobToken: completedJobToken, + jobUUID: cancelledJobUUID, + jobToken: cancelledJobToken, runnerToken, - expectedStatus: HttpStatusCode.BAD_REQUEST_400 + expectedStatus: HttpStatusCode.NOT_FOUND_404 }) }) })