github/overleaf
1
0
Fork 0
mirror of https://github.com/overleaf/overleaf.git synced 2024-11-21 20:47:08 -05:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #13510 from overleaf/em-promisify-clsi-manager

Browse source 
Clean up of ClsiManager and ClsiCookieManager

GitOrigin-RevId: e5047b253613e87fd6cb4f12855b821028fcaf8e
This commit is contained in:
Eric Mc Sween 2023-06-26 13:42:11 +01:00 committed by Copybot
parent 8cdfffa6f9
commit 1fc13b8cbf
5 changed files with 1723 additions and 2088 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

73
services/web/app/src/Features/Compile/ClsiCookieManager.js
View file

@ -1,32 +1,19 @@
/* eslint-disable
n/handle-callback-err,
max-len,
*/
// TODO: This file was created by bulk-decaffeinate.
// Fix any style issues and re-enable lint.
/*
* decaffeinate suggestions:
* DS102: Remove unnecessary code created because of implicit returns
* DS207: Consider shorter variations of null checks
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let rclientSecondary
const { URL, URLSearchParams } = require('url') const { URL, URLSearchParams } = require('url')
const OError = require('@overleaf/o-error') const OError = require('@overleaf/o-error')
const Settings = require('@overleaf/settings') const Settings = require('@overleaf/settings')
const request = require('request').defaults({ timeout: 30 * 1000 }) const request = require('request').defaults({ timeout: 30 * 1000 })
const RedisWrapper = require('../../infrastructure/RedisWrapper') const RedisWrapper = require('../../infrastructure/RedisWrapper')
const rclient = RedisWrapper.client('clsi_cookie')
if (Settings.redis.clsi_cookie_secondary != null) {
rclientSecondary = RedisWrapper.client('clsi_cookie_secondary')
}
const Cookie = require('cookie') const Cookie = require('cookie')
const logger = require('@overleaf/logger') const logger = require('@overleaf/logger')
const Metrics = require('@overleaf/metrics') const Metrics = require('@overleaf/metrics')
const clsiCookiesEnabled = const clsiCookiesEnabled = (Settings.clsiCookie?.key ?? '') !== ''
(Settings.clsiCookie != null ? Settings.clsiCookie.key : undefined) != null &&
Settings.clsiCookie.key.length !== 0 const rclient = RedisWrapper.client('clsi_cookie')
let rclientSecondary
if (Settings.redis.clsi_cookie_secondary != null) {
rclientSecondary = RedisWrapper.client('clsi_cookie_secondary')
}
module.exports = function (backendGroup) { module.exports = function (backendGroup) {
return { return {
@ -45,15 +32,12 @@ module.exports = function (backendGroup) {
compileBackendClass, compileBackendClass,
callback callback
) { ) {
if (callback == null) { rclient.get(this.buildKey(projectId, userId), (err, serverId) => {
callback = function () {} if (err) {
}
return rclient.get(this.buildKey(projectId, userId), (err, serverId) => {
if (err != null) {
return callback(err) return callback(err)
} }
if (serverId == null || serverId === '') { if (serverId == null || serverId === '') {
return this._populateServerIdViaRequest( this._populateServerIdViaRequest(
projectId, projectId,
userId, userId,
compileGroup, compileGroup,
@ -61,7 +45,7 @@ module.exports = function (backendGroup) {
callback callback
) )
} else { } else {
return callback(null, serverId) callback(null, serverId)
} }
}) })
}, },
@ -73,16 +57,13 @@ module.exports = function (backendGroup) {
compileBackendClass, compileBackendClass,
callback callback
) { ) {
if (callback == null) {
callback = function () {}
}
const u = new URL(`${Settings.apis.clsi.url}/project/${projectId}/status`) const u = new URL(`${Settings.apis.clsi.url}/project/${projectId}/status`)
u.search = new URLSearchParams({ u.search = new URLSearchParams({
compileGroup, compileGroup,
compileBackendClass, compileBackendClass,
}).toString() }).toString()
request.post(u.href, (err, res, body) => { request.post(u.href, (err, res, body) => {
if (err != null) { if (err) {
OError.tag(err, 'error getting initial server id for project', { OError.tag(err, 'error getting initial server id for project', {
project_id: projectId, project_id: projectId,
}) })
@ -96,25 +77,21 @@ module.exports = function (backendGroup) {
res, res,
null, null,
function (err, serverId) { function (err, serverId) {
if (err != null) { if (err) {
logger.warn( logger.warn(
{ err, projectId }, { err, projectId },
'error setting server id via populate request' 'error setting server id via populate request'
) )
} }
return callback(err, serverId) callback(err, serverId)
} }
) )
}) })
}, },
_parseServerIdFromResponse(response) { _parseServerIdFromResponse(response) {
const cookies = Cookie.parse( const cookies = Cookie.parse(response.headers['set-cookie']?.[0] || '')
(response.headers['set-cookie'] != null return cookies?.[Settings.clsiCookie.key]
? response.headers['set-cookie'][0]
: undefined) || ''
)
return cookies != null ? cookies[Settings.clsiCookie.key] : undefined
}, },
checkIsLoadSheddingEvent(clsiserverid, compileGroup, compileBackendClass) { checkIsLoadSheddingEvent(clsiserverid, compileGroup, compileBackendClass) {
@ -155,9 +132,6 @@ module.exports = function (backendGroup) {
previous, previous,
callback callback
) { ) {
if (callback == null) {
callback = function () {}
}
if (!clsiCookiesEnabled) { if (!clsiCookiesEnabled) {
return callback() return callback()
} }
@ -195,9 +169,6 @@ module.exports = function (backendGroup) {
}, },
_setServerIdInRedis(rclient, projectId, userId, serverId, callback) { _setServerIdInRedis(rclient, projectId, userId, serverId, callback) {
if (callback == null) {
callback = function () {}
}
rclient.setex( rclient.setex(
this.buildKey(projectId, userId), this.buildKey(projectId, userId),
this._getTTLInSeconds(serverId), this._getTTLInSeconds(serverId),
@ -207,13 +178,10 @@ module.exports = function (backendGroup) {
}, },
clearServerId(projectId, userId, callback) { clearServerId(projectId, userId, callback) {
if (callback == null) {
callback = function () {}
}
if (!clsiCookiesEnabled) { if (!clsiCookiesEnabled) {
return callback() return callback()
} }
return rclient.del(this.buildKey(projectId, userId), callback) rclient.del(this.buildKey(projectId, userId), callback)
}, },
getCookieJar( getCookieJar(
@ -223,13 +191,10 @@ module.exports = function (backendGroup) {
compileBackendClass, compileBackendClass,
callback callback
) { ) {
if (callback == null) {
callback = function () {}
}
if (!clsiCookiesEnabled) { if (!clsiCookiesEnabled) {
return callback(null, request.jar(), undefined) return callback(null, request.jar(), undefined)
} }
return this._getServerId( this._getServerId(
projectId, projectId,
userId, userId,
compileGroup, compileGroup,
@ -246,7 +211,7 @@ module.exports = function (backendGroup) {
) )
const jar = request.jar() const jar = request.jar()
jar.setCookie(serverCookie, Settings.apis.clsi.url) jar.setCookie(serverCookie, Settings.apis.clsi.url)
return callback(null, jar, serverId) callback(null, jar, serverId)
} }
) )
}, },

1853
services/web/app/src/Features/Compile/ClsiManager.js
View file

File diff suppressed because it is too large Load diff

2
services/web/app/src/router.js
View file

@ -1272,7 +1272,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
const sendRes = _.once(function (statusCode, message) { const sendRes = _.once(function (statusCode, message) {
res.status(statusCode) res.status(statusCode)
plainTextResponse(res, message) plainTextResponse(res, message)
ClsiCookieManager.clearServerId(projectId) ClsiCookieManager.clearServerId(projectId, () => {})
}) // force every compile to a new server }) // force every compile to a new server
// set a timeout // set a timeout
let handler = setTimeout(function () { let handler = setTimeout(function () {

113
services/web/test/unit/src/Compile/ClsiCookieManagerTests.js
View file

@ -1,16 +1,3 @@
/* eslint-disable
n/handle-callback-err,
max-len,
no-return-assign,
no-unused-vars,
*/
// TODO: This file was created by bulk-decaffeinate.
// Fix any style issues and re-enable lint.
/*
* decaffeinate suggestions:
* DS102: Remove unnecessary code created because of implicit returns
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
const sinon = require('sinon') const sinon = require('sinon')
const { assert, expect } = require('chai') const { assert, expect } = require('chai')
const modulePath = '../../../../app/src/Features/Compile/ClsiCookieManager.js' const modulePath = '../../../../app/src/Features/Compile/ClsiCookieManager.js'
@ -19,7 +6,6 @@ const realRequst = require('request')
describe('ClsiCookieManager', function () { describe('ClsiCookieManager', function () {
beforeEach(function () { beforeEach(function () {
const self = this
this.redis = { this.redis = {
auth() {}, auth() {},
get: sinon.stub(), get: sinon.stub(),
@ -57,25 +43,28 @@ describe('ClsiCookieManager', function () {
'@overleaf/settings': this.settings, '@overleaf/settings': this.settings,
request: this.request, request: this.request,
} }
return (this.ClsiCookieManager = SandboxedModule.require(modulePath, { this.ClsiCookieManager = SandboxedModule.require(modulePath, {
requires: this.requires, requires: this.requires,
})()) })()
}) })
describe('getServerId', function () { describe('getServerId', function () {
it('should call get for the key', function (done) { it('should call get for the key', function (done) {
this.redis.get.callsArgWith(1, null, 'clsi-7') this.redis.get.callsArgWith(1, null, 'clsi-7')
return this.ClsiCookieManager._getServerId( this.ClsiCookieManager._getServerId(
this.project_id, this.project_id,
this.user_id, this.user_id,
'', '',
'e2', 'e2',
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
this.redis.get this.redis.get
.calledWith(`clsiserver:${this.project_id}:${this.user_id}`) .calledWith(`clsiserver:${this.project_id}:${this.user_id}`)
.should.equal(true) .should.equal(true)
serverId.should.equal('clsi-7') serverId.should.equal('clsi-7')
return done() done()
} }
) )
}) })
@ -85,15 +74,18 @@ describe('ClsiCookieManager', function () {
.stub() .stub()
.yields(null) .yields(null)
this.redis.get.callsArgWith(1, null) this.redis.get.callsArgWith(1, null)
return this.ClsiCookieManager._getServerId( this.ClsiCookieManager._getServerId(
this.project_id, this.project_id,
this.user_id, this.user_id,
'', '',
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
this.ClsiCookieManager._populateServerIdViaRequest this.ClsiCookieManager._populateServerIdViaRequest
.calledWith(this.project_id, this.user_id) .calledWith(this.project_id, this.user_id)
.should.equal(true) .should.equal(true)
return done() done()
} }
) )
}) })
@ -103,16 +95,19 @@ describe('ClsiCookieManager', function () {
.stub() .stub()
.yields(null) .yields(null)
this.redis.get.callsArgWith(1, null, '') this.redis.get.callsArgWith(1, null, '')
return this.ClsiCookieManager._getServerId( this.ClsiCookieManager._getServerId(
this.project_id, this.project_id,
this.user_id, this.user_id,
'', '',
'e2', 'e2',
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
this.ClsiCookieManager._populateServerIdViaRequest this.ClsiCookieManager._populateServerIdViaRequest
.calledWith(this.project_id, this.user_id) .calledWith(this.project_id, this.user_id)
.should.equal(true) .should.equal(true)
return done() done()
} }
) )
}) })
@ -122,38 +117,42 @@ describe('ClsiCookieManager', function () {
beforeEach(function () { beforeEach(function () {
this.response = 'some data' this.response = 'some data'
this.request.post.callsArgWith(1, null, this.response) this.request.post.callsArgWith(1, null, this.response)
return (this.ClsiCookieManager.setServerId = sinon this.ClsiCookieManager.setServerId = sinon.stub().yields(null, 'clsi-9')
.stub()
.yields(null, 'clsi-9'))
}) })
it('should make a request to the clsi', function (done) { it('should make a request to the clsi', function (done) {
return this.ClsiCookieManager._populateServerIdViaRequest( this.ClsiCookieManager._populateServerIdViaRequest(
this.project_id, this.project_id,
this.user_id, this.user_id,
'standard', 'standard',
'e2', 'e2',
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
const args = this.ClsiCookieManager.setServerId.args[0] const args = this.ClsiCookieManager.setServerId.args[0]
args[0].should.equal(this.project_id) args[0].should.equal(this.project_id)
args[1].should.equal(this.user_id) args[1].should.equal(this.user_id)
args[2].should.equal('standard') args[2].should.equal('standard')
args[3].should.equal('e2') args[3].should.equal('e2')
args[4].should.deep.equal(this.response) args[4].should.deep.equal(this.response)
return done() done()
} }
) )
}) })
it('should return the server id', function (done) { it('should return the server id', function (done) {
return this.ClsiCookieManager._populateServerIdViaRequest( this.ClsiCookieManager._populateServerIdViaRequest(
this.project_id, this.project_id,
this.user_id, this.user_id,
'', '',
'e2', 'e2',
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
serverId.should.equal('clsi-9') serverId.should.equal('clsi-9')
return done() done()
} }
) )
}) })
@ -168,7 +167,7 @@ describe('ClsiCookieManager', function () {
}) })
it('should set the server id with a ttl', function (done) { it('should set the server id with a ttl', function (done) {
return this.ClsiCookieManager.setServerId( this.ClsiCookieManager.setServerId(
this.project_id, this.project_id,
this.user_id, this.user_id,
'standard', 'standard',
@ -176,6 +175,9 @@ describe('ClsiCookieManager', function () {
this.response, this.response,
null, null,
err => { err => {
if (err) {
return done(err)
}
this.redis.setex this.redis.setex
.calledWith( .calledWith(
`clsiserver:${this.project_id}:${this.user_id}`, `clsiserver:${this.project_id}:${this.user_id}`,
@ -183,7 +185,7 @@ describe('ClsiCookieManager', function () {
'clsi-8' 'clsi-8'
) )
.should.equal(true) .should.equal(true)
return done() done()
} }
) )
}) })
@ -200,6 +202,9 @@ describe('ClsiCookieManager', function () {
this.response, this.response,
null, null,
err => { err => {
if (err) {
return done(err)
}
expect(this.redis.setex).to.have.been.calledWith( expect(this.redis.setex).to.have.been.calledWith(
`clsiserver:${this.project_id}:${this.user_id}`, `clsiserver:${this.project_id}:${this.user_id}`,
this.settings.clsiCookie.ttlInSecondsRegular, this.settings.clsiCookie.ttlInSecondsRegular,
@ -211,7 +216,7 @@ describe('ClsiCookieManager', function () {
}) })
it('should return the server id', function (done) { it('should return the server id', function (done) {
return this.ClsiCookieManager.setServerId( this.ClsiCookieManager.setServerId(
this.project_id, this.project_id,
this.user_id, this.user_id,
'standard', 'standard',
@ -219,8 +224,11 @@ describe('ClsiCookieManager', function () {
this.response, this.response,
null, null,
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
serverId.should.equal('clsi-8') serverId.should.equal('clsi-8')
return done() done()
} }
) )
}) })
@ -233,7 +241,7 @@ describe('ClsiCookieManager', function () {
}, },
requires: this.requires, requires: this.requires,
})() })()
return this.ClsiCookieManager.setServerId( this.ClsiCookieManager.setServerId(
this.project_id, this.project_id,
this.user_id, this.user_id,
'standard', 'standard',
@ -241,8 +249,11 @@ describe('ClsiCookieManager', function () {
this.response, this.response,
null, null,
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
this.redis.setex.called.should.equal(false) this.redis.setex.called.should.equal(false)
return done() done()
} }
) )
}) })
@ -251,7 +262,7 @@ describe('ClsiCookieManager', function () {
this.ClsiCookieManager._parseServerIdFromResponse = sinon this.ClsiCookieManager._parseServerIdFromResponse = sinon
.stub() .stub()
.returns(null) .returns(null)
return this.ClsiCookieManager.setServerId( this.ClsiCookieManager.setServerId(
this.project_id, this.project_id,
this.user_id, this.user_id,
'standard', 'standard',
@ -259,8 +270,11 @@ describe('ClsiCookieManager', function () {
this.response, this.response,
null, null,
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
this.redis.setex.called.should.equal(false) this.redis.setex.called.should.equal(false)
return done() done()
} }
) )
}) })
@ -282,7 +296,7 @@ describe('ClsiCookieManager', function () {
this.ClsiCookieManager._parseServerIdFromResponse = sinon this.ClsiCookieManager._parseServerIdFromResponse = sinon
.stub() .stub()
.returns('clsi-8') .returns('clsi-8')
return this.ClsiCookieManager.setServerId( this.ClsiCookieManager.setServerId(
this.project_id, this.project_id,
this.user_id, this.user_id,
'standard', 'standard',
@ -290,6 +304,9 @@ describe('ClsiCookieManager', function () {
this.response, this.response,
null, null,
(err, serverId) => { (err, serverId) => {
if (err) {
return done(err)
}
this.redis_secondary.setex this.redis_secondary.setex
.calledWith( .calledWith(
`clsiserver:${this.project_id}:${this.user_id}`, `clsiserver:${this.project_id}:${this.user_id}`,
@ -297,7 +314,7 @@ describe('ClsiCookieManager', function () {
'clsi-8' 'clsi-8'
) )
.should.equal(true) .should.equal(true)
return done() done()
} }
) )
}) })
@ -305,25 +322,26 @@ describe('ClsiCookieManager', function () {
describe('getCookieJar', function () { describe('getCookieJar', function () {
beforeEach(function () { beforeEach(function () {
return (this.ClsiCookieManager._getServerId = sinon this.ClsiCookieManager._getServerId = sinon.stub().yields(null, 'clsi-11')
.stub()
.yields(null, 'clsi-11'))
}) })
it('should return a jar with the cookie set populated from redis', function (done) { it('should return a jar with the cookie set populated from redis', function (done) {
return this.ClsiCookieManager.getCookieJar( this.ClsiCookieManager.getCookieJar(
this.project_id, this.project_id,
this.user_id, this.user_id,
'', '',
'e2', 'e2',
(err, jar) => { (err, jar) => {
if (err) {
return done(err)
}
jar._jar.store.idx['clsi.example.com']['/'][ jar._jar.store.idx['clsi.example.com']['/'][
this.settings.clsiCookie.key this.settings.clsiCookie.key
].key.should.equal ].key.should.equal
jar._jar.store.idx['clsi.example.com']['/'][ jar._jar.store.idx['clsi.example.com']['/'][
this.settings.clsiCookie.key this.settings.clsiCookie.key
].value.should.equal('clsi-11') ].value.should.equal('clsi-11')
return done() done()
} }
) )
}) })
@ -336,14 +354,17 @@ describe('ClsiCookieManager', function () {
}, },
requires: this.requires, requires: this.requires,
})() })()
return this.ClsiCookieManager.getCookieJar( this.ClsiCookieManager.getCookieJar(
this.project_id, this.project_id,
this.user_id, this.user_id,
'', '',
'e2', 'e2',
(err, jar) => { (err, jar) => {
if (err) {
return done(err)
}
assert.deepEqual(jar, realRequst.jar()) assert.deepEqual(jar, realRequst.jar())
return done() done()
} }
) )
}) })

1770
services/web/test/unit/src/Compile/ClsiManagerTests.js
View file

File diff suppressed because it is too large Load diff