[document-updater] adopt strict zod types and extend test coverage (#36077)

* [document-updater] adopt strict zod types and extend test coverage

* [saas-e2e] add test for project restore with binary and linked file

GitOrigin-RevId: a8e45bd1e46b6ac8a293da24c8b8ad7b236fd9bd

services/document-updater/test/acceptance/js/ApplyingUpdatesToProjectStructureTests.js

@@ -1,5 +1,7 @@
 const sinon = require('sinon')
+const { expect } = require('chai')
 const { setTimeout } = require('node:timers/promises')
+const { RequestFailedError } = require('@overleaf/fetch-utils')
 const Settings = require('@overleaf/settings')
 const rclientProjectHistory = require('@overleaf/redis-wrapper').createClient(
   Settings.redis.project_history
@@ -11,8 +13,20 @@ const MockWebApi = require('./helpers/MockWebApi')
 const DocUpdaterClient = require('./helpers/DocUpdaterClient')
 const DocUpdaterApp = require('./helpers/DocUpdaterApp')
 
-async function sendProjectUpdateAndWait(projectId, docId, update, version) {
-  await DocUpdaterClient.sendProjectUpdate(projectId, docId, update, version)
+async function sendProjectUpdateAndWait(
+  projectId,
+  docId,
+  update,
+  version,
+  source
+) {
+  await DocUpdaterClient.sendProjectUpdate(
+    projectId,
+    docId,
+    update,
+    version,
+    source
+  )
 
   // It seems that we need to wait for a little while
   await setTimeout(200)
@@ -20,7 +34,7 @@ async function sendProjectUpdateAndWait(projectId, docId, update, version) {
 
 describe("Applying updates to a project's structure", function () {
   before(async function () {
-    this.user_id = 'user-id-123'
+    this.user_id = DocUpdaterClient.randomId()
     this.version = 1234
 
     await DocUpdaterApp.ensureRunning()
@@ -438,6 +452,136 @@ describe("Applying updates to a project's structure", function () {
         }
       )
     })
+
+    describe('with importedAt metadata', function () {
+      before(async function () {
+        this.project_id = DocUpdaterClient.randomId()
+        this.metadata = { importedAt: new Date().toISOString() }
+        this.fileUpdate = {
+          type: 'add-file',
+          id: DocUpdaterClient.randomId(),
+          pathname: '/file-path',
+          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 main and importedAt metadata', function () {
+      before(async function () {
+        this.project_id = DocUpdaterClient.randomId()
+        this.metadata = { main: true, importedAt: new Date().toISOString() }
+        this.fileUpdate = {
+          type: 'add-file',
+          id: DocUpdaterClient.randomId(),
+          pathname: '/file-path',
+          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()
+        this.fileUpdate = {
+          type: 'add-file',
+          id: DocUpdaterClient.randomId(),
+          pathname: '/file-path',
+          url: 'filestore.example.com',
+          metadata: { provider: 'not-a-real-provider' },
+        }
+        this.updates = [this.fileUpdate]
+        try {
+          await sendProjectUpdateAndWait(
+            this.project_id,
+            this.user_id,
+            this.updates,
+            this.version
+          )
+          this.statusCode = 200
+        } catch (err) {
+          if (err instanceof RequestFailedError) {
+            this.statusCode = err.response.status
+          } else {
+            throw err
+          }
+        }
+      })
+
+      it('should return a 400 status code', function () {
+        this.statusCode.should.equal(400)
+      })
+
+      it('should not push anything to the project history api', function (done) {
+        rclientProjectHistory.lrange(
+          ProjectHistoryKeys.projectHistoryOps({ project_id: this.project_id }),
+          0,
+          -1,
+          (error, updates) => {
+            if (error) {
+              return done(error)
+            }
+
+            updates.should.deep.equal([])
+
+            done()
+          }
+        )
+      })
+    })
   })
 
   describe('adding a doc', function () {
@@ -482,6 +626,195 @@ describe("Applying updates to a project's structure", function () {
     })
   })
 
+  describe('adding a doc with a null source (e.g. project creation)', function () {
+    // web sends an explicit `null` source (not an omitted field) when
+    // creating the root/bib doc for a new project, duplicating a project, or
+    // importing a zip upload -- see
+    // web/app/src/Features/Project/ProjectCreationHandler.mjs _createRootDoc.
+    before(async function () {
+      this.project_id = DocUpdaterClient.randomId()
+      this.docUpdate = {
+        type: 'add-doc',
+        id: DocUpdaterClient.randomId(),
+        pathname: '/file-path',
+        docLines: 'a\nb',
+      }
+      this.updates = [this.docUpdate]
+      await sendProjectUpdateAndWait(
+        this.project_id,
+        this.user_id,
+        this.updates,
+        this.version,
+        null
+      )
+    })
+
+    it('should push the doc addition to the project history api without a source', 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.doc.should.equal(this.docUpdate.id)
+          update.meta.user_id.should.equal(this.user_id)
+          expect(update.meta.source).to.not.exist
+
+          done()
+        }
+      )
+    })
+  })
+
+  describe('adding a doc with an origin object source (e.g. restoring a project from history)', function () {
+    // web's RestoreManager.mjs sends the richer Origin/RestoreProjectOrigin
+    // raw shape (not a plain string) as the "source" of a project-structure
+    // change when restoring a file or project from history -- see
+    // overleaf-editor-core/lib/origin/restore_project_origin.js. It carries
+    // the version/timestamp of the restored version, needed downstream to
+    // build a valid history Change origin.
+    before(async function () {
+      this.project_id = DocUpdaterClient.randomId()
+      this.docUpdate = {
+        type: 'add-doc',
+        id: DocUpdaterClient.randomId(),
+        pathname: '/file-path',
+        docLines: 'a\nb',
+      }
+      this.updates = [this.docUpdate]
+      this.origin = {
+        kind: 'project-restore',
+        version: 3,
+        timestamp: new Date().toISOString(),
+      }
+      await sendProjectUpdateAndWait(
+        this.project_id,
+        this.user_id,
+        this.updates,
+        this.version,
+        this.origin
+      )
+    })
+
+    it('should push the doc addition to the project history api with the origin', 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.doc.should.equal(this.docUpdate.id)
+          update.meta.user_id.should.equal(this.user_id)
+          expect(update.meta.source).to.not.exist
+          expect(update.meta.origin).to.deep.equal(this.origin)
+          expect(update.meta.type).to.equal('external')
+
+          done()
+        }
+      )
+    })
+  })
+
+  describe('adding a doc with a null userId (e.g. a TPDS/GitHub-sync update)', function () {
+    // web sends an explicit `null` userId (not an omitted field) for
+    // system-initiated project-structure changes that aren't attributable to
+    // a specific user -- e.g. TpdsController.mjs's updateProjectContents
+    // calls UpdateMerger.promises.mergeUpdate(null, ...) for GitHub-sync
+    // updates.
+    before(async function () {
+      this.project_id = DocUpdaterClient.randomId()
+      this.docUpdate = {
+        type: 'add-doc',
+        id: DocUpdaterClient.randomId(),
+        pathname: '/file-path',
+        docLines: 'a\nb',
+      }
+      this.updates = [this.docUpdate]
+      await sendProjectUpdateAndWait(
+        this.project_id,
+        null,
+        this.updates,
+        this.version
+      )
+    })
+
+    it('should push the doc addition to the project history api without a user_id', 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.doc.should.equal(this.docUpdate.id)
+          expect(update.meta.user_id).to.be.null
+
+          done()
+        }
+      )
+    })
+  })
+
+  describe('adding a doc with an invalid userId', function () {
+    before(async function () {
+      this.project_id = DocUpdaterClient.randomId()
+      this.docUpdate = {
+        type: 'add-doc',
+        id: DocUpdaterClient.randomId(),
+        pathname: '/file-path',
+        docLines: 'a\nb',
+      }
+      this.updates = [this.docUpdate]
+      try {
+        await sendProjectUpdateAndWait(
+          this.project_id,
+          'not-an-object-id',
+          this.updates,
+          this.version
+        )
+        this.statusCode = 200
+      } catch (err) {
+        if (err instanceof RequestFailedError) {
+          this.statusCode = err.response.status
+        } else {
+          throw err
+        }
+      }
+    })
+
+    it('should return a 400 status code', function () {
+      this.statusCode.should.equal(400)
+    })
+
+    it('should not push anything to the project history api', function (done) {
+      rclientProjectHistory.lrange(
+        ProjectHistoryKeys.projectHistoryOps({ project_id: this.project_id }),
+        0,
+        -1,
+        (error, updates) => {
+          if (error) {
+            return done(error)
+          }
+
+          updates.should.deep.equal([])
+
+          done()
+        }
+      )
+    })
+  })
+
   describe('with enough updates to flush to the history service', function () {
     before(async function () {
       this.project_id = DocUpdaterClient.randomId()

services/document-updater/test/acceptance/js/GettingADocumentTests.js

@@ -5,6 +5,10 @@ const MockWebApi = require('./helpers/MockWebApi')
 const DocUpdaterClient = require('./helpers/DocUpdaterClient')
 const DocUpdaterApp = require('./helpers/DocUpdaterApp')
 const { RequestFailedError } = require('@overleaf/fetch-utils')
+const { getRawReqInput } = require('@overleaf/validation-tools')
+const {
+  expectValidationError,
+} = require('@overleaf/validation-tools/testUtils')
 const PersistenceManager = require('../../../app/js/PersistenceManager')
 
 describe('Getting a document', function () {
@@ -57,7 +61,7 @@ describe('Getting a document', function () {
       sinon
         .stub(MockWebApi, 'getDocumentController')
         .callsFake((req, res, next) => {
-          expect(req.query.peek).to.equal('true')
+          expect(getRawReqInput(req).query.peek).to.equal('true')
           return origGetDocumentController(req, res, next)
         })
       this.project_id = DocUpdaterClient.randomId()
@@ -257,6 +261,41 @@ describe('Getting a document', function () {
     })
   })
 
+  describe('when the doc id is malformed', function () {
+    it('should return 404', async function () {
+      const projectId = DocUpdaterClient.randomId()
+      let err
+      try {
+        await DocUpdaterClient.getDoc(projectId, 'not-an-object-id')
+        expect.fail('should have thrown')
+      } catch (error) {
+        err = error
+      }
+      expectValidationError(err, 404, 'doc_id')
+    })
+  })
+
+  describe('when the project id is malformed', function () {
+    it('should return 404', async function () {
+      const docId = DocUpdaterClient.randomId()
+      let err
+      try {
+        // encodeURIComponent so the traversal payload arrives as a literal
+        // path segment (the value of params.project_id) instead of being
+        // collapsed by fetch()/URL's dot-segment normalization before the
+        // request reaches the wire.
+        await DocUpdaterClient.getDoc(
+          encodeURIComponent('../../etc/passwd'),
+          docId
+        )
+        expect.fail('should have thrown')
+      } catch (error) {
+        err = error
+      }
+      expectValidationError(err, 404, 'project_id')
+    })
+  })
+
   describe('when the web api returns an error', function () {
     beforeEach(function () {
       sinon.stub(MockWebApi, 'getDocument').rejects(new Error('oops'))

services/document-updater/test/acceptance/js/SettingADocumentTests.js

@@ -34,7 +34,7 @@ describe('Setting a document', function () {
     this.result = ['one', 'one and a half', 'two', 'three']
     this.newLines = ['these', 'are', 'the', 'new', 'lines']
     this.source = 'dropbox'
-    this.user_id = 'user-id-123'
+    this.user_id = DocUpdaterClient.randomId()
 
     sinon.spy(MockProjectHistoryApi, 'flushProject')
     sinon.spy(MockWebApi, 'setDocument')
@@ -83,7 +83,15 @@ describe('Setting a document', function () {
 
     it('should send the updated doc lines and version to the web api', function () {
       MockWebApi.setDocument
-        .calledWith(this.project_id, this.doc_id, this.newLines)
+        .calledWith(
+          this.project_id,
+          this.doc_id,
+          this.newLines,
+          this.version + 2,
+          {},
+          sinon.match.string,
+          this.user_id
+        )
         .should.equal(true)
     })
 
@@ -302,6 +310,83 @@ describe('Setting a document', function () {
     })
   })
 
+  describe('with a null user id', function () {
+    before(async function () {
+      numberOfReceivedUpdates = 0
+      this.project_id = DocUpdaterClient.randomId()
+      this.doc_id = DocUpdaterClient.randomId()
+      MockWebApi.insertDoc(this.project_id, this.doc_id, {
+        lines: this.lines,
+        version: this.version,
+      })
+      await DocUpdaterClient.preloadDoc(this.project_id, this.doc_id)
+      await DocUpdaterClient.sendUpdate(
+        this.project_id,
+        this.doc_id,
+        this.update
+      )
+      await setTimeout(200)
+      this.body = await DocUpdaterClient.setDocLines(
+        this.project_id,
+        this.doc_id,
+        this.newLines,
+        this.source,
+        null,
+        false
+      )
+    })
+
+    after(function () {
+      MockProjectHistoryApi.flushProject.resetHistory()
+      MockWebApi.setDocument.resetHistory()
+    })
+
+    it('should emit two updates (from sendUpdate and setDocLines)', function () {
+      expect(numberOfReceivedUpdates).to.equal(2)
+    })
+
+    it('should send the updated doc lines and version to the web api', function () {
+      MockWebApi.setDocument
+        .calledWith(
+          this.project_id,
+          this.doc_id,
+          this.newLines,
+          this.version + 2,
+          {},
+          sinon.match.string,
+          null
+        )
+        .should.equal(true)
+    })
+
+    it('should update the lines in the doc updater', async function () {
+      const doc = await DocUpdaterClient.getDoc(this.project_id, this.doc_id)
+      doc.lines.should.deep.equal(this.newLines)
+    })
+
+    it('should bump the version in the doc updater', async function () {
+      const doc = await DocUpdaterClient.getDoc(this.project_id, this.doc_id)
+      doc.version.should.equal(this.version + 2)
+    })
+
+    it('should leave the document in redis', function (done) {
+      docUpdaterRedis.get(
+        Keys.docLines({ doc_id: this.doc_id }),
+        (error, lines) => {
+          if (error) {
+            throw error
+          }
+          expect(JSON.parse(lines)).to.deep.equal(this.newLines)
+          done()
+        }
+      )
+    })
+
+    it('should return the mongo rev in the json response', function () {
+      this.body.should.deep.equal({ rev: '123' })
+    })
+  })
+
   const DOC_TOO_LARGE_TEST_CASES = [
     {
       desc: 'when the updated doc is too large for the body parser',

services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js

@@ -237,10 +237,10 @@ module.exports = DocUpdaterClient = {
     )
   },
 
-  async sendProjectUpdate(projectId, userId, updates, version) {
+  async sendProjectUpdate(projectId, userId, updates, version, source) {
     await fetchNothing(`http://127.0.0.1:3003/project/${projectId}`, {
       method: 'POST',
-      json: { userId, updates, version },
+      json: { userId, updates, version, source },
     })
   },
 }

services/document-updater/test/acceptance/js/helpers/MockDocstoreApi.js

@@ -1,8 +1,41 @@
 const express = require('express')
 const { expressify } = require('@overleaf/promise-utils')
+const {
+  handleValidationError,
+  parseReq,
+  z,
+  zz,
+} = require('@overleaf/validation-tools')
 const app = express()
 const MAX_REQUEST_SIZE = 2 * (2 * 1024 * 1024 + 64 * 1024)
 
+const projectParamsSchema = z.object({
+  params: z.strictObject({
+    project_id: zz.objectId(),
+  }),
+})
+
+const docParamsSchema = z.object({
+  params: z.strictObject({
+    project_id: zz.objectId(),
+    doc_id: zz.objectId(),
+  }),
+})
+
+// the partially-deleted-doc fixup payload sent by
+// scripts/check_redis_mongo_sync_state.js
+const patchDocumentSchema = z.object({
+  params: z.strictObject({
+    project_id: zz.objectId(),
+    doc_id: zz.objectId(),
+  }),
+  body: z.strictObject({
+    name: z.string(),
+    deleted: z.boolean(),
+    deletedAt: z.iso.datetime(),
+  }),
+})
+
 const MockDocstoreApi = {
   docs: {},
 
@@ -48,8 +81,9 @@ const MockDocstoreApi = {
     app.get(
       '/project/:project_id/doc-deleted',
       expressify(async (req, res) => {
+        const { params } = parseReq(req, projectParamsSchema)
         try {
-          const docs = await this.getAllDeletedDocs(req.params.project_id)
+          const docs = await this.getAllDeletedDocs(params.project_id)
           return res.json(docs)
         } catch (error) {
           return res.sendStatus(500)
@@ -60,11 +94,9 @@ const MockDocstoreApi = {
     app.get(
       '/project/:project_id/doc/:doc_id/peek',
       expressify(async (req, res) => {
+        const { params } = parseReq(req, docParamsSchema)
         try {
-          const doc = await this.peekDocument(
-            req.params.project_id,
-            req.params.doc_id
-          )
+          const doc = await this.peekDocument(params.project_id, params.doc_id)
           if (doc) {
             return res.json(doc)
           } else {
@@ -80,11 +112,12 @@ const MockDocstoreApi = {
       '/project/:project_id/doc/:doc_id',
       express.json({ limit: MAX_REQUEST_SIZE }),
       expressify(async (req, res) => {
+        const { params, body } = parseReq(req, patchDocumentSchema)
         try {
           await MockDocstoreApi.patchDocument(
-            req.params.project_id,
-            req.params.doc_id,
-            req.body
+            params.project_id,
+            params.doc_id,
+            body
           )
           return res.sendStatus(204)
         } catch (error) {
@@ -93,6 +126,8 @@ const MockDocstoreApi = {
       })
     )
 
+    app.use(handleValidationError)
+
     app
       .listen(3016, error => {
         if (error) {

services/document-updater/test/acceptance/js/helpers/MockProjectHistoryApi.js

@@ -1,8 +1,20 @@
 let MockProjectHistoryApi
 const { expressify } = require('@overleaf/promise-utils')
 const express = require('express')
+const {
+  handleValidationError,
+  parseReq,
+  z,
+  zz,
+} = require('@overleaf/validation-tools')
 const app = express()
 
+const flushProjectSchema = z.object({
+  params: z.strictObject({
+    project_id: zz.objectId(),
+  }),
+})
+
 module.exports = MockProjectHistoryApi = {
   async flushProject(docId) {},
 
@@ -10,8 +22,9 @@ module.exports = MockProjectHistoryApi = {
     app.post(
       '/project/:project_id/flush',
       expressify(async (req, res, next) => {
+        const { params } = parseReq(req, flushProjectSchema)
         try {
-          await this.flushProject(req.params.project_id)
+          await this.flushProject(params.project_id)
           return res.sendStatus(204)
         } catch (error) {
           return res.sendStatus(500)
@@ -19,6 +32,8 @@ module.exports = MockProjectHistoryApi = {
       })
     )
 
+    app.use(handleValidationError)
+
     return app.listen(3054, error => {
       if (error != null) {
         throw error

services/document-updater/test/acceptance/js/helpers/MockWebApi.js

@@ -4,9 +4,41 @@ const tsscmp = require('tsscmp')
 const express = require('express')
 const { expressify } = require('@overleaf/promise-utils')
 const Settings = require('@overleaf/settings')
+const {
+  handleValidationError,
+  parseReq,
+  z,
+  zz,
+} = require('@overleaf/validation-tools')
+const { rawStringFileData } = require('overleaf-editor-core/lib/schemas')
+const schemas = require('../../../../app/js/schemas')
 const app = express()
 const MAX_REQUEST_SIZE = 2 * (2 * 1024 * 1024 + 64 * 1024)
 
+const getDocumentSchema = z.object({
+  params: z.strictObject({
+    project_id: zz.objectId(),
+    doc_id: zz.objectId(),
+  }),
+})
+
+// the flush payload built by PersistenceManager.setDoc: lines are either
+// sharejs lines or history-ot raw file data; lastUpdatedAt is a redis string
+// (ms since epoch) unless freshly set in-process
+const setDocumentSchema = z.object({
+  params: z.strictObject({
+    project_id: zz.objectId(),
+    doc_id: zz.objectId(),
+  }),
+  body: z.strictObject({
+    lines: z.array(z.string()).or(rawStringFileData),
+    version: z.number().int(),
+    ranges: schemas.ranges.nullish(),
+    lastUpdatedAt: z.union([z.string(), z.number()]).nullish(),
+    lastUpdatedBy: z.string().nullish(),
+  }),
+})
+
 module.exports = MockWebApi = {
   docs: {},
 
@@ -52,11 +84,9 @@ module.exports = MockWebApi = {
   },
 
   async getDocumentController(req, res, next) {
+    const { params } = parseReq(req, getDocumentSchema)
     try {
-      const doc = await this.getDocument(
-        req.params.project_id,
-        req.params.doc_id
-      )
+      const doc = await this.getDocument(params.project_id, params.doc_id)
       if (doc != null) {
         return res.send(JSON.stringify(doc))
       } else {
@@ -68,15 +98,16 @@ module.exports = MockWebApi = {
   },
 
   async setDocumentController(req, res, next) {
+    const { params, body } = parseReq(req, setDocumentSchema)
     try {
       const ok = await this.setDocument(
-        req.params.project_id,
-        req.params.doc_id,
-        req.body.lines,
-        req.body.version,
-        req.body.ranges,
-        req.body.lastUpdatedAt,
-        req.body.lastUpdatedBy
+        params.project_id,
+        params.doc_id,
+        body.lines,
+        body.version,
+        body.ranges,
+        body.lastUpdatedAt,
+        body.lastUpdatedBy
       )
       if (!ok) {
         return res.sendStatus(404)
@@ -118,6 +149,8 @@ module.exports = MockWebApi = {
       })
     )
 
+    app.use(handleValidationError)
+
     return app
       .listen(3000, error => {
         if (error != null) {