diff --git a/services/document-updater/app/js/RedisManager.js b/services/document-updater/app/js/RedisManager.js index 104a720c95..35f62c8222 100644 --- a/services/document-updater/app/js/RedisManager.js +++ b/services/document-updater/app/js/RedisManager.js @@ -190,18 +190,19 @@ module.exports = RedisManager = { ) {} } const timer = new metrics.Timer('redis.get-doc') - const multi = rclient.multi() - multi.get(keys.docLines({ doc_id })) - multi.get(keys.docVersion({ doc_id })) - multi.get(keys.docHash({ doc_id })) - multi.get(keys.projectKey({ doc_id })) - multi.get(keys.ranges({ doc_id })) - multi.get(keys.pathname({ doc_id })) - multi.get(keys.projectHistoryId({ doc_id })) - multi.get(keys.unflushedTime({ doc_id })) - multi.get(keys.lastUpdatedAt({ doc_id })) - multi.get(keys.lastUpdatedBy({ doc_id })) - return multi.exec(function (error, ...rest) { + const collectKeys = [ + keys.docLines({ doc_id }), + keys.docVersion({ doc_id }), + keys.docHash({ doc_id }), + keys.projectKey({ doc_id }), + keys.ranges({ doc_id }), + keys.pathname({ doc_id }), + keys.projectHistoryId({ doc_id }), + keys.unflushedTime({ doc_id }), + keys.lastUpdatedAt({ doc_id }), + keys.lastUpdatedBy({ doc_id }) + ] + rclient.mget(...collectKeys, (error, ...rest) => { let [ docLines, version, diff --git a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js index d14d0c23de..1937ddfb86 100644 --- a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js +++ b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js @@ -141,10 +141,9 @@ describe('RedisManager', function () { this.json_ranges = JSON.stringify(this.ranges) this.unflushed_time = 12345 this.pathname = '/a/b/c.tex' - this.multi.get = sinon.stub() - this.multi.exec = sinon + this.rclient.mget = sinon .stub() - .callsArgWith(0, null, [ + .yields(null, [ this.jsonlines, this.version, this.hash, @@ -166,57 +165,20 @@ describe('RedisManager', function () { ) }) - it('should get the lines from redis', function () { - return this.multi.get - .calledWith(`doclines:${this.doc_id}`) - .should.equal(true) - }) - - it('should get the version from', function () { - return this.multi.get - .calledWith(`DocVersion:${this.doc_id}`) - .should.equal(true) - }) - - it('should get the hash', function () { - return this.multi.get - .calledWith(`DocHash:${this.doc_id}`) - .should.equal(true) - }) - - it('should get the ranges', function () { - return this.multi.get - .calledWith(`Ranges:${this.doc_id}`) - .should.equal(true) - }) - - it('should get the unflushed time', function () { - return this.multi.get - .calledWith(`UnflushedTime:${this.doc_id}`) - .should.equal(true) - }) - - it('should get the pathname', function () { - return this.multi.get - .calledWith(`Pathname:${this.doc_id}`) - .should.equal(true) - }) - - it('should get the projectHistoryId as an integer', function () { - return this.multi.get - .calledWith(`ProjectHistoryId:${this.doc_id}`) - .should.equal(true) - }) - - it('should get lastUpdatedAt', function () { - return this.multi.get - .calledWith(`lastUpdatedAt:${this.doc_id}`) - .should.equal(true) - }) - - it('should get lastUpdatedBy', function () { - return this.multi.get - .calledWith(`lastUpdatedBy:${this.doc_id}`) + it('should get all the details in one call to redis', function () { + this.rclient.mget + .calledWith( + `doclines:${this.doc_id}`, + `DocVersion:${this.doc_id}`, + `DocHash:${this.doc_id}`, + `ProjectId:${this.doc_id}`, + `Ranges:${this.doc_id}`, + `Pathname:${this.doc_id}`, + `ProjectHistoryId:${this.doc_id}`, + `UnflushedTime:${this.doc_id}`, + `lastUpdatedAt:${this.doc_id}`, + `lastUpdatedBy:${this.doc_id}` + ) .should.equal(true) }) @@ -249,9 +211,9 @@ describe('RedisManager', function () { describe('when the document is not present', function () { beforeEach(function () { - this.multi.exec = sinon + this.rclient.mget = sinon .stub() - .callsArgWith(0, null, [ + .yields(null, [ null, null, null, @@ -322,9 +284,9 @@ describe('RedisManager', function () { describe('with a corrupted document', function () { beforeEach(function () { this.badHash = 'INVALID-HASH-VALUE' - this.multi.exec = sinon + this.rclient.mget = sinon .stub() - .callsArgWith(0, null, [ + .yields(null, [ this.jsonlines, this.version, this.badHash, @@ -351,19 +313,9 @@ describe('RedisManager', function () { describe('with a slow request to redis', function () { beforeEach(function () { - this.multi.exec = sinon - .stub() - .callsArgWith(0, null, [ - this.jsonlines, - this.version, - this.badHash, - this.project_id, - this.json_ranges, - this.pathname, - this.unflushed_time - ]) this.clock = sinon.useFakeTimers() - this.multi.exec = (cb) => { + this.rclient.mget = (...args) => { + const cb = args.pop() this.clock.tick(6000) return cb(null, [ this.jsonlines, @@ -396,9 +348,9 @@ describe('RedisManager', function () { return describe('getDoc with an invalid project id', function () { beforeEach(function () { this.another_project_id = 'project-id-456' - this.multi.exec = sinon + this.rclient.mget = sinon .stub() - .callsArgWith(0, null, [ + .yields(null, [ this.jsonlines, this.version, this.hash,