diff --git a/lib/cartodb/cache/named_map_provider_cache.js b/lib/cartodb/cache/named_map_provider_cache.js index 0048cb63..7bb8e25c 100644 --- a/lib/cartodb/cache/named_map_provider_cache.js +++ b/lib/cartodb/cache/named_map_provider_cache.js @@ -7,12 +7,13 @@ var queue = require('queue-async'); var LruCache = require("lru-cache"); -function NamedMapProviderCache(templateMaps, pgConnection, userLimitsApi) { +function NamedMapProviderCache(templateMaps, pgConnection, userLimitsApi, turboCartocssAdapter) { this.templateMaps = templateMaps; this.pgConnection = pgConnection; this.userLimitsApi = userLimitsApi; this.namedLayersAdapter = new MapConfigNamedLayersAdapter(templateMaps); + this.turboCartocssAdapter = turboCartocssAdapter; this.providerCache = new LruCache({ max: 2000 }); } @@ -30,6 +31,7 @@ NamedMapProviderCache.prototype.get = function(user, templateId, config, authTok this.pgConnection, this.userLimitsApi, this.namedLayersAdapter, + this.turboCartocssAdapter, user, templateId, config, diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 2f93caa0..36b7b975 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -231,6 +231,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn self.pgConnection, self.userLimitsApi, self.namedLayersAdapter, + self.turboCartoCssAdapter, cdbuser, req.params.template_id, templateParams, diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 34013ece..c2d82f8c 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -15,12 +15,11 @@ var userMiddleware = require('../middleware/user'); * @param {TemplateMaps} templateMaps * @constructor */ -function NamedMapsAdminController(authApi, pgConnection, templateMaps, turboCartoCssAdapter) { +function NamedMapsAdminController(authApi, pgConnection, templateMaps) { BaseController.call(this, authApi, pgConnection); this.authApi = authApi; this.templateMaps = templateMaps; - this.turboCartoCssAdapter = turboCartoCssAdapter; } util.inherits(NamedMapsAdminController, BaseController); @@ -45,30 +44,11 @@ NamedMapsAdminController.prototype.create = function(req, res) { function checkPerms(){ self.authApi.authorizedByAPIKey(cdbuser, req, this); }, - function parseTurboCartoCss(err, authenticated) { + function addTemplate(err, authenticated) { assert.ifError(err); - ifUnauthenticated(authenticated, 'Only authenticated users can get template maps'); ifInvalidContentType(req, 'template POST data must be of type application/json'); - - var next = this; var cfg = req.body; - - self.turboCartoCssAdapter.getLayers(req.context.user, cfg.layergroup.layers, function (err, layers) { - if (err) { - return next(err); - } - - if (layers) { - cfg.layergroup.layers = layers; - } - - return next(null, cfg); - }); - }, - function addTemplate(err, cfg) { - assert.ifError(err); - self.templateMaps.addTemplate(cdbuser, cfg, this); }, function prepareResponse(err, tpl_id){ @@ -90,34 +70,14 @@ NamedMapsAdminController.prototype.update = function(req, res) { function checkPerms(){ self.authApi.authorizedByAPIKey(cdbuser, req, this); }, - function parseTurboCartoCss(err, authenticated) { + function updateTemplate(err, authenticated) { assert.ifError(err); - ifUnauthenticated(authenticated, 'Only authenticated users can get template maps'); ifInvalidContentType(req, 'template POST data must be of type application/json'); - var next = this; template = req.body; - - self.turboCartoCssAdapter.getLayers(cdbuser, template.layergroup.layers, function (err, layers) { - if (err) { - return next(err); - } - - if (layers) { - template.layergroup.layers = layers; - } - - return next(); - }); - }, - function updateTemplate(err) { - assert.ifError(err); - - var next = this; - tpl_id = templateName(req.params.template_id); - self.templateMaps.updTemplate(cdbuser, tpl_id, template, next); + self.templateMaps.updTemplate(cdbuser, tpl_id, template, this); }, function prepareResponse(err){ assert.ifError(err); diff --git a/lib/cartodb/models/mapconfig/named_map_provider.js b/lib/cartodb/models/mapconfig/named_map_provider.js index 3e09b466..a31bda13 100644 --- a/lib/cartodb/models/mapconfig/named_map_provider.js +++ b/lib/cartodb/models/mapconfig/named_map_provider.js @@ -12,11 +12,12 @@ var QueryTables = require('cartodb-query-tables'); * @type {NamedMapMapConfigProvider} */ function NamedMapMapConfigProvider(templateMaps, pgConnection, userLimitsApi, namedLayersAdapter, - owner, templateId, config, authToken, params) { + turboCartoCssAdapter, owner, templateId, config, authToken, params) { this.templateMaps = templateMaps; this.pgConnection = pgConnection; this.userLimitsApi = userLimitsApi; this.namedLayersAdapter = namedLayersAdapter; + this.turboCartoCssAdapter = turboCartoCssAdapter; this.owner = owner; this.templateName = templateName(templateId); @@ -91,6 +92,18 @@ NamedMapMapConfigProvider.prototype.getMapConfig = function(callback) { } ); }, + function parseTurboCartoCss(err, _mapConfig, datasource) { + assert.ifError(err); + var next = this; + + self.turboCartoCssAdapter.getLayers(self.owner, _mapConfig.layers, function (err, layers) { + if (!err && layers) { + _mapConfig.layers = layers; + } + + return next(null, _mapConfig, datasource); + }); + }, function beforeLayergroupCreate(err, _mapConfig, _datasource) { assert.ifError(err); mapConfig = _mapConfig; diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index dd599485..28eea3c5 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -142,7 +142,16 @@ module.exports = function(serverOptions) { var layergroupAffectedTablesCache = new LayergroupAffectedTablesCache(); app.layergroupAffectedTablesCache = layergroupAffectedTablesCache; - var namedMapProviderCache = new NamedMapProviderCache(templateMaps, pgConnection, userLimitsApi); + var turboCartoCssParser = new TurboCartocssParser(pgQueryRunner); + var turboCartocssAdapter = new TurboCartocssAdapter(turboCartoCssParser); + + var namedMapProviderCache = new NamedMapProviderCache( + templateMaps, + pgConnection, + userLimitsApi, + turboCartocssAdapter + ); + ['update', 'delete'].forEach(function(eventType) { templateMaps.on(eventType, namedMapProviderCache.invalidate.bind(namedMapProviderCache)); }); @@ -152,9 +161,6 @@ module.exports = function(serverOptions) { var TablesExtentApi = require('./api/tables_extent_api'); var tablesExtentApi = new TablesExtentApi(pgQueryRunner); - var turboCartoCssParser = new TurboCartocssParser(pgQueryRunner); - var turboCartocssAdapter = new TurboCartocssAdapter(turboCartoCssParser); - /******************************************************************************************************************* * Routing ******************************************************************************************************************/ @@ -196,7 +202,7 @@ module.exports = function(serverOptions) { metadataBackend ).register(app); - new controller.NamedMapsAdmin(authApi, pgConnection, templateMaps, turboCartocssAdapter).register(app); + new controller.NamedMapsAdmin(authApi, pgConnection, templateMaps).register(app); new controller.ServerInfo().register(app); diff --git a/test/acceptance/turbo-cartocss-named-maps.js b/test/acceptance/turbo-cartocss-named-maps.js index 4d22e78a..07c81918 100644 --- a/test/acceptance/turbo-cartocss-named-maps.js +++ b/test/acceptance/turbo-cartocss-named-maps.js @@ -5,6 +5,8 @@ var testHelper = require(__dirname + '/../support/test_helper'); var CartodbWindshaft = require(__dirname + '/../../lib/cartodb/server'); var serverOptions = require(__dirname + '/../../lib/cartodb/server_options'); var server = new CartodbWindshaft(serverOptions); +var mapnik = require('windshaft').mapnik; +var IMAGE_TOLERANCE_PER_MIL = 1; describe('turbo-cartocss for named maps', function() { @@ -18,48 +20,18 @@ describe('turbo-cartocss for named maps', function() { testHelper.deleteRedisKeys(keysToDelete, done); }); - var turboCartocss = [ - '#layer {' + - ' marker-fill: ramp([price], colorbrewer(Reds));' + - ' marker-allow-overlap:true;' + - '}' - ].join(''); - - var expectedCartocss = [ - '#layer {', - ' marker-allow-overlap:true;', - ' marker-fill:#fee5d9;', - ' [ price > 10.25 ] { marker-fill:#fcae91}', - ' [ price > 10.75 ] { marker-fill:#fb6a4a}', - ' [ price > 11.5 ] { marker-fill:#de2d26}', - ' [ price > 16.5 ] { marker-fill:#a50f15}', - '}' - ].join(''); - - var turboCartocssModified = [ - '#layer {' + - ' marker-fill: ramp([price], colorbrewer(Blues));' + - ' marker-allow-overlap:true;' + - '}' - ].join(''); - - var expectedUpdatedCartocss = [ - '#layer {', - ' marker-allow-overlap:true;', - ' marker-fill:#eff3ff;', - ' [ price > 10.25 ] { marker-fill:#bdd7e7}', - ' [ price > 10.75 ] { marker-fill:#6baed6}', - ' [ price > 11.5 ] { marker-fill:#3182bd}', - ' [ price > 16.5 ] { marker-fill:#08519c}', - '}' - ].join(''); - var templateId = 'turbo-cartocss-template-1'; var template = { version: '0.0.1', name: templateId, auth: { method: 'open' }, + placeholders: { + color: { + type: "css_color", + default: "Reds" + } + }, layergroup: { version: '1.0.0', layers: [{ @@ -77,7 +49,12 @@ describe('turbo-cartocss for named maps', function() { ' SELECT 5, 21.00', ') _prices ON _prices.cartodb_id = test_table.cartodb_id' ].join('\n'), - cartocss: turboCartocss, + cartocss: [ + '#layer {', + ' marker-fill: ramp([price], colorbrewer(<%= color %>));', + ' marker-allow-overlap:true;', + '}' + ].join('\n'), cartocss_version: '2.0.2' } } @@ -85,15 +62,8 @@ describe('turbo-cartocss for named maps', function() { } }; - var layergroup = { - version: '1.3.0', - layers: [{ - type: 'named', - options: { - name: templateId, - } - }] - }; + var templateParamsReds = { color: 'Reds' }; + var templateParamsBlues = { color: 'Blues' }; it('should create a template with turbo-cartocss parsed properly', function (done) { step( @@ -119,23 +89,26 @@ describe('turbo-cartocss for named maps', function() { return null; }, - function createLayergroup(err) { + function instantiateTemplateWithReds(err) { assert.ifError(err); var next = this; - assert.response(server, { - url: '/api/v1/map', + url: '/api/v1/map/named/' + templateId, method: 'POST', - headers: { host: 'localhost', 'Content-Type': 'application/json' }, - data: JSON.stringify(layergroup) + headers: { + host: 'localhost', + 'Content-Type': 'application/json' + }, + data: JSON.stringify(templateParamsReds) }, {}, - function (res, err) { - next(err, res); + function(res, err) { + return next(err, res); }); }, - function checkLayergroup(err, res) { + function checkInstanciationWithReds(err, res) { assert.ifError(err); + assert.equal(res.statusCode, 200); var parsedBody = JSON.parse(res.body); @@ -148,7 +121,7 @@ describe('turbo-cartocss for named maps', function() { return parsedBody.layergroupid; }, - function requestTile(err, layergroupId) { + function requestTileReds(err, layergroupId) { assert.ifError(err); var next = this; @@ -163,87 +136,77 @@ describe('turbo-cartocss for named maps', function() { next(err, res); }); }, - function checkTile(err, res) { + function checkTileReds(err, res) { assert.ifError(err); + var next = this; + assert.equal(res.statusCode, 200); assert.equal(res.headers['content-type'], 'image/png'); - testHelper.checkCache(res); + var fixturePath = './test/fixtures/turbo-cartocss-named-maps-reds.png'; + var image = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); - return null; + assert.imageIsSimilarToFile(image, fixturePath, IMAGE_TOLERANCE_PER_MIL, next); }, - function getTemplate() { + function instantiateTemplateWithBlues(err) { + assert.ifError(err); + + var next = this; + assert.response(server, { + url: '/api/v1/map/named/' + templateId, + method: 'POST', + headers: { + host: 'localhost', + 'Content-Type': 'application/json' + }, + data: JSON.stringify(templateParamsBlues) + }, {}, + function(res, err) { + return next(err, res); + }); + }, + function checkInstanciationWithBlues(err, res) { + assert.ifError(err); + assert.equal(res.statusCode, 200); + + var parsedBody = JSON.parse(res.body); + + keysToDelete['map_cfg|' + LayergroupToken.parse(parsedBody.layergroupid).token] = 0; + keysToDelete['user:localhost:mapviews:global'] = 5; + + assert.ok(parsedBody.layergroupid); + assert.ok(parsedBody.last_updated); + + return parsedBody.layergroupid; + }, + function requestTileBlues(err, layergroupId) { + assert.ifError(err); + var next = this; assert.response(server, { - url: '/api/v1/map/named/' + templateId + '?api_key=1234', + url: '/api/v1/map/' + layergroupId + '/0/0/0.png', method: 'GET', - headers: { host: 'localhost' } + headers: { host: 'localhost' }, + encoding: 'binary' }, {}, function(res, err) { next(err, res); }); }, - function checkTemplate(err, res) { + function checkTileBlues(err, res) { assert.ifError(err); - assert.equal(res.statusCode, 200); - - var bodyParsed = JSON.parse(res.body); - assert.equal(bodyParsed.template.layergroup.layers[0].options.cartocss, expectedCartocss); - - return null; - }, - function updateTemplate() { var next = this; - // clone the previous one and rename it - var changedTemplate = JSON.parse(JSON.stringify(template)); - changedTemplate.layergroup.layers[0].options.cartocss = turboCartocssModified; - - assert.response(server, { - url: '/api/v1/map/named/' + templateId + '/?api_key=1234', - method: 'PUT', - headers: { host: 'localhost', 'Content-Type': 'application/json' }, - data: JSON.stringify(changedTemplate) - }, {}, - function (res, err) { - next(err, res); - }); - }, - function checkUpdatedTemplate(err, res) { - assert.ifError(err); - assert.equal(res.statusCode, 200); + assert.equal(res.headers['content-type'], 'image/png'); - assert.deepEqual(JSON.parse(res.body), { - template_id: templateId - }); + var fixturePath = './test/fixtures/turbo-cartocss-named-maps-blues.png'; + var image = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); - return null; - }, - function getUpdatedTemplate() { - var next = this; - - assert.response(server, { - url: '/api/v1/map/named/' + templateId + '?api_key=1234', - method: 'GET', - headers: { host: 'localhost' } - }, {}, - function(res, err) { - next(err, res); - }); - }, - function checkGetUpdatedTemplate(err, res) { - assert.ifError(err); - - var bodyParsed = JSON.parse(res.body); - - assert.equal(res.statusCode, 200); - assert.equal(bodyParsed.template.layergroup.layers[0].options.cartocss, expectedUpdatedCartocss); - - return null; + assert.imageIsSimilarToFile(image, fixturePath, IMAGE_TOLERANCE_PER_MIL, next); }, function deleteTemplate(err) { assert.ifError(err); diff --git a/test/fixtures/turbo-cartocss-named-maps-blues.png b/test/fixtures/turbo-cartocss-named-maps-blues.png new file mode 100644 index 00000000..79f0d5d4 Binary files /dev/null and b/test/fixtures/turbo-cartocss-named-maps-blues.png differ diff --git a/test/fixtures/turbo-cartocss-named-maps-reds.png b/test/fixtures/turbo-cartocss-named-maps-reds.png new file mode 100644 index 00000000..65664285 Binary files /dev/null and b/test/fixtures/turbo-cartocss-named-maps-reds.png differ