Эх сурвалжийг харах

[project-history] migrate from joi to zod (#30187)

* Add @overleaf/validation-tools in project-history

* use error handler for zod

* Replace joi validations with zod schema

* fix unit tests

* remove Joi validation middleware

* remove optional when there is a default

* fix tests after checking for objectid

* ran build_scripts

* monorepo npm install

GitOrigin-RevId: f8396a3caadd536edf64c6f971c77f4c941f000a
Domagoj Kriskovic 7 сар өмнө
parent
commit
6d3dd3601a

+ 1 - 0
package-lock.json

@@ -57862,6 +57862,7 @@
         "@overleaf/redis-wrapper": "*",
         "@overleaf/settings": "*",
         "@overleaf/stream-utils": "*",
+        "@overleaf/validation-tools": "*",
         "async": "^3.2.5",
         "body-parser": "^1.20.3",
         "bunyan": "^1.8.15",

+ 2 - 0
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/
 

+ 1 - 0
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/* \

+ 287 - 46
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 => {

+ 10 - 157
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'))
 

+ 0 - 12
services/project-history/app/js/Validation.js

@@ -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 })
-}

+ 2 - 2
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)

+ 1 - 0
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.3",
     "bunyan": "^1.8.15",

+ 8 - 6
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)
     })