From 52e5fb349811a4e8df63e90fc122f83782aa3828 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 3 Aug 2026 12:45:34 +0200 Subject: [PATCH] [project-history] adopt strict zod types and extend test coverage (#36087) GitOrigin-RevId: ce1396e87926fc8816afa424c9db2a052c7da5e6 --- .../app/js/HistoryStoreManager.js | 7 +- .../project-history/app/js/HttpController.js | 497 ++++++++++++++++-- .../test/acceptance/js/CloneProjectTests.js | 83 +++ .../test/acceptance/js/DebugInfoTests.js | 86 +++ .../test/acceptance/js/FlushManagerTests.js | 18 + .../js/RangesAndMetadataSnapshotTests.js | 109 ++++ .../test/acceptance/js/ValidationTests.js | 95 ++++ .../js/helpers/ProjectHistoryClient.js | 58 ++ .../HistoryStoreManagerTests.js | 175 ++++++ .../js/HttpController/HttpControllerTests.js | 4 +- 10 files changed, 1077 insertions(+), 55 deletions(-) create mode 100644 services/project-history/test/acceptance/js/CloneProjectTests.js create mode 100644 services/project-history/test/acceptance/js/DebugInfoTests.js create mode 100644 services/project-history/test/acceptance/js/RangesAndMetadataSnapshotTests.js create mode 100644 services/project-history/test/acceptance/js/ValidationTests.js diff --git a/services/project-history/app/js/HistoryStoreManager.js b/services/project-history/app/js/HistoryStoreManager.js index 664f8decbb..9308edb3c7 100644 --- a/services/project-history/app/js/HistoryStoreManager.js +++ b/services/project-history/app/js/HistoryStoreManager.js @@ -268,7 +268,7 @@ export function sendChanges( endVersion, errorCode: error.code, statusCode: error.statusCode, - body: error.body, + errorBody: error.body, }) return callback(error) } @@ -625,10 +625,13 @@ function _requestHistoryService(options, callback) { }, err => { if (err instanceof RequestFailedError) { + const body = [400, 409, 413, 422].includes(err.response.status) + ? err.body + : null const error = new OError( // Keep the status code in the message. It is used by the ErrorRecorder. `history store a non-success status code: ${err.response.status}`, - { method, url, qs: options.qs, statusCode: err.response.status } + { method, url, qs: options.qs, statusCode: err.response.status, body } ) return callback(error) } diff --git a/services/project-history/app/js/HttpController.js b/services/project-history/app/js/HttpController.js index c9deb47393..c92c7a5c7c 100644 --- a/services/project-history/app/js/HttpController.js +++ b/services/project-history/app/js/HttpController.js @@ -18,10 +18,33 @@ import Stream, { pipeline } from 'node:stream' import { fetchNothing, RequestFailedError } from '@overleaf/fetch-utils' import { z, zz, parseReq } from '@overleaf/validation-tools' import { IncrementalResponse } from '@overleaf/stream-utils' +import editorCoreSchemas from 'overleaf-editor-core/lib/schemas.js' const ONE_DAY_IN_SECONDS = 24 * 60 * 60 +// Most :project_id params accept either a Mongo ObjectId (web-linked +// projects) or a legacy v1-only numeric id (see NumericProjectIdTests) — +// project-history stores/queues these opaquely (Redis keys, plain string +// Mongo fields) without requiring ObjectId format. +const historyIdSchema = zz.objectId().or(z.coerce.number()) + +// cloneProject only clones data that is keyed by a real Mongo ObjectId +// (projectHistoryLabels/projectHistorySyncState/projectHistoryFailures all +// convert this id with `new ObjectId(...)`), so — unlike most routes here — +// it does not accept the legacy numeric v1-only id. const cloneProjectSchema = z.object({ + body: z.strictObject({ + targetProjectId: zz.objectId(), + }), + + params: z.strictObject({ + project_id: zz.objectId(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const cloneProjectFallbackSchema = z.object({ body: z.object({ targetProjectId: z.string(), }), @@ -35,7 +58,9 @@ export function cloneProject(req, res) { const { params: { project_id: sourceProjectId }, body: { targetProjectId }, - } = parseReq(req, cloneProjectSchema) + } = parseReq(req, cloneProjectSchema, { + fallbackSchema: cloneProjectFallbackSchema, + }) const incrResp = new IncrementalResponse({ res, timeout: 10 * 60_000 - 5_000, @@ -138,14 +163,25 @@ export function cloneProject(req, res) { } const getProjectBlobSchema = z.object({ + params: z.strictObject({ + history_id: historyIdSchema, + hash: editorCoreSchemas.rawBlobHash, + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getProjectBlobFallbackSchema = z.object({ params: z.object({ - history_id: zz.objectId().or(z.coerce.number()), + history_id: historyIdSchema, hash: z.string(), }), }) export function getProjectBlob(req, res, next) { - const { params } = parseReq(req, getProjectBlobSchema) + const { params } = parseReq(req, getProjectBlobSchema, { + fallbackSchema: getProjectBlobFallbackSchema, + }) const historyId = params.history_id const blobHash = params.hash HistoryStoreManager.getProjectBlobStream( @@ -167,8 +203,27 @@ export function getProjectBlob(req, res, next) { ) } +const initializeProjectSchema = z.object({ + body: z.strictObject({ + historyId: historyIdSchema.optional(), + }), +}) + +// Rollout-temporary fallback (loosened primary schema; no zod validation +// existed for this route on main); delete when this route's +// REQ_VALIDATION_MODE instrumentation is removed. +const initializeProjectFallbackSchema = z.object({ + body: z.object({ + historyId: z.string().or(z.coerce.number()).optional(), + }), +}) + export function initializeProject(req, res, next) { - const { historyId } = req.body + const { body } = parseReq(req, initializeProjectSchema, { + logOnly: true, + fallbackSchema: initializeProjectFallbackSchema, + }) + const { historyId } = body HistoryStoreManager.initializeProject(historyId, (error, id) => { if (error != null) { return next(OError.tag(error)) @@ -178,8 +233,26 @@ export function initializeProject(req, res, next) { } const flushProjectSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), + query: z.strictObject({ + debug: z.stringbool().default(false), + bisect: z.stringbool().default(false), + // sent by document-updater's background flush (HistoryManager.js + // flushProjectChangesAsync); read only for the side effect of being + // present on the query string, not consumed here -- the flush is + // always processed the same way, this just distinguishes background + // flushes in logs/metrics upstream. + background: z.stringbool().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const flushProjectFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), query: z.object({ debug: z.stringbool().default(false), @@ -188,7 +261,9 @@ const flushProjectSchema = z.object({ }) export function flushProject(req, res, next) { - const { query, params } = parseReq(req, flushProjectSchema) + const { query, params } = parseReq(req, flushProjectSchema, { + fallbackSchema: flushProjectFallbackSchema, + }) const projectId = params.project_id if (query.debug) { logger.debug( @@ -225,8 +300,19 @@ export function flushProject(req, res, next) { } const dumpProjectSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), + query: z.strictObject({ + count: z.coerce.number().int().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const dumpProjectFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), query: z.object({ count: z.coerce.number().int().optional(), @@ -234,7 +320,9 @@ const dumpProjectSchema = z.object({ }) export function dumpProject(req, res, next) { - const { query, params } = parseReq(req, dumpProjectSchema) + const { query, params } = parseReq(req, dumpProjectSchema, { + fallbackSchema: dumpProjectFallbackSchema, + }) const projectId = params.project_id const batchSize = query.count || UpdatesProcessor.REDIS_READ_BATCH_SIZE logger.debug({ projectId }, 'retrieving raw updates') @@ -247,7 +335,7 @@ export function dumpProject(req, res, next) { } const flushOldSchema = z.object({ - query: z.object({ + query: z.strictObject({ // flush projects with queued ops older than this maxAge: z.coerce .number() @@ -267,8 +355,28 @@ const flushOldSchema = z.object({ }), }) +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const flushOldFallbackSchema = z.object({ + query: z.object({ + maxAge: z.coerce + .number() + .int() + .default(6 * 3600), + queueDelay: z.coerce.number().int().default(100), + limit: z.coerce.number().int().default(1000), + timeout: z.coerce + .number() + .int() + .default(60 * 1000), + background: z.stringbool().default(false), + }), +}) + export function flushOld(req, res, next) { - const { query } = parseReq(req, flushOldSchema) + const { query } = parseReq(req, flushOldSchema, { + fallbackSchema: flushOldFallbackSchema, + }) const { maxAge, queueDelay, limit, timeout, background } = query const options = { maxAge, queueDelay, limit, timeout, background } FlushManager.flushOldOps(options, (error, results) => { @@ -280,8 +388,21 @@ export function flushOld(req, res, next) { } const getDiffSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), + query: z.strictObject({ + pathname: zz.filepath(), + from: z.coerce.number().int(), + to: z.coerce.number().int(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getDiffFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), query: z.object({ pathname: z.string(), @@ -291,7 +412,9 @@ const getDiffSchema = z.object({ }) export function getDiff(req, res, next) { - const { query, params } = parseReq(req, getDiffSchema) + const { query, params } = parseReq(req, getDiffSchema, { + fallbackSchema: getDiffFallbackSchema, + }) const { pathname, from, to } = query const projectId = params.project_id @@ -305,8 +428,20 @@ export function getDiff(req, res, next) { } const getFileTreeDiffSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), + query: z.strictObject({ + from: z.coerce.number().int(), + to: z.coerce.number().int(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getFileTreeDiffFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), query: z.object({ from: z.coerce.number().int(), @@ -315,7 +450,9 @@ const getFileTreeDiffSchema = z.object({ }) export function getFileTreeDiff(req, res, next) { - const { query, params } = parseReq(req, getFileTreeDiffSchema) + const { query, params } = parseReq(req, getFileTreeDiffSchema, { + fallbackSchema: getFileTreeDiffFallbackSchema, + }) const { from, to } = query const projectId = params.project_id @@ -328,8 +465,20 @@ export function getFileTreeDiff(req, res, next) { } const getUpdatesSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), + query: z.strictObject({ + before: z.coerce.number().int().optional(), + min_count: z.coerce.number().int().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getUpdatesFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), query: z.object({ before: z.coerce.number().int().optional(), @@ -338,7 +487,9 @@ const getUpdatesSchema = z.object({ }) export function getUpdates(req, res, next) { - const { query, params } = parseReq(req, getUpdatesSchema) + const { query, params } = parseReq(req, getUpdatesSchema, { + fallbackSchema: getUpdatesFallbackSchema, + }) const projectId = params.project_id const { before, min_count: minCount } = query SummarizedUpdatesManager.getSummarizedProjectUpdates( @@ -360,7 +511,20 @@ export function getUpdates(req, res, next) { ) } +// Unlike most :project_id params in this file, this does NOT also accept a +// numeric legacy id: SyncManager.getResyncState does +// `new ObjectId(projectId.toString())`, which throws for a numeric id (the +// ObjectId constructor only special-cases a raw number argument, not a +// numeric string). const getResyncPendingSchema = z.object({ + params: z.strictObject({ + project_id: zz.objectId(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getResyncPendingFallbackSchema = z.object({ params: z.object({ project_id: zz.objectId(), }), @@ -369,7 +533,9 @@ const getResyncPendingSchema = z.object({ export function getResyncPending(req, res, next) { const { params: { project_id: projectId }, - } = parseReq(req, getResyncPendingSchema) + } = parseReq(req, getResyncPendingSchema, { + fallbackSchema: getResyncPendingFallbackSchema, + }) SyncManager.getResyncState(projectId, (err, state) => { if (err) return next(err) res.json({ @@ -379,7 +545,16 @@ export function getResyncPending(req, res, next) { }) } +// See getResyncPendingSchema: no numeric-id union, for the same reason. const getDebugInfoSchema = z.object({ + params: z.strictObject({ + project_id: zz.objectId(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getDebugInfoFallbackSchema = z.object({ params: z.object({ project_id: zz.objectId(), }), @@ -388,7 +563,9 @@ const getDebugInfoSchema = z.object({ export function getDebugInfo(req, res, next) { const { params: { project_id: projectId }, - } = parseReq(req, getDebugInfoSchema) + } = parseReq(req, getDebugInfoSchema, { + fallbackSchema: getDebugInfoFallbackSchema, + }) SyncManager.getResyncState(projectId, (err, state) => { if (err) return next(err) ErrorRecorder.getFailureRecord(projectId, (err, failureRecord) => { @@ -409,13 +586,23 @@ export function getDebugInfo(req, res, next) { } const latestVersionSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const latestVersionFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), }) export function latestVersion(req, res, next) { - const { params } = parseReq(req, latestVersionSchema) + const { params } = parseReq(req, latestVersionSchema, { + fallbackSchema: latestVersionFallbackSchema, + }) const projectId = params.project_id logger.debug({ projectId }, 'compressing project history and getting version') UpdatesProcessor.processUpdatesForProject(projectId, error => { @@ -445,15 +632,27 @@ export function latestVersion(req, res, next) { } const getFileSnapshotSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + version: z.coerce.number().int(), + pathname: zz.filepath(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getFileSnapshotFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, version: z.coerce.number().int(), pathname: z.string(), }), }) export function getFileSnapshot(req, res, next) { - const { params } = parseReq(req, getFileSnapshotSchema) + const { params } = parseReq(req, getFileSnapshotSchema, { + fallbackSchema: getFileSnapshotFallbackSchema, + }) const { project_id: projectId, version, pathname } = params SnapshotManager.getFileSnapshotStream( projectId, @@ -472,15 +671,27 @@ export function getFileSnapshot(req, res, next) { } const getRangesSnapshotSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + version: z.coerce.number().int(), + pathname: zz.filepath(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getRangesSnapshotFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, version: z.coerce.number().int(), pathname: z.string(), }), }) export function getRangesSnapshot(req, res, next) { - const { params } = parseReq(req, getRangesSnapshotSchema) + const { params } = parseReq(req, getRangesSnapshotSchema, { + fallbackSchema: getRangesSnapshotFallbackSchema, + }) const { project_id: projectId, version, pathname } = params SnapshotManager.getRangesSnapshot( projectId, @@ -496,15 +707,27 @@ export function getRangesSnapshot(req, res, next) { } const getFileMetadataSnapshotSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + version: z.coerce.number().int(), + pathname: zz.filepath(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getFileMetadataSnapshotFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, version: z.coerce.number().int(), pathname: z.string(), }), }) export function getFileMetadataSnapshot(req, res, next) { - const { params } = parseReq(req, getFileMetadataSnapshotSchema) + const { params } = parseReq(req, getFileMetadataSnapshotSchema, { + fallbackSchema: getFileMetadataSnapshotFallbackSchema, + }) const { project_id: projectId, version, pathname } = params SnapshotManager.getFileMetadataSnapshot( projectId, @@ -520,13 +743,23 @@ export function getFileMetadataSnapshot(req, res, next) { } const getLatestSnapshotSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getLatestSnapshotFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), }) export function getLatestSnapshot(req, res, next) { - const { params } = parseReq(req, getLatestSnapshotSchema) + const { params } = parseReq(req, getLatestSnapshotSchema, { + fallbackSchema: getLatestSnapshotFallbackSchema, + }) const { project_id: projectId } = params WebApiManager.getHistoryId(projectId, (error, historyId) => { if (error) return next(OError.tag(error)) @@ -545,8 +778,19 @@ export function getLatestSnapshot(req, res, next) { } const getChangesInChunkSinceSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), + query: z.strictObject({ + since: z.coerce.number().int().min(0), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getChangesInChunkSinceFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), query: z.object({ since: z.coerce.number().int().min(0), @@ -554,7 +798,9 @@ const getChangesInChunkSinceSchema = z.object({ }) export function getChangesInChunkSince(req, res, next) { - const { query, params } = parseReq(req, getChangesInChunkSinceSchema) + const { query, params } = parseReq(req, getChangesInChunkSinceSchema, { + fallbackSchema: getChangesInChunkSinceFallbackSchema, + }) const { project_id: projectId } = params const { since } = query WebApiManager.getHistoryId(projectId, (error, historyId) => { @@ -578,14 +824,25 @@ export function getChangesInChunkSince(req, res, next) { } const getProjectSnapshotSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + version: z.coerce.number().int(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getProjectSnapshotFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, version: z.coerce.number().int(), }), }) export function getProjectSnapshot(req, res, next) { - const { params } = parseReq(req, getProjectSnapshotSchema) + const { params } = parseReq(req, getProjectSnapshotSchema, { + fallbackSchema: getProjectSnapshotFallbackSchema, + }) const { project_id: projectId, version } = params SnapshotManager.getProjectSnapshot( projectId, @@ -600,14 +857,25 @@ export function getProjectSnapshot(req, res, next) { } const getPathsAtVersionSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + version: z.coerce.number().int(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getPathsAtVersionFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, version: z.coerce.number().int(), }), }) export function getPathsAtVersion(req, res, next) { - const { params } = parseReq(req, getPathsAtVersionSchema) + const { params } = parseReq(req, getPathsAtVersionSchema, { + fallbackSchema: getPathsAtVersionFallbackSchema, + }) const { project_id: projectId, version } = params SnapshotManager.getPathsAtVersion(projectId, version, (error, result) => { if (error != null) { @@ -640,8 +908,28 @@ export function checkLock(req, res) { } const resyncProjectSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), + query: z.strictObject({ + force: z.stringbool().default(false), + recoverCorruptedFiles: z.stringbool().default(false), + }), + body: z.strictObject({ + force: z.boolean().default(false), + recoverCorruptedFiles: z.boolean().default(false), + // shared with SyncUpdateExpander, which types this as RawOrigin + // (overleaf-editor-core/lib/types.js) + origin: editorCoreSchemas.rawOrigin.optional(), + historyRangesMigration: z.enum(['forwards', 'backwards']).optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const resyncProjectFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), query: z.object({ force: z.stringbool().default(false), @@ -660,7 +948,9 @@ const resyncProjectSchema = z.object({ }) export function resyncProject(req, res, next) { - const { query, params, body } = parseReq(req, resyncProjectSchema) + const { query, params, body } = parseReq(req, resyncProjectSchema, { + fallbackSchema: resyncProjectFallbackSchema, + }) const projectId = params.project_id const options = {} if (body.origin) { @@ -704,8 +994,19 @@ export function resyncProject(req, res, next) { } const forceDebugProjectSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), + query: z.strictObject({ + clear: z.stringbool().default(false), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const forceDebugProjectFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), query: z.object({ clear: z.stringbool().default(false), @@ -713,7 +1014,9 @@ const forceDebugProjectSchema = z.object({ }) export function forceDebugProject(req, res, next) { - const { query, params } = parseReq(req, forceDebugProjectSchema) + const { query, params } = parseReq(req, forceDebugProjectSchema, { + fallbackSchema: forceDebugProjectFallbackSchema, + }) const projectId = params.project_id // set the debug flag to true unless we see ?clear=true const state = !query.clear @@ -757,13 +1060,23 @@ export function getQueueCounts(req, res, next) { } const getLabelsSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const getLabelsFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), }) export function getLabels(req, res, next) { - const { params } = parseReq(req, getLabelsSchema) + const { params } = parseReq(req, getLabelsSchema, { + fallbackSchema: getLabelsFallbackSchema, + }) const projectId = params.project_id HistoryApiManager.shouldUseProjectHistory( projectId, @@ -786,8 +1099,27 @@ export function getLabels(req, res, next) { } const createLabelSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + // vestigial: no route mounts createLabel with a :user_id param, so this + // is always undefined; kept only to preserve the params/body dual-read + // rollout behaviour below (see the comment at its use site) + user_id: zz.objectId().optional(), + }), + body: z.strictObject({ + version: z.number().int(), + comment: z.string(), + created_at: zz.datetime().optional(), + validate_exists: z.boolean().default(true), + user_id: zz.objectId().nullable().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const createLabelFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, user_id: zz.objectId().optional(), }), body: z.object({ @@ -800,7 +1132,9 @@ const createLabelSchema = z.object({ }) export function createLabel(req, res, next) { - const { params, body } = parseReq(req, createLabelSchema) + const { params, body } = parseReq(req, createLabelSchema, { + fallbackSchema: createLabelFallbackSchema, + }) const { project_id: projectId, user_id: userIdParam } = params const { version, @@ -858,15 +1192,27 @@ export function createLabel(req, res, next) { * delete a label regardless of the current user, then use `deleteLabel` instead. */ const deleteLabelForUserSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + user_id: zz.objectId(), + label_id: zz.objectId(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const deleteLabelForUserFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, user_id: zz.objectId(), label_id: zz.objectId(), }), }) export function deleteLabelForUser(req, res, next) { - const { params } = parseReq(req, deleteLabelForUserSchema) + const { params } = parseReq(req, deleteLabelForUserSchema, { + fallbackSchema: deleteLabelForUserFallbackSchema, + }) const { project_id: projectId, user_id: userId, label_id: labelId } = params LabelsManager.deleteLabelForUser(projectId, userId, labelId, error => { @@ -878,14 +1224,25 @@ export function deleteLabelForUser(req, res, next) { } const deleteLabelSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + label_id: zz.objectId(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const deleteLabelFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, label_id: zz.objectId(), }), }) export function deleteLabel(req, res, next) { - const { params } = parseReq(req, deleteLabelSchema) + const { params } = parseReq(req, deleteLabelSchema, { + fallbackSchema: deleteLabelFallbackSchema, + }) const { project_id: projectId, label_id: labelId } = params LabelsManager.deleteLabel(projectId, labelId, error => { @@ -897,18 +1254,31 @@ export function deleteLabel(req, res, next) { } const retryFailuresSchema = z.object({ - query: z.object({ + query: z.strictObject({ 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.url().optional(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const retryFailuresFallbackSchema = z.object({ + query: z.object({ + failureType: z.enum(['soft', 'hard']).optional(), + timeout: z.coerce.number().int().default(300), + limit: z.coerce.number().int().default(100), callbackUrl: z.string().optional(), }), }) export function retryFailures(req, res, next) { - const { query } = parseReq(req, retryFailuresSchema) + const { query } = parseReq(req, retryFailuresSchema, { + fallbackSchema: retryFailuresFallbackSchema, + }) const { failureType, timeout, limit, callbackUrl } = query if (callbackUrl) { // send response but run in background when callbackUrl provided @@ -920,6 +1290,10 @@ export function retryFailures(req, res, next) { if (callbackUrl) { // if present, notify the callbackUrl on success if (!error) { + // req.headers is not covered by the request-input lockdown (only + // body/query/params are), so this dynamic X-CALLBACK-* prefix scan + // doesn't need parseReq()/getRawReqInput() — it's forwarded + // verbatim to the caller-supplied callback, never read by name. // Needs Node 12 // const callbackHeaders = Object.fromEntries(Object.entries(req.headers || {}).filter(([k,v]) => k.match(/^X-CALLBACK-/i))) const callbackHeaders = {} @@ -944,6 +1318,15 @@ export function retryFailures(req, res, next) { } const transferLabelsSchema = z.object({ + params: z.strictObject({ + from_user: zz.objectId(), + to_user: zz.objectId(), + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const transferLabelsFallbackSchema = z.object({ params: z.object({ from_user: zz.objectId(), to_user: zz.objectId(), @@ -951,7 +1334,9 @@ const transferLabelsSchema = z.object({ }) export function transferLabels(req, res, next) { - const { params } = parseReq(req, transferLabelsSchema) + const { params } = parseReq(req, transferLabelsSchema, { + fallbackSchema: transferLabelsFallbackSchema, + }) const { from_user: fromUser, to_user: toUser } = params LabelsManager.transferLabels(fromUser, toUser, error => { if (error != null) { @@ -962,13 +1347,23 @@ export function transferLabels(req, res, next) { } const deleteProjectSchema = z.object({ + params: z.strictObject({ + project_id: historyIdSchema, + }), +}) + +// Rollout-temporary fallback (pre-refinement schema from main); delete +// when this route's REQ_VALIDATION_MODE instrumentation is removed. +const deleteProjectFallbackSchema = z.object({ params: z.object({ - project_id: zz.objectId().or(z.coerce.number()), + project_id: historyIdSchema, }), }) export function deleteProject(req, res, next) { - const { params } = parseReq(req, deleteProjectSchema) + const { params } = parseReq(req, deleteProjectSchema, { + fallbackSchema: deleteProjectFallbackSchema, + }) const { project_id: projectId } = params // clear the timestamp before clearing the queue, // because the queue location is used in the migration diff --git a/services/project-history/test/acceptance/js/CloneProjectTests.js b/services/project-history/test/acceptance/js/CloneProjectTests.js new file mode 100644 index 0000000000..048e5766fb --- /dev/null +++ b/services/project-history/test/acceptance/js/CloneProjectTests.js @@ -0,0 +1,83 @@ +import { expect } from 'chai' +import mongodb from 'mongodb-legacy' +import nock from 'nock' +import { expectValidationError } from '@overleaf/validation-tools/testUtils.js' +import * as ProjectHistoryClient from './helpers/ProjectHistoryClient.js' +import * as ProjectHistoryApp from './helpers/ProjectHistoryApp.js' +const { ObjectId } = mongodb + +const MockHistoryStore = () => nock('http://127.0.0.1:3100') +const MockWeb = () => nock('http://127.0.0.1:3000') + +describe('CloneProject', function () { + beforeEach(async function () { + await ProjectHistoryApp.ensureRunning() + + this.sourceProjectId = new ObjectId().toString() + this.targetProjectId = new ObjectId().toString() + this.sourceHistoryId = new ObjectId().toString() + this.targetHistoryId = new ObjectId().toString() + + MockHistoryStore().post('/api/projects').reply(200, { + projectId: this.sourceHistoryId, + }) + await ProjectHistoryClient.initializeProject(this.sourceHistoryId) + + MockWeb() + .get(`/project/${this.sourceProjectId}/details`) + .reply(200, { + name: 'Source Project', + overleaf: { history: { id: this.sourceHistoryId } }, + }) + MockWeb() + .get(`/project/${this.targetProjectId}/details`) + .reply(200, { + name: 'Target Project', + overleaf: { history: { id: this.targetHistoryId } }, + }) + }) + + afterEach(function () { + nock.cleanAll() + }) + + it('should reject a malformed source project id', async function () { + let err + try { + await ProjectHistoryClient.cloneProject( + 'not-a-valid-id', + this.targetProjectId + ) + expect.fail('should have thrown') + } catch (error) { + err = error + } + expectValidationError(err, 404, 'project_id') + }) + + it('should reject a request missing targetProjectId', async function () { + let err + try { + await ProjectHistoryClient.cloneProject(this.sourceProjectId, undefined) + expect.fail('should have thrown') + } catch (error) { + err = error + } + expectValidationError(err, 400, 'targetProjectId') + }) + + it('should clone the project history data', async function () { + MockHistoryStore() + .post(`/api/projects/${this.sourceHistoryId}/clone`, { + targetProjectId: this.targetHistoryId, + }) + .reply(200, '') + + const { response, body } = await ProjectHistoryClient.cloneProject( + this.sourceProjectId, + this.targetProjectId + ) + expect(response.status).to.equal(200) + expect(body).to.include('done') + }) +}) diff --git a/services/project-history/test/acceptance/js/DebugInfoTests.js b/services/project-history/test/acceptance/js/DebugInfoTests.js new file mode 100644 index 0000000000..788b5bb194 --- /dev/null +++ b/services/project-history/test/acceptance/js/DebugInfoTests.js @@ -0,0 +1,86 @@ +import { expect } from 'chai' +import mongodb from 'mongodb-legacy' +import nock from 'nock' +import * as ProjectHistoryClient from './helpers/ProjectHistoryClient.js' +import * as ProjectHistoryApp from './helpers/ProjectHistoryApp.js' +const { ObjectId } = mongodb + +const MockHistoryStore = () => nock('http://127.0.0.1:3100') +const MockWeb = () => nock('http://127.0.0.1:3000') + +describe('DebugInfo', function () { + beforeEach(async function () { + await ProjectHistoryApp.ensureRunning() + + this.historyId = new ObjectId().toString() + this.projectId = new ObjectId().toString() + + MockHistoryStore().post('/api/projects').reply(200, { + projectId: this.historyId, + }) + MockWeb() + .get(`/project/${this.projectId}/details`) + .reply(200, { + name: 'Test Project', + overleaf: { history: { id: this.historyId } }, + }) + await ProjectHistoryClient.initializeProject(this.historyId) + }) + + afterEach(function () { + nock.cleanAll() + }) + + describe('getDebugInfo', function () { + it('should return the sync state and failure record for a project', async function () { + const info = await ProjectHistoryClient.getDebugInfo(this.projectId) + expect(info.failureRecord).to.be.null + expect(info.syncState.resyncPending).to.equal(false) + expect(info.syncState.resyncCount).to.equal(0) + }) + }) + + describe('forceDebugProject', function () { + it('should set the forceDebug flag and return the failure record', async function () { + const result = await ProjectHistoryClient.forceDebugProject( + this.projectId + ) + expect(result.forceDebug).to.equal(true) + }) + + it('should clear the forceDebug flag when clear=true', async function () { + await ProjectHistoryClient.forceDebugProject(this.projectId) + const result = await ProjectHistoryClient.forceDebugProject( + this.projectId, + { clear: true } + ) + expect(result.forceDebug).to.equal(false) + }) + }) +}) + +describe('Status', function () { + beforeEach(async function () { + await ProjectHistoryApp.ensureRunning() + }) + + describe('getQueueCounts', function () { + it('should return the count of queued projects', async function () { + const counts = await ProjectHistoryClient.getQueueCounts() + expect(counts).to.have.property('queuedProjects') + }) + }) + + describe('getFailuresFull', function () { + it('should return the full failure records', async function () { + const projectId = new ObjectId().toString() + await ProjectHistoryClient.setFailures([ + { project_id: projectId, attempts: 1, error: 'Error: some-error' }, + ]) + const failures = await ProjectHistoryClient.getFailuresFull() + expect(failures).to.be.an('array') + const failure = failures.find(f => f.project_id === projectId) + expect(failure).to.include({ project_id: projectId, attempts: 1 }) + }) + }) +}) diff --git a/services/project-history/test/acceptance/js/FlushManagerTests.js b/services/project-history/test/acceptance/js/FlushManagerTests.js index 39df23d747..61db2d6880 100644 --- a/services/project-history/test/acceptance/js/FlushManagerTests.js +++ b/services/project-history/test/acceptance/js/FlushManagerTests.js @@ -55,6 +55,20 @@ describe('Flushing old queues', function () { nock.cleanAll() }) + describe('flushing a project with the background flag', function () { + // document-updater's background flush (HistoryManager.js + // flushProjectChangesAsync) sends ?background=true on the plain + // project flush route; project-history does not read the flag, it + // just needs to accept it rather than reject it as an unknown field. + it('should accept the background query flag', async function () { + const response = await fetchNothing( + `http://127.0.0.1:3054/project/${this.projectId}/flush?background=true`, + { method: 'POST' } + ) + expect(response.status).to.equal(204) + }) + }) + describe('retrying an unflushed project', function () { describe('when the update is older than the cutoff', function () { beforeEach(async function () { @@ -165,6 +179,10 @@ describe('Flushing old queues', function () { expect(statusCode).to.equal(423) } finally { await lockRClient.del(key) + // the lock timeout above is recorded as a failure (ErrorRecorder + // categorizes it as "lock-overrun"); clear it so it doesn't leak + // into other tests' failure-count assertions (e.g. RetryTests) + await ProjectHistoryClient.clearFailure(this.projectId) } }) }) diff --git a/services/project-history/test/acceptance/js/RangesAndMetadataSnapshotTests.js b/services/project-history/test/acceptance/js/RangesAndMetadataSnapshotTests.js new file mode 100644 index 0000000000..9826f6f31c --- /dev/null +++ b/services/project-history/test/acceptance/js/RangesAndMetadataSnapshotTests.js @@ -0,0 +1,109 @@ +import { expect } from 'chai' +import mongodb from 'mongodb-legacy' +import nock from 'nock' +import * as ProjectHistoryClient from './helpers/ProjectHistoryClient.js' +import * as ProjectHistoryApp from './helpers/ProjectHistoryApp.js' +const { ObjectId } = mongodb + +const MockHistoryStore = () => nock('http://127.0.0.1:3100') +const MockWeb = () => nock('http://127.0.0.1:3000') + +const fixture = path => new URL(`../fixtures/${path}`, import.meta.url) + +describe('RangesAndMetadataSnapshot', function () { + beforeEach(async function () { + await ProjectHistoryApp.ensureRunning() + + this.historyId = new ObjectId().toString() + MockHistoryStore().post('/api/projects').reply(200, { + projectId: this.historyId, + }) + + const v1Project = await ProjectHistoryClient.initializeProject( + this.historyId + ) + this.projectId = new ObjectId().toString() + MockWeb() + .get(`/project/${this.projectId}/details`) + .reply(200, { + name: 'Test Project', + overleaf: { history: { id: v1Project.id } }, + }) + .persist() + + MockHistoryStore() + .get(`/api/projects/${this.historyId}/versions/4/history`) + .replyWithFile(200, fixture('chunks/4-6.json')) + .persist() + + // getRangesSnapshot loads the full blob content of editable (text) + // files to compute tracked-changes/comments, even when there are none + MockHistoryStore() + .get( + `/api/projects/${this.historyId}/blobs/c6654ea913979e13e22022653d284444f284a172` + ) + .replyWithFile( + 200, + fixture('blobs/c6654ea913979e13e22022653d284444f284a172') + ) + .persist() + }) + + afterEach(function () { + nock.cleanAll() + }) + + describe('getRangesSnapshot', function () { + it('should return empty ranges for a plain text file', async function () { + const ranges = await ProjectHistoryClient.getRangesSnapshot( + this.projectId, + 'foo.tex', + 4 + ) + expect(ranges).to.deep.equal({ changes: [], comments: [] }) + }) + + it('should 404 for a pathname that does not exist at that version', async function () { + try { + await ProjectHistoryClient.getRangesSnapshot( + this.projectId, + 'missing.tex', + 4 + ) + expect.fail('should have thrown') + } catch (error) { + expect(error.response.status).to.equal(404) + } + }) + }) + + describe('getFileMetadataSnapshot', function () { + it('should return the metadata for a file that has some', async function () { + const data = await ProjectHistoryClient.getFileMetadataSnapshot( + this.projectId, + 'main.tex', + 4 + ) + expect(data).to.deep.equal({ metadata: { main: true } }) + }) + + it('should return no metadata for a file that has none', async function () { + const data = await ProjectHistoryClient.getFileMetadataSnapshot( + this.projectId, + 'foo.tex', + 4 + ) + expect(data).to.deep.equal({}) + }) + }) + + describe('getPathsAtVersion', function () { + it('should return the paths of all files at that version', async function () { + const data = await ProjectHistoryClient.getPathsAtVersion( + this.projectId, + 4 + ) + expect(data.paths.slice().sort()).to.deep.equal(['foo.tex', 'main.tex']) + }) + }) +}) diff --git a/services/project-history/test/acceptance/js/ValidationTests.js b/services/project-history/test/acceptance/js/ValidationTests.js new file mode 100644 index 0000000000..558e0ecff4 --- /dev/null +++ b/services/project-history/test/acceptance/js/ValidationTests.js @@ -0,0 +1,95 @@ +import { expect } from 'chai' +import mongodb from 'mongodb-legacy' +import { fetchNothing } from '@overleaf/fetch-utils' +import { expectValidationError } from '@overleaf/validation-tools/testUtils.js' +import * as ProjectHistoryApp from './helpers/ProjectHistoryApp.js' +const { ObjectId } = mongodb + +describe('Validation', function () { + beforeEach(async function () { + await ProjectHistoryApp.ensureRunning() + }) + + it('should return 404 for a malformed project id in the URL path', async function () { + let err + try { + await fetchNothing( + 'http://127.0.0.1:3054/project/not-a-valid-id/snapshot' + ) + expect.fail('should have thrown') + } catch (error) { + err = error + } + expectValidationError(err, 404, 'project_id') + }) + + it('should return 404 for a path-traversal pathname param', async function () { + const projectId = new ObjectId().toString() + let err + try { + await fetchNothing( + `http://127.0.0.1:3054/project/${projectId}/version/1/${encodeURIComponent( + '../../../etc/passwd' + )}` + ) + expect.fail('should have thrown') + } catch (error) { + err = error + } + expectValidationError(err, 404, 'pathname') + }) + + it('should return 400 for a path-traversal pathname query param', async function () { + const projectId = new ObjectId().toString() + let err + try { + await fetchNothing( + `http://127.0.0.1:3054/project/${projectId}/diff?pathname=${encodeURIComponent( + '../../../etc/passwd' + )}&from=1&to=2` + ) + expect.fail('should have thrown') + } catch (error) { + err = error + } + expectValidationError(err, 400, 'pathname') + }) + + it('should return 400 for an unknown field in the resync body', async function () { + const projectId = new ObjectId().toString() + let err + try { + await fetchNothing(`http://127.0.0.1:3054/project/${projectId}/resync`, { + method: 'POST', + json: { notAField: true }, + }) + expect.fail('should have thrown') + } catch (error) { + err = error + } + expectValidationError(err, 400, 'notAField') + }) + + it('should return 400 for a non-datetime created_at in the label body', async function () { + // created_at previously accepted a bare string (e.g. an epoch-ms + // string), so an invalid value would silently become an Invalid Date + // downstream instead of being rejected at the schema boundary -- see + // zod-string-field-hardening.md's housekeeping note. + const projectId = new ObjectId().toString() + let err + try { + await fetchNothing(`http://127.0.0.1:3054/project/${projectId}/labels`, { + method: 'POST', + json: { + comment: 'a comment', + version: 1, + created_at: 'not-a-datetime', + }, + }) + expect.fail('should have thrown') + } catch (error) { + err = error + } + expectValidationError(err, 400, 'created_at') + }) +}) diff --git a/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js b/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js index f3e87b59a3..e52a68da0e 100644 --- a/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js +++ b/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js @@ -224,6 +224,10 @@ export function getFailure(projectId, callback) { db.projectHistoryFailures.findOne({ project_id: projectId }, callback) } +export async function clearFailure(projectId) { + await db.projectHistoryFailures.deleteOne({ project_id: projectId }) +} + export async function transferLabelOwnership(fromUser, toUser) { const response = await fetchNothing( `http://127.0.0.1:3054/user/${fromUser}/labels/transfer/${toUser}`, @@ -253,6 +257,60 @@ export async function getResyncPending(projectId) { ) } +export async function getDebugInfo(projectId) { + return await fetchJson( + `http://127.0.0.1:3054/project/${projectId}/debug-info` + ) +} + +export async function forceDebugProject(projectId, options = {}) { + const url = new URL(`http://127.0.0.1:3054/project/${projectId}/force`) + if (options.clear) { + url.searchParams.set('clear', 'true') + } + return await fetchJson(url.toString(), { method: 'POST' }) +} + +export async function getRangesSnapshot(projectId, pathname, version) { + return await fetchJson( + `http://127.0.0.1:3054/project/${projectId}/ranges/version/${version}/${encodeURIComponent( + pathname + )}` + ) +} + +export async function getFileMetadataSnapshot(projectId, pathname, version) { + return await fetchJson( + `http://127.0.0.1:3054/project/${projectId}/metadata/version/${version}/${encodeURIComponent( + pathname + )}` + ) +} + +export async function getPathsAtVersion(projectId, version) { + return await fetchJson( + `http://127.0.0.1:3054/project/${projectId}/paths/version/${version}` + ) +} + +export async function getQueueCounts() { + return await fetchJson('http://127.0.0.1:3054/status/queue') +} + +export async function getFailuresFull() { + return await fetchJson('http://127.0.0.1:3054/status/failures-full') +} + +export async function cloneProject(sourceProjectId, targetProjectId) { + return await fetchStringWithResponse( + `http://127.0.0.1:3054/project/${sourceProjectId}/clone`, + { + method: 'POST', + json: { targetProjectId }, + } + ) +} + export async function injectStuckSyncState(projectId, docPaths) { await db.projectHistorySyncState.replaceOne( { project_id: new ObjectId(projectId) }, diff --git a/services/project-history/test/unit/js/HistoryStoreManager/HistoryStoreManagerTests.js b/services/project-history/test/unit/js/HistoryStoreManager/HistoryStoreManagerTests.js index 3a6771c053..7b568d93fd 100644 --- a/services/project-history/test/unit/js/HistoryStoreManager/HistoryStoreManagerTests.js +++ b/services/project-history/test/unit/js/HistoryStoreManager/HistoryStoreManagerTests.js @@ -2,6 +2,7 @@ import sinon from 'sinon' import { expect } from 'chai' import { strict as esmock } from 'esmock' import EventEmitter from 'node:events' +import OError from '@overleaf/o-error' import { RequestFailedError } from '@overleaf/fetch-utils' import * as Errors from '../../../../app/js/Errors.js' @@ -396,6 +397,107 @@ describe('HistoryStoreManager', function () { }) }) + describe('_requestHistoryService error handling', function () { + function itCallsBackWithTheHistoryServiceError(statusCode) { + it('should call the callback with an error naming the status code', function () { + const err = this.callback.firstCall.args[0] + expect(err.message).to.equal( + `history store a non-success status code: ${statusCode}` + ) + }) + } + + describe('with a 409 response carrying a body', function () { + beforeEach(function (done) { + this.FetchUtils.fetchString.rejects( + new RequestFailedError( + `${this.settings.overleaf.history.host}/projects/${this.historyId}/latest/history`, + { method: 'GET' }, + { status: 409 }, + 'conflict-detail' + ) + ) + this.HistoryStoreManager.getMostRecentVersion( + this.projectId, + this.historyId, + (...args) => { + this.callback(...args) + done() + } + ) + }) + + itCallsBackWithTheHistoryServiceError(409) + + it('should include the response body and status code in the error info', function () { + const err = this.callback.firstCall.args[0] + const info = OError.getFullInfo(err) + expect(info.body).to.equal('conflict-detail') + expect(info.statusCode).to.equal(409) + }) + }) + + describe('with a 422 response carrying a body', function () { + beforeEach(function (done) { + this.FetchUtils.fetchString.rejects( + new RequestFailedError( + `${this.settings.overleaf.history.host}/projects/${this.historyId}/latest/history`, + { method: 'GET' }, + { status: 422 }, + 'schema-validation-detail' + ) + ) + this.HistoryStoreManager.getMostRecentVersion( + this.projectId, + this.historyId, + (...args) => { + this.callback(...args) + done() + } + ) + }) + + itCallsBackWithTheHistoryServiceError(422) + + it('should include the response body and status code in the error info', function () { + const err = this.callback.firstCall.args[0] + const info = OError.getFullInfo(err) + expect(info.body).to.equal('schema-validation-detail') + expect(info.statusCode).to.equal(422) + }) + }) + + describe('with a 500 response and no informative body', function () { + beforeEach(function (done) { + this.FetchUtils.fetchString.rejects( + new RequestFailedError( + `${this.settings.overleaf.history.host}/projects/${this.historyId}/latest/history`, + { method: 'GET' }, + { status: 500 }, + null + ) + ) + this.HistoryStoreManager.getMostRecentVersion( + this.projectId, + this.historyId, + (...args) => { + this.callback(...args) + done() + } + ) + }) + + itCallsBackWithTheHistoryServiceError(500) + + it('should not include a response body in the error info', function () { + const err = this.callback.firstCall.args[0] + const info = OError.getFullInfo(err) + expect(info.body).to.equal(null) + expect(info.statusCode).to.equal(500) + }) + }) + }) + describe('createBlobForUpdate', function () { beforeEach(function () { this.fileStream = {} @@ -735,4 +837,77 @@ describe('HistoryStoreManager', function () { ) }) }) + + describe('sendChanges', function () { + beforeEach(function () { + this.changes = [{ op: 'mock-change' }] + this.endVersion = 42 + }) + + describe('successfully', function () { + beforeEach(function (done) { + this.FetchUtils.fetchString.resolves( + JSON.stringify({ resyncNeeded: true }) + ) + this.HistoryStoreManager.sendChanges( + this.projectId, + this.historyId, + this.changes, + this.endVersion, + (...args) => { + this.callback(...args) + done() + } + ) + }) + + it('should send the changes to the history store', function () { + expect(this.FetchUtils.fetchString).to.have.been.calledWithMatch( + `${this.settings.overleaf.history.host}/projects/${this.historyId}/legacy_changes?end_version=${this.endVersion}`, + { + method: 'POST', + json: this.changes, + } + ) + }) + + it('should call the callback with the resync flag from the response', function () { + expect(this.callback).to.have.been.calledWith(null, { + resyncNeeded: true, + }) + }) + }) + + describe('when the history store returns a non-success status code', function () { + beforeEach(function (done) { + this.FetchUtils.fetchString.rejects( + new RequestFailedError( + `${this.settings.overleaf.history.host}/projects/${this.historyId}/legacy_changes`, + { method: 'POST' }, + { status: 409 }, + 'conflict-detail' + ) + ) + this.HistoryStoreManager.sendChanges( + this.projectId, + this.historyId, + this.changes, + this.endVersion, + (...args) => { + this.callback(...args) + done() + } + ) + }) + + it('should call the callback with an error', function () { + expect(this.callback.firstCall.args[0]).to.be.an.instanceOf(Error) + }) + + it('should keep the response body from the history store in the error info', function () { + const err = this.callback.firstCall.args[0] + expect(OError.getFullInfo(err).body).to.equal('conflict-detail') + }) + }) + }) }) diff --git a/services/project-history/test/unit/js/HttpController/HttpControllerTests.js b/services/project-history/test/unit/js/HttpController/HttpControllerTests.js index 7d819c22cb..58268c7b6c 100644 --- a/services/project-history/test/unit/js/HttpController/HttpControllerTests.js +++ b/services/project-history/test/unit/js/HttpController/HttpControllerTests.js @@ -88,7 +88,7 @@ describe('HttpController', function () { describe('getProjectBlob', function () { beforeEach(function () { - this.blobHash = 'abcd' + this.blobHash = 'a'.repeat(40) this.stream = {} this.historyId = new ObjectId().toString() this.HistoryStoreManager.getProjectBlobStream.yields(null, this.stream) @@ -414,7 +414,7 @@ describe('HttpController', function () { body: { version: (this.version = 1), comment: (this.comment = 'a comment'), - created_at: (this.created_at = Date.now().toString()), + created_at: (this.created_at = new Date()), validate_exists: true, user_id: this.userId, },