Fix concurrent getOrGenerate bug. Closes #85

hapijs · Sep 16, 2014 · 4dd1294 · 4dd1294
1 parent 4c7a451
commit 4dd1294
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 9 deletions.
diff --git 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
@@ -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 = {