diff --git a/package-lock.json b/package-lock.json index 4bff2d1793..9aac41ed23 100644 --- a/package-lock.json +++ b/package-lock.json @@ -57864,7 +57864,6 @@ "@overleaf/redis-wrapper": "*", "@overleaf/settings": "*", "@overleaf/stream-utils": "*", - "@overleaf/validation-tools": "*", "async": "^3.2.5", "body-parser": "^1.20.3", "bunyan": "^1.8.15", diff --git a/services/project-history/Dockerfile b/services/project-history/Dockerfile index e5166c3b55..5966fca8cd 100644 --- a/services/project-history/Dockerfile +++ b/services/project-history/Dockerfile @@ -23,7 +23,6 @@ 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/ @@ -40,7 +39,6 @@ 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 55dc4470d4..efaea519f2 100644 --- a/services/project-history/Makefile +++ b/services/project-history/Makefile @@ -25,7 +25,6 @@ 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 d04dd566ed..927248726f 100644 --- a/services/project-history/app/js/HttpController.js +++ b/services/project-history/app/js/HttpController.js @@ -17,21 +17,12 @@ 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 { params } = validateReq(req, getProjectBlobSchema) - const historyId = params.history_id - const blobHash = params.hash + const historyId = req.params.history_id + const blobHash = req.params.hash HistoryStoreManager.getProjectBlobStream( historyId, blobHash, @@ -61,20 +52,9 @@ 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 { query, params } = validateReq(req, flushProjectSchema) - const projectId = params.project_id - if (query.debug) { + const projectId = req.params.project_id + if (req.query.debug) { logger.debug( { projectId }, 'compressing project history in single-step mode' @@ -85,7 +65,7 @@ export function flushProject(req, res, next) { } res.sendStatus(204) }) - } else if (query.bisect) { + } else if (req.query.bisect) { logger.debug({ projectId }, 'compressing project history in bisect mode') UpdatesProcessor.processUpdatesForProjectUsingBisect( projectId, @@ -108,19 +88,9 @@ 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 { query, params } = validateReq(req, dumpProjectSchema) - const projectId = params.project_id - const batchSize = query.count || UpdatesProcessor.REDIS_READ_BATCH_SIZE + const projectId = req.params.project_id + const batchSize = req.query.count || UpdatesProcessor.REDIS_READ_BATCH_SIZE logger.debug({ projectId }, 'retrieving raw updates') UpdatesProcessor.getRawUpdates(projectId, batchSize, (error, rawUpdates) => { if (error != null) { @@ -130,30 +100,8 @@ 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 { query } = validateReq(req, flushOldSchema) - const { maxAge, queueDelay, limit, timeout, background } = query + const { maxAge, queueDelay, limit, timeout, background } = req.query const options = { maxAge, queueDelay, limit, timeout, background } FlushManager.flushOldOps(options, (error, results) => { if (error != null) { @@ -163,21 +111,12 @@ 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 { query, params } = validateReq(req, getDiffSchema) - const { pathname, from, to } = query - const projectId = params.project_id + const projectId = req.params.project_id + const { pathname, from, to } = req.query + if (pathname == null) { + return res.sendStatus(400) + } logger.debug({ projectId, pathname, from, to }, 'getting diff') DiffManager.getDiff(projectId, pathname, from, to, (error, diff) => { @@ -188,20 +127,9 @@ 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 { query, params } = validateReq(req, getFileTreeDiffSchema) - const { from, to } = query - const projectId = params.project_id + const projectId = req.params.project_id + const { to, from } = req.query DiffManager.getFileTreeDiff(projectId, from, to, (error, diff) => { if (error != null) { @@ -211,20 +139,9 @@ 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 { query, params } = validateReq(req, getUpdatesSchema) - const projectId = params.project_id - const { before, min_count: minCount } = query + const projectId = req.params.project_id + const { before, min_count: minCount } = req.query SummarizedUpdatesManager.getSummarizedProjectUpdates( projectId, { before, min_count: minCount }, @@ -244,15 +161,8 @@ export function getUpdates(req, res, next) { ) } -const latestVersionSchema = z.object({ - params: z.object({ - project_id: zz.objectId(), - }), -}) - export function latestVersion(req, res, next) { - const { params } = validateReq(req, latestVersionSchema) - const projectId = params.project_id + const projectId = req.params.project_id logger.debug({ projectId }, 'compressing project history and getting version') UpdatesProcessor.processUpdatesForProject(projectId, error => { if (error != null) { @@ -280,17 +190,8 @@ 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 { params } = validateReq(req, getFileSnapshotSchema) - const { project_id: projectId, version, pathname } = params + const { project_id: projectId, version, pathname } = req.params SnapshotManager.getFileSnapshotStream( projectId, version, @@ -307,17 +208,8 @@ 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 { params } = validateReq(req, getRangesSnapshotSchema) - const { project_id: projectId, version, pathname } = params + const { project_id: projectId, version, pathname } = req.params SnapshotManager.getRangesSnapshot( projectId, version, @@ -331,17 +223,8 @@ 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 { params } = validateReq(req, getFileMetadataSnapshotSchema) - const { project_id: projectId, version, pathname } = params + const { project_id: projectId, version, pathname } = req.params SnapshotManager.getFileMetadataSnapshot( projectId, version, @@ -355,15 +238,8 @@ export function getFileMetadataSnapshot(req, res, next) { ) } -const getLatestSnapshotSchema = z.object({ - params: z.object({ - project_id: zz.objectId(), - }), -}) - export function getLatestSnapshot(req, res, next) { - const { params } = validateReq(req, getLatestSnapshotSchema) - const { project_id: projectId } = params + const { project_id: projectId } = req.params WebApiManager.getHistoryId(projectId, (error, historyId) => { if (error) return next(OError.tag(error)) SnapshotManager.getLatestSnapshot( @@ -380,19 +256,9 @@ 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 { query, params } = validateReq(req, getChangesInChunkSinceSchema) - const { project_id: projectId } = params - const { since } = query + const { project_id: projectId } = req.params + const { since } = req.query WebApiManager.getHistoryId(projectId, (error, historyId) => { if (error) return next(OError.tag(error)) SnapshotManager.getChangesInChunkSince( @@ -413,16 +279,8 @@ 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 { params } = validateReq(req, getProjectSnapshotSchema) - const { project_id: projectId, version } = params + const { project_id: projectId, version } = req.params SnapshotManager.getProjectSnapshot( projectId, version, @@ -435,16 +293,8 @@ 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 { params } = validateReq(req, getPathsAtVersionSchema) - const { project_id: projectId, version } = params + const { project_id: projectId, version } = req.params SnapshotManager.getPathsAtVersion(projectId, version, (error, result) => { if (error != null) { return next(error) @@ -475,35 +325,16 @@ 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 { query, params, body } = validateReq(req, resyncProjectSchema) - const projectId = params.project_id + const projectId = req.params.project_id const options = {} - if (body.origin) { - options.origin = body.origin + if (req.body.origin) { + options.origin = req.body.origin } - if (body.historyRangesMigration) { - options.historyRangesMigration = body.historyRangesMigration + if (req.body.historyRangesMigration) { + options.historyRangesMigration = req.body.historyRangesMigration } - if (query.force || body.force) { + if (req.query.force || req.body.force) { // this will delete the queue and clear the sync state // use if the project is completely broken SyncManager.startHardResync(projectId, options, error => { @@ -534,20 +365,10 @@ 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 { query, params } = validateReq(req, forceDebugProjectSchema) - const projectId = params.project_id + const projectId = req.params.project_id // set the debug flag to true unless we see ?clear=true - const state = !query.clear + const state = !req.query.clear ErrorRecorder.setForceDebug(projectId, state, error => { if (error != null) { return next(error) @@ -580,15 +401,8 @@ export function getQueueCounts(req, res, next) { }) } -const getLabelsSchema = z.object({ - params: z.object({ - project_id: zz.objectId(), - }), -}) - export function getLabels(req, res, next) { - const { params } = validateReq(req, getLabelsSchema) - const projectId = params.project_id + const projectId = req.params.project_id HistoryApiManager.shouldUseProjectHistory( projectId, (error, shouldUseProjectHistory) => { @@ -609,30 +423,15 @@ 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 { params, body } = validateReq(req, createLabelSchema) - const { project_id: projectId, user_id: userIdParam } = params + const { project_id: projectId, user_id: userIdParam } = req.params const { version, comment, user_id: userIdBody, created_at: createdAt, validate_exists: validateExists, - } = body + } = req.body // Temporarily looking up both params and body while rolling out changes // in the router path - https://github.com/overleaf/internal/pull/20200 @@ -681,17 +480,12 @@ 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 { params } = validateReq(req, deleteLabelForUserSchema) - const { project_id: projectId, user_id: userId, label_id: labelId } = params + const { + project_id: projectId, + user_id: userId, + label_id: labelId, + } = req.params LabelsManager.deleteLabelForUser(projectId, userId, labelId, error => { if (error != null) { @@ -701,16 +495,8 @@ 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 { params } = validateReq(req, deleteLabelSchema) - const { project_id: projectId, label_id: labelId } = params + const { project_id: projectId, label_id: labelId } = req.params LabelsManager.deleteLabel(projectId, labelId, error => { if (error != null) { @@ -720,20 +506,8 @@ 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 { query } = validateReq(req, retryFailuresSchema) - const { failureType, timeout, limit, callbackUrl } = query + const { failureType, timeout, limit, callbackUrl } = req.query if (callbackUrl) { // send response but run in background when callbackUrl provided res.send({ retryStatus: 'running retryFailures in background' }) @@ -765,16 +539,8 @@ 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 { params } = validateReq(req, transferLabelsSchema) - const { from_user: fromUser, to_user: toUser } = params + const { from_user: fromUser, to_user: toUser } = req.params LabelsManager.transferLabels(fromUser, toUser, error => { if (error != null) { return next(error) @@ -783,15 +549,8 @@ export function transferLabels(req, res, next) { }) } -const deleteProjectSchema = z.object({ - params: z.object({ - project_id: zz.objectId(), - }), -}) - export function deleteProject(req, res, next) { - const { params } = validateReq(req, deleteProjectSchema) - const { project_id: projectId } = params + const { project_id: projectId } = req.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 c73839f74f..ec9a4f0582 100644 --- a/services/project-history/app/js/Router.js +++ b/services/project-history/app/js/Router.js @@ -1,7 +1,19 @@ 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) @@ -11,36 +23,129 @@ export function initialize(app) { app.get('/project/:project_id/snapshot', HttpController.getLatestSnapshot) - app.get('/project/:project_id/diff', HttpController.getDiff) + 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/filetree/diff', HttpController.getFileTreeDiff) + 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/updates', HttpController.getUpdates) + app.get( + '/project/:project_id/updates', + validate({ + query: { + before: Joi.number().integer(), + min_count: Joi.number().integer(), + }, + }), + 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', HttpController.flushProject) + 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/resync', HttpController.resyncProject) + 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.get('/project/:project_id/dump', HttpController.dumpProject) + app.get( + '/project/:project_id/dump', + validate({ + query: { + count: Joi.number().integer(), + }, + }), + HttpController.dumpProject + ) app.get('/project/:project_id/labels', HttpController.getLabels) - app.post('/project/:project_id/labels', HttpController.createLabel) + 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.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 ) @@ -74,7 +179,15 @@ export function initialize(app) { HttpController.getPathsAtVersion ) - app.post('/project/:project_id/force', HttpController.forceDebugProject) + app.post( + '/project/:project_id/force', + validate({ + query: { + clear: Joi.boolean().default(false), + }, + }), + HttpController.forceDebugProject + ) app.get('/project/:history_id/blob/:hash', HttpController.getProjectBlob) @@ -82,9 +195,43 @@ export function initialize(app) { app.get('/status/queue', HttpController.getQueueCounts) - app.post('/retry/failures', HttpController.retryFailures) + 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('/flush/old', HttpController.flushOld) + 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.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 new file mode 100644 index 0000000000..846cc12e24 --- /dev/null +++ b/services/project-history/app/js/Validation.js @@ -0,0 +1,12 @@ +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 91a127ef52..4f13198eea 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 { handleValidationError } from '@overleaf/validation-tools' +import * as Validation from './Validation.js' 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(handleValidationError) +app.use(Validation.errorMiddleware) 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 28e743c09c..8163799e75 100644 --- a/services/project-history/package.json +++ b/services/project-history/package.json @@ -27,7 +27,6 @@ "@overleaf/redis-wrapper": "*", "@overleaf/settings": "*", "@overleaf/stream-utils": "*", - "@overleaf/validation-tools": "*", "async": "^3.2.5", "body-parser": "^1.20.3", "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 d00797e2c2..1b7adf0ef5 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 = new ObjectId().toString() + this.historyId = 1337 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 = 1), + version: (this.version = 'label-1'), comment: (this.comment = 'a comment'), created_at: (this.created_at = Date.now().toString()), validate_exists: true, @@ -486,12 +486,11 @@ 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.toString(), + label_id: (this.label_id = new ObjectId()), }, } this.HttpController.deleteLabelForUser(this.req, this.res, this.next) @@ -499,7 +498,7 @@ describe('HttpController', function () { it('should delete a label for a project', function () { this.LabelsManager.deleteLabelForUser - .calledWith(this.projectId, this.userId, this.label_id.toString()) + .calledWith(this.projectId, this.userId, this.label_id) .should.equal(true) }) @@ -510,11 +509,10 @@ 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.toString(), + label_id: (this.label_id = new ObjectId()), }, } this.HttpController.deleteLabel(this.req, this.res, this.next) @@ -522,7 +520,7 @@ describe('HttpController', function () { it('should delete a label for a project', function () { this.LabelsManager.deleteLabel - .calledWith(this.projectId, this.label_id.toString()) + .calledWith(this.projectId, this.label_id) .should.equal(true) })