diff --git a/NEWS.md b/NEWS.md index ad315d16..66b9078a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,6 +11,7 @@ Enhancements: * Tested with node-0.10 (#141) * Use single redis pooler for torque and grainstore * Reduce cost of garbage collection for localized resources + * Allow limiting number of templates for each user (#136) 1.7.1 -- 2014-02-11 ------------------- diff --git a/config/environments/development.js.example b/config/environments/development.js.example index 791ebec7..97d2315c 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -24,6 +24,8 @@ var config = { // Maximum number of connections for one process // 128 is a good value with a limit of 1024 open file descriptors ,maxConnections:128 + // Maximum number of templates per user. Unlimited by default. + ,maxUserTemplates:1024 // idle socket timeout, in miliseconds ,socket_timeout: 600000 ,enable_cors: true diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 5f8ad1ab..255ae061 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -24,6 +24,8 @@ var config = { // Maximum number of connections for one process // 128 is a good value with a limit of 1024 open file descriptors ,maxConnections:128 + // Maximum number of templates per user. Unlimited by default. + ,maxUserTemplates:1024 // idle socket timeout, in miliseconds ,socket_timeout: 600000 ,enable_cors: true diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index 6dd5cc8a..1fb61fa9 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -24,6 +24,8 @@ var config = { // Maximum number of connections for one process // 128 is a good value with a limit of 1024 open file descriptors ,maxConnections:128 + // Maximum number of templates per user. Unlimited by default. + ,maxUserTemplates:1024 // idle socket timeout, in miliseconds ,socket_timeout: 600000 ,enable_cors: true diff --git a/config/environments/test.js.example b/config/environments/test.js.example index 43dfa507..b829e577 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -24,6 +24,8 @@ var config = { // Maximum number of connections for one process // 128 is a good value with a limit of 1024 open file descriptors ,maxConnections:128 + // Maximum number of templates per user. Unlimited by default. + ,maxUserTemplates:1024 // idle socket timeout, in miliseconds ,socket_timeout: 600000 ,enable_cors: true diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index b543e7eb..14160f48 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -30,6 +30,9 @@ var CartodbWindshaft = function(serverOptions) { } serverOptions.signedMaps = new SignedMaps(redisPool); + var templateMapsOps = { + max_user_templates: global.environment.maxUserTemplates + } var templateMaps = new TemplateMaps(redisPool, serverOptions.signedMaps); // boot diff --git a/lib/cartodb/template_maps.js b/lib/cartodb/template_maps.js index 117146ff..c5ca91d4 100644 --- a/lib/cartodb/template_maps.js +++ b/lib/cartodb/template_maps.js @@ -16,10 +16,15 @@ var user_template_locks = {}; // // @param signed_maps an instance of a "signed_maps" class, // See signed_maps.js +// +// @param opts TemplateMap options. Supported elements: +// 'max_user_templates' limit on the number of per-user +// // -function TemplateMaps(redis_pool, signed_maps) { +function TemplateMaps(redis_pool, signed_maps, opts) { this.redis_pool = redis_pool; this.signed_maps = signed_maps; + this.opts = opts || {}; // Database containing templates // TODO: allow configuring ? @@ -49,6 +54,10 @@ var o = TemplateMaps.prototype; //--------------- PRIVATE METHODS -------------------------------- +o._userTemplateLimit = function() { + return this.opts['max_user_templates'] || 0; +}; + o._acquireRedis = function(callback) { this.redis_pool.acquire(this.db_signatures, callback); }; @@ -189,6 +198,7 @@ o.addTemplate = function(owner, template, callback) { // Procedure: // + // - Check against limit // 0. Obtain a lock for user+template_name, fail if impossible // 1. Check no other template exists with the same name // 2. Install certificate extracted from template, extending @@ -202,9 +212,18 @@ o.addTemplate = function(owner, template, callback) { var usr_tpl_key = _.template(this.key_usr_tpl, {owner:owner}); var gotLock = false; var that = this; + var limit = that._userTemplateLimit(); Step( + function checkLimit() { + if ( ! limit ) return 0; + that._redisCmd('HLEN', [ usr_tpl_key ], this); + }, // try to obtain a lock - function obtainLock() { + function obtainLock(err, len) { + if ( err ) throw err; + if ( limit && len >= limit ) { + throw new Error("User '" + owner + "' reached limit on number of templates (" + len + "/" + limit + ")"); + } that._obtainTemplateLock(owner, tplname, this); }, function getExistingTemplate(err, locked) { @@ -238,10 +257,7 @@ o.addTemplate = function(owner, template, callback) { // TODO: how to recover this ?! } - if ( ! gotLock ) { - if ( err ) throw err; - return null; - } + if ( err && ! gotLock ) throw err; // release the lock var next = this; diff --git a/test/unit/cartodb/template_maps.test.js b/test/unit/cartodb/template_maps.test.js index 4ef04ed4..1314dbe6 100644 --- a/test/unit/cartodb/template_maps.test.js +++ b/test/unit/cartodb/template_maps.test.js @@ -420,5 +420,87 @@ suite('template_maps', function() { catch (e) { err = e; } assert.ok(!err); }); + + // Can set a limit on the number of user templates + test('can limit number of user templates', function(done) { + var tmap = new TemplateMaps(redis_pool, signed_maps, { + max_user_templates: 2 + }); + assert.ok(tmap); + var tpl = { version:'0.0.1', auth: {}, layergroup: {} }; + var expectErr = false; + var idMe = []; + var idYou = []; + Step( + function oneForMe() { + tpl.name = 'oneForMe'; + tmap.addTemplate('me', tpl, this); + }, + function twoForMe(err, id) { + if ( err ) throw err; + assert.ok(id); + idMe.push(id); + tpl.name = 'twoForMe'; + tmap.addTemplate('me', tpl, this); + }, + function threeForMe(err, id) { + if ( err ) throw err; + assert.ok(id); + idMe.push(id); + tpl.name = 'threeForMe'; + expectErr = true; + tmap.addTemplate('me', tpl, this); + }, + function errForMe(err, id) { + if ( err && ! expectErr ) throw err; + expectErr = false; + assert.ok(err); + assert.ok(err.message.match(/limit.*template/), err); + return null; + }, + function delOneMe(err) { + if ( err ) throw err; + tmap.delTemplate('me', idMe.shift(), this); + }, + function threeForMeRetry(err) { + if ( err ) throw err; + tpl.name = 'threeForMe'; + tmap.addTemplate('me', tpl, this); + }, + function oneForYou(err, id) { + if ( err ) throw err; + assert.ok(id); + idMe.push(id); + tpl.name = 'oneForYou'; + tmap.addTemplate('you', tpl, this); + }, + function twoForYou(err, id) { + if ( err ) throw err; + assert.ok(id); + idYou.push(id); + tpl.name = 'twoForYou'; + tmap.addTemplate('you', tpl, this); + }, + function threeForYou(err, id) { + if ( err ) throw err; + assert.ok(id); + idYou.push(id); + tpl.name = 'threeForYou'; + expectErr = true; + tmap.addTemplate('you', tpl, this); + }, + function errForYou(err, id) { + if ( err && ! expectErr ) throw err; + expectErr = false; + assert.ok(err); + assert.ok(err.message.match(/limit.*template/), err); + return null; + }, + function finish(err) { + // TODO: delete all templates + done(err); + } + ); + }); });