Merge pull request #3117 from overleaf/sk-block-tpds-update-on-archived-project

Block TPDS update on archived/trashed project

GitOrigin-RevId: 7db41a313b03721b93cbb601add6f91ae31f3f2a

services/web/app/src/Features/Errors/Errors.js

@@ -169,6 +169,8 @@ class InvalidQueryError extends OErrorV2CompatibleError {
   }
 }
 
+class ProjectIsArchivedOrTrashedError extends BackwardCompatibleError {}
+
 module.exports = {
   OError,
   BackwardCompatibleError,
@@ -197,5 +199,6 @@ module.exports = {
   UserNotFoundError,
   UserNotCollaboratorError,
   DocHasRangesError,
-  InvalidQueryError
+  InvalidQueryError,
+  ProjectIsArchivedOrTrashedError
 }

services/web/app/src/Features/Project/ProjectLocator.js

@@ -281,13 +281,37 @@ const ProjectLocator = {
         const { owned, readAndWrite } = allProjects
         const projects = owned.concat(readAndWrite)
         projectName = projectName.toLowerCase()
-        const project = _.find(
-          projects,
-          project =>
-            project.name.toLowerCase() === projectName &&
-            !ProjectHelper.isArchivedOrTrashed(project, userId)
-        )
-        callback(null, project)
+        const _findNonArchivedProject = () =>
+          _.find(
+            projects,
+            project =>
+              project.name.toLowerCase() === projectName &&
+              !ProjectHelper.isArchivedOrTrashed(project, userId)
+          )
+        const _findArchivedProject = () =>
+          _.find(
+            projects,
+            project =>
+              project.name.toLowerCase() === projectName &&
+              ProjectHelper.isArchivedOrTrashed(project, userId)
+          )
+        const nonArchivedProject = _findNonArchivedProject()
+        if (nonArchivedProject) {
+          return callback(null, {
+            project: nonArchivedProject,
+            isArchivedOrTrashed: false
+          })
+        } else {
+          const archivedProject = _findArchivedProject()
+          if (archivedProject) {
+            return callback(null, {
+              project: archivedProject,
+              isArchivedOrTrashed: true
+            })
+          } else {
+            return callback(null, { project: null })
+          }
+        }
       }
     )
   }

services/web/app/src/Features/ThirdPartyDataStore/TpdsController.js

@@ -45,6 +45,12 @@ module.exports = {
               'tpds update failed to be processed, too many requests'
             )
             return res.sendStatus(429)
+          } else if (err.name === 'ProjectIsArchivedOrTrashedError') {
+            logger.info(
+              { err, user_id, filePath, projectName },
+              'tpds project is archived'
+            )
+            return res.sendStatus(409)
           } else if (err.message === 'project_has_too_many_files') {
             logger.warn(
               { err, user_id, filePath },

services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js

@@ -30,7 +30,14 @@ module.exports = {
       return projectLocator.findUsersProjectByName(
         user_id,
         projectName,
-        (err, project) => {
+        (err, result) => {
+          if (err) {
+            return callback(err)
+          }
+          if (!result) {
+            return callback(new Error('no data from project locator'))
+          }
+          const { project, isArchivedOrTrashed } = result
           if (project == null) {
             return projectCreationHandler.createBlankProject(
               user_id,
@@ -47,6 +54,9 @@ module.exports = {
               }
             )
           } else {
+            if (isArchivedOrTrashed) {
+              return cb(new Errors.ProjectIsArchivedOrTrashedError())
+            }
             return cb(err, project)
           }
         }
@@ -89,8 +99,15 @@ module.exports = {
     logger.log({ user_id, filePath: path }, 'handling delete update from tpds')
     return projectLocator.findUsersProjectByName(user_id, projectName, function(
       err,
-      project
+      result
     ) {
+      if (err) {
+        return callback(err)
+      }
+      if (!result) {
+        return callback(new Error('no data from project locator'))
+      }
+      const { project, isArchivedOrTrashed } = result
       if (project == null) {
         logger.log(
           { user_id, filePath: path, projectName },
@@ -98,6 +115,13 @@ module.exports = {
         )
         return callback()
       }
+      if (isArchivedOrTrashed) {
+        logger.log(
+          { user_id, filePath: path, projectName },
+          'project is archived or trashed, ignoring folder or project'
+        )
+        return callback()
+      }
       if (path === '/') {
         logger.log(
           { user_id, filePath: path, projectName, project_id: project._id },

services/web/test/acceptance/src/TpdsUpdateTests.js

@@ -37,6 +37,42 @@ describe('TpdsUpdateTests', function() {
     })
   })
 
+  describe('adding a file', function() {
+    beforeEach(function(done) {
+      return request(
+        {
+          method: 'POST',
+          url: `/project/${this.project_id}/contents/test.tex`,
+          auth: {
+            username: 'sharelatex',
+            password: 'password',
+            sendImmediately: true
+          },
+          body: 'test one two'
+        },
+        (error, response, body) => {
+          if (error != null) {
+            throw error
+          }
+          expect(response.statusCode).to.equal(200)
+          return done()
+        }
+      )
+    })
+
+    it('should have added the file', function(done) {
+      return ProjectGetter.getProject(this.project_id, (error, project) => {
+        if (error != null) {
+          throw error
+        }
+        const projectFolder = project.rootFolder[0]
+        const file = projectFolder.docs.find(e => e.name === 'test.tex')
+        expect(file).to.exist
+        return done()
+      })
+    })
+  })
+
   describe('deleting a file', function() {
     beforeEach(function(done) {
       return request(
@@ -74,4 +110,97 @@ describe('TpdsUpdateTests', function() {
       })
     })
   })
+
+  describe('update a new file', function() {
+    beforeEach(function(done) {
+      return request(
+        {
+          method: 'POST',
+          url: `/user/${this.owner._id}/update/test-project/other.tex`,
+          auth: {
+            username: 'sharelatex',
+            password: 'password',
+            sendImmediately: true
+          },
+          body: 'test one two'
+        },
+        (error, response, body) => {
+          if (error != null) {
+            throw error
+          }
+          expect(response.statusCode).to.equal(200)
+          return done()
+        }
+      )
+    })
+
+    it('should have added the file', function(done) {
+      return ProjectGetter.getProject(this.project_id, (error, project) => {
+        if (error != null) {
+          throw error
+        }
+        const projectFolder = project.rootFolder[0]
+        const file = projectFolder.docs.find(e => e.name === 'other.tex')
+        expect(file).to.exist
+        return done()
+      })
+    })
+  })
+
+  describe('update when the project is archived', function() {
+    beforeEach(function(done) {
+      this.owner.request(
+        {
+          url: `/Project/${this.project_id}/archive`,
+          method: 'post'
+        },
+        (err, response, body) => {
+          expect(err).to.not.exist
+          return request(
+            {
+              method: 'POST',
+              url: `/user/${this.owner._id}/update/test-project/test.tex`,
+              auth: {
+                username: 'sharelatex',
+                password: 'password',
+                sendImmediately: true
+              },
+              body: 'test one two'
+            },
+            (error, response, body) => {
+              if (error != null) {
+                throw error
+              }
+              expect(response.statusCode).to.equal(409)
+              return done()
+            }
+          )
+        }
+      )
+    })
+
+    it('should not have created a new project', function(done) {
+      ProjectGetter.findAllUsersProjects(
+        this.owner._id,
+        'name',
+        (err, projects) => {
+          expect(err).to.not.exist
+          expect(projects.owned.length).to.equal(1)
+          done()
+        }
+      )
+    })
+
+    it('should not have added the file', function(done) {
+      return ProjectGetter.getProject(this.project_id, (error, project) => {
+        if (error != null) {
+          throw error
+        }
+        const projectFolder = project.rootFolder[0]
+        const file = projectFolder.docs.find(e => e.name === 'test.tex')
+        expect(file).to.not.exist
+        return done()
+      })
+    })
+  })
 })

services/web/test/unit/src/Project/ProjectLocatorTests.js

@@ -579,7 +579,8 @@ describe('ProjectLocator', function() {
           { name: 'wellll' },
           stubbedProject,
           { name: 'Noooo' }
-        ]
+        ],
+        readAndWrite: []
       }
       this.ProjectGetter.findAllUsersProjects = sinon
         .stub()
@@ -587,17 +588,19 @@ describe('ProjectLocator', function() {
       this.locator.findUsersProjectByName(
         userId,
         stubbedProject.name.toLowerCase(),
-        (err, project) => {
+        (err, result) => {
           if (err != null) {
             return done(err)
           }
+          expect(result).to.exist
+          const { project } = result
           project.should.equal(stubbedProject)
           done()
         }
       )
     })
 
-    it('should return the project which is not archived', function(done) {
+    it('should return the project which is not archived first', function(done) {
       const userId = '123jojoidns'
       const stubbedProject = { name: 'findThis', _id: 12331321 }
       const projects = {
@@ -608,7 +611,8 @@ describe('ProjectLocator', function() {
           stubbedProject,
           { name: 'findThis', archived: true, trashed: false },
           { name: 'Noooo', trashed: true }
-        ]
+        ],
+        readAndWrite: []
       }
 
       this.ProjectHelper.isArchivedOrTrashed
@@ -636,11 +640,63 @@ describe('ProjectLocator', function() {
       this.locator.findUsersProjectByName(
         userId,
         stubbedProject.name.toLowerCase(),
-        (err, project) => {
+        (err, result) => {
           if (err != null) {
             return done(err)
           }
+          expect(result).to.exist
+          const { project, isArchivedOrTrashed } = result
           project._id.should.equal(stubbedProject._id)
+          expect(isArchivedOrTrashed).to.equal(false)
+          done()
+        }
+      )
+    })
+
+    it('should return archived project, and a flag indicating it is archived', function(done) {
+      const userId = '123jojoidns'
+      const stubbedProject = { name: 'findThis', _id: 12331321 }
+      const projects = {
+        owned: [
+          { name: 'notThis' },
+          { name: 'wellll' },
+          { name: 'findThis', archived: true, trashed: true, _id: 1234 },
+          { name: 'findThis', archived: true, trashed: false },
+          { name: 'Noooo', trashed: true }
+        ],
+        readAndWrite: []
+      }
+
+      this.ProjectHelper.isArchivedOrTrashed
+        .withArgs(projects.owned[0], userId)
+        .returns(false)
+      this.ProjectHelper.isArchivedOrTrashed
+        .withArgs(projects.owned[1], userId)
+        .returns(false)
+      this.ProjectHelper.isArchivedOrTrashed
+        .withArgs(projects.owned[2], userId)
+        .returns(true)
+      this.ProjectHelper.isArchivedOrTrashed
+        .withArgs(projects.owned[3], userId)
+        .returns(true)
+      this.ProjectHelper.isArchivedOrTrashed
+        .withArgs(projects.owned[4], userId)
+        .returns(true)
+
+      this.ProjectGetter.findAllUsersProjects = sinon
+        .stub()
+        .callsArgWith(2, null, projects)
+      this.locator.findUsersProjectByName(
+        userId,
+        stubbedProject.name.toLowerCase(),
+        (err, result) => {
+          if (err != null) {
+            return done(err)
+          }
+          expect(result).to.exist
+          const { project, isArchivedOrTrashed } = result
+          project._id.should.equal(1234)
+          expect(isArchivedOrTrashed).to.equal(true)
           done()
         }
       )
@@ -659,10 +715,12 @@ describe('ProjectLocator', function() {
       this.locator.findUsersProjectByName(
         userId,
         stubbedProject.name.toLowerCase(),
-        (err, project) => {
+        (err, result) => {
           if (err != null) {
             return done(err)
           }
+          expect(result).to.exist
+          const { project } = result
           project.should.equal(stubbedProject)
           done()
         }

services/web/test/unit/src/ThirdPartyDataStore/TpdsControllerTests.js

@@ -45,6 +45,7 @@ describe('TpdsController', function() {
         'logger-sharelatex': {
           log() {},
           warn() {},
+          info() {},
           err() {}
         },
         'metrics-sharelatex': {
@@ -109,6 +110,28 @@ describe('TpdsController', function() {
       res.sendStatus.calledWith(500).should.equal(true)
     })
 
+    it('should return a 409 error when the project is archived', function() {
+      const path = '/projectName/here.txt'
+      const req = {
+        pause() {},
+        params: { 0: path, user_id: this.user_id },
+        session: {
+          destroy() {}
+        },
+        headers: {
+          'x-sl-update-source': (this.source = 'dropbox')
+        }
+      }
+      this.TpdsUpdateHandler.newUpdate = sinon
+        .stub()
+        .callsArgWith(5, new Errors.ProjectIsArchivedOrTrashedError())
+      const res = {
+        sendStatus: sinon.stub()
+      }
+      this.TpdsController.mergeUpdate(req, res)
+      res.sendStatus.calledWith(409).should.equal(true)
+    })
+
     it('should return a 400 error when the project is too big', function() {
       const path = '/projectName/here.txt'
       const req = {

services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandlerTests.js

@@ -35,7 +35,9 @@ describe('TpdsUpdateHandler', function() {
     this.project_id = 'dsjajilknaksdn'
     this.project = { _id: this.project_id, name: 'projectNameHere' }
     this.projectLocator = {
-      findUsersProjectByName: sinon.stub().callsArgWith(2, null, this.project)
+      findUsersProjectByName: sinon
+        .stub()
+        .callsArgWith(2, null, { project: this.project })
     }
     this.projectCreationHandler = {
       createBlankProject: sinon.stub().callsArgWith(2, null, this.project)
@@ -96,7 +98,9 @@ describe('TpdsUpdateHandler', function() {
     })
 
     it('should create a new project if one does not already exit', function(done) {
-      this.projectLocator.findUsersProjectByName = sinon.stub().callsArgWith(2)
+      this.projectLocator.findUsersProjectByName = sinon
+        .stub()
+        .callsArgWith(2, null, { project: null })
       const path = '/'
       return this.handler.newUpdate(
         this.user_id,
@@ -113,8 +117,59 @@ describe('TpdsUpdateHandler', function() {
       )
     })
 
+    it('should not create a new project if one with the same name is archived', function(done) {
+      this.projectLocator.findUsersProjectByName = sinon
+        .stub()
+        .callsArgWith(2, null, {
+          project: this.project,
+          isArchivedOrTrashed: true
+        })
+      const path = '/'
+      return this.handler.newUpdate(
+        this.user_id,
+        this.project.name,
+        path,
+        {},
+        this.source,
+        err => {
+          expect(err).to.exist
+          expect(err.name).to.equal('ProjectIsArchivedOrTrashedError')
+          this.projectCreationHandler.createBlankProject
+            .calledWith(this.user_id, this.project.name)
+            .should.equal(false)
+          return done()
+        }
+      )
+    })
+
+    it('should not create a new project if one is found', function(done) {
+      this.projectLocator.findUsersProjectByName = sinon
+        .stub()
+        .callsArgWith(2, null, {
+          project: this.project,
+          isArchivedOrTrashed: false
+        })
+      const path = '/'
+      return this.handler.newUpdate(
+        this.user_id,
+        this.project.name,
+        path,
+        {},
+        this.source,
+        err => {
+          expect(err).to.not.exist
+          this.projectCreationHandler.createBlankProject
+            .calledWith(this.user_id, this.project.name)
+            .should.equal(false)
+          return done()
+        }
+      )
+    })
+
     it('should set the root doc automatically if a new project is created', function(done) {
-      this.projectLocator.findUsersProjectByName = sinon.stub().callsArgWith(2)
+      this.projectLocator.findUsersProjectByName = sinon
+        .stub()
+        .callsArgWith(2, null, { project: null })
       this.handler._rootDocTimeoutLength = 0
       const path = '/'
       return this.handler.newUpdate(
@@ -138,7 +193,9 @@ describe('TpdsUpdateHandler', function() {
       this.FileTypeManager.shouldIgnore = sinon
         .stub()
         .callsArgWith(1, null, true)
-      this.projectLocator.findUsersProjectByName = sinon.stub().callsArgWith(2)
+      this.projectLocator.findUsersProjectByName = sinon
+        .stub()
+        .callsArgWith(2, null, { project: null })
       const path = '/.gitignore'
       this.updateMerger.mergeUpdate = sinon.stub()
       return this.handler.newUpdate(
@@ -158,7 +215,9 @@ describe('TpdsUpdateHandler', function() {
       this.CooldownManager.isProjectOnCooldown = sinon
         .stub()
         .callsArgWith(1, null, false)
-      this.projectLocator.findUsersProjectByName = sinon.stub().callsArgWith(2)
+      this.projectLocator.findUsersProjectByName = sinon
+        .stub()
+        .callsArgWith(2, null, { project: null })
       const path = '/path/here'
       const update = {}
       this.updateMerger.mergeUpdate = sinon.stub()
@@ -188,7 +247,9 @@ describe('TpdsUpdateHandler', function() {
       this.CooldownManager.isProjectOnCooldown = sinon
         .stub()
         .callsArgWith(1, null, true)
-      this.projectLocator.findUsersProjectByName = sinon.stub().callsArgWith(2)
+      this.projectLocator.findUsersProjectByName = sinon
+        .stub()
+        .callsArgWith(2, null, { project: null })
       this.FileTypeManager.shouldIgnore = sinon
         .stub()
         .callsArgWith(1, null, false)