From 4dd1294e3c8ee7b3ace60f8c6866ac5ec1759044 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Tue, 16 Sep 2014 13:13:11 -0700 Subject: [PATCH] Fix concurrent getOrGenerate bug. Closes #85 --- lib/policy.js | 19 ++++++++++--------- test/policy.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/lib/policy.js b/lib/policy.js index de77851..b926524 100755 --- a/lib/policy.js +++ b/lib/policy.js @@ -28,7 +28,7 @@ exports = module.exports = internals.Policy = function (options, cache, segment) }; -internals.Policy.prototype.get = function (key, callback) { // key: string or { id: 'id' } +internals.Policy.prototype.get = function (key, callback, _generateFunc) { // key: string or { id: 'id' } var self = this; @@ -53,14 +53,16 @@ internals.Policy.prototype.get = function (key, callback) { // key: string o // No generate method - if (!self.rule.generateFunc) { + if (!self.rule.generateFunc && + !_generateFunc) { + return self._finalize(id, err, cached); // Pass 'cached' as 'value' and omit other arguments for backwards compatibility } // Error / Not found if (err || !cached) { - return self._generate(id, key, null, { msec: timer.elapsed(), error: err }, callback); + return self._generate(id, key, null, { msec: timer.elapsed(), error: err }, callback, _generateFunc); } // Found @@ -78,7 +80,7 @@ internals.Policy.prototype.get = function (key, callback) { // key: string o return self._finalize(id, null, cached.item, cached, report); } - return self._generate(id, key, cached, report, callback); + return self._generate(id, key, cached, report, callback, _generateFunc); }); }; @@ -93,7 +95,7 @@ internals.Policy.prototype._get = function (id, callback) { }; -internals.Policy.prototype._generate = function (id, key, cached, report, callback) { +internals.Policy.prototype._generate = function (id, key, cached, report, callback, _generateFunc) { var self = this; @@ -123,7 +125,7 @@ internals.Policy.prototype._generate = function (id, key, cached, report, callba // Generate new value try { - this.rule.generateFunc.call(null, key, function (err, value, ttl) { + (this.rule.generateFunc || _generateFunc).call(null, key, function (err, value, ttl) { // Error or not cached @@ -164,13 +166,12 @@ internals.Policy.prototype.getOrGenerate = function (id, generateFunc, callback) var self = this; - this.rule.generateFunc = function (id, next) { + var generateFuncWrapper = function (id, next) { - self.rule.generateFunc = null; return generateFunc(next); }; - return this.get(id, callback); + return this.get(id, callback, generateFuncWrapper); }; diff --git a/test/policy.js b/test/policy.js index 1587e9e..0a4761d 100755 --- a/test/policy.js +++ b/test/policy.js @@ -1545,6 +1545,40 @@ describe('Policy', function () { }); }); + it('handles concurrent requests for different keys', function (done) { + + var rule = { + expiresIn: 100, + staleIn: 20, + staleTimeout: 5 + }; + + var client = new Catbox.Client(Import, { partition: 'test-partition' }); + var policy = new Catbox.Policy(rule, client, 'test-segment'); + + var generate = function (gen) { + + return function (callback) { + + return callback(null, { gen: gen }); + }; + }; + + client.start(function () { + + policy.getOrGenerate('1', generate(1), function (err, value, cached) { + + expect(value.gen).to.equal(1); + }); + + policy.getOrGenerate('2', generate(2), function (err, value, cached) { + + expect(value.gen).to.equal(2); + done(); + }); + }); + }); + it('returns the processed cached item after cache error', function (done) { var rule = {