diff --git a/libraries/overleaf-editor-core/lib/schemas.js b/libraries/overleaf-editor-core/lib/schemas.js index 6c97ca399f..c6912d9c6c 100644 --- a/libraries/overleaf-editor-core/lib/schemas.js +++ b/libraries/overleaf-editor-core/lib/schemas.js @@ -28,7 +28,9 @@ const rawRetainOp = z.union([ z.strictObject({ r: z.number().int().min(1), commentIds: z.array(zz.objectId()).optional(), - tracking: rawTrackingProps.or(rawClearTrackingProps).optional(), + tracking: z + .discriminatedUnion('type', [rawTrackingProps, rawClearTrackingProps]) + .optional(), }), z.number().int().min(1), ]) @@ -101,14 +103,14 @@ const rawLinkedFileData = z.discriminatedUnion('provider', [ z.strictObject({ provider: z.literal('project_file'), source_project_id: zz.objectId().optional(), - v1_source_doc_id: z.string().optional(), + v1_source_doc_id: z.number().optional(), source_entity_path: z.string(), importedAt: z.iso.datetime().optional(), }), z.strictObject({ provider: z.literal('project_output_file'), source_project_id: zz.objectId().optional(), - v1_source_doc_id: z.string().optional(), + v1_source_doc_id: z.number().optional(), source_output_file_path: z.string(), build_id: zz.buildId().optional(), clsiServerId: z.string().optional(), @@ -116,7 +118,12 @@ const rawLinkedFileData = z.discriminatedUnion('provider', [ }), z.strictObject({ provider: z.literal('mendeley'), - group_id: zz.routeSegment().optional(), + // null, not just absent, for an import from a personal library: the web + // agents always build the data object with a group_id key, so no group + // means `group_id: undefined`, and mongoose keeps that key when $set-ing + // the Mixed linkedFileData (upserting over an existing file), where the + // driver serializes it as null. Same for the zotero/papers branches. + group_id: zz.routeSegment().nullish(), importer_id: z.string().optional(), v1_importer_id: z.number().optional(), importedAt: z.iso.datetime().optional(), @@ -126,14 +133,14 @@ const rawLinkedFileData = z.discriminatedUnion('provider', [ // absent on files imported before the `format` field existed; agents // default this to 'bibtex' (ZoteroAgent `_getFormat`) format: z.enum(['bibtex', 'biblatex']).optional(), - group_id: zz.routeSegment().optional(), + group_id: zz.routeSegment().nullish(), importer_id: z.string().optional(), v1_importer_id: z.number().optional(), importedAt: z.iso.datetime().optional(), }), z.strictObject({ provider: z.literal('papers'), - group_id: zz.routeSegment().optional(), + group_id: zz.routeSegment().nullish(), importer_id: z.string().optional(), v1_importer_id: z.number().optional(), importedAt: z.iso.datetime().optional(), diff --git a/libraries/overleaf-editor-core/test/unit/schemas.test.js b/libraries/overleaf-editor-core/test/unit/schemas.test.js index 2e7a900504..4936e0a33f 100644 --- a/libraries/overleaf-editor-core/test/unit/schemas.test.js +++ b/libraries/overleaf-editor-core/test/unit/schemas.test.js @@ -1,7 +1,58 @@ const { expect } = require('chai') -const { rawLinkedFileData, rawFileMetadata } = require('../../lib/schemas') +const { + rawLinkedFileData, + rawFileMetadata, + rawRetainOp, +} = require('../../lib/schemas') describe('schemas', function () { + describe('rawRetainOp', function () { + it('accepts a bare retain length', function () { + const result = rawRetainOp.safeParse(5) + expect(result.success).to.equal(true) + }) + + it('accepts a retain with tracked-insert props', function () { + const result = rawRetainOp.safeParse({ + r: 5, + tracking: { + type: 'insert', + userId: '507f1f77bcf86cd799439011', + ts: '2024-01-01T00:00:00.000Z', + }, + }) + expect(result.success).to.equal(true) + }) + + it('accepts a retain with clear-tracking props', function () { + const result = rawRetainOp.safeParse({ + r: 5, + tracking: { type: 'none' }, + }) + expect(result.success).to.equal(true) + }) + + it('rejects a retain with an unrecognized tracking type', function () { + const result = rawRetainOp.safeParse({ + r: 5, + tracking: { type: 'not-a-tracking-type' }, + }) + expect(result.success).to.equal(false) + }) + + it('rejects a retain with a malformed tracking userId', function () { + const result = rawRetainOp.safeParse({ + r: 5, + tracking: { + type: 'insert', + userId: 'not-an-object-id', + ts: '2024-01-01T00:00:00.000Z', + }, + }) + expect(result.success).to.equal(false) + }) + }) + describe('rawLinkedFileData', function () { it('rejects an empty object', function () { const result = rawLinkedFileData.safeParse({}) @@ -41,6 +92,15 @@ describe('schemas', function () { expect(result.success).to.equal(true) }) + it('accepts a project_file provider with a v1 source doc id', function () { + const result = rawLinkedFileData.safeParse({ + provider: 'project_file', + v1_source_doc_id: 1234, + source_entity_path: '/main.tex', + }) + expect(result.success).to.equal(true) + }) + it('rejects a project_file provider with a malformed source_project_id', function () { const result = rawLinkedFileData.safeParse({ provider: 'project_file', @@ -68,6 +128,15 @@ describe('schemas', function () { expect(result.success).to.equal(false) }) + it('accepts a project_output_file provider with a v1 source doc id', function () { + const result = rawLinkedFileData.safeParse({ + provider: 'project_output_file', + v1_source_doc_id: 1234, + source_output_file_path: 'output.pdf', + }) + expect(result.success).to.equal(true) + }) + it('accepts a project_output_file provider with a valid build_id', function () { const result = rawLinkedFileData.safeParse({ provider: 'project_output_file', @@ -100,6 +169,14 @@ describe('schemas', function () { expect(result.success).to.equal(true) }) + it(`accepts a ${provider} provider with a null group_id`, function () { + const result = rawLinkedFileData.safeParse({ + provider, + group_id: null, + }) + expect(result.success).to.equal(true) + }) + it(`rejects a ${provider} provider with a group_id containing a path separator`, function () { const result = rawLinkedFileData.safeParse({ provider, @@ -134,6 +211,16 @@ describe('schemas', function () { expect(result.success).to.equal(true) }) + it('accepts linked-file metadata for a v1 project_file import', function () { + const result = rawFileMetadata.safeParse({ + importedAt: '2024-01-01T00:00:00.000Z', + provider: 'project_file', + v1_source_doc_id: 1234, + source_entity_path: '/main.tex', + }) + expect(result.success).to.equal(true) + }) + it('accepts an importedAt timestamp with no provider', function () { const result = rawFileMetadata.safeParse({ importedAt: '2024-01-01T00:00:00.000Z', diff --git a/services/document-updater/app/js/HttpController.js b/services/document-updater/app/js/HttpController.js index f2191793ad..36f06502bb 100644 --- a/services/document-updater/app/js/HttpController.js +++ b/services/document-updater/app/js/HttpController.js @@ -651,7 +651,11 @@ const updateProjectSchema = z.object({ body: z.strictObject({ projectHistoryId: z.union([z.number(), zz.projectHistoryId()]).optional(), userId: zz.objectId().nullish(), - updates: z.array(renameUpdateSchema.or(addUpdateSchema)).default([]), + updates: z + .array( + z.discriminatedUnion('type', [renameUpdateSchema, addUpdateSchema]) + ) + .default([]), version: z.union([z.number(), z.string()]), // `source` is polymorphic (see Utils.extractOriginOrSource): either a // plain descriptive string (e.g. 'editor' for live user edits) or the @@ -680,7 +684,12 @@ const updateProjectFallbackSchema = z.object({ projectHistoryId: z.union([z.number(), z.string()]).optional(), userId: z.string().nullish(), updates: z - .array(renameUpdateFallbackSchema.or(addUpdateFallbackSchema)) + .array( + z.discriminatedUnion('type', [ + renameUpdateFallbackSchema, + addUpdateFallbackSchema, + ]) + ) .default([]), version: z.union([z.number(), z.string()]), // loosened equivalent of editorCoreSchemas.rawOrigin -- see the same diff --git a/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js b/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js index 41b2b6a4b1..619cd57080 100644 --- a/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js +++ b/services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js @@ -533,6 +533,54 @@ describe("Applying updates to a project's structure", function () { }) }) + describe('with linked-file metadata for a personal reference library', function () { + before(async function () { + this.project_id = DocUpdaterClient.randomId() + this.metadata = { + provider: 'zotero', + format: 'bibtex', + // web persists the absent group as null, see the rawLinkedFileData + // comment in overleaf-editor-core + group_id: null, + importer_id: this.user_id, + importedAt: new Date().toISOString(), + } + this.fileUpdate = { + type: 'add-file', + id: DocUpdaterClient.randomId(), + pathname: '/references.bib', + url: 'filestore.example.com', + metadata: this.metadata, + } + this.updates = [this.fileUpdate] + await sendProjectUpdateAndWait( + this.project_id, + this.user_id, + this.updates, + this.version + ) + }) + + it('should push the file addition to the project history api with the metadata', function (done) { + rclientProjectHistory.lrange( + ProjectHistoryKeys.projectHistoryOps({ project_id: this.project_id }), + 0, + -1, + (error, updates) => { + if (error) { + return done(error) + } + + const update = JSON.parse(updates[0]) + update.file.should.equal(this.fileUpdate.id) + expect(update.metadata).to.deep.equal(this.metadata) + + done() + } + ) + }) + }) + describe('with invalid metadata', function () { before(async function () { this.project_id = DocUpdaterClient.randomId() diff --git a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js index 16969bb22c..aaaace6a03 100644 --- a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js +++ b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js @@ -1162,6 +1162,47 @@ describe('HttpController', function () { }) }) + describe('when an add-file update has linked-file metadata with a null group_id', function () { + beforeEach(async function () { + this.req.body.updates[3].metadata = { + provider: 'zotero', + format: 'bibtex', + group_id: null, + importer_id: this.userId, + importedAt: '2024-01-01T00:00:00.000Z', + } + await this.HttpController.updateProject(this.req, this.res, this.next) + }) + + it('should accept the change', function () { + this.ProjectManager.promises.updateProjectWithLocks.should.have.been.calledWith( + this.project_id, + this.projectHistoryId, + this.userId, + this.updates, + this.version, + this.source + ) + }) + }) + + describe('when an update has an unrecognized type', function () { + beforeEach(async function () { + this.req.body.updates[3].type = 'add-folder' + await this.HttpController.updateProject(this.req, this.res, this.next) + }) + + it('should call next with the error', function () { + this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true) + }) + + it('should not update the project', function () { + this.ProjectManager.promises.updateProjectWithLocks.called.should.equal( + false + ) + }) + }) + describe('when an add-file update has a malformed hash', function () { beforeEach(async function () { this.req.body.updates[3].hash = 'not-a-valid-hash'