Moved turbo-cartocss integration from named maps admin to named map provider

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,

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,

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);

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;

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);
 

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 =  {
+    var templateParamsReds = { color: 'Reds' };
-        version: '1.3.0',
+    var templateParamsBlues = { color: 'Blues' };
-        layers: [{
-            type: 'named',
-            options: {
-                name: templateId,
-            }
-        }]
-    };
 
     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' },
+                    headers: {
-                    data: JSON.stringify(layergroup)
+                        host: 'localhost',
+                        'Content-Type': 'application/json'
+                    },
+                    data: JSON.stringify(templateParamsReds)
                 }, {},
                 function(res, err) {
-                    next(err, res);
+                    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), {
+                var fixturePath = './test/fixtures/turbo-cartocss-named-maps-blues.png';
-                    template_id: templateId
+                var image = mapnik.Image.fromBytes(new Buffer(res.body, 'binary'));
-                });
 
-                return null;
+                assert.imageIsSimilarToFile(image, fixturePath, IMAGE_TOLERANCE_PER_MIL, next);
-            },
-            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;
             },
             function deleteTemplate(err) {
                 assert.ifError(err);