diff --git a/lib/cartodb/controllers/template_maps.js b/lib/cartodb/controllers/template_maps.js index be2022a8..f24336ec 100644 --- a/lib/cartodb/controllers/template_maps.js +++ b/lib/cartodb/controllers/template_maps.js @@ -1,4 +1,5 @@ var step = require('step'); +var assert = require('assert'); var _ = require('underscore'); var CdbRequest = require('../models/cdb_request'); @@ -42,7 +43,7 @@ TemplateMapsController.prototype.create = function(req, res) { self.serverOptions.authorizedByAPIKey(req, this); }, function addTemplate(err, authenticated) { - if ( err ) throw err; + assert.ifError(err); if (authenticated !== 1) { err = new Error("Only authenticated user can create templated maps"); err.http_status = 403; @@ -54,12 +55,11 @@ TemplateMapsController.prototype.create = function(req, res) { self.templateMaps.addTemplate(cdbuser, cfg, this); }, function prepareResponse(err, tpl_id){ - if ( err ) throw err; - // NOTE: might omit "cdbuser" if == dbowner ... - return { template_id: cdbuser + '@' + tpl_id }; + assert.ifError(err); + return { template_id: tpl_id }; }, function finish(err, response){ - if (err){ + if (err) { response = { error: ''+err }; var statusCode = 400; if ( ! _.isUndefined(err.http_status) ) { @@ -87,7 +87,8 @@ TemplateMapsController.prototype.update = function(req, res) { self.serverOptions.authorizedByAPIKey(req, this); }, function updateTemplate(err, authenticated) { - if ( err ) throw err; + assert.ifError(err); + if (authenticated !== 1) { err = new Error("Only authenticated user can list templated maps"); err.http_status = 403; @@ -96,23 +97,16 @@ TemplateMapsController.prototype.update = function(req, res) { if ( ! req.headers['content-type'] || req.headers['content-type'].split(';')[0] != 'application/json' ) throw new Error('template PUT data must be of type application/json'); template = req.body; - tpl_id = req.params.template_id.split('@'); - if ( tpl_id.length > 1 ) { - if ( tpl_id[0] != cdbuser ) { - err = new Error("Invalid template id '" + req.params.template_id + "' for user '" + cdbuser + "'"); - err.http_status = 404; - throw err; - } - tpl_id = tpl_id[1]; - } + tpl_id = templateName(req.params.template_id); self.templateMaps.updTemplate(cdbuser, tpl_id, template, this); }, function prepareResponse(err){ - if ( err ) throw err; - return { template_id: cdbuser + '@' + tpl_id }; + assert.ifError(err); + + return { template_id: tpl_id }; }, function finish(err, response){ - if (err){ + if (err) { var statusCode = 400; response = { error: ''+err }; if ( ! _.isUndefined(err.http_status) ) { @@ -149,19 +143,10 @@ TemplateMapsController.prototype.retrieve = function(req, res) { err.http_status = 403; throw err; } - tpl_id = req.params.template_id.split('@'); - if ( tpl_id.length > 1 ) { - if ( tpl_id[0] != cdbuser ) { - var templateNotFoundErr = new Error("Cannot get template id '" + req.params.template_id + - "' for user '" + cdbuser + "'"); - templateNotFoundErr.http_status = 404; - throw templateNotFoundErr; - } - tpl_id = tpl_id[1]; - } + tpl_id = templateName(req.params.template_id); self.templateMaps.getTemplate(cdbuser, tpl_id, this); }, - function prepareResponse(err, tpl_val){ + function prepareResponse(err, tpl_val) { if ( err ) throw err; if ( ! tpl_val ) { err = new Error("Cannot find template '" + tpl_id + "' of user '" + cdbuser + "'"); @@ -210,16 +195,7 @@ TemplateMapsController.prototype.destroy = function(req, res) { err.http_status = 403; throw err; } - tpl_id = req.params.template_id.split('@'); - if ( tpl_id.length > 1 ) { - if ( tpl_id[0] != cdbuser ) { - var templateNotFoundErr = new Error("Cannot find template id '" + req.params.template_id + - "' for user '" + cdbuser + "'"); - templateNotFoundErr.http_status = 404; - throw templateNotFoundErr; - } - tpl_id = tpl_id[1]; - } + tpl_id = templateName(req.params.template_id); self.templateMaps.delTemplate(cdbuser, tpl_id, this); }, function prepareResponse(err/*, tpl_val*/){ @@ -265,15 +241,13 @@ TemplateMapsController.prototype.list = function(req, res) { self.templateMaps.listTemplates(cdbuser, this); }, function prepareResponse(err, tpl_ids){ - if ( err ) throw err; - // NOTE: might omit "cbduser" if == dbowner ... - var ids = _.map(tpl_ids, function(id) { return cdbuser + '@' + id; }); - return { template_ids: ids }; + assert.ifError(err); + return { template_ids: tpl_ids }; }, function finish(err, response){ var statusCode = 200; - if (err){ - response = { error: ''+err }; + if (err) { + response = { error: '' + err }; if ( ! _.isUndefined(err.http_status) ) { statusCode = err.http_status; } @@ -350,17 +324,7 @@ TemplateMapsController.prototype.instantiateTemplate = function(req, res, templa var fakereq; // used for call to createLayergroup var cdbuser = cdbRequest.userByReq(req); // Format of template_id: []@ - var tpl_id = req.params.template_id.split('@'); - if ( tpl_id.length > 1 ) { - if ( tpl_id[0] && tpl_id[0] != cdbuser ) { - var err = new Error('Cannot instanciate map of user "' + tpl_id[0] + '" on database of user "' + cdbuser + - '"'); - err.http_status = 403; - callback(err); - return; - } - tpl_id = tpl_id[1]; - } + var tpl_id = templateName(req.params.template_id); var auth_token = req.query.auth_token; step( function getTemplate(){ @@ -468,3 +432,14 @@ TemplateMapsController.prototype.setDBParams = function(cdbuser, params, callbac } ); }; + +function templateName(templateId) { + var templateIdTokens = templateId.split('@'); + var name = templateIdTokens[0]; + + if (templateIdTokens.length > 1) { + name = templateIdTokens[1]; + } + + return name; +} diff --git a/test/acceptance/cache/surrogate_keys_invalidation.js b/test/acceptance/cache/surrogate_keys_invalidation.js index 8b626701..b4f4a165 100644 --- a/test/acceptance/cache/surrogate_keys_invalidation.js +++ b/test/acceptance/cache/surrogate_keys_invalidation.js @@ -23,7 +23,7 @@ describe('templates surrogate keys', function() { var templateOwner = 'localhost', templateName = 'acceptance', - expectedTemplateId = templateOwner + '@' + templateName, + expectedTemplateId = templateName, template = { version: '0.0.1', name: templateName, diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index aaf3e6b0..829abd12 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -69,7 +69,7 @@ describe('template_api', function() { it("can add template, returning id", function(done) { var errors = []; - var expected_tpl_id = "localhost@acceptance1"; + var expected_tpl_id = "acceptance1"; var post_request_1 = { url: '/api/v1/map/named', method: 'POST', @@ -801,7 +801,7 @@ describe('template_api', function() { ); }); - it("can instanciate a template by id", function(done) { + it("can instanciate a template by id wadus", function(done) { // This map fetches data from a private table var template_acceptance2 = { @@ -880,12 +880,10 @@ describe('template_api', function() { function instanciateAuth(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 403, res.statusCode + ': ' + res.body); + assert.equal(res.statusCode, 404, res.statusCode + ': ' + res.body); var parsed = JSON.parse(res.body); - assert.ok(parsed.hasOwnProperty('error'), - "Missing 'error' from response body: " + res.body); - assert.ok(parsed.error.match(/cannot instanciate/i), - 'Unexpected error for forbidden instance : ' + parsed.error); + assert.ok(parsed.hasOwnProperty('error'), "Missing 'error' from response body: " + res.body); + assert.ok(parsed.error.match(/not found/i), 'Unexpected error for forbidden instance : ' + parsed.error); var post_request = { url: '/api/v1/map/named/' + tpl_id + '?auth_token=valid2', method: 'POST', @@ -1907,7 +1905,7 @@ describe('template_api', function() { var template_params = {}; var errors = []; - var expectedTemplateId = username + '@' + httpTemplateName; + var expectedTemplateId = httpTemplateName; var layergroupid; step( function createTemplate()