diff --git a/NEWS.md b/NEWS.md index 3dd16c6f..baf2cf05 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,8 @@ Released 2016-mm-dd +Bug fixes: + - Correct URLs for widgets in named maps #381 ## 2.25.1 diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index a5555a2f..035537d8 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -186,6 +186,8 @@ MapController.prototype.create = function(req, res, prepareConfigFn) { if (err) { self.sendError(req, res, err, 'ANONYMOUS LAYERGROUP'); } else { + addWidgetsUrl(req.context.user, layergroup); + res.set('X-Layergroup-Id', layergroup.layergroupid); self.send(req, res, layergroup, 200); } @@ -260,6 +262,8 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; + addWidgetsUrl(req.context.user, layergroup); + res.set('X-Layergroup-Id', layergroup.layergroupid); self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); @@ -343,9 +347,6 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, la layergroup.layergroupid = layergroup.layergroupid + ':' + result.lastUpdatedTime; layergroup.last_updated = new Date(result.lastUpdatedTime).toISOString(); - // TODO this should take into account several URL patterns - addWidgetsUrl(username, layergroup); - if (req.method === 'GET') { var tableCacheEntry = new TablesCacheEntry(dbName, result.affectedTables); var ttl = global.environment.varnish.layergroupTtl || 86400; diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index ededd1b7..acd13ddc 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -190,38 +190,50 @@ describe(suiteName, function() { }); - it("should include serverMedata in the response", function(done) { - global.environment.serverMetadata = { cdn_url : { http:'test', https: 'tests' } }; - var layergroup = { - version: '1.0.0', - layers: [ - { options: { - sql: 'select cartodb_id, ST_Translate(the_geom_webmercator, 5e6, 0) as the_geom_webmercator' + - ' from test_table limit 2', - cartocss: '#layer { marker-fill:red; marker-width:32; marker-allow-overlap:true; }', - cartocss_version: '2.0.1' - } } - ] - }; + describe('server-metadata', function() { + var serverMetadata; + beforeEach(function() { + serverMetadata = global.environment.serverMetadata; + global.environment.serverMetadata = { cdn_url : { http:'test', https: 'tests' } }; + }); + + afterEach(function() { + global.environment.serverMetadata = serverMetadata; + }); + + it("should include serverMedata in the response", function(done) { + var layergroup = { + version: '1.0.0', + layers: [ + { options: { + sql: 'select cartodb_id, ST_Translate(the_geom_webmercator, 5e6, 0) as the_geom_webmercator' + + ' from test_table limit 2', + cartocss: '#layer { marker-fill:red; marker-width:32; marker-allow-overlap:true; }', + cartocss_version: '2.0.1' + } } + ] + }; + + step( + function do_create_get() + { + var next = this; + assert.response(server, { + url: layergroup_url + '?config=' + encodeURIComponent(JSON.stringify(layergroup)), + method: 'GET', + headers: {host: 'localhost'} + }, {}, function(res, err) { next(err, res); }); + }, + function do_check_create(err, res) { + var parsed = JSON.parse(res.body); + keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; + keysToDelete['user:localhost:mapviews:global'] = 5; + assert.ok(_.isEqual(parsed.cdn_url, global.environment.serverMetadata.cdn_url)); + done(); + } + ); + }); - step( - function do_create_get() - { - var next = this; - assert.response(server, { - url: layergroup_url + '?config=' + encodeURIComponent(JSON.stringify(layergroup)), - method: 'GET', - headers: {host: 'localhost'} - }, {}, function(res, err) { next(err, res); }); - }, - function do_check_create(err, res) { - var parsed = JSON.parse(res.body); - keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; - keysToDelete['user:localhost:mapviews:global'] = 5; - assert.ok(_.isEqual(parsed.cdn_url, global.environment.serverMetadata.cdn_url)); - done(); - } - ); }); diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index d4eb304b..0e1bd644 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -313,51 +313,63 @@ describe('template_api', function() { }); }); - it("instance endpoint should return server metadata", function(done){ - global.environment.serverMetadata = { cdn_url : { http:'test', https: 'tests' } }; - var tmpl = _.clone(template_acceptance1); - tmpl.name = "rambotemplate2"; + describe('server-metadata', function() { + var serverMetadata; + beforeEach(function() { + serverMetadata = global.environment.serverMetadata; + global.environment.serverMetadata = { cdn_url : { http:'test', https: 'tests' } }; + }); - step(function postTemplate1() { - var next = this; - var post_request = { - url: '/api/v1/map/named?api_key=1234', - method: 'POST', - headers: {host: 'localhost', 'Content-Type': 'application/json' }, - data: JSON.stringify(tmpl) - }; - assert.response(server, post_request, {}, function(res) { - next(null, res); - }); - }, - function testCORS() { - var next = this; - assert.response(server, { - url: '/api/v1/map/named/' + tmpl.name, - method: 'POST', - headers: {host: 'localhost', 'Content-Type': 'application/json' } - },{ - status: 200 - }, function(res) { - var parsed = JSON.parse(res.body); - keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; - keysToDelete['user:localhost:mapviews:global'] = 5; - assert.ok(_.isEqual(parsed.cdn_url, global.environment.serverMetadata.cdn_url)); - next(null); - }); - }, - function deleteTemplate(err) { - assert.ifError(err); - var del_request = { - url: '/api/v1/map/named/' + tmpl.name + '?api_key=1234', - method: 'DELETE', - headers: {host: 'localhost', 'Content-Type': 'application/json' } - }; - assert.response(server, del_request, {}, function() { - done(); - }); - } - ); + afterEach(function() { + global.environment.serverMetadata = serverMetadata; + }); + + + it("instance endpoint should return server metadata", function(done){ + var tmpl = _.clone(template_acceptance1); + tmpl.name = "rambotemplate2"; + + step(function postTemplate1() { + var next = this; + var post_request = { + url: '/api/v1/map/named?api_key=1234', + method: 'POST', + headers: {host: 'localhost', 'Content-Type': 'application/json' }, + data: JSON.stringify(tmpl) + }; + assert.response(server, post_request, {}, function(res) { + next(null, res); + }); + }, + function testCORS() { + var next = this; + assert.response(server, { + url: '/api/v1/map/named/' + tmpl.name, + method: 'POST', + headers: {host: 'localhost', 'Content-Type': 'application/json' } + },{ + status: 200 + }, function(res) { + var parsed = JSON.parse(res.body); + keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; + keysToDelete['user:localhost:mapviews:global'] = 5; + assert.ok(_.isEqual(parsed.cdn_url, global.environment.serverMetadata.cdn_url)); + next(null); + }); + }, + function deleteTemplate(err) { + assert.ifError(err); + var del_request = { + url: '/api/v1/map/named/' + tmpl.name + '?api_key=1234', + method: 'DELETE', + headers: {host: 'localhost', 'Content-Type': 'application/json' } + }; + assert.response(server, del_request, {}, function() { + done(); + }); + } + ); + }); }); diff --git a/test/acceptance/widgets/named-maps.js b/test/acceptance/widgets/named-maps.js index 4af90245..ee40d1ce 100644 --- a/test/acceptance/widgets/named-maps.js +++ b/test/acceptance/widgets/named-maps.js @@ -1,6 +1,9 @@ var assert = require('../../support/assert'); var step = require('step'); +var url = require('url'); +var queue = require('queue-async'); + var helper = require('../../support/test_helper'); var CartodbWindshaft = require('../../../lib/cartodb/server'); @@ -15,6 +18,7 @@ describe('named-maps widgets', function() { var widgetsTemplateName = 'widgets-template'; var layergroupid; + var layergroup; var keysToDelete; beforeEach(function(done) { @@ -33,6 +37,13 @@ describe('named-maps widgets', function() { cartocss: '#layer { marker-fill: blue; }', cartocss_version: '2.3.0', widgets: { + pop_max_formula_sum: { + type: 'formula', + options: { + column: 'pop_max', + operation: 'sum' + } + }, country_places_count: { type: 'aggregation', options: { @@ -105,12 +116,11 @@ describe('named-maps widgets', function() { function fetchTile(err, res) { assert.ifError(err); - var parsed = JSON.parse(res.body); - assert.ok( - parsed.hasOwnProperty('layergroupid'), "Missing 'layergroupid' from response body: " + res.body); - layergroupid = parsed.layergroupid; + layergroup = JSON.parse(res.body); + assert.ok(layergroup.hasOwnProperty('layergroupid'), "Missing 'layergroupid' from: " + res.body); + layergroupid = layergroup.layergroupid; - keysToDelete['map_cfg|' + LayergroupToken.parse(parsed.layergroupid).token] = 0; + keysToDelete['map_cfg|' + LayergroupToken.parse(layergroup.layergroupid).token] = 0; keysToDelete['user:localhost:mapviews:global'] = 5; return done(); @@ -171,6 +181,49 @@ describe('named-maps widgets', function() { ); } + it('should be able to retrieve widgets from all URLs', function(done) { + var widgetsPaths = layergroup.metadata.layers.reduce(function(paths, layer) { + var widgets = layer.widgets || {}; + Object.keys(widgets).forEach(function(widget) { + paths.push(url.parse(widgets[widget].url.http).path); + }); + + return paths; + }, []); + + var widgetsQueue = queue(widgetsPaths.length); + + widgetsPaths.forEach(function(path) { + widgetsQueue.defer(function(path, done) { + assert.response( + server, + { + url: path, + method: 'GET', + headers: { + host: username + } + }, + { + status: 200 + }, + function(res, err) { + if (err) { + return done(err); + } + var parsedBody = JSON.parse(res.body); + return done(null, parsedBody); + } + ); + }, path); + }); + + widgetsQueue.awaitAll(function(err, results) { + assert.equal(results.length, 3); + done(err); + }); + }); + it("should retrieve aggregation", function(done) { getWidget('country_places_count', function(err, response, aggregation) {