From ef5ea5b4cbc2713afa3fb39f58c2bfb58ced9364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 19:31:02 +0100 Subject: [PATCH 01/15] Create and use getNamedMapProvider middleware --- lib/cartodb/controllers/named_maps.js | 55 ++++++++++++--------------- yarn.lock | 10 ++--- 2 files changed, 29 insertions(+), 36 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index cd2d120f..6486379c 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -27,6 +27,7 @@ NamedMapsController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, + this.getNamedMapProvider(), this.tile.bind(this), vectorError() ); @@ -37,10 +38,29 @@ NamedMapsController.prototype.register = function(app) { userMiddleware, allowQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), this.prepareContext, + this.getNamedMapProvider(), this.staticMap.bind(this) ); }; +NamedMapsController.prototype.getNamedMapProvider = function () { + return function getNamedMapProviderMiddleware (req, res, next) { + const { user } = res.locals; + const { config, auth_token } = req.query; + const { template_id } = req.params; + + this.namedMapProviderCache.get(user, template_id, config, auth_token, res.locals, (err, namedMapProvider) => { + if (err) { + return next(err); + } + + res.locals.namedMapProvider = namedMapProvider; + + next(); + }); + }.bind(this); +}; + NamedMapsController.prototype.sendResponse = function(req, res, body, headers, namedMapProvider) { this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(res.locals.user, namedMapProvider.getTemplateName())); res.set('Content-Type', headers['content-type'] || headers['Content-Type'] || 'image/png'); @@ -83,23 +103,10 @@ NamedMapsController.prototype.sendResponse = function(req, res, body, headers, n NamedMapsController.prototype.tile = function(req, res, next) { var self = this; - var cdbUser = res.locals.user; + const { namedMapProvider } = res.locals; - var namedMapProvider; step( - function getNamedMapProvider() { - self.namedMapProviderCache.get( - cdbUser, - req.params.template_id, - req.query.config, - req.query.auth_token, - res.locals, - this - ); - }, - function getTile(err, _namedMapProvider) { - assert.ifError(err); - namedMapProvider = _namedMapProvider; + function getTile() { self.tileBackend.getTile(namedMapProvider, req.params, this); }, function handleImage(err, tile, headers, stats) { @@ -126,23 +133,9 @@ NamedMapsController.prototype.staticMap = function(req, res, next) { res.locals.format = 'png'; res.locals.layer = res.locals.layer || 'all'; - var namedMapProvider; + const { namedMapProvider } = res.locals; step( - function getNamedMapProvider() { - self.namedMapProviderCache.get( - cdbUser, - req.params.template_id, - req.query.config, - req.query.auth_token, - res.locals, - this - ); - }, - function prepareLayerVisibility(err, _namedMapProvider) { - assert.ifError(err); - - namedMapProvider = _namedMapProvider; - + function prepareLayerVisibility() { self.prepareLayerFilterFromPreviewLayers(cdbUser, req, res.locals, namedMapProvider, this); }, function prepareImageOptions(err) { diff --git a/yarn.lock b/yarn.lock index 26e59383..d689972f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,7 +2,7 @@ # yarn lockfile v1 -"abaculus@github:cartodb/abaculus#2.0.3-cdb1": +abaculus@cartodb/abaculus#2.0.3-cdb1: version "2.0.3-cdb1" resolved "https://codeload.github.com/cartodb/abaculus/tar.gz/f5f34e1c80cdd8d49edd1d6fe3b2220ab2e23aaf" dependencies: @@ -226,7 +226,7 @@ camshaft@0.60.0: dot "^1.0.3" request "^2.69.0" -"canvas@github:cartodb/node-canvas#1.6.2-cdb2": +canvas@cartodb/node-canvas#1.6.2-cdb2: version "1.6.2-cdb2" resolved "https://codeload.github.com/cartodb/node-canvas/tar.gz/8acf04557005c633f9e68524488a2657c04f3766" dependencies: @@ -252,7 +252,7 @@ carto@CartoDB/carto#0.15.1-cdb1: optimist "~0.6.0" underscore "~1.6.0" -"carto@github:cartodb/carto#0.15.1-cdb3": +carto@cartodb/carto#0.15.1-cdb3: version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" dependencies: @@ -2223,7 +2223,7 @@ through@2: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" -"tilelive-bridge@github:cartodb/tilelive-bridge#2.3.1-cdb4": +tilelive-bridge@cartodb/tilelive-bridge#2.3.1-cdb4: version "2.3.1-cdb4" resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/faa2b638da2d119b78281575d40255cb523f6ca6" dependencies: @@ -2231,7 +2231,7 @@ through@2: mapnik-pool "~0.1.3" sphericalmercator "1.0.x" -"tilelive-mapnik@github:cartodb/tilelive-mapnik#0.6.18-cdb3": +tilelive-mapnik@cartodb/tilelive-mapnik#0.6.18-cdb3: version "0.6.18-cdb3" resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/23bd1c31dd57d0b76c86b9f1eaf62462b3c17d01" dependencies: From 108a319143d697ae881ccbd1c8b4270811030922 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 29 Dec 2017 19:33:49 +0100 Subject: [PATCH 02/15] Do not use step --- lib/cartodb/controllers/named_maps.js | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 6486379c..0d657858 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -101,25 +101,18 @@ NamedMapsController.prototype.sendResponse = function(req, res, body, headers, n }; NamedMapsController.prototype.tile = function(req, res, next) { - var self = this; - const { namedMapProvider } = res.locals; - step( - function getTile() { - self.tileBackend.getTile(namedMapProvider, req.params, this); - }, - function handleImage(err, tile, headers, stats) { - req.profiler.add(stats); + this.tileBackend.getTile(namedMapProvider, req.params, (err, tile, headers, stats) => { + req.profiler.add(stats); - if (err) { - err.label = 'NAMED_MAP_TILE'; - next(err); - } else { - self.sendResponse(req, res, tile, headers, namedMapProvider); - } + if (err) { + err.label = 'NAMED_MAP_TILE'; + return next(err); } - ); + + this.sendResponse(req, res, tile, headers, namedMapProvider); + }); }; NamedMapsController.prototype.staticMap = function(req, res, next) { From 500cbb959f08ac5a9a8899eb7afd2ded2de4a6c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sat, 30 Dec 2017 14:13:23 +0100 Subject: [PATCH 03/15] Move method to a middleware --- lib/cartodb/controllers/named_maps.js | 94 +++++++++++++-------------- 1 file changed, 46 insertions(+), 48 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 0d657858..2c52d2fc 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -39,6 +39,7 @@ NamedMapsController.prototype.register = function(app) { allowQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), this.prepareContext, this.getNamedMapProvider(), + this.prepareLayerFilterFromPreviewLayers(), this.staticMap.bind(this) ); }; @@ -61,6 +62,51 @@ NamedMapsController.prototype.getNamedMapProvider = function () { }.bind(this); }; +NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function () { + return function prepareLayerFilterFromPreviewLayersMiddleware (req, res, next) { + const { user, namedMapProvider } = res.locals; + const { template_id } = req.params; + const { config, auth_token } = req.query; + + namedMapProvider.getTemplate((err, template) => { + if (err) { + return next(err); + } + + if (!template || !template.view || !template.view.preview_layers) { + return next(); + } + + var previewLayers = template.view.preview_layers; + var layerVisibilityFilter = []; + + template.layergroup.layers.forEach(function (layer, index) { + if (previewLayers[''+index] !== false && previewLayers[layer.id] !== false) { + layerVisibilityFilter.push(''+index); + } + }); + + if (!layerVisibilityFilter.length) { + return next(); + } + + // overwrites 'all' default filter + res.locals.layer = layerVisibilityFilter.join(','); + + // recreates the provider + this.namedMapProviderCache.get(user, template_id, config, auth_token, res.locals, (err, provider) => { + if (err) { + return next(err); + } + + res.locals.namedMapProvider = provider; + + next(); + }); + }); + }.bind(this); +}; + NamedMapsController.prototype.sendResponse = function(req, res, body, headers, namedMapProvider) { this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(res.locals.user, namedMapProvider.getTemplateName())); res.set('Content-Type', headers['content-type'] || headers['Content-Type'] || 'image/png'); @@ -128,9 +174,6 @@ NamedMapsController.prototype.staticMap = function(req, res, next) { const { namedMapProvider } = res.locals; step( - function prepareLayerVisibility() { - self.prepareLayerFilterFromPreviewLayers(cdbUser, req, res.locals, namedMapProvider, this); - }, function prepareImageOptions(err) { assert.ifError(err); self.getStaticImageOptions(cdbUser, res.locals, namedMapProvider, this); @@ -176,51 +219,6 @@ NamedMapsController.prototype.staticMap = function(req, res, next) { ); }; -NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function ( - user, - req, - params, - namedMapProvider, - callback -) { - var self = this; - namedMapProvider.getTemplate(function (err, template) { - if (err) { - return callback(err); - } - - if (!template || !template.view || !template.view.preview_layers) { - return callback(); - } - - var previewLayers = template.view.preview_layers; - var layerVisibilityFilter = []; - - template.layergroup.layers.forEach(function (layer, index) { - if (previewLayers[''+index] !== false && previewLayers[layer.id] !== false) { - layerVisibilityFilter.push(''+index); - } - }); - - if (!layerVisibilityFilter.length) { - return callback(); - } - - // overwrites 'all' default filter - params.layer = layerVisibilityFilter.join(','); - - // recreates the provider - self.namedMapProviderCache.get( - user, - req.params.template_id, - req.query.config, - req.query.auth_token, - params, - callback - ); - }); -}; - var DEFAULT_ZOOM_CENTER = { zoom: 1, center: { From 731fe4c00fb4efe4e039f0cea738ed274af408a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sat, 30 Dec 2017 15:21:20 +0100 Subject: [PATCH 04/15] Move getStaticImageOptions and getImage to a middlewares --- lib/cartodb/controllers/named_maps.js | 230 +++++++++++++++----------- 1 file changed, 131 insertions(+), 99 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 2c52d2fc..c285b25f 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -40,6 +40,8 @@ NamedMapsController.prototype.register = function(app) { this.prepareContext, this.getNamedMapProvider(), this.prepareLayerFilterFromPreviewLayers(), + this.getStaticImageOptions(), + this.getImage(), this.staticMap.bind(this) ); }; @@ -166,47 +168,23 @@ NamedMapsController.prototype.staticMap = function(req, res, next) { var cdbUser = res.locals.user; - var format = req.params.format === 'jpg' ? 'jpeg' : 'png'; - // We force always the tile to be generated using PNG because - // is the only format we support by now - res.locals.format = 'png'; - res.locals.layer = res.locals.layer || 'all'; + const { namedMapProvider, image, headers, stats } = res.locals; - const { namedMapProvider } = res.locals; step( - function prepareImageOptions(err) { - assert.ifError(err); - self.getStaticImageOptions(cdbUser, res.locals, namedMapProvider, this); - }, - function getImage(err, imageOpts) { - assert.ifError(err); - - var width = +req.params.width; - var height = +req.params.height; - - if (!_.isUndefined(imageOpts.zoom) && imageOpts.center) { - self.previewBackend.getImage( - namedMapProvider, format, width, height, imageOpts.zoom, imageOpts.center, this); - } else { - self.previewBackend.getImage( - namedMapProvider, format, width, height, imageOpts.bounds, this); - } - }, - function incrementMapViews(err, image, headers, stats) { - assert.ifError(err); - + function incrementMapViews() { var next = this; namedMapProvider.getMapConfig(function(mapConfigErr, mapConfig) { self.metadataBackend.incMapviewCount(cdbUser, mapConfig.obj().stat_tag, function(sErr) { - if (err) { + if (sErr) { global.logger.log("ERROR: failed to increment mapview count for user '%s': %s", cdbUser, sErr); } - next(err, image, headers, stats); + + next(null, image, headers, stats); }); }); }, function handleImage(err, image, headers, stats) { - req.profiler.done('render-' + format); + req.profiler.done('render-' + res.locals.format); req.profiler.add(stats || {}); if (err) { @@ -231,84 +209,138 @@ function numMapper(n) { return +n; } -NamedMapsController.prototype.getStaticImageOptions = function(cdbUser, params, namedMapProvider, callback) { - var self = this; +NamedMapsController.prototype.getStaticImageOptions = function () { + return function getStaticImageOptionsMiddleware(req, res, next) { + var self = this; - if ([params.zoom, params.lon, params.lat].map(numMapper).every(Number.isFinite)) { - return callback(null, { - zoom: params.zoom, - center: { - lng: params.lon, - lat: params.lat - } - }); - } + const { user, namedMapProvider, zoom, lon, lat, bbox } = res.locals; - if (params.bbox) { - var bbox = params.bbox.split(',').map(numMapper); - if (bbox.length === 4 && bbox.every(Number.isFinite)) { - return callback(null, { - bounds: { - west: bbox[0], - south: bbox[1], - east: bbox[2], - north: bbox[3] + if ([zoom, lon, lat].map(numMapper).every(Number.isFinite)) { + res.locals.imageOpts = { + zoom: zoom, + center: { + lng: lon, + lat: lat } - }); + }; + + return next(); } - } - step( - function getTemplate() { - namedMapProvider.getTemplate(this); - }, - function handleTemplateView(err, template) { - assert.ifError(err); - - if (template.view) { - var zoomCenter = templateZoomCenter(template.view); - if (zoomCenter) { - if (Number.isFinite(+params.zoom)) { - zoomCenter.zoom = +params.zoom; + if (bbox) { + var _bbox = bbox.split(',').map(numMapper); + if (_bbox.length === 4 && _bbox.every(Number.isFinite)) { + res.locals.imageOpts = { + bounds: { + west: _bbox[0], + south: _bbox[1], + east: _bbox[2], + north: _bbox[3] } - return zoomCenter; - } + }; - var bounds = templateBounds(template.view); - if (bounds) { - return bounds; - } + return next(); } - - return false; - }, - function estimateBoundsIfNoImageOpts(err, imageOpts) { - if (imageOpts) { - return imageOpts; - } - - var next = this; - namedMapProvider.getAffectedTablesAndLastUpdatedTime(function(err, affectedTablesAndLastUpdate) { - if (err) { - return next(null); - } - - var affectedTables = affectedTablesAndLastUpdate.tables || []; - - if (affectedTables.length === 0) { - return next(null); - } - - self.tablesExtentApi.getBounds(cdbUser, affectedTables, function(err, result) { - return next(null, result); - }); - }); - - }, - function returnCallback(err, imageOpts) { - return callback(err, imageOpts || DEFAULT_ZOOM_CENTER); } - ); + + step( + function getTemplate() { + namedMapProvider.getTemplate(this); + }, + function handleTemplateView(err, template) { + assert.ifError(err); + + if (template.view) { + var zoomCenter = templateZoomCenter(template.view); + if (zoomCenter) { + if (Number.isFinite(+zoom)) { + zoomCenter.zoom = +zoom; + } + return zoomCenter; + } + + var bounds = templateBounds(template.view); + if (bounds) { + return bounds; + } + } + + return false; + }, + function estimateBoundsIfNoImageOpts(err, imageOpts) { + if (imageOpts) { + return imageOpts; + } + + var _next = this; + namedMapProvider.getAffectedTablesAndLastUpdatedTime(function(err, affectedTablesAndLastUpdate) { + if (err) { + return _next(null); + } + + var affectedTables = affectedTablesAndLastUpdate.tables || []; + + if (affectedTables.length === 0) { + return _next(null); + } + + self.tablesExtentApi.getBounds(user, affectedTables, function (err, result) { + return _next(null, result); + }); + }); + + }, + function returnCallback(err, imageOpts) { + res.locals.imageOpts = imageOpts || DEFAULT_ZOOM_CENTER; + return next(); + } + ); + }.bind(this); +}; + +NamedMapsController.prototype.getImage = function () { + return function getImageMiddleware (req, res, next) { + const { imageOpts, namedMapProvider } = res.locals; + const { zoom, center, bounds } = imageOpts; + + let { width, height } = req.params; + + width = +width; + height = +height; + + const format = req.params.format === 'jpg' ? 'jpeg' : 'png'; + // We force always the tile to be generated using PNG because + // is the only format we support by now + res.locals.format = 'png'; + res.locals.layer = res.locals.layer || 'all'; + + if (!_.isUndefined(zoom) && center) { + return this.previewBackend.getImage(namedMapProvider, format, width, height, zoom, center, + (err, image, headers, stats) => { + if (err) { + return next(err); + } + + res.locals.image = image; + res.locals.headers = headers; + res.locals.stats = stats; + + next(); + }); + } + + this.previewBackend.getImage(namedMapProvider, format, width, height, bounds, (err, image, headers, stats) => { + if (err) { + return next(err); + } + + res.locals.image = image; + res.locals.headers = headers; + res.locals.stats = stats; + + next(); + }); + }.bind(this); }; function templateZoomCenter(view) { From f13b45862d7acb0f2c89711b593792a8fec5cbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sat, 30 Dec 2017 16:04:24 +0100 Subject: [PATCH 05/15] Move incrementMapViews to a middlewares --- lib/cartodb/controllers/named_maps.js | 72 ++++++++++++++------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index c285b25f..07ee91da 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -42,7 +42,8 @@ NamedMapsController.prototype.register = function(app) { this.prepareLayerFilterFromPreviewLayers(), this.getStaticImageOptions(), this.getImage(), - this.staticMap.bind(this) + this.incrementMapViews(), + this.handleImage() ); }; @@ -163,39 +164,6 @@ NamedMapsController.prototype.tile = function(req, res, next) { }); }; -NamedMapsController.prototype.staticMap = function(req, res, next) { - var self = this; - - var cdbUser = res.locals.user; - - const { namedMapProvider, image, headers, stats } = res.locals; - - step( - function incrementMapViews() { - var next = this; - namedMapProvider.getMapConfig(function(mapConfigErr, mapConfig) { - self.metadataBackend.incMapviewCount(cdbUser, mapConfig.obj().stat_tag, function(sErr) { - if (sErr) { - global.logger.log("ERROR: failed to increment mapview count for user '%s': %s", cdbUser, sErr); - } - - next(null, image, headers, stats); - }); - }); - }, - function handleImage(err, image, headers, stats) { - req.profiler.done('render-' + res.locals.format); - req.profiler.add(stats || {}); - - if (err) { - err.label = 'STATIC_VIZ_MAP'; - next(err); - } else { - self.sendResponse(req, res, image, headers, namedMapProvider); - } - } - ); -}; var DEFAULT_ZOOM_CENTER = { zoom: 1, @@ -343,6 +311,42 @@ NamedMapsController.prototype.getImage = function () { }.bind(this); }; +const incrementMapViewsError = ctx => `ERROR: failed to increment mapview count for user '${ctx.user}': ${ctx.err}`; + +NamedMapsController.prototype.incrementMapViews = function () { + return function incrementMapViewsMiddleware(req, res, next) { + const { user, namedMapProvider } = res.locals; + + namedMapProvider.getMapConfig((err, mapConfig) => { + if (err) { + global.logger.log(incrementMapViewsError({ user, err })); + return next(); + } + + const statTag = mapConfig.obj().stat_tag; + + this.metadataBackend.incMapviewCount(user, statTag, (err) => { + if (err) { + global.logger.log(incrementMapViewsError({ user, err })); + } + + next(); + }); + }); + }.bind(this); +}; + +NamedMapsController.prototype.handleImage = function () { + return function handleImageMiddleware (req, res) { + const { namedMapProvider, image, headers, stats = {}, format } = res.locals; + + req.profiler.done('render-' + format); + req.profiler.add(stats); + + this.sendResponse(req, res, image, headers, namedMapProvider); + }.bind(this); +}; + function templateZoomCenter(view) { if (!_.isUndefined(view.zoom) && view.center) { return { From 8a023e3d2f6f0a5c7ac5b80060897a0a59c44c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sat, 30 Dec 2017 16:08:46 +0100 Subject: [PATCH 06/15] Keep error label --- lib/cartodb/controllers/named_maps.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 07ee91da..9ab6a78b 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -38,16 +38,16 @@ NamedMapsController.prototype.register = function(app) { userMiddleware, allowQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), this.prepareContext, - this.getNamedMapProvider(), - this.prepareLayerFilterFromPreviewLayers(), + this.getNamedMapProvider('STATIC_VIZ_MAP'), + this.prepareLayerFilterFromPreviewLayers('STATIC_VIZ_MAP'), this.getStaticImageOptions(), - this.getImage(), + this.getImage('STATIC_VIZ_MAP'), this.incrementMapViews(), this.handleImage() ); }; -NamedMapsController.prototype.getNamedMapProvider = function () { +NamedMapsController.prototype.getNamedMapProvider = function (label) { return function getNamedMapProviderMiddleware (req, res, next) { const { user } = res.locals; const { config, auth_token } = req.query; @@ -55,6 +55,7 @@ NamedMapsController.prototype.getNamedMapProvider = function () { this.namedMapProviderCache.get(user, template_id, config, auth_token, res.locals, (err, namedMapProvider) => { if (err) { + err.label = label; return next(err); } @@ -65,7 +66,7 @@ NamedMapsController.prototype.getNamedMapProvider = function () { }.bind(this); }; -NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function () { +NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function (label) { return function prepareLayerFilterFromPreviewLayersMiddleware (req, res, next) { const { user, namedMapProvider } = res.locals; const { template_id } = req.params; @@ -73,6 +74,7 @@ NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function () namedMapProvider.getTemplate((err, template) => { if (err) { + err.label = label; return next(err); } @@ -99,6 +101,7 @@ NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function () // recreates the provider this.namedMapProviderCache.get(user, template_id, config, auth_token, res.locals, (err, provider) => { if (err) { + err.label = label; return next(err); } @@ -266,7 +269,7 @@ NamedMapsController.prototype.getStaticImageOptions = function () { }.bind(this); }; -NamedMapsController.prototype.getImage = function () { +NamedMapsController.prototype.getImage = function (label) { return function getImageMiddleware (req, res, next) { const { imageOpts, namedMapProvider } = res.locals; const { zoom, center, bounds } = imageOpts; @@ -286,6 +289,7 @@ NamedMapsController.prototype.getImage = function () { return this.previewBackend.getImage(namedMapProvider, format, width, height, zoom, center, (err, image, headers, stats) => { if (err) { + err.label = label; return next(err); } @@ -299,6 +303,7 @@ NamedMapsController.prototype.getImage = function () { this.previewBackend.getImage(namedMapProvider, format, width, height, bounds, (err, image, headers, stats) => { if (err) { + err.label = label; return next(err); } From 543d257a20add1660c84507e82c83773344cf226 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sat, 30 Dec 2017 17:18:12 +0100 Subject: [PATCH 07/15] Move sendResponse to a middleware --- lib/cartodb/controllers/named_maps.js | 131 ++++++++++++++------------ 1 file changed, 69 insertions(+), 62 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 9ab6a78b..8b828f02 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -28,7 +28,9 @@ NamedMapsController.prototype.register = function(app) { userMiddleware, this.prepareContext, this.getNamedMapProvider(), - this.tile.bind(this), + this.getTile(), + this.getAffectedTablesAndLastUpdatedTime(), + this.respond(), vectorError() ); @@ -43,7 +45,8 @@ NamedMapsController.prototype.register = function(app) { this.getStaticImageOptions(), this.getImage('STATIC_VIZ_MAP'), this.incrementMapViews(), - this.handleImage() + this.getAffectedTablesAndLastUpdatedTime(), + this.respond() ); }; @@ -113,61 +116,27 @@ NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function (la }.bind(this); }; -NamedMapsController.prototype.sendResponse = function(req, res, body, headers, namedMapProvider) { - 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'); +NamedMapsController.prototype.getTile = function () { + return function getTileMiddleware (req, res, next) { + const { namedMapProvider } = res.locals; - var self = this; + this.tileBackend.getTile(namedMapProvider, req.params, (err, tile, headers, stats) => { + req.profiler.add(stats); - step( - function getAffectedTablesAndLastUpdatedTime() { - namedMapProvider.getAffectedTablesAndLastUpdatedTime(this); - }, - function sendResponse(err, result) { - req.profiler.done('affectedTables'); if (err) { - global.logger.log('ERROR generating cache channel: ' + err); + err.label = 'NAMED_MAP_TILE'; + return next(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.locals.body = tile; + res.locals.headers = headers; + res.locals.stats = stats; - res.set('X-Cache-Channel', result.getCacheChannel()); - if (result.tables.length > 0) { - self.surrogateKeysCache.tag(res, result); - } - } - res.status(200); - res.send(body); - } - ); + next(); + }); + }.bind(this); }; -NamedMapsController.prototype.tile = function(req, res, next) { - const { namedMapProvider } = res.locals; - - this.tileBackend.getTile(namedMapProvider, req.params, (err, tile, headers, stats) => { - req.profiler.add(stats); - - if (err) { - err.label = 'NAMED_MAP_TILE'; - return next(err); - } - - this.sendResponse(req, res, tile, headers, namedMapProvider); - }); -}; - - var DEFAULT_ZOOM_CENTER = { zoom: 1, center: { @@ -293,7 +262,7 @@ NamedMapsController.prototype.getImage = function (label) { return next(err); } - res.locals.image = image; + res.locals.body = image; res.locals.headers = headers; res.locals.stats = stats; @@ -307,7 +276,7 @@ NamedMapsController.prototype.getImage = function (label) { return next(err); } - res.locals.image = image; + res.locals.body = image; res.locals.headers = headers; res.locals.stats = stats; @@ -341,17 +310,6 @@ NamedMapsController.prototype.incrementMapViews = function () { }.bind(this); }; -NamedMapsController.prototype.handleImage = function () { - return function handleImageMiddleware (req, res) { - const { namedMapProvider, image, headers, stats = {}, format } = res.locals; - - req.profiler.done('render-' + format); - req.profiler.add(stats); - - this.sendResponse(req, res, image, headers, namedMapProvider); - }.bind(this); -}; - function templateZoomCenter(view) { if (!_.isUndefined(view.zoom) && view.center) { return { @@ -382,3 +340,52 @@ function templateBounds(view) { } return false; } + +NamedMapsController.prototype.getAffectedTablesAndLastUpdatedTime = function () { + return function getAffectedTablesAndLastUpdatedTimeMiddleware (req, res, next) { + const { namedMapProvider, headers, user } = res.locals; + + this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(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'); + + namedMapProvider.getAffectedTablesAndLastUpdatedTime((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); + } + } + + next(); + }); + }.bind(this); +}; + +NamedMapsController.prototype.respond = function () { + return function respondMiddleware (req, res) { + const { body, stats = {}, format } = res.locals; + + req.profiler.done('render-' + format); + req.profiler.add(stats); + + res.status(200); + res.send(body); + }; +}; From 80e4306fbc6f54a1cc91bed758194f6b0bc6c739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sat, 30 Dec 2017 18:02:48 +0100 Subject: [PATCH 08/15] Remove step and assert dependencies --- lib/cartodb/controllers/named_maps.js | 99 ++++++++++++--------------- 1 file changed, 44 insertions(+), 55 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 8b828f02..d042887f 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -1,5 +1,3 @@ -var step = require('step'); -var assert = require('assert'); var _ = require('underscore'); var NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); @@ -151,8 +149,6 @@ function numMapper(n) { NamedMapsController.prototype.getStaticImageOptions = function () { return function getStaticImageOptionsMiddleware(req, res, next) { - var self = this; - const { user, namedMapProvider, zoom, lon, lat, bbox } = res.locals; if ([zoom, lon, lat].map(numMapper).every(Number.isFinite)) { @@ -183,58 +179,51 @@ NamedMapsController.prototype.getStaticImageOptions = function () { } } - step( - function getTemplate() { - namedMapProvider.getTemplate(this); - }, - function handleTemplateView(err, template) { - assert.ifError(err); - - if (template.view) { - var zoomCenter = templateZoomCenter(template.view); - if (zoomCenter) { - if (Number.isFinite(+zoom)) { - zoomCenter.zoom = +zoom; - } - return zoomCenter; - } - - var bounds = templateBounds(template.view); - if (bounds) { - return bounds; - } - } - - return false; - }, - function estimateBoundsIfNoImageOpts(err, imageOpts) { - if (imageOpts) { - return imageOpts; - } - - var _next = this; - namedMapProvider.getAffectedTablesAndLastUpdatedTime(function(err, affectedTablesAndLastUpdate) { - if (err) { - return _next(null); - } - - var affectedTables = affectedTablesAndLastUpdate.tables || []; - - if (affectedTables.length === 0) { - return _next(null); - } - - self.tablesExtentApi.getBounds(user, affectedTables, function (err, result) { - return _next(null, result); - }); - }); - - }, - function returnCallback(err, imageOpts) { - res.locals.imageOpts = imageOpts || DEFAULT_ZOOM_CENTER; - return next(); + namedMapProvider.getTemplate((err, template) => { + if (err) { + return next(err); } - ); + + if (template.view) { + var zoomCenter = templateZoomCenter(template.view); + if (zoomCenter) { + if (Number.isFinite(+zoom)) { + zoomCenter.zoom = +zoom; + } + res.locals.imageOpts = zoomCenter; + return next(); + } + + var bounds = templateBounds(template.view); + if (bounds) { + res.locals.imageOpts = bounds; + return next(); + } + } + + res.locals.imageOpts = DEFAULT_ZOOM_CENTER; + + namedMapProvider.getAffectedTablesAndLastUpdatedTime((err, affectedTablesAndLastUpdate) => { + if (err) { + return next(); + } + + var affectedTables = affectedTablesAndLastUpdate.tables || []; + + if (affectedTables.length === 0) { + return next(); + } + + this.tablesExtentApi.getBounds(user, affectedTables, (err, bounds) => { + if (err) { + return next(); + } + + res.locals.imageOpts = bounds; + return next(); + }); + }); + }); }.bind(this); }; From 80c4207c74c4d402d5615bd192f941e4aa6b1fff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sat, 30 Dec 2017 18:18:37 +0100 Subject: [PATCH 09/15] Remove underscore dependencie --- lib/cartodb/controllers/named_maps.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index d042887f..d6ba0d1b 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -1,4 +1,3 @@ -var _ = require('underscore'); var NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); var cors = require('../middleware/cors'); @@ -243,7 +242,7 @@ NamedMapsController.prototype.getImage = function (label) { res.locals.format = 'png'; res.locals.layer = res.locals.layer || 'all'; - if (!_.isUndefined(zoom) && center) { + if (zoom !== undefined && center) { return this.previewBackend.getImage(namedMapProvider, format, width, height, zoom, center, (err, image, headers, stats) => { if (err) { @@ -300,7 +299,7 @@ NamedMapsController.prototype.incrementMapViews = function () { }; function templateZoomCenter(view) { - if (!_.isUndefined(view.zoom) && view.center) { + if (view.zoom !== undefined && view.center) { return { zoom: view.zoom, center: view.center @@ -311,9 +310,10 @@ function templateZoomCenter(view) { function templateBounds(view) { if (view.bounds) { - var hasAllBounds = _.every(['west', 'south', 'east', 'north'], function(prop) { + var hasAllBounds = ['west', 'south', 'east', 'north'].every(function(prop) { return Number.isFinite(view.bounds[prop]); }); + if (hasAllBounds) { return { bounds: { From e3bdeec8ca28eab533f1813e68e8d3ebbdb41ad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 1 Jan 2018 16:21:22 +0100 Subject: [PATCH 10/15] Simplify middleware --- lib/cartodb/controllers/named_maps.js | 120 +++++++++++++++++++------- 1 file changed, 91 insertions(+), 29 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index d6ba0d1b..ebdabb8f 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -25,8 +25,13 @@ NamedMapsController.prototype.register = function(app) { userMiddleware, this.prepareContext, this.getNamedMapProvider(), + this.getAffectedTables(), this.getTile(), - this.getAffectedTablesAndLastUpdatedTime(), + this.setSurrogateKey(), + this.setCacheChannelHeader(), + this.setLastModifiedHeader(), + this.setCacheControlHeader(), + this.setContentTypeHeader(), this.respond(), vectorError() ); @@ -38,11 +43,16 @@ NamedMapsController.prototype.register = function(app) { allowQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), this.prepareContext, this.getNamedMapProvider('STATIC_VIZ_MAP'), + this.getAffectedTables(), this.prepareLayerFilterFromPreviewLayers('STATIC_VIZ_MAP'), this.getStaticImageOptions(), this.getImage('STATIC_VIZ_MAP'), this.incrementMapViews(), - this.getAffectedTablesAndLastUpdatedTime(), + this.setSurrogateKey(), + this.setCacheChannelHeader(), + this.setLastModifiedHeader(), + this.setCacheControlHeader(), + this.setContentTypeHeader(), this.respond() ); }; @@ -66,6 +76,24 @@ NamedMapsController.prototype.getNamedMapProvider = function (label) { }.bind(this); }; +NamedMapsController.prototype.getAffectedTables = function () { + return function getAffectedTables (req, res, next) { + const { namedMapProvider } = res.locals; + + namedMapProvider.getAffectedTablesAndLastUpdatedTime((err, affectedTablesAndLastUpdate) => { + req.profiler.done('affectedTables'); + + if (err) { + return next(err); + } + + res.locals.affectedTablesAndLastUpdate = affectedTablesAndLastUpdate; + + next(); + }); + }.bind(this); +}; + NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function (label) { return function prepareLayerFilterFromPreviewLayersMiddleware (req, res, next) { const { user, namedMapProvider } = res.locals; @@ -330,41 +358,75 @@ function templateBounds(view) { return false; } -NamedMapsController.prototype.getAffectedTablesAndLastUpdatedTime = function () { - return function getAffectedTablesAndLastUpdatedTimeMiddleware (req, res, next) { - const { namedMapProvider, headers, user } = res.locals; +NamedMapsController.prototype.setCacheChannelHeader = function () { + return function setCacheChannelHeaderMiddleware (req, res, next) { + const { affectedTablesAndLastUpdate } = res.locals; + + if (!affectedTablesAndLastUpdate || !!affectedTablesAndLastUpdate.tables) { + res.set('X-Cache-Channel', affectedTablesAndLastUpdate.getCacheChannel()); + } + + next(); + }; +}; + +NamedMapsController.prototype.setSurrogateKey = function () { + return function setSurrogateKeyMiddleware(req, res, next) { + const { user, namedMapProvider, affectedTablesAndLastUpdate } = res.locals; this.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, namedMapProvider.getTemplateName())); - res.set('Content-Type', headers['content-type'] || headers['Content-Type'] || 'image/png'); + if (!affectedTablesAndLastUpdate || !!affectedTablesAndLastUpdate.tables) { + if (affectedTablesAndLastUpdate.tables.length > 0) { + this.surrogateKeysCache.tag(res, affectedTablesAndLastUpdate); + } + } + + next(); + }.bind(this); +}; + +NamedMapsController.prototype.setLastModifiedHeader = function () { + return function setLastModifiedHeaderMiddleware(req, res, next) { + const { affectedTablesAndLastUpdate } = res.locals; + + if (!affectedTablesAndLastUpdate || !!affectedTablesAndLastUpdate.tables) { + var lastModifiedDate; + if (Number.isFinite(affectedTablesAndLastUpdate.lastUpdatedTime)) { + lastModifiedDate = new Date(affectedTablesAndLastUpdate.getLastUpdatedAt()); + } else { + lastModifiedDate = new Date(); + } + + res.set('Last-Modified', lastModifiedDate.toUTCString()); + } + + next(); + }; + }; + +NamedMapsController.prototype.setCacheControlHeader = function () { + return function setCacheControlHeaderMiddleware(req, res, next) { + const { affectedTablesAndLastUpdate } = res.locals; + res.set('Cache-Control', 'public,max-age=7200,must-revalidate'); - namedMapProvider.getAffectedTablesAndLastUpdatedTime((err, result) => { - req.profiler.done('affectedTables'); - if (err) { - global.logger.log('ERROR generating cache channel: ' + err); - } + if (!affectedTablesAndLastUpdate || !!affectedTablesAndLastUpdate.tables) { + // we increase cache control as we can invalidate it + res.set('Cache-Control', 'public,max-age=31536000'); + } - if (!result || !!result.tables) { - // we increase cache control as we can invalidate it - res.set('Cache-Control', 'public,max-age=31536000'); + next(); + }; + }; - var lastModifiedDate; - if (Number.isFinite(result.lastUpdatedTime)) { - lastModifiedDate = new Date(result.getLastUpdatedAt()); - } else { - lastModifiedDate = new Date(); - } - res.set('Last-Modified', lastModifiedDate.toUTCString()); +NamedMapsController.prototype.setContentTypeHeader = function () { + return function setContentTypeHeaderMiddleware(req, res, next) { + const { headers = {} } = res.locals; - res.set('X-Cache-Channel', result.getCacheChannel()); - if (result.tables.length > 0) { - this.surrogateKeysCache.tag(res, result); - } - } + res.set('Content-Type', headers['content-type'] || headers['Content-Type'] || 'image/png'); - next(); - }); - }.bind(this); + next(); + }; }; NamedMapsController.prototype.respond = function () { From feae766e62eefc9370dfd07326d2b69cac105cfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 1 Jan 2018 16:54:35 +0100 Subject: [PATCH 11/15] Create middleware to fetch named map template --- lib/cartodb/controllers/named_maps.js | 172 +++++++++++++------------- 1 file changed, 88 insertions(+), 84 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index ebdabb8f..9eb40ee4 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -1,9 +1,20 @@ -var NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); +const NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); +const cors = require('../middleware/cors'); +const userMiddleware = require('../middleware/user'); +const allowQueryParams = require('../middleware/allow-query-params'); +const vectorError = require('../middleware/vector-error'); -var cors = require('../middleware/cors'); -var userMiddleware = require('../middleware/user'); -var allowQueryParams = require('../middleware/allow-query-params'); -var vectorError = require('../middleware/vector-error'); +const DEFAULT_ZOOM_CENTER = { + zoom: 1, + center: { + lng: 0, + lat: 0 + } +}; + +function numMapper(n) { + return +n; +} function NamedMapsController(prepareContext, namedMapProviderCache, tileBackend, previewBackend, surrogateKeysCache, tablesExtentApi, metadataBackend) { @@ -44,6 +55,7 @@ NamedMapsController.prototype.register = function(app) { this.prepareContext, this.getNamedMapProvider('STATIC_VIZ_MAP'), this.getAffectedTables(), + this.getTemplate('STATIC_VIZ_MAP'), this.prepareLayerFilterFromPreviewLayers('STATIC_VIZ_MAP'), this.getStaticImageOptions(), this.getImage('STATIC_VIZ_MAP'), @@ -94,11 +106,9 @@ NamedMapsController.prototype.getAffectedTables = function () { }.bind(this); }; -NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function (label) { - return function prepareLayerFilterFromPreviewLayersMiddleware (req, res, next) { - const { user, namedMapProvider } = res.locals; - const { template_id } = req.params; - const { config, auth_token } = req.query; +NamedMapsController.prototype.getTemplate = function (label) { + return function getTemplateMiddleware (req, res, next) { + const { namedMapProvider } = res.locals; namedMapProvider.getTemplate((err, template) => { if (err) { @@ -106,37 +116,49 @@ NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function (la return next(err); } - if (!template || !template.view || !template.view.preview_layers) { - return next(); + res.locals.template = template; + + next(); + }); + }; +}; + +NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function (label) { + return function prepareLayerFilterFromPreviewLayersMiddleware (req, res, next) { + const { user, template } = res.locals; + const { template_id } = req.params; + const { config, auth_token } = req.query; + + if (!template || !template.view || !template.view.preview_layers) { + return next(); + } + + var previewLayers = template.view.preview_layers; + var layerVisibilityFilter = []; + + template.layergroup.layers.forEach(function (layer, index) { + if (previewLayers[''+index] !== false && previewLayers[layer.id] !== false) { + layerVisibilityFilter.push(''+index); + } + }); + + if (!layerVisibilityFilter.length) { + return next(); + } + + // overwrites 'all' default filter + res.locals.layer = layerVisibilityFilter.join(','); + + // recreates the provider + this.namedMapProviderCache.get(user, template_id, config, auth_token, res.locals, (err, provider) => { + if (err) { + err.label = label; + return next(err); } - var previewLayers = template.view.preview_layers; - var layerVisibilityFilter = []; + res.locals.namedMapProvider = provider; - template.layergroup.layers.forEach(function (layer, index) { - if (previewLayers[''+index] !== false && previewLayers[layer.id] !== false) { - layerVisibilityFilter.push(''+index); - } - }); - - if (!layerVisibilityFilter.length) { - return next(); - } - - // overwrites 'all' default filter - res.locals.layer = layerVisibilityFilter.join(','); - - // recreates the provider - this.namedMapProviderCache.get(user, template_id, config, auth_token, res.locals, (err, provider) => { - if (err) { - err.label = label; - return next(err); - } - - res.locals.namedMapProvider = provider; - - next(); - }); + next(); }); }.bind(this); }; @@ -162,21 +184,9 @@ NamedMapsController.prototype.getTile = function () { }.bind(this); }; -var DEFAULT_ZOOM_CENTER = { - zoom: 1, - center: { - lng: 0, - lat: 0 - } -}; - -function numMapper(n) { - return +n; -} - NamedMapsController.prototype.getStaticImageOptions = function () { return function getStaticImageOptionsMiddleware(req, res, next) { - const { user, namedMapProvider, zoom, lon, lat, bbox } = res.locals; + const { user, namedMapProvider, template, zoom, lon, lat, bbox } = res.locals; if ([zoom, lon, lat].map(numMapper).every(Number.isFinite)) { res.locals.imageOpts = { @@ -206,49 +216,43 @@ NamedMapsController.prototype.getStaticImageOptions = function () { } } - namedMapProvider.getTemplate((err, template) => { + if (template.view) { + var zoomCenter = templateZoomCenter(template.view); + if (zoomCenter) { + if (Number.isFinite(+zoom)) { + zoomCenter.zoom = +zoom; + } + res.locals.imageOpts = zoomCenter; + return next(); + } + + var bounds = templateBounds(template.view); + if (bounds) { + res.locals.imageOpts = bounds; + return next(); + } + } + + res.locals.imageOpts = DEFAULT_ZOOM_CENTER; + + namedMapProvider.getAffectedTablesAndLastUpdatedTime((err, affectedTablesAndLastUpdate) => { if (err) { - return next(err); + return next(); } - if (template.view) { - var zoomCenter = templateZoomCenter(template.view); - if (zoomCenter) { - if (Number.isFinite(+zoom)) { - zoomCenter.zoom = +zoom; - } - res.locals.imageOpts = zoomCenter; - return next(); - } + var affectedTables = affectedTablesAndLastUpdate.tables || []; - var bounds = templateBounds(template.view); - if (bounds) { - res.locals.imageOpts = bounds; - return next(); - } + if (affectedTables.length === 0) { + return next(); } - res.locals.imageOpts = DEFAULT_ZOOM_CENTER; - - namedMapProvider.getAffectedTablesAndLastUpdatedTime((err, affectedTablesAndLastUpdate) => { + this.tablesExtentApi.getBounds(user, affectedTables, (err, bounds) => { if (err) { return next(); } - var affectedTables = affectedTablesAndLastUpdate.tables || []; - - if (affectedTables.length === 0) { - return next(); - } - - this.tablesExtentApi.getBounds(user, affectedTables, (err, bounds) => { - if (err) { - return next(); - } - - res.locals.imageOpts = bounds; - return next(); - }); + res.locals.imageOpts = bounds; + return next(); }); }); }.bind(this); From 41e65a9633fa2b954d29fef8de507e9b2084bba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 1 Jan 2018 18:06:56 +0100 Subject: [PATCH 12/15] Remove max cyclomatic complexity --- lib/cartodb/controllers/named_maps.js | 113 ++++++++++++++++---------- 1 file changed, 71 insertions(+), 42 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 9eb40ee4..b878890d 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -186,53 +186,15 @@ NamedMapsController.prototype.getTile = function () { NamedMapsController.prototype.getStaticImageOptions = function () { return function getStaticImageOptionsMiddleware(req, res, next) { - const { user, namedMapProvider, template, zoom, lon, lat, bbox } = res.locals; + const { user, namedMapProvider, template } = res.locals; - if ([zoom, lon, lat].map(numMapper).every(Number.isFinite)) { - res.locals.imageOpts = { - zoom: zoom, - center: { - lng: lon, - lat: lat - } - }; + const imageOpts = getImageOptions(res.locals, template); + if (imageOpts) { + res.locals.imageOpts = imageOpts; return next(); } - if (bbox) { - var _bbox = bbox.split(',').map(numMapper); - if (_bbox.length === 4 && _bbox.every(Number.isFinite)) { - res.locals.imageOpts = { - bounds: { - west: _bbox[0], - south: _bbox[1], - east: _bbox[2], - north: _bbox[3] - } - }; - - return next(); - } - } - - if (template.view) { - var zoomCenter = templateZoomCenter(template.view); - if (zoomCenter) { - if (Number.isFinite(+zoom)) { - zoomCenter.zoom = +zoom; - } - res.locals.imageOpts = zoomCenter; - return next(); - } - - var bounds = templateBounds(template.view); - if (bounds) { - res.locals.imageOpts = bounds; - return next(); - } - } - res.locals.imageOpts = DEFAULT_ZOOM_CENTER; namedMapProvider.getAffectedTablesAndLastUpdatedTime((err, affectedTablesAndLastUpdate) => { @@ -252,12 +214,79 @@ NamedMapsController.prototype.getStaticImageOptions = function () { } res.locals.imageOpts = bounds; + return next(); }); }); }.bind(this); }; +function getImageOptions (params, template) { + const { zoom, lon, lat, bbox } = params; + + let imageOpts = getImageOptionsFromCoordinates(zoom, lon, lat); + if (imageOpts) { + return imageOpts; + } + + imageOpts = getImageOptionsFromBoundingBox(bbox); + if (imageOpts) { + return imageOpts; + } + + imageOpts = getImageOptionsFromTemplate(template, zoom); + if (imageOpts) { + return imageOpts; + } +} + +function getImageOptionsFromCoordinates (zoom, lon, lat) { + if ([zoom, lon, lat].map(numMapper).every(Number.isFinite)) { + return { + zoom: zoom, + center: { + lng: lon, + lat: lat + } + }; + } +} + + +function getImageOptionsFromTemplate (template, zoom) { + if (template.view) { + var zoomCenter = templateZoomCenter(template.view); + if (zoomCenter) { + if (Number.isFinite(+zoom)) { + zoomCenter.zoom = +zoom; + } + + return zoomCenter; + } + + var bounds = templateBounds(template.view); + if (bounds) { + return bounds; + } + } +} + +function getImageOptionsFromBoundingBox (bbox) { + if (bbox) { + var _bbox = bbox.split(',').map(numMapper); + if (_bbox.length === 4 && _bbox.every(Number.isFinite)) { + return { + bounds: { + west: _bbox[0], + south: _bbox[1], + east: _bbox[2], + north: _bbox[3] + } + }; + } + } +} + NamedMapsController.prototype.getImage = function (label) { return function getImageMiddleware (req, res, next) { const { imageOpts, namedMapProvider } = res.locals; From 49c97e2cf279b41bb6429ef3dfe818c40bff68c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 2 Jan 2018 10:56:45 +0100 Subject: [PATCH 13/15] Use default argument --- lib/cartodb/controllers/named_maps.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index b878890d..03008261 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -271,19 +271,18 @@ function getImageOptionsFromTemplate (template, zoom) { } } -function getImageOptionsFromBoundingBox (bbox) { - if (bbox) { - var _bbox = bbox.split(',').map(numMapper); - if (_bbox.length === 4 && _bbox.every(Number.isFinite)) { - return { - bounds: { - west: _bbox[0], - south: _bbox[1], - east: _bbox[2], - north: _bbox[3] - } - }; - } +function getImageOptionsFromBoundingBox (bbox = '') { + var _bbox = bbox.split(',').map(numMapper); + + if (_bbox.length === 4 && _bbox.every(Number.isFinite)) { + return { + bounds: { + west: _bbox[0], + south: _bbox[1], + east: _bbox[2], + north: _bbox[3] + } + }; } } From d908ffdbca29cd18d35841c6b45fa10b6453823b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 9 Jan 2018 11:17:07 +0100 Subject: [PATCH 14/15] Don't use arrow functions when there is no needed --- lib/cartodb/controllers/named_maps.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 03008261..af281a02 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -333,7 +333,9 @@ NamedMapsController.prototype.getImage = function (label) { }.bind(this); }; -const incrementMapViewsError = ctx => `ERROR: failed to increment mapview count for user '${ctx.user}': ${ctx.err}`; +function incrementMapViewsError (ctx) { + return `ERROR: failed to increment mapview count for user '${ctx.user}': ${ctx.err}`; +} NamedMapsController.prototype.incrementMapViews = function () { return function incrementMapViewsMiddleware(req, res, next) { From 183c8291bc67a0e1758ec3a23480429d71b8255a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 9 Jan 2018 11:20:20 +0100 Subject: [PATCH 15/15] Use arrow functions when it applies --- lib/cartodb/controllers/named_maps.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index af281a02..f668d638 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -136,7 +136,7 @@ NamedMapsController.prototype.prepareLayerFilterFromPreviewLayers = function (la var previewLayers = template.view.preview_layers; var layerVisibilityFilter = []; - template.layergroup.layers.forEach(function (layer, index) { + template.layergroup.layers.forEach((layer, index) => { if (previewLayers[''+index] !== false && previewLayers[layer.id] !== false) { layerVisibilityFilter.push(''+index); } @@ -372,9 +372,7 @@ function templateZoomCenter(view) { function templateBounds(view) { if (view.bounds) { - var hasAllBounds = ['west', 'south', 'east', 'north'].every(function(prop) { - return Number.isFinite(view.bounds[prop]); - }); + var hasAllBounds = ['west', 'south', 'east', 'north'].every(prop => Number.isFinite(view.bounds[prop])); if (hasAllBounds) { return {