cartodb4/Windshaft-cartodb
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #214 from CartoDB/CDB-4008

Browse Source 
TTL for template locks so they are not kept forever
This commit is contained in:
Raul Ochoa
2014-09-26 11:16:05 +02:00
parent b41d1e84da e8e03585ff
commit c13dbc9a57
4 changed files with 172 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/cartodb/template_maps.js
+74 -81
View File
@@ -1,10 +1,7 @@
var crypto = require('crypto');
var Step = require('step');
var _ = require('underscore');
// Templates in this hash (keyed as <username>@<template_name>)
// are being worked on.
var user_template_locks = {};
var crypto = require('crypto'),
Step = require('step'),
_ = require('underscore'),
dot = require('dot');
// Class handling map templates
//
@@ -38,17 +35,18 @@ function TemplateMaps(redis_pool, signed_maps, opts) {
//
// We have the following datastores:
//
// 1. User teplates: set of per-user map templates
// 1. User templates: set of per-user map templates
// NOTE: each template would have an associated auth
// reference, see signed_maps.js
// User templates (HASH:tpl_id->tpl_val)
this.key_usr_tpl = "map_tpl|<%= owner %>";
this.key_usr_tpl = dot.template("map_tpl|{{=it.owner}}");
// User template locks (HASH:tpl_id->ctime)
this.key_usr_tpl_lck = "map_tpl|<%= owner %>|locks";
this.key_usr_tpl_lck = dot.template("map_tpl|{{=it.owner}}|locks");
};
this.lock_ttl = this.opts['lock_ttl'] || 5000;
}
var o = TemplateMaps.prototype;
@@ -97,36 +95,34 @@ o._redisCmd = function(redisFunc, redisArgs, callback) {
// @param callback function(err, obtained)
o._obtainTemplateLock = function(owner, tpl_id, callback) {
var usr_tpl_lck_key = _.template(this.key_usr_tpl_lck, {owner:owner});
var that = this;
var gotLock = false;
Step (
function obtainLock() {
var ctime = Date.now();
that._redisCmd('HSETNX', [usr_tpl_lck_key, tpl_id, ctime], this);
},
function checkLock(err, locked) {
if ( err ) throw err;
if ( ! locked ) {
// Already locked
// TODO: unlock if expired ?
throw new Error("Template '" + tpl_id + "' of user '" + owner + "' is locked");
}
return gotLock = true;
},
function finish(err) {
callback(err, gotLock);
}
);
var that = this,
lockKey = this.key_usr_tpl_lck({owner:owner});
Step (
function obtainLock() {
that._redisCmd('HGET', [lockKey, tpl_id], this);
},
function checkLock(err, lockTime) {
if (err) { throw err; }
var _newLockTime = Date.now();
if (!lockTime || ((_newLockTime - lockTime) > that.lock_ttl)) {
that._redisCmd('HSET', [lockKey, tpl_id, _newLockTime], this);
} else {
throw new Error("Template '" + tpl_id + "' of user '" + owner + "' is locked");
}
},
function finish(err, hsetValue) {
callback(err, !!hsetValue);
}
);
};
// @param callback function(err, deleted)
o._releaseTemplateLock = function(owner, tpl_id, callback) {
var usr_tpl_lck_key = _.template(this.key_usr_tpl_lck, {owner:owner});
this._redisCmd('HDEL', [usr_tpl_lck_key, tpl_id], callback);
this._redisCmd('HDEL', [this.key_usr_tpl_lck({owner:owner}), tpl_id], callback);
};
o._reValidIdentifier = /^[a-zA-Z][0-9a-zA-Z_]*$/;
var _reValidIdentifier = /^[a-zA-Z][0-9a-zA-Z_]*$/;
o._checkInvalidTemplate = function(template) {
if ( template.version != '0.0.1' ) {
return new Error("Unsupported template version " + template.version);
@@ -135,22 +131,26 @@ o._checkInvalidTemplate = function(template) {
if ( ! tplname ) {
return new Error("Missing template name");
}
if ( ! tplname.match(this._reValidIdentifier) ) {
if ( ! tplname.match(_reValidIdentifier) ) {
return new Error("Invalid characters in template name '" + tplname + "'");
}
var phold = template.placeholders;
for (var k in phold) {
if ( ! k.match(this._reValidIdentifier) ) {
return new Error("Invalid characters in placeholder name '" + k + "'");
}
if ( ! phold[k].hasOwnProperty('default') ) {
return new Error("Missing default for placeholder '" + k + "'");
}
if ( ! phold[k].hasOwnProperty('type') ) {
return new Error("Missing type for placeholder '" + k + "'");
}
};
var placeholders = template.placeholders || {};
var placeholderKeys = Object.keys(placeholders);
for (var i = 0, len = placeholderKeys.length; i < len; i++) {
var placeholderKey = placeholderKeys[i];
if (!placeholderKey.match(_reValidIdentifier)) {
return new Error("Invalid characters in placeholder name '" + placeholderKey + "'");
}
if ( ! placeholders[placeholderKey].hasOwnProperty('default') ) {
return new Error("Missing default for placeholder '" + placeholderKey + "'");
}
if ( ! placeholders[placeholderKey].hasOwnProperty('type') ) {
return new Error("Missing type for placeholder '" + placeholderKey + "'");
}
}
// Check certificate validity
var cert = this.getTemplateCertificate(template);
@@ -168,12 +168,11 @@ o._checkInvalidTemplate = function(template) {
// SignedMaps.addCertificate or SignedMaps.authorizedByCert
//
o.getTemplateCertificate = function(template) {
var cert = {
version: '0.0.1',
template_id: template.name,
auth: template.auth
return {
version: '0.0.1',
template_id: template.name,
auth: template.auth
};
return cert;
};
// Add a template
@@ -209,7 +208,7 @@ o.addTemplate = function(owner, template, callback) {
//
//
var usr_tpl_key = _.template(this.key_usr_tpl, {owner:owner});
var usr_tpl_key = this.key_usr_tpl({owner:owner});
var gotLock = false;
var that = this;
var limit = that._userTemplateLimit();
@@ -293,7 +292,7 @@ o.addTemplate = function(owner, template, callback) {
// @param callback function(err)
//
o.delTemplate = function(owner, tpl_id, callback) {
var usr_tpl_key = _.template(this.key_usr_tpl, {owner:owner});
var usr_tpl_key = this.key_usr_tpl({owner:owner});
var gotLock = false;
var that = this;
Step(
@@ -402,7 +401,7 @@ o.updTemplate = function(owner, tpl_id, template, callback) {
return;
}
var usr_tpl_key = _.template(this.key_usr_tpl, {owner:owner});
var usr_tpl_key = this.key_usr_tpl({owner:owner});
var gotLock = false;
var that = this;
Step(
@@ -496,8 +495,7 @@ o.updTemplate = function(owner, tpl_id, template, callback) {
// Returns a list of template identifiers
//
o.listTemplates = function(owner, callback) {
var usr_tpl_key = _.template(this.key_usr_tpl, {owner:owner});
this._redisCmd('HKEYS', [ usr_tpl_key ], callback);
this._redisCmd('HKEYS', [ this.key_usr_tpl({owner:owner}) ], callback);
};
// Get a templates
@@ -511,17 +509,15 @@ o.listTemplates = function(owner, callback) {
// Return full template definition
//
o.getTemplate = function(owner, tpl_id, callback) {
var usr_tpl_key = _.template(this.key_usr_tpl, {owner:owner});
var that = this;
Step(
function getTemplate() {
that._redisCmd('HGET', [ usr_tpl_key, tpl_id ], this);
that._redisCmd('HGET', [ that.key_usr_tpl({owner:owner}), tpl_id ], this);
},
function parseTemplate(err, tpl_val) {
if ( err ) throw err;
var tpl = JSON.parse(tpl_val);
// Should we strip auth_id ?
return tpl;
return JSON.parse(tpl_val);
},
function finish(err, tpl) {
callback(err, tpl);
@@ -541,25 +537,22 @@ o.getTemplate = function(owner, tpl_id, callback) {
//
// @throws Error on malformed template or parameter
//
o._reNumber = /^([-+]?[\d\.]?\d+([eE][+-]?\d+)?)$/;
o._reCSSColorName = /^[a-zA-Z]+$/;
o._reCSSColorVal = /^#[0-9a-fA-F]{3,6}$/;
o._replaceVars = function(str, params) {
var _reNumber = /^([-+]?[\d\.]?\d+([eE][+-]?\d+)?)$/,
_reCSSColorName = /^[a-zA-Z]+$/,
_reCSSColorVal = /^#[0-9a-fA-F]{3,6}$/;
_replaceVars = function(str, params) {
//return _.template(str, params); // lazy way, possibly dangerous
// Construct regular expressions for each param
if ( ! params._re ) {
params._re = {};
for (var k in params) {
params._re[k] = RegExp("<%=\\s*" + k + "\\s*%>", "g");
}
}
for (var k in params) str = str.replace(params._re[k], params[k]);
return str;
Object.keys(params).forEach(function(k) {
str = str.replace(new RegExp("<%=\\s*" + k + "\\s*%>", "g"), params[k]);
});
return str;
};
o.instance = function(template, params) {
var all_params = {};
var phold = template.placeholders;
for (var k in phold) {
var phold = template.placeholders || {};
Object.keys(phold).forEach(function(k) {
var val = params.hasOwnProperty(k) ? params[k] : phold[k].default;
var type = phold[k].type;
// properly escape
@@ -573,7 +566,7 @@ o.instance = function(template, params) {
}
else if ( type === 'number' ) {
// check it's a number
if ( typeof(val) !== 'number' && ! val.match(this._reNumber) ) {
if ( typeof(val) !== 'number' && ! val.match(_reNumber) ) {
throw new Error("Invalid number value for template parameter '"
+ k + "': " + val);
}
@@ -581,7 +574,7 @@ o.instance = function(template, params) {
else if ( type === 'css_color' ) {
// check it only contains letters or
// starts with # and only contains hexdigits
if ( ! val.match(this._reCSSColorName) && ! val.match(this._reCSSColorVal) ) {
if ( ! val.match(_reCSSColorName) && ! val.match(_reCSSColorVal) ) {
throw new Error("Invalid css_color value for template parameter '"
+ k + "': " + val);
}
@@ -591,14 +584,14 @@ o.instance = function(template, params) {
throw new Error("Invalid placeholder type '" + type + "'");
}
all_params[k] = val;
}
});
// NOTE: we're deep-cloning the layergroup here
var layergroup = JSON.parse(JSON.stringify(template.layergroup));
for (var i=0; i<layergroup.layers.length; ++i) {
var lyropt = layergroup.layers[i].options;
if ( lyropt.cartocss ) lyropt.cartocss = this._replaceVars(lyropt.cartocss, all_params);
if ( lyropt.sql) lyropt.sql = this._replaceVars(lyropt.sql, all_params);
if ( lyropt.cartocss ) lyropt.cartocss = _replaceVars(lyropt.cartocss, all_params);
if ( lyropt.sql) lyropt.sql = _replaceVars(lyropt.sql, all_params);
// Anything else ?
}
return layergroup;
npm-shrinkwrap.json
Generated
+4 -6
View File
@@ -9,6 +9,9 @@
"underscore": {
"version": "1.6.0"
},
"dot": {
"version": "1.0.2"
},
"windshaft": {
"version": "0.27.0",
"from": "https://github.com/CartoDB/Windshaft/tarball/0.27.0",
@@ -1230,12 +1233,7 @@
},
"cartodb-redis": {
"version": "0.11.0",
"from": "https://github.com/CartoDB/node-cartodb-redis/tarball/0.11.0",
"dependencies": {
"dot": {
"version": "1.0.2"
}
}
"from": "https://github.com/CartoDB/node-cartodb-redis/tarball/0.11.0"
},
"cartodb-psql": {
"version": "0.4.0",
package.json
+1
View File
@@ -24,6 +24,7 @@
"dependencies": {
"node-varnish": "https://github.com/Vizzuality/node-varnish/tarball/0.3.0",
"underscore" : "~1.6.0",
"dot": "~1.0.2",
"windshaft": "https://github.com/CartoDB/Windshaft/tarball/0.27.0",
"step": "~0.0.5",
"request": "~2.9.203",
test/unit/cartodb/template_maps.test.js
+93 -1
View File
@@ -12,6 +12,14 @@ suite('template_maps', function() {
// configure redis pool instance to use in tests
var redis_pool = RedisPool(global.environment.redis);
var signed_maps = new SignedMaps(redis_pool);
var validTemplate = {
version:'0.0.1',
name: 'first',
auth: {},
layergroup: {}
};
var owner = 'me';
test('does not accept template with unsupported version', function(done) {
var tmap = new TemplateMaps(redis_pool, signed_maps);
@@ -502,5 +510,89 @@ suite('template_maps', function() {
}
);
});
var redisCmdFunc = TemplateMaps.prototype._redisCmd;
function runWithRedisStubbed(stubbedCommands, func) {
TemplateMaps.prototype._redisCmd = function(redisFunc, redisArgs, callback) {
redisFunc = redisFunc.toLowerCase();
if (stubbedCommands.hasOwnProperty(redisFunc)) {
callback(null, stubbedCommands[redisFunc]);
} else {
throw 'Unknown command';
}
};
func();
TemplateMaps.prototype._redisCmd = redisCmdFunc;
}
test('_obtainTemplateLock with no previous value, happy case', function(done) {
runWithRedisStubbed({hget: null, hset: 1}, function() {
var templateMaps = new TemplateMaps(redis_pool, signed_maps);
templateMaps._obtainTemplateLock(owner, validTemplate.name, function(err, gotLock) {
assert.ok(!err);
assert.ok(gotLock);
done();
});
});
});
test('_obtainTemplateLock no lock for non expired ttl, simulates obtaining two locks at same time', function(done) {
runWithRedisStubbed({hget: Date.now()}, function() {
var templateMaps = new TemplateMaps(redis_pool, signed_maps);
templateMaps._obtainTemplateLock(owner, validTemplate.name, function(err, gotLock) {
assert.ok(!!err);
assert.equal(gotLock, false);
done();
});
});
});
test('_obtainTemplateLock no lock for non expired ttl, last millisecond of valid ttl', function(done) {
var nowValue = Date.now(),
nowFunc = Date.now;
Date.now = function() {
return nowValue;
};
var lockTtl = 1000;
runWithRedisStubbed({hget: Date.now() - lockTtl, hset: true}, function() {
var templateMaps = new TemplateMaps(redis_pool, signed_maps, {lock_ttl: lockTtl});
templateMaps._obtainTemplateLock(owner, validTemplate.name, function(err, gotLock) {
assert.ok(!!err);
assert.equal(gotLock, false);
Date.now = nowFunc;
done();
});
});
});
test('_obtainTemplateLock gets lock for expired ttl, first millisecond of invalid ttl', function(done) {
var nowValue = Date.now(),
nowFunc = Date.now;
Date.now = function() {
return nowValue;
};
var lockTtl = 1000;
runWithRedisStubbed({hget: Date.now() - lockTtl - 1, hset: true}, function() {
var templateMaps = new TemplateMaps(redis_pool, signed_maps, {lock_ttl: lockTtl});
templateMaps._obtainTemplateLock(owner, validTemplate.name, function(err, gotLock) {
assert.ok(!err);
assert.ok(gotLock);
Date.now = nowFunc;
done();
});
});
});
});