From 235f5e456687bcbf0db4d6de3310b83ca72a4ad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 11:38:18 +0100 Subject: [PATCH 01/47] Extract cache channel to its own method --- lib/cartodb/controllers/layergroup.js | 71 ++++++++++++++------------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index dc6f85ff..d9551c9c 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -385,8 +385,6 @@ LayergroupController.prototype.staticMap = function(req, res, width, height, zoo }; LayergroupController.prototype.sendResponse = function(req, res, body, status, headers) { - var self = this; - req.profiler.done('res'); res.set('Cache-Control', 'public,max-age=31536000'); @@ -401,43 +399,48 @@ LayergroupController.prototype.sendResponse = function(req, res, body, status, h } res.set('Last-Modified', lastUpdated.toUTCString()); - var dbName = res.locals.dbname; - step( - function getAffectedTables() { - self.getAffectedTables(res.locals.user, dbName, res.locals.token, this); - }, - function sendResponse(err, affectedTables) { - req.profiler.done('affectedTables'); - if (err) { - global.logger.warn('ERROR generating cache channel: ' + err); - } - if (!!affectedTables) { - res.set('X-Cache-Channel', affectedTables.getCacheChannel()); - self.surrogateKeysCache.tag(res, affectedTables); - } - - if (headers) { - res.set(headers); - } - - res.status(status); - - if (!Buffer.isBuffer(body) && typeof body === 'object') { - if (req.query && req.query.callback) { - res.jsonp(body); - } else { - res.json(body); - } - } else { - res.send(body); - } + this.setCacheChannel(req, res, (err) => { + if (err) { + global.logger.warn('ERROR generating cache channel: ' + err); } - ); + if (headers) { + res.set(headers); + } + + res.status(status); + + if (!Buffer.isBuffer(body) && typeof body === 'object') { + if (req.query && req.query.callback) { + res.jsonp(body); + } else { + res.json(body); + } + } else { + res.send(body); + } + }); +}; + +LayergroupController.prototype.setCacheChannel = function(req, res, callback) { + const { dbname, user, token } = res.locals; + + this.getAffectedTables(user, dbname, token, (err, affectedTables) => { + req.profiler.done('affectedTables'); + if (err) { + return callback(err); + } + + if (!!affectedTables) { + res.set('X-Cache-Channel', affectedTables.getCacheChannel()); + this.surrogateKeysCache.tag(res, affectedTables); + } + + callback(); + }); }; LayergroupController.prototype.getAffectedTables = function(user, dbName, layergroupId, callback) { - if (this.layergroupAffectedTables.hasAffectedTables(dbName, layergroupId)) { return callback(null, this.layergroupAffectedTables.get(dbName, layergroupId)); } From 3f2ef63976decb9e828abb23efc296288f75f2b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 11:38:54 +0100 Subject: [PATCH 02/47] Extract cache channel to its own method --- lib/cartodb/controllers/named_maps.js | 85 +++++++++++++++------------ 1 file changed, 49 insertions(+), 36 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 55790bbf..ffea03a2 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -39,43 +39,56 @@ NamedMapsController.prototype.register = function(app) { ); }; -NamedMapsController.prototype.sendResponse = function(req, res, body, headers, namedMapProvider) { +NamedMapsController.prototype.sendResponse = function(req, res, body, headers) { + const { namedMapProvider } = res.locals; + this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(res.locals.user, namedMapProvider.getTemplateName())); res.set('Content-Type', headers['content-type'] || headers['Content-Type'] || 'image/png'); res.set('Cache-Control', 'public,max-age=7200,must-revalidate'); - var self = this; - - step( - function getAffectedTablesAndLastUpdatedTime() { - namedMapProvider.getAffectedTablesAndLastUpdatedTime(this); - }, - function sendResponse(err, result) { - req.profiler.done('affectedTables'); - if (err) { - global.logger.log('ERROR generating cache channel: ' + err); - } - if (!result || !!result.tables) { - // we increase cache control as we can invalidate it - res.set('Cache-Control', 'public,max-age=31536000'); - - var lastModifiedDate; - if (Number.isFinite(result.lastUpdatedTime)) { - lastModifiedDate = new Date(result.getLastUpdatedAt()); - } else { - lastModifiedDate = new Date(); - } - res.set('Last-Modified', lastModifiedDate.toUTCString()); - - res.set('X-Cache-Channel', result.getCacheChannel()); - if (result.tables.length > 0) { - self.surrogateKeysCache.tag(res, result); - } - } - res.status(200); - res.send(body); + this.setCacheChannel(req, res, (err) => { + if (err) { + global.logger.log('ERROR generating cache channel: ' + err); } - ); + + res.status(200); + res.send(body); + }); +}; + +NamedMapsController.prototype.setCacheChannel = function(req, res, callback) { + const { namedMapProvider } = res.locals; + + this.getAffectedTables(namedMapProvider, (err, result) => { + + req.profiler.done('affectedTables'); + if (err) { + global.logger.log('ERROR generating cache channel: ' + err); + } + if (!result || !!result.tables) { + // we increase cache control as we can invalidate it + res.set('Cache-Control', 'public,max-age=31536000'); + + var lastModifiedDate; + if (Number.isFinite(result.lastUpdatedTime)) { + lastModifiedDate = new Date(result.getLastUpdatedAt()); + } else { + lastModifiedDate = new Date(); + } + res.set('Last-Modified', lastModifiedDate.toUTCString()); + + res.set('X-Cache-Channel', result.getCacheChannel()); + if (result.tables.length > 0) { + this.surrogateKeysCache.tag(res, result); + } + } + + callback(); + }); +}; + +NamedMapsController.prototype.getAffectedTables = function(namedMapProvider, callback) { + namedMapProvider.getAffectedTablesAndLastUpdatedTime(callback); }; NamedMapsController.prototype.tile = function(req, res, next) { @@ -97,7 +110,7 @@ NamedMapsController.prototype.tile = function(req, res, next) { }, function getTile(err, _namedMapProvider) { assert.ifError(err); - namedMapProvider = _namedMapProvider; + res.locals.namedMapProvider = namedMapProvider = _namedMapProvider; self.tileBackend.getTile(namedMapProvider, req.params, this); }, function handleImage(err, tile, headers, stats) { @@ -106,7 +119,7 @@ NamedMapsController.prototype.tile = function(req, res, next) { err.label = 'NAMED_MAP_TILE'; next(err); } else { - self.sendResponse(req, res, tile, headers, namedMapProvider); + self.sendResponse(req, res, tile, headers); } } ); @@ -136,7 +149,7 @@ NamedMapsController.prototype.staticMap = function(req, res, next) { function prepareLayerVisibility(err, _namedMapProvider) { assert.ifError(err); - namedMapProvider = _namedMapProvider; + res.locals.namedMapProvider = namedMapProvider = _namedMapProvider; self.prepareLayerFilterFromPreviewLayers(cdbUser, req, res.locals, namedMapProvider, this); }, @@ -179,7 +192,7 @@ NamedMapsController.prototype.staticMap = function(req, res, next) { err.label = 'STATIC_VIZ_MAP'; next(err); } else { - self.sendResponse(req, res, image, headers, namedMapProvider); + self.sendResponse(req, res, image, headers); } } ); From 467bee4c911fddb5e780bffc560916fc49469c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 13:13:20 +0100 Subject: [PATCH 03/47] Split afterLayergroupCreate method in multiple "pre-middlewares" --- lib/cartodb/controllers/map.js | 157 +++++++++++++++++++++++---------- 1 file changed, 108 insertions(+), 49 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index f1bbe55e..4603a7b3 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -312,8 +312,6 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, layergroup, analysesResults, callback) { var self = this; - var username = res.locals.user; - var tasksleft = 2; // redis key and affectedTables var errors = []; @@ -335,6 +333,7 @@ function(req, res, mapconfig, layergroup, analysesResults, callback) { // Don't wait for the mapview count increment to // take place before proceeding. Error will be logged // asynchronously + var username = res.locals.user; this.metadataBackend.incMapviewCount(username, mapconfig.obj().stat_tag, function(err) { req.profiler.done('incMapviewCount'); if ( err ) { @@ -343,6 +342,40 @@ function(req, res, mapconfig, layergroup, analysesResults, callback) { done(); }); + res.locals.layergroup = layergroup; + res.locals.mapconfig = mapconfig; + res.locals.analysesResults = analysesResults; + + step( + function getAffectedTables () { + self.getAffectedTables(req, res, this); + }, + function setCacheChannel (err) { + assert.ifError(err); + self.setCacheChannel(req, res, this); + }, + function setLastUpdatedTime (err) { + assert.ifError(err); + self.setLastUpdatedTimeToLayergroup(req, res, this); + }, + function setCacheControl (err) { + assert.ifError(err); + self.setCacheControl(req, res, this); + }, + function setLayerStats (err) { + assert.ifError(err); + self.setLayerStats(req, res, this); + }, + function finish(err) { + done(err); + } + ); +}; + +MapController.prototype.getAffectedTables = function (req, res, callback) { + const self = this; + const { dbname, layergroup, user, mapconfig } = res.locals; + var sql = []; mapconfig.getLayers().forEach(function(layer) { sql.push(layer.options.sql); @@ -353,65 +386,91 @@ function(req, res, mapconfig, layergroup, analysesResults, callback) { } }); - var dbName = res.locals.dbname; - var layergroupId = layergroup.layergroupid; - var dbConnection; - step( function getPgConnection() { - self.pgConnection.getConnection(username, this); + self.pgConnection.getConnection(user, this); }, function getAffectedTablesAndLastUpdatedTime(err, connection) { assert.ifError(err); - dbConnection = connection; - QueryTables.getAffectedTablesFromQuery(dbConnection, sql.join(';'), this); + QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), this); }, - function handleAffectedTablesAndLastUpdatedTime(err, result) { - req.profiler.done('queryTablesAndLastUpdated'); - assert.ifError(err); - // feed affected tables cache so it can be reused from, for instance, layergroup controller - self.layergroupAffectedTables.set(dbName, layergroupId, result); - - var lastUpdateTime = result.getLastUpdatedAt(); - lastUpdateTime = getLastUpdatedTime(analysesResults, lastUpdateTime) || lastUpdateTime; - - // last update for layergroup cache buster - layergroup.layergroupid = layergroup.layergroupid + ':' + lastUpdateTime; - layergroup.last_updated = new Date(lastUpdateTime).toISOString(); - - if (req.method === 'GET') { - var ttl = global.environment.varnish.layergroupTtl || 86400; - res.set('Cache-Control', 'public,max-age='+ttl+',must-revalidate'); - res.set('Last-Modified', (new Date()).toUTCString()); - res.set('X-Cache-Channel', result.getCacheChannel()); - if (result.tables && result.tables.length > 0) { - self.surrogateKeysCache.tag(res, result); - } + function handleAffectedTablesAndLastUpdatedTime(err, affectedTables) { + if (err) { + return callback(err); } - return null; - }, - function fetchLayersStats(err) { - assert.ifError(err); - var next = this; - self.statsBackend.getStats(mapconfig, dbConnection, function(err, layersStats) { - if (err) { - return next(err); - } - if (layersStats.length > 0) { - layergroup.metadata.layers.forEach(function (layer, index) { - layer.meta.stats = layersStats[index]; - }); - } - return next(); - }); - }, - function finish(err) { - done(err); + // feed affected tables cache so it can be reused from, for instance, layergroup controller + self.layergroupAffectedTables.set(dbname, layergroup.layergroupId, affectedTables); + + res.locals.affectedTables = affectedTables; + + callback(); } ); }; +MapController.prototype.setCacheChannel = function (req, res, callback) { + const self = this; + const { affectedTables } = res.locals; + + if (req.method === 'GET') { + res.set('Last-Modified', (new Date()).toUTCString()); + res.set('X-Cache-Channel', affectedTables.getCacheChannel()); + if (affectedTables.tables && affectedTables.tables.length > 0) { + self.surrogateKeysCache.tag(res, affectedTables); + } + } + + callback(); +}; + +MapController.prototype.setLastUpdatedTimeToLayergroup = function (req, res, callback) { + const { affectedTables, layergroup, analysesResults } = res.locals; + + var lastUpdateTime = affectedTables.getLastUpdatedAt(); + + lastUpdateTime = getLastUpdatedTime(analysesResults, lastUpdateTime) || lastUpdateTime; + + // last update for layergroup cache buster + layergroup.layergroupid = layergroup.layergroupid + ':' + lastUpdateTime; + layergroup.last_updated = new Date(lastUpdateTime).toISOString(); + + callback(); +}; + +MapController.prototype.setCacheControl = function (req, res, callback) { + if (req.method === 'GET') { + var ttl = global.environment.varnish.layergroupTtl || 86400; + res.set('Cache-Control', 'public,max-age='+ttl+',must-revalidate'); + } + + callback(); +}; + +MapController.prototype.setLayerStats = function (req, res, callback) { + const { user, mapconfig, layergroup } = res.locals; + + this.pgConnection.getConnection(user, (err, connection) => { + if (err) { + return callback(err); + } + + this.statsBackend.getStats(mapconfig, connection, function(err, layersStats) { + if (err) { + return callback(err); + } + + if (layersStats.length > 0) { + layergroup.metadata.layers.forEach(function (layer, index) { + layer.meta.stats = layersStats[index]; + }); + } + + callback(); + }); + }); +}; + function getLastUpdatedTime(analysesResults, lastUpdateTime) { if (!Array.isArray(analysesResults)) { return lastUpdateTime; From 48172d4dc1475cd67ba7c52824b2225cdd11f27f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 13:36:17 +0100 Subject: [PATCH 04/47] make afterLayergroupCreate to follow the middleware signature --- lib/cartodb/controllers/map.js | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 4603a7b3..51d38d99 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -180,7 +180,11 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { }, function afterLayergroupCreate(err, layergroup) { assert.ifError(err); - self.afterLayergroupCreate(req, res, mapConfig, layergroup, context.analysesResults, this); + res.locals.mapconfig = mapConfig; + res.locals.analysesResults = context.analysesResults; + res.locals.layergroup = layergroup; + + self.afterLayergroupCreate(req, res, this); }, function finish(err, layergroup) { if (err) { @@ -276,9 +280,12 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn }, function afterLayergroupCreate(err, layergroup) { assert.ifError(err); - self.afterLayergroupCreate(req, res, mapConfig, layergroup, - mapConfigProvider.analysesResults, - this); + + res.locals.mapconfig = mapConfig; + res.locals.analysesResults = mapConfigProvider.analysesResults; + res.locals.layergroup = layergroup; + + self.afterLayergroupCreate(req, res, this); }, function finishTemplateInstantiation(err, layergroup) { if (err) { @@ -308,9 +315,9 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn ); }; -MapController.prototype.afterLayergroupCreate = -function(req, res, mapconfig, layergroup, analysesResults, callback) { +MapController.prototype.afterLayergroupCreate = function (req, res, callback) { var self = this; + const { layergroup, mapconfig, user } = res.locals; var tasksleft = 2; // redis key and affectedTables var errors = []; @@ -333,19 +340,14 @@ function(req, res, mapconfig, layergroup, analysesResults, callback) { // Don't wait for the mapview count increment to // take place before proceeding. Error will be logged // asynchronously - var username = res.locals.user; - this.metadataBackend.incMapviewCount(username, mapconfig.obj().stat_tag, function(err) { + this.metadataBackend.incMapviewCount(user, mapconfig.obj().stat_tag, function(err) { req.profiler.done('incMapviewCount'); if ( err ) { - global.logger.log("ERROR: failed to increment mapview count for user '" + username + "': " + err); + global.logger.log("ERROR: failed to increment mapview count for user '" + user + "': " + err); } done(); }); - res.locals.layergroup = layergroup; - res.locals.mapconfig = mapconfig; - res.locals.analysesResults = analysesResults; - step( function getAffectedTables () { self.getAffectedTables(req, res, this); From 3cf4a8f70b0966d47046cda0c21d1af3f90c2bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 13:46:03 +0100 Subject: [PATCH 05/47] Extract layergroup data augmentation to its own "middleware" --- lib/cartodb/controllers/map.js | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 51d38d99..6ebb3010 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -332,11 +332,6 @@ MapController.prototype.afterLayergroupCreate = function (req, res, callback) { } }; - // include in layergroup response the variables in serverMedata - // those variables are useful to send to the client information - // about how to reach this server or information about it - _.extend(layergroup, global.environment.serverMetadata); - // Don't wait for the mapview count increment to // take place before proceeding. Error will be logged // asynchronously @@ -349,7 +344,11 @@ MapController.prototype.afterLayergroupCreate = function (req, res, callback) { }); step( - function getAffectedTables () { + function () { + self.augmentLayergroupData(req, res, this); + }, + function getAffectedTables (err) { + assert.ifError(err); self.getAffectedTables(req, res, this); }, function setCacheChannel (err) { @@ -374,6 +373,17 @@ MapController.prototype.afterLayergroupCreate = function (req, res, callback) { ); }; +MapController.prototype.augmentLayergroupData = function (req, res, callback) { + const { layergroup } = res.locals; + + // include in layergroup response the variables in serverMedata + // those variables are useful to send to the client information + // about how to reach this server or information about it + _.extend(layergroup, global.environment.serverMetadata); + + callback(); +} + MapController.prototype.getAffectedTables = function (req, res, callback) { const self = this; const { dbname, layergroup, user, mapconfig } = res.locals; From 34e219353cff41fdab306bf1d27515877ea30c32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 14:47:29 +0100 Subject: [PATCH 06/47] do not pass layergroup since it's already available in res.locals --- lib/cartodb/controllers/map.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 6ebb3010..b2663299 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -317,7 +317,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn MapController.prototype.afterLayergroupCreate = function (req, res, callback) { var self = this; - const { layergroup, mapconfig, user } = res.locals; + const { mapconfig, user } = res.locals; var tasksleft = 2; // redis key and affectedTables var errors = []; @@ -328,7 +328,7 @@ MapController.prototype.afterLayergroupCreate = function (req, res, callback) { } if ( ! --tasksleft ) { err = errors.length ? new Error(errors.join('\n')) : null; - callback(err, layergroup); + callback(err); } }; @@ -340,7 +340,8 @@ MapController.prototype.afterLayergroupCreate = function (req, res, callback) { if ( err ) { global.logger.log("ERROR: failed to increment mapview count for user '" + user + "': " + err); } - done(); + + done(err); }); step( From fab87e21688ddce977d5584d8d99eace6a75afde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 15:47:59 +0100 Subject: [PATCH 07/47] Get layergroup from locals. It's not provided by previous middleware anymore --- lib/cartodb/controllers/map.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index b2663299..0c860f05 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -186,7 +186,7 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { self.afterLayergroupCreate(req, res, this); }, - function finish(err, layergroup) { + function finish(err) { if (err) { err = Number.isFinite(err.layerIndex) ? populateError(err, mapConfig) : err; @@ -195,6 +195,8 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { return next(err); } + const { layergroup } = res.locals; + var analysesResults = context.analysesResults || []; self.addDataviewsAndWidgetsUrls(res.locals.user, layergroup, mapConfig.obj()); self.addAnalysesMetadata(res.locals.user, layergroup, analysesResults, true); @@ -287,11 +289,13 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn self.afterLayergroupCreate(req, res, this); }, - function finishTemplateInstantiation(err, layergroup) { + function finishTemplateInstantiation(err) { if (err) { err.label = 'NAMED MAP LAYERGROUP'; next(err); } else { + const { layergroup } = res.locals; + var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; From 12822c4341bd95b8641c9eee27c5046d2da69f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 15:49:10 +0100 Subject: [PATCH 08/47] Follow node.js convention regarding early returns --- lib/cartodb/controllers/map.js | 40 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 0c860f05..94418b77 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -293,27 +293,27 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn if (err) { err.label = 'NAMED MAP LAYERGROUP'; next(err); + } + + const { layergroup } = res.locals; + + var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); + layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; + + var _mapConfig = mapConfig.obj(); + self.addDataviewsAndWidgetsUrls(cdbuser, layergroup, _mapConfig); + self.addAnalysesMetadata(cdbuser, layergroup, mapConfigProvider.analysesResults); + addContextMetadata(layergroup, _mapConfig, mapConfigProvider.context); + + res.set('X-Layergroup-Id', layergroup.layergroupid); + self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); + + res.status(200); + + if (req.query && req.query.callback) { + res.jsonp(layergroup); } else { - const { layergroup } = res.locals; - - var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); - layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; - - var _mapConfig = mapConfig.obj(); - self.addDataviewsAndWidgetsUrls(cdbuser, layergroup, _mapConfig); - self.addAnalysesMetadata(cdbuser, layergroup, mapConfigProvider.analysesResults); - addContextMetadata(layergroup, _mapConfig, mapConfigProvider.context); - - res.set('X-Layergroup-Id', layergroup.layergroupid); - self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); - - res.status(200); - - if (req.query && req.query.callback) { - res.jsonp(layergroup); - } else { - res.json(layergroup); - } + res.json(layergroup); } } ); From d1093686a3a1d9f0bcce35f1205af14ba60a19bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 15:51:42 +0100 Subject: [PATCH 09/47] Avoid to hold info in local variables --- lib/cartodb/controllers/map.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 94418b77..5f4f6e45 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -300,10 +300,9 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; - var _mapConfig = mapConfig.obj(); - self.addDataviewsAndWidgetsUrls(cdbuser, layergroup, _mapConfig); + self.addDataviewsAndWidgetsUrls(cdbuser, layergroup, mapConfig.obj()); self.addAnalysesMetadata(cdbuser, layergroup, mapConfigProvider.analysesResults); - addContextMetadata(layergroup, _mapConfig, mapConfigProvider.context); + addContextMetadata(layergroup, mapConfig.obj(), mapConfigProvider.context); res.set('X-Layergroup-Id', layergroup.layergroupid); self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); From a4b2044e10b475a0f86eabdc9aad41acab0ad106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 15:58:41 +0100 Subject: [PATCH 10/47] missing early return --- lib/cartodb/controllers/map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 5f4f6e45..26653ad3 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -292,7 +292,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn function finishTemplateInstantiation(err) { if (err) { err.label = 'NAMED MAP LAYERGROUP'; - next(err); + return next(err); } const { layergroup } = res.locals; From 5f7d5f6ec8993c4481bcda3ce7aa6a62ce1f60f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 16:01:18 +0100 Subject: [PATCH 11/47] Get analyses results from res.locals --- lib/cartodb/controllers/map.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 26653ad3..80f99d8e 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -189,15 +189,12 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { function finish(err) { if (err) { err = Number.isFinite(err.layerIndex) ? populateError(err, mapConfig) : err; - err.label = 'ANONYMOUS LAYERGROUP'; - return next(err); } - const { layergroup } = res.locals; + const { layergroup, analysesResults = [] } = res.locals; - var analysesResults = context.analysesResults || []; self.addDataviewsAndWidgetsUrls(res.locals.user, layergroup, mapConfig.obj()); self.addAnalysesMetadata(res.locals.user, layergroup, analysesResults, true); addContextMetadata(layergroup, mapConfig.obj(), context); @@ -295,13 +292,13 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn return next(err); } - const { layergroup } = res.locals; + const { layergroup, analysesResults = [] } = res.locals; var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; self.addDataviewsAndWidgetsUrls(cdbuser, layergroup, mapConfig.obj()); - self.addAnalysesMetadata(cdbuser, layergroup, mapConfigProvider.analysesResults); + self.addAnalysesMetadata(cdbuser, layergroup, analysesResults); addContextMetadata(layergroup, mapConfig.obj(), mapConfigProvider.context); res.set('X-Layergroup-Id', layergroup.layergroupid); From 39eb0f7bec5bc745b8b34002174e3e0bee86d266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 16:58:00 +0100 Subject: [PATCH 12/47] Avoid regression and update comment --- lib/cartodb/controllers/map.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 80f99d8e..05363a06 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -332,16 +332,15 @@ MapController.prototype.afterLayergroupCreate = function (req, res, callback) { } }; - // Don't wait for the mapview count increment to - // take place before proceeding. Error will be logged - // asynchronously + // Perform mapview count increment in parallel along the rest of after-layergroup-create + // tasks. Error won't blow up, just logged. this.metadataBackend.incMapviewCount(user, mapconfig.obj().stat_tag, function(err) { req.profiler.done('incMapviewCount'); if ( err ) { global.logger.log("ERROR: failed to increment mapview count for user '" + user + "': " + err); } - done(err); + done(); }); step( @@ -383,7 +382,7 @@ MapController.prototype.augmentLayergroupData = function (req, res, callback) { _.extend(layergroup, global.environment.serverMetadata); callback(); -} +}; MapController.prototype.getAffectedTables = function (req, res, callback) { const self = this; From bb02494e02b471bb9c5138e986712eb3ae16a048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 17:09:42 +0100 Subject: [PATCH 13/47] Do not perform "increment map view count" in parallel --- lib/cartodb/controllers/map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 05363a06..4f4213f4 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -333,7 +333,7 @@ MapController.prototype.afterLayergroupCreate = function (req, res, callback) { }; // Perform mapview count increment in parallel along the rest of after-layergroup-create - // tasks. Error won't blow up, just logged. + // tasks. Error won't blow up, just be logged. this.metadataBackend.incMapviewCount(user, mapconfig.obj().stat_tag, function(err) { req.profiler.done('incMapviewCount'); if ( err ) { From d85a5d83b79253bdc58ccd15dadee342b9922bac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 17:29:55 +0100 Subject: [PATCH 14/47] Make afterLayergroupCreate function as a 'middleware' builder --- lib/cartodb/controllers/map.js | 102 ++++++++++++++++----------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 4f4213f4..de0561cf 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -184,7 +184,9 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { res.locals.analysesResults = context.analysesResults; res.locals.layergroup = layergroup; - self.afterLayergroupCreate(req, res, this); + const afterLayergroupCreate = self.afterLayergroupCreateBuilder(); + + afterLayergroupCreate(req, res, this); }, function finish(err) { if (err) { @@ -284,7 +286,9 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn res.locals.analysesResults = mapConfigProvider.analysesResults; res.locals.layergroup = layergroup; - self.afterLayergroupCreate(req, res, this); + const afterLayergroupCreate = self.afterLayergroupCreateBuilder(); + + afterLayergroupCreate(req, res, this); }, function finishTemplateInstantiation(err) { if (err) { @@ -315,62 +319,58 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn ); }; -MapController.prototype.afterLayergroupCreate = function (req, res, callback) { +MapController.prototype.afterLayergroupCreateBuilder = function () { var self = this; + + return function (req, res, callback) { + step( + function incrementMapViewCount () { + self.incrementMapViewCount(req, res, this); + }, + function augmentLayergroupData (err) { + assert.ifError(err); + self.augmentLayergroupData(req, res, this); + }, + function getAffectedTables (err) { + assert.ifError(err); + self.getAffectedTables(req, res, this); + }, + function setCacheChannel (err) { + assert.ifError(err); + self.setCacheChannel(req, res, this); + }, + function setLastUpdatedTime (err) { + assert.ifError(err); + self.setLastUpdatedTimeToLayergroup(req, res, this); + }, + function setCacheControl (err) { + assert.ifError(err); + self.setCacheControl(req, res, this); + }, + function setLayerStats (err) { + assert.ifError(err); + self.setLayerStats(req, res, this); + }, + function finish(err) { + callback(err); + } + ); + }; +}; + +MapController.prototype.incrementMapViewCount = function (req, res, callback) { const { mapconfig, user } = res.locals; - var tasksleft = 2; // redis key and affectedTables - var errors = []; - - var done = function(err) { - if ( err ) { - errors.push('' + err); - } - if ( ! --tasksleft ) { - err = errors.length ? new Error(errors.join('\n')) : null; - callback(err); - } - }; - - // Perform mapview count increment in parallel along the rest of after-layergroup-create - // tasks. Error won't blow up, just be logged. - this.metadataBackend.incMapviewCount(user, mapconfig.obj().stat_tag, function(err) { + // Error won't blow up, just be logged. + this.metadataBackend.incMapviewCount(user, mapconfig.obj().stat_tag, (err) => { req.profiler.done('incMapviewCount'); - if ( err ) { - global.logger.log("ERROR: failed to increment mapview count for user '" + user + "': " + err); + + if (err) { + global.logger.log(`ERROR: failed to increment mapview count for user '${user}': ${err.message}`); } - done(); + callback(); }); - - step( - function () { - self.augmentLayergroupData(req, res, this); - }, - function getAffectedTables (err) { - assert.ifError(err); - self.getAffectedTables(req, res, this); - }, - function setCacheChannel (err) { - assert.ifError(err); - self.setCacheChannel(req, res, this); - }, - function setLastUpdatedTime (err) { - assert.ifError(err); - self.setLastUpdatedTimeToLayergroup(req, res, this); - }, - function setCacheControl (err) { - assert.ifError(err); - self.setCacheControl(req, res, this); - }, - function setLayerStats (err) { - assert.ifError(err); - self.setLayerStats(req, res, this); - }, - function finish(err) { - done(err); - } - ); }; MapController.prototype.augmentLayergroupData = function (req, res, callback) { From 99fa66c02691dea9517d7b26ae0ae5edc10869b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 18:02:23 +0100 Subject: [PATCH 15/47] Extract hash template for layergroup id and dataviews/widgets to a middlewares --- lib/cartodb/controllers/map.js | 45 ++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index de0561cf..41575629 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -197,7 +197,6 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { const { layergroup, analysesResults = [] } = res.locals; - self.addDataviewsAndWidgetsUrls(res.locals.user, layergroup, mapConfig.obj()); self.addAnalysesMetadata(res.locals.user, layergroup, analysesResults, true); addContextMetadata(layergroup, mapConfig.obj(), context); res.set('X-Layergroup-Id', layergroup.layergroupid); @@ -285,8 +284,11 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn res.locals.mapconfig = mapConfig; res.locals.analysesResults = mapConfigProvider.analysesResults; res.locals.layergroup = layergroup; + res.locals.template = mapConfigProvider.template; - const afterLayergroupCreate = self.afterLayergroupCreateBuilder(); + const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ + useTemplateHash: true + }); afterLayergroupCreate(req, res, this); }, @@ -298,10 +300,6 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn const { layergroup, analysesResults = [] } = res.locals; - var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); - layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; - - self.addDataviewsAndWidgetsUrls(cdbuser, layergroup, mapConfig.obj()); self.addAnalysesMetadata(cdbuser, layergroup, analysesResults); addContextMetadata(layergroup, mapConfig.obj(), mapConfigProvider.context); @@ -319,8 +317,9 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn ); }; -MapController.prototype.afterLayergroupCreateBuilder = function () { - var self = this; +MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { + const self = this; + const { useTemplateHash = false } = options; return function (req, res, callback) { step( @@ -351,6 +350,19 @@ MapController.prototype.afterLayergroupCreateBuilder = function () { assert.ifError(err); self.setLayerStats(req, res, this); }, + function augmentLayergroupIdWithTemplateHash (err) { + assert.ifError(err); + + if (!useTemplateHash) { + return null; + } + + self.augmentLayergroupIdWithTemplateHash(req, res, this); + }, + function (err) { + assert.ifError(err); + self.setDataviewsAndWidgetsUrlsToLayergroupMetadata(req, res, this); + }, function finish(err) { callback(err); } @@ -483,6 +495,23 @@ MapController.prototype.setLayerStats = function (req, res, callback) { }); }; +MapController.prototype.augmentLayergroupIdWithTemplateHash = function (req, res, callback) { + const { layergroup, user, template } = res.locals; + + var templateHash = this.templateMaps.fingerPrint(template).substring(0, 8); + layergroup.layergroupid = `${user}@${templateHash}@${layergroup.layergroupid}`; + + callback(); +}; + +MapController.prototype.setDataviewsAndWidgetsUrlsToLayergroupMetadata = function (req, res, callback) { + const { layergroup, user, mapconfig } = res.locals; + + this.addDataviewsAndWidgetsUrls(user, layergroup, mapconfig.obj()); + + callback(); +} + function getLastUpdatedTime(analysesResults, lastUpdateTime) { if (!Array.isArray(analysesResults)) { return lastUpdateTime; From cdc39c8cae5d3862741c7c7396eb2f98d25169e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 18:25:17 +0100 Subject: [PATCH 16/47] Extract addAnalysesMetadata functionallity to its own middleware --- lib/cartodb/controllers/map.js | 36 +++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 41575629..0163160a 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -184,7 +184,9 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { res.locals.analysesResults = context.analysesResults; res.locals.layergroup = layergroup; - const afterLayergroupCreate = self.afterLayergroupCreateBuilder(); + const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ + includeQuery: true + }); afterLayergroupCreate(req, res, this); }, @@ -195,9 +197,8 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { return next(err); } - const { layergroup, analysesResults = [] } = res.locals; + const { layergroup } = res.locals; - self.addAnalysesMetadata(res.locals.user, layergroup, analysesResults, true); addContextMetadata(layergroup, mapConfig.obj(), context); res.set('X-Layergroup-Id', layergroup.layergroupid); @@ -298,9 +299,8 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn return next(err); } - const { layergroup, analysesResults = [] } = res.locals; + const { layergroup } = res.locals; - self.addAnalysesMetadata(cdbuser, layergroup, analysesResults); addContextMetadata(layergroup, mapConfig.obj(), mapConfigProvider.context); res.set('X-Layergroup-Id', layergroup.layergroupid); @@ -319,7 +319,10 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { const self = this; - const { useTemplateHash = false } = options; + const { + useTemplateHash = false, + includeQuery = false + } = options; return function (req, res, callback) { step( @@ -363,6 +366,13 @@ MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { assert.ifError(err); self.setDataviewsAndWidgetsUrlsToLayergroupMetadata(req, res, this); }, + function setAnalysesMetadataToLayergroup(err) { + assert.ifError(err); + + const setAnalysesMetadataToLayergroup = self.setAnalysesMetadataToLayergroupBuilder(includeQuery); + + setAnalysesMetadataToLayergroup(req, res, this); + }, function finish(err) { callback(err); } @@ -510,7 +520,19 @@ MapController.prototype.setDataviewsAndWidgetsUrlsToLayergroupMetadata = functio this.addDataviewsAndWidgetsUrls(user, layergroup, mapconfig.obj()); callback(); -} +}; + +MapController.prototype.setAnalysesMetadataToLayergroupBuilder = function (includeQuery) { + const self = this; + + return function setAnalysesMetadataToLayergroup (req, res, callback) { + const { layergroup, user, analysesResults = [] } = res.locals; + + self.addAnalysesMetadata(user, layergroup, analysesResults, includeQuery); + + callback(); + }; +}; function getLastUpdatedTime(analysesResults, lastUpdateTime) { if (!Array.isArray(analysesResults)) { From da2228088e16103323cb2f0cbdee005cf4c94f1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 18:42:11 +0100 Subject: [PATCH 17/47] Extract context metadata (turbo-carto) functionallity to its own middleware --- lib/cartodb/controllers/map.js | 39 ++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 0163160a..b6348f3f 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -183,6 +183,7 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { res.locals.mapconfig = mapConfig; res.locals.analysesResults = context.analysesResults; res.locals.layergroup = layergroup; + res.locals.context = context; const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ includeQuery: true @@ -199,7 +200,6 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { const { layergroup } = res.locals; - addContextMetadata(layergroup, mapConfig.obj(), context); res.set('X-Layergroup-Id', layergroup.layergroupid); res.status(200); @@ -232,17 +232,6 @@ function populateError(err, mapConfig) { return error; } -function addContextMetadata(layergroup, mapConfig, context) { - if (layergroup.metadata && Array.isArray(layergroup.metadata.layers) && Array.isArray(mapConfig.layers)) { - layergroup.metadata.layers = layergroup.metadata.layers.map(function(layer, layerIndex) { - if (context.turboCarto && Array.isArray(context.turboCarto.layers)) { - layer.meta.cartocss_meta = context.turboCarto.layers[layerIndex]; - } - return layer; - }); - } -} - MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn, next) { var self = this; @@ -286,6 +275,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn res.locals.analysesResults = mapConfigProvider.analysesResults; res.locals.layergroup = layergroup; res.locals.template = mapConfigProvider.template; + res.locals.context = mapConfigProvider.context; const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ useTemplateHash: true @@ -301,8 +291,6 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn const { layergroup } = res.locals; - addContextMetadata(layergroup, mapConfig.obj(), mapConfigProvider.context); - res.set('X-Layergroup-Id', layergroup.layergroupid); self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); @@ -373,6 +361,10 @@ MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { setAnalysesMetadataToLayergroup(req, res, this); }, + function setTurboCartoMetadataToLayergroup (err) { + assert.ifError(err); + self.setTurboCartoMetadataToLayergroup(req, res, this); + }, function finish(err) { callback(err); } @@ -534,6 +526,25 @@ MapController.prototype.setAnalysesMetadataToLayergroupBuilder = function (inclu }; }; +MapController.prototype.setTurboCartoMetadataToLayergroup = function (req, res, callback) { + const { layergroup, mapconfig, context } = res.locals; + + addContextMetadata(layergroup, mapconfig.obj(), context); + + callback(); +}; + +function addContextMetadata(layergroup, mapConfig, context) { + if (layergroup.metadata && Array.isArray(layergroup.metadata.layers) && Array.isArray(mapConfig.layers)) { + layergroup.metadata.layers = layergroup.metadata.layers.map(function(layer, layerIndex) { + if (context.turboCarto && Array.isArray(context.turboCarto.layers)) { + layer.meta.cartocss_meta = context.turboCarto.layers[layerIndex]; + } + return layer; + }); + } +} + function getLastUpdatedTime(analysesResults, lastUpdateTime) { if (!Array.isArray(analysesResults)) { return lastUpdateTime; From e06f8fe25ec137596bc166e2865f70684f1aa347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 18:54:32 +0100 Subject: [PATCH 18/47] Set layergroup-id header in the right middleware --- lib/cartodb/controllers/map.js | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index b6348f3f..1859dd07 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -200,8 +200,6 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { const { layergroup } = res.locals; - res.set('X-Layergroup-Id', layergroup.layergroupid); - res.status(200); if (req.query && req.query.callback) { @@ -291,7 +289,6 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn const { layergroup } = res.locals; - res.set('X-Layergroup-Id', layergroup.layergroupid); self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); res.status(200); @@ -341,14 +338,12 @@ MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { assert.ifError(err); self.setLayerStats(req, res, this); }, - function augmentLayergroupIdWithTemplateHash (err) { + function setLayergroupIdHeader (err) { assert.ifError(err); - if (!useTemplateHash) { - return null; - } + const setLayergroupIdHeader = self.setLayergroupIdHeaderBuilder(useTemplateHash); - self.augmentLayergroupIdWithTemplateHash(req, res, this); + setLayergroupIdHeader(req, res, this); }, function (err) { assert.ifError(err); @@ -497,13 +492,20 @@ MapController.prototype.setLayerStats = function (req, res, callback) { }); }; -MapController.prototype.augmentLayergroupIdWithTemplateHash = function (req, res, callback) { - const { layergroup, user, template } = res.locals; +MapController.prototype.setLayergroupIdHeaderBuilder = function (useTemplateHash) { + const self = this; + return function setLayergroupIdHeader(req, res, callback) { + const { layergroup, user, template } = res.locals; - var templateHash = this.templateMaps.fingerPrint(template).substring(0, 8); - layergroup.layergroupid = `${user}@${templateHash}@${layergroup.layergroupid}`; + if (useTemplateHash) { + var templateHash = self.templateMaps.fingerPrint(template).substring(0, 8); + layergroup.layergroupid = `${user}@${templateHash}@${layergroup.layergroupid}`; + } - callback(); + res.set('X-Layergroup-Id', layergroup.layergroupid); + + callback(); + }; }; MapController.prototype.setDataviewsAndWidgetsUrlsToLayergroupMetadata = function (req, res, callback) { From e0ffeb0adc0c5ea6b248667b3c41b0dfed2b8af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 19:49:12 +0100 Subject: [PATCH 19/47] extract surrogate key functionality to its own middleware --- lib/cartodb/controllers/map.js | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 1859dd07..ac874e59 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -274,6 +274,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn res.locals.layergroup = layergroup; res.locals.template = mapConfigProvider.template; res.locals.context = mapConfigProvider.context; + res.locals.templateName = mapConfigProvider.getTemplateName(); const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ useTemplateHash: true @@ -289,8 +290,6 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn const { layergroup } = res.locals; - self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); - res.status(200); if (req.query && req.query.callback) { @@ -360,6 +359,10 @@ MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { assert.ifError(err); self.setTurboCartoMetadataToLayergroup(req, res, this); }, + function setSurrogateKeyHeader (err) { + assert.ifError(err); + self.setSurrogateKeyHeader(req, res, this); + }, function finish(err) { callback(err); } @@ -431,15 +434,11 @@ MapController.prototype.getAffectedTables = function (req, res, callback) { }; MapController.prototype.setCacheChannel = function (req, res, callback) { - const self = this; const { affectedTables } = res.locals; if (req.method === 'GET') { res.set('Last-Modified', (new Date()).toUTCString()); res.set('X-Cache-Channel', affectedTables.getCacheChannel()); - if (affectedTables.tables && affectedTables.tables.length > 0) { - self.surrogateKeysCache.tag(res, affectedTables); - } } callback(); @@ -547,6 +546,20 @@ function addContextMetadata(layergroup, mapConfig, context) { } } +MapController.prototype.setSurrogateKeyHeader = function (req, res, callback) { + const { affectedTables, user, templateName } = res.locals; + + if (req.method === 'GET' && affectedTables.tables && affectedTables.tables.length > 0) { + this.surrogateKeysCache.tag(res, affectedTables); + } + + if (templateName) { + this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, templateName)); + } + + callback(); +}; + function getLastUpdatedTime(analysesResults, lastUpdateTime) { if (!Array.isArray(analysesResults)) { return lastUpdateTime; From 3d15551cb5831a17afa6372bc388fed803e0bba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 20:10:37 +0100 Subject: [PATCH 20/47] Minor style umprovements --- lib/cartodb/controllers/map.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index ac874e59..054a901e 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -180,6 +180,7 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { }, function afterLayergroupCreate(err, layergroup) { assert.ifError(err); + res.locals.mapconfig = mapConfig; res.locals.analysesResults = context.analysesResults; res.locals.layergroup = layergroup; @@ -273,8 +274,8 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn res.locals.analysesResults = mapConfigProvider.analysesResults; res.locals.layergroup = layergroup; res.locals.template = mapConfigProvider.template; - res.locals.context = mapConfigProvider.context; res.locals.templateName = mapConfigProvider.getTemplateName(); + res.locals.context = mapConfigProvider.context; const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ useTemplateHash: true From 6bbaeaa2860337782e6be604fc2d00c02c62da86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 31 Oct 2017 20:49:26 +0100 Subject: [PATCH 21/47] Create a custom error middleware to augment error info --- lib/cartodb/controllers/map.js | 85 ++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 30 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 054a901e..82a7732b 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -56,28 +56,42 @@ MapController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.createGet.bind(this) + this.createGet.bind(this), + mapErrorMiddleware({ + label: 'ANONYMOUS LAYERGROUP', + augmentError: true + }) ); app.post( app.base_url_mapconfig, cors(), userMiddleware, this.prepareContext, - this.createPost.bind(this) + this.createPost.bind(this), + mapErrorMiddleware({ + label: 'ANONYMOUS LAYERGROUP', + augmentError: true + }) ); app.get( app.base_url_templated + '/:template_id/jsonp', cors(), userMiddleware, this.prepareContext, - this.jsonp.bind(this) + this.jsonp.bind(this), + mapErrorMiddleware({ + label: 'NAMED MAP LAYERGROUP' + }) ); app.post( app.base_url_templated + '/:template_id', cors(), userMiddleware, this.prepareContext, - this.instantiate.bind(this) + this.instantiate.bind(this), + mapErrorMiddleware({ + label: 'NAMED MAP LAYERGROUP' + }) ); app.options(app.base_url_mapconfig, cors('Content-Type')); }; @@ -170,7 +184,7 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { function createLayergroup(err, requestMapConfig) { assert.ifError(err); var datasource = context.datasource || Datasource.EmptyDatasource(); - mapConfig = new MapConfig(requestMapConfig, datasource); + res.locals.mapconfig = mapConfig = new MapConfig(requestMapConfig, datasource); self.mapBackend.createLayergroup( mapConfig, res.locals, @@ -181,7 +195,6 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { function afterLayergroupCreate(err, layergroup) { assert.ifError(err); - res.locals.mapconfig = mapConfig; res.locals.analysesResults = context.analysesResults; res.locals.layergroup = layergroup; res.locals.context = context; @@ -194,8 +207,6 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { }, function finish(err) { if (err) { - err = Number.isFinite(err.layerIndex) ? populateError(err, mapConfig) : err; - err.label = 'ANONYMOUS LAYERGROUP'; return next(err); } @@ -212,25 +223,6 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { ); }; -function populateError(err, mapConfig) { - var error = new Error(err.message); - error.http_status = err.http_status; - - if (!err.http_status && err.message.indexOf('column "the_geom_webmercator" does not exist') >= 0) { - error.http_status = 400; - } - - error.type = 'layer'; - error.subtype = err.message.indexOf('Postgis Plugin') >= 0 ? 'query' : undefined; - error.layer = { - id: mapConfig.getLayerId(err.layerIndex), - index: err.layerIndex, - type: mapConfig.layerType(err.layerIndex) - }; - - return error; -} - MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn, next) { var self = this; @@ -260,7 +252,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn }, function createLayergroup(err, mapConfig_, rendererParams) { assert.ifError(err); - mapConfig = mapConfig_; + res.locals.mapconfig = mapConfig = mapConfig_; self.mapBackend.createLayergroup( mapConfig, rendererParams, new CreateLayergroupMapConfigProvider(mapConfig, cdbuser, self.userLimitsApi, rendererParams), @@ -270,7 +262,6 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn function afterLayergroupCreate(err, layergroup) { assert.ifError(err); - res.locals.mapconfig = mapConfig; res.locals.analysesResults = mapConfigProvider.analysesResults; res.locals.layergroup = layergroup; res.locals.template = mapConfigProvider.template; @@ -285,7 +276,6 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn }, function finishTemplateInstantiation(err) { if (err) { - err.label = 'NAMED MAP LAYERGROUP'; return next(err); } @@ -640,3 +630,38 @@ MapController.prototype.addWidgetsUrl = function(username, layergroup, mapConfig }.bind(this)); } }; + +function mapErrorMiddleware (options) { + const { augmentError = false, label = 'MAPS CONTROLLER' } = options; + + return function mapError (err, req, res, next) { + const { mapconfig } = res.locals; + + if (augmentError) { + err = Number.isFinite(err.layerIndex) ? populateError(err, mapconfig) : err; + } + + err.label = label; + + next(err); + }; +} + +function populateError(err, mapConfig) { + var error = new Error(err.message); + error.http_status = err.http_status; + + if (!err.http_status && err.message.indexOf('column "the_geom_webmercator" does not exist') >= 0) { + error.http_status = 400; + } + + error.type = 'layer'; + error.subtype = err.message.indexOf('Postgis Plugin') >= 0 ? 'query' : undefined; + error.layer = { + id: mapConfig.getLayerId(err.layerIndex), + index: err.layerIndex, + type: mapConfig.layerType(err.layerIndex) + }; + + return error; +} From 8ed5df00727724abe1201d4f498eff18ee100bdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 1 Nov 2017 17:57:35 +0100 Subject: [PATCH 22/47] Move prepeareConfigFn to a middleware --- lib/cartodb/controllers/map.js | 115 +++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 49 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 82a7732b..4566cdb3 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -56,6 +56,7 @@ MapController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, + createGetPrepareConfig, this.createGet.bind(this), mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -67,6 +68,7 @@ MapController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, + createPostPrepareConfig, this.createPost.bind(this), mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -78,6 +80,7 @@ MapController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, + prepareJsonTemplateParams, this.jsonp.bind(this), mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -88,6 +91,7 @@ MapController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, + prepareTemplateParams, this.instantiate.bind(this), mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -96,62 +100,80 @@ MapController.prototype.register = function(app) { app.options(app.base_url_mapconfig, cors('Content-Type')); }; +function createGetPrepareConfig (req, res, next) { + const { config } = res.locals; + + if (!config) { + return next(new Error('layergroup GET needs a "config" parameter')); + } + + try { + req.body = JSON.parse(config); + } catch (err) { + return next(err); + } + + return next(); +} + +function createPostPrepareConfig(req, res, next) { + if (!req.is('application/json')) { + return next(new Error('layergroup POST data must be of type application/json')); + } + + next(); +} + +function prepareTemplateParams(req, res, next) { + if (!req.is('application/json')) { + return next(new Error('Template POST data must be of type application/json')); + } + + return next(); +} + +function prepareJsonTemplateParams(req, res, next) { + const { callback, config } = req.query; + + if (callback === undefined || callback.length === 0) { + return next(new Error('callback parameter should be present and be a function name')); + } + + if (config) { + try { + req.body = JSON.parse(config); + } catch(e) { + return next(new Error('Invalid config parameter, should be a valid JSON')); + } + } + + return next(); +} + MapController.prototype.createGet = function(req, res, next){ req.profiler.start('windshaft.createmap_get'); - this.create(req, res, function createGet$prepareConfig(req, config) { - if ( ! config ) { - throw new Error('layergroup GET needs a "config" parameter'); - } - return JSON.parse(config); - }, next); + this.create(req, res, next); }; MapController.prototype.createPost = function(req, res, next) { req.profiler.start('windshaft.createmap_post'); - this.create(req, res, function createPost$prepareConfig(req) { - if (!req.is('application/json')) { - throw new Error('layergroup POST data must be of type application/json'); - } - return req.body; - }, next); + this.create(req, res, next); }; MapController.prototype.instantiate = function(req, res, next) { req.profiler.start('windshaft-cartodb.instance_template_post'); - this.instantiateTemplate(req, res, function prepareTemplateParams(callback) { - if (!req.is('application/json')) { - return callback(new Error('Template POST data must be of type application/json')); - } - return callback(null, req.body); - }, next); + this.instantiateTemplate(req, res, next); }; MapController.prototype.jsonp = function(req, res, next) { req.profiler.start('windshaft-cartodb.instance_template_get'); - - this.instantiateTemplate(req, res, function prepareJsonTemplateParams(callback) { - var err = null; - if ( req.query.callback === undefined || req.query.callback.length === 0) { - err = new Error('callback parameter should be present and be a function name'); - } - - var templateParams = {}; - if (req.query.config) { - try { - templateParams = JSON.parse(req.query.config); - } catch(e) { - err = new Error('Invalid config parameter, should be a valid JSON'); - } - } - - return callback(err, templateParams); - }, next); + this.instantiateTemplate(req, res, next); }; -MapController.prototype.create = function(req, res, prepareConfigFn, next) { +MapController.prototype.create = function(req, res, next) { var self = this; var mapConfig; @@ -159,12 +181,9 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { var context = {}; step( - function prepareConfig () { - const requestMapConfig = prepareConfigFn(req, res.locals.config); - return requestMapConfig; - }, - function prepareAdapterMapConfig(err, requestMapConfig) { - assert.ifError(err); + function prepareAdapterMapConfig() { + const requestMapConfig = req.body; + context.analysisConfiguration = { user: res.locals.user, db: { @@ -223,7 +242,7 @@ MapController.prototype.create = function(req, res, prepareConfigFn, next) { ); }; -MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn, next) { +MapController.prototype.instantiateTemplate = function(req, res, next) { var self = this; var cdbuser = res.locals.user; @@ -231,11 +250,9 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn var mapConfigProvider; var mapConfig; step( - function getTemplateParams() { - prepareParamsFn(this); - }, - function getTemplate(err, templateParams) { - assert.ifError(err); + function getTemplate() { + const templateParams = req.body; + mapConfigProvider = new NamedMapMapConfigProvider( self.templateMaps, self.pgConnection, From aeb9585708fd60eef65d03e9744b0324062f747c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 1 Nov 2017 19:02:07 +0100 Subject: [PATCH 23/47] extract prepare mapconfig and get template to their respective middlewares --- lib/cartodb/controllers/map.js | 141 ++++++++++++++++++++------------- 1 file changed, 85 insertions(+), 56 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 4566cdb3..96844ecf 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -57,6 +57,7 @@ MapController.prototype.register = function(app) { userMiddleware, this.prepareContext, createGetPrepareConfig, + this.prepareAdapterMapConfig.bind(this), this.createGet.bind(this), mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -69,6 +70,7 @@ MapController.prototype.register = function(app) { userMiddleware, this.prepareContext, createPostPrepareConfig, + this.prepareAdapterMapConfig.bind(this), this.createPost.bind(this), mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -81,6 +83,7 @@ MapController.prototype.register = function(app) { userMiddleware, this.prepareContext, prepareJsonTemplateParams, + this.getTemplate.bind(this), this.jsonp.bind(this), mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -92,6 +95,7 @@ MapController.prototype.register = function(app) { userMiddleware, this.prepareContext, prepareTemplateParams, + this.getTemplate.bind(this), this.instantiate.bind(this), mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -173,47 +177,61 @@ MapController.prototype.jsonp = function(req, res, next) { this.instantiateTemplate(req, res, next); }; +MapController.prototype.prepareAdapterMapConfig = function (req, res, next) { + const requestMapConfig = req.body; + const { user, dbhost, dbport, dbname, dbuser, dbpassword, api_key } = res.locals; + + const context = { + analysisConfiguration: { + user, + db: { + host: dbhost, + port: dbport, + dbname: dbname, + user: dbuser, + pass: dbpassword + }, + batch: { + username: user, + apiKey: api_key + } + } + }; + + this.mapConfigAdapter.getMapConfig(user, requestMapConfig, res.locals, context, (err, requestMapConfig) => { + if (err) { + return next(err); + } + + req.body = requestMapConfig; + res.locals.context = context; + + next(); + }); +}; + MapController.prototype.create = function(req, res, next) { var self = this; - var mapConfig; - - var context = {}; - step( - function prepareAdapterMapConfig() { + function createLayergroup() { const requestMapConfig = req.body; + const { context } = res.locals; - context.analysisConfiguration = { - user: res.locals.user, - db: { - host: res.locals.dbhost, - port: res.locals.dbport, - dbname: res.locals.dbname, - user: res.locals.dbuser, - pass: res.locals.dbpassword - }, - batch: { - username: res.locals.user, - apiKey: res.locals.api_key - } - }; - self.mapConfigAdapter.getMapConfig(res.locals.user, requestMapConfig, res.locals, context, this); - }, - function createLayergroup(err, requestMapConfig) { - assert.ifError(err); - var datasource = context.datasource || Datasource.EmptyDatasource(); - res.locals.mapconfig = mapConfig = new MapConfig(requestMapConfig, datasource); + const datasource = context.datasource || Datasource.EmptyDatasource(); + const mapconfig = res.locals.mapconfig = new MapConfig(requestMapConfig, datasource); self.mapBackend.createLayergroup( - mapConfig, + mapconfig, res.locals, - new CreateLayergroupMapConfigProvider(mapConfig, res.locals.user, self.userLimitsApi, res.locals), + new CreateLayergroupMapConfigProvider(mapconfig, res.locals.user, self.userLimitsApi, res.locals), this ); }, function afterLayergroupCreate(err, layergroup) { assert.ifError(err); + const { context } = res.locals; + res.locals.analysesResults = context.analysesResults; res.locals.layergroup = layergroup; res.locals.context = context; @@ -242,48 +260,59 @@ MapController.prototype.create = function(req, res, next) { ); }; +MapController.prototype.getTemplate = function(req, res, next) { + const templateParams = req.body; + const { user } = res.locals; + + const mapConfigProvider = res.locals.mapconfigProvider = new NamedMapMapConfigProvider( + this.templateMaps, + this.pgConnection, + this.metadataBackend, + this.userLimitsApi, + this.mapConfigAdapter, + user, + req.params.template_id, + templateParams, + res.locals.auth_token, + res.locals + ); + + mapConfigProvider.getMapConfig((err, mapconfig, rendererParams) => { + if (err) { + return next(err); + } + + res.locals.mapconfig = mapconfig; + res.locals.rendererParams = rendererParams; + + next(); + }); +}; + MapController.prototype.instantiateTemplate = function(req, res, next) { var self = this; - var cdbuser = res.locals.user; - - var mapConfigProvider; - var mapConfig; step( - function getTemplate() { - const templateParams = req.body; + function createLayergroup() { + const { user, mapconfig, rendererParams } = res.locals; - mapConfigProvider = new NamedMapMapConfigProvider( - self.templateMaps, - self.pgConnection, - self.metadataBackend, - self.userLimitsApi, - self.mapConfigAdapter, - cdbuser, - req.params.template_id, - templateParams, - res.locals.auth_token, - res.locals - ); - mapConfigProvider.getMapConfig(this); - }, - function createLayergroup(err, mapConfig_, rendererParams) { - assert.ifError(err); - res.locals.mapconfig = mapConfig = mapConfig_; self.mapBackend.createLayergroup( - mapConfig, rendererParams, - new CreateLayergroupMapConfigProvider(mapConfig, cdbuser, self.userLimitsApi, rendererParams), + mapconfig, rendererParams, + new CreateLayergroupMapConfigProvider(mapconfig, user, self.userLimitsApi, rendererParams), this ); }, function afterLayergroupCreate(err, layergroup) { assert.ifError(err); - res.locals.analysesResults = mapConfigProvider.analysesResults; res.locals.layergroup = layergroup; - res.locals.template = mapConfigProvider.template; - res.locals.templateName = mapConfigProvider.getTemplateName(); - res.locals.context = mapConfigProvider.context; + + const { mapconfigProvider } = res.locals; + + res.locals.analysesResults = mapconfigProvider.analysesResults; + res.locals.template = mapconfigProvider.template; + res.locals.templateName = mapconfigProvider.getTemplateName(); + res.locals.context = mapconfigProvider.context; const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ useTemplateHash: true From 125587522f73f8a4ec331d497450c467031266d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 1 Nov 2017 19:27:01 +0100 Subject: [PATCH 24/47] Create middleware for layergroup creation --- lib/cartodb/controllers/map.js | 52 +++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 96844ecf..342fdeb2 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -84,6 +84,7 @@ MapController.prototype.register = function(app) { this.prepareContext, prepareJsonTemplateParams, this.getTemplate.bind(this), + this.createLayergroupFromTemplate.bind(this), this.jsonp.bind(this), mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -96,6 +97,7 @@ MapController.prototype.register = function(app) { this.prepareContext, prepareTemplateParams, this.getTemplate.bind(this), + this.createLayergroupFromTemplate.bind(this), this.instantiate.bind(this), mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -264,7 +266,7 @@ MapController.prototype.getTemplate = function(req, res, next) { const templateParams = req.body; const { user } = res.locals; - const mapConfigProvider = res.locals.mapconfigProvider = new NamedMapMapConfigProvider( + const mapconfigProvider = new NamedMapMapConfigProvider( this.templateMaps, this.pgConnection, this.metadataBackend, @@ -277,13 +279,37 @@ MapController.prototype.getTemplate = function(req, res, next) { res.locals ); - mapConfigProvider.getMapConfig((err, mapconfig, rendererParams) => { + mapconfigProvider.getMapConfig((err, mapconfig, rendererParams) => { if (err) { return next(err); } res.locals.mapconfig = mapconfig; res.locals.rendererParams = rendererParams; + res.locals.mapconfigProvider = mapconfigProvider; + + next(); + }); +}; + +MapController.prototype.createLayergroupFromTemplate = function(req, res, next) { + const { user, mapconfig, rendererParams } = res.locals; + const mapconfigProvider = new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, rendererParams); + + this.mapBackend.createLayergroup(mapconfig, rendererParams, mapconfigProvider, (err, layergroup) => { + if (err) { + return next(err); + } + + res.locals.layergroup = layergroup; + + // TODO: Do not provide shortcuts + const { mapconfigProvider } = res.locals; + + res.locals.analysesResults = mapconfigProvider.analysesResults; + res.locals.template = mapconfigProvider.template; + res.locals.templateName = mapconfigProvider.getTemplateName(); + res.locals.context = mapconfigProvider.context; next(); }); @@ -293,27 +319,7 @@ MapController.prototype.instantiateTemplate = function(req, res, next) { var self = this; step( - function createLayergroup() { - const { user, mapconfig, rendererParams } = res.locals; - - self.mapBackend.createLayergroup( - mapconfig, rendererParams, - new CreateLayergroupMapConfigProvider(mapconfig, user, self.userLimitsApi, rendererParams), - this - ); - }, - function afterLayergroupCreate(err, layergroup) { - assert.ifError(err); - - res.locals.layergroup = layergroup; - - const { mapconfigProvider } = res.locals; - - res.locals.analysesResults = mapconfigProvider.analysesResults; - res.locals.template = mapconfigProvider.template; - res.locals.templateName = mapconfigProvider.getTemplateName(); - res.locals.context = mapconfigProvider.context; - + function afterLayergroupCreate() { const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ useTemplateHash: true }); From e6bec5ccb025024e6c1d5f65fe863e8d086805ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 1 Nov 2017 19:28:32 +0100 Subject: [PATCH 25/47] Make style linter happy --- lib/cartodb/controllers/map.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 342fdeb2..4bd4457b 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -294,7 +294,8 @@ MapController.prototype.getTemplate = function(req, res, next) { MapController.prototype.createLayergroupFromTemplate = function(req, res, next) { const { user, mapconfig, rendererParams } = res.locals; - const mapconfigProvider = new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, rendererParams); + const mapconfigProvider = + new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, rendererParams); this.mapBackend.createLayergroup(mapconfig, rendererParams, mapconfigProvider, (err, layergroup) => { if (err) { From 46c76d6a4c1ffde26c85243453bb5c1b278674f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 1 Nov 2017 19:57:20 +0100 Subject: [PATCH 26/47] Create middleware for layergroup creation (anonymous map) --- lib/cartodb/controllers/map.js | 46 ++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 4bd4457b..2d65b801 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -58,6 +58,7 @@ MapController.prototype.register = function(app) { this.prepareContext, createGetPrepareConfig, this.prepareAdapterMapConfig.bind(this), + this.createLayergroup.bind(this), this.createGet.bind(this), mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -71,6 +72,7 @@ MapController.prototype.register = function(app) { this.prepareContext, createPostPrepareConfig, this.prepareAdapterMapConfig.bind(this), + this.createLayergroup.bind(this), this.createPost.bind(this), mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -212,32 +214,32 @@ MapController.prototype.prepareAdapterMapConfig = function (req, res, next) { }); }; +MapController.prototype.createLayergroup = function(req, res, next) { + const requestMapConfig = req.body; + const { context, user } = res.locals; + const datasource = context.datasource || Datasource.EmptyDatasource(); + const mapconfig = new MapConfig(requestMapConfig, datasource); + const mapconfigProvider = new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, res.locals); + + res.locals.mapconfig = mapconfig; + res.locals.analysesResults = context.analysesResults; + + this.mapBackend.createLayergroup(mapconfig, res.locals, mapconfigProvider, (err, layergroup) => { + if (err) { + return next(err); + } + + res.locals.layergroup = layergroup; + + next(); + }); +}; + MapController.prototype.create = function(req, res, next) { var self = this; step( - function createLayergroup() { - const requestMapConfig = req.body; - const { context } = res.locals; - - const datasource = context.datasource || Datasource.EmptyDatasource(); - const mapconfig = res.locals.mapconfig = new MapConfig(requestMapConfig, datasource); - self.mapBackend.createLayergroup( - mapconfig, - res.locals, - new CreateLayergroupMapConfigProvider(mapconfig, res.locals.user, self.userLimitsApi, res.locals), - this - ); - }, - function afterLayergroupCreate(err, layergroup) { - assert.ifError(err); - - const { context } = res.locals; - - res.locals.analysesResults = context.analysesResults; - res.locals.layergroup = layergroup; - res.locals.context = context; - + function afterLayergroupCreate() { const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ includeQuery: true }); From c8000e5cf8a2fa2bd6af71c0fc4a9ba67dc3bcb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 1 Nov 2017 20:06:32 +0100 Subject: [PATCH 27/47] Make a middleware to respond layergroup --- lib/cartodb/controllers/map.js | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 2d65b801..901cc886 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -60,6 +60,7 @@ MapController.prototype.register = function(app) { this.prepareAdapterMapConfig.bind(this), this.createLayergroup.bind(this), this.createGet.bind(this), + respond, mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', augmentError: true @@ -74,6 +75,7 @@ MapController.prototype.register = function(app) { this.prepareAdapterMapConfig.bind(this), this.createLayergroup.bind(this), this.createPost.bind(this), + respond, mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', augmentError: true @@ -88,6 +90,7 @@ MapController.prototype.register = function(app) { this.getTemplate.bind(this), this.createLayergroupFromTemplate.bind(this), this.jsonp.bind(this), + respond, mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' }) @@ -101,6 +104,7 @@ MapController.prototype.register = function(app) { this.getTemplate.bind(this), this.createLayergroupFromTemplate.bind(this), this.instantiate.bind(this), + respond, mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' }) @@ -250,16 +254,7 @@ MapController.prototype.create = function(req, res, next) { if (err) { return next(err); } - - const { layergroup } = res.locals; - - res.status(200); - - if (req.query && req.query.callback) { - res.jsonp(layergroup); - } else { - res.json(layergroup); - } + next(); } ); }; @@ -333,20 +328,23 @@ MapController.prototype.instantiateTemplate = function(req, res, next) { if (err) { return next(err); } - - const { layergroup } = res.locals; - - res.status(200); - - if (req.query && req.query.callback) { - res.jsonp(layergroup); - } else { - res.json(layergroup); - } + next(); } ); }; +function respond (req, res) { + const { layergroup } = res.locals; + + res.status(200); + + if (req.query && req.query.callback) { + res.jsonp(layergroup); + } else { + res.json(layergroup); + } +} + MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { const self = this; const { From eb5bf52bd9be1f5b97aca748f574cb10462cf2dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 10:22:30 +0100 Subject: [PATCH 28/47] Move profiler start to the right place --- lib/cartodb/controllers/map.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 901cc886..a50bc72e 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -113,6 +113,8 @@ MapController.prototype.register = function(app) { }; function createGetPrepareConfig (req, res, next) { + req.profiler.start('windshaft.createmap_get'); + const { config } = res.locals; if (!config) { @@ -129,6 +131,8 @@ function createGetPrepareConfig (req, res, next) { } function createPostPrepareConfig(req, res, next) { + req.profiler.start('windshaft.createmap_post'); + if (!req.is('application/json')) { return next(new Error('layergroup POST data must be of type application/json')); } @@ -137,6 +141,8 @@ function createPostPrepareConfig(req, res, next) { } function prepareTemplateParams(req, res, next) { + req.profiler.start('windshaft-cartodb.instance_template_post'); + if (!req.is('application/json')) { return next(new Error('Template POST data must be of type application/json')); } @@ -145,6 +151,8 @@ function prepareTemplateParams(req, res, next) { } function prepareJsonTemplateParams(req, res, next) { + req.profiler.start('windshaft-cartodb.instance_template_get'); + const { callback, config } = req.query; if (callback === undefined || callback.length === 0) { @@ -163,25 +171,18 @@ function prepareJsonTemplateParams(req, res, next) { } MapController.prototype.createGet = function(req, res, next){ - req.profiler.start('windshaft.createmap_get'); - this.create(req, res, next); }; MapController.prototype.createPost = function(req, res, next) { - req.profiler.start('windshaft.createmap_post'); - this.create(req, res, next); }; MapController.prototype.instantiate = function(req, res, next) { - req.profiler.start('windshaft-cartodb.instance_template_post'); - this.instantiateTemplate(req, res, next); }; MapController.prototype.jsonp = function(req, res, next) { - req.profiler.start('windshaft-cartodb.instance_template_get'); this.instantiateTemplate(req, res, next); }; From d2b5eaa8c334923c6bd0442573b94790ae145eb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 10:28:33 +0100 Subject: [PATCH 29/47] Do not proxy create and intantiate middlewares --- lib/cartodb/controllers/map.js | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index a50bc72e..72d597a1 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -59,7 +59,7 @@ MapController.prototype.register = function(app) { createGetPrepareConfig, this.prepareAdapterMapConfig.bind(this), this.createLayergroup.bind(this), - this.createGet.bind(this), + this.create.bind(this), respond, mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -74,7 +74,7 @@ MapController.prototype.register = function(app) { createPostPrepareConfig, this.prepareAdapterMapConfig.bind(this), this.createLayergroup.bind(this), - this.createPost.bind(this), + this.create.bind(this), respond, mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -89,7 +89,7 @@ MapController.prototype.register = function(app) { prepareJsonTemplateParams, this.getTemplate.bind(this), this.createLayergroupFromTemplate.bind(this), - this.jsonp.bind(this), + this.instantiateTemplate.bind(this), respond, mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -103,7 +103,7 @@ MapController.prototype.register = function(app) { prepareTemplateParams, this.getTemplate.bind(this), this.createLayergroupFromTemplate.bind(this), - this.instantiate.bind(this), + this.instantiateTemplate.bind(this), respond, mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -170,22 +170,6 @@ function prepareJsonTemplateParams(req, res, next) { return next(); } -MapController.prototype.createGet = function(req, res, next){ - this.create(req, res, next); -}; - -MapController.prototype.createPost = function(req, res, next) { - this.create(req, res, next); -}; - -MapController.prototype.instantiate = function(req, res, next) { - this.instantiateTemplate(req, res, next); -}; - -MapController.prototype.jsonp = function(req, res, next) { - this.instantiateTemplate(req, res, next); -}; - MapController.prototype.prepareAdapterMapConfig = function (req, res, next) { const requestMapConfig = req.body; const { user, dbhost, dbport, dbname, dbuser, dbpassword, api_key } = res.locals; From 658763da8c7d7d246cd775b1d7404e43940c1ea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 10:33:39 +0100 Subject: [PATCH 30/47] Build after layergroup create while registering routes --- lib/cartodb/controllers/map.js | 56 ++++++++-------------------------- 1 file changed, 12 insertions(+), 44 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 72d597a1..384678cb 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -59,7 +59,9 @@ MapController.prototype.register = function(app) { createGetPrepareConfig, this.prepareAdapterMapConfig.bind(this), this.createLayergroup.bind(this), - this.create.bind(this), + this.afterLayergroupCreateBuilder({ + includeQuery: true + }), respond, mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -74,7 +76,9 @@ MapController.prototype.register = function(app) { createPostPrepareConfig, this.prepareAdapterMapConfig.bind(this), this.createLayergroup.bind(this), - this.create.bind(this), + this.afterLayergroupCreateBuilder({ + includeQuery: true + }), respond, mapErrorMiddleware({ label: 'ANONYMOUS LAYERGROUP', @@ -89,7 +93,9 @@ MapController.prototype.register = function(app) { prepareJsonTemplateParams, this.getTemplate.bind(this), this.createLayergroupFromTemplate.bind(this), - this.instantiateTemplate.bind(this), + this.afterLayergroupCreateBuilder({ + useTemplateHash: true + }), respond, mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -103,7 +109,9 @@ MapController.prototype.register = function(app) { prepareTemplateParams, this.getTemplate.bind(this), this.createLayergroupFromTemplate.bind(this), - this.instantiateTemplate.bind(this), + this.afterLayergroupCreateBuilder({ + useTemplateHash: true + }), respond, mapErrorMiddleware({ label: 'NAMED MAP LAYERGROUP' @@ -224,26 +232,6 @@ MapController.prototype.createLayergroup = function(req, res, next) { }); }; -MapController.prototype.create = function(req, res, next) { - var self = this; - - step( - function afterLayergroupCreate() { - const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ - includeQuery: true - }); - - afterLayergroupCreate(req, res, this); - }, - function finish(err) { - if (err) { - return next(err); - } - next(); - } - ); -}; - MapController.prototype.getTemplate = function(req, res, next) { const templateParams = req.body; const { user } = res.locals; @@ -298,26 +286,6 @@ MapController.prototype.createLayergroupFromTemplate = function(req, res, next) }); }; -MapController.prototype.instantiateTemplate = function(req, res, next) { - var self = this; - - step( - function afterLayergroupCreate() { - const afterLayergroupCreate = self.afterLayergroupCreateBuilder({ - useTemplateHash: true - }); - - afterLayergroupCreate(req, res, this); - }, - function finishTemplateInstantiation(err) { - if (err) { - return next(err); - } - next(); - } - ); -}; - function respond (req, res) { const { layergroup } = res.locals; From 93bd2c9e509e69db8aa1f5b591057e6afa730aa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 10:43:22 +0100 Subject: [PATCH 31/47] Build afterLayergroupCreate middleware as an array of middlewares instead of preforming all them as one middleware --- lib/cartodb/controllers/map.js | 75 +++++++--------------------------- 1 file changed, 14 insertions(+), 61 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 384678cb..7478ce49 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -299,72 +299,25 @@ function respond (req, res) { } MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { - const self = this; const { useTemplateHash = false, includeQuery = false } = options; - return function (req, res, callback) { - step( - function incrementMapViewCount () { - self.incrementMapViewCount(req, res, this); - }, - function augmentLayergroupData (err) { - assert.ifError(err); - self.augmentLayergroupData(req, res, this); - }, - function getAffectedTables (err) { - assert.ifError(err); - self.getAffectedTables(req, res, this); - }, - function setCacheChannel (err) { - assert.ifError(err); - self.setCacheChannel(req, res, this); - }, - function setLastUpdatedTime (err) { - assert.ifError(err); - self.setLastUpdatedTimeToLayergroup(req, res, this); - }, - function setCacheControl (err) { - assert.ifError(err); - self.setCacheControl(req, res, this); - }, - function setLayerStats (err) { - assert.ifError(err); - self.setLayerStats(req, res, this); - }, - function setLayergroupIdHeader (err) { - assert.ifError(err); - - const setLayergroupIdHeader = self.setLayergroupIdHeaderBuilder(useTemplateHash); - - setLayergroupIdHeader(req, res, this); - }, - function (err) { - assert.ifError(err); - self.setDataviewsAndWidgetsUrlsToLayergroupMetadata(req, res, this); - }, - function setAnalysesMetadataToLayergroup(err) { - assert.ifError(err); - - const setAnalysesMetadataToLayergroup = self.setAnalysesMetadataToLayergroupBuilder(includeQuery); - - setAnalysesMetadataToLayergroup(req, res, this); - }, - function setTurboCartoMetadataToLayergroup (err) { - assert.ifError(err); - self.setTurboCartoMetadataToLayergroup(req, res, this); - }, - function setSurrogateKeyHeader (err) { - assert.ifError(err); - self.setSurrogateKeyHeader(req, res, this); - }, - function finish(err) { - callback(err); - } - ); - }; + return [ + this.incrementMapViewCount.bind(this), + this.augmentLayergroupData.bind(this), + this.getAffectedTables.bind(this), + this.setCacheChannel.bind(this), + this.setLastUpdatedTimeToLayergroup.bind(this), + this.setCacheControl.bind(this), + this.setLayerStats.bind(this), + this.setLayergroupIdHeaderBuilder(useTemplateHash), + this.setDataviewsAndWidgetsUrlsToLayergroupMetadata.bind(this), + this.setAnalysesMetadataToLayergroupBuilder(includeQuery), + this.setTurboCartoMetadataToLayergroup.bind(this), + this.setSurrogateKeyHeader.bind(this) + ]; }; MapController.prototype.incrementMapViewCount = function (req, res, callback) { From b11b872b755ce4204befbd7da1ac593ceaa808a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 11:29:43 +0100 Subject: [PATCH 32/47] Remove step requirement --- lib/cartodb/controllers/map.js | 46 +++++++++++++++------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 7478ce49..372dd567 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -1,6 +1,4 @@ var _ = require('underscore'); -var assert = require('assert'); -var step = require('step'); var windshaft = require('windshaft'); var QueryTables = require('cartodb-query-tables'); @@ -346,41 +344,37 @@ MapController.prototype.augmentLayergroupData = function (req, res, callback) { callback(); }; -MapController.prototype.getAffectedTables = function (req, res, callback) { - const self = this; +MapController.prototype.getAffectedTables = function (req, res, next) { const { dbname, layergroup, user, mapconfig } = res.locals; - var sql = []; - mapconfig.getLayers().forEach(function(layer) { - sql.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(function(table) { - sql.push('SELECT * FROM ' + table + ' LIMIT 0'); - }); + this.pgConnection.getConnection(user, (err, connection) => { + if (err) { + return next(err); } - }); - step( - function getPgConnection() { - self.pgConnection.getConnection(user, this); - }, - function getAffectedTablesAndLastUpdatedTime(err, connection) { - assert.ifError(err); - QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), this); - }, - function handleAffectedTablesAndLastUpdatedTime(err, affectedTables) { + const sql = []; + mapconfig.getLayers().forEach(function(layer) { + sql.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(function(table) { + sql.push('SELECT * FROM ' + table + ' LIMIT 0'); + }); + } + }); + + QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), (err, affectedTables) => { if (err) { - return callback(err); + return next(err); } // feed affected tables cache so it can be reused from, for instance, layergroup controller - self.layergroupAffectedTables.set(dbname, layergroup.layergroupId, affectedTables); + this.layergroupAffectedTables.set(dbname, layergroup.layergroupId, affectedTables); res.locals.affectedTables = affectedTables; - callback(); - } - ); + next(); + }); + }); }; MapController.prototype.setCacheChannel = function (req, res, callback) { From 1d08734721c3125559103fe702d665735be23273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 18:28:37 +0100 Subject: [PATCH 33/47] Rename middleware --- lib/cartodb/controllers/map.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 372dd567..4d98b2b7 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -90,7 +90,7 @@ MapController.prototype.register = function(app) { this.prepareContext, prepareJsonTemplateParams, this.getTemplate.bind(this), - this.createLayergroupFromTemplate.bind(this), + this.instantiateLayergroup.bind(this), this.afterLayergroupCreateBuilder({ useTemplateHash: true }), @@ -106,7 +106,7 @@ MapController.prototype.register = function(app) { this.prepareContext, prepareTemplateParams, this.getTemplate.bind(this), - this.createLayergroupFromTemplate.bind(this), + this.instantiateLayergroup.bind(this), this.afterLayergroupCreateBuilder({ useTemplateHash: true }), @@ -260,7 +260,7 @@ MapController.prototype.getTemplate = function(req, res, next) { }); }; -MapController.prototype.createLayergroupFromTemplate = function(req, res, next) { +MapController.prototype.instantiateLayergroup = function(req, res, next) { const { user, mapconfig, rendererParams } = res.locals; const mapconfigProvider = new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, rendererParams); From 08b91f935d52a9eb9501339f35b44a6ec6825647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 18:38:34 +0100 Subject: [PATCH 34/47] Rename error middleware --- lib/cartodb/controllers/map.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 4d98b2b7..a9f2f327 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -61,9 +61,9 @@ MapController.prototype.register = function(app) { includeQuery: true }), respond, - mapErrorMiddleware({ + augmentError({ label: 'ANONYMOUS LAYERGROUP', - augmentError: true + addContext: true }) ); app.post( @@ -78,9 +78,9 @@ MapController.prototype.register = function(app) { includeQuery: true }), respond, - mapErrorMiddleware({ + augmentError({ label: 'ANONYMOUS LAYERGROUP', - augmentError: true + addContext: true }) ); app.get( @@ -95,7 +95,7 @@ MapController.prototype.register = function(app) { useTemplateHash: true }), respond, - mapErrorMiddleware({ + augmentError({ label: 'NAMED MAP LAYERGROUP' }) ); @@ -111,7 +111,7 @@ MapController.prototype.register = function(app) { useTemplateHash: true }), respond, - mapErrorMiddleware({ + augmentError({ label: 'NAMED MAP LAYERGROUP' }) ); @@ -584,8 +584,8 @@ MapController.prototype.addWidgetsUrl = function(username, layergroup, mapConfig } }; -function mapErrorMiddleware (options) { - const { augmentError = false, label = 'MAPS CONTROLLER' } = options; +function augmentError (options) { + const { addContext = false, label = 'MAPS CONTROLLER' } = options; return function mapError (err, req, res, next) { const { mapconfig } = res.locals; From 3e7106002dff0adb285c2ed8c11ea32107287297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 18:39:46 +0100 Subject: [PATCH 35/47] Rename response middleware --- lib/cartodb/controllers/map.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index a9f2f327..b153b607 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -60,7 +60,7 @@ MapController.prototype.register = function(app) { this.afterLayergroupCreateBuilder({ includeQuery: true }), - respond, + sendResponse, augmentError({ label: 'ANONYMOUS LAYERGROUP', addContext: true @@ -77,7 +77,7 @@ MapController.prototype.register = function(app) { this.afterLayergroupCreateBuilder({ includeQuery: true }), - respond, + sendResponse, augmentError({ label: 'ANONYMOUS LAYERGROUP', addContext: true @@ -94,7 +94,7 @@ MapController.prototype.register = function(app) { this.afterLayergroupCreateBuilder({ useTemplateHash: true }), - respond, + sendResponse, augmentError({ label: 'NAMED MAP LAYERGROUP' }) @@ -110,7 +110,7 @@ MapController.prototype.register = function(app) { this.afterLayergroupCreateBuilder({ useTemplateHash: true }), - respond, + sendResponse, augmentError({ label: 'NAMED MAP LAYERGROUP' }) @@ -284,7 +284,7 @@ MapController.prototype.instantiateLayergroup = function(req, res, next) { }); }; -function respond (req, res) { +function sendResponse (req, res) { const { layergroup } = res.locals; res.status(200); From 4607e4a12deb72c701013c64bc8b6c2748fc90f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 19:03:20 +0100 Subject: [PATCH 36/47] Compose create layergroup middleware --- lib/cartodb/controllers/map.js | 79 +++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index b153b607..362281ca 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -49,40 +49,15 @@ function MapController(prepareContext, pgConnection, templateMaps, mapBackend, m module.exports = MapController; MapController.prototype.register = function(app) { - app.get( - app.base_url_mapconfig, - cors(), - userMiddleware, - this.prepareContext, - createGetPrepareConfig, - this.prepareAdapterMapConfig.bind(this), - this.createLayergroup.bind(this), - this.afterLayergroupCreateBuilder({ - includeQuery: true - }), - sendResponse, - augmentError({ - label: 'ANONYMOUS LAYERGROUP', - addContext: true - }) - ); - app.post( - app.base_url_mapconfig, - cors(), - userMiddleware, - this.prepareContext, - createPostPrepareConfig, - this.prepareAdapterMapConfig.bind(this), - this.createLayergroup.bind(this), - this.afterLayergroupCreateBuilder({ - includeQuery: true - }), - sendResponse, - augmentError({ - label: 'ANONYMOUS LAYERGROUP', - addContext: true - }) - ); + app.get(app.base_url_mapconfig, this.composeCreateLayergroupMiddleware({ + includeQuery: true, + parseConfigQueryParam: true + })); + + app.post(app.base_url_mapconfig, this.composeCreateLayergroupMiddleware({ + includeQuery: true + })); + app.get( app.base_url_templated + '/:template_id/jsonp', cors(), @@ -118,8 +93,42 @@ MapController.prototype.register = function(app) { app.options(app.base_url_mapconfig, cors('Content-Type')); }; +MapController.prototype.composeCreateLayergroupMiddleware = function (options) { + const { + parseConfigQueryParam = false, + useTemplateHash = false, + includeQuery = false + } = options; + + return [ + cors(), + userMiddleware, + this.prepareContext, + parseConfigQueryParam ? createGetPrepareConfig : createPostPrepareConfig, + this.prepareAdapterMapConfig.bind(this), + this.createLayergroup.bind(this), + this.incrementMapViewCount.bind(this), + this.augmentLayergroupData.bind(this), + this.getAffectedTables.bind(this), + this.setCacheChannel.bind(this), + this.setLastUpdatedTimeToLayergroup.bind(this), + this.setCacheControl.bind(this), + this.setLayerStats.bind(this), + this.setLayergroupIdHeaderBuilder(useTemplateHash), + this.setDataviewsAndWidgetsUrlsToLayergroupMetadata.bind(this), + this.setAnalysesMetadataToLayergroupBuilder(includeQuery), + this.setTurboCartoMetadataToLayergroup.bind(this), + this.setSurrogateKeyHeader.bind(this), + sendResponse, + augmentError({ + label: 'ANONYMOUS LAYERGROUP', + addContext: true + }) + ]; +}; + function createGetPrepareConfig (req, res, next) { - req.profiler.start('windshaft.createmap_get'); + req.profiler.start(`windshaft.createmap_${req.method.toLowecase}`); const { config } = res.locals; From 717332d9416214b6dfc4802662c08da92fe9ff23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 2 Nov 2017 19:24:33 +0100 Subject: [PATCH 37/47] Compose instantiate layergroup middleware --- lib/cartodb/controllers/map.js | 95 ++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 40 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 362281ca..9bb23dac 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -50,46 +50,29 @@ module.exports = MapController; MapController.prototype.register = function(app) { app.get(app.base_url_mapconfig, this.composeCreateLayergroupMiddleware({ + parseConfigQueryParam: true, includeQuery: true, - parseConfigQueryParam: true + label: 'ANONYMOUS LAYERGROUP', + addContext: true })); app.post(app.base_url_mapconfig, this.composeCreateLayergroupMiddleware({ - includeQuery: true + includeQuery: true, + label: 'ANONYMOUS LAYERGROUP', + addContext: true + })); + + app.get(app.base_url_templated + '/:template_id/jsonp', this.composeInstantiateLayergroupMiddleware({ + parseConfigQueryParam: true, + useTemplateHash: true, + label: 'NAMED MAP LAYERGROUP' + })); + + app.post(app.base_url_templated + '/:template_id', this.composeInstantiateLayergroupMiddleware({ + useTemplateHash: true, + label: 'NAMED MAP LAYERGROUP' })); - app.get( - app.base_url_templated + '/:template_id/jsonp', - cors(), - userMiddleware, - this.prepareContext, - prepareJsonTemplateParams, - this.getTemplate.bind(this), - this.instantiateLayergroup.bind(this), - this.afterLayergroupCreateBuilder({ - useTemplateHash: true - }), - sendResponse, - augmentError({ - label: 'NAMED MAP LAYERGROUP' - }) - ); - app.post( - app.base_url_templated + '/:template_id', - cors(), - userMiddleware, - this.prepareContext, - prepareTemplateParams, - this.getTemplate.bind(this), - this.instantiateLayergroup.bind(this), - this.afterLayergroupCreateBuilder({ - useTemplateHash: true - }), - sendResponse, - augmentError({ - label: 'NAMED MAP LAYERGROUP' - }) - ); app.options(app.base_url_mapconfig, cors('Content-Type')); }; @@ -97,7 +80,9 @@ MapController.prototype.composeCreateLayergroupMiddleware = function (options) { const { parseConfigQueryParam = false, useTemplateHash = false, - includeQuery = false + includeQuery = false, + label, + addContext = false } = options; return [ @@ -120,15 +105,45 @@ MapController.prototype.composeCreateLayergroupMiddleware = function (options) { this.setTurboCartoMetadataToLayergroup.bind(this), this.setSurrogateKeyHeader.bind(this), sendResponse, - augmentError({ - label: 'ANONYMOUS LAYERGROUP', - addContext: true - }) + augmentError({ label, addContext }) + ]; +}; + +MapController.prototype.composeInstantiateLayergroupMiddleware = function (options) { + const { + parseConfigQueryParam = false, + useTemplateHash = false, + includeQuery = false, + label, + addContext = false + } = options; + + return [ + cors(), + userMiddleware, + this.prepareContext, + parseConfigQueryParam ? prepareJsonTemplateParams : prepareTemplateParams, + this.getTemplate.bind(this), + this.instantiateLayergroup.bind(this), + this.incrementMapViewCount.bind(this), + this.augmentLayergroupData.bind(this), + this.getAffectedTables.bind(this), + this.setCacheChannel.bind(this), + this.setLastUpdatedTimeToLayergroup.bind(this), + this.setCacheControl.bind(this), + this.setLayerStats.bind(this), + this.setLayergroupIdHeaderBuilder(useTemplateHash), + this.setDataviewsAndWidgetsUrlsToLayergroupMetadata.bind(this), + this.setAnalysesMetadataToLayergroupBuilder(includeQuery), + this.setTurboCartoMetadataToLayergroup.bind(this), + this.setSurrogateKeyHeader.bind(this), + sendResponse, + augmentError({ label, addContext }) ]; }; function createGetPrepareConfig (req, res, next) { - req.profiler.start(`windshaft.createmap_${req.method.toLowecase}`); + req.profiler.start(`windshaft.createmap_get`); const { config } = res.locals; From 2854d0252cb780f8fe4c2109f065467ec37317f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 3 Nov 2017 08:48:13 +0100 Subject: [PATCH 38/47] Fix typo --- lib/cartodb/controllers/map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 9bb23dac..96a66e37 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -614,7 +614,7 @@ function augmentError (options) { return function mapError (err, req, res, next) { const { mapconfig } = res.locals; - if (augmentError) { + if (addContext) { err = Number.isFinite(err.layerIndex) ? populateError(err, mapconfig) : err; } From cb167313d22dca88042fdbe466f920305ad2578e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 3 Nov 2017 09:37:01 +0100 Subject: [PATCH 39/47] Unify middleware builder functions --- lib/cartodb/controllers/map.js | 149 +++++++++++---------------------- 1 file changed, 47 insertions(+), 102 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 96a66e37..ab65b604 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -49,49 +49,34 @@ function MapController(prepareContext, pgConnection, templateMaps, mapBackend, m module.exports = MapController; MapController.prototype.register = function(app) { - app.get(app.base_url_mapconfig, this.composeCreateLayergroupMiddleware({ - parseConfigQueryParam: true, - includeQuery: true, - label: 'ANONYMOUS LAYERGROUP', - addContext: true + app.get(app.base_url_mapconfig, this.composeCreateMapMiddleware()); + + app.post(app.base_url_mapconfig, this.composeCreateMapMiddleware()); + + app.get(app.base_url_templated + '/:template_id/jsonp', this.composeCreateMapMiddleware({ + useTemplate: true })); - app.post(app.base_url_mapconfig, this.composeCreateLayergroupMiddleware({ - includeQuery: true, - label: 'ANONYMOUS LAYERGROUP', - addContext: true - })); - - app.get(app.base_url_templated + '/:template_id/jsonp', this.composeInstantiateLayergroupMiddleware({ - parseConfigQueryParam: true, - useTemplateHash: true, - label: 'NAMED MAP LAYERGROUP' - })); - - app.post(app.base_url_templated + '/:template_id', this.composeInstantiateLayergroupMiddleware({ - useTemplateHash: true, - label: 'NAMED MAP LAYERGROUP' + app.post(app.base_url_templated + '/:template_id', this.composeCreateMapMiddleware({ + useTemplate: true })); app.options(app.base_url_mapconfig, cors('Content-Type')); }; -MapController.prototype.composeCreateLayergroupMiddleware = function (options) { - const { - parseConfigQueryParam = false, - useTemplateHash = false, - includeQuery = false, - label, - addContext = false - } = options; +MapController.prototype.composeCreateMapMiddleware = function ({ useTemplate = false } = {}) { + const useTemplateHash = useTemplate; + const includeQuery = !useTemplate; + const label = useTemplate ? 'NAMED MAP LAYERGROUP' : 'ANONYMOUS LAYERGROUP'; + const addContext = !useTemplate; return [ cors(), userMiddleware, this.prepareContext, - parseConfigQueryParam ? createGetPrepareConfig : createPostPrepareConfig, - this.prepareAdapterMapConfig.bind(this), - this.createLayergroup.bind(this), + useTemplate ? checkIntantiteLayergroup : checkCreateLayergroup, + useTemplate ? this.getTemplate.bind(this) : this.prepareAdapterMapConfig.bind(this), + useTemplate ? this.instantiateLayergroup.bind(this) : this.createLayergroup.bind(this), this.incrementMapViewCount.bind(this), this.augmentLayergroupData.bind(this), this.getAffectedTables.bind(this), @@ -109,91 +94,51 @@ MapController.prototype.composeCreateLayergroupMiddleware = function (options) { ]; }; -MapController.prototype.composeInstantiateLayergroupMiddleware = function (options) { - const { - parseConfigQueryParam = false, - useTemplateHash = false, - includeQuery = false, - label, - addContext = false - } = options; +function checkCreateLayergroup (req, res, next) { + req.profiler.start(`windshaft.createmap_${req.method.toLowerCase()}`); - return [ - cors(), - userMiddleware, - this.prepareContext, - parseConfigQueryParam ? prepareJsonTemplateParams : prepareTemplateParams, - this.getTemplate.bind(this), - this.instantiateLayergroup.bind(this), - this.incrementMapViewCount.bind(this), - this.augmentLayergroupData.bind(this), - this.getAffectedTables.bind(this), - this.setCacheChannel.bind(this), - this.setLastUpdatedTimeToLayergroup.bind(this), - this.setCacheControl.bind(this), - this.setLayerStats.bind(this), - this.setLayergroupIdHeaderBuilder(useTemplateHash), - this.setDataviewsAndWidgetsUrlsToLayergroupMetadata.bind(this), - this.setAnalysesMetadataToLayergroupBuilder(includeQuery), - this.setTurboCartoMetadataToLayergroup.bind(this), - this.setSurrogateKeyHeader.bind(this), - sendResponse, - augmentError({ label, addContext }) - ]; -}; - -function createGetPrepareConfig (req, res, next) { - req.profiler.start(`windshaft.createmap_get`); - - const { config } = res.locals; - - if (!config) { - return next(new Error('layergroup GET needs a "config" parameter')); - } - - try { - req.body = JSON.parse(config); - } catch (err) { - return next(err); - } - - return next(); -} - -function createPostPrepareConfig(req, res, next) { - req.profiler.start('windshaft.createmap_post'); - - if (!req.is('application/json')) { + if (req.method === 'POST' && !req.is('application/json')) { return next(new Error('layergroup POST data must be of type application/json')); } - next(); -} + if (req.method === 'GET') { + const { config } = res.locals; -function prepareTemplateParams(req, res, next) { - req.profiler.start('windshaft-cartodb.instance_template_post'); + if (!config) { + return next(new Error('layergroup GET needs a "config" parameter')); + } - if (!req.is('application/json')) { - return next(new Error('Template POST data must be of type application/json')); + try { + req.body = JSON.parse(config); + } catch (err) { + return next(err); + } } return next(); } -function prepareJsonTemplateParams(req, res, next) { - req.profiler.start('windshaft-cartodb.instance_template_get'); +function checkIntantiteLayergroup(req, res, next) { + // jshint maxcomplexity: 7 + req.profiler.start(`windshaft-cartodb.instance_template_${req.method.toLowerCase()}`); - const { callback, config } = req.query; - - if (callback === undefined || callback.length === 0) { - return next(new Error('callback parameter should be present and be a function name')); + if (req.method === 'POST' && !req.is('application/json')) { + return next(new Error('Template POST data must be of type application/json')); } - if (config) { - try { - req.body = JSON.parse(config); - } catch(e) { - return next(new Error('Invalid config parameter, should be a valid JSON')); + if (req.method === 'GET') { + const { callback, config } = req.query; + + if (callback === undefined || callback.length === 0) { + return next(new Error('callback parameter should be present and be a function name')); + } + + if (config) { + try { + req.body = JSON.parse(config); + } catch(e) { + return next(new Error('Invalid config parameter, should be a valid JSON')); + } } } From 677f6caab80b9e8fc1b3504f6803f61602d73691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 3 Nov 2017 09:38:36 +0100 Subject: [PATCH 40/47] remove funtion --- lib/cartodb/controllers/map.js | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index ab65b604..7a1dc6be 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -265,28 +265,6 @@ function sendResponse (req, res) { } } -MapController.prototype.afterLayergroupCreateBuilder = function (options = {}) { - const { - useTemplateHash = false, - includeQuery = false - } = options; - - return [ - this.incrementMapViewCount.bind(this), - this.augmentLayergroupData.bind(this), - this.getAffectedTables.bind(this), - this.setCacheChannel.bind(this), - this.setLastUpdatedTimeToLayergroup.bind(this), - this.setCacheControl.bind(this), - this.setLayerStats.bind(this), - this.setLayergroupIdHeaderBuilder(useTemplateHash), - this.setDataviewsAndWidgetsUrlsToLayergroupMetadata.bind(this), - this.setAnalysesMetadataToLayergroupBuilder(includeQuery), - this.setTurboCartoMetadataToLayergroup.bind(this), - this.setSurrogateKeyHeader.bind(this) - ]; -}; - MapController.prototype.incrementMapViewCount = function (req, res, callback) { const { mapconfig, user } = res.locals; From 65e8609fec41586d895050a46e25a67a1cb31a62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 3 Nov 2017 09:47:46 +0100 Subject: [PATCH 41/47] Do not bind context if not needed --- lib/cartodb/controllers/map.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 7a1dc6be..2d26cf5d 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -78,16 +78,16 @@ MapController.prototype.composeCreateMapMiddleware = function ({ useTemplate = f useTemplate ? this.getTemplate.bind(this) : this.prepareAdapterMapConfig.bind(this), useTemplate ? this.instantiateLayergroup.bind(this) : this.createLayergroup.bind(this), this.incrementMapViewCount.bind(this), - this.augmentLayergroupData.bind(this), + this.augmentLayergroupData, this.getAffectedTables.bind(this), - this.setCacheChannel.bind(this), - this.setLastUpdatedTimeToLayergroup.bind(this), - this.setCacheControl.bind(this), + this.setCacheChannel, + this.setLastUpdatedTimeToLayergroup, + this.setCacheControl, this.setLayerStats.bind(this), this.setLayergroupIdHeaderBuilder(useTemplateHash), this.setDataviewsAndWidgetsUrlsToLayergroupMetadata.bind(this), this.setAnalysesMetadataToLayergroupBuilder(includeQuery), - this.setTurboCartoMetadataToLayergroup.bind(this), + this.setTurboCartoMetadataToLayergroup, this.setSurrogateKeyHeader.bind(this), sendResponse, augmentError({ label, addContext }) From 6acb873d959d0f5d9aae3c91147e8fc3cb64e2fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 3 Nov 2017 15:06:15 +0100 Subject: [PATCH 42/47] Enforce all middlewares to follow the same constructor pattern --- lib/cartodb/controllers/map.js | 594 +++++++++++++++++---------------- 1 file changed, 313 insertions(+), 281 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 2d26cf5d..ea443253 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -74,356 +74,386 @@ MapController.prototype.composeCreateMapMiddleware = function ({ useTemplate = f cors(), userMiddleware, this.prepareContext, - useTemplate ? checkIntantiteLayergroup : checkCreateLayergroup, - useTemplate ? this.getTemplate.bind(this) : this.prepareAdapterMapConfig.bind(this), - useTemplate ? this.instantiateLayergroup.bind(this) : this.createLayergroup.bind(this), - this.incrementMapViewCount.bind(this), - this.augmentLayergroupData, - this.getAffectedTables.bind(this), - this.setCacheChannel, - this.setLastUpdatedTimeToLayergroup, - this.setCacheControl, - this.setLayerStats.bind(this), + useTemplate ? checkIntantiteLayergroup() : checkCreateLayergroup(), + useTemplate ? this.getTemplate() : this.prepareAdapterMapConfig(), + useTemplate ? this.instantiateLayergroup() : this.createLayergroup(), + this.incrementMapViewCount(), + this.augmentLayergroupData(), + this.getAffectedTables(), + this.setCacheChannel(), + this.setLastUpdatedTimeToLayergroup(), + this.setCacheControl(), + this.setLayerStats(), this.setLayergroupIdHeaderBuilder(useTemplateHash), - this.setDataviewsAndWidgetsUrlsToLayergroupMetadata.bind(this), - this.setAnalysesMetadataToLayergroupBuilder(includeQuery), - this.setTurboCartoMetadataToLayergroup, - this.setSurrogateKeyHeader.bind(this), - sendResponse, + this.setDataviewsAndWidgetsUrlsToLayergroupMetadata(), + this.setAnalysesMetadataToLayergroup(includeQuery), + this.setTurboCartoMetadataToLayergroup(), + this.setSurrogateKeyHeader(), + sendResponse(), augmentError({ label, addContext }) ]; }; -function checkCreateLayergroup (req, res, next) { - req.profiler.start(`windshaft.createmap_${req.method.toLowerCase()}`); - if (req.method === 'POST' && !req.is('application/json')) { - return next(new Error('layergroup POST data must be of type application/json')); - } +function checkCreateLayergroup () { + return function checkCreateLayergroupMiddleware (req, res, next) { + req.profiler.start(`windshaft.createmap_${req.method.toLowerCase()}`); - if (req.method === 'GET') { - const { config } = res.locals; - - if (!config) { - return next(new Error('layergroup GET needs a "config" parameter')); + if (req.method === 'POST' && !req.is('application/json')) { + return next(new Error('layergroup POST data must be of type application/json')); } - try { - req.body = JSON.parse(config); - } catch (err) { - return next(err); - } - } + if (req.method === 'GET') { + const { config } = res.locals; - return next(); -} + if (!config) { + return next(new Error('layergroup GET needs a "config" parameter')); + } -function checkIntantiteLayergroup(req, res, next) { - // jshint maxcomplexity: 7 - req.profiler.start(`windshaft-cartodb.instance_template_${req.method.toLowerCase()}`); - - if (req.method === 'POST' && !req.is('application/json')) { - return next(new Error('Template POST data must be of type application/json')); - } - - if (req.method === 'GET') { - const { callback, config } = req.query; - - if (callback === undefined || callback.length === 0) { - return next(new Error('callback parameter should be present and be a function name')); - } - - if (config) { try { req.body = JSON.parse(config); - } catch(e) { - return next(new Error('Invalid config parameter, should be a valid JSON')); + } catch (err) { + return next(err); } } - } - return next(); -} - -MapController.prototype.prepareAdapterMapConfig = function (req, res, next) { - const requestMapConfig = req.body; - const { user, dbhost, dbport, dbname, dbuser, dbpassword, api_key } = res.locals; - - const context = { - analysisConfiguration: { - user, - db: { - host: dbhost, - port: dbport, - dbname: dbname, - user: dbuser, - pass: dbpassword - }, - batch: { - username: user, - apiKey: api_key - } - } + return next(); }; - - this.mapConfigAdapter.getMapConfig(user, requestMapConfig, res.locals, context, (err, requestMapConfig) => { - if (err) { - return next(err); - } - - req.body = requestMapConfig; - res.locals.context = context; - - next(); - }); -}; - -MapController.prototype.createLayergroup = function(req, res, next) { - const requestMapConfig = req.body; - const { context, user } = res.locals; - const datasource = context.datasource || Datasource.EmptyDatasource(); - const mapconfig = new MapConfig(requestMapConfig, datasource); - const mapconfigProvider = new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, res.locals); - - res.locals.mapconfig = mapconfig; - res.locals.analysesResults = context.analysesResults; - - this.mapBackend.createLayergroup(mapconfig, res.locals, mapconfigProvider, (err, layergroup) => { - if (err) { - return next(err); - } - - res.locals.layergroup = layergroup; - - next(); - }); -}; - -MapController.prototype.getTemplate = function(req, res, next) { - const templateParams = req.body; - const { user } = res.locals; - - const mapconfigProvider = new NamedMapMapConfigProvider( - this.templateMaps, - this.pgConnection, - this.metadataBackend, - this.userLimitsApi, - this.mapConfigAdapter, - user, - req.params.template_id, - templateParams, - res.locals.auth_token, - res.locals - ); - - mapconfigProvider.getMapConfig((err, mapconfig, rendererParams) => { - if (err) { - return next(err); - } - - res.locals.mapconfig = mapconfig; - res.locals.rendererParams = rendererParams; - res.locals.mapconfigProvider = mapconfigProvider; - - next(); - }); -}; - -MapController.prototype.instantiateLayergroup = function(req, res, next) { - const { user, mapconfig, rendererParams } = res.locals; - const mapconfigProvider = - new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, rendererParams); - - this.mapBackend.createLayergroup(mapconfig, rendererParams, mapconfigProvider, (err, layergroup) => { - if (err) { - return next(err); - } - - res.locals.layergroup = layergroup; - - // TODO: Do not provide shortcuts - const { mapconfigProvider } = res.locals; - - res.locals.analysesResults = mapconfigProvider.analysesResults; - res.locals.template = mapconfigProvider.template; - res.locals.templateName = mapconfigProvider.getTemplateName(); - res.locals.context = mapconfigProvider.context; - - next(); - }); -}; - -function sendResponse (req, res) { - const { layergroup } = res.locals; - - res.status(200); - - if (req.query && req.query.callback) { - res.jsonp(layergroup); - } else { - res.json(layergroup); - } } -MapController.prototype.incrementMapViewCount = function (req, res, callback) { - const { mapconfig, user } = res.locals; +function checkIntantiteLayergroup () { + return function checkIntantiteLayergroupMiddleware(req, res, next) { + // jshint maxcomplexity: 7 + req.profiler.start(`windshaft-cartodb.instance_template_${req.method.toLowerCase()}`); - // Error won't blow up, just be logged. - this.metadataBackend.incMapviewCount(user, mapconfig.obj().stat_tag, (err) => { - req.profiler.done('incMapviewCount'); - - if (err) { - global.logger.log(`ERROR: failed to increment mapview count for user '${user}': ${err.message}`); + if (req.method === 'POST' && !req.is('application/json')) { + return next(new Error('Template POST data must be of type application/json')); } - callback(); - }); -}; + if (req.method === 'GET') { + const { callback, config } = req.query; -MapController.prototype.augmentLayergroupData = function (req, res, callback) { - const { layergroup } = res.locals; - - // include in layergroup response the variables in serverMedata - // those variables are useful to send to the client information - // about how to reach this server or information about it - _.extend(layergroup, global.environment.serverMetadata); - - callback(); -}; - -MapController.prototype.getAffectedTables = function (req, res, next) { - const { dbname, layergroup, user, mapconfig } = res.locals; - - this.pgConnection.getConnection(user, (err, connection) => { - if (err) { - return next(err); - } - - const sql = []; - mapconfig.getLayers().forEach(function(layer) { - sql.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(function(table) { - sql.push('SELECT * FROM ' + table + ' LIMIT 0'); - }); + if (callback === undefined || callback.length === 0) { + return next(new Error('callback parameter should be present and be a function name')); } - }); - QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), (err, affectedTables) => { + if (config) { + try { + req.body = JSON.parse(config); + } catch(e) { + return next(new Error('Invalid config parameter, should be a valid JSON')); + } + } + } + + return next(); + }; +} + +MapController.prototype.prepareAdapterMapConfig = function () { + return function prepareAdapterMapConfigMiddleware(req, res, next) { + const requestMapConfig = req.body; + const { user, dbhost, dbport, dbname, dbuser, dbpassword, api_key } = res.locals; + + const context = { + analysisConfiguration: { + user, + db: { + host: dbhost, + port: dbport, + dbname: dbname, + user: dbuser, + pass: dbpassword + }, + batch: { + username: user, + apiKey: api_key + } + } + }; + + this.mapConfigAdapter.getMapConfig(user, requestMapConfig, res.locals, context, (err, requestMapConfig) => { if (err) { return next(err); } - // feed affected tables cache so it can be reused from, for instance, layergroup controller - this.layergroupAffectedTables.set(dbname, layergroup.layergroupId, affectedTables); - - res.locals.affectedTables = affectedTables; + req.body = requestMapConfig; + res.locals.context = context; next(); }); - }); + }.bind(this); }; -MapController.prototype.setCacheChannel = function (req, res, callback) { - const { affectedTables } = res.locals; +MapController.prototype.createLayergroup = function () { + return function createLayergroupMiddleware (req, res, next) { + const requestMapConfig = req.body; + const { context, user } = res.locals; + const datasource = context.datasource || Datasource.EmptyDatasource(); + const mapconfig = new MapConfig(requestMapConfig, datasource); + const mapconfigProvider = new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, res.locals); - if (req.method === 'GET') { - res.set('Last-Modified', (new Date()).toUTCString()); - res.set('X-Cache-Channel', affectedTables.getCacheChannel()); - } + res.locals.mapconfig = mapconfig; + res.locals.analysesResults = context.analysesResults; - callback(); + this.mapBackend.createLayergroup(mapconfig, res.locals, mapconfigProvider, (err, layergroup) => { + if (err) { + return next(err); + } + + res.locals.layergroup = layergroup; + + next(); + }); + }.bind(this); }; -MapController.prototype.setLastUpdatedTimeToLayergroup = function (req, res, callback) { - const { affectedTables, layergroup, analysesResults } = res.locals; +MapController.prototype.getTemplate = function () { + return function getTemplateMiddleware (req, res, next) { + const templateParams = req.body; + const { user } = res.locals; - var lastUpdateTime = affectedTables.getLastUpdatedAt(); + const mapconfigProvider = new NamedMapMapConfigProvider( + this.templateMaps, + this.pgConnection, + this.metadataBackend, + this.userLimitsApi, + this.mapConfigAdapter, + user, + req.params.template_id, + templateParams, + res.locals.auth_token, + res.locals + ); - lastUpdateTime = getLastUpdatedTime(analysesResults, lastUpdateTime) || lastUpdateTime; + mapconfigProvider.getMapConfig((err, mapconfig, rendererParams) => { + if (err) { + return next(err); + } - // last update for layergroup cache buster - layergroup.layergroupid = layergroup.layergroupid + ':' + lastUpdateTime; - layergroup.last_updated = new Date(lastUpdateTime).toISOString(); + res.locals.mapconfig = mapconfig; + res.locals.rendererParams = rendererParams; + res.locals.mapconfigProvider = mapconfigProvider; - callback(); + next(); + }); + }.bind(this); }; -MapController.prototype.setCacheControl = function (req, res, callback) { - if (req.method === 'GET') { - var ttl = global.environment.varnish.layergroupTtl || 86400; - res.set('Cache-Control', 'public,max-age='+ttl+',must-revalidate'); - } +MapController.prototype.instantiateLayergroup = function () { + return function instantiateLayergroupMiddleware (req, res, next) { + const { user, mapconfig, rendererParams } = res.locals; + const mapconfigProvider = + new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, rendererParams); - callback(); + this.mapBackend.createLayergroup(mapconfig, rendererParams, mapconfigProvider, (err, layergroup) => { + if (err) { + return next(err); + } + + res.locals.layergroup = layergroup; + + // TODO: Do not provide shortcuts + const { mapconfigProvider } = res.locals; + + res.locals.analysesResults = mapconfigProvider.analysesResults; + res.locals.template = mapconfigProvider.template; + res.locals.templateName = mapconfigProvider.getTemplateName(); + res.locals.context = mapconfigProvider.context; + + next(); + }); + }.bind(this); }; -MapController.prototype.setLayerStats = function (req, res, callback) { - const { user, mapconfig, layergroup } = res.locals; +function sendResponse() { + return function sendResponseMiddleware (req, res) { + const { layergroup } = res.locals; - this.pgConnection.getConnection(user, (err, connection) => { - if (err) { - return callback(err); + res.status(200); + + if (req.query && req.query.callback) { + res.jsonp(layergroup); + } else { + res.json(layergroup); + } + }; +} + +MapController.prototype.incrementMapViewCount = function () { + return function incrementMapViewCountMiddleware(req, res, next) { + const { mapconfig, user } = res.locals; + + // Error won't blow up, just be logged. + this.metadataBackend.incMapviewCount(user, mapconfig.obj().stat_tag, (err) => { + req.profiler.done('incMapviewCount'); + + if (err) { + global.logger.log(`ERROR: failed to increment mapview count for user '${user}': ${err.message}`); + } + + next(); + }); + }.bind(this); +}; + +MapController.prototype.augmentLayergroupData = function () { + return function augmentLayergroupDataMiddleware (req, res, next) { + const { layergroup } = res.locals; + + // include in layergroup response the variables in serverMedata + // those variables are useful to send to the client information + // about how to reach this server or information about it + _.extend(layergroup, global.environment.serverMetadata); + + next(); + }; +}; + +MapController.prototype.getAffectedTables = function () { + return function getAffectedTablesMiddleware (req, res, next) { + const { dbname, layergroup, user, mapconfig } = res.locals; + + this.pgConnection.getConnection(user, (err, connection) => { + if (err) { + return next(err); + } + + const sql = []; + mapconfig.getLayers().forEach(function(layer) { + sql.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(function(table) { + sql.push('SELECT * FROM ' + table + ' LIMIT 0'); + }); + } + }); + + QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), (err, affectedTables) => { + if (err) { + return next(err); + } + + // feed affected tables cache so it can be reused from, for instance, layergroup controller + this.layergroupAffectedTables.set(dbname, layergroup.layergroupId, affectedTables); + + res.locals.affectedTables = affectedTables; + + next(); + }); + }); + }.bind(this); +}; + +MapController.prototype.setCacheChannel = function () { + return function setCacheChannelMiddleware (req, res, next) { + const { affectedTables } = res.locals; + + if (req.method === 'GET') { + res.set('Last-Modified', (new Date()).toUTCString()); + res.set('X-Cache-Channel', affectedTables.getCacheChannel()); } - this.statsBackend.getStats(mapconfig, connection, function(err, layersStats) { + next(); + }; +}; + +MapController.prototype.setLastUpdatedTimeToLayergroup = function () { + return function setLastUpdatedTimeToLayergroupMiddleware (req, res, next) { + const { affectedTables, layergroup, analysesResults } = res.locals; + + var lastUpdateTime = affectedTables.getLastUpdatedAt(); + + lastUpdateTime = getLastUpdatedTime(analysesResults, lastUpdateTime) || lastUpdateTime; + + // last update for layergroup cache buster + layergroup.layergroupid = layergroup.layergroupid + ':' + lastUpdateTime; + layergroup.last_updated = new Date(lastUpdateTime).toISOString(); + + next(); + }; +}; + +MapController.prototype.setCacheControl = function () { + return function setCacheControlMiddleware (req, res, next) { + if (req.method === 'GET') { + var ttl = global.environment.varnish.layergroupTtl || 86400; + res.set('Cache-Control', 'public,max-age='+ttl+',must-revalidate'); + } + + next(); + }; +}; + +MapController.prototype.setLayerStats = function () { + return function setLayerStatsMiddleware(req, res, next) { + const { user, mapconfig, layergroup } = res.locals; + + this.pgConnection.getConnection(user, (err, connection) => { if (err) { - return callback(err); + return next(err); } - if (layersStats.length > 0) { - layergroup.metadata.layers.forEach(function (layer, index) { - layer.meta.stats = layersStats[index]; - }); - } + this.statsBackend.getStats(mapconfig, connection, function(err, layersStats) { + if (err) { + return next(err); + } - callback(); + if (layersStats.length > 0) { + layergroup.metadata.layers.forEach(function (layer, index) { + layer.meta.stats = layersStats[index]; + }); + } + + next(); + }); }); - }); + }.bind(this); }; MapController.prototype.setLayergroupIdHeaderBuilder = function (useTemplateHash) { - const self = this; - return function setLayergroupIdHeader(req, res, callback) { + return function setLayergroupIdHeaderMiddleware (req, res, next) { const { layergroup, user, template } = res.locals; if (useTemplateHash) { - var templateHash = self.templateMaps.fingerPrint(template).substring(0, 8); + var templateHash = this.templateMaps.fingerPrint(template).substring(0, 8); layergroup.layergroupid = `${user}@${templateHash}@${layergroup.layergroupid}`; } res.set('X-Layergroup-Id', layergroup.layergroupid); - callback(); - }; + next(); + }.bind(); }; -MapController.prototype.setDataviewsAndWidgetsUrlsToLayergroupMetadata = function (req, res, callback) { - const { layergroup, user, mapconfig } = res.locals; +MapController.prototype.setDataviewsAndWidgetsUrlsToLayergroupMetadata = function () { + return function setDataviewsAndWidgetsUrlsToLayergroupMetadataMiddleware (req, res, next) { + const { layergroup, user, mapconfig } = res.locals; - this.addDataviewsAndWidgetsUrls(user, layergroup, mapconfig.obj()); + this.addDataviewsAndWidgetsUrls(user, layergroup, mapconfig.obj()); - callback(); + next(); + }.bind(this); }; -MapController.prototype.setAnalysesMetadataToLayergroupBuilder = function (includeQuery) { - const self = this; - - return function setAnalysesMetadataToLayergroup (req, res, callback) { +MapController.prototype.setAnalysesMetadataToLayergroup = function (includeQuery) { + return function setAnalysesMetadataToLayergroupMiddleware (req, res, next) { const { layergroup, user, analysesResults = [] } = res.locals; - self.addAnalysesMetadata(user, layergroup, analysesResults, includeQuery); + this.addAnalysesMetadata(user, layergroup, analysesResults, includeQuery); - callback(); - }; + next(); + }.bind(this); }; -MapController.prototype.setTurboCartoMetadataToLayergroup = function (req, res, callback) { - const { layergroup, mapconfig, context } = res.locals; +MapController.prototype.setTurboCartoMetadataToLayergroup = function () { + return function (req, res, next) { + const { layergroup, mapconfig, context } = res.locals; - addContextMetadata(layergroup, mapconfig.obj(), context); + addContextMetadata(layergroup, mapconfig.obj(), context); - callback(); + next(); + }; }; function addContextMetadata(layergroup, mapConfig, context) { @@ -437,18 +467,20 @@ function addContextMetadata(layergroup, mapConfig, context) { } } -MapController.prototype.setSurrogateKeyHeader = function (req, res, callback) { - const { affectedTables, user, templateName } = res.locals; +MapController.prototype.setSurrogateKeyHeader = function () { + return function setSurrogateKeyHeaderMiddleware(req, res, next) { + const { affectedTables, user, templateName } = res.locals; - if (req.method === 'GET' && affectedTables.tables && affectedTables.tables.length > 0) { - this.surrogateKeysCache.tag(res, affectedTables); - } + if (req.method === 'GET' && affectedTables.tables && affectedTables.tables.length > 0) { + this.surrogateKeysCache.tag(res, affectedTables); + } - if (templateName) { - this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, templateName)); - } + if (templateName) { + this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, templateName)); + } - callback(); + next(); + }.bind(this); }; function getLastUpdatedTime(analysesResults, lastUpdateTime) { @@ -534,7 +566,7 @@ MapController.prototype.addWidgetsUrl = function(username, layergroup, mapConfig function augmentError (options) { const { addContext = false, label = 'MAPS CONTROLLER' } = options; - return function mapError (err, req, res, next) { + return function augmentErrorMiddleware (err, req, res, next) { const { mapconfig } = res.locals; if (addContext) { From 05ccf2063403dba3efd8472c747bb5bfa16e31a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 3 Nov 2017 15:12:18 +0100 Subject: [PATCH 43/47] Rename function --- lib/cartodb/controllers/map.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index ea443253..cae3ff9a 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -84,7 +84,7 @@ MapController.prototype.composeCreateMapMiddleware = function ({ useTemplate = f this.setLastUpdatedTimeToLayergroup(), this.setCacheControl(), this.setLayerStats(), - this.setLayergroupIdHeaderBuilder(useTemplateHash), + this.setLayergroupIdHeader(useTemplateHash), this.setDataviewsAndWidgetsUrlsToLayergroupMetadata(), this.setAnalysesMetadataToLayergroup(includeQuery), this.setTurboCartoMetadataToLayergroup(), @@ -411,7 +411,7 @@ MapController.prototype.setLayerStats = function () { }.bind(this); }; -MapController.prototype.setLayergroupIdHeaderBuilder = function (useTemplateHash) { +MapController.prototype.setLayergroupIdHeader = function (useTemplateHash) { return function setLayergroupIdHeaderMiddleware (req, res, next) { const { layergroup, user, template } = res.locals; @@ -423,7 +423,7 @@ MapController.prototype.setLayergroupIdHeaderBuilder = function (useTemplateHash res.set('X-Layergroup-Id', layergroup.layergroupid); next(); - }.bind(); + }.bind(this); }; MapController.prototype.setDataviewsAndWidgetsUrlsToLayergroupMetadata = function () { From 46289f27df6745c6f0cf8e270cd7597b5278fee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 3 Nov 2017 15:26:25 +0100 Subject: [PATCH 44/47] Remove TODO --- lib/cartodb/controllers/map.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index cae3ff9a..ff88b453 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -253,7 +253,6 @@ MapController.prototype.instantiateLayergroup = function () { res.locals.layergroup = layergroup; - // TODO: Do not provide shortcuts const { mapconfigProvider } = res.locals; res.locals.analysesResults = mapconfigProvider.analysesResults; From f9ba3c41d306fc89c04ec6057d3d0f15edcf1f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sun, 5 Nov 2017 18:55:23 +0100 Subject: [PATCH 45/47] Create new middlewares to init profiling and another to check JSON content-type --- lib/cartodb/controllers/map.js | 74 ++++++++++--------- .../ported/multilayer_error_cases.js | 2 +- 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index ff88b453..93a917b8 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -49,22 +49,18 @@ function MapController(prepareContext, pgConnection, templateMaps, mapBackend, m module.exports = MapController; MapController.prototype.register = function(app) { - app.get(app.base_url_mapconfig, this.composeCreateMapMiddleware()); - - app.post(app.base_url_mapconfig, this.composeCreateMapMiddleware()); - - app.get(app.base_url_templated + '/:template_id/jsonp', this.composeCreateMapMiddleware({ - useTemplate: true - })); - - app.post(app.base_url_templated + '/:template_id', this.composeCreateMapMiddleware({ - useTemplate: true - })); + const { base_url_mapconfig, base_url_templated } = app; + const useTemplate = true; + app.get(base_url_mapconfig, this.composeCreateMapMiddleware()); + app.post(base_url_mapconfig, this.composeCreateMapMiddleware()); + app.get(`${base_url_templated}/:template_id/jsonp`, this.composeCreateMapMiddleware(useTemplate)); + app.post(`${base_url_templated}/:template_id`, this.composeCreateMapMiddleware(useTemplate)); app.options(app.base_url_mapconfig, cors('Content-Type')); }; -MapController.prototype.composeCreateMapMiddleware = function ({ useTemplate = false } = {}) { +MapController.prototype.composeCreateMapMiddleware = function (useTemplate = false) { + const isTemplateInstantiation = useTemplate; const useTemplateHash = useTemplate; const includeQuery = !useTemplate; const label = useTemplate ? 'NAMED MAP LAYERGROUP' : 'ANONYMOUS LAYERGROUP'; @@ -74,7 +70,9 @@ MapController.prototype.composeCreateMapMiddleware = function ({ useTemplate = f cors(), userMiddleware, this.prepareContext, - useTemplate ? checkIntantiteLayergroup() : checkCreateLayergroup(), + this.initProfiler(isTemplateInstantiation), + this.checkJsonContentType(), + useTemplate ? this.checkInstantiteLayergroup() : this.checkCreateLayergroup(), useTemplate ? this.getTemplate() : this.prepareAdapterMapConfig(), useTemplate ? this.instantiateLayergroup() : this.createLayergroup(), this.incrementMapViewCount(), @@ -89,20 +87,32 @@ MapController.prototype.composeCreateMapMiddleware = function ({ useTemplate = f this.setAnalysesMetadataToLayergroup(includeQuery), this.setTurboCartoMetadataToLayergroup(), this.setSurrogateKeyHeader(), - sendResponse(), - augmentError({ label, addContext }) + this.sendResponse(), + this.augmentError({ label, addContext }) ]; }; +MapController.prototype.initProfiler = function (isTemplateInstantiation) { + const operation = isTemplateInstantiation ? 'instance_template' : 'createmap'; -function checkCreateLayergroup () { - return function checkCreateLayergroupMiddleware (req, res, next) { - req.profiler.start(`windshaft.createmap_${req.method.toLowerCase()}`); + return function initProfilerMiddleware (req, res, next) { + req.profiler.start(`windshaft-cartodb.${operation}_${req.method.toLowerCase()}`); + next(); + }; +}; +MapController.prototype.checkJsonContentType = function () { + return function checkJsonContentTypeMiddleware(req, res, next) { if (req.method === 'POST' && !req.is('application/json')) { - return next(new Error('layergroup POST data must be of type application/json')); + return next(new Error('POST data must be of type application/json')); } + next(); + }; +}; + +MapController.prototype.checkCreateLayergroup = function () { + return function checkCreateLayergroupMiddleware (req, res, next) { if (req.method === 'GET') { const { config } = res.locals; @@ -119,17 +129,10 @@ function checkCreateLayergroup () { return next(); }; -} - -function checkIntantiteLayergroup () { - return function checkIntantiteLayergroupMiddleware(req, res, next) { - // jshint maxcomplexity: 7 - req.profiler.start(`windshaft-cartodb.instance_template_${req.method.toLowerCase()}`); - - if (req.method === 'POST' && !req.is('application/json')) { - return next(new Error('Template POST data must be of type application/json')); - } +}; +MapController.prototype.checkInstantiteLayergroup = function () { + return function checkInstantiteLayergroupMiddleware(req, res, next) { if (req.method === 'GET') { const { callback, config } = req.query; @@ -148,7 +151,7 @@ function checkIntantiteLayergroup () { return next(); }; -} +}; MapController.prototype.prepareAdapterMapConfig = function () { return function prepareAdapterMapConfigMiddleware(req, res, next) { @@ -191,7 +194,8 @@ MapController.prototype.createLayergroup = function () { const { context, user } = res.locals; const datasource = context.datasource || Datasource.EmptyDatasource(); const mapconfig = new MapConfig(requestMapConfig, datasource); - const mapconfigProvider = new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, res.locals); + const mapconfigProvider = + new CreateLayergroupMapConfigProvider(mapconfig, user, this.userLimitsApi, res.locals); res.locals.mapconfig = mapconfig; res.locals.analysesResults = context.analysesResults; @@ -265,7 +269,7 @@ MapController.prototype.instantiateLayergroup = function () { }.bind(this); }; -function sendResponse() { +MapController.prototype.sendResponse = function () { return function sendResponseMiddleware (req, res) { const { layergroup } = res.locals; @@ -277,7 +281,7 @@ function sendResponse() { res.json(layergroup); } }; -} +}; MapController.prototype.incrementMapViewCount = function () { return function incrementMapViewCountMiddleware(req, res, next) { @@ -562,7 +566,7 @@ MapController.prototype.addWidgetsUrl = function(username, layergroup, mapConfig } }; -function augmentError (options) { +MapController.prototype.augmentError = function (options) { const { addContext = false, label = 'MAPS CONTROLLER' } = options; return function augmentErrorMiddleware (err, req, res, next) { @@ -576,7 +580,7 @@ function augmentError (options) { next(err); }; -} +}; function populateError(err, mapConfig) { var error = new Error(err.message); diff --git a/test/acceptance/ported/multilayer_error_cases.js b/test/acceptance/ported/multilayer_error_cases.js index 92ce0b1f..9efd7c7b 100644 --- a/test/acceptance/ported/multilayer_error_cases.js +++ b/test/acceptance/ported/multilayer_error_cases.js @@ -28,7 +28,7 @@ describe('multilayer error cases', function() { }, {}, function(res) { assert.equal(res.statusCode, 400, res.body); var parsedBody = JSON.parse(res.body); - assert.deepEqual(parsedBody.errors, ["layergroup POST data must be of type application/json"]); + assert.deepEqual(parsedBody.errors, ["POST data must be of type application/json"]); done(); }); }); From 693a2e7beefe6ca92b0b95b3a4699f61a1ffa0e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sun, 5 Nov 2017 19:13:56 +0100 Subject: [PATCH 46/47] Order middlewares --- lib/cartodb/controllers/map.js | 312 ++++++++++++++++----------------- 1 file changed, 156 insertions(+), 156 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 93a917b8..1510dfc2 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -111,26 +111,6 @@ MapController.prototype.checkJsonContentType = function () { }; }; -MapController.prototype.checkCreateLayergroup = function () { - return function checkCreateLayergroupMiddleware (req, res, next) { - if (req.method === 'GET') { - const { config } = res.locals; - - if (!config) { - return next(new Error('layergroup GET needs a "config" parameter')); - } - - try { - req.body = JSON.parse(config); - } catch (err) { - return next(err); - } - } - - return next(); - }; -}; - MapController.prototype.checkInstantiteLayergroup = function () { return function checkInstantiteLayergroupMiddleware(req, res, next) { if (req.method === 'GET') { @@ -153,6 +133,58 @@ MapController.prototype.checkInstantiteLayergroup = function () { }; }; +MapController.prototype.checkCreateLayergroup = function () { + return function checkCreateLayergroupMiddleware (req, res, next) { + if (req.method === 'GET') { + const { config } = res.locals; + + if (!config) { + return next(new Error('layergroup GET needs a "config" parameter')); + } + + try { + req.body = JSON.parse(config); + } catch (err) { + return next(err); + } + } + + return next(); + }; +}; + +MapController.prototype.getTemplate = function () { + return function getTemplateMiddleware (req, res, next) { + const templateParams = req.body; + const { user } = res.locals; + + const mapconfigProvider = new NamedMapMapConfigProvider( + this.templateMaps, + this.pgConnection, + this.metadataBackend, + this.userLimitsApi, + this.mapConfigAdapter, + user, + req.params.template_id, + templateParams, + res.locals.auth_token, + res.locals + ); + + mapconfigProvider.getMapConfig((err, mapconfig, rendererParams) => { + if (err) { + return next(err); + } + + res.locals.mapconfig = mapconfig; + res.locals.rendererParams = rendererParams; + res.locals.mapconfigProvider = mapconfigProvider; + + next(); + }); + }.bind(this); +}; + MapController.prototype.prepareAdapterMapConfig = function () { return function prepareAdapterMapConfigMiddleware(req, res, next) { const requestMapConfig = req.body; @@ -212,38 +244,6 @@ MapController.prototype.createLayergroup = function () { }.bind(this); }; -MapController.prototype.getTemplate = function () { - return function getTemplateMiddleware (req, res, next) { - const templateParams = req.body; - const { user } = res.locals; - - const mapconfigProvider = new NamedMapMapConfigProvider( - this.templateMaps, - this.pgConnection, - this.metadataBackend, - this.userLimitsApi, - this.mapConfigAdapter, - user, - req.params.template_id, - templateParams, - res.locals.auth_token, - res.locals - ); - - mapconfigProvider.getMapConfig((err, mapconfig, rendererParams) => { - if (err) { - return next(err); - } - - res.locals.mapconfig = mapconfig; - res.locals.rendererParams = rendererParams; - res.locals.mapconfigProvider = mapconfigProvider; - - next(); - }); - }.bind(this); -}; - MapController.prototype.instantiateLayergroup = function () { return function instantiateLayergroupMiddleware (req, res, next) { const { user, mapconfig, rendererParams } = res.locals; @@ -269,20 +269,6 @@ MapController.prototype.instantiateLayergroup = function () { }.bind(this); }; -MapController.prototype.sendResponse = function () { - return function sendResponseMiddleware (req, res) { - const { layergroup } = res.locals; - - res.status(200); - - if (req.query && req.query.callback) { - res.jsonp(layergroup); - } else { - res.json(layergroup); - } - }; -}; - MapController.prototype.incrementMapViewCount = function () { return function incrementMapViewCountMiddleware(req, res, next) { const { mapconfig, user } = res.locals; @@ -377,6 +363,19 @@ MapController.prototype.setLastUpdatedTimeToLayergroup = function () { }; }; +function getLastUpdatedTime(analysesResults, lastUpdateTime) { + if (!Array.isArray(analysesResults)) { + return lastUpdateTime; + } + return analysesResults.reduce(function(lastUpdateTime, analysis) { + return analysis.getNodes().reduce(function(lastNodeUpdatedAtTime, node) { + var nodeUpdatedAtDate = node.getUpdatedAt(); + var nodeUpdatedTimeAt = (nodeUpdatedAtDate && nodeUpdatedAtDate.getTime()) || 0; + return nodeUpdatedTimeAt > lastNodeUpdatedAtTime ? nodeUpdatedTimeAt : lastNodeUpdatedAtTime; + }, lastUpdateTime); + }, lastUpdateTime); +} + MapController.prototype.setCacheControl = function () { return function setCacheControlMiddleware (req, res, next) { if (req.method === 'GET') { @@ -439,96 +438,6 @@ MapController.prototype.setDataviewsAndWidgetsUrlsToLayergroupMetadata = functio }.bind(this); }; -MapController.prototype.setAnalysesMetadataToLayergroup = function (includeQuery) { - return function setAnalysesMetadataToLayergroupMiddleware (req, res, next) { - const { layergroup, user, analysesResults = [] } = res.locals; - - this.addAnalysesMetadata(user, layergroup, analysesResults, includeQuery); - - next(); - }.bind(this); -}; - -MapController.prototype.setTurboCartoMetadataToLayergroup = function () { - return function (req, res, next) { - const { layergroup, mapconfig, context } = res.locals; - - addContextMetadata(layergroup, mapconfig.obj(), context); - - next(); - }; -}; - -function addContextMetadata(layergroup, mapConfig, context) { - if (layergroup.metadata && Array.isArray(layergroup.metadata.layers) && Array.isArray(mapConfig.layers)) { - layergroup.metadata.layers = layergroup.metadata.layers.map(function(layer, layerIndex) { - if (context.turboCarto && Array.isArray(context.turboCarto.layers)) { - layer.meta.cartocss_meta = context.turboCarto.layers[layerIndex]; - } - return layer; - }); - } -} - -MapController.prototype.setSurrogateKeyHeader = function () { - return function setSurrogateKeyHeaderMiddleware(req, res, next) { - const { affectedTables, user, templateName } = res.locals; - - if (req.method === 'GET' && affectedTables.tables && affectedTables.tables.length > 0) { - this.surrogateKeysCache.tag(res, affectedTables); - } - - if (templateName) { - this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, templateName)); - } - - next(); - }.bind(this); -}; - -function getLastUpdatedTime(analysesResults, lastUpdateTime) { - if (!Array.isArray(analysesResults)) { - return lastUpdateTime; - } - return analysesResults.reduce(function(lastUpdateTime, analysis) { - return analysis.getNodes().reduce(function(lastNodeUpdatedAtTime, node) { - var nodeUpdatedAtDate = node.getUpdatedAt(); - var nodeUpdatedTimeAt = (nodeUpdatedAtDate && nodeUpdatedAtDate.getTime()) || 0; - return nodeUpdatedTimeAt > lastNodeUpdatedAtTime ? nodeUpdatedTimeAt : lastNodeUpdatedAtTime; - }, lastUpdateTime); - }, lastUpdateTime); -} - -MapController.prototype.addAnalysesMetadata = function(username, layergroup, analysesResults, includeQuery) { - includeQuery = includeQuery || false; - analysesResults = analysesResults || []; - layergroup.metadata.analyses = []; - - analysesResults.forEach(function(analysis) { - var nodes = analysis.getNodes(); - layergroup.metadata.analyses.push({ - nodes: nodes.reduce(function(nodesIdMap, node) { - if (node.params.id) { - var nodeResource = layergroup.layergroupid + '/analysis/node/' + node.id(); - var nodeRepr = { - status: node.getStatus(), - url: this.resourceLocator.getUrls(username, nodeResource) - }; - if (includeQuery) { - nodeRepr.query = node.getQuery(); - } - if (node.getStatus() === 'failed') { - nodeRepr.error_message = node.getErrorMessage(); - } - nodesIdMap[node.params.id] = nodeRepr; - } - - return nodesIdMap; - }.bind(this), {}) - }); - }.bind(this)); -}; - // TODO this should take into account several URL patterns MapController.prototype.addDataviewsAndWidgetsUrls = function(username, layergroup, mapConfig) { this.addDataviewsUrls(username, layergroup, mapConfig); @@ -566,6 +475,97 @@ MapController.prototype.addWidgetsUrl = function(username, layergroup, mapConfig } }; +MapController.prototype.setAnalysesMetadataToLayergroup = function (includeQuery) { + return function setAnalysesMetadataToLayergroupMiddleware (req, res, next) { + const { layergroup, user, analysesResults = [] } = res.locals; + + this.addAnalysesMetadata(user, layergroup, analysesResults, includeQuery); + + next(); + }.bind(this); +}; + +MapController.prototype.addAnalysesMetadata = function(username, layergroup, analysesResults, includeQuery) { + includeQuery = includeQuery || false; + analysesResults = analysesResults || []; + layergroup.metadata.analyses = []; + + analysesResults.forEach(function(analysis) { + var nodes = analysis.getNodes(); + layergroup.metadata.analyses.push({ + nodes: nodes.reduce(function(nodesIdMap, node) { + if (node.params.id) { + var nodeResource = layergroup.layergroupid + '/analysis/node/' + node.id(); + var nodeRepr = { + status: node.getStatus(), + url: this.resourceLocator.getUrls(username, nodeResource) + }; + if (includeQuery) { + nodeRepr.query = node.getQuery(); + } + if (node.getStatus() === 'failed') { + nodeRepr.error_message = node.getErrorMessage(); + } + nodesIdMap[node.params.id] = nodeRepr; + } + + return nodesIdMap; + }.bind(this), {}) + }); + }.bind(this)); +}; + +MapController.prototype.setTurboCartoMetadataToLayergroup = function () { + return function setTurboCartoMetadataToLayergroupMiddleware (req, res, next) { + const { layergroup, mapconfig, context } = res.locals; + + addContextMetadata(layergroup, mapconfig.obj(), context); + + next(); + }; +}; + +function addContextMetadata(layergroup, mapConfig, context) { + if (layergroup.metadata && Array.isArray(layergroup.metadata.layers) && Array.isArray(mapConfig.layers)) { + layergroup.metadata.layers = layergroup.metadata.layers.map(function(layer, layerIndex) { + if (context.turboCarto && Array.isArray(context.turboCarto.layers)) { + layer.meta.cartocss_meta = context.turboCarto.layers[layerIndex]; + } + return layer; + }); + } +} + +MapController.prototype.setSurrogateKeyHeader = function () { + return function setSurrogateKeyHeaderMiddleware(req, res, next) { + const { affectedTables, user, templateName } = res.locals; + + if (req.method === 'GET' && affectedTables.tables && affectedTables.tables.length > 0) { + this.surrogateKeysCache.tag(res, affectedTables); + } + + if (templateName) { + this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, templateName)); + } + + next(); + }.bind(this); +}; + +MapController.prototype.sendResponse = function () { + return function sendResponseMiddleware (req, res) { + const { layergroup } = res.locals; + + res.status(200); + + if (req.query && req.query.callback) { + res.jsonp(layergroup); + } else { + res.json(layergroup); + } + }; +}; + MapController.prototype.augmentError = function (options) { const { addContext = false, label = 'MAPS CONTROLLER' } = options; From c48e89826d117225050f6f9dafb8cfc29ab45d1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 7 Nov 2017 09:50:52 +0100 Subject: [PATCH 47/47] Split middleware to follow SRP --- lib/cartodb/controllers/map.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 1510dfc2..4c2d6764 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -79,6 +79,7 @@ MapController.prototype.composeCreateMapMiddleware = function (useTemplate = fal this.augmentLayergroupData(), this.getAffectedTables(), this.setCacheChannel(), + this.setLastModified(), this.setLastUpdatedTimeToLayergroup(), this.setCacheControl(), this.setLayerStats(), @@ -339,7 +340,6 @@ MapController.prototype.setCacheChannel = function () { const { affectedTables } = res.locals; if (req.method === 'GET') { - res.set('Last-Modified', (new Date()).toUTCString()); res.set('X-Cache-Channel', affectedTables.getCacheChannel()); } @@ -347,6 +347,16 @@ MapController.prototype.setCacheChannel = function () { }; }; +MapController.prototype.setLastModified = function () { + return function setLastModifiedMiddleware (req, res, next) { + if (req.method === 'GET') { + res.set('Last-Modified', (new Date()).toUTCString()); + } + + next(); + }; +}; + MapController.prototype.setLastUpdatedTimeToLayergroup = function () { return function setLastUpdatedTimeToLayergroupMiddleware (req, res, next) { const { affectedTables, layergroup, analysesResults } = res.locals;