diff --git a/services/document-updater/app/js/RedisManager.js b/services/document-updater/app/js/RedisManager.js index 35f62c8222..4c0d144529 100644 --- a/services/document-updater/app/js/RedisManager.js +++ b/services/document-updater/app/js/RedisManager.js @@ -84,28 +84,23 @@ module.exports = RedisManager = { logger.error({ err: error, doc_id, project_id }, error.message) return callback(error) } - const multi = rclient.multi() - multi.set(keys.docLines({ doc_id }), docLines) - multi.set(keys.projectKey({ doc_id }), project_id) - multi.set(keys.docVersion({ doc_id }), version) - multi.set(keys.docHash({ doc_id }), docHash) - if (ranges != null) { - multi.set(keys.ranges({ doc_id }), ranges) - } else { - multi.del(keys.ranges({ doc_id })) - } - multi.set(keys.pathname({ doc_id }), pathname) - multi.set(keys.projectHistoryId({ doc_id }), projectHistoryId) - return multi.exec(function (error, result) { - if (error != null) { - return callback(error) + // update docsInProject set before writing doc contents + rclient.sadd(keys.docsInProject({ project_id }), doc_id, (error) => { + if (error) return callback(error) + + const multi = rclient.multi() + multi.set(keys.docLines({ doc_id }), docLines) + multi.set(keys.projectKey({ doc_id }), project_id) + multi.set(keys.docVersion({ doc_id }), version) + multi.set(keys.docHash({ doc_id }), docHash) + if (ranges != null) { + multi.set(keys.ranges({ doc_id }), ranges) + } else { + multi.del(keys.ranges({ doc_id })) } - // update docsInProject set - return rclient.sadd( - keys.docsInProject({ project_id }), - doc_id, - callback - ) + multi.set(keys.pathname({ doc_id }), pathname) + multi.set(keys.projectHistoryId({ doc_id }), projectHistoryId) + multi.exec(callback) }) }) }, @@ -269,48 +264,17 @@ module.exports = RedisManager = { projectHistoryId = parseInt(projectHistoryId) } - // doc is not in redis, bail out - if (docLines == null) { - return callback( - null, - docLines, - version, - ranges, - pathname, - projectHistoryId, - unflushedTime, - lastUpdatedAt, - lastUpdatedBy - ) - } - - // doc should be in project set, check if missing (workaround for missing docs from putDoc) - return rclient.sadd(keys.docsInProject({ project_id }), doc_id, function ( - error, - result - ) { - if (error != null) { - return callback(error) - } - if (result !== 0) { - // doc should already be in set - logger.error( - { project_id, doc_id, doc_project_id }, - 'doc missing from docsInProject set' - ) - } - return callback( - null, - docLines, - version, - ranges, - pathname, - projectHistoryId, - unflushedTime, - lastUpdatedAt, - lastUpdatedBy - ) - }) + callback( + null, + docLines, + version, + ranges, + pathname, + projectHistoryId, + unflushedTime, + lastUpdatedAt, + lastUpdatedBy + ) }) }, diff --git a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js index 1937ddfb86..d8f21844cd 100644 --- a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js +++ b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js @@ -182,12 +182,6 @@ describe('RedisManager', function () { .should.equal(true) }) - it('should check if the document is in the DocsIn set', function () { - return this.rclient.sadd - .calledWith(`DocsIn:${this.project_id}`) - .should.equal(true) - }) - it('should return the document', function () { return this.callback .calledWithExactly( @@ -209,78 +203,6 @@ describe('RedisManager', function () { }) }) - describe('when the document is not present', function () { - beforeEach(function () { - this.rclient.mget = sinon - .stub() - .yields(null, [ - null, - null, - null, - null, - null, - null, - null, - null, - null, - null - ]) - this.rclient.sadd = sinon.stub().yields() - return this.RedisManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) - }) - - it('should not check if the document is in the DocsIn set', function () { - return this.rclient.sadd - .calledWith(`DocsIn:${this.project_id}`) - .should.equal(false) - }) - - it('should return an empty result', function () { - return this.callback - .calledWithExactly(null, null, 0, {}, null, null, null, null, null) - .should.equal(true) - }) - - return it('should not log any errors', function () { - return this.logger.error.calledWith().should.equal(false) - }) - }) - - describe('when the document is missing from the DocsIn set', function () { - beforeEach(function () { - this.rclient.sadd = sinon.stub().yields(null, 1) - return this.RedisManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) - }) - - it('should log an error', function () { - return this.logger.error.calledWith().should.equal(true) - }) - - return it('should return the document', function () { - return this.callback - .calledWithExactly( - null, - this.lines, - this.version, - this.ranges, - this.pathname, - this.projectHistoryId, - this.unflushed_time, - this.lastUpdatedAt, - this.lastUpdatedBy - ) - .should.equal(true) - }) - }) - describe('with a corrupted document', function () { beforeEach(function () { this.badHash = 'INVALID-HASH-VALUE'