[document-updater] address zod violations (#36147)

* [editor-core] address zod violation for group_id

* [monorepo] use discriminated union in zod schemas where possible

* [editor-core] v1_source_doc_id is a number

GitOrigin-RevId: 38f591b07cca4e22954bfe30bf855545a6af675d

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(),

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',

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

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()

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'