Skip to content

Commit

Permalink
fix: handle interrupted ycb fetch (#26)
Browse files Browse the repository at this point in the history
  • Loading branch information
@BillDorn
BillDorn authored Feb 18, 2020
1 parent 652520b commit ab5d3d6
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 16 deletions.
39 changes: 28 additions & 11 deletions lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ Config.prototype = {
if (!self._configIdMap[bundleName]) {
self._configIdMap[bundleName] = {};
}

self._configIdCounter = (self._configIdCounter+1) % Number.MAX_SAFE_INTEGER;
self._configIdMap[bundleName][configName] = self._configIdCounter;

Expand Down Expand Up @@ -340,16 +341,15 @@ Config.prototype = {
*/
read: function (bundleName, configName, context, callback) {
var self = this;
self._getYCB(bundleName, configName, function (err, ycb) {
self._getYCB(bundleName, configName, function (err, groupId, ycb) {
if (err) {
callback(err);
return;
}
if(self._options.baseContext) {
context = self._mergeBaseContext(context);
}
var key, config, groupId;
groupId = self._configIdMap[bundleName][configName];
var key, config;
key = self._getCacheKey(bundleName, ':m:', configName, ycb.getCacheKey(context));
if (self.timeAware) {
var now = context[self.timeDimension];
Expand Down Expand Up @@ -406,16 +406,15 @@ Config.prototype = {
*/
readNoMerge: function (bundleName, configName, context, callback) {
var self = this;
self._getYCB(bundleName, configName, function (err, ycb) {
self._getYCB(bundleName, configName, function (err, groupId, ycb) {
if (err) {
callback(err);
return;
}
if(self._options.baseContext) {
context = self._mergeBaseContext(context);
}
var key, config, groupId;
groupId = self._configIdMap[bundleName][configName];
var key, config;
key = self._getCacheKey(bundleName, ':um:', configName, ycb.getCacheKey(context));
if(self.timeAware) {
var now = context[self.timeDimension];
Expand Down Expand Up @@ -513,8 +512,9 @@ Config.prototype = {
return callback(new Error(util.format(MESSAGES['unknown config'], configName, bundleName)));
}

var groupId = self._configIdMap[bundleName][configName];
if (self._configYCBs[path]) {
return callback(null, self._configYCBs[path]);
return callback(null, groupId, self._configYCBs[path]);
}

self._readConfigContents(path, function (err, contents) {
Expand All @@ -532,11 +532,13 @@ Config.prototype = {

dimensions = data;
ycb = self._makeYCBFromDimensions(path, dimensions, contents);
callback(null, ycb);
self._cacheYCB(bundleName, configName, path, groupId, ycb);
callback(null, groupId, ycb);
});
} else {
ycb = self._makeYCBFromDimensions(path, dimensions, contents);
callback(null, ycb);
self._cacheYCB(bundleName, configName, path, groupId, ycb);
callback(null, groupId, ycb);
}
});
},
Expand All @@ -556,11 +558,26 @@ Config.prototype = {
} else {
ycb = makeFakeYCB(dimensions, contents);
}
this._configYCBs[path] = ycb;
delete this._configContents[path]; // no longer need to keep a copy of the config
return ycb;
},

/**
* Saves a YCB instance for reuse.
* Checks the config-bundle tags so we don't set it with a stale YCB instance
* @param {string} bundleName the bundleName corresponding to the ycb instance
* @param {string} configName the configName corresponding to the ycb instance
* @param {string} path full path corresponding to the ycb instance
* @param {number} groupId the initial id value to check against current value
* @param ycb the ycb instance to cache
* @private
*/
_cacheYCB: function(bundleName, configName, path, groupId, ycb) {
if(this._configIdMap[bundleName] && this._configIdMap[bundleName][configName] === groupId) {
this._configYCBs[path] = ycb;
delete this._configContents[path]; // no longer need to keep a copy of the config
}
},

/**
* Reads the contents of a configuration file.
* @private
Expand Down
35 changes: 30 additions & 5 deletions tests/lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,31 @@ describe('config', function () {
expect(have.color).to.equal('red');
});
});
it('should not set stale ycb', function (done) {
var ycbConfig = new Config({
dimensionsPath: libpath.resolve(fixtures, 'touchdown-simple/configs/dimensions.json')
});
var bundleName = 'bundle';
var configName = 'config';

var config1 = [{'settings':['master'],'msg':'FIRST'}];
var config2 = [{'settings':['master'],'msg':'SECOND'}];

ycbConfig.addConfigContents(bundleName, configName, 'example-config.json', config1, function(err, config) {
ycbConfig.read(bundleName, configName, {}, function(err, config) {
expect(err).to.equal(null);
expect(config.msg).to.equal('FIRST');
});
});
ycbConfig.addConfigContents(bundleName, configName, 'example-config.json', config2, function(err, config) {});
setTimeout(function () {
ycbConfig.read(bundleName, configName, {}, function(err, config) {
expect(err).to.equal(null);
expect(config.msg).to.equal('SECOND');
done();
});
}, 100);
});
});


Expand Down Expand Up @@ -1473,7 +1498,7 @@ describe('config', function () {
describe('_getYCB()', function () {
it('fails on unknown bundle', function (next) {
var config = new Config();
config._getYCB('foo', 'bar', function (err, ycb) {
config._getYCB('foo', 'bar', function (err, tag, ycb) {
try {
expect(err.message).to.equal('Unknown bundle "foo"');
next();
Expand All @@ -1491,7 +1516,7 @@ describe('config', function () {
libpath.resolve(mojito, 'application.json'),
function (err) {
if (err) { throw err; }
config._getYCB('modown-newsboxes', 'foo', function (err, have) {
config._getYCB('modown-newsboxes', 'foo', function (err, tag, have) {
try {
expect(err.message).to.equal('Unknown config "foo" in bundle "modown-newsboxes"');
next();
Expand All @@ -1510,7 +1535,7 @@ describe('config', function () {
'routes',
libpath.resolve(touchdown, 'configs/dimensions.json'),
function (err) {
config._getYCB('simple', 'routes', function (err, ycb) {
config._getYCB('simple', 'routes', function (err, tag, ycb) {
var have = ycb.read({});
try {
expect(have).to.be.an('array');
Expand Down Expand Up @@ -1538,7 +1563,7 @@ describe('config', function () {
'foo',
libpath.resolve(touchdown, 'configs/foo.js'),
function (err) {
config._getYCB('simple', 'foo', function (err, ycb) {
config._getYCB('simple', 'foo', function (err, tag, ycb) {
var have;
try {
have = ycb.read({device: 'mobile'});
Expand Down Expand Up @@ -1569,7 +1594,7 @@ describe('config', function () {
'application',
libpath.resolve(mojito, 'application.json'),
function (err) {
config._getYCB('modown-newsboxes', 'application', function (err, ycb) {
config._getYCB('modown-newsboxes', 'application', function (err, tag, ycb) {
var have;
try {
have = ycb.read({device: 'mobile'});
Expand Down

0 comments on commit ab5d3d6

Please sign in to comment.