Merge pull request #8396 from overleaf/jpa-split-test-mongo-user

[web] implement split test assignment based on mongo user GitOrigin-RevId: d3e2dff6a5e925cfd0426e9ebfeb7b64dc803f42
2022-06-16 08:03:20 +00:00
parent dfc33805cb
commit b19c56ccaf
13 changed files with 334 additions and 40 deletions
@@ -70,6 +70,9 @@ describe('OwnershipTransferHandler', function () {
        '../Project/ProjectAuditLogHandler': this.ProjectAuditLogHandler,
        '../Email/EmailHandler': this.EmailHandler,
        './CollaboratorsHandler': this.CollaboratorsHandler,
+        '../Analytics/AnalyticsManager': {
+          recordEventForUser: (this.recordEventForUser = sinon.stub()),
+        },
      },
    })
  })
@@ -192,6 +195,23 @@ describe('OwnershipTransferHandler', function () {
      )
    })

+    it('should track the change in BigQuery', async function () {
+      const sessionUserId = ObjectId()
+      await this.handler.promises.transferOwnership(
+        this.project._id,
+        this.collaborator._id,
+        { sessionUserId }
+      )
+      expect(this.recordEventForUser).to.have.been.calledWith(
+        this.user._id,
+        'project-ownership-transfer',
+        {
+          projectId: this.project._id,
+          newOwnerId: this.collaborator._id,
+        }
+      )
+    })
+
    it('should write an entry in the audit log', async function () {
      const sessionUserId = ObjectId()
      await this.handler.promises.transferOwnership(
@@ -43,6 +43,7 @@ describe('ClsiManager', function () {
            },
            clsi: {
              url: 'http://clsi.example.com',
+              defaultBackendClass: 'e2',
            },
            clsi_priority: {
              url: 'https://clsipremium.example.com',
@@ -61,6 +62,11 @@ describe('ClsiManager', function () {
        request: this.request,
        './ClsiFormatChecker': this.ClsiFormatChecker,
        '@overleaf/metrics': this.Metrics,
+        '../SplitTests/SplitTestHandler': {
+          getAssignment: (this.getAssignment = sinon.stub().yields(null, {
+            variant: 'default',
+          })),
+        },
      },
    })
    this.project_id = 'project-id'
@@ -78,7 +84,7 @@ describe('ClsiManager', function () {

    describe('with a successful compile', function () {
      beforeEach(function () {
-        this.ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, {
+        this.ClsiManager._postToClsi = sinon.stub().yields(null, {
          compile: {
            status: (this.status = 'success'),
            outputFiles: [
@@ -100,7 +106,7 @@ describe('ClsiManager', function () {
        this.ClsiManager.sendRequest(
          this.project_id,
          this.user_id,
-          { compileGroup: 'standard' },
+          { compileBackendClass: 'e2', compileGroup: 'standard' },
          this.callback
        )
      })
@@ -113,7 +119,13 @@ describe('ClsiManager', function () {

      it('should send the request to the CLSI', function () {
        this.ClsiManager._postToClsi
-          .calledWith(this.project_id, this.user_id, this.request, 'standard')
+          .calledWith(
+            this.project_id,
+            this.user_id,
+            this.request,
+            'e2',
+            'standard'
+          )
          .should.equal(true)
      })

@@ -180,7 +192,7 @@ describe('ClsiManager', function () {
        this.ClsiManager.sendRequest(
          this.project_id,
          this.user_id,
-          { compileGroup: 'standard' },
+          { compileBackendClass: 'e2', compileGroup: 'standard' },
          this.callback
        )
      })
@@ -222,7 +234,7 @@ describe('ClsiManager', function () {

    describe('with a failed compile', function () {
      beforeEach(function () {
-        this.ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, {
+        this.ClsiManager._postToClsi = sinon.stub().yields(null, {
          compile: {
            status: (this.status = 'failure'),
          },
@@ -327,7 +339,7 @@ describe('ClsiManager', function () {
        this.ClsiFormatChecker.checkRecoursesForProblems = sinon
          .stub()
          .callsArgWith(1, new Error('failed'))
-        this.ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, {
+        this.ClsiManager._postToClsi = sinon.stub().yields(null, {
          compile: {
            status: (this.status = 'failure'),
          },
@@ -359,7 +371,7 @@ describe('ClsiManager', function () {

    describe('with a successful compile', function () {
      beforeEach(function () {
-        this.ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, {
+        this.ClsiManager._postToClsi = sinon.stub().yields(null, {
          compile: {
            status: (this.status = 'success'),
            outputFiles: [
@@ -381,14 +393,20 @@ describe('ClsiManager', function () {
        this.ClsiManager.sendExternalRequest(
          this.submission_id,
          this.clsi_request,
-          { compileGroup: 'standard' },
+          { compileBackendClass: 'e2', compileGroup: 'standard' },
          this.callback
        )
      })

      it('should send the request to the CLSI', function () {
        this.ClsiManager._postToClsi
-          .calledWith(this.submission_id, null, this.clsi_request, 'standard')
+          .calledWith(
+            this.submission_id,
+            null,
+            this.clsi_request,
+            'e2',
+            'standard'
+          )
          .should.equal(true)
      })

@@ -423,7 +441,7 @@ describe('ClsiManager', function () {

    describe('with a failed compile', function () {
      beforeEach(function () {
-        this.ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, {
+        this.ClsiManager._postToClsi = sinon.stub().yields(null, {
          compile: {
            status: (this.status = 'failure'),
          },
@@ -446,7 +464,7 @@ describe('ClsiManager', function () {
        this.ClsiFormatChecker.checkRecoursesForProblems = sinon
          .stub()
          .callsArgWith(1, new Error('failed'))
-        this.ClsiManager._postToClsi = sinon.stub().callsArgWith(4, null, {
+        this.ClsiManager._postToClsi = sinon.stub().yields(null, {
          compile: {
            status: (this.status = 'failure'),
          },
@@ -480,7 +498,7 @@ describe('ClsiManager', function () {
        this.ClsiManager.deleteAuxFiles(
          this.project_id,
          this.user_id,
-          { compileGroup: 'standard' },
+          { compileBackendClass: 'e2', compileGroup: 'standard' },
          'node-1',
          this.callback
        )
@@ -494,7 +512,7 @@ describe('ClsiManager', function () {
            'standard',
            {
              method: 'DELETE',
-              url: `${this.settings.apis.clsi.url}/project/${this.project_id}/user/${this.user_id}?compileGroup=standard`,
+              url: `${this.settings.apis.clsi.url}/project/${this.project_id}/user/${this.user_id}?compileBackendClass=e2&compileGroup=standard`,
            },
            'node-1'
          )
@@ -568,7 +586,7 @@ describe('ClsiManager', function () {
      beforeEach(function (done) {
        this.ClsiManager._buildRequest(
          this.project_id,
-          { timeout: 100, compileGroup: 'standard' },
+          { timeout: 100, compileBackendClass: 'e2', compileGroup: 'standard' },
          (err, request) => {
            if (err != null) {
              return done(err)
@@ -930,13 +948,14 @@ describe('ClsiManager', function () {
          this.project_id,
          this.user_id,
          this.req,
+          'e2',
          'standard',
          this.callback
        )
      })

      it('should send the request to the CLSI', function () {
-        const url = `${this.settings.apis.clsi.url}/project/${this.project_id}/user/${this.user_id}/compile?compileGroup=standard`
+        const url = `${this.settings.apis.clsi.url}/project/${this.project_id}/user/${this.user_id}/compile?compileBackendClass=e2&compileGroup=standard`
        this.ClsiManager._makeRequest
          .calledWith(this.project_id, this.user_id, 'standard', {
            method: 'POST',
@@ -960,6 +979,7 @@ describe('ClsiManager', function () {
          this.project_id,
          this.user_id,
          this.req,
+          'e2',
          'standard',
          this.callback
        )
@@ -990,7 +1010,7 @@ describe('ClsiManager', function () {
          this.project_id,
          this.user_id,
          false,
-          { compileGroup: 'standard' },
+          { compileBackendClass: 'e2', compileGroup: 'standard' },
          'node-1',
          this.callback
        )
@@ -1004,7 +1024,7 @@ describe('ClsiManager', function () {
            'standard',
            {
              method: 'GET',
-              url: `http://clsi.example.com/project/${this.project_id}/user/${this.user_id}/wordcount?compileGroup=standard`,
+              url: `http://clsi.example.com/project/${this.project_id}/user/${this.user_id}/wordcount?compileBackendClass=e2&compileGroup=standard`,
              qs: {
                file: 'rootfile.text',
                image: undefined,
@@ -1027,7 +1047,7 @@ describe('ClsiManager', function () {
          this.project_id,
          this.user_id,
          'main.tex',
-          { compileGroup: 'standard' },
+          { compileBackendClass: 'e2', compileGroup: 'standard' },
          'node-2',
          this.callback
        )
@@ -1041,7 +1061,7 @@ describe('ClsiManager', function () {
            'standard',
            {
              method: 'GET',
-              url: `http://clsi.example.com/project/${this.project_id}/user/${this.user_id}/wordcount?compileGroup=standard`,
+              url: `http://clsi.example.com/project/${this.project_id}/user/${this.user_id}/wordcount?compileBackendClass=e2&compileGroup=standard`,
              qs: { file: 'main.tex', image: undefined },
              json: true,
            },
@@ -1059,7 +1079,7 @@ describe('ClsiManager', function () {
          this.project_id,
          this.user_id,
          'main.tex',
-          { compileGroup: 'standard' },
+          { compileBackendClass: 'e2', compileGroup: 'standard' },
          'node-3',
          this.callback
        )
@@ -1073,7 +1093,7 @@ describe('ClsiManager', function () {
            'standard',
            {
              method: 'GET',
-              url: `http://clsi.example.com/project/${this.project_id}/user/${this.user_id}/wordcount?compileGroup=standard`,
+              url: `http://clsi.example.com/project/${this.project_id}/user/${this.user_id}/wordcount?compileBackendClass=e2&compileGroup=standard`,
              qs: { file: 'main.tex', image: this.image },
              json: true,
            },
@@ -1230,7 +1250,11 @@ describe('ClsiManager', function () {
      this.response = { there: 'something' }
      this.request.callsArgWith(1, null, this.response)
      this.opts = {
-        url: this.ClsiManager._getCompilerUrl('standard', this.project_id),
+        url: this.ClsiManager._getCompilerUrl(
+          'e2',
+          'standard',
+          this.project_id
+        ),
      }
    })

@@ -1243,7 +1267,7 @@ describe('ClsiManager', function () {
        () => {
          const args = this.request.args[0]
          args[0].url.should.equal(
-            `https://compiles.somewhere.test/project/${this.project_id}?compileGroup=standard`
+            `https://compiles.somewhere.test/project/${this.project_id}?compileBackendClass=e2&compileGroup=standard`
          )
          done()
        }
@@ -61,6 +61,9 @@ describe('CompileController', function () {
            variant: 'default',
          })),
        },
+        '../Analytics/AnalyticsManager': {
+          recordEventForSession: sinon.stub(),
+        },
      },
    })
    this.projectId = 'project-id'
@@ -1,3 +1,4 @@
+const { expect } = require('chai')
 const sinon = require('sinon')
 const modulePath = '../../../../app/src/Features/Compile/CompileManager.js'
 const SandboxedModule = require('sandboxed-module')
@@ -16,6 +17,7 @@ describe('CompileManager', function () {
    this.CompileManager = SandboxedModule.require(modulePath, {
      requires: {
        '@overleaf/settings': (this.settings = {
+          apis: { clsi: { defaultBackendClass: 'e2' } },
          redis: { web: { host: 'localhost', port: 42 } },
          rateLimit: { autoCompile: {} },
        }),
@@ -28,6 +30,13 @@ describe('CompileManager', function () {
        './ClsiManager': (this.ClsiManager = {}),
        '../../infrastructure/RateLimiter': this.ratelimiter,
        '@overleaf/metrics': this.Metrics,
+        '../SplitTests/SplitTestHandler': {
+          getAssignmentForMongoUser: (this.getAssignmentForMongoUser = sinon
+            .stub()
+            .yields(null, {
+              variant: 'default',
+            })),
+        },
      },
    })
    this.project_id = 'mock-project-id-123'
@@ -175,7 +184,11 @@ describe('CompileManager', function () {
        )
      this.UserGetter.getUser = sinon
        .stub()
-        .callsArgWith(2, null, (this.user = { features: this.features }))
+        .callsArgWith(
+          2,
+          null,
+          (this.user = { features: this.features, analyticsId: 'abc' })
+        )
      this.CompileManager.getProjectCompileLimits(
        this.project_id,
        this.callback
@@ -191,9 +204,12 @@ describe('CompileManager', function () {
    it("should look up the owner's features", function () {
      this.UserGetter.getUser
        .calledWith(this.project.owner_ref, {
+          _id: 1,
          alphaProgram: 1,
+          analyticsId: 1,
          betaProgram: 1,
          features: 1,
+          splitTests: 1,
        })
        .should.equal(true)
    })
@@ -203,11 +219,114 @@ describe('CompileManager', function () {
        .calledWith(null, {
          timeout: this.timeout,
          compileGroup: this.group,
+          compileBackendClass: 'e2',
+          ownerAnalyticsId: 'abc',
+          emitCompileResultEvent: false,
        })
        .should.equal(true)
    })
  })

+  describe('compileBackendClass', function () {
+    beforeEach(function () {
+      this.features = {
+        compileTimeout: 42,
+        compileGroup: 'standard',
+      }
+      this.ProjectGetter.getProject = sinon
+        .stub()
+        .yields(null, { owner_ref: 'owner-id-123' })
+      this.UserGetter.getUser = sinon
+        .stub()
+        .yields(null, { features: this.features, analyticsId: 'abc' })
+    })
+
+    describe('with standard compile', function () {
+      beforeEach(function () {
+        this.features.compileGroup = 'standard'
+      })
+      it('should return the default class and disable event', function (done) {
+        this.CompileManager.getProjectCompileLimits(
+          this.project_id,
+          (err, { compileBackendClass, emitCompileResultEvent }) => {
+            if (err) return done(err)
+            expect(compileBackendClass).to.equal('e2')
+            expect(emitCompileResultEvent).to.equal(false)
+            done()
+          }
+        )
+      })
+    })
+
+    describe('with priority compile', function () {
+      beforeEach(function () {
+        this.features.compileGroup = 'priority'
+      })
+      describe('split test not active', function () {
+        beforeEach(function () {
+          this.getAssignmentForMongoUser.yields(null, {
+            analytics: { segmentation: {} },
+            variant: 'default',
+          })
+        })
+
+        it('should return the default class and disable event', function (done) {
+          this.CompileManager.getProjectCompileLimits(
+            this.project_id,
+            (err, { compileBackendClass, emitCompileResultEvent }) => {
+              if (err) return done(err)
+              expect(compileBackendClass).to.equal('e2')
+              expect(emitCompileResultEvent).to.equal(false)
+              done()
+            }
+          )
+        })
+      })
+
+      describe('split test active', function () {
+        describe('default variant', function () {
+          beforeEach(function () {
+            this.getAssignmentForMongoUser.yields(null, {
+              analytics: { segmentation: { splitTest: 'foo' } },
+              variant: 'default',
+            })
+          })
+          it('should return the default class and enable event', function (done) {
+            this.CompileManager.getProjectCompileLimits(
+              this.project_id,
+              (err, { compileBackendClass, emitCompileResultEvent }) => {
+                if (err) return done(err)
+                expect(compileBackendClass).to.equal('e2')
+                expect(emitCompileResultEvent).to.equal(true)
+                done()
+              }
+            )
+          })
+        })
+
+        describe('c2d variant', function () {
+          beforeEach(function () {
+            this.getAssignmentForMongoUser.yields(null, {
+              analytics: { segmentation: { splitTest: 'foo' } },
+              variant: 'c2d',
+            })
+          })
+          it('should return the c2d class and enable event', function (done) {
+            this.CompileManager.getProjectCompileLimits(
+              this.project_id,
+              (err, { compileBackendClass, emitCompileResultEvent }) => {
+                if (err) return done(err)
+                expect(compileBackendClass).to.equal('c2d')
+                expect(emitCompileResultEvent).to.equal(true)
+                done()
+              }
+            )
+          })
+        })
+      })
+    })
+  })
+
  describe('deleteAuxFiles', function () {
    beforeEach(function () {
      this.CompileManager.getProjectCompileLimits = sinon