From 5e43a7145a3efa904b1c737819e8ace970bd28ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 5 Mar 2018 17:44:04 +0100 Subject: [PATCH 01/44] Middlewarify dataview endpoint --- lib/cartodb/controllers/layergroup.js | 45 ++++++++++++++------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index d9251dd0..20046a76 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -124,7 +124,8 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - this.dataview.bind(this) + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.getDataview() ); app.get( @@ -133,7 +134,8 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - this.dataview.bind(this) + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.getDataview() ); app.get( @@ -186,32 +188,31 @@ LayergroupController.prototype.analysisNodeStatus = function(req, res, next) { ); }; -LayergroupController.prototype.dataview = function(req, res, next) { - var self = this; +LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore, userLimitsApi) { + return function getMapStoreMapConfigProviderMiddleware (req, res, next) { + const { user } = res.locals; - step( - function retrieveDataview() { - var mapConfigProvider = new MapStoreMapConfigProvider( - self.mapStore, res.locals.user, self.userLimitsApi, res.locals - ); - self.dataviewBackend.getDataview( - mapConfigProvider, - res.locals.user, - res.locals, - this - ); - }, - function finish(err, dataview, stats) { + res.locals.mapConfigProvider = new MapStoreMapConfigProvider(mapStore, user, userLimitsApi, res.locals); + + next(); + }; +}; + +LayergroupController.prototype.getDataview = function () { + return function getDataviewMiddleware (req, res, next) { + const { user, mapConfigProvider } = res.locals; + + this.dataviewBackend.getDataview(mapConfigProvider, user, res.locals, (err, dataview, stats) => { req.profiler.add(stats || {}); if (err) { err.label = 'GET DATAVIEW'; - next(err); - } else { - self.sendResponse(req, res, dataview, 200); + return next(err); } - } - ); + + this.sendResponse(req, res, dataview, 200); + }); + }.bind(this); }; LayergroupController.prototype.dataviewSearch = function(req, res, next) { From acb9ce33b1c4df1cc96c4df02d75657e5cc42763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 5 Mar 2018 18:04:50 +0100 Subject: [PATCH 02/44] Pass dataview-backend as middleware option --- lib/cartodb/controllers/layergroup.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 20046a76..279bd5c7 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -125,7 +125,7 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.getDataview() + this.getDataview(this.dataviewBackend) ); app.get( @@ -135,7 +135,7 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.getDataview() + this.getDataview(this.dataviewBackend) ); app.get( @@ -198,11 +198,11 @@ LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore }; }; -LayergroupController.prototype.getDataview = function () { +LayergroupController.prototype.getDataview = function (dataviewBackend) { return function getDataviewMiddleware (req, res, next) { const { user, mapConfigProvider } = res.locals; - this.dataviewBackend.getDataview(mapConfigProvider, user, res.locals, (err, dataview, stats) => { + dataviewBackend.getDataview(mapConfigProvider, user, res.locals, (err, dataview, stats) => { req.profiler.add(stats || {}); if (err) { From 40712a2e62a22d38e91cf2aeebf8d82cddabe66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 5 Mar 2018 18:05:42 +0100 Subject: [PATCH 03/44] Middlewarify search dataview endpoint --- lib/cartodb/controllers/layergroup.js | 29 +++++++++++---------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 279bd5c7..a7c6ad2d 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -144,7 +144,8 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - this.dataviewSearch.bind(this) + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.dataviewSearch(this.dataviewBackend) ); app.get( @@ -153,7 +154,8 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - this.dataviewSearch.bind(this) + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.dataviewSearch(this.dataviewBackend) ); app.get( @@ -215,28 +217,21 @@ LayergroupController.prototype.getDataview = function (dataviewBackend) { }.bind(this); }; -LayergroupController.prototype.dataviewSearch = function(req, res, next) { - var self = this; +LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { + return function dataviewSearchMiddlewarify (req, res, next) { + const { user, dataviewName, mapConfigProvider } = res.locals; - step( - function searchDataview() { - var mapConfigProvider = new MapStoreMapConfigProvider( - self.mapStore, res.locals.user, self.userLimitsApi, res.locals - ); - self.dataviewBackend.search(mapConfigProvider, res.locals.user, req.params.dataviewName, res.locals, this); - }, - function finish(err, searchResult, stats) { + dataviewBackend.search(mapConfigProvider, user, dataviewName, res.locals, (err, searchResult, stats) => { req.profiler.add(stats || {}); if (err) { err.label = 'GET DATAVIEW SEARCH'; - next(err); - } else { - self.sendResponse(req, res, searchResult, 200); + return next(err); } - } - ); + this.sendResponse(req, res, searchResult, 200); + }); + }.bind(this); }; LayergroupController.prototype.attributes = function(req, res, next) { From d8a4209768066da1dc348a72204ad46e0e3970c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 5 Mar 2018 18:13:19 +0100 Subject: [PATCH 04/44] Middlewarify analysis-node-status endpoint --- lib/cartodb/controllers/layergroup.js | 29 +++++++++++---------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index a7c6ad2d..502ba16b 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -163,31 +163,26 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.analysisNodeStatus.bind(this) + this.analysisNodeStatus(this.analysisStatusBackend) ); }; -LayergroupController.prototype.analysisNodeStatus = function(req, res, next) { - var self = this; - - step( - function retrieveNodeStatus() { - self.analysisStatusBackend.getNodeStatus(res.locals, this); - }, - function finish(err, nodeStatus, stats) { +LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBackend) { + return function analysisNodeStatusMiddleware(req, res, next) { + analysisStatusBackend.getNodeStatus(res.locals, (err, nodeStatus, stats) => { req.profiler.add(stats || {}); if (err) { err.label = 'GET NODE STATUS'; - next(err); - } else { - self.sendResponse(req, res, nodeStatus, 200, { - 'Cache-Control': 'public,max-age=5', - 'Last-Modified': new Date().toUTCString() - }); + return next(err); } - } - ); + + this.sendResponse(req, res, nodeStatus, 200, { + 'Cache-Control': 'public,max-age=5', + 'Last-Modified': new Date().toUTCString() + }); + }); + }.bind(this); }; LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore, userLimitsApi) { From ca56df5cfe44b919aba895b16c9a0a6e11ba3b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 5 Mar 2018 18:28:52 +0100 Subject: [PATCH 05/44] Middlewarify attributes endpoint --- lib/cartodb/controllers/layergroup.js | 28 +++++++++++---------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 502ba16b..3122113e 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -79,7 +79,8 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.attributes.bind(this) + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.attributes(this.attributesBackend) ); app.get( @@ -229,30 +230,23 @@ LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { }.bind(this); }; -LayergroupController.prototype.attributes = function(req, res, next) { - var self = this; +LayergroupController.prototype.attributes = function (attributesBackend) { + return function attributesMiddleware (req, res, next) { + req.profiler.start('windshaft.maplayer_attribute'); - req.profiler.start('windshaft.maplayer_attribute'); + const { mapConfigProvider } = res.locals; - step( - function retrieveFeatureAttributes() { - var mapConfigProvider = new MapStoreMapConfigProvider( - self.mapStore, res.locals.user, self.userLimitsApi, res.locals - ); - self.attributesBackend.getFeatureAttributes(mapConfigProvider, res.locals, false, this); - }, - function finish(err, tile, stats) { + attributesBackend.getFeatureAttributes(mapConfigProvider, res.locals, false, (err, tile, stats) => { req.profiler.add(stats || {}); if (err) { err.label = 'GET ATTRIBUTES'; - next(err); - } else { - self.sendResponse(req, res, tile, 200); + return next(err); } - } - ); + this.sendResponse(req, res, tile, 200); + }); + }.bind(this); }; // Gets a tile for a given token and set of tile ZXY coords. (OSM style) From 0f0cde1093d58a68e2e88f50f122ae7d653a9b82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 5 Mar 2018 19:26:26 +0100 Subject: [PATCH 06/44] Middlewarify static-api (bbox/center) endpoints --- lib/cartodb/controllers/layergroup.js | 104 +++++++++++++++----------- 1 file changed, 62 insertions(+), 42 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 3122113e..eb4a8266 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -89,7 +89,8 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(['layer']), this.prepareContext, - this.center.bind(this) + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.center(this.previewBackend) ); app.get( @@ -98,7 +99,8 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(['layer']), this.prepareContext, - this.bbox.bind(this) + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.bbox(this.previewBackend) ); // Undocumented/non-supported API endpoint methods. @@ -325,56 +327,74 @@ LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, t } }; -LayergroupController.prototype.bbox = function(req, res, next) { - this.staticMap(req, res, +req.params.width, +req.params.height, { - west: +req.params.west, - north: +req.params.north, - east: +req.params.east, - south: +req.params.south - }, null, next); -}; +LayergroupController.prototype.center = function (previewBackend) { + return function centerMiddleware (req, res, next) { + const width = +req.params.width; + const height = +req.params.height; + const zoom = +req.params.z; + const center = { + lng: +req.params.lng, + lat: +req.params.lat + }; -LayergroupController.prototype.center = function(req, res, next) { - this.staticMap(req, res, +req.params.width, +req.params.height, +req.params.z, { - lng: +req.params.lng, - lat: +req.params.lat - }, next); -}; + const format = req.params.format === 'jpg' ? 'jpeg' : 'png'; -LayergroupController.prototype.staticMap = function(req, res, width, height, zoom /* bounds */, center, next) { - 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'; + // 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'; - var self = this; + const { mapConfigProvider } = res.locals; - step( - function getImage() { - if (center) { - self.previewBackend.getImage( - new MapStoreMapConfigProvider(self.mapStore, res.locals.user, self.userLimitsApi, res.locals), - format, width, height, zoom, center, this); - } else { - self.previewBackend.getImage( - new MapStoreMapConfigProvider(self.mapStore, res.locals.user, self.userLimitsApi, res.locals), - format, width, height, zoom /* bounds */, this); - } - }, - function handleImage(err, image, headers, stats) { + previewBackend.getImage(mapConfigProvider, format, width, height, zoom, center, (err, image, headers, stats) => { req.profiler.done('render-' + format); req.profiler.add(stats || {}); if (err) { err.label = 'STATIC_MAP'; - next(err); - } else { - res.set('Content-Type', headers['Content-Type'] || 'image/' + format); - self.sendResponse(req, res, image, 200); + return next(err); } - } - ); + + res.set('Content-Type', headers['Content-Type'] || 'image/' + format); + this.sendResponse(req, res, image, 200); + }); + }.bind(this); +}; + +LayergroupController.prototype.bbox = function (previewBackend) { + return function bboxMiddleware (req, res, next) { + const width = +req.params.width; + const height = +req.params.height; + const bounds = { + west: +req.params.west, + north: +req.params.north, + east: +req.params.east, + south: +req.params.south + }; + + 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'; + + const { mapConfigProvider } = res.locals; + + previewBackend.getImage(mapConfigProvider, format, width, height, bounds, (err, image, headers, stats) => { + req.profiler.done('render-' + format); + req.profiler.add(stats || {}); + + if (err) { + err.label = 'STATIC_MAP'; + return next(err); + } + + res.set('Content-Type', headers['Content-Type'] || 'image/' + format); + + this.sendResponse(req, res, image, 200); + }); + }.bind(this); }; LayergroupController.prototype.sendResponse = function(req, res, body, status, headers) { From 95f3d5838322757675592d6faf8236f4195d3e30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 5 Mar 2018 19:33:46 +0100 Subject: [PATCH 07/44] Make jshint happy --- lib/cartodb/controllers/layergroup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index eb4a8266..0e995c8a 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -346,7 +346,7 @@ LayergroupController.prototype.center = function (previewBackend) { const { mapConfigProvider } = res.locals; - previewBackend.getImage(mapConfigProvider, format, width, height, zoom, center, (err, image, headers, stats) => { + previewBackend.getImage(mapConfigProvider, format, width, height, zoom, center,(err, image, headers, stats) => { req.profiler.done('render-' + format); req.profiler.add(stats || {}); From 9b4037079437a8f1e2e9fc17f95f04da5776be2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 12:44:17 +0100 Subject: [PATCH 08/44] Now that mapConfigProvider is linked to 'res.locals' do not pass the whole 'res.locals' to map-config-provider to avoid converting circular structure to JSON --- lib/cartodb/controllers/layergroup.js | 39 +++++++++++++++------------ 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 0e995c8a..7afd2346 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -83,13 +83,15 @@ LayergroupController.prototype.register = function(app) { this.attributes(this.attributesBackend) ); + const forcedFormat = 'png'; + app.get( app.base_url_mapconfig + '/static/center/:token/:z/:lat/:lng/:width/:height.:format', cors(), userMiddleware, allowQueryParams(['layer']), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), this.center(this.previewBackend) ); @@ -99,7 +101,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(['layer']), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), this.bbox(this.previewBackend) ); @@ -188,11 +190,27 @@ LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBack }.bind(this); }; -LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore, userLimitsApi) { +function getRequestParams(locals) { + const params = Object.assign({}, locals); + + delete params.mapConfigProvider; + delete params.allowedQueryParams; + + return params; +} + +LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore, userLimitsApi, forcedFormat = null) { return function getMapStoreMapConfigProviderMiddleware (req, res, next) { const { user } = res.locals; - res.locals.mapConfigProvider = new MapStoreMapConfigProvider(mapStore, user, userLimitsApi, res.locals); + const params = getRequestParams(res.locals); + + if (forcedFormat) { + params.format = forcedFormat; + params.layer = params.layer || 'all'; + } + + res.locals.mapConfigProvider = new MapStoreMapConfigProvider(mapStore, user, userLimitsApi, params); next(); }; @@ -338,12 +356,6 @@ LayergroupController.prototype.center = function (previewBackend) { }; 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'; - const { mapConfigProvider } = res.locals; previewBackend.getImage(mapConfigProvider, format, width, height, zoom, center,(err, image, headers, stats) => { @@ -371,14 +383,7 @@ LayergroupController.prototype.bbox = function (previewBackend) { east: +req.params.east, south: +req.params.south }; - 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'; - const { mapConfigProvider } = res.locals; previewBackend.getImage(mapConfigProvider, format, width, height, bounds, (err, image, headers, stats) => { From 585b5929aa760de772bdb6cb3a071a252f21985c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 16:19:53 +0100 Subject: [PATCH 09/44] Middlewarify tile and layer endpoints --- lib/cartodb/controllers/layergroup.js | 157 +++++++++++++++----------- 1 file changed, 91 insertions(+), 66 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 7afd2346..e07ba56b 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -51,7 +51,8 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.tile.bind(this), + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.tile(this.tileBackend), vectorError() ); @@ -60,7 +61,8 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.tile.bind(this), + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.tile(this.tileBackend), vectorError() ); @@ -70,7 +72,8 @@ LayergroupController.prototype.register = function(app) { userMiddleware, validateLayerRouteMiddleware, this.prepareContext, - this.layer.bind(this), + this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + this.layer(this.tileBackend), vectorError() ); @@ -269,80 +272,102 @@ LayergroupController.prototype.attributes = function (attributesBackend) { }.bind(this); }; -// Gets a tile for a given token and set of tile ZXY coords. (OSM style) -LayergroupController.prototype.tile = function(req, res, next) { - req.profiler.start('windshaft.map_tile'); - this.tileOrLayer(req, res, next); -}; - -// Gets a tile for a given token, layer set of tile ZXY coords. (OSM style) -LayergroupController.prototype.layer = function(req, res, next) { - req.profiler.start('windshaft.maplayer_tile'); - this.tileOrLayer(req, res, next); -}; - -LayergroupController.prototype.tileOrLayer = function (req, res, next) { - var self = this; - - step( - function mapController$getTileOrGrid() { - self.tileBackend.getTile( - new MapStoreMapConfigProvider(self.mapStore, res.locals.user, self.userLimitsApi, res.locals), - res.locals, this - ); - }, - function mapController$finalize(err, tile, headers, stats) { - req.profiler.add(stats); - self.finalizeGetTileOrGrid(err, req, res, tile, headers, next); - } - ); -}; - function getStatusCode(tile, format){ return tile.length===0 && format==='mvt'? 204:200; } -// This function is meant for being called as the very last -// step by all endpoints serving tiles or grids -LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, tile, headers, next) { - var supportedFormats = { - grid_json: true, - json_torque: true, - torque_json: true, - png: true, - png32: true, - mvt: true - }; +const supportedFormats = { + grid_json: true, + json_torque: true, + torque_json: true, + png: true, + png32: true, + mvt: true +}; - var formatStat = 'invalid'; - if (req.params.format) { - var format = req.params.format.replace('.', '_'); - if (supportedFormats[format]) { - formatStat = format; - } +function parseFormat (format = null) { + const prettyFormat = format.replace('.', '_'); + let formatStat = 'invalid'; + + if (supportedFormats[prettyFormat]) { + formatStat = prettyFormat; } - if (err) { - // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 - var errMsg = err.message ? ( '' + err.message ) : ( '' + err ); + return formatStat; +} - // Rewrite mapnik parsing errors to start with layer number - var matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); - if (matches) { - errMsg = 'style'+matches[2]+': ' + matches[1]; - } - err.message = errMsg; +function augmentError (err) { + // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 + let errMsg = err.message ? ( '' + err.message ) : ( '' + err ); - err.label = 'TILE RENDER'; - next(err); + // Rewrite mapnik parsing errors to start with layer number + const matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); - global.statsClient.increment('windshaft.tiles.error'); - global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); - } else { - this.sendResponse(req, res, tile, getStatusCode(tile, formatStat), headers); - global.statsClient.increment('windshaft.tiles.success'); - global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); + if (matches) { + errMsg = 'style' + matches[2] + ': ' + matches[1]; } + + err.message = errMsg; + err.label = 'TILE RENDER'; + + return err; +} + +LayergroupController.prototype.tile = function (tileBackend) { + return function tileMiddleware (req, res, next) { + req.profiler.start('windshaft.map_tile'); + + const { mapConfigProvider } = res.locals; + const params = getRequestParams(res.locals); + + tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats) => { + req.profiler.add(stats); + + const formatStat = parseFormat(req.params.format); + + if (err) { + next(augmentError(err)); + + global.statsClient.increment('windshaft.tiles.error'); + global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); + + return; + } + + this.sendResponse(req, res, tile, getStatusCode(tile, formatStat), headers); + + global.statsClient.increment('windshaft.tiles.success'); + global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); + }); + }.bind(this); +}; + +LayergroupController.prototype.layer = function (tileBackend) { + return function layerMiddleware (req, res, next) { + req.profiler.start('windshaft.maplayer_tile'); + + const { mapConfigProvider } = res.locals; + const params = getRequestParams(res.locals); + + tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats) => { + req.profiler.add(stats); + + const formatStat = parseFormat(req.params.format); + + if (err) { + next(augmentError(err)); + + global.statsClient.increment('windshaft.tiles.error'); + global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); + + return; + } + + this.sendResponse(req, res, tile, getStatusCode(tile, formatStat), headers); + global.statsClient.increment('windshaft.tiles.success'); + global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); + }); + }.bind(this); }; LayergroupController.prototype.center = function (previewBackend) { From 3695e1e3e54b573db609d0cf3aee7d0aed55fd25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 16:21:46 +0100 Subject: [PATCH 10/44] Place function closer to where is called --- lib/cartodb/controllers/layergroup.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index e07ba56b..0e706b1d 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -175,6 +175,14 @@ LayergroupController.prototype.register = function(app) { ); }; +function validateLayerRouteMiddleware(req, res, next) { + if (req.params.token === 'static') { + return next('route'); + } + + next(); +} + LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBackend) { return function analysisNodeStatusMiddleware(req, res, next) { analysisStatusBackend.getNodeStatus(res.locals, (err, nodeStatus, stats) => { @@ -537,12 +545,3 @@ LayergroupController.prototype.getAffectedTables = function(user, dbName, layerg callback ); }; - - -function validateLayerRouteMiddleware(req, res, next) { - if (req.params.token === 'static') { - return next('route'); - } - - next(); -} From f30f83331fe855a7c627597745c941bb8a29ec14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 16:44:37 +0100 Subject: [PATCH 11/44] Extract tile error middleware --- lib/cartodb/controllers/layergroup.js | 43 ++++++++++++--------- test/unit/cartodb/ported/tile_stats.test.js | 4 +- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 0e706b1d..97eed665 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -53,6 +53,7 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.tile(this.tileBackend), + this.tileError(), vectorError() ); @@ -63,6 +64,7 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.tile(this.tileBackend), + this.tileError(), vectorError() ); @@ -74,6 +76,7 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.layer(this.tileBackend), + this.tileError(), vectorError() ); @@ -304,23 +307,6 @@ function parseFormat (format = null) { return formatStat; } -function augmentError (err) { - // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 - let errMsg = err.message ? ( '' + err.message ) : ( '' + err ); - - // Rewrite mapnik parsing errors to start with layer number - const matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); - - if (matches) { - errMsg = 'style' + matches[2] + ': ' + matches[1]; - } - - err.message = errMsg; - err.label = 'TILE RENDER'; - - return err; -} - LayergroupController.prototype.tile = function (tileBackend) { return function tileMiddleware (req, res, next) { req.profiler.start('windshaft.map_tile'); @@ -334,7 +320,7 @@ LayergroupController.prototype.tile = function (tileBackend) { const formatStat = parseFormat(req.params.format); if (err) { - next(augmentError(err)); + next(err); global.statsClient.increment('windshaft.tiles.error'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); @@ -363,7 +349,7 @@ LayergroupController.prototype.layer = function (tileBackend) { const formatStat = parseFormat(req.params.format); if (err) { - next(augmentError(err)); + next(err); global.statsClient.increment('windshaft.tiles.error'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); @@ -378,6 +364,25 @@ LayergroupController.prototype.layer = function (tileBackend) { }.bind(this); }; +LayergroupController.prototype.tileError = function () { + return function tileErrorMiddleware (err, req, res, next) { + // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 + let errMsg = err.message ? ( '' + err.message ) : ( '' + err ); + + // Rewrite mapnik parsing errors to start with layer number + const matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); + + if (matches) { + errMsg = 'style' + matches[2] + ': ' + matches[1]; + } + + err.message = errMsg; + err.label = 'TILE RENDER'; + + next(err); + }; +}; + LayergroupController.prototype.center = function (previewBackend) { return function centerMiddleware (req, res, next) { const width = +req.params.width; diff --git a/test/unit/cartodb/ported/tile_stats.test.js b/test/unit/cartodb/ported/tile_stats.test.js index a2d30cd0..f8e0d80a 100644 --- a/test/unit/cartodb/ported/tile_stats.test.js +++ b/test/unit/cartodb/ported/tile_stats.test.js @@ -14,7 +14,7 @@ describe('tile stats', function() { global.statsClient = this.statsClient; }); - it('finalizeGetTileOrGrid does not call statsClient when format is not supported', function() { + it.skip('finalizeGetTileOrGrid does not call statsClient when format is not supported', function() { var expectedCalls = 2, // it will call increment once for the general error invalidFormat = 'png2', invalidFormatRegexp = new RegExp('invalid'), @@ -49,7 +49,7 @@ describe('tile stats', function() { assert.equal(expectedCalls, 0, 'Unexpected number of calls to increment method'); }); - it('finalizeGetTileOrGrid calls statsClient when format is supported', function() { + it.skip('finalizeGetTileOrGrid calls statsClient when format is supported', function() { var expectedCalls = 2, // general error + format error validFormat = 'png', validFormatRegexp = new RegExp(validFormat), From 4762aa0897f56f7a0e6289c66a5eb5a6f62569dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 16:55:27 +0100 Subject: [PATCH 12/44] Remove step from sendResponse function --- lib/cartodb/controllers/layergroup.js | 55 ++++++++++++--------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 97eed665..45493db4 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -441,8 +441,6 @@ LayergroupController.prototype.bbox = function (previewBackend) { }; LayergroupController.prototype.sendResponse = function(req, res, body, status, headers) { - var self = this; - req.profiler.done('res'); res.set('Cache-Control', 'public,max-age=31536000'); @@ -458,37 +456,34 @@ 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); - } + this.getAffectedTables(res.locals.user, dbName, res.locals.token, (err, affectedTables) => { + req.profiler.done('affectedTables'); - 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); - } + if (err) { + global.logger.warn('ERROR generating cache channel: ' + err); } - ); + if (!!affectedTables) { + res.set('X-Cache-Channel', affectedTables.getCacheChannel()); + this.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); + } + }); }; LayergroupController.prototype.getAffectedTables = function(user, dbName, layergroupId, callback) { From 2f011c3266da6dac03d2eacf788568b624639a54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 17:01:51 +0100 Subject: [PATCH 13/44] Remove nested steps --- lib/cartodb/controllers/layergroup.js | 34 ++++++++++++--------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 45493db4..1d937374 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -493,29 +493,25 @@ LayergroupController.prototype.getAffectedTables = function(user, dbName, layerg } var self = this; + step( - function extractSQL() { - step( - function loadFromStore() { - self.mapStore.load(layergroupId, this); - }, - function getSQL(err, mapConfig) { - assert.ifError(err); + function loadFromStore() { + self.mapStore.load(layergroupId, this); + }, + function getSQL(err, mapConfig) { + assert.ifError(err); - var queries = []; - mapConfig.getLayers().forEach(function(layer) { - queries.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(function(table) { - queries.push('SELECT * FROM ' + table + ' LIMIT 0'); - }); - } + var queries = []; + mapConfig.getLayers().forEach(function(layer) { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(function(table) { + queries.push('SELECT * FROM ' + table + ' LIMIT 0'); }); + } + }); - return queries.length ? queries.join(';') : null; - }, - this - ); + return queries.length ? queries.join(';') : null; }, function findAffectedTables(err, sql) { assert.ifError(err); From 7c1e2a6af05109300ab1aea58ee1309e6d313f2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 17:08:39 +0100 Subject: [PATCH 14/44] Avoid nested steps --- lib/cartodb/controllers/layergroup.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 1d937374..5747be57 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -513,24 +513,27 @@ LayergroupController.prototype.getAffectedTables = function(user, dbName, layerg return queries.length ? queries.join(';') : null; }, - function findAffectedTables(err, sql) { + function getConnection(err, sql) { assert.ifError(err); if ( ! sql ) { throw new Error("this request doesn't need an X-Cache-Channel generated"); } - step( - function getConnection() { - self.pgConnection.getConnection(user, this); - }, - function getAffectedTables(err, connection) { - assert.ifError(err); + const next = this; - QueryTables.getAffectedTablesFromQuery(connection, sql, this); - }, - this - ); + self.pgConnection.getConnection(user, function (err, connection) { + if (err) { + return next(); + } + + next(null, connection, sql); + }); + }, + function getAffectedTables(err, connection, sql) { + assert.ifError(err); + + QueryTables.getAffectedTablesFromQuery(connection, sql, this); }, function buildCacheChannel(err, tables) { assert.ifError(err); From 7022fb87b45a51280b77b20e48e0ccd170f188d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 18:28:40 +0100 Subject: [PATCH 15/44] Extract header, affected-tables and response middlewares --- lib/cartodb/controllers/layergroup.js | 202 +++++++++++++++++++------- 1 file changed, 150 insertions(+), 52 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 5747be57..98089eac 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -53,6 +53,10 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.tile(this.tileBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse(), this.tileError(), vectorError() ); @@ -64,6 +68,10 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.tile(this.tileBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse(), this.tileError(), vectorError() ); @@ -76,6 +84,10 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.layer(this.tileBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse(), this.tileError(), vectorError() ); @@ -86,7 +98,11 @@ LayergroupController.prototype.register = function(app) { userMiddleware, this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.attributes(this.attributesBackend) + this.attributes(this.attributesBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse() ); const forcedFormat = 'png'; @@ -98,7 +114,11 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(['layer']), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), - this.center(this.previewBackend) + this.center(this.previewBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse() ); app.get( @@ -108,7 +128,11 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(['layer']), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), - this.bbox(this.previewBackend) + this.bbox(this.previewBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse() ); // Undocumented/non-supported API endpoint methods. @@ -136,7 +160,11 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.getDataview(this.dataviewBackend) + this.getDataview(this.dataviewBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse() ); app.get( @@ -146,7 +174,11 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.getDataview(this.dataviewBackend) + this.getDataview(this.dataviewBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse() ); app.get( @@ -156,7 +188,11 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.dataviewSearch(this.dataviewBackend) + this.dataviewSearch(this.dataviewBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse() ); app.get( @@ -166,7 +202,11 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(allowedDataviewQueryParams), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.dataviewSearch(this.dataviewBackend) + this.dataviewSearch(this.dataviewBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse() ); app.get( @@ -174,7 +214,11 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.analysisNodeStatus(this.analysisStatusBackend) + this.analysisNodeStatus(this.analysisStatusBackend), + this.setCacheControlHeader(), + this.setLastModifiedHeader(), + this.affectedTables(), + this.sendResponse() ); }; @@ -196,10 +240,14 @@ LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBack return next(err); } - this.sendResponse(req, res, nodeStatus, 200, { + res.set({ 'Cache-Control': 'public,max-age=5', 'Last-Modified': new Date().toUTCString() }); + + res.body = nodeStatus; + + next(); }); }.bind(this); }; @@ -242,7 +290,9 @@ LayergroupController.prototype.getDataview = function (dataviewBackend) { return next(err); } - this.sendResponse(req, res, dataview, 200); + res.body = dataview; + + next(); }); }.bind(this); }; @@ -259,7 +309,9 @@ LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { return next(err); } - this.sendResponse(req, res, searchResult, 200); + res.body = searchResult; + + next(); }); }.bind(this); }; @@ -278,13 +330,15 @@ LayergroupController.prototype.attributes = function (attributesBackend) { return next(err); } - this.sendResponse(req, res, tile, 200); + res.body = tile; + + next(); }); }.bind(this); }; function getStatusCode(tile, format){ - return tile.length===0 && format==='mvt'? 204:200; + return tile.length === 0 && format === 'mvt'? 204 : 200; } const supportedFormats = { @@ -328,7 +382,14 @@ LayergroupController.prototype.tile = function (tileBackend) { return; } - this.sendResponse(req, res, tile, getStatusCode(tile, formatStat), headers); + if (headers) { + res.set(headers); + } + + res.statusCode = getStatusCode(tile, formatStat); + res.body = tile; + + next(); global.statsClient.increment('windshaft.tiles.success'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); @@ -357,7 +418,15 @@ LayergroupController.prototype.layer = function (tileBackend) { return; } - this.sendResponse(req, res, tile, getStatusCode(tile, formatStat), headers); + if (headers) { + res.set(headers); + } + + res.statusCode = getStatusCode(tile, formatStat); + res.body = tile; + + next(); + global.statsClient.increment('windshaft.tiles.success'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); }); @@ -405,8 +474,15 @@ LayergroupController.prototype.center = function (previewBackend) { return next(err); } + if (headers) { + res.set(headers); + } + res.set('Content-Type', headers['Content-Type'] || 'image/' + format); - this.sendResponse(req, res, image, 200); + + res.body = image; + + next(); }); }.bind(this); }; @@ -433,61 +509,65 @@ LayergroupController.prototype.bbox = function (previewBackend) { return next(err); } + if (headers) { + res.set(headers); + } + res.set('Content-Type', headers['Content-Type'] || 'image/' + format); - this.sendResponse(req, res, image, 200); + res.body = image; + + next(); }); }.bind(this); }; -LayergroupController.prototype.sendResponse = function(req, res, body, status, headers) { - req.profiler.done('res'); +LayergroupController.prototype.setLastModifiedHeader = function () { + return function setLastModifiedHeaderMiddleware (req, res, next) { + let { cache_buster: cacheBuster } = res.locals; - res.set('Cache-Control', 'public,max-age=31536000'); + cacheBuster = parseInt(cacheBuster); - // Set Last-Modified header - var lastUpdated; - if (res.locals.cache_buster) { - // Assuming cache_buster is a timestamp - lastUpdated = new Date(parseInt(res.locals.cache_buster)); - } else { - lastUpdated = new Date(); - } - res.set('Last-Modified', lastUpdated.toUTCString()); + const lastUpdated = res.locals.cache_buster ? new Date(cacheBuster) : new Date(); - var dbName = res.locals.dbname; + res.set('Last-Modified', lastUpdated.toUTCString()); - this.getAffectedTables(res.locals.user, dbName, res.locals.token, (err, affectedTables) => { - req.profiler.done('affectedTables'); + next(); + }; +}; - if (err) { - global.logger.warn('ERROR generating cache channel: ' + err); - } - if (!!affectedTables) { - res.set('X-Cache-Channel', affectedTables.getCacheChannel()); - this.surrogateKeysCache.tag(res, affectedTables); +LayergroupController.prototype.setCacheControlHeader = function () { + return function setCacheControlHeaderMiddleware (req, res, next) { + if (!res.get('Cache-Control')) { + res.set('Cache-Control', 'public,max-age=31536000'); } - if (headers) { - res.set(headers); - } + next(); + }; +}; - res.status(status); +LayergroupController.prototype.affectedTables = function () { + return function affectedTablesMiddleware (req, res, next) { + const { user, dbname, token } = res.locals; - if (!Buffer.isBuffer(body) && typeof body === 'object') { - if (req.query && req.query.callback) { - res.jsonp(body); - } else { - res.json(body); + this.getAffectedTables(user, dbname, token, (err, affectedTables) => { + req.profiler.done('affectedTables'); + + if (err) { + global.logger.warn('ERROR generating cache channel: ' + err); } - } else { - res.send(body); - } - }); + + if (!!affectedTables) { + res.set('X-Cache-Channel', affectedTables.getCacheChannel()); + this.surrogateKeysCache.tag(res, affectedTables); + } + + next(); + }); + }.bind(this); }; LayergroupController.prototype.getAffectedTables = function(user, dbName, layergroupId, callback) { - if (this.layergroupAffectedTables.hasAffectedTables(dbName, layergroupId)) { return callback(null, this.layergroupAffectedTables.get(dbName, layergroupId)); } @@ -544,3 +624,21 @@ LayergroupController.prototype.getAffectedTables = function(user, dbName, layerg callback ); }; + +LayergroupController.prototype.sendResponse = function () { + return function sendResponseMiddleware (req, res) { + req.profiler.done('res'); + + res.status(res.statusCode || 200); + + if (!Buffer.isBuffer(res.body) && typeof res.body === 'object') { + if (req.query && req.query.callback) { + res.jsonp(res.body); + } else { + res.json(res.body); + } + } else { + res.send(res.body); + } + }; +}; From 874ea99d19d1dbcfb7971321ef17bf123137fbdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 18:43:23 +0100 Subject: [PATCH 16/44] Remove step --- lib/cartodb/controllers/layergroup.js | 73 ++++++++++----------------- 1 file changed, 28 insertions(+), 45 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 98089eac..0f5512d6 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -1,6 +1,3 @@ -var assert = require('assert'); -var step = require('step'); - var cors = require('../middleware/cors'); var userMiddleware = require('../middleware/user'); var allowQueryParams = require('../middleware/allow-query-params'); @@ -567,62 +564,48 @@ LayergroupController.prototype.affectedTables = function () { }.bind(this); }; -LayergroupController.prototype.getAffectedTables = function(user, dbName, layergroupId, callback) { +LayergroupController.prototype.getAffectedTables = function (user, dbName, layergroupId, callback) { if (this.layergroupAffectedTables.hasAffectedTables(dbName, layergroupId)) { return callback(null, this.layergroupAffectedTables.get(dbName, layergroupId)); } - var self = this; + this.mapStore.load(layergroupId, (err, mapConfig) => { + if (err) { + return callback(err); + } - step( - function loadFromStore() { - self.mapStore.load(layergroupId, this); - }, - function getSQL(err, mapConfig) { - assert.ifError(err); + var queries = []; + mapConfig.getLayers().forEach(function(layer) { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(function(table) { + queries.push('SELECT * FROM ' + table + ' LIMIT 0'); + }); + } + }); - var queries = []; - mapConfig.getLayers().forEach(function(layer) { - queries.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(function(table) { - queries.push('SELECT * FROM ' + table + ' LIMIT 0'); - }); - } - }); + const sql = queries.length ? queries.join(';') : null; - return queries.length ? queries.join(';') : null; - }, - function getConnection(err, sql) { - assert.ifError(err); + if ( ! sql ) { + return callback(new Error("this request doesn't need an X-Cache-Channel generated")); + } - if ( ! sql ) { - throw new Error("this request doesn't need an X-Cache-Channel generated"); + this.pgConnection.getConnection(user, (err, connection) => { + if (err) { + return callback(err); } - const next = this; - - self.pgConnection.getConnection(user, function (err, connection) { + QueryTables.getAffectedTablesFromQuery(connection, sql, (err, tables) => { if (err) { - return next(); + return callback(err); } - next(null, connection, sql); + this.layergroupAffectedTables.set(dbName, layergroupId, tables); + + callback(null, tables); }); - }, - function getAffectedTables(err, connection, sql) { - assert.ifError(err); - - QueryTables.getAffectedTablesFromQuery(connection, sql, this); - }, - function buildCacheChannel(err, tables) { - assert.ifError(err); - self.layergroupAffectedTables.set(dbName, layergroupId, tables); - - return tables; - }, - callback - ); + }); + }); }; LayergroupController.prototype.sendResponse = function () { From 3399db1cff841d80e7d304a79c3f07c031048861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 18:58:09 +0100 Subject: [PATCH 17/44] Add comment --- lib/cartodb/controllers/layergroup.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 0f5512d6..899e35eb 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -600,6 +600,7 @@ LayergroupController.prototype.getAffectedTables = function (user, dbName, layer return callback(err); } + // feed affected tables cache so it can be reused from, for instance, map controller this.layergroupAffectedTables.set(dbName, layergroupId, tables); callback(null, tables); From 94d1667d705049a864a536f748d70938ed14b1c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 20:01:43 +0100 Subject: [PATCH 18/44] Refactor affected tables --- lib/cartodb/controllers/layergroup.js | 155 +++++++++++++++----------- 1 file changed, 92 insertions(+), 63 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 899e35eb..cecd03fd 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -52,7 +52,9 @@ LayergroupController.prototype.register = function(app) { this.tile(this.tileBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse(), this.tileError(), vectorError() @@ -67,7 +69,9 @@ LayergroupController.prototype.register = function(app) { this.tile(this.tileBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse(), this.tileError(), vectorError() @@ -83,7 +87,9 @@ LayergroupController.prototype.register = function(app) { this.layer(this.tileBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse(), this.tileError(), vectorError() @@ -98,7 +104,9 @@ LayergroupController.prototype.register = function(app) { this.attributes(this.attributesBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse() ); @@ -114,7 +122,9 @@ LayergroupController.prototype.register = function(app) { this.center(this.previewBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse() ); @@ -128,7 +138,9 @@ LayergroupController.prototype.register = function(app) { this.bbox(this.previewBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse() ); @@ -160,7 +172,9 @@ LayergroupController.prototype.register = function(app) { this.getDataview(this.dataviewBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse() ); @@ -174,7 +188,9 @@ LayergroupController.prototype.register = function(app) { this.getDataview(this.dataviewBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse() ); @@ -188,7 +204,9 @@ LayergroupController.prototype.register = function(app) { this.dataviewSearch(this.dataviewBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse() ); @@ -202,7 +220,9 @@ LayergroupController.prototype.register = function(app) { this.dataviewSearch(this.dataviewBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), + this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + this.setCacheChannelHeader(), + this.setSurrogateKeyHeader(this.surrogateKeysCache), this.sendResponse() ); @@ -214,7 +234,6 @@ LayergroupController.prototype.register = function(app) { this.analysisNodeStatus(this.analysisStatusBackend), this.setCacheControlHeader(), this.setLastModifiedHeader(), - this.affectedTables(), this.sendResponse() ); }; @@ -269,9 +288,18 @@ LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore params.layer = params.layer || 'all'; } - res.locals.mapConfigProvider = new MapStoreMapConfigProvider(mapStore, user, userLimitsApi, params); + const mapConfigProvider = new MapStoreMapConfigProvider(mapStore, user, userLimitsApi, params); - next(); + mapConfigProvider.getMapConfig((err, mapconfig) => { + if (err) { + return next(err); + } + + res.locals.mapConfigProvider = mapConfigProvider; + res.locals.mapconfig = mapconfig; + + next(); + }); }; }; @@ -543,70 +571,71 @@ LayergroupController.prototype.setCacheControlHeader = function () { }; }; -LayergroupController.prototype.affectedTables = function () { - return function affectedTablesMiddleware (req, res, next) { - const { user, dbname, token } = res.locals; +LayergroupController.prototype.getAffectedTables = function (layergroupAffectedTables, pgConnection) { + return function getAffectedTablesMiddleware (req, res, next) { + const { user, dbname, token, mapconfig } = res.locals; - this.getAffectedTables(user, dbname, token, (err, affectedTables) => { - req.profiler.done('affectedTables'); + if (layergroupAffectedTables.hasAffectedTables(dbname, token)) { + res.locals.affectedTables = layergroupAffectedTables.get(dbname, token); + return next(); + } + pgConnection.getConnection(user, (err, connection) => { if (err) { global.logger.warn('ERROR generating cache channel: ' + err); + return next(); } - if (!!affectedTables) { - res.set('X-Cache-Channel', affectedTables.getCacheChannel()); - this.surrogateKeysCache.tag(res, 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'); + }); + } + }); - next(); - }); - }.bind(this); -}; - -LayergroupController.prototype.getAffectedTables = function (user, dbName, layergroupId, callback) { - if (this.layergroupAffectedTables.hasAffectedTables(dbName, layergroupId)) { - return callback(null, this.layergroupAffectedTables.get(dbName, layergroupId)); - } - - this.mapStore.load(layergroupId, (err, mapConfig) => { - if (err) { - return callback(err); - } - - var queries = []; - mapConfig.getLayers().forEach(function(layer) { - queries.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(function(table) { - queries.push('SELECT * FROM ' + table + ' LIMIT 0'); - }); - } - }); - - const sql = queries.length ? queries.join(';') : null; - - if ( ! sql ) { - return callback(new Error("this request doesn't need an X-Cache-Channel generated")); - } - - this.pgConnection.getConnection(user, (err, connection) => { - if (err) { - return callback(err); - } - - QueryTables.getAffectedTablesFromQuery(connection, sql, (err, tables) => { + QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), (err, affectedTables) => { + req.profiler.done('getAffectedTablesFromQuery'); if (err) { - return callback(err); + global.logger.warn('ERROR generating cache channel: ' + err); + return next(); } // feed affected tables cache so it can be reused from, for instance, map controller - this.layergroupAffectedTables.set(dbName, layergroupId, tables); + layergroupAffectedTables.set(dbname, token, affectedTables); - callback(null, tables); + res.locals.affectedTables = affectedTables; + + next(); }); }); - }); + }; +}; + +LayergroupController.prototype.setCacheChannelHeader = function () { + return function setCacheChannelHeaderMiddleware (req, res, next) { + const { affectedTables } = res.locals; + + if (affectedTables) { + res.set('X-Cache-Channel', affectedTables.getCacheChannel()); + } + + next(); + }; +}; + +LayergroupController.prototype.setSurrogateKeyHeader = function (surrogateKeysCache) { + return function setSurrogateKeyHeaderMiddleware (req, res, next) { + const { affectedTables } = res.locals; + + if (affectedTables) { + surrogateKeysCache.tag(res, affectedTables); + } + + next(); + }; }; LayergroupController.prototype.sendResponse = function () { From a66c19c6c7e3f7e80b6946ee52bfcb2c90cde415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 6 Mar 2018 20:05:55 +0100 Subject: [PATCH 19/44] Do not bind context when unneeded --- lib/cartodb/controllers/layergroup.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index cecd03fd..8cf3f064 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -265,7 +265,7 @@ LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBack next(); }); - }.bind(this); + }; }; function getRequestParams(locals) { @@ -319,7 +319,7 @@ LayergroupController.prototype.getDataview = function (dataviewBackend) { next(); }); - }.bind(this); + }; }; LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { @@ -338,7 +338,7 @@ LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { next(); }); - }.bind(this); + }; }; LayergroupController.prototype.attributes = function (attributesBackend) { @@ -359,7 +359,7 @@ LayergroupController.prototype.attributes = function (attributesBackend) { next(); }); - }.bind(this); + }; }; function getStatusCode(tile, format){ @@ -419,7 +419,7 @@ LayergroupController.prototype.tile = function (tileBackend) { global.statsClient.increment('windshaft.tiles.success'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); }); - }.bind(this); + }; }; LayergroupController.prototype.layer = function (tileBackend) { @@ -455,7 +455,7 @@ LayergroupController.prototype.layer = function (tileBackend) { global.statsClient.increment('windshaft.tiles.success'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); }); - }.bind(this); + }; }; LayergroupController.prototype.tileError = function () { @@ -509,7 +509,7 @@ LayergroupController.prototype.center = function (previewBackend) { next(); }); - }.bind(this); + }; }; LayergroupController.prototype.bbox = function (previewBackend) { @@ -544,7 +544,7 @@ LayergroupController.prototype.bbox = function (previewBackend) { next(); }); - }.bind(this); + }; }; LayergroupController.prototype.setLastModifiedHeader = function () { From b786164e8aa1943b0aa449c91b6cee323c17e8ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 11:56:57 +0100 Subject: [PATCH 20/44] Middlewarify metrics increment whether success or error --- lib/cartodb/controllers/layergroup.js | 58 ++++++++++++++++----------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 8cf3f064..08722815 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -50,6 +50,8 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.tile(this.tileBackend), + this.incrementSuccessMetrics(global.statsClient), + this.incrementErrorMetrics(global.statsClient), this.setCacheControlHeader(), this.setLastModifiedHeader(), this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -67,6 +69,8 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.tile(this.tileBackend), + this.incrementSuccessMetrics(global.statsClient), + this.incrementErrorMetrics(global.statsClient), this.setCacheControlHeader(), this.setLastModifiedHeader(), this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -85,6 +89,8 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.layer(this.tileBackend), + this.incrementSuccessMetrics(global.statsClient), + this.incrementErrorMetrics(global.statsClient), this.setCacheControlHeader(), this.setLastModifiedHeader(), this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -375,7 +381,7 @@ const supportedFormats = { mvt: true }; -function parseFormat (format = null) { +function parseFormat (format = '') { const prettyFormat = format.replace('.', '_'); let formatStat = 'invalid'; @@ -396,28 +402,20 @@ LayergroupController.prototype.tile = function (tileBackend) { tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats) => { req.profiler.add(stats); - const formatStat = parseFormat(req.params.format); - if (err) { - next(err); - - global.statsClient.increment('windshaft.tiles.error'); - global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); - - return; + return next(err); } if (headers) { res.set(headers); } + const formatStat = parseFormat(req.params.format); + res.statusCode = getStatusCode(tile, formatStat); res.body = tile; next(); - - global.statsClient.increment('windshaft.tiles.success'); - global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); }); }; }; @@ -432,32 +430,46 @@ LayergroupController.prototype.layer = function (tileBackend) { tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats) => { req.profiler.add(stats); - const formatStat = parseFormat(req.params.format); - if (err) { - next(err); - - global.statsClient.increment('windshaft.tiles.error'); - global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); - - return; + return next(err); } if (headers) { res.set(headers); } + const formatStat = parseFormat(req.params.format); + res.statusCode = getStatusCode(tile, formatStat); res.body = tile; next(); - - global.statsClient.increment('windshaft.tiles.success'); - global.statsClient.increment('windshaft.tiles.' + formatStat + '.success'); }); }; }; +LayergroupController.prototype.incrementSuccessMetrics = function (statsClient) { + return function incrementSuccessMetricsMiddleware (req, res, next) { + const formatStat = parseFormat(req.params.format); + + statsClient.increment('windshaft.tiles.success'); + statsClient.increment(`windshaft.tiles.${formatStat}.success`); + + next(); + }; +}; + +LayergroupController.prototype.incrementErrorMetrics = function (statsClient) { + return function incrementErrorMetricsMiddleware (err, req, res, next) { + const formatStat = parseFormat(req.params.format); + + statsClient.increment('windshaft.tiles.error'); + statsClient.increment(`windshaft.tiles.${formatStat}.error`); + + next(err); + }; +}; + LayergroupController.prototype.tileError = function () { return function tileErrorMiddleware (err, req, res, next) { // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 From 82446e5ffaaff127ff12fb44618a76028ddc7d5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 12:05:53 +0100 Subject: [PATCH 21/44] Use template string to define routes --- lib/cartodb/controllers/layergroup.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 08722815..3762e0c5 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -43,8 +43,10 @@ function LayergroupController(prepareContext, pgConnection, mapStore, tileBacken module.exports = LayergroupController; LayergroupController.prototype.register = function(app) { + const { base_url_mapconfig: basePath } = app; + app.get( - app.base_url_mapconfig + '/:token/:z/:x/:y@:scale_factor?x.:format', + `${basePath}/:token/:z/:x/:y@:scale_factor?x.:format`, cors(), userMiddleware, this.prepareContext, @@ -63,7 +65,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - app.base_url_mapconfig + '/:token/:z/:x/:y.:format', + `${basePath}/:token/:z/:x/:y.:format`, cors(), userMiddleware, this.prepareContext, @@ -82,7 +84,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - app.base_url_mapconfig + '/:token/:layer/:z/:x/:y.(:format)', + `${basePath}/:token/:layer/:z/:x/:y.(:format)`, cors(), userMiddleware, validateLayerRouteMiddleware, @@ -102,7 +104,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - app.base_url_mapconfig + '/:token/:layer/attributes/:fid', + `${basePath}/:token/:layer/attributes/:fid`, cors(), userMiddleware, this.prepareContext, @@ -119,7 +121,7 @@ LayergroupController.prototype.register = function(app) { const forcedFormat = 'png'; app.get( - app.base_url_mapconfig + '/static/center/:token/:z/:lat/:lng/:width/:height.:format', + `${basePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, cors(), userMiddleware, allowQueryParams(['layer']), @@ -135,7 +137,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - app.base_url_mapconfig + '/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format', + `${basePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, cors(), userMiddleware, allowQueryParams(['layer']), @@ -169,7 +171,7 @@ LayergroupController.prototype.register = function(app) { ]; app.get( - app.base_url_mapconfig + '/:token/dataview/:dataviewName', + `${basePath}/:token/dataview/:dataviewName`, cors(), userMiddleware, allowQueryParams(allowedDataviewQueryParams), @@ -185,7 +187,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - app.base_url_mapconfig + '/:token/:layer/widget/:dataviewName', + `${basePath}/:token/:layer/widget/:dataviewName`, cors(), userMiddleware, allowQueryParams(allowedDataviewQueryParams), @@ -201,7 +203,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - app.base_url_mapconfig + '/:token/dataview/:dataviewName/search', + `${basePath}/:token/dataview/:dataviewName/search`, cors(), userMiddleware, allowQueryParams(allowedDataviewQueryParams), @@ -217,7 +219,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - app.base_url_mapconfig + '/:token/:layer/widget/:dataviewName/search', + `${basePath}/:token/:layer/widget/:dataviewName/search`, cors(), userMiddleware, allowQueryParams(allowedDataviewQueryParams), @@ -233,7 +235,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - app.base_url_mapconfig + '/:token/analysis/node/:nodeId', + `${basePath}/:token/analysis/node/:nodeId`, cors(), userMiddleware, this.prepareContext, From d351c8d14ccb559ce4610d3aae11d8f929408818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 12:09:41 +0100 Subject: [PATCH 22/44] Define var as const --- lib/cartodb/controllers/layergroup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 3762e0c5..a7628c21 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -155,7 +155,7 @@ LayergroupController.prototype.register = function(app) { // Undocumented/non-supported API endpoint methods. // Use at your own peril. - var allowedDataviewQueryParams = [ + const allowedDataviewQueryParams = [ 'filters', // json 'own_filter', // 0, 1 'no_filters', // 0, 1 From 33089be2cd6d08d71be6b565a05c3d66f7189089 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 12:30:59 +0100 Subject: [PATCH 23/44] Do not attach header middlewares to node status endpoint --- lib/cartodb/controllers/layergroup.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index a7628c21..2783552c 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -240,8 +240,6 @@ LayergroupController.prototype.register = function(app) { userMiddleware, this.prepareContext, this.analysisNodeStatus(this.analysisStatusBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), this.sendResponse() ); }; @@ -577,9 +575,7 @@ LayergroupController.prototype.setLastModifiedHeader = function () { LayergroupController.prototype.setCacheControlHeader = function () { return function setCacheControlHeaderMiddleware (req, res, next) { - if (!res.get('Cache-Control')) { - res.set('Cache-Control', 'public,max-age=31536000'); - } + res.set('Cache-Control', 'public,max-age=31536000'); next(); }; From 5871f8290d8b2e5f993e6e28e2deefc115dad089 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 12:46:18 +0100 Subject: [PATCH 24/44] Use default param values --- lib/cartodb/controllers/layergroup.js | 32 +++++++++++++-------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 2783552c..405857ad 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -254,8 +254,8 @@ function validateLayerRouteMiddleware(req, res, next) { LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBackend) { return function analysisNodeStatusMiddleware(req, res, next) { - analysisStatusBackend.getNodeStatus(res.locals, (err, nodeStatus, stats) => { - req.profiler.add(stats || {}); + analysisStatusBackend.getNodeStatus(res.locals, (err, nodeStatus, stats = {}) => { + req.profiler.add(stats); if (err) { err.label = 'GET NODE STATUS'; @@ -313,8 +313,8 @@ LayergroupController.prototype.getDataview = function (dataviewBackend) { return function getDataviewMiddleware (req, res, next) { const { user, mapConfigProvider } = res.locals; - dataviewBackend.getDataview(mapConfigProvider, user, res.locals, (err, dataview, stats) => { - req.profiler.add(stats || {}); + dataviewBackend.getDataview(mapConfigProvider, user, res.locals, (err, dataview, stats = {}) => { + req.profiler.add(stats); if (err) { err.label = 'GET DATAVIEW'; @@ -332,8 +332,8 @@ LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { return function dataviewSearchMiddlewarify (req, res, next) { const { user, dataviewName, mapConfigProvider } = res.locals; - dataviewBackend.search(mapConfigProvider, user, dataviewName, res.locals, (err, searchResult, stats) => { - req.profiler.add(stats || {}); + dataviewBackend.search(mapConfigProvider, user, dataviewName, res.locals, (err, searchResult, stats = {}) => { + req.profiler.add(stats); if (err) { err.label = 'GET DATAVIEW SEARCH'; @@ -353,8 +353,8 @@ LayergroupController.prototype.attributes = function (attributesBackend) { const { mapConfigProvider } = res.locals; - attributesBackend.getFeatureAttributes(mapConfigProvider, res.locals, false, (err, tile, stats) => { - req.profiler.add(stats || {}); + attributesBackend.getFeatureAttributes(mapConfigProvider, res.locals, false, (err, tile, stats = {}) => { + req.profiler.add(stats); if (err) { err.label = 'GET ATTRIBUTES'; @@ -399,7 +399,7 @@ LayergroupController.prototype.tile = function (tileBackend) { const { mapConfigProvider } = res.locals; const params = getRequestParams(res.locals); - tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats) => { + tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats = {}) => { req.profiler.add(stats); if (err) { @@ -427,7 +427,7 @@ LayergroupController.prototype.layer = function (tileBackend) { const { mapConfigProvider } = res.locals; const params = getRequestParams(res.locals); - tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats) => { + tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats = {}) => { req.profiler.add(stats); if (err) { @@ -500,11 +500,11 @@ LayergroupController.prototype.center = function (previewBackend) { }; const format = req.params.format === 'jpg' ? 'jpeg' : 'png'; - const { mapConfigProvider } = res.locals; + const { mapConfigProvider: provider } = res.locals; - previewBackend.getImage(mapConfigProvider, format, width, height, zoom, center,(err, image, headers, stats) => { + previewBackend.getImage(provider, format, width, height, zoom, center, (err, image, headers, stats = {}) => { req.profiler.done('render-' + format); - req.profiler.add(stats || {}); + req.profiler.add(stats); if (err) { err.label = 'STATIC_MAP'; @@ -535,11 +535,11 @@ LayergroupController.prototype.bbox = function (previewBackend) { south: +req.params.south }; const format = req.params.format === 'jpg' ? 'jpeg' : 'png'; - const { mapConfigProvider } = res.locals; + const { mapConfigProvider: provider } = res.locals; - previewBackend.getImage(mapConfigProvider, format, width, height, bounds, (err, image, headers, stats) => { + previewBackend.getImage(provider, format, width, height, bounds, (err, image, headers, stats = {}) => { req.profiler.done('render-' + format); - req.profiler.add(stats || {}); + req.profiler.add(stats); if (err) { err.label = 'STATIC_MAP'; From ec41cddb19b71390b5c85b9c457e52815a3b4aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 12:52:44 +0100 Subject: [PATCH 25/44] Do not pass the whole res.locals to backends --- lib/cartodb/controllers/layergroup.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 405857ad..6a50862f 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -312,8 +312,9 @@ LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore LayergroupController.prototype.getDataview = function (dataviewBackend) { return function getDataviewMiddleware (req, res, next) { const { user, mapConfigProvider } = res.locals; + const params = getRequestParams(res.locals); - dataviewBackend.getDataview(mapConfigProvider, user, res.locals, (err, dataview, stats = {}) => { + dataviewBackend.getDataview(mapConfigProvider, user, params, (err, dataview, stats = {}) => { req.profiler.add(stats); if (err) { @@ -331,8 +332,9 @@ LayergroupController.prototype.getDataview = function (dataviewBackend) { LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { return function dataviewSearchMiddlewarify (req, res, next) { const { user, dataviewName, mapConfigProvider } = res.locals; + const params = getRequestParams(res.locals); - dataviewBackend.search(mapConfigProvider, user, dataviewName, res.locals, (err, searchResult, stats = {}) => { + dataviewBackend.search(mapConfigProvider, user, dataviewName, params, (err, searchResult, stats = {}) => { req.profiler.add(stats); if (err) { @@ -352,8 +354,9 @@ LayergroupController.prototype.attributes = function (attributesBackend) { req.profiler.start('windshaft.maplayer_attribute'); const { mapConfigProvider } = res.locals; + const params = getRequestParams(res.locals); - attributesBackend.getFeatureAttributes(mapConfigProvider, res.locals, false, (err, tile, stats = {}) => { + attributesBackend.getFeatureAttributes(mapConfigProvider, params, false, (err, tile, stats = {}) => { req.profiler.add(stats); if (err) { From 292dad130dd62f30e50afaefbff792202262823d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 14:42:21 +0100 Subject: [PATCH 26/44] Move middlewares to the right place --- lib/cartodb/controllers/layergroup.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 6a50862f..1a853bd9 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -52,14 +52,14 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.tile(this.tileBackend), - this.incrementSuccessMetrics(global.statsClient), - this.incrementErrorMetrics(global.statsClient), this.setCacheControlHeader(), this.setLastModifiedHeader(), this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), this.setCacheChannelHeader(), this.setSurrogateKeyHeader(this.surrogateKeysCache), + this.incrementSuccessMetrics(global.statsClient), this.sendResponse(), + this.incrementErrorMetrics(global.statsClient), this.tileError(), vectorError() ); @@ -71,14 +71,14 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.tile(this.tileBackend), - this.incrementSuccessMetrics(global.statsClient), - this.incrementErrorMetrics(global.statsClient), this.setCacheControlHeader(), this.setLastModifiedHeader(), this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), this.setCacheChannelHeader(), this.setSurrogateKeyHeader(this.surrogateKeysCache), + this.incrementSuccessMetrics(global.statsClient), this.sendResponse(), + this.incrementErrorMetrics(global.statsClient), this.tileError(), vectorError() ); @@ -91,14 +91,14 @@ LayergroupController.prototype.register = function(app) { this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.layer(this.tileBackend), - this.incrementSuccessMetrics(global.statsClient), - this.incrementErrorMetrics(global.statsClient), this.setCacheControlHeader(), this.setLastModifiedHeader(), this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), this.setCacheChannelHeader(), this.setSurrogateKeyHeader(this.surrogateKeysCache), + this.incrementSuccessMetrics(global.statsClient), this.sendResponse(), + this.incrementErrorMetrics(global.statsClient), this.tileError(), vectorError() ); From eb3414f07f5e4bc41224e4b3d8e0bfb7dd18dda6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 14:48:21 +0100 Subject: [PATCH 27/44] Follow middleware pattern --- lib/cartodb/controllers/layergroup.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 1a853bd9..7010afa6 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -87,7 +87,7 @@ LayergroupController.prototype.register = function(app) { `${basePath}/:token/:layer/:z/:x/:y.(:format)`, cors(), userMiddleware, - validateLayerRouteMiddleware, + validateLayerRoute(), this.prepareContext, this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), this.layer(this.tileBackend), @@ -244,12 +244,14 @@ LayergroupController.prototype.register = function(app) { ); }; -function validateLayerRouteMiddleware(req, res, next) { - if (req.params.token === 'static') { - return next('route'); - } +function validateLayerRoute () { + return function validateLayerRouteMiddleware(req, res, next) { + if (req.params.token === 'static') { + return next('route'); + } - next(); + next(); + }; } LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBackend) { From b2cc7ab84fe150e17b745bb6f911fec9d8c13921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 14:53:13 +0100 Subject: [PATCH 28/44] Move functions to improve readablity --- lib/cartodb/controllers/layergroup.js | 82 +++++++++++++-------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 7010afa6..cc64cbe9 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -453,47 +453,6 @@ LayergroupController.prototype.layer = function (tileBackend) { }; }; -LayergroupController.prototype.incrementSuccessMetrics = function (statsClient) { - return function incrementSuccessMetricsMiddleware (req, res, next) { - const formatStat = parseFormat(req.params.format); - - statsClient.increment('windshaft.tiles.success'); - statsClient.increment(`windshaft.tiles.${formatStat}.success`); - - next(); - }; -}; - -LayergroupController.prototype.incrementErrorMetrics = function (statsClient) { - return function incrementErrorMetricsMiddleware (err, req, res, next) { - const formatStat = parseFormat(req.params.format); - - statsClient.increment('windshaft.tiles.error'); - statsClient.increment(`windshaft.tiles.${formatStat}.error`); - - next(err); - }; -}; - -LayergroupController.prototype.tileError = function () { - return function tileErrorMiddleware (err, req, res, next) { - // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 - let errMsg = err.message ? ( '' + err.message ) : ( '' + err ); - - // Rewrite mapnik parsing errors to start with layer number - const matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); - - if (matches) { - errMsg = 'style' + matches[2] + ': ' + matches[1]; - } - - err.message = errMsg; - err.label = 'TILE RENDER'; - - next(err); - }; -}; - LayergroupController.prototype.center = function (previewBackend) { return function centerMiddleware (req, res, next) { const width = +req.params.width; @@ -653,6 +612,17 @@ LayergroupController.prototype.setSurrogateKeyHeader = function (surrogateKeysCa }; }; +LayergroupController.prototype.incrementSuccessMetrics = function (statsClient) { + return function incrementSuccessMetricsMiddleware (req, res, next) { + const formatStat = parseFormat(req.params.format); + + statsClient.increment('windshaft.tiles.success'); + statsClient.increment(`windshaft.tiles.${formatStat}.success`); + + next(); + }; +}; + LayergroupController.prototype.sendResponse = function () { return function sendResponseMiddleware (req, res) { req.profiler.done('res'); @@ -670,3 +640,33 @@ LayergroupController.prototype.sendResponse = function () { } }; }; + +LayergroupController.prototype.incrementErrorMetrics = function (statsClient) { + return function incrementErrorMetricsMiddleware (err, req, res, next) { + const formatStat = parseFormat(req.params.format); + + statsClient.increment('windshaft.tiles.error'); + statsClient.increment(`windshaft.tiles.${formatStat}.error`); + + next(err); + }; +}; + +LayergroupController.prototype.tileError = function () { + return function tileErrorMiddleware (err, req, res, next) { + // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 + let errMsg = err.message ? ( '' + err.message ) : ( '' + err ); + + // Rewrite mapnik parsing errors to start with layer number + const matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); + + if (matches) { + errMsg = 'style' + matches[2] + ': ' + matches[1]; + } + + err.message = errMsg; + err.label = 'TILE RENDER'; + + next(err); + }; +}; From a95b3f2f998ea5fb3cee7cdb9b7767d1e531fa93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 14:54:09 +0100 Subject: [PATCH 29/44] Fix comment --- lib/cartodb/controllers/layergroup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index cc64cbe9..a459af7d 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -11,7 +11,7 @@ var MapStoreMapConfigProvider = require('../models/mapconfig/provider/map-store- var QueryTables = require('cartodb-query-tables'); /** - * @param {AuthApi} authApi + * @param {prepareContext} prepareContext * @param {PgConnection} pgConnection * @param {MapStore} mapStore * @param {TileBackend} tileBackend From 48be15b74245d5263d7b96cd88a09f4c427772b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 15:01:04 +0100 Subject: [PATCH 30/44] Use const in favour of var --- lib/cartodb/controllers/layergroup.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index a459af7d..80cb4b08 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -1,14 +1,11 @@ -var cors = require('../middleware/cors'); -var userMiddleware = require('../middleware/user'); -var allowQueryParams = require('../middleware/allow-query-params'); -var vectorError = require('../middleware/vector-error'); - -var DataviewBackend = require('../backends/dataview'); -var AnalysisStatusBackend = require('../backends/analysis-status'); - -var MapStoreMapConfigProvider = require('../models/mapconfig/provider/map-store-provider'); - -var QueryTables = require('cartodb-query-tables'); +const cors = require('../middleware/cors'); +const userMiddleware = require('../middleware/user'); +const allowQueryParams = require('../middleware/allow-query-params'); +const vectorError = require('../middleware/vector-error'); +const DataviewBackend = require('../backends/dataview'); +const AnalysisStatusBackend = require('../backends/analysis-status'); +const MapStoreMapConfigProvider = require('../models/mapconfig/provider/map-store-provider'); +const QueryTables = require('cartodb-query-tables'); /** * @param {prepareContext} prepareContext From 90aaed0f2c0dc93ed2b21f19428cc311b5739483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 15:05:36 +0100 Subject: [PATCH 31/44] Typo --- lib/cartodb/controllers/layergroup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 80cb4b08..9e0e7d28 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -329,7 +329,7 @@ LayergroupController.prototype.getDataview = function (dataviewBackend) { }; LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { - return function dataviewSearchMiddlewarify (req, res, next) { + return function dataviewSearchMiddleware (req, res, next) { const { user, dataviewName, mapConfigProvider } = res.locals; const params = getRequestParams(res.locals); From c8e8317ea44bab71730e19915a55e569d56cff16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 15:20:47 +0100 Subject: [PATCH 32/44] Do not attach middleware to LayergroupController classs --- lib/cartodb/controllers/layergroup.js | 252 +++++++++++++------------- 1 file changed, 126 insertions(+), 126 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 9e0e7d28..bf2abeed 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -47,17 +47,17 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.tile(this.tileBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.incrementSuccessMetrics(global.statsClient), - this.sendResponse(), - this.incrementErrorMetrics(global.statsClient), - this.tileError(), + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + tile(this.tileBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + incrementSuccessMetrics(global.statsClient), + sendResponse(), + incrementErrorMetrics(global.statsClient), + tileError(), vectorError() ); @@ -66,17 +66,17 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.tile(this.tileBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.incrementSuccessMetrics(global.statsClient), - this.sendResponse(), - this.incrementErrorMetrics(global.statsClient), - this.tileError(), + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + tile(this.tileBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + incrementSuccessMetrics(global.statsClient), + sendResponse(), + incrementErrorMetrics(global.statsClient), + tileError(), vectorError() ); @@ -86,17 +86,17 @@ LayergroupController.prototype.register = function(app) { userMiddleware, validateLayerRoute(), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.layer(this.tileBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.incrementSuccessMetrics(global.statsClient), - this.sendResponse(), - this.incrementErrorMetrics(global.statsClient), - this.tileError(), + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + layer(this.tileBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + incrementSuccessMetrics(global.statsClient), + sendResponse(), + incrementErrorMetrics(global.statsClient), + tileError(), vectorError() ); @@ -105,14 +105,14 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.attributes(this.attributesBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.sendResponse() + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + attributes(this.attributesBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + sendResponse() ); const forcedFormat = 'png'; @@ -123,14 +123,14 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(['layer']), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), - this.center(this.previewBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.sendResponse() + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), + center(this.previewBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + sendResponse() ); app.get( @@ -139,14 +139,14 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(['layer']), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), - this.bbox(this.previewBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.sendResponse() + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), + bbox(this.previewBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + sendResponse() ); // Undocumented/non-supported API endpoint methods. @@ -173,14 +173,14 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.getDataview(this.dataviewBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.sendResponse() + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + getDataview(this.dataviewBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + sendResponse() ); app.get( @@ -189,14 +189,14 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.getDataview(this.dataviewBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.sendResponse() + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + getDataview(this.dataviewBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + sendResponse() ); app.get( @@ -205,14 +205,14 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.dataviewSearch(this.dataviewBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.sendResponse() + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + dataviewSearch(this.dataviewBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + sendResponse() ); app.get( @@ -221,14 +221,14 @@ LayergroupController.prototype.register = function(app) { userMiddleware, allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - this.getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - this.dataviewSearch(this.dataviewBackend), - this.setCacheControlHeader(), - this.setLastModifiedHeader(), - this.getAffectedTables(this.layergroupAffectedTables, this.pgConnection), - this.setCacheChannelHeader(), - this.setSurrogateKeyHeader(this.surrogateKeysCache), - this.sendResponse() + getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + dataviewSearch(this.dataviewBackend), + setCacheControlHeader(), + setLastModifiedHeader(), + getAffectedTables(this.layergroupAffectedTables, this.pgConnection), + setCacheChannelHeader(), + setSurrogateKeyHeader(this.surrogateKeysCache), + sendResponse() ); app.get( @@ -236,8 +236,8 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware, this.prepareContext, - this.analysisNodeStatus(this.analysisStatusBackend), - this.sendResponse() + analysisNodeStatus(this.analysisStatusBackend), + sendResponse() ); }; @@ -251,7 +251,7 @@ function validateLayerRoute () { }; } -LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBackend) { +function analysisNodeStatus (analysisStatusBackend) { return function analysisNodeStatusMiddleware(req, res, next) { analysisStatusBackend.getNodeStatus(res.locals, (err, nodeStatus, stats = {}) => { req.profiler.add(stats); @@ -271,7 +271,7 @@ LayergroupController.prototype.analysisNodeStatus = function (analysisStatusBack next(); }); }; -}; +} function getRequestParams(locals) { const params = Object.assign({}, locals); @@ -282,7 +282,7 @@ function getRequestParams(locals) { return params; } -LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore, userLimitsApi, forcedFormat = null) { +function getMapStoreMapConfigProvider (mapStore, userLimitsApi, forcedFormat = null) { return function getMapStoreMapConfigProviderMiddleware (req, res, next) { const { user } = res.locals; @@ -306,9 +306,9 @@ LayergroupController.prototype.getMapStoreMapConfigProvider = function (mapStore next(); }); }; -}; +} -LayergroupController.prototype.getDataview = function (dataviewBackend) { +function getDataview (dataviewBackend) { return function getDataviewMiddleware (req, res, next) { const { user, mapConfigProvider } = res.locals; const params = getRequestParams(res.locals); @@ -326,9 +326,9 @@ LayergroupController.prototype.getDataview = function (dataviewBackend) { next(); }); }; -}; +} -LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { +function dataviewSearch (dataviewBackend) { return function dataviewSearchMiddleware (req, res, next) { const { user, dataviewName, mapConfigProvider } = res.locals; const params = getRequestParams(res.locals); @@ -346,9 +346,9 @@ LayergroupController.prototype.dataviewSearch = function (dataviewBackend) { next(); }); }; -}; +} -LayergroupController.prototype.attributes = function (attributesBackend) { +function attributes (attributesBackend) { return function attributesMiddleware (req, res, next) { req.profiler.start('windshaft.maplayer_attribute'); @@ -368,7 +368,7 @@ LayergroupController.prototype.attributes = function (attributesBackend) { next(); }); }; -}; +} function getStatusCode(tile, format){ return tile.length === 0 && format === 'mvt'? 204 : 200; @@ -394,7 +394,7 @@ function parseFormat (format = '') { return formatStat; } -LayergroupController.prototype.tile = function (tileBackend) { +function tile (tileBackend) { return function tileMiddleware (req, res, next) { req.profiler.start('windshaft.map_tile'); @@ -420,9 +420,9 @@ LayergroupController.prototype.tile = function (tileBackend) { next(); }); }; -}; +} -LayergroupController.prototype.layer = function (tileBackend) { +function layer (tileBackend) { return function layerMiddleware (req, res, next) { req.profiler.start('windshaft.maplayer_tile'); @@ -448,9 +448,9 @@ LayergroupController.prototype.layer = function (tileBackend) { next(); }); }; -}; +} -LayergroupController.prototype.center = function (previewBackend) { +function center (previewBackend) { return function centerMiddleware (req, res, next) { const width = +req.params.width; const height = +req.params.height; @@ -483,9 +483,9 @@ LayergroupController.prototype.center = function (previewBackend) { next(); }); }; -}; +} -LayergroupController.prototype.bbox = function (previewBackend) { +function bbox (previewBackend) { return function bboxMiddleware (req, res, next) { const width = +req.params.width; const height = +req.params.height; @@ -518,9 +518,9 @@ LayergroupController.prototype.bbox = function (previewBackend) { next(); }); }; -}; +} -LayergroupController.prototype.setLastModifiedHeader = function () { +function setLastModifiedHeader () { return function setLastModifiedHeaderMiddleware (req, res, next) { let { cache_buster: cacheBuster } = res.locals; @@ -532,17 +532,17 @@ LayergroupController.prototype.setLastModifiedHeader = function () { next(); }; -}; +} -LayergroupController.prototype.setCacheControlHeader = function () { +function setCacheControlHeader () { return function setCacheControlHeaderMiddleware (req, res, next) { res.set('Cache-Control', 'public,max-age=31536000'); next(); }; -}; +} -LayergroupController.prototype.getAffectedTables = function (layergroupAffectedTables, pgConnection) { +function getAffectedTables (layergroupAffectedTables, pgConnection) { return function getAffectedTablesMiddleware (req, res, next) { const { user, dbname, token, mapconfig } = res.locals; @@ -583,9 +583,9 @@ LayergroupController.prototype.getAffectedTables = function (layergroupAffectedT }); }); }; -}; +} -LayergroupController.prototype.setCacheChannelHeader = function () { +function setCacheChannelHeader () { return function setCacheChannelHeaderMiddleware (req, res, next) { const { affectedTables } = res.locals; @@ -595,9 +595,9 @@ LayergroupController.prototype.setCacheChannelHeader = function () { next(); }; -}; +} -LayergroupController.prototype.setSurrogateKeyHeader = function (surrogateKeysCache) { +function setSurrogateKeyHeader (surrogateKeysCache) { return function setSurrogateKeyHeaderMiddleware (req, res, next) { const { affectedTables } = res.locals; @@ -607,9 +607,9 @@ LayergroupController.prototype.setSurrogateKeyHeader = function (surrogateKeysCa next(); }; -}; +} -LayergroupController.prototype.incrementSuccessMetrics = function (statsClient) { +function incrementSuccessMetrics (statsClient) { return function incrementSuccessMetricsMiddleware (req, res, next) { const formatStat = parseFormat(req.params.format); @@ -618,9 +618,9 @@ LayergroupController.prototype.incrementSuccessMetrics = function (statsClient) next(); }; -}; +} -LayergroupController.prototype.sendResponse = function () { +function sendResponse () { return function sendResponseMiddleware (req, res) { req.profiler.done('res'); @@ -636,9 +636,9 @@ LayergroupController.prototype.sendResponse = function () { res.send(res.body); } }; -}; +} -LayergroupController.prototype.incrementErrorMetrics = function (statsClient) { +function incrementErrorMetrics (statsClient) { return function incrementErrorMetricsMiddleware (err, req, res, next) { const formatStat = parseFormat(req.params.format); @@ -649,7 +649,7 @@ LayergroupController.prototype.incrementErrorMetrics = function (statsClient) { }; }; -LayergroupController.prototype.tileError = function () { +function tileError () { return function tileErrorMiddleware (err, req, res, next) { // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 let errMsg = err.message ? ( '' + err.message ) : ( '' + err ); @@ -666,4 +666,4 @@ LayergroupController.prototype.tileError = function () { next(err); }; -}; +} From 56213219e49a30cf52a06e4ea68bd100a4ffa9e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 15:25:30 +0100 Subject: [PATCH 33/44] Rename middleware --- lib/cartodb/controllers/layergroup.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 1f6f8d4f..341dc5f3 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -48,7 +48,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), this.prepareContext, getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - tile(this.tileBackend), + getTile(this.tileBackend), setCacheControlHeader(), setLastModifiedHeader(), getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -67,7 +67,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), this.prepareContext, getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - tile(this.tileBackend), + getTile(this.tileBackend), setCacheControlHeader(), setLastModifiedHeader(), getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -394,7 +394,7 @@ function parseFormat (format = '') { return formatStat; } -function tile (tileBackend) { +function getTile (tileBackend) { return function tileMiddleware (req, res, next) { req.profiler.start('windshaft.map_tile'); From c6635f63c10fd3a44436d8ee9634a60ce441778b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 15:39:59 +0100 Subject: [PATCH 34/44] Unify layer and tile middlewares --- lib/cartodb/controllers/layergroup.js | 42 +++++---------------------- 1 file changed, 7 insertions(+), 35 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 341dc5f3..2b37dade 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -48,7 +48,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), this.prepareContext, getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - getTile(this.tileBackend), + getTile(this.tileBackend, 'map_tile'), setCacheControlHeader(), setLastModifiedHeader(), getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -67,7 +67,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), this.prepareContext, getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - getTile(this.tileBackend), + getTile(this.tileBackend, 'map_tile'), setCacheControlHeader(), setLastModifiedHeader(), getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -87,7 +87,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), this.prepareContext, getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - layer(this.tileBackend), + getTile(this.tileBackend, 'maplayer_tile'), setCacheControlHeader(), setLastModifiedHeader(), getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -394,37 +394,9 @@ function parseFormat (format = '') { return formatStat; } -function getTile (tileBackend) { - return function tileMiddleware (req, res, next) { - req.profiler.start('windshaft.map_tile'); - - const { mapConfigProvider } = res.locals; - const params = getRequestParams(res.locals); - - tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats = {}) => { - req.profiler.add(stats); - - if (err) { - return next(err); - } - - if (headers) { - res.set(headers); - } - - const formatStat = parseFormat(req.params.format); - - res.statusCode = getStatusCode(tile, formatStat); - res.body = tile; - - next(); - }); - }; -} - -function layer (tileBackend) { - return function layerMiddleware (req, res, next) { - req.profiler.start('windshaft.maplayer_tile'); +function getTile (tileBackend, profileLabel = 'tile') { + return function getTileMiddleware (req, res, next) { + req.profiler.start(`windshaft.${profileLabel}`); const { mapConfigProvider } = res.locals; const params = getRequestParams(res.locals); @@ -647,7 +619,7 @@ function incrementErrorMetrics (statsClient) { next(err); }; -}; +} function tileError () { return function tileErrorMiddleware (err, req, res, next) { From 9dcd5ff33238a41ae18aa9159067f06e03c6112f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 15:56:16 +0100 Subject: [PATCH 35/44] Impreve naming --- lib/cartodb/controllers/layergroup.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 2b37dade..452cdfca 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -82,7 +82,7 @@ LayergroupController.prototype.register = function(app) { app.get( `${basePath}/:token/:layer/:z/:x/:y.(:format)`, - validateLayerRoute(), + distinguishLayergroupFromStaticRoute(), cors(), userMiddleware(), this.prepareContext, @@ -241,8 +241,8 @@ LayergroupController.prototype.register = function(app) { ); }; -function validateLayerRoute () { - return function validateLayerRouteMiddleware(req, res, next) { +function distinguishLayergroupFromStaticRoute () { + return function distinguishLayergroupFromStaticRouteMiddleware(req, res, next) { if (req.params.token === 'static') { return next('route'); } From 83ab65163d92c85629f8f8edc8c414913bc58679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 18:41:41 +0100 Subject: [PATCH 36/44] Rename attributes middleware --- lib/cartodb/controllers/layergroup.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 452cdfca..7c487c1e 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -106,7 +106,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), this.prepareContext, getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), - attributes(this.attributesBackend), + getFeatureAttributes(this.attributesBackend), setCacheControlHeader(), setLastModifiedHeader(), getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -348,8 +348,8 @@ function dataviewSearch (dataviewBackend) { }; } -function attributes (attributesBackend) { - return function attributesMiddleware (req, res, next) { +function getFeatureAttributes (attributesBackend) { + return function getFeatureAttributesMiddleware (req, res, next) { req.profiler.start('windshaft.maplayer_attribute'); const { mapConfigProvider } = res.locals; From faaf121eb63c0826246e3ac3a91ffb11e3e3835a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 18:51:43 +0100 Subject: [PATCH 37/44] Rename center and bbox middlewares --- lib/cartodb/controllers/layergroup.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 7c487c1e..9fbcce2c 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -124,7 +124,7 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(['layer']), this.prepareContext, getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), - center(this.previewBackend), + getPreviewImageByCenter(this.previewBackend), setCacheControlHeader(), setLastModifiedHeader(), getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -140,7 +140,7 @@ LayergroupController.prototype.register = function(app) { allowQueryParams(['layer']), this.prepareContext, getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), - bbox(this.previewBackend), + getPreviewImageByBoundingBox(this.previewBackend), setCacheControlHeader(), setLastModifiedHeader(), getAffectedTables(this.layergroupAffectedTables, this.pgConnection), @@ -422,8 +422,8 @@ function getTile (tileBackend, profileLabel = 'tile') { }; } -function center (previewBackend) { - return function centerMiddleware (req, res, next) { +function getPreviewImageByCenter (previewBackend) { + return function getPreviewImageByCenterMiddleware (req, res, next) { const width = +req.params.width; const height = +req.params.height; const zoom = +req.params.z; @@ -457,8 +457,8 @@ function center (previewBackend) { }; } -function bbox (previewBackend) { - return function bboxMiddleware (req, res, next) { +function getPreviewImageByBoundingBox (previewBackend) { + return function getPreviewImageByBoundingBoxMiddleware (req, res, next) { const width = +req.params.width; const height = +req.params.height; const bounds = { From 6c2f8936516f496259bbff0a36d6a0ee5e2ad845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 7 Mar 2018 18:53:20 +0100 Subject: [PATCH 38/44] Rename map-store-map-config-provider middleware --- lib/cartodb/controllers/layergroup.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 9fbcce2c..9e09bec9 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -47,7 +47,7 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware(), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), getTile(this.tileBackend, 'map_tile'), setCacheControlHeader(), setLastModifiedHeader(), @@ -66,7 +66,7 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware(), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), getTile(this.tileBackend, 'map_tile'), setCacheControlHeader(), setLastModifiedHeader(), @@ -86,7 +86,7 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware(), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), getTile(this.tileBackend, 'maplayer_tile'), setCacheControlHeader(), setLastModifiedHeader(), @@ -105,7 +105,7 @@ LayergroupController.prototype.register = function(app) { cors(), userMiddleware(), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), getFeatureAttributes(this.attributesBackend), setCacheControlHeader(), setLastModifiedHeader(), @@ -123,7 +123,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), allowQueryParams(['layer']), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), getPreviewImageByCenter(this.previewBackend), setCacheControlHeader(), setLastModifiedHeader(), @@ -139,7 +139,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), allowQueryParams(['layer']), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), getPreviewImageByBoundingBox(this.previewBackend), setCacheControlHeader(), setLastModifiedHeader(), @@ -173,7 +173,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), getDataview(this.dataviewBackend), setCacheControlHeader(), setLastModifiedHeader(), @@ -189,7 +189,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), getDataview(this.dataviewBackend), setCacheControlHeader(), setLastModifiedHeader(), @@ -205,7 +205,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), dataviewSearch(this.dataviewBackend), setCacheControlHeader(), setLastModifiedHeader(), @@ -221,7 +221,7 @@ LayergroupController.prototype.register = function(app) { userMiddleware(), allowQueryParams(allowedDataviewQueryParams), this.prepareContext, - getMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), dataviewSearch(this.dataviewBackend), setCacheControlHeader(), setLastModifiedHeader(), @@ -282,8 +282,8 @@ function getRequestParams(locals) { return params; } -function getMapStoreMapConfigProvider (mapStore, userLimitsApi, forcedFormat = null) { - return function getMapStoreMapConfigProviderMiddleware (req, res, next) { +function createMapStoreMapConfigProvider (mapStore, userLimitsApi, forcedFormat = null) { + return function createMapStoreMapConfigProviderMiddleware (req, res, next) { const { user } = res.locals; const params = getRequestParams(res.locals); From abffc4b067b2a14cdf64ca9527a15ef6dd0d4406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Mar 2018 12:23:00 +0100 Subject: [PATCH 39/44] Uppercase for actual constants --- lib/cartodb/controllers/layergroup.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 9e09bec9..6711b401 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -6,6 +6,14 @@ const DataviewBackend = require('../backends/dataview'); const AnalysisStatusBackend = require('../backends/analysis-status'); const MapStoreMapConfigProvider = require('../models/mapconfig/provider/map-store-provider'); const QueryTables = require('cartodb-query-tables'); +const SUPPORTED_FORMATS = { + grid_json: true, + json_torque: true, + torque_json: true, + png: true, + png32: true, + mvt: true +}; /** * @param {prepareContext} prepareContext @@ -374,20 +382,11 @@ function getStatusCode(tile, format){ return tile.length === 0 && format === 'mvt'? 204 : 200; } -const supportedFormats = { - grid_json: true, - json_torque: true, - torque_json: true, - png: true, - png32: true, - mvt: true -}; - function parseFormat (format = '') { const prettyFormat = format.replace('.', '_'); let formatStat = 'invalid'; - if (supportedFormats[prettyFormat]) { + if (SUPPORTED_FORMATS[prettyFormat]) { formatStat = prettyFormat; } From 3f6f2e4e23bee0af43a53075c3a14cb12c33652c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Mar 2018 12:23:43 +0100 Subject: [PATCH 40/44] Use template string --- lib/cartodb/controllers/layergroup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 6711b401..9838838b 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -435,7 +435,7 @@ function getPreviewImageByCenter (previewBackend) { const { mapConfigProvider: provider } = res.locals; previewBackend.getImage(provider, format, width, height, zoom, center, (err, image, headers, stats = {}) => { - req.profiler.done('render-' + format); + req.profiler.done(`render-${format}`); req.profiler.add(stats); if (err) { From 555e04f9e7a4904e8054af011f51c449028d3638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Mar 2018 12:27:49 +0100 Subject: [PATCH 41/44] Use ternary operator --- lib/cartodb/controllers/layergroup.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 9838838b..a9eb5899 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -384,13 +384,7 @@ function getStatusCode(tile, format){ function parseFormat (format = '') { const prettyFormat = format.replace('.', '_'); - let formatStat = 'invalid'; - - if (SUPPORTED_FORMATS[prettyFormat]) { - formatStat = prettyFormat; - } - - return formatStat; + return SUPPORTED_FORMATS[prettyFormat] ? prettyFormat : 'invalid'; } function getTile (tileBackend, profileLabel = 'tile') { From 49bcc5368d9478135d5c0054809146302ad8036b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Mar 2018 12:30:27 +0100 Subject: [PATCH 42/44] Use base number as radix to pare intergers --- lib/cartodb/controllers/layergroup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index a9eb5899..61e5bf85 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -489,7 +489,7 @@ function setLastModifiedHeader () { return function setLastModifiedHeaderMiddleware (req, res, next) { let { cache_buster: cacheBuster } = res.locals; - cacheBuster = parseInt(cacheBuster); + cacheBuster = parseInt(cacheBuster, 10); const lastUpdated = res.locals.cache_buster ? new Date(cacheBuster) : new Date(); From aae814a15600fc4ebdc3b4f7ecbc0b28c8c20e72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Mar 2018 12:35:54 +0100 Subject: [PATCH 43/44] Use template strings --- lib/cartodb/controllers/layergroup.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 61e5bf85..182fcff5 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -441,7 +441,7 @@ function getPreviewImageByCenter (previewBackend) { res.set(headers); } - res.set('Content-Type', headers['Content-Type'] || 'image/' + format); + res.set('Content-Type', headers['Content-Type'] || `image/${format}`); res.body = image; @@ -464,7 +464,7 @@ function getPreviewImageByBoundingBox (previewBackend) { const { mapConfigProvider: provider } = res.locals; previewBackend.getImage(provider, format, width, height, bounds, (err, image, headers, stats = {}) => { - req.profiler.done('render-' + format); + req.profiler.done(`render-${format}`); req.profiler.add(stats); if (err) { @@ -476,7 +476,7 @@ function getPreviewImageByBoundingBox (previewBackend) { res.set(headers); } - res.set('Content-Type', headers['Content-Type'] || 'image/' + format); + res.set('Content-Type', headers['Content-Type'] || `image/${format}`); res.body = image; @@ -518,7 +518,7 @@ function getAffectedTables (layergroupAffectedTables, pgConnection) { pgConnection.getConnection(user, (err, connection) => { if (err) { - global.logger.warn('ERROR generating cache channel: ' + err); + global.logger.warn('ERROR generating cache channel:', err); return next(); } @@ -527,7 +527,7 @@ function getAffectedTables (layergroupAffectedTables, pgConnection) { sql.push(layer.options.sql); if (layer.options.affected_tables) { layer.options.affected_tables.map(function(table) { - sql.push('SELECT * FROM ' + table + ' LIMIT 0'); + sql.push(`SELECT * FROM ${table} LIMIT 0`); }); } }); @@ -535,7 +535,7 @@ function getAffectedTables (layergroupAffectedTables, pgConnection) { QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), (err, affectedTables) => { req.profiler.done('getAffectedTablesFromQuery'); if (err) { - global.logger.warn('ERROR generating cache channel: ' + err); + global.logger.warn('ERROR generating cache channel: ', err); return next(); } @@ -623,7 +623,7 @@ function tileError () { const matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); if (matches) { - errMsg = 'style' + matches[2] + ': ' + matches[1]; + errMsg = `style${matches[2]}: ${matches[1]}`; } err.message = errMsg; From d8202d881d37fd71ff4851fc2dc3d95da33a819d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Mar 2018 12:46:04 +0100 Subject: [PATCH 44/44] Remove legacy test --- test/unit/cartodb/ported/tile_stats.test.js | 90 --------------------- 1 file changed, 90 deletions(-) delete mode 100644 test/unit/cartodb/ported/tile_stats.test.js diff --git a/test/unit/cartodb/ported/tile_stats.test.js b/test/unit/cartodb/ported/tile_stats.test.js deleted file mode 100644 index f8e0d80a..00000000 --- a/test/unit/cartodb/ported/tile_stats.test.js +++ /dev/null @@ -1,90 +0,0 @@ -require('../../../support/test_helper.js'); - -var assert = require('assert'); - -var LayergroupController = require('../../../../lib/cartodb/controllers/layergroup'); - -describe('tile stats', function() { - - beforeEach(function () { - this.statsClient = global.statsClient; - }); - - afterEach(function() { - global.statsClient = this.statsClient; - }); - - it.skip('finalizeGetTileOrGrid does not call statsClient when format is not supported', function() { - var expectedCalls = 2, // it will call increment once for the general error - invalidFormat = 'png2', - invalidFormatRegexp = new RegExp('invalid'), - formatMatched = false; - mockStatsClientGetInstance({ - increment: function(label) { - formatMatched = formatMatched || !!label.match(invalidFormatRegexp); - expectedCalls--; - } - }); - - var layergroupController = new LayergroupController(); - - var reqMock = { - profiler: { toJSONString:function() {} }, - params: { - format: invalidFormat - } - }; - var resMock = { - status: function() { return this; }, - set: function() {}, - json: function() {}, - jsonp: function() {}, - send: function() {} - }; - - var next = function () {}; - layergroupController.finalizeGetTileOrGrid('Unsupported format png2', reqMock, resMock, null, null, next); - - assert.ok(formatMatched, 'Format was never matched in increment method'); - assert.equal(expectedCalls, 0, 'Unexpected number of calls to increment method'); - }); - - it.skip('finalizeGetTileOrGrid calls statsClient when format is supported', function() { - var expectedCalls = 2, // general error + format error - validFormat = 'png', - validFormatRegexp = new RegExp(validFormat), - formatMatched = false; - mockStatsClientGetInstance({ - increment: function(label) { - formatMatched = formatMatched || !!label.match(validFormatRegexp); - expectedCalls--; - } - }); - var reqMock = { - profiler: { toJSONString:function() {} }, - params: { - format: validFormat - } - }; - var resMock = { - status: function() { return this; }, - set: function() {}, - json: function() {}, - jsonp: function() {}, - send: function() {} - }; - - var layergroupController = new LayergroupController(); - - var next = function () {}; - layergroupController.finalizeGetTileOrGrid('Another error happened', reqMock, resMock, null, null, next); - - assert.ok(formatMatched, 'Format was never matched in increment method'); - assert.equal(expectedCalls, 0, 'Unexpected number of calls to increment method'); - }); - - function mockStatsClientGetInstance(instance) { - global.statsClient = Object.assign(global.statsClient, instance); - } - -});