From 2024f85941ef3e997523ad7ba97ab3f6eb5276f7 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:02:51 -0400 Subject: [PATCH] Merge pull request #36842 from overleaf/em-tpds-typescript Convert third-party-datastore to TypeScript GitOrigin-RevId: 028172063d0cfe03c0f105f48ab66020c79ea50e --- libraries/logger/logging-manager.js | 27 ++++++++++++ .../logger/test/unit/logging-manager-tests.js | 32 +++++++++++++++ .../test/unit/src/zodHelpers.test.ts | 33 +++++++++++++++ libraries/validation-tools/zodHelpers.js | 17 ++++++++ .../ThirdPartyDataStore/TpdsUpdateSender.mjs | 8 +++- tsconfig.backend.json | 3 ++ yarn.lock | 41 +++++++++++++++++++ 7 files changed, 159 insertions(+), 2 deletions(-) diff --git a/libraries/logger/logging-manager.js b/libraries/logger/logging-manager.js index 56ef50203e..847ac1a99e 100644 --- a/libraries/logger/logging-manager.js +++ b/libraries/logger/logging-manager.js @@ -16,6 +16,14 @@ const { const EXIT_DELAY = 500 const LoggingManager = { + /** + * The serializers registered by addSerializer, kept here rather than only on + * the logger so that they survive a later initialize() call. A module that + * registers one as it is imported would otherwise lose it, since a service + * initializes its logger after its imports have run. + */ + customSerializers: {}, + /** * @param {string} name - The name of the logger */ @@ -34,6 +42,7 @@ const LoggingManager = { error: Serializers.err, req: Serializers.req, res: Serializers.res, + ...this.customSerializers, }, streams: options.streams ?? [this._getOutputStreamConfig()], }) @@ -44,6 +53,24 @@ const LoggingManager = { return this }, + /** + * Register a serializer, which decides what gets recorded for a named log + * field, e.g. `logger.error({ dropboxResponseError: err }, '...')`. Callers + * add their own on top of the ones initialize() sets up, and keep them across + * a later initialize(), so registering one as a module is imported works. + * + * A serializer takes whatever was logged under that field, so it receives an + * unknown and narrows it itself, and returns the value to record in its + * place, which is only ever JSON encoded. + * + * @param {string} name - the log field the serializer applies to + * @param {(value: unknown) => unknown} serializer + */ + addSerializer(name, serializer) { + this.customSerializers[name] = serializer + this.logger.serializers[name] = serializer + }, + /** * @param {Record|string} attributes - Attributes to log (nice serialization for err, req, res) * @param {string} [message] - Optional message diff --git a/libraries/logger/test/unit/logging-manager-tests.js b/libraries/logger/test/unit/logging-manager-tests.js index b52a93eef3..932ee231e9 100644 --- a/libraries/logger/test/unit/logging-manager-tests.js +++ b/libraries/logger/test/unit/logging-manager-tests.js @@ -17,6 +17,7 @@ describe('LoggingManager', function () { info: sinon.stub(), level: sinon.stub(), warn: sinon.stub(), + serializers: {}, } this.Bunyan = { createLogger: sinon.stub().returns(this.bunyanLogger), @@ -126,6 +127,37 @@ describe('LoggingManager', function () { }) }) + describe('addSerializer', function () { + beforeEach(function () { + this.serializer = sinon.stub() + }) + + it('registers the serializer on the current logger', function () { + this.LoggingManager.addSerializer('custom', this.serializer) + expect(this.bunyanLogger.serializers.custom).to.equal(this.serializer) + }) + + it('keeps the serializer through a later initialize', function () { + // A module registering a serializer as it is imported does so before the + // service initializes its own logger, which replaces the bunyan instance. + this.LoggingManager.addSerializer('custom', this.serializer) + this.Bunyan.createLogger.reset() + this.LoggingManager.initialize(this.loggerName) + expect( + this.Bunyan.createLogger.firstCall.args[0].serializers.custom + ).to.equal(this.serializer) + }) + + it('keeps the serializers initialize sets up', function () { + this.LoggingManager.addSerializer('custom', this.serializer) + this.Bunyan.createLogger.reset() + this.LoggingManager.initialize(this.loggerName) + expect( + this.Bunyan.createLogger.firstCall.args[0].serializers + ).to.include.keys('err', 'error', 'req', 'res') + }) + }) + describe('bunyan logging', function () { beforeEach(function () { this.logArgs = [{ foo: 'bar' }, 'foo', 'bar'] diff --git a/libraries/validation-tools/test/unit/src/zodHelpers.test.ts b/libraries/validation-tools/test/unit/src/zodHelpers.test.ts index baac7fe03f..06b849282b 100644 --- a/libraries/validation-tools/test/unit/src/zodHelpers.test.ts +++ b/libraries/validation-tools/test/unit/src/zodHelpers.test.ts @@ -1,10 +1,43 @@ import { zz } from '../../../zodHelpers' import { describe, expect, it } from 'vitest' import mongodb from 'mongodb' +import { z } from 'zod' const { ObjectId } = mongodb describe('zodHelpers', () => { + describe('optional', () => { + it('parses a present value with the wrapped schema', () => { + const parsed = zz.optional(z.string()).safeParse('a value') + expect(parsed.success).toBe(true) + expect(parsed.data).toBe('a value') + }) + + it('normalises null to undefined', () => { + const parsed = zz.optional(z.string()).safeParse(null) + expect(parsed.success).toBe(true) + expect(parsed.data).toBe(undefined) + }) + + it('normalises undefined to undefined', () => { + const parsed = zz.optional(z.string()).safeParse(undefined) + expect(parsed.success).toBe(true) + expect(parsed.data).toBe(undefined) + }) + + it('leaves a null unparsed by the wrapped schema', () => { + // A coercing schema would otherwise turn null into a value of its own, + // such as the 0 that Number(null) gives. + const parsed = zz.optional(z.coerce.number()).safeParse(null) + expect(parsed.success).toBe(true) + expect(parsed.data).toBe(undefined) + }) + + it('reports a present value the wrapped schema rejects', () => { + const parsed = zz.optional(z.string()).safeParse(7) + expect(parsed.success).toBe(false) + }) + }) describe('objectId', () => { it('fails to parse when provided with an invalid ObjectId', () => { const parsed = zz.objectId().safeParse('aa') diff --git a/libraries/validation-tools/zodHelpers.js b/libraries/validation-tools/zodHelpers.js index 46e0c39247..b08183b619 100644 --- a/libraries/validation-tools/zodHelpers.js +++ b/libraries/validation-tools/zodHelpers.js @@ -21,6 +21,23 @@ const datetimeSchema = ({ allowNull, allowUndefined, ...zodOptions } = {}) => { } const zz = { + /** + * A field whose absence its source represents inconsistently: left out of + * some records and null in others. JSON has no undefined, and Mongo stores a + * $set of undefined as null, so an absent-ish field arrives as null about as + * often as it is missing outright. + * + * Unlike z.optional(), which accepts only undefined, both forms are accepted + * and normalised to undefined, so callers have a single absent value to + * check rather than a `T | null | undefined` to narrow. + * + * The one combinator here: it wraps another schema instead of describing a + * value type of its own. + * + * @template Output + * @param {z.ZodType} schema + */ + optional: schema => schema.nullish().transform(value => value ?? undefined), objectId: () => z.string().refine(ObjectId.isValid, { message: 'invalid Mongo ObjectId' }), coercedObjectId: () => diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.mjs b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.mjs index a301c6703f..662f5e3ad0 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.mjs +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.mjs @@ -56,7 +56,8 @@ async function addEntity(params) { method: 'post', headers: { 'x-entity-id': entityId, - 'x-entity-rev': rev, + // Sent as a string, since that is what a header carries either way + ...(rev != null ? { 'x-entity-rev': String(rev) } : {}), 'x-entity-type': entityType, 'x-folder-id': folderId, 'x-project-id': projectId, @@ -218,7 +219,10 @@ async function moveEntity(params) { for (const userId of projectUserIds) { const headers = { 'x-project-id': projectId, - 'x-entity-rev': rev, + } + if (rev != null) { + // Sent as a string, since that is what a header carries either way + headers['x-entity-rev'] = String(rev) } if (entityId != null) { headers['x-entity-id'] = entityId diff --git a/tsconfig.backend.json b/tsconfig.backend.json index 65ca9f211a..2eb043e0ce 100644 --- a/tsconfig.backend.json +++ b/tsconfig.backend.json @@ -11,6 +11,9 @@ "@overleaf/access-token-encryptor": [ "./libraries/access-token-encryptor" ], + "@overleaf/access-token-encryptor/*": [ + "./libraries/access-token-encryptor/*" + ], "@overleaf/fetch-utils": ["./libraries/fetch-utils"], "@overleaf/logger": ["./libraries/logger"], "@overleaf/metrics": ["./libraries/metrics"], diff --git a/yarn.lock b/yarn.lock index 6d81b2346e..82d8ffdf68 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7823,7 +7823,14 @@ __metadata: "@overleaf/promise-utils": "workspace:*" "@overleaf/settings": "workspace:*" "@overleaf/validation-tools": "workspace:*" + "@types/express": "npm:4.17.23" + "@types/ioredis": "npm:^4.28.10" + "@types/lodash": "npm:^4.17.24" + "@types/lru-cache": "npm:^5.1.1" + "@types/minimist": "npm:^1.2.5" "@types/node": "npm:^24.5.2" + "@types/node-fetch": "npm:^2.6.13" + "@types/redlock": "npm:^4.0.8" async: "npm:^3.2.5" bunyan: "npm:^1.8.15" chai: "npm:^4.3.6" @@ -10652,6 +10659,13 @@ __metadata: languageName: node linkType: hard +"@types/bluebird@npm:*": + version: 3.5.42 + resolution: "@types/bluebird@npm:3.5.42" + checksum: 10c0/ce752a5e277bbc0cdee3dee9c875ac093d1331905a8a5175665ccb8cb76b7b95a84836f92bc46076c2fd1e384a107866893eeeb20f6fb33427482663879faf93 + languageName: node + linkType: hard + "@types/body-parser@npm:*": version: 1.19.6 resolution: "@types/body-parser@npm:1.19.6" @@ -11068,6 +11082,13 @@ __metadata: languageName: node linkType: hard +"@types/lru-cache@npm:^5.1.1": + version: 5.1.1 + resolution: "@types/lru-cache@npm:5.1.1" + checksum: 10c0/1f17ec9b202c01a89337cc5528198a690be6b61a6688242125fbfb7fa17770e453e00e4685021abf5ae605860ca0722209faac5c254b780d0104730bb0b9e354 + languageName: node + linkType: hard + "@types/mdast@npm:^4.0.0, @types/mdast@npm:^4.0.4": version: 4.0.4 resolution: "@types/mdast@npm:4.0.4" @@ -11378,6 +11399,26 @@ __metadata: languageName: node linkType: hard +"@types/redis@npm:^2.8.0": + version: 2.8.32 + resolution: "@types/redis@npm:2.8.32" + dependencies: + "@types/node": "npm:*" + checksum: 10c0/99fd52ffc1f4c2cbe3f1ec7a38f9a85a128a430fcbf8e241914d8687ea53555e90dc7b66b3ff1a0c9b74afa1c677551ee405b1f73926c944dadc6026e5e6a80a + languageName: node + linkType: hard + +"@types/redlock@npm:^4.0.8": + version: 4.0.8 + resolution: "@types/redlock@npm:4.0.8" + dependencies: + "@types/bluebird": "npm:*" + "@types/ioredis": "npm:^4.28.10" + "@types/redis": "npm:^2.8.0" + checksum: 10c0/b570792496d2ee426c4222b97dd4d0918342612a4b28e38e044aa4a8c21fa6f3207b8051f4f936879e25c393b5fe82f964fc86f79d540f4d478077c7a9e004ff + languageName: node + linkType: hard + "@types/request@npm:^2.48.8": version: 2.48.13 resolution: "@types/request@npm:2.48.13"