diff --git a/package-lock.json b/package-lock.json index 3c9bcd2779..51150bc543 100644 --- a/package-lock.json +++ b/package-lock.json @@ -56942,6 +56942,7 @@ "@overleaf/redis-wrapper": "*", "@overleaf/settings": "*", "@overleaf/stream-utils": "*", + "@overleaf/validation-tools": "*", "async": "^3.2.5", "body-parser": "1.20.4", "bunyan": "^1.8.15", diff --git a/services/project-history/Dockerfile b/services/project-history/Dockerfile index 79a279d6ec..846fc51457 100644 --- a/services/project-history/Dockerfile +++ b/services/project-history/Dockerfile @@ -23,6 +23,7 @@ COPY libraries/promise-utils/package.json /overleaf/libraries/promise-utils/pack COPY libraries/redis-wrapper/package.json /overleaf/libraries/redis-wrapper/package.json COPY libraries/settings/package.json /overleaf/libraries/settings/package.json COPY libraries/stream-utils/package.json /overleaf/libraries/stream-utils/package.json +COPY libraries/validation-tools/package.json /overleaf/libraries/validation-tools/package.json COPY services/project-history/package.json /overleaf/services/project-history/package.json COPY tools/migrations/package.json /overleaf/tools/migrations/package.json COPY patches/ /overleaf/patches/ @@ -39,6 +40,7 @@ COPY libraries/promise-utils/ /overleaf/libraries/promise-utils/ COPY libraries/redis-wrapper/ /overleaf/libraries/redis-wrapper/ COPY libraries/settings/ /overleaf/libraries/settings/ COPY libraries/stream-utils/ /overleaf/libraries/stream-utils/ +COPY libraries/validation-tools/ /overleaf/libraries/validation-tools/ COPY services/project-history/ /overleaf/services/project-history/ COPY tools/migrations/ /overleaf/tools/migrations/ diff --git a/services/project-history/Makefile b/services/project-history/Makefile index e0590e5ae5..cf786c260c 100644 --- a/services/project-history/Makefile +++ b/services/project-history/Makefile @@ -25,6 +25,7 @@ IMAGE_CACHE ?= $(IMAGE_REPO):cache-$(shell cat \ $(MONOREPO)/libraries/redis-wrapper/package.json \ $(MONOREPO)/libraries/settings/package.json \ $(MONOREPO)/libraries/stream-utils/package.json \ + $(MONOREPO)/libraries/validation-tools/package.json \ $(MONOREPO)/services/project-history/package.json \ $(MONOREPO)/tools/migrations/package.json \ $(MONOREPO)/patches/* \ diff --git a/services/project-history/app/js/HttpController.js b/services/project-history/app/js/HttpController.js index 927248726f..d04dd566ed 100644 --- a/services/project-history/app/js/HttpController.js +++ b/services/project-history/app/js/HttpController.js @@ -17,12 +17,21 @@ import * as RetryManager from './RetryManager.js' import * as FlushManager from './FlushManager.js' import { pipeline } from 'node:stream' import { RequestFailedError } from '@overleaf/fetch-utils' +import { z, zz, validateReq } from '@overleaf/validation-tools' const ONE_DAY_IN_SECONDS = 24 * 60 * 60 +const getProjectBlobSchema = z.object({ + params: z.object({ + history_id: zz.objectId(), + hash: z.string(), + }), +}) + export function getProjectBlob(req, res, next) { - const historyId = req.params.history_id - const blobHash = req.params.hash + const { params } = validateReq(req, getProjectBlobSchema) + const historyId = params.history_id + const blobHash = params.hash HistoryStoreManager.getProjectBlobStream( historyId, blobHash, @@ -52,9 +61,20 @@ export function initializeProject(req, res, next) { }) } +const flushProjectSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), + query: z.object({ + debug: z.stringbool().default(false), + bisect: z.stringbool().default(false), + }), +}) + export function flushProject(req, res, next) { - const projectId = req.params.project_id - if (req.query.debug) { + const { query, params } = validateReq(req, flushProjectSchema) + const projectId = params.project_id + if (query.debug) { logger.debug( { projectId }, 'compressing project history in single-step mode' @@ -65,7 +85,7 @@ export function flushProject(req, res, next) { } res.sendStatus(204) }) - } else if (req.query.bisect) { + } else if (query.bisect) { logger.debug({ projectId }, 'compressing project history in bisect mode') UpdatesProcessor.processUpdatesForProjectUsingBisect( projectId, @@ -88,9 +108,19 @@ export function flushProject(req, res, next) { } } +const dumpProjectSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), + query: z.object({ + count: z.coerce.number().int().optional(), + }), +}) + export function dumpProject(req, res, next) { - const projectId = req.params.project_id - const batchSize = req.query.count || UpdatesProcessor.REDIS_READ_BATCH_SIZE + const { query, params } = validateReq(req, dumpProjectSchema) + const projectId = params.project_id + const batchSize = query.count || UpdatesProcessor.REDIS_READ_BATCH_SIZE logger.debug({ projectId }, 'retrieving raw updates') UpdatesProcessor.getRawUpdates(projectId, batchSize, (error, rawUpdates) => { if (error != null) { @@ -100,8 +130,30 @@ export function dumpProject(req, res, next) { }) } +const flushOldSchema = z.object({ + query: z.object({ + // flush projects with queued ops older than this + maxAge: z.coerce + .number() + .int() + .default(6 * 3600), + // pause this amount of time between checking queues + queueDelay: z.coerce.number().int().default(100), + // maximum number of queues to check + limit: z.coerce.number().int().default(1000), + // maximum amount of time allowed + timeout: z.coerce + .number() + .int() + .default(60 * 1000), + // whether to run in the background + background: z.stringbool().default(false), + }), +}) + export function flushOld(req, res, next) { - const { maxAge, queueDelay, limit, timeout, background } = req.query + const { query } = validateReq(req, flushOldSchema) + const { maxAge, queueDelay, limit, timeout, background } = query const options = { maxAge, queueDelay, limit, timeout, background } FlushManager.flushOldOps(options, (error, results) => { if (error != null) { @@ -111,12 +163,21 @@ export function flushOld(req, res, next) { }) } +const getDiffSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), + query: z.object({ + pathname: z.string(), + from: z.coerce.number().int(), + to: z.coerce.number().int(), + }), +}) + export function getDiff(req, res, next) { - const projectId = req.params.project_id - const { pathname, from, to } = req.query - if (pathname == null) { - return res.sendStatus(400) - } + const { query, params } = validateReq(req, getDiffSchema) + const { pathname, from, to } = query + const projectId = params.project_id logger.debug({ projectId, pathname, from, to }, 'getting diff') DiffManager.getDiff(projectId, pathname, from, to, (error, diff) => { @@ -127,9 +188,20 @@ export function getDiff(req, res, next) { }) } +const getFileTreeDiffSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), + query: z.object({ + from: z.coerce.number().int(), + to: z.coerce.number().int(), + }), +}) + export function getFileTreeDiff(req, res, next) { - const projectId = req.params.project_id - const { to, from } = req.query + const { query, params } = validateReq(req, getFileTreeDiffSchema) + const { from, to } = query + const projectId = params.project_id DiffManager.getFileTreeDiff(projectId, from, to, (error, diff) => { if (error != null) { @@ -139,9 +211,20 @@ export function getFileTreeDiff(req, res, next) { }) } +const getUpdatesSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), + query: z.object({ + before: z.coerce.number().int().optional(), + min_count: z.coerce.number().int().optional(), + }), +}) + export function getUpdates(req, res, next) { - const projectId = req.params.project_id - const { before, min_count: minCount } = req.query + const { query, params } = validateReq(req, getUpdatesSchema) + const projectId = params.project_id + const { before, min_count: minCount } = query SummarizedUpdatesManager.getSummarizedProjectUpdates( projectId, { before, min_count: minCount }, @@ -161,8 +244,15 @@ export function getUpdates(req, res, next) { ) } +const latestVersionSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), +}) + export function latestVersion(req, res, next) { - const projectId = req.params.project_id + const { params } = validateReq(req, latestVersionSchema) + const projectId = params.project_id logger.debug({ projectId }, 'compressing project history and getting version') UpdatesProcessor.processUpdatesForProject(projectId, error => { if (error != null) { @@ -190,8 +280,17 @@ export function latestVersion(req, res, next) { }) } +const getFileSnapshotSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + version: z.coerce.number().int(), + pathname: z.string(), + }), +}) + export function getFileSnapshot(req, res, next) { - const { project_id: projectId, version, pathname } = req.params + const { params } = validateReq(req, getFileSnapshotSchema) + const { project_id: projectId, version, pathname } = params SnapshotManager.getFileSnapshotStream( projectId, version, @@ -208,8 +307,17 @@ export function getFileSnapshot(req, res, next) { ) } +const getRangesSnapshotSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + version: z.coerce.number().int(), + pathname: z.string(), + }), +}) + export function getRangesSnapshot(req, res, next) { - const { project_id: projectId, version, pathname } = req.params + const { params } = validateReq(req, getRangesSnapshotSchema) + const { project_id: projectId, version, pathname } = params SnapshotManager.getRangesSnapshot( projectId, version, @@ -223,8 +331,17 @@ export function getRangesSnapshot(req, res, next) { ) } +const getFileMetadataSnapshotSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + version: z.coerce.number().int(), + pathname: z.string(), + }), +}) + export function getFileMetadataSnapshot(req, res, next) { - const { project_id: projectId, version, pathname } = req.params + const { params } = validateReq(req, getFileMetadataSnapshotSchema) + const { project_id: projectId, version, pathname } = params SnapshotManager.getFileMetadataSnapshot( projectId, version, @@ -238,8 +355,15 @@ export function getFileMetadataSnapshot(req, res, next) { ) } +const getLatestSnapshotSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), +}) + export function getLatestSnapshot(req, res, next) { - const { project_id: projectId } = req.params + const { params } = validateReq(req, getLatestSnapshotSchema) + const { project_id: projectId } = params WebApiManager.getHistoryId(projectId, (error, historyId) => { if (error) return next(OError.tag(error)) SnapshotManager.getLatestSnapshot( @@ -256,9 +380,19 @@ export function getLatestSnapshot(req, res, next) { }) } +const getChangesInChunkSinceSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), + query: z.object({ + since: z.coerce.number().int().min(0), + }), +}) + export function getChangesInChunkSince(req, res, next) { - const { project_id: projectId } = req.params - const { since } = req.query + const { query, params } = validateReq(req, getChangesInChunkSinceSchema) + const { project_id: projectId } = params + const { since } = query WebApiManager.getHistoryId(projectId, (error, historyId) => { if (error) return next(OError.tag(error)) SnapshotManager.getChangesInChunkSince( @@ -279,8 +413,16 @@ export function getChangesInChunkSince(req, res, next) { }) } +const getProjectSnapshotSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + version: z.coerce.number().int(), + }), +}) + export function getProjectSnapshot(req, res, next) { - const { project_id: projectId, version } = req.params + const { params } = validateReq(req, getProjectSnapshotSchema) + const { project_id: projectId, version } = params SnapshotManager.getProjectSnapshot( projectId, version, @@ -293,8 +435,16 @@ export function getProjectSnapshot(req, res, next) { ) } +const getPathsAtVersionSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + version: z.coerce.number().int(), + }), +}) + export function getPathsAtVersion(req, res, next) { - const { project_id: projectId, version } = req.params + const { params } = validateReq(req, getPathsAtVersionSchema) + const { project_id: projectId, version } = params SnapshotManager.getPathsAtVersion(projectId, version, (error, result) => { if (error != null) { return next(error) @@ -325,16 +475,35 @@ export function checkLock(req, res) { }) } +const resyncProjectSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), + query: z.object({ + force: z.stringbool().default(false), + }), + body: z.object({ + force: z.boolean().default(false), + origin: z + .object({ + kind: z.string(), + }) + .optional(), + historyRangesMigration: z.enum(['forwards', 'backwards']).optional(), + }), +}) + export function resyncProject(req, res, next) { - const projectId = req.params.project_id + const { query, params, body } = validateReq(req, resyncProjectSchema) + const projectId = params.project_id const options = {} - if (req.body.origin) { - options.origin = req.body.origin + if (body.origin) { + options.origin = body.origin } - if (req.body.historyRangesMigration) { - options.historyRangesMigration = req.body.historyRangesMigration + if (body.historyRangesMigration) { + options.historyRangesMigration = body.historyRangesMigration } - if (req.query.force || req.body.force) { + if (query.force || body.force) { // this will delete the queue and clear the sync state // use if the project is completely broken SyncManager.startHardResync(projectId, options, error => { @@ -365,10 +534,20 @@ export function resyncProject(req, res, next) { } } +const forceDebugProjectSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), + query: z.object({ + clear: z.stringbool().default(false), + }), +}) + export function forceDebugProject(req, res, next) { - const projectId = req.params.project_id + const { query, params } = validateReq(req, forceDebugProjectSchema) + const projectId = params.project_id // set the debug flag to true unless we see ?clear=true - const state = !req.query.clear + const state = !query.clear ErrorRecorder.setForceDebug(projectId, state, error => { if (error != null) { return next(error) @@ -401,8 +580,15 @@ export function getQueueCounts(req, res, next) { }) } +const getLabelsSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), +}) + export function getLabels(req, res, next) { - const projectId = req.params.project_id + const { params } = validateReq(req, getLabelsSchema) + const projectId = params.project_id HistoryApiManager.shouldUseProjectHistory( projectId, (error, shouldUseProjectHistory) => { @@ -423,15 +609,30 @@ export function getLabels(req, res, next) { ) } +const createLabelSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + user_id: zz.objectId().optional(), + }), + body: z.object({ + version: z.number().int(), + comment: z.string(), + created_at: z.string().optional(), + validate_exists: z.boolean().default(true), + user_id: zz.objectId().nullable().optional(), + }), +}) + export function createLabel(req, res, next) { - const { project_id: projectId, user_id: userIdParam } = req.params + const { params, body } = validateReq(req, createLabelSchema) + const { project_id: projectId, user_id: userIdParam } = params const { version, comment, user_id: userIdBody, created_at: createdAt, validate_exists: validateExists, - } = req.body + } = body // Temporarily looking up both params and body while rolling out changes // in the router path - https://github.com/overleaf/internal/pull/20200 @@ -480,12 +681,17 @@ export function createLabel(req, res, next) { * This will delete a label if it is owned by the current user. If you wish to * delete a label regardless of the current user, then use `deleteLabel` instead. */ +const deleteLabelForUserSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + user_id: zz.objectId(), + label_id: zz.objectId(), + }), +}) + export function deleteLabelForUser(req, res, next) { - const { - project_id: projectId, - user_id: userId, - label_id: labelId, - } = req.params + const { params } = validateReq(req, deleteLabelForUserSchema) + const { project_id: projectId, user_id: userId, label_id: labelId } = params LabelsManager.deleteLabelForUser(projectId, userId, labelId, error => { if (error != null) { @@ -495,8 +701,16 @@ export function deleteLabelForUser(req, res, next) { }) } +const deleteLabelSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + label_id: zz.objectId(), + }), +}) + export function deleteLabel(req, res, next) { - const { project_id: projectId, label_id: labelId } = req.params + const { params } = validateReq(req, deleteLabelSchema) + const { project_id: projectId, label_id: labelId } = params LabelsManager.deleteLabel(projectId, labelId, error => { if (error != null) { @@ -506,8 +720,20 @@ export function deleteLabel(req, res, next) { }) } +const retryFailuresSchema = z.object({ + query: z.object({ + failureType: z.enum(['soft', 'hard']).optional(), + // bail out after this time limit + timeout: z.coerce.number().int().default(300), + // maximum number of projects to check + limit: z.coerce.number().int().default(100), + callbackUrl: z.string().optional(), + }), +}) + export function retryFailures(req, res, next) { - const { failureType, timeout, limit, callbackUrl } = req.query + const { query } = validateReq(req, retryFailuresSchema) + const { failureType, timeout, limit, callbackUrl } = query if (callbackUrl) { // send response but run in background when callbackUrl provided res.send({ retryStatus: 'running retryFailures in background' }) @@ -539,8 +765,16 @@ export function retryFailures(req, res, next) { ) } +const transferLabelsSchema = z.object({ + params: z.object({ + from_user: zz.objectId(), + to_user: zz.objectId(), + }), +}) + export function transferLabels(req, res, next) { - const { from_user: fromUser, to_user: toUser } = req.params + const { params } = validateReq(req, transferLabelsSchema) + const { from_user: fromUser, to_user: toUser } = params LabelsManager.transferLabels(fromUser, toUser, error => { if (error != null) { return next(error) @@ -549,8 +783,15 @@ export function transferLabels(req, res, next) { }) } +const deleteProjectSchema = z.object({ + params: z.object({ + project_id: zz.objectId(), + }), +}) + export function deleteProject(req, res, next) { - const { project_id: projectId } = req.params + const { params } = validateReq(req, deleteProjectSchema) + const { project_id: projectId } = params // clear the timestamp before clearing the queue, // because the queue location is used in the migration RedisManager.clearFirstOpTimestamp(projectId, err => { diff --git a/services/project-history/app/js/Router.js b/services/project-history/app/js/Router.js index ec9a4f0582..c73839f74f 100644 --- a/services/project-history/app/js/Router.js +++ b/services/project-history/app/js/Router.js @@ -1,19 +1,7 @@ import OError from '@overleaf/o-error' import * as HttpController from './HttpController.js' -import { Joi, validate } from './Validation.js' export function initialize(app) { - app.use( - validate({ - params: Joi.object({ - project_id: Joi.string().regex(/^[0-9a-f]{24}$/), - user_id: Joi.string().regex(/^[0-9a-f]{24}$/), - label_id: Joi.string().regex(/^[0-9a-f]{24}$/), - version: Joi.number().integer(), - }), - }) - ) - // use an extended timeout on all endpoints, to allow for long requests to history-v1 app.use(longerTimeout) @@ -23,129 +11,36 @@ export function initialize(app) { app.get('/project/:project_id/snapshot', HttpController.getLatestSnapshot) - app.get( - '/project/:project_id/diff', - validate({ - query: { - pathname: Joi.string().required(), - from: Joi.number().integer().required(), - to: Joi.number().integer().required(), - }, - }), - HttpController.getDiff - ) + app.get('/project/:project_id/diff', HttpController.getDiff) - app.get( - '/project/:project_id/filetree/diff', - validate({ - query: { - from: Joi.number().integer().required(), - to: Joi.number().integer().required(), - }, - }), - HttpController.getFileTreeDiff - ) + app.get('/project/:project_id/filetree/diff', HttpController.getFileTreeDiff) - app.get( - '/project/:project_id/updates', - validate({ - query: { - before: Joi.number().integer(), - min_count: Joi.number().integer(), - }, - }), - HttpController.getUpdates - ) + app.get('/project/:project_id/updates', HttpController.getUpdates) app.get( '/project/:project_id/changes-in-chunk', - validate({ - query: { - since: Joi.number().integer().min(0), - }, - }), HttpController.getChangesInChunkSince ) app.get('/project/:project_id/version', HttpController.latestVersion) - app.post( - '/project/:project_id/flush', - validate({ - query: { - debug: Joi.boolean().default(false), - bisect: Joi.boolean().default(false), - }, - }), - HttpController.flushProject - ) + app.post('/project/:project_id/flush', HttpController.flushProject) - app.post( - '/project/:project_id/resync', - validate({ - query: { - force: Joi.boolean().default(false), - }, - body: { - force: Joi.boolean().default(false), - origin: Joi.object({ - kind: Joi.string().required(), - }), - historyRangesMigration: Joi.string() - .optional() - .valid('forwards', 'backwards'), - }, - }), - HttpController.resyncProject - ) + app.post('/project/:project_id/resync', HttpController.resyncProject) - app.get( - '/project/:project_id/dump', - validate({ - query: { - count: Joi.number().integer(), - }, - }), - HttpController.dumpProject - ) + app.get('/project/:project_id/dump', HttpController.dumpProject) app.get('/project/:project_id/labels', HttpController.getLabels) - app.post( - '/project/:project_id/labels', - validate({ - body: { - version: Joi.number().integer().required(), - comment: Joi.string().required(), - created_at: Joi.string(), - validate_exists: Joi.boolean().default(true), - user_id: Joi.string().allow(null), - }, - }), - - HttpController.createLabel - ) + app.post('/project/:project_id/labels', HttpController.createLabel) app.delete( '/project/:project_id/user/:user_id/labels/:label_id', - validate({ - params: Joi.object({ - project_id: Joi.string().regex(/^[0-9a-f]{24}$/), - user_id: Joi.string().regex(/^[0-9a-f]{24}$/), - label_id: Joi.string().regex(/^[0-9a-f]{24}$/), - }), - }), HttpController.deleteLabelForUser ) app.delete( '/project/:project_id/labels/:label_id', - validate({ - params: Joi.object({ - project_id: Joi.string().regex(/^[0-9a-f]{24}$/), - label_id: Joi.string().regex(/^[0-9a-f]{24}$/), - }), - }), HttpController.deleteLabel ) @@ -179,15 +74,7 @@ export function initialize(app) { HttpController.getPathsAtVersion ) - app.post( - '/project/:project_id/force', - validate({ - query: { - clear: Joi.boolean().default(false), - }, - }), - HttpController.forceDebugProject - ) + app.post('/project/:project_id/force', HttpController.forceDebugProject) app.get('/project/:history_id/blob/:hash', HttpController.getProjectBlob) @@ -195,43 +82,9 @@ export function initialize(app) { app.get('/status/queue', HttpController.getQueueCounts) - app.post( - '/retry/failures', - validate({ - query: { - failureType: Joi.string().valid('soft', 'hard'), - // bail out after this time limit - timeout: Joi.number().integer().default(300), - // maximum number of projects to check - limit: Joi.number().integer().default(100), - callbackUrl: Joi.string(), - }, - }), - HttpController.retryFailures - ) + app.post('/retry/failures', HttpController.retryFailures) - app.post( - '/flush/old', - validate({ - query: { - // flush projects with queued ops older than this - maxAge: Joi.number() - .integer() - .default(6 * 3600), - // pause this amount of time between checking queues - queueDelay: Joi.number().integer().default(100), - // maximum number of queues to check - limit: Joi.number().integer().default(1000), - // maximum amount of time allowed - timeout: Joi.number() - .integer() - .default(60 * 1000), - // whether to run in the background - background: Joi.boolean().falsy('0').truthy('1').default(false), - }, - }), - HttpController.flushOld - ) + app.post('/flush/old', HttpController.flushOld) app.get('/status', (req, res, next) => res.send('project-history is up')) diff --git a/services/project-history/app/js/Validation.js b/services/project-history/app/js/Validation.js deleted file mode 100644 index 846cc12e24..0000000000 --- a/services/project-history/app/js/Validation.js +++ /dev/null @@ -1,12 +0,0 @@ -import { celebrate, errors } from 'celebrate' - -export { Joi } from 'celebrate' - -export const errorMiddleware = errors() - -/** - * Validation middleware - */ -export function validate(schema) { - return celebrate(schema, { allowUnknown: true }) -} diff --git a/services/project-history/app/js/server.js b/services/project-history/app/js/server.js index 4f13198eea..91a127ef52 100644 --- a/services/project-history/app/js/server.js +++ b/services/project-history/app/js/server.js @@ -4,7 +4,7 @@ import express from 'express' import bodyParser from 'body-parser' import * as Errors from './Errors.js' import * as Router from './Router.js' -import * as Validation from './Validation.js' +import { handleValidationError } from '@overleaf/validation-tools' const HistoryLogger = logger.initialize('project-history').logger @@ -44,7 +44,7 @@ app.use(bodyParser.urlencoded({ extended: true })) app.use(Metrics.http.monitor(logger)) Router.initialize(app) Metrics.injectMetricsRoute(app) -app.use(Validation.errorMiddleware) +app.use(handleValidationError) app.use(function (error, req, res, next) { if (error instanceof Errors.NotFoundError) { res.sendStatus(404) diff --git a/services/project-history/package.json b/services/project-history/package.json index d7543b5142..bed6861cee 100644 --- a/services/project-history/package.json +++ b/services/project-history/package.json @@ -27,6 +27,7 @@ "@overleaf/redis-wrapper": "*", "@overleaf/settings": "*", "@overleaf/stream-utils": "*", + "@overleaf/validation-tools": "*", "async": "^3.2.5", "body-parser": "1.20.4", "bunyan": "^1.8.15", diff --git a/services/project-history/test/unit/js/HttpController/HttpControllerTests.js b/services/project-history/test/unit/js/HttpController/HttpControllerTests.js index 1b7adf0ef5..d00797e2c2 100644 --- a/services/project-history/test/unit/js/HttpController/HttpControllerTests.js +++ b/services/project-history/test/unit/js/HttpController/HttpControllerTests.js @@ -92,7 +92,7 @@ describe('HttpController', function () { beforeEach(function () { this.blobHash = 'abcd' this.stream = {} - this.historyId = 1337 + this.historyId = new ObjectId().toString() this.HistoryStoreManager.getProjectBlobStream.yields(null, this.stream) this.HttpController.getProjectBlob( { params: { history_id: this.historyId, hash: this.blobHash } }, @@ -414,7 +414,7 @@ describe('HttpController', function () { project_id: this.projectId, }, body: { - version: (this.version = 'label-1'), + version: (this.version = 1), comment: (this.comment = 'a comment'), created_at: (this.created_at = Date.now().toString()), validate_exists: true, @@ -486,11 +486,12 @@ describe('HttpController', function () { describe('deleteLabelForUser', function () { beforeEach(function () { + this.label_id = new ObjectId() this.req = { params: { project_id: this.projectId, user_id: this.userId, - label_id: (this.label_id = new ObjectId()), + label_id: this.label_id.toString(), }, } this.HttpController.deleteLabelForUser(this.req, this.res, this.next) @@ -498,7 +499,7 @@ describe('HttpController', function () { it('should delete a label for a project', function () { this.LabelsManager.deleteLabelForUser - .calledWith(this.projectId, this.userId, this.label_id) + .calledWith(this.projectId, this.userId, this.label_id.toString()) .should.equal(true) }) @@ -509,10 +510,11 @@ describe('HttpController', function () { describe('deleteLabel', function () { beforeEach(function () { + this.label_id = new ObjectId() this.req = { params: { project_id: this.projectId, - label_id: (this.label_id = new ObjectId()), + label_id: this.label_id.toString(), }, } this.HttpController.deleteLabel(this.req, this.res, this.next) @@ -520,7 +522,7 @@ describe('HttpController', function () { it('should delete a label for a project', function () { this.LabelsManager.deleteLabel - .calledWith(this.projectId, this.label_id) + .calledWith(this.projectId, this.label_id.toString()) .should.equal(true) })