From 8333b39928cce0ba3082cca641cc259447ec74fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 19 Mar 2018 19:16:18 +0100 Subject: [PATCH 01/62] Use res.body as placeholder of layergroup --- lib/cartodb/controllers/map.js | 40 ++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 52d71f93..9f15383f 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -280,7 +280,7 @@ function createLayergroup (mapBackend, userLimitsApi) { return next(err); } - res.locals.layergroup = layergroup; + res.body = layergroup; next(); }); @@ -299,7 +299,7 @@ function instantiateLayergroup (mapBackend, userLimitsApi) { return next(err); } - res.locals.layergroup = layergroup; + res.body = layergroup; const { mapconfigProvider } = res.locals; @@ -332,7 +332,7 @@ function incrementMapViewCount (metadataBackend) { function augmentLayergroupData () { return function augmentLayergroupDataMiddleware (req, res, next) { - const { layergroup } = res.locals; + const layergroup = res.body; // include in layergroup response the variables in serverMedata // those variables are useful to send to the client information @@ -345,7 +345,8 @@ function augmentLayergroupData () { function getAffectedTables (pgConnection, layergroupAffectedTables) { return function getAffectedTablesMiddleware (req, res, next) { - const { dbname, layergroup, user, mapconfig } = res.locals; + const { dbname, user, mapconfig } = res.locals; + const layergroup = res.body; pgConnection.getConnection(user, (err, connection) => { if (err) { @@ -403,7 +404,8 @@ function setLastModified () { function setLastUpdatedTimeToLayergroup () { return function setLastUpdatedTimeToLayergroupMiddleware (req, res, next) { - const { affectedTables, layergroup, analysesResults } = res.locals; + const { affectedTables, analysesResults } = res.locals; + const layergroup = res.body; var lastUpdateTime = affectedTables.getLastUpdatedAt(); @@ -443,7 +445,8 @@ function setCacheControl () { function setLayerStats (pgConnection, statsBackend) { return function setLayerStatsMiddleware(req, res, next) { - const { user, mapconfig, layergroup } = res.locals; + const { user, mapconfig } = res.locals; + const layergroup = res.body; pgConnection.getConnection(user, (err, connection) => { if (err) { @@ -469,7 +472,8 @@ function setLayerStats (pgConnection, statsBackend) { function setLayergroupIdHeader (templateMaps, useTemplateHash) { return function setLayergroupIdHeaderMiddleware (req, res, next) { - const { layergroup, user, template } = res.locals; + const { user, template } = res.locals; + const layergroup = res.body; if (useTemplateHash) { var templateHash = templateMaps.fingerPrint(template).substring(0, 8); @@ -484,7 +488,8 @@ function setLayergroupIdHeader (templateMaps, useTemplateHash) { function setDataviewsAndWidgetsUrlsToLayergroupMetadata (layergroupMetadata) { return function setDataviewsAndWidgetsUrlsToLayergroupMetadataMiddleware (req, res, next) { - const { layergroup, user, mapconfig } = res.locals; + const { user, mapconfig } = res.locals; + const layergroup = res.body; layergroupMetadata.addDataviewsAndWidgetsUrls(user, layergroup, mapconfig.obj()); @@ -494,7 +499,8 @@ function setDataviewsAndWidgetsUrlsToLayergroupMetadata (layergroupMetadata) { function setAnalysesMetadataToLayergroup (layergroupMetadata, includeQuery) { return function setAnalysesMetadataToLayergroupMiddleware (req, res, next) { - const { layergroup, user, analysesResults = [] } = res.locals; + const { user, analysesResults = [] } = res.locals; + const layergroup = res.body; layergroupMetadata.addAnalysesMetadata(user, layergroup, analysesResults, includeQuery); @@ -504,7 +510,8 @@ function setAnalysesMetadataToLayergroup (layergroupMetadata, includeQuery) { function setTurboCartoMetadataToLayergroup (layergroupMetadata) { return function setTurboCartoMetadataToLayergroupMiddleware (req, res, next) { - const { layergroup, mapconfig, context } = res.locals; + const { mapconfig, context } = res.locals; + const layergroup = res.body; layergroupMetadata.addTurboCartoContextMetadata(layergroup, mapconfig.obj(), context); @@ -514,7 +521,8 @@ function setTurboCartoMetadataToLayergroup (layergroupMetadata) { function setAggregationMetadataToLayergroup (layergroupMetadata) { return function setAggregationMetadataToLayergroupMiddleware (req, res, next) { - const { layergroup, mapconfig, context } = res.locals; + const { mapconfig, context } = res.locals; + const layergroup = res.body; layergroupMetadata.addAggregationContextMetadata(layergroup, mapconfig.obj(), context); @@ -524,7 +532,8 @@ function setAggregationMetadataToLayergroup (layergroupMetadata) { function setTilejsonMetadataToLayergroup (layergroupMetadata) { return function augmentLayergroupTilejsonMiddleware (req, res, next) { - const { layergroup, user, mapconfig } = res.locals; + const { user, mapconfig } = res.locals; + const layergroup = res.body; layergroupMetadata.addTileJsonMetadata(layergroup, user, mapconfig); @@ -551,14 +560,13 @@ function setSurrogateKeyHeader (surrogateKeysCache) { function sendResponse () { return function sendResponseMiddleware (req, res) { req.profiler.done('res'); - const { layergroup } = res.locals; - res.status(200); + res.status(res.statusCode || 200); if (req.query && req.query.callback) { - res.jsonp(layergroup); + res.jsonp(res.body); } else { - res.json(layergroup); + res.json(res.body); } }; } From bb170ee208ea188a4d08791712c2cfdf35a79c9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 19 Mar 2018 19:27:38 +0100 Subject: [PATCH 02/62] Please, jshint --- test/acceptance/ported/support/test_client.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/acceptance/ported/support/test_client.js b/test/acceptance/ported/support/test_client.js index f110079a..ab4576b6 100644 --- a/test/acceptance/ported/support/test_client.js +++ b/test/acceptance/ported/support/test_client.js @@ -456,17 +456,17 @@ function withLayergroup(layergroupConfig, options, callback) { const signerTpl = function ({ signer }) { return `${signer ? `:${signer}@` : ''}`; - } + }; const cacheTpl = function ({ cache_buster, cacheBuster }) { return `${cache_buster ? `:${cache_buster}` : `:${cacheBuster}`}`; - } + }; const urlTpl = function ({layergroupid, cache_buster = null, tile }) { const { signer, token , cacheBuster } = LayergroupToken.parse(layergroupid); - const base = '/database/windshaft_test/layergroup/' + const base = '/database/windshaft_test/layergroup/'; return `${base}${signerTpl({signer})}${token}${cacheTpl({cache_buster, cacheBuster})}${tile}`; - } + }; const finalUrl = urlTpl({ layergroupid, cache_buster: options.cache_buster, tile: layergroupUrl }); From 9211fa065b46acd306ff7a6d29dae67855a5dd95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 19 Mar 2018 19:48:14 +0100 Subject: [PATCH 03/62] Extract sendResponse middleware --- lib/cartodb/controllers/analyses.js | 13 +------------ lib/cartodb/controllers/layergroup.js | 19 +------------------ lib/cartodb/controllers/map.js | 15 +-------------- lib/cartodb/controllers/named_maps.js | 16 ++++------------ lib/cartodb/controllers/named_maps_admin.js | 10 +--------- lib/cartodb/middleware/send-response.js | 17 +++++++++++++++++ 6 files changed, 25 insertions(+), 65 deletions(-) create mode 100644 lib/cartodb/middleware/send-response.js diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index 57e64a61..908ab3c6 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -7,6 +7,7 @@ const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const authorize = require('../middleware/authorize'); const dbConnSetup = require('../middleware/db-conn-setup'); +const sendResponse = require('../middleware/send-response'); function AnalysesController(pgConnection, authApi) { this.pgConnection = pgConnection; @@ -114,18 +115,6 @@ function setCacheControlHeader () { }; } -function sendResponse () { - return function sendResponseMiddleware (req, res) { - res.status(200); - - if (req.query && req.query.callback) { - res.jsonp(res.body); - } else { - res.json(res.body); - } - }; -} - function unauthorizedError () { return function unathorizedErrorMiddleware(err, req, res, next) { if (err.message.match(/permission\sdenied/)) { diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index f3976cb6..53d2ba7f 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -7,6 +7,7 @@ const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); +const sendResponse = require('../middleware/send-response'); const DataviewBackend = require('../backends/dataview'); const AnalysisStatusBackend = require('../backends/analysis-status'); const MapStoreMapConfigProvider = require('../models/mapconfig/provider/map-store-provider'); @@ -644,24 +645,6 @@ function incrementSuccessMetrics (statsClient) { }; } -function sendResponse () { - 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); - } - }; -} - function incrementErrorMetrics (statsClient) { return function incrementErrorMetricsMiddleware (err, req, res, next) { const formatStat = parseFormat(req.params.format); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 9f15383f..8022c7a2 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -12,6 +12,7 @@ const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); +const sendResponse = require('../middleware/send-response'); const NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); const NamedMapMapConfigProvider = require('../models/mapconfig/provider/named-map-provider'); const CreateLayergroupMapConfigProvider = require('../models/mapconfig/provider/create-layergroup-provider'); @@ -557,20 +558,6 @@ function setSurrogateKeyHeader (surrogateKeysCache) { }; } -function sendResponse () { - return function sendResponseMiddleware (req, res) { - req.profiler.done('res'); - - res.status(res.statusCode || 200); - - if (req.query && req.query.callback) { - res.jsonp(res.body); - } else { - res.json(res.body); - } - }; -} - function augmentError (options) { const { addContext = false, label = 'MAPS CONTROLLER' } = options; diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 6bff861c..95fe24b8 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -7,6 +7,7 @@ const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); +const sendResponse = require('../middleware/send-response'); const vectorError = require('../middleware/vector-error'); const DEFAULT_ZOOM_CENTER = { @@ -214,10 +215,11 @@ function prepareLayerFilterFromPreviewLayers ({ namedMapProviderCache, label }) function getTile ({ tileBackend, label }) { return function getTileMiddleware (req, res, next) { - const { namedMapProvider } = res.locals; + const { namedMapProvider, format } = res.locals; tileBackend.getTile(namedMapProvider, req.params, (err, tile, headers, stats) => { req.profiler.add(stats); + req.profiler.done('render-' + format); if (err) { err.label = label; @@ -371,6 +373,7 @@ function getImage({ previewBackend, label }) { previewBackend.getImage(namedMapProvider, format, width, height, bounds, (err, image, headers, stats) => { req.profiler.add(stats); + req.profiler.done('render-' + format); if (err) { err.label = label; @@ -514,14 +517,3 @@ function setContentTypeHeader () { next(); }; } - -function sendResponse () { - return function sendResponseMiddleware (req, res) { - const { format } = res.locals; - - req.profiler.done('render-' + format); - - res.status(200); - res.send(res.body); - }; -} diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index c2e7725e..f70af2cf 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -3,6 +3,7 @@ const cors = require('../middleware/cors'); const user = require('../middleware/user'); const locals = require('../middleware/locals'); const credentials = require('../middleware/credentials'); +const sendResponse = require('../middleware/send-response'); /** * @param {AuthApi} authApi @@ -216,12 +217,3 @@ function listTemplates ({ templateMaps }) { }); }; } - -function sendResponse () { - return function sendResponseMiddleware (req, res) { - res.status(res.statusCode || 200); - - const method = req.query.callback ? 'jsonp' : 'json'; - res[method](res.body); - }; -} diff --git a/lib/cartodb/middleware/send-response.js b/lib/cartodb/middleware/send-response.js new file mode 100644 index 00000000..469cf0a7 --- /dev/null +++ b/lib/cartodb/middleware/send-response.js @@ -0,0 +1,17 @@ +module.exports = function sendResponse () { + return function sendResponseMiddleware (req, res) { + req.profiler.done('res'); + + res.status(res.statusCode || 200); + + if (Buffer.isBuffer(res.body)) { + return res.send(res.body); + } + + if (req.query.callback) { + return res.jsonp(res.body); + } + + res.json(res.body); + }; +}; From 325bdfe92ffe51b2a733c1638942f66a9e703b2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 20 Mar 2018 09:34:06 +0100 Subject: [PATCH 04/62] Move middleware --- lib/cartodb/controllers/map.js | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 8022c7a2..61d42ed4 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -89,6 +89,7 @@ MapController.prototype.composeCreateMapMiddleware = function (useTemplate = fal augmentLayergroupData(), getAffectedTables(this.pgConnection, this.layergroupAffectedTables), setCacheChannel(), + setSurrogateKeyHeader(this.surrogateKeysCache), setLastModified(), setLastUpdatedTimeToLayergroup(), setCacheControl(), @@ -99,7 +100,6 @@ MapController.prototype.composeCreateMapMiddleware = function (useTemplate = fal setTurboCartoMetadataToLayergroup(this.layergroupMetadata), setAggregationMetadataToLayergroup(this.layergroupMetadata), setTilejsonMetadataToLayergroup(this.layergroupMetadata), - setSurrogateKeyHeader(this.surrogateKeysCache), sendResponse(), augmentError({ label, addContext }) ]; @@ -393,6 +393,22 @@ function setCacheChannel () { }; } +function setSurrogateKeyHeader (surrogateKeysCache) { + return function setSurrogateKeyHeaderMiddleware(req, res, next) { + const { affectedTables, user, templateName } = res.locals; + + if (req.method === 'GET' && affectedTables.tables && affectedTables.tables.length > 0) { + surrogateKeysCache.tag(res, affectedTables); + } + + if (templateName) { + surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, templateName)); + } + + next(); + }; +} + function setLastModified () { return function setLastModifiedMiddleware (req, res, next) { if (req.method === 'GET') { @@ -542,22 +558,6 @@ function setTilejsonMetadataToLayergroup (layergroupMetadata) { }; } -function setSurrogateKeyHeader (surrogateKeysCache) { - return function setSurrogateKeyHeaderMiddleware(req, res, next) { - const { affectedTables, user, templateName } = res.locals; - - if (req.method === 'GET' && affectedTables.tables && affectedTables.tables.length > 0) { - surrogateKeysCache.tag(res, affectedTables); - } - - if (templateName) { - surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, templateName)); - } - - next(); - }; -} - function augmentError (options) { const { addContext = false, label = 'MAPS CONTROLLER' } = options; From 9fd2519c12b5982c06d3f37d472f99aafa06fbfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 20 Mar 2018 09:34:50 +0100 Subject: [PATCH 05/62] Rename middleware --- lib/cartodb/controllers/map.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 61d42ed4..d9834a05 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -88,7 +88,7 @@ MapController.prototype.composeCreateMapMiddleware = function (useTemplate = fal incrementMapViewCount(this.metadataBackend), augmentLayergroupData(), getAffectedTables(this.pgConnection, this.layergroupAffectedTables), - setCacheChannel(), + setCacheChannelHeader(), setSurrogateKeyHeader(this.surrogateKeysCache), setLastModified(), setLastUpdatedTimeToLayergroup(), @@ -381,8 +381,8 @@ function getAffectedTables (pgConnection, layergroupAffectedTables) { }; } -function setCacheChannel () { - return function setCacheChannelMiddleware (req, res, next) { +function setCacheChannelHeader () { + return function setCacheChannelHeaderMiddleware (req, res, next) { const { affectedTables } = res.locals; if (req.method === 'GET') { From a142620b70311f2654cff6dd5a10284e35c5013b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Mar 2018 14:11:54 +0100 Subject: [PATCH 06/62] Make generic middlewares to calculate surrogate key and cache channel headers: - In controllers: all reference to map config are now camelized, for instance: mapconfig -> mapConfig or mapconfigProvider -> mapConfigProvider - In controllers: all map config providers created in req/res cycle are saved into `res.locals` and `mapConfigProvider` as key. - In map-config-providers: all of them implement `.getAffectedTables()`, in order to calculate the tables involved for a given map-config. For that, `pgConnection` and `affectedTablesCache` are injected as constructor argument. - Named Map Provider: rename references from `affectedTablesAndLastUpdate` to `affectedTables`. - Named Map Provider Cache: In order to create new named map providers, needs affectedTablesCache. - Extract locals middlewares (surrogate-key and cache-channel) from controllers and create an unified version of them. - Extract last-modified middleware from named maps controller (draft). --- lib/cartodb/cache/named_map_provider_cache.js | 11 +- lib/cartodb/controllers/layergroup.js | 252 ++++++++---------- lib/cartodb/controllers/map.js | 213 +++++++-------- lib/cartodb/controllers/named_maps.js | 122 ++------- .../middleware/cache-channel-header.js | 24 ++ .../middleware/last-modified-header.js | 40 +++ .../middleware/surrogate-key-header.js | 31 +++ .../provider/create-layergroup-provider.js | 64 ++++- .../mapconfig/provider/map-store-provider.js | 82 +++++- .../mapconfig/provider/named-map-provider.js | 67 ++++- lib/cartodb/server.js | 3 +- 11 files changed, 538 insertions(+), 371 deletions(-) create mode 100644 lib/cartodb/middleware/cache-channel-header.js create mode 100644 lib/cartodb/middleware/last-modified-header.js create mode 100644 lib/cartodb/middleware/surrogate-key-header.js diff --git a/lib/cartodb/cache/named_map_provider_cache.js b/lib/cartodb/cache/named_map_provider_cache.js index ebafbbac..e0850822 100644 --- a/lib/cartodb/cache/named_map_provider_cache.js +++ b/lib/cartodb/cache/named_map_provider_cache.js @@ -6,12 +6,20 @@ var queue = require('queue-async'); var LruCache = require("lru-cache"); -function NamedMapProviderCache(templateMaps, pgConnection, metadataBackend, userLimitsApi, mapConfigAdapter) { +function NamedMapProviderCache( + templateMaps, + pgConnection, + metadataBackend, + userLimitsApi, + mapConfigAdapter, + affectedTablesCache +) { this.templateMaps = templateMaps; this.pgConnection = pgConnection; this.metadataBackend = metadataBackend; this.userLimitsApi = userLimitsApi; this.mapConfigAdapter = mapConfigAdapter; + this.affectedTablesCache = affectedTablesCache; this.providerCache = new LruCache({ max: 2000 }); } @@ -30,6 +38,7 @@ NamedMapProviderCache.prototype.get = function(user, templateId, config, authTok this.metadataBackend, this.userLimitsApi, this.mapConfigAdapter, + this.affectedTablesCache, user, templateId, config, diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 29adf431..b7b6803a 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -9,11 +9,12 @@ const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); const rateLimit = require('../middleware/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const cacheChannelHeader = require('../middleware/cache-channel-header'); +const surrogateKeyHeader = require('../middleware/surrogate-key-header'); const sendResponse = require('../middleware/send-response'); 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, @@ -44,7 +45,7 @@ function LayergroupController( attributesBackend, surrogateKeysCache, userLimitsApi, - layergroupAffectedTables, + layergroupAffectedTablesCache, analysisBackend, authApi ) { @@ -55,7 +56,7 @@ function LayergroupController( this.attributesBackend = attributesBackend; this.surrogateKeysCache = surrogateKeysCache; this.userLimitsApi = userLimitsApi; - this.layergroupAffectedTables = layergroupAffectedTables; + this.layergroupAffectedTablesCache = layergroupAffectedTablesCache; this.dataviewBackend = new DataviewBackend(analysisBackend); this.analysisStatusBackend = new AnalysisStatusBackend(); @@ -65,10 +66,10 @@ function LayergroupController( module.exports = LayergroupController; LayergroupController.prototype.register = function(app) { - const { base_url_mapconfig: mapconfigBasePath } = app; + const { base_url_mapconfig: mapConfigBasePath } = app; app.get( - `${mapconfigBasePath}/:token/:z/:x/:y@:scale_factor?x.:format`, + `${mapConfigBasePath}/:token/:z/:x/:y@:scale_factor?x.:format`, cors(), cleanUpQueryParams(), locals(), @@ -78,13 +79,17 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), getTile(this.tileBackend, 'map_tile'), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), incrementSuccessMetrics(global.statsClient), sendResponse(), incrementErrorMetrics(global.statsClient), @@ -93,7 +98,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - `${mapconfigBasePath}/:token/:z/:x/:y.:format`, + `${mapConfigBasePath}/:token/:z/:x/:y.:format`, cors(), cleanUpQueryParams(), locals(), @@ -103,13 +108,17 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), getTile(this.tileBackend, 'map_tile'), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), incrementSuccessMetrics(global.statsClient), sendResponse(), incrementErrorMetrics(global.statsClient), @@ -118,7 +127,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - `${mapconfigBasePath}/:token/:layer/:z/:x/:y.(:format)`, + `${mapConfigBasePath}/:token/:layer/:z/:x/:y.(:format)`, distinguishLayergroupFromStaticRoute(), cors(), cleanUpQueryParams(), @@ -129,13 +138,17 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), getTile(this.tileBackend, 'maplayer_tile'), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), incrementSuccessMetrics(global.statsClient), sendResponse(), incrementErrorMetrics(global.statsClient), @@ -144,7 +157,7 @@ LayergroupController.prototype.register = function(app) { ); app.get( - `${mapconfigBasePath}/:token/:layer/attributes/:fid`, + `${mapConfigBasePath}/:token/:layer/attributes/:fid`, cors(), cleanUpQueryParams(), locals(), @@ -154,20 +167,24 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), getFeatureAttributes(this.attributesBackend), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), sendResponse() ); const forcedFormat = 'png'; app.get( - `${mapconfigBasePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, + `${mapConfigBasePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, cors(), cleanUpQueryParams(['layer']), locals(), @@ -177,18 +194,23 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache, + forcedFormat + ), getPreviewImageByCenter(this.previewBackend), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), sendResponse() ); app.get( - `${mapconfigBasePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, + `${mapConfigBasePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, cors(), cleanUpQueryParams(['layer']), locals(), @@ -198,13 +220,18 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi, forcedFormat), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache, + forcedFormat + ), getPreviewImageByBoundingBox(this.previewBackend), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), sendResponse() ); @@ -227,7 +254,7 @@ LayergroupController.prototype.register = function(app) { ]; app.get( - `${mapconfigBasePath}/:token/dataview/:dataviewName`, + `${mapConfigBasePath}/:token/dataview/:dataviewName`, cors(), cleanUpQueryParams(allowedDataviewQueryParams), locals(), @@ -237,18 +264,22 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), getDataview(this.dataviewBackend), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), sendResponse() ); app.get( - `${mapconfigBasePath}/:token/:layer/widget/:dataviewName`, + `${mapConfigBasePath}/:token/:layer/widget/:dataviewName`, cors(), cleanUpQueryParams(allowedDataviewQueryParams), locals(), @@ -258,18 +289,22 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), getDataview(this.dataviewBackend), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), sendResponse() ); app.get( - `${mapconfigBasePath}/:token/dataview/:dataviewName/search`, + `${mapConfigBasePath}/:token/dataview/:dataviewName/search`, cors(), cleanUpQueryParams(allowedDataviewQueryParams), locals(), @@ -279,18 +314,22 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), dataviewSearch(this.dataviewBackend), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), sendResponse() ); app.get( - `${mapconfigBasePath}/:token/:layer/widget/:dataviewName/search`, + `${mapConfigBasePath}/:token/:layer/widget/:dataviewName/search`, cors(), cleanUpQueryParams(allowedDataviewQueryParams), locals(), @@ -300,18 +339,22 @@ LayergroupController.prototype.register = function(app) { credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), - createMapStoreMapConfigProvider(this.mapStore, this.userLimitsApi), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), dataviewSearch(this.dataviewBackend), setCacheControlHeader(), setLastModifiedHeader(), - getAffectedTables(this.layergroupAffectedTables, this.pgConnection, this.mapStore), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), sendResponse() ); app.get( - `${mapconfigBasePath}/:token/analysis/node/:nodeId`, + `${mapConfigBasePath}/:token/analysis/node/:nodeId`, cors(), cleanUpQueryParams(), locals(), @@ -367,7 +410,13 @@ function getRequestParams(locals) { return params; } -function createMapStoreMapConfigProvider (mapStore, userLimitsApi, forcedFormat = null) { +function createMapStoreMapConfigProvider ( + mapStore, + userLimitsApi, + pgConnection, + affectedTablesCache, + forcedFormat = null +) { return function createMapStoreMapConfigProviderMiddleware (req, res, next) { const { user } = res.locals; @@ -378,7 +427,14 @@ function createMapStoreMapConfigProvider (mapStore, userLimitsApi, forcedFormat params.layer = params.layer || 'all'; } - res.locals.mapConfigProvider = new MapStoreMapConfigProvider(mapStore, user, userLimitsApi, params); + res.locals.mapConfigProvider = new MapStoreMapConfigProvider( + mapStore, + user, + userLimitsApi, + pgConnection, + affectedTablesCache, + params + ); next(); }; @@ -575,88 +631,6 @@ function setCacheControlHeader () { }; } -function getAffectedTables (layergroupAffectedTables, pgConnection, mapStore) { - return function getAffectedTablesMiddleware (req, res, next) { - const { user, dbname, token } = res.locals; - - if (layergroupAffectedTables.hasAffectedTables(dbname, token)) { - res.locals.affectedTables = layergroupAffectedTables.get(dbname, token); - return next(); - } - - mapStore.load(token, (err, mapconfig) => { - if (err) { - global.logger.warn('ERROR generating cache channel:', err); - return next(); - } - - const 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) { - global.logger.warn('ERROR generating cache channel:' + - ' this request doesn\'t need an X-Cache-Channel generated'); - return next(); - } - - pgConnection.getConnection(user, (err, connection) => { - if (err) { - global.logger.warn('ERROR generating cache channel:', err); - return next(); - } - - QueryTables.getAffectedTablesFromQuery(connection, sql, (err, affectedTables) => { - req.profiler.done('getAffectedTablesFromQuery'); - if (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 - layergroupAffectedTables.set(dbname, token, affectedTables); - - res.locals.affectedTables = affectedTables; - - next(); - }); - }); - }); - }; -} - -function setCacheChannelHeader () { - return function setCacheChannelHeaderMiddleware (req, res, next) { - const { affectedTables } = res.locals; - - if (affectedTables) { - res.set('X-Cache-Channel', affectedTables.getCacheChannel()); - } - - next(); - }; -} - -function setSurrogateKeyHeader (surrogateKeysCache) { - return function setSurrogateKeyHeaderMiddleware (req, res, next) { - const { affectedTables } = res.locals; - - if (affectedTables) { - surrogateKeysCache.tag(res, affectedTables); - } - - next(); - }; -} - function incrementSuccessMetrics (statsClient) { return function incrementSuccessMetricsMiddleware (req, res, next) { const formatStat = parseFormat(req.params.format); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 413f023d..ea8b309a 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -2,7 +2,6 @@ const _ = require('underscore'); const windshaft = require('windshaft'); const MapConfig = windshaft.model.MapConfig; const Datasource = windshaft.model.Datasource; -const QueryTables = require('cartodb-query-tables'); const ResourceLocator = require('../models/resource-locator'); const cors = require('../middleware/cors'); const user = require('../middleware/user'); @@ -12,8 +11,9 @@ const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); +const cacheChannelHeader = require('../middleware/cache-channel-header'); +const surrogateKeyHeader = require('../middleware/surrogate-key-header'); const sendResponse = require('../middleware/send-response'); -const NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); const NamedMapMapConfigProvider = require('../models/mapconfig/provider/named-map-provider'); const CreateLayergroupMapConfigProvider = require('../models/mapconfig/provider/create-layergroup-provider'); const LayergroupMetadata = require('../utils/layergroup-metadata'); @@ -64,15 +64,15 @@ function MapController ( module.exports = MapController; MapController.prototype.register = function(app) { - const { base_url_mapconfig: mapconfigBasePath, base_url_templated: templateBasePath } = app; + const { base_url_mapconfig: mapConfigBasePath, base_url_templated: templateBasePath } = app; app.get( - `${mapconfigBasePath}`, + `${mapConfigBasePath}`, this.composeCreateMapMiddleware(RATE_LIMIT_ENDPOINTS_GROUPS.ANONYMOUS) ); app.post( - `${mapconfigBasePath}`, + `${mapConfigBasePath}`, this.composeCreateMapMiddleware(RATE_LIMIT_ENDPOINTS_GROUPS.ANONYMOUS) ); @@ -88,7 +88,7 @@ MapController.prototype.register = function(app) { this.composeCreateMapMiddleware(RATE_LIMIT_ENDPOINTS_GROUPS.NAMED, useTemplate) ); - app.options(app.base_url_mapconfig, cors('Content-Type')); + app.options(`${mapConfigBasePath}`, cors('Content-Type')); }; MapController.prototype.composeCreateMapMiddleware = function (endpointGroup, useTemplate = false) { @@ -113,9 +113,8 @@ MapController.prototype.composeCreateMapMiddleware = function (endpointGroup, us this.getCreateMapMiddlewares(useTemplate), incrementMapViewCount(this.metadataBackend), augmentLayergroupData(), - getAffectedTables(this.pgConnection, this.layergroupAffectedTables), - setCacheChannelHeader(), - setSurrogateKeyHeader(this.surrogateKeysCache), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), setLastModified(), setLastUpdatedTimeToLayergroup(), setCacheControl(), @@ -140,16 +139,27 @@ MapController.prototype.getCreateMapMiddlewares = function (useTemplate) { this.pgConnection, this.metadataBackend, this.userLimitsApi, - this.mapConfigAdapter + this.mapConfigAdapter, + this.layergroupAffectedTables ), - instantiateLayergroup(this.mapBackend, this.userLimitsApi) + instantiateLayergroup( + this.mapBackend, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTables + ) ]; } return [ checkCreateLayergroup(), prepareAdapterMapConfig(this.mapConfigAdapter), - createLayergroup (this.mapBackend, this.userLimitsApi) + createLayergroup ( + this.mapBackend, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTables + ) ]; }; @@ -220,17 +230,25 @@ function checkCreateLayergroup () { }; } -function getTemplate (templateMaps, pgConnection, metadataBackend, userLimitsApi, mapConfigAdapter) { +function getTemplate ( + templateMaps, + pgConnection, + metadataBackend, + userLimitsApi, + mapConfigAdapter, + affectedTablesCache +) { return function getTemplateMiddleware (req, res, next) { const templateParams = req.body; const { user } = res.locals; - const mapconfigProvider = new NamedMapMapConfigProvider( + const mapConfigProvider = new NamedMapMapConfigProvider( templateMaps, pgConnection, metadataBackend, userLimitsApi, mapConfigAdapter, + affectedTablesCache, user, req.params.template_id, templateParams, @@ -238,15 +256,15 @@ function getTemplate (templateMaps, pgConnection, metadataBackend, userLimitsApi res.locals ); - mapconfigProvider.getMapConfig((err, mapconfig, rendererParams) => { + mapConfigProvider.getMapConfig((err, mapConfig, rendererParams) => { req.profiler.done('named.getMapConfig'); if (err) { return next(err); } - res.locals.mapconfig = mapconfig; + res.locals.mapConfig = mapConfig; res.locals.rendererParams = rendererParams; - res.locals.mapconfigProvider = mapconfigProvider; + res.locals.mapConfigProvider = mapConfigProvider; next(); }); @@ -289,38 +307,51 @@ function prepareAdapterMapConfig (mapConfigAdapter) { }; } -function createLayergroup (mapBackend, userLimitsApi) { +function createLayergroup (mapBackend, userLimitsApi, pgConnection, affectedTablesCache) { return function createLayergroupMiddleware (req, res, next) { const requestMapConfig = req.body; const { context, user } = res.locals; const datasource = context.datasource || Datasource.EmptyDatasource(); - const mapconfig = new MapConfig(requestMapConfig, datasource); - const mapconfigProvider = - new CreateLayergroupMapConfigProvider(mapconfig, user, userLimitsApi, res.locals); + const mapConfig = new MapConfig(requestMapConfig, datasource); + const mapConfigProvider = new CreateLayergroupMapConfigProvider( + mapConfig, + user, + userLimitsApi, + pgConnection, + affectedTablesCache, + res.locals + ); - res.locals.mapconfig = mapconfig; + res.locals.mapConfig = mapConfig; res.locals.analysesResults = context.analysesResults; - mapBackend.createLayergroup(mapconfig, res.locals, mapconfigProvider, (err, layergroup) => { + mapBackend.createLayergroup(mapConfig, res.locals, mapConfigProvider, (err, layergroup) => { req.profiler.done('createLayergroup'); if (err) { return next(err); } res.body = layergroup; + res.locals.mapConfigProvider = mapConfigProvider; next(); }); }; } -function instantiateLayergroup (mapBackend, userLimitsApi) { +function instantiateLayergroup (mapBackend, userLimitsApi, pgConnection, affectedTablesCache) { return function instantiateLayergroupMiddleware (req, res, next) { - const { user, mapconfig, rendererParams } = res.locals; - const mapconfigProvider = - new CreateLayergroupMapConfigProvider(mapconfig, user, userLimitsApi, rendererParams); + const { user, mapConfig, rendererParams } = res.locals; + const mapConfigProvider = new CreateLayergroupMapConfigProvider( + mapConfig, + user, + userLimitsApi, + pgConnection, + affectedTablesCache, + rendererParams + ); - mapBackend.createLayergroup(mapconfig, rendererParams, mapconfigProvider, (err, layergroup) => { + mapBackend.createLayergroup(mapConfig, rendererParams, mapConfigProvider, (err, layergroup) => { req.profiler.done('createLayergroup'); if (err) { return next(err); @@ -328,12 +359,11 @@ function instantiateLayergroup (mapBackend, userLimitsApi) { res.body = layergroup; - const { mapconfigProvider } = res.locals; + const { mapConfigProvider } = res.locals; - res.locals.analysesResults = mapconfigProvider.analysesResults; - res.locals.template = mapconfigProvider.template; - res.locals.templateName = mapconfigProvider.getTemplateName(); - res.locals.context = mapconfigProvider.context; + res.locals.analysesResults = mapConfigProvider.analysesResults; + res.locals.template = mapConfigProvider.template; + res.locals.context = mapConfigProvider.context; next(); }); @@ -342,10 +372,10 @@ function instantiateLayergroup (mapBackend, userLimitsApi) { function incrementMapViewCount (metadataBackend) { return function incrementMapViewCountMiddleware(req, res, next) { - const { mapconfig, user } = res.locals; + const { mapConfig, user } = res.locals; // Error won't blow up, just be logged. - metadataBackend.incMapviewCount(user, mapconfig.obj().stat_tag, (err) => { + metadataBackend.incMapviewCount(user, mapConfig.obj().stat_tag, (err) => { req.profiler.done('incMapviewCount'); if (err) { @@ -370,71 +400,6 @@ function augmentLayergroupData () { }; } -function getAffectedTables (pgConnection, layergroupAffectedTables) { - return function getAffectedTablesMiddleware (req, res, next) { - const { dbname, user, mapconfig } = res.locals; - const layergroup = res.body; - - pgConnection.getConnection(user, (err, connection) => { - if (err) { - return next(err); - } - - const sql = []; - mapconfig.getLayers().forEach(function(layer) { - sql.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(function(table) { - sql.push('SELECT * FROM ' + table + ' LIMIT 0'); - }); - } - }); - - QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), (err, affectedTables) => { - req.profiler.done('getAffectedTablesFromQuery'); - if (err) { - return next(err); - } - - // feed affected tables cache so it can be reused from, for instance, layergroup controller - layergroupAffectedTables.set(dbname, layergroup.layergroupId, affectedTables); - - res.locals.affectedTables = affectedTables; - - next(); - }); - }); - }; -} - -function setCacheChannelHeader () { - return function setCacheChannelHeaderMiddleware (req, res, next) { - const { affectedTables } = res.locals; - - if (req.method === 'GET') { - res.set('X-Cache-Channel', affectedTables.getCacheChannel()); - } - - next(); - }; -} - -function setSurrogateKeyHeader (surrogateKeysCache) { - return function setSurrogateKeyHeaderMiddleware(req, res, next) { - const { affectedTables, user, templateName } = res.locals; - - if (req.method === 'GET' && affectedTables.tables && affectedTables.tables.length > 0) { - surrogateKeysCache.tag(res, affectedTables); - } - - if (templateName) { - surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, templateName)); - } - - next(); - }; -} - function setLastModified () { return function setLastModifiedMiddleware (req, res, next) { if (req.method === 'GET') { @@ -447,18 +412,28 @@ function setLastModified () { function setLastUpdatedTimeToLayergroup () { return function setLastUpdatedTimeToLayergroupMiddleware (req, res, next) { - const { affectedTables, analysesResults } = res.locals; + const { mapConfigProvider, analysesResults } = res.locals; const layergroup = res.body; - var lastUpdateTime = affectedTables.getLastUpdatedAt(); + mapConfigProvider.getAffectedTables((err, affectedTables) => { + if (err) { + return next(err); + } - lastUpdateTime = getLastUpdatedTime(analysesResults, lastUpdateTime) || lastUpdateTime; + if (!affectedTables) { + return next(); + } - // last update for layergroup cache buster - layergroup.layergroupid = layergroup.layergroupid + ':' + lastUpdateTime; - layergroup.last_updated = new Date(lastUpdateTime).toISOString(); + var lastUpdateTime = affectedTables.getLastUpdatedAt(); - next(); + lastUpdateTime = getLastUpdatedTime(analysesResults, lastUpdateTime) || lastUpdateTime; + + // last update for layergroup cache buster + layergroup.layergroupid = layergroup.layergroupid + ':' + lastUpdateTime; + layergroup.last_updated = new Date(lastUpdateTime).toISOString(); + + next(); + }); }; } @@ -488,7 +463,7 @@ function setCacheControl () { function setLayerStats (pgConnection, statsBackend) { return function setLayerStatsMiddleware(req, res, next) { - const { user, mapconfig } = res.locals; + const { user, mapConfig } = res.locals; const layergroup = res.body; pgConnection.getConnection(user, (err, connection) => { @@ -496,7 +471,7 @@ function setLayerStats (pgConnection, statsBackend) { return next(err); } - statsBackend.getStats(mapconfig, connection, function(err, layersStats) { + statsBackend.getStats(mapConfig, connection, function(err, layersStats) { if (err) { return next(err); } @@ -531,10 +506,10 @@ function setLayergroupIdHeader (templateMaps, useTemplateHash) { function setDataviewsAndWidgetsUrlsToLayergroupMetadata (layergroupMetadata) { return function setDataviewsAndWidgetsUrlsToLayergroupMetadataMiddleware (req, res, next) { - const { user, mapconfig } = res.locals; + const { user, mapConfig } = res.locals; const layergroup = res.body; - layergroupMetadata.addDataviewsAndWidgetsUrls(user, layergroup, mapconfig.obj()); + layergroupMetadata.addDataviewsAndWidgetsUrls(user, layergroup, mapConfig.obj()); next(); }; @@ -553,10 +528,10 @@ function setAnalysesMetadataToLayergroup (layergroupMetadata, includeQuery) { function setTurboCartoMetadataToLayergroup (layergroupMetadata) { return function setTurboCartoMetadataToLayergroupMiddleware (req, res, next) { - const { mapconfig, context } = res.locals; + const { mapConfig, context } = res.locals; const layergroup = res.body; - layergroupMetadata.addTurboCartoContextMetadata(layergroup, mapconfig.obj(), context); + layergroupMetadata.addTurboCartoContextMetadata(layergroup, mapConfig.obj(), context); next(); }; @@ -564,10 +539,10 @@ function setTurboCartoMetadataToLayergroup (layergroupMetadata) { function setAggregationMetadataToLayergroup (layergroupMetadata) { return function setAggregationMetadataToLayergroupMiddleware (req, res, next) { - const { mapconfig, context } = res.locals; + const { mapConfig, context } = res.locals; const layergroup = res.body; - layergroupMetadata.addAggregationContextMetadata(layergroup, mapconfig.obj(), context); + layergroupMetadata.addAggregationContextMetadata(layergroup, mapConfig.obj(), context); next(); }; @@ -575,10 +550,10 @@ function setAggregationMetadataToLayergroup (layergroupMetadata) { function setTilejsonMetadataToLayergroup (layergroupMetadata) { return function augmentLayergroupTilejsonMiddleware (req, res, next) { - const { user, mapconfig } = res.locals; + const { user, mapConfig } = res.locals; const layergroup = res.body; - layergroupMetadata.addTileJsonMetadata(layergroup, user, mapconfig); + layergroupMetadata.addTileJsonMetadata(layergroup, user, mapConfig); next(); }; @@ -589,10 +564,10 @@ function augmentError (options) { return function augmentErrorMiddleware (err, req, res, next) { req.profiler.done('error'); - const { mapconfig } = res.locals; + const { mapConfig } = res.locals; if (addContext) { - err = Number.isFinite(err.layerIndex) ? populateError(err, mapconfig) : err; + err = Number.isFinite(err.layerIndex) ? populateError(err, mapConfig) : err; } err.label = label; diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 74c628a5..c9d67c95 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -1,4 +1,3 @@ -const NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); const cors = require('../middleware/cors'); const user = require('../middleware/user'); const locals = require('../middleware/locals'); @@ -7,6 +6,9 @@ const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); +const cacheChannelHeader = require('../middleware/cache-channel-header'); +const surrogateKeyHeader = require('../middleware/surrogate-key-header'); +const lastModifiedHeader = require('../middleware/last-modified-header'); const sendResponse = require('../middleware/send-response'); const vectorError = require('../middleware/vector-error'); const rateLimit = require('../middleware/rate-limit'); @@ -28,8 +30,8 @@ function getRequestParams(locals) { const params = Object.assign({}, locals); delete params.template; - delete params.affectedTablesAndLastUpdate; - delete params.namedMapProvider; + delete params.affectedTables; + delete params.mapConfigProvider; delete params.allowedQueryParams; return params; @@ -77,14 +79,13 @@ NamedMapsController.prototype.register = function(app) { namedMapProviderCache: this.namedMapProviderCache, label: 'NAMED_MAP_TILE' }), - getAffectedTables(), getTile({ tileBackend: this.tileBackend, label: 'NAMED_MAP_TILE' }), - setSurrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - setCacheChannelHeader(), - setLastModifiedHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), setCacheControlHeader(), setContentTypeHeader(), sendResponse(), @@ -106,7 +107,6 @@ NamedMapsController.prototype.register = function(app) { namedMapProviderCache: this.namedMapProviderCache, label: 'STATIC_VIZ_MAP', forcedFormat: 'png' }), - getAffectedTables(), getTemplate({ label: 'STATIC_VIZ_MAP' }), prepareLayerFilterFromPreviewLayers({ namedMapProviderCache: this.namedMapProviderCache, @@ -115,9 +115,9 @@ NamedMapsController.prototype.register = function(app) { getStaticImageOptions({ tablesExtentApi: this.tablesExtentApi }), getImage({ previewBackend: this.previewBackend, label: 'STATIC_VIZ_MAP' }), incrementMapViews({ metadataBackend: this.metadataBackend }), - setSurrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - setCacheChannelHeader(), - setLastModifiedHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + cacheChannelHeader(), + lastModifiedHeader(), setCacheControlHeader(), setContentTypeHeader(), sendResponse() @@ -143,25 +143,7 @@ function getNamedMapProvider ({ namedMapProviderCache, label, forcedFormat = nul return next(err); } - res.locals.namedMapProvider = namedMapProvider; - - next(); - }); - }; -} - -function getAffectedTables () { - return function getAffectedTables (req, res, next) { - const { namedMapProvider } = res.locals; - - namedMapProvider.getAffectedTablesAndLastUpdatedTime((err, affectedTablesAndLastUpdate) => { - req.profiler.done('affectedTables'); - - if (err) { - return next(err); - } - - res.locals.affectedTablesAndLastUpdate = affectedTablesAndLastUpdate; + res.locals.mapConfigProvider = namedMapProvider; next(); }); @@ -170,9 +152,9 @@ function getAffectedTables () { function getTemplate ({ label }) { return function getTemplateMiddleware (req, res, next) { - const { namedMapProvider } = res.locals; + const { mapConfigProvider } = res.locals; - namedMapProvider.getTemplate((err, template) => { + mapConfigProvider.getTemplate((err, template) => { if (err) { err.label = label; return next(err); @@ -220,7 +202,7 @@ function prepareLayerFilterFromPreviewLayers ({ namedMapProviderCache, label }) return next(err); } - res.locals.namedMapProvider = provider; + res.locals.mapConfigProvider = provider; next(); }); @@ -229,9 +211,9 @@ function prepareLayerFilterFromPreviewLayers ({ namedMapProviderCache, label }) function getTile ({ tileBackend, label }) { return function getTileMiddleware (req, res, next) { - const { namedMapProvider, format } = res.locals; + const { mapConfigProvider, format } = res.locals; - tileBackend.getTile(namedMapProvider, req.params, (err, tile, headers, stats) => { + tileBackend.getTile(mapConfigProvider, req.params, (err, tile, headers, stats) => { req.profiler.add(stats); req.profiler.done('render-' + format); @@ -253,7 +235,7 @@ function getTile ({ tileBackend, label }) { function getStaticImageOptions ({ tablesExtentApi }) { return function getStaticImageOptionsMiddleware(req, res, next) { - const { user, namedMapProvider, template } = res.locals; + const { user, mapConfigProvider, template } = res.locals; const imageOpts = getImageOptions(res.locals, template); @@ -264,18 +246,18 @@ function getStaticImageOptions ({ tablesExtentApi }) { res.locals.imageOpts = DEFAULT_ZOOM_CENTER; - namedMapProvider.getAffectedTablesAndLastUpdatedTime((err, affectedTablesAndLastUpdate) => { + mapConfigProvider.getAffectedTables((err, affectedTables) => { if (err) { return next(); } - var affectedTables = affectedTablesAndLastUpdate.tables || []; + var tables = affectedTables.tables || []; - if (affectedTables.length === 0) { + if (tables.length === 0) { return next(); } - tablesExtentApi.getBounds(user, affectedTables, (err, bounds) => { + tablesExtentApi.getBounds(user, tables, (err, bounds) => { if (err) { return next(); } @@ -355,7 +337,7 @@ function getImageOptionsFromBoundingBox (bbox = '') { function getImage({ previewBackend, label }) { return function getImageMiddleware (req, res, next) { - const { imageOpts, namedMapProvider } = res.locals; + const { imageOpts, mapConfigProvider } = res.locals; const { zoom, center, bounds } = imageOpts; let { width, height } = req.params; @@ -366,7 +348,7 @@ function getImage({ previewBackend, label }) { const format = req.params.format === 'jpg' ? 'jpeg' : 'png'; if (zoom !== undefined && center) { - return previewBackend.getImage(namedMapProvider, format, width, height, zoom, center, + return previewBackend.getImage(mapConfigProvider, format, width, height, zoom, center, (err, image, headers, stats) => { req.profiler.add(stats); @@ -385,7 +367,7 @@ function getImage({ previewBackend, label }) { }); } - previewBackend.getImage(namedMapProvider, format, width, height, bounds, (err, image, headers, stats) => { + previewBackend.getImage(mapConfigProvider, format, width, height, bounds, (err, image, headers, stats) => { req.profiler.add(stats); req.profiler.done('render-' + format); @@ -411,9 +393,9 @@ function incrementMapViewsError (ctx) { function incrementMapViews ({ metadataBackend }) { return function incrementMapViewsMiddleware(req, res, next) { - const { user, namedMapProvider } = res.locals; + const { user, mapConfigProvider } = res.locals; - namedMapProvider.getMapConfig((err, mapConfig) => { + mapConfigProvider.getMapConfig((err, mapConfig) => { if (err) { global.logger.log(incrementMapViewsError({ user, err })); return next(); @@ -462,59 +444,13 @@ function templateBounds(view) { return false; } -function setSurrogateKeyHeader ({ surrogateKeysCache }) { - return function setSurrogateKeyHeaderMiddleware(req, res, next) { - const { user, namedMapProvider, affectedTablesAndLastUpdate } = res.locals; - - surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, namedMapProvider.getTemplateName())); - if (!affectedTablesAndLastUpdate || !!affectedTablesAndLastUpdate.tables) { - if (affectedTablesAndLastUpdate.tables.length > 0) { - surrogateKeysCache.tag(res, affectedTablesAndLastUpdate); - } - } - - next(); - }; -} - -function setCacheChannelHeader () { - return function setCacheChannelHeaderMiddleware (req, res, next) { - const { affectedTablesAndLastUpdate } = res.locals; - - if (!affectedTablesAndLastUpdate || !!affectedTablesAndLastUpdate.tables) { - res.set('X-Cache-Channel', affectedTablesAndLastUpdate.getCacheChannel()); - } - - next(); - }; -} - -function setLastModifiedHeader () { - return function setLastModifiedHeaderMiddleware(req, res, next) { - const { affectedTablesAndLastUpdate } = res.locals; - - if (!affectedTablesAndLastUpdate || !!affectedTablesAndLastUpdate.tables) { - var lastModifiedDate; - if (Number.isFinite(affectedTablesAndLastUpdate.lastUpdatedTime)) { - lastModifiedDate = new Date(affectedTablesAndLastUpdate.getLastUpdatedAt()); - } else { - lastModifiedDate = new Date(); - } - - res.set('Last-Modified', lastModifiedDate.toUTCString()); - } - - next(); - }; - } - function setCacheControlHeader () { return function setCacheControlHeaderMiddleware(req, res, next) { - const { affectedTablesAndLastUpdate } = res.locals; + const { affectedTables } = res.locals; res.set('Cache-Control', 'public,max-age=7200,must-revalidate'); - if (!affectedTablesAndLastUpdate || !!affectedTablesAndLastUpdate.tables) { + if (!affectedTables || !!affectedTables.tables) { // we increase cache control as we can invalidate it res.set('Cache-Control', 'public,max-age=31536000'); } diff --git a/lib/cartodb/middleware/cache-channel-header.js b/lib/cartodb/middleware/cache-channel-header.js new file mode 100644 index 00000000..d6bf394b --- /dev/null +++ b/lib/cartodb/middleware/cache-channel-header.js @@ -0,0 +1,24 @@ +module.exports = function setCacheChannelHeader () { + return function setCacheChannelHeaderMiddleware (req, res, next) { + if (req.method !== 'GET') { + return next(); + } + + const { mapConfigProvider } = res.locals; + + mapConfigProvider.getAffectedTables((err, affectedTables) => { + if (err) { + global.logger.warn('ERROR generating Cache Channel Header:', err); + return next(); + } + + if (!affectedTables) { + return next(); + } + + res.set('X-Cache-Channel', affectedTables.getCacheChannel()); + + next(); + }); + }; +}; diff --git a/lib/cartodb/middleware/last-modified-header.js b/lib/cartodb/middleware/last-modified-header.js new file mode 100644 index 00000000..51211661 --- /dev/null +++ b/lib/cartodb/middleware/last-modified-header.js @@ -0,0 +1,40 @@ +module.exports = function setLastModifiedHeader () { + return function setLastModifiedHeaderMiddleware(req, res, next) { + if (req.method !== 'GET') { + return next(); + } + + const { mapConfigProvider, cache_buster } = res.locals; + + if (cache_buster) { + const cacheBuster = parseInt(cache_buster, 10); + + if (Number.isFinite(cacheBuster)) { + res.set('Last-Modified', new Date(cacheBuster).toUTCString()); + } + + return next(); + } + + mapConfigProvider.getAffectedTables((err, affectedTables) => { + if (err) { + global.logger.warn('ERROR generating Last Modified Header:', err); + return next(); + } + + if (!affectedTables) { + return next(); + } + + const lastUpdatedAt = affectedTables.getLastUpdatedAt(); + + const lastModifiedDate = Number.isFinite(lastUpdatedAt) ? + new Date(lastUpdatedAt) : + new Date(); + + res.set('Last-Modified', lastModifiedDate.toUTCString()); + + next(); + }); + }; +}; diff --git a/lib/cartodb/middleware/surrogate-key-header.js b/lib/cartodb/middleware/surrogate-key-header.js new file mode 100644 index 00000000..51cec1c1 --- /dev/null +++ b/lib/cartodb/middleware/surrogate-key-header.js @@ -0,0 +1,31 @@ +const NamedMapsCacheEntry = require('../cache/model/named_maps_entry'); +const NamedMapMapConfigProvider = require('../models/mapconfig/provider/named-map-provider'); + +module.exports = function setSurrogateKeyHeader ({ surrogateKeysCache }) { + return function setSurrogateKeyHeaderMiddleware(req, res, next) { + const { user, mapConfigProvider } = res.locals; + + if (mapConfigProvider instanceof NamedMapMapConfigProvider) { + surrogateKeysCache.tag(res, new NamedMapsCacheEntry(user, mapConfigProvider.getTemplateName())); + } + + if (req.method !== 'GET') { + return next(); + } + + mapConfigProvider.getAffectedTables((err, affectedTables) => { + if (err) { + global.logger.warn('ERROR generating Surrogate Key Header:', err); + return next(); + } + + if (!affectedTables || !affectedTables.tables || affectedTables.tables.length === 0) { + return next(); + } + + surrogateKeysCache.tag(res, affectedTables); + + next(); + }); + }; +}; diff --git a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js index 340073b5..0eef42f8 100644 --- a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js +++ b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js @@ -2,6 +2,7 @@ var assert = require('assert'); var step = require('step'); var MapStoreMapConfigProvider = require('./map-store-provider'); +const QueryTables = require('cartodb-query-tables'); /** * @param {MapConfig} mapConfig @@ -11,10 +12,13 @@ var MapStoreMapConfigProvider = require('./map-store-provider'); * @constructor * @type {CreateLayergroupMapConfigProvider} */ -function CreateLayergroupMapConfigProvider(mapConfig, user, userLimitsApi, params) { + +function CreateLayergroupMapConfigProvider(mapConfig, user, userLimitsApi, pgConnection, affectedTablesCache, params) { this.mapConfig = mapConfig; this.user = user; this.userLimitsApi = userLimitsApi; + this.pgConnection = pgConnection; + this.affectedTablesCache = affectedTablesCache; this.params = params; this.cacheBuster = params.cache_buster || 0; } @@ -46,3 +50,61 @@ CreateLayergroupMapConfigProvider.prototype.getCacheBuster = MapStoreMapConfigPr CreateLayergroupMapConfigProvider.prototype.filter = MapStoreMapConfigProvider.prototype.filter; CreateLayergroupMapConfigProvider.prototype.createKey = MapStoreMapConfigProvider.prototype.createKey; + +CreateLayergroupMapConfigProvider.prototype.getAffectedTables = function (callback) { + var self = this; + + const { dbname } = self.params; + const token = self.mapConfig.id(); + + if (self.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = self.affectedTablesCache.get(dbname, token); + return callback(null, affectedTables); + } + + step( + function getSql() { + const queries = []; + + self.mapConfig.getLayers().forEach(function(layer) { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(table => { + queries.push(`SELECT * FROM ${table} LIMIT 0`); + }); + } + }); + + const sql = queries.length ? queries.join(';') : null; + + if (!sql) { + return callback(); + } + + return sql; + }, + function getAffectedTables(err, sql) { + assert.ifError(err); + + step( + function getConnection() { + self.pgConnection.getConnection(self.user, this); + }, + function getAffectedTables(err, connection) { + assert.ifError(err); + QueryTables.getAffectedTablesFromQuery(connection, sql, this); + }, + this + ); + }, + function finish(err, affectedTables) { + if (err) { + return callback(err); + } + + self.affectedTablesCache.set(dbname, token, affectedTables); + + return callback(null, affectedTables); + } + ); +}; diff --git a/lib/cartodb/models/mapconfig/provider/map-store-provider.js b/lib/cartodb/models/mapconfig/provider/map-store-provider.js index 177322d4..2c94de92 100644 --- a/lib/cartodb/models/mapconfig/provider/map-store-provider.js +++ b/lib/cartodb/models/mapconfig/provider/map-store-provider.js @@ -2,6 +2,7 @@ var _ = require('underscore'); var assert = require('assert'); var dot = require('dot'); var step = require('step'); +const QueryTables = require('cartodb-query-tables'); /** * @param {MapStore} mapStore @@ -11,20 +12,30 @@ var step = require('step'); * @constructor * @type {MapStoreMapConfigProvider} */ -function MapStoreMapConfigProvider(mapStore, user, userLimitsApi, params) { +function MapStoreMapConfigProvider(mapStore, user, userLimitsApi, pgConnection, affectedTablesCache, params) { this.mapStore = mapStore; this.user = user; this.userLimitsApi = userLimitsApi; - this.params = params; + this.pgConnection = pgConnection; + this.affectedTablesCache = affectedTablesCache; this.token = params.token; this.cacheBuster = params.cache_buster || 0; + this.mapConfig = null; + this.params = params; + this.context = null; } module.exports = MapStoreMapConfigProvider; MapStoreMapConfigProvider.prototype.getMapConfig = function(callback) { var self = this; + + if (this.mapConfig !== null) { + return callback(null, this.mapConfig, this.params, this.context); + } + var context = {}; + step( function prepareContextLimits() { self.userLimitsApi.getRenderLimits(self.user, self.params.api_key, this); @@ -39,6 +50,8 @@ MapStoreMapConfigProvider.prototype.getMapConfig = function(callback) { self.mapStore.load(self.token, this); }, function finish(err, mapConfig) { + self.mapConfig = mapConfig; + self.context = context; return callback(err, mapConfig, self.params, context); } ); @@ -74,4 +87,67 @@ MapStoreMapConfigProvider.prototype.createKey = function(base) { scale_factor: 1 }); return (base) ? baseKeyTpl(tplValues) : rendererKeyTpl(tplValues); -}; \ No newline at end of file +}; + +MapStoreMapConfigProvider.prototype.getAffectedTables = function(callback) { + var self = this; + + const { dbname, token } = self.params; + + if (self.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = self.affectedTablesCache.get(dbname, token); + + return callback(null, affectedTables); + } + + step( + function getMapConfig() { + self.getMapConfig(this); + }, + function getSql(err, mapConfig) { + assert.ifError(err); + + const queries = []; + + mapConfig.getLayers().forEach(function(layer) { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(table => { + queries.push(`SELECT * FROM ${table} LIMIT 0`); + }); + } + }); + + const sql = queries.length ? queries.join(';') : null; + + if (!sql) { + return callback(); + } + + return sql; + }, + function getAffectedTables(err, sql) { + assert.ifError(err); + + step( + function getConnection() { + self.pgConnection.getConnection(self.user, this); + }, + function getAffectedTables(err, connection) { + assert.ifError(err); + QueryTables.getAffectedTablesFromQuery(connection, sql, this); + }, + this + ); + }, + function finish(err, affectedTables) { + if (err) { + return callback(err); + } + + self.affectedTablesCache.set(dbname, token, affectedTables); + + return callback(err, affectedTables); + } + ); +}; diff --git a/lib/cartodb/models/mapconfig/provider/named-map-provider.js b/lib/cartodb/models/mapconfig/provider/named-map-provider.js index 9b085dbc..0fa003e2 100644 --- a/lib/cartodb/models/mapconfig/provider/named-map-provider.js +++ b/lib/cartodb/models/mapconfig/provider/named-map-provider.js @@ -11,8 +11,19 @@ var QueryTables = require('cartodb-query-tables'); * @constructor * @type {NamedMapMapConfigProvider} */ -function NamedMapMapConfigProvider(templateMaps, pgConnection, metadataBackend, userLimitsApi, mapConfigAdapter, - owner, templateId, config, authToken, params) { +function NamedMapMapConfigProvider( + templateMaps, + pgConnection, + metadataBackend, + userLimitsApi, + mapConfigAdapter, + affectedTablesCache, + owner, + templateId, + config, + authToken, + params +) { this.templateMaps = templateMaps; this.pgConnection = pgConnection; this.metadataBackend = metadataBackend; @@ -30,7 +41,7 @@ function NamedMapMapConfigProvider(templateMaps, pgConnection, metadataBackend, // use template after call to mapConfig this.template = null; - this.affectedTablesAndLastUpdate = null; + this.affectedTablesCache = affectedTablesCache; // providing this.err = null; @@ -189,7 +200,7 @@ NamedMapMapConfigProvider.prototype.getCacheBuster = function() { NamedMapMapConfigProvider.prototype.reset = function() { this.template = null; - this.affectedTablesAndLastUpdate = null; + this.affectedTables = null; this.err = null; this.mapConfig = null; @@ -251,12 +262,11 @@ NamedMapMapConfigProvider.prototype.getTemplateName = function() { return this.templateName; }; -NamedMapMapConfigProvider.prototype.getAffectedTablesAndLastUpdatedTime = function(callback) { +NamedMapMapConfigProvider.prototype.getAffectedTables = function(callback) { var self = this; - if (this.affectedTablesAndLastUpdate !== null) { - return callback(null, this.affectedTablesAndLastUpdate); - } + let dbname = null; + let token = null; step( function getMapConfig() { @@ -264,9 +274,33 @@ NamedMapMapConfigProvider.prototype.getAffectedTablesAndLastUpdatedTime = functi }, function getSql(err, mapConfig) { assert.ifError(err); - return mapConfig.getLayers().map(function(layer) { - return layer.options.sql; - }).join(';'); + + dbname = self.rendererParams; + token = mapConfig.id(); + + if (self.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = self.affectedTablesCache.get(dbname, token); + return callback(null, affectedTables); + } + + const queries = []; + + mapConfig.getLayers().forEach(layer => { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(table => { + queries.push(`SELECT * FROM ${table} LIMIT 0`); + }); + } + }); + + const sql = queries.length ? queries.join(';') : null; + + if (!sql) { + return callback(); + } + + return sql; }, function getAffectedTables(err, sql) { assert.ifError(err); @@ -281,9 +315,14 @@ NamedMapMapConfigProvider.prototype.getAffectedTablesAndLastUpdatedTime = functi this ); }, - function finish(err, result) { - self.affectedTablesAndLastUpdate = result; - return callback(err, result); + function finish(err, affectedTables) { + if (err) { + return callback(err); + } + + self.affectedTablesCache.set(dbname, token, affectedTables); + + return callback(err, affectedTables); } ); }; diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 7f7e2712..8535783d 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -200,7 +200,8 @@ module.exports = function(serverOptions) { pgConnection, metadataBackend, userLimitsApi, - mapConfigAdapter + mapConfigAdapter, + layergroupAffectedTablesCache ); ['update', 'delete'].forEach(function(eventType) { From d022a1fa5e15de958c185c89974e0f23e6da1409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Mar 2018 14:43:00 +0100 Subject: [PATCH 07/62] Extract last-modified header middlleware --- lib/cartodb/controllers/layergroup.js | 35 ++++++------------- lib/cartodb/controllers/map.js | 13 ++----- .../middleware/last-modified-header.js | 21 ++++++----- 3 files changed, 26 insertions(+), 43 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index b7b6803a..9da86ff4 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -11,6 +11,7 @@ const rateLimit = require('../middleware/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; const cacheChannelHeader = require('../middleware/cache-channel-header'); const surrogateKeyHeader = require('../middleware/surrogate-key-header'); +const lastModifiedHeader = require('../middleware/last-modified-header'); const sendResponse = require('../middleware/send-response'); const DataviewBackend = require('../backends/dataview'); const AnalysisStatusBackend = require('../backends/analysis-status'); @@ -87,9 +88,9 @@ LayergroupController.prototype.register = function(app) { ), getTile(this.tileBackend, 'map_tile'), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), sendResponse(), incrementErrorMetrics(global.statsClient), @@ -116,9 +117,9 @@ LayergroupController.prototype.register = function(app) { ), getTile(this.tileBackend, 'map_tile'), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), sendResponse(), incrementErrorMetrics(global.statsClient), @@ -146,9 +147,9 @@ LayergroupController.prototype.register = function(app) { ), getTile(this.tileBackend, 'maplayer_tile'), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), sendResponse(), incrementErrorMetrics(global.statsClient), @@ -175,9 +176,9 @@ LayergroupController.prototype.register = function(app) { ), getFeatureAttributes(this.attributesBackend), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), sendResponse() ); @@ -203,9 +204,9 @@ LayergroupController.prototype.register = function(app) { ), getPreviewImageByCenter(this.previewBackend), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), sendResponse() ); @@ -229,9 +230,9 @@ LayergroupController.prototype.register = function(app) { ), getPreviewImageByBoundingBox(this.previewBackend), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), sendResponse() ); @@ -272,9 +273,9 @@ LayergroupController.prototype.register = function(app) { ), getDataview(this.dataviewBackend), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), sendResponse() ); @@ -297,9 +298,9 @@ LayergroupController.prototype.register = function(app) { ), getDataview(this.dataviewBackend), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), sendResponse() ); @@ -322,9 +323,9 @@ LayergroupController.prototype.register = function(app) { ), dataviewSearch(this.dataviewBackend), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), sendResponse() ); @@ -347,9 +348,9 @@ LayergroupController.prototype.register = function(app) { ), dataviewSearch(this.dataviewBackend), setCacheControlHeader(), - setLastModifiedHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), sendResponse() ); @@ -609,20 +610,6 @@ function getPreviewImageByBoundingBox (previewBackend) { }; } -function setLastModifiedHeader () { - return function setLastModifiedHeaderMiddleware (req, res, next) { - let { cache_buster: cacheBuster } = res.locals; - - cacheBuster = parseInt(cacheBuster, 10); - - const lastUpdated = res.locals.cache_buster ? new Date(cacheBuster) : new Date(); - - res.set('Last-Modified', lastUpdated.toUTCString()); - - next(); - }; -} - function setCacheControlHeader () { return function setCacheControlHeaderMiddleware (req, res, next) { res.set('Cache-Control', 'public,max-age=31536000'); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index ea8b309a..670ed9d7 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -13,6 +13,7 @@ const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); const cacheChannelHeader = require('../middleware/cache-channel-header'); const surrogateKeyHeader = require('../middleware/surrogate-key-header'); +const lastModifiedHeader = require('../middleware/last-modified-header'); const sendResponse = require('../middleware/send-response'); const NamedMapMapConfigProvider = require('../models/mapconfig/provider/named-map-provider'); const CreateLayergroupMapConfigProvider = require('../models/mapconfig/provider/create-layergroup-provider'); @@ -115,7 +116,7 @@ MapController.prototype.composeCreateMapMiddleware = function (endpointGroup, us augmentLayergroupData(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - setLastModified(), + lastModifiedHeader({ now: true }), setLastUpdatedTimeToLayergroup(), setCacheControl(), setLayerStats(this.pgConnection, this.statsBackend), @@ -400,16 +401,6 @@ function augmentLayergroupData () { }; } -function setLastModified () { - return function setLastModifiedMiddleware (req, res, next) { - if (req.method === 'GET') { - res.set('Last-Modified', (new Date()).toUTCString()); - } - - next(); - }; -} - function setLastUpdatedTimeToLayergroup () { return function setLastUpdatedTimeToLayergroupMiddleware (req, res, next) { const { mapConfigProvider, analysesResults } = res.locals; diff --git a/lib/cartodb/middleware/last-modified-header.js b/lib/cartodb/middleware/last-modified-header.js index 51211661..18c0d961 100644 --- a/lib/cartodb/middleware/last-modified-header.js +++ b/lib/cartodb/middleware/last-modified-header.js @@ -1,4 +1,4 @@ -module.exports = function setLastModifiedHeader () { +module.exports = function setLastModifiedHeader ({ now = false } = {}) { return function setLastModifiedHeaderMiddleware(req, res, next) { if (req.method !== 'GET') { return next(); @@ -8,10 +8,16 @@ module.exports = function setLastModifiedHeader () { if (cache_buster) { const cacheBuster = parseInt(cache_buster, 10); + const lastModifiedDate = Number.isFinite(cacheBuster) ? new Date(cacheBuster) : new Date(); - if (Number.isFinite(cacheBuster)) { - res.set('Last-Modified', new Date(cacheBuster).toUTCString()); - } + res.set('Last-Modified', lastModifiedDate.toUTCString()); + + return next(); + } + + // REVIEW: to keep 100% compatibility with maps controller + if (now) { + res.set('Last-Modified', new Date().toUTCString()); return next(); } @@ -23,14 +29,13 @@ module.exports = function setLastModifiedHeader () { } if (!affectedTables) { + res.set('Last-Modified', new Date().toUTCString()); + return next(); } const lastUpdatedAt = affectedTables.getLastUpdatedAt(); - - const lastModifiedDate = Number.isFinite(lastUpdatedAt) ? - new Date(lastUpdatedAt) : - new Date(); + const lastModifiedDate = Number.isFinite(lastUpdatedAt) ? new Date(lastUpdatedAt) : new Date(); res.set('Last-Modified', lastModifiedDate.toUTCString()); From 72c4a7abd6fc9ee22514b1ff0333b6cff58d2290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Mar 2018 16:38:37 +0100 Subject: [PATCH 08/62] Extract cache control header middleware --- lib/cartodb/controllers/analyses.js | 10 ++----- lib/cartodb/controllers/layergroup.js | 29 +++++++------------ lib/cartodb/controllers/map.js | 3 +- lib/cartodb/controllers/named_maps.js | 23 +++------------ .../middleware/cache-control-header.js | 17 +++++++++++ test/acceptance/multilayer.js | 4 +++ 6 files changed, 40 insertions(+), 46 deletions(-) create mode 100644 lib/cartodb/middleware/cache-control-header.js diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index a9eee758..f632b394 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -9,6 +9,7 @@ const authorize = require('../middleware/authorize'); const dbConnSetup = require('../middleware/db-conn-setup'); const rateLimit = require('../middleware/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const cacheControlHeader = require('../middleware/cache-control-header'); const sendResponse = require('../middleware/send-response'); function AnalysesController(pgConnection, authApi, userLimitsApi) { @@ -37,7 +38,7 @@ AnalysesController.prototype.register = function (app) { getDataFromQuery({ queryTemplate: catalogQueryTpl, key: 'catalog' }), getDataFromQuery({ queryTemplate: tablesQueryTpl, key: 'tables' }), prepareResponse(), - setCacheControlHeader(), + cacheControlHeader({ ttl: 10, revalidate: true }), sendResponse(), unauthorizedError() ); @@ -112,13 +113,6 @@ function prepareResponse () { }; } -function setCacheControlHeader () { - return function setCacheControlHeaderMiddleware (req, res, next) { - res.set('Cache-Control', 'public,max-age=10,must-revalidate'); - next(); - }; -} - function unauthorizedError () { return function unathorizedErrorMiddleware(err, req, res, next) { if (err.message.match(/permission\sdenied/)) { diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 9da86ff4..a77a7168 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -9,6 +9,7 @@ const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); const rateLimit = require('../middleware/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const cacheControlHeader = require('../middleware/cache-control-header'); const cacheChannelHeader = require('../middleware/cache-channel-header'); const surrogateKeyHeader = require('../middleware/surrogate-key-header'); const lastModifiedHeader = require('../middleware/last-modified-header'); @@ -87,7 +88,7 @@ LayergroupController.prototype.register = function(app) { this.layergroupAffectedTablesCache ), getTile(this.tileBackend, 'map_tile'), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -116,7 +117,7 @@ LayergroupController.prototype.register = function(app) { this.layergroupAffectedTablesCache ), getTile(this.tileBackend, 'map_tile'), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -146,7 +147,7 @@ LayergroupController.prototype.register = function(app) { this.layergroupAffectedTablesCache ), getTile(this.tileBackend, 'maplayer_tile'), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -175,7 +176,7 @@ LayergroupController.prototype.register = function(app) { this.layergroupAffectedTablesCache ), getFeatureAttributes(this.attributesBackend), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -203,7 +204,7 @@ LayergroupController.prototype.register = function(app) { forcedFormat ), getPreviewImageByCenter(this.previewBackend), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -229,7 +230,7 @@ LayergroupController.prototype.register = function(app) { forcedFormat ), getPreviewImageByBoundingBox(this.previewBackend), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -272,7 +273,7 @@ LayergroupController.prototype.register = function(app) { this.layergroupAffectedTablesCache ), getDataview(this.dataviewBackend), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -297,7 +298,7 @@ LayergroupController.prototype.register = function(app) { this.layergroupAffectedTablesCache ), getDataview(this.dataviewBackend), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -322,7 +323,7 @@ LayergroupController.prototype.register = function(app) { this.layergroupAffectedTablesCache ), dataviewSearch(this.dataviewBackend), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -347,7 +348,7 @@ LayergroupController.prototype.register = function(app) { this.layergroupAffectedTablesCache ), dataviewSearch(this.dataviewBackend), - setCacheControlHeader(), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), @@ -610,14 +611,6 @@ function getPreviewImageByBoundingBox (previewBackend) { }; } -function setCacheControlHeader () { - return function setCacheControlHeaderMiddleware (req, res, next) { - res.set('Cache-Control', 'public,max-age=31536000'); - - next(); - }; -} - function incrementSuccessMetrics (statsClient) { return function incrementSuccessMetricsMiddleware (req, res, next) { const formatStat = parseFormat(req.params.format); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 670ed9d7..0caa61ce 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -11,6 +11,7 @@ const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); +const cacheControlHeader = require('../middleware/cache-control-header'); const cacheChannelHeader = require('../middleware/cache-channel-header'); const surrogateKeyHeader = require('../middleware/surrogate-key-header'); const lastModifiedHeader = require('../middleware/last-modified-header'); @@ -114,11 +115,11 @@ MapController.prototype.composeCreateMapMiddleware = function (endpointGroup, us this.getCreateMapMiddlewares(useTemplate), incrementMapViewCount(this.metadataBackend), augmentLayergroupData(), + cacheControlHeader({ ttl: global.environment.varnish.layergroupTtl || 86400, revalidate: true }), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader({ now: true }), setLastUpdatedTimeToLayergroup(), - setCacheControl(), setLayerStats(this.pgConnection, this.statsBackend), setLayergroupIdHeader(this.templateMaps ,useTemplateHash), setDataviewsAndWidgetsUrlsToLayergroupMetadata(this.layergroupMetadata), diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index c9d67c95..05dba8f8 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -6,6 +6,7 @@ const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); +const cacheControlHeader = require('../middleware/cache-control-header'); const cacheChannelHeader = require('../middleware/cache-channel-header'); const surrogateKeyHeader = require('../middleware/surrogate-key-header'); const lastModifiedHeader = require('../middleware/last-modified-header'); @@ -83,10 +84,10 @@ NamedMapsController.prototype.register = function(app) { tileBackend: this.tileBackend, label: 'NAMED_MAP_TILE' }), + cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), - setCacheControlHeader(), setContentTypeHeader(), sendResponse(), vectorError() @@ -115,10 +116,10 @@ NamedMapsController.prototype.register = function(app) { getStaticImageOptions({ tablesExtentApi: this.tablesExtentApi }), getImage({ previewBackend: this.previewBackend, label: 'STATIC_VIZ_MAP' }), incrementMapViews({ metadataBackend: this.metadataBackend }), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + cacheControlHeader(), cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), - setCacheControlHeader(), setContentTypeHeader(), sendResponse() ); @@ -444,24 +445,8 @@ function templateBounds(view) { return false; } -function setCacheControlHeader () { - return function setCacheControlHeaderMiddleware(req, res, next) { - const { affectedTables } = res.locals; - - res.set('Cache-Control', 'public,max-age=7200,must-revalidate'); - - if (!affectedTables || !!affectedTables.tables) { - // we increase cache control as we can invalidate it - res.set('Cache-Control', 'public,max-age=31536000'); - } - - next(); - }; - } - function setContentTypeHeader () { return function setContentTypeHeaderMiddleware(req, res, next) { - res.set('Content-Type', res.get('content-type') || res.get('Content-Type') || 'image/png'); next(); diff --git a/lib/cartodb/middleware/cache-control-header.js b/lib/cartodb/middleware/cache-control-header.js new file mode 100644 index 00000000..25e2b04f --- /dev/null +++ b/lib/cartodb/middleware/cache-control-header.js @@ -0,0 +1,17 @@ +module.exports = function setCacheControlHeader ({ ttl = 31536000, revalidate = false } = {}) { + return function setCacheControlHeaderMiddleware (req, res, next) { + if (req.method !== 'GET') { + return next(); + } + + const directives = [ 'public', `max-age=${ttl}` ]; + + if (revalidate) { + directives.push('must-revalidate'); + } + + res.set('Cache-Control', directives.join(',')); + + next(); + }; +} diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index a1a93e40..148ec6db 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -1272,6 +1272,8 @@ describe(suiteName, function() { it("cache control for layergroup default value", function(done) { global.environment.varnish.layergroupTtl = null; + var server = new CartodbWindshaft(serverOptions); + assert.response(server, layergroupTtlRequest, layergroupTtlResponseExpectation, function(res) { assert.equal(res.headers['cache-control'], 'public,max-age=86400,must-revalidate'); @@ -1287,6 +1289,8 @@ describe(suiteName, function() { var layergroupTtl = 300; global.environment.varnish.layergroupTtl = layergroupTtl; + var server = new CartodbWindshaft(serverOptions); + assert.response(server, layergroupTtlRequest, layergroupTtlResponseExpectation, function(res) { assert.equal(res.headers['cache-control'], 'public,max-age=' + layergroupTtl + ',must-revalidate'); From 52c8c9341ac296050e63a17642f02fc58ec5b287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Mar 2018 16:40:09 +0100 Subject: [PATCH 09/62] Remove function defined but never used --- lib/cartodb/controllers/map.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 0caa61ce..cdb1515f 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -442,17 +442,6 @@ function getLastUpdatedTime(analysesResults, lastUpdateTime) { }, lastUpdateTime); } -function setCacheControl () { - return function setCacheControlMiddleware (req, res, next) { - if (req.method === 'GET') { - var ttl = global.environment.varnish.layergroupTtl || 86400; - res.set('Cache-Control', 'public,max-age='+ttl+',must-revalidate'); - } - - next(); - }; -} - function setLayerStats (pgConnection, statsBackend) { return function setLayerStatsMiddleware(req, res, next) { const { user, mapConfig } = res.locals; From 4a2580c9ea20c0b07d6fef509d7849789441e4f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Mar 2018 16:43:34 +0100 Subject: [PATCH 10/62] Missing semicolon --- lib/cartodb/middleware/cache-control-header.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/middleware/cache-control-header.js b/lib/cartodb/middleware/cache-control-header.js index 25e2b04f..aebb64de 100644 --- a/lib/cartodb/middleware/cache-control-header.js +++ b/lib/cartodb/middleware/cache-control-header.js @@ -14,4 +14,4 @@ module.exports = function setCacheControlHeader ({ ttl = 31536000, revalidate = next(); }; -} +}; From 672b19b106a2ca068196ded6684b3431d069e6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Mar 2018 16:48:21 +0100 Subject: [PATCH 11/62] Magic number --- lib/cartodb/middleware/cache-control-header.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/middleware/cache-control-header.js b/lib/cartodb/middleware/cache-control-header.js index aebb64de..574ae3b5 100644 --- a/lib/cartodb/middleware/cache-control-header.js +++ b/lib/cartodb/middleware/cache-control-header.js @@ -1,4 +1,6 @@ -module.exports = function setCacheControlHeader ({ ttl = 31536000, revalidate = false } = {}) { +const ONE_YEAR_IN_SECONDS = 60 * 60 * 24 * 365; + +module.exports = function setCacheControlHeader ({ ttl = ONE_YEAR_IN_SECONDS, revalidate = false } = {}) { return function setCacheControlHeaderMiddleware (req, res, next) { if (req.method !== 'GET') { return next(); From 6ada8ba6a2864cbe29e1040e37e16fd721b30d6a Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 21 Mar 2018 17:01:32 +0100 Subject: [PATCH 12/62] Implement aggregation filters --- .../aggregation/aggregation-mapconfig.js | 26 +- .../models/aggregation/aggregation-query.js | 129 ++++- .../aggregation/aggregation-validator.js | 32 ++ test/acceptance/aggregation.js | 504 ++++++++++++++++++ 4 files changed, 680 insertions(+), 11 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js index 50e4dcfe..9306518c 100644 --- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js +++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js @@ -4,7 +4,8 @@ const aggregationValidator = require('./aggregation-validator'); const { createPositiveNumberValidator, createIncludesValueValidator, - createAggregationColumnsValidator + createAggregationColumnsValidator, + createAggregationFiltersValidator } = aggregationValidator; const SubstitutionTokens = require('../../utils/substitution-tokens'); @@ -43,6 +44,18 @@ module.exports = class AggregationMapConfig extends MapConfig { ]; } + static get FILTER_PARAMETERS () { + return [ + // TODO: valid combinations of parameters: + // * Except for less/greater params, only one parameter allowed per filter. + // * Any less parameter can be combined with one of the greater paramters. (to define a range) + 'less_than', 'less_than_or_equal_to', + 'greater_than', 'greater_than_or_equal_to', + 'equal', 'not_equal', + 'between', 'in', 'not_in' + ]; + } + static supportsGeometryType(geometryType) { return AggregationMapConfig.SUPPORTED_GEOMETRY_TYPES.includes(geometryType); } @@ -58,11 +71,15 @@ module.exports = class AggregationMapConfig extends MapConfig { const positiveNumberValidator = createPositiveNumberValidator(this); const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS); const aggregationColumnsValidator = createAggregationColumnsValidator(this, AggregationMapConfig.AGGREGATIONS); + const aggregationFiltersValidator = createAggregationFiltersValidator( + this, AggregationMapConfig.FILTER_PARAMETERS + ); validate('resolution', positiveNumberValidator); validate('placement', includesValidPlacementsValidator); validate('threshold', positiveNumberValidator); validate('columns', aggregationColumnsValidator); + validate('filters', aggregationFiltersValidator); this.user = user; this.pgConnection = connection; @@ -77,7 +94,8 @@ module.exports = class AggregationMapConfig extends MapConfig { threshold = AggregationMapConfig.THRESHOLD, placement, columns = {}, - dimensions = {} + dimensions = {}, + filters = {} } = this.getAggregation(index); return aggregationQuery({ @@ -87,6 +105,7 @@ module.exports = class AggregationMapConfig extends MapConfig { placement, columns, dimensions, + filters, isDefaultAggregation: this._isDefaultLayerAggregation(index) }); } @@ -220,7 +239,8 @@ module.exports = class AggregationMapConfig extends MapConfig { _isDefaultAggregation (aggregation) { return aggregation.placement === undefined && aggregation.columns === undefined && - this._isEmptyParameter(aggregation.dimensions); + this._isEmptyParameter(aggregation.dimensions) && + this._isEmptyParameter(aggregation.filters); } _isEmptyParameter(parameter) { diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index 16e897ab..bfd2def6 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -42,7 +42,8 @@ const queryForOptions = (options) => templateForOptions(options)({ sourceQuery: options.query, res: 256/options.resolution, columns: options.columns, - dimensions: options.dimensions + dimensions: options.dimensions, + filters: options.filters }); module.exports = queryForOptions; @@ -93,20 +94,23 @@ const aggregateColumnNames = (ctx, table) => { return sep(Object.keys(columns)); }; +const aggregateExpression = (column_name, column_parameters) => { + const aggregate_function = column_parameters.aggregate_function || 'count'; + const aggregate_definition = SUPPORTED_AGGREGATE_FUNCTIONS[aggregate_function]; + if (!aggregate_definition) { + throw new Error("Invalid Aggregate function: '" + aggregate_function + "'"); + } + return aggregate_definition.sql(column_name, column_parameters); +}; + const aggregateColumnDefs = ctx => { let columns = aggregateColumns(ctx); return sep(Object.keys(columns).map(column_name => { - const aggregate_function = columns[column_name].aggregate_function || 'count'; - const aggregate_definition = SUPPORTED_AGGREGATE_FUNCTIONS[aggregate_function]; - if (!aggregate_definition) { - throw new Error("Invalid Aggregate function: '" + aggregate_function + "'"); - } - const aggregate_expression = aggregate_definition.sql(column_name, columns[column_name]); + const aggregate_expression = aggregateExpression(column_name, columns[column_name]); return `${aggregate_expression} AS ${column_name}`; })); }; - const aggregateDimensions = ctx => ctx.dimensions || {}; const dimensionNames = (ctx, table) => { @@ -127,6 +131,111 @@ const dimensionDefs = ctx => { })); }; +const aggregateFilters = ctx => ctx.filters || {}; + +const filterConditionSQL = (expr, filter) => { + // TODO: validate filter parameters (e.g. cannot have both greater_than and greater_than or equal to) + + if (filter) { + if (!Array.isArray(filter)) { + filter = [filter]; + } + if (filter.length > 0) { + return filter.map(f => filterSingleConditionSQL(expr, f)).join(' OR '); + } + } +}; + +const filterSingleConditionSQL = (expr, filter) => { + let cond; + Object.keys(FILTERS).some(f => { + cond = FILTERS[f](expr, filter); + return cond; + }); + return cond; +}; + +const sqlQ = (value) => { + if (isFinite(value)) { + return String(value); + } + return `'${value}'`; // TODO: escape single quotes! (by doubling them) +}; + +/* jshint eqeqeq: false */ +/* x != null is used to check for both null and undefined; triple !== wouldn't do the trick */ + +const FILTERS = { + between: (expr, filter) => { + const lo = filter.greater_than_or_equal_to, hi = filter.less_than_or_equal_to; + if (lo != null && hi != null) { + return `(${expr} BETWEEN ${sqlQ(lo)} AND ${sqlQ(hi)})`; + } + }, + in: (expr, filter) => { + if (filter.in != null) { + return `(${expr} IN (${filter.in.map(v => sqlQ(v)).join(',')}))`; + } + }, + notin: (expr, filter) => { + if (filter.not_in != null) { + return `(${expr} NOT IN (${filter.not_in.map(v => sqlQ(v)).join(',')}))`; + } + }, + equal: (expr, filter) => { + if (filter.equal != null) { + return `(${expr} = ${sqlQ(filter.equal)})`; + } + }, + not_equal: (expr, filter) => { + if (filter.not_equal != null) { + return `(${expr} <> ${sqlQ(filter.not_equal)})`; + } + }, + range: (expr, filter) => { + let conds = []; + if (filter.greater_than_or_equal_to != null) { + conds.push(`(${expr} >= ${sqlQ(filter.greater_than_or_equal_to)})`); + } + if (filter.greater_than != null) { + conds.push(`(${expr} > ${sqlQ(filter.greater_than)})`); + } + if (filter.less_than_or_equal_to != null) { + conds.push(`(${expr} <= ${sqlQ(filter.less_than_or_equal_to)})`); + } + if (filter.less_than != null) { + conds.push(`(${expr} < ${sqlQ(filter.less_than)})`); + } + if (conds.length > 0) { + return conds.join(' AND '); + } + } +}; + +const filterConditions = ctx => { + let columns = aggregateColumns(ctx); + let dimensions = aggregateDimensions(ctx); + let filters = aggregateFilters(ctx); + return Object.keys(filters).map(filtered_column => { + let filtered_expr; + if (columns[filtered_column]) { + filtered_expr = aggregateExpression(filtered_column, columns[filtered_column]); + } + else if (dimensions[filtered_column]) { + filtered_expr = dimensions[filtered_column]; + } + if (!filtered_expr) { + throw new Error("Invalid filtered column: '" + filtered_column + "'"); + } + return filterConditionSQL(filtered_expr, filters[filtered_column]); + }).join(' AND '); +}; + +const havingClause = ctx => { + let cond = filterConditions(ctx); + return cond ? `HAVING ${cond}` : ''; +}; + // SQL expression to compute the aggregation resolution (grid cell size). // This is equivalent to `${256/ctx.res}*CDB_XYZ_Resolution(CDB_ZoomFromScale(!scale_denominator!))` // This is defined by the ctx.res parameter, which is the number of grid cells per tile linear dimension @@ -164,6 +273,7 @@ const defaultAggregationQueryTemplate = ctx => ` Floor(ST_X(_cdb_query.the_geom_webmercator)/_cdb_params.res), Floor(ST_Y(_cdb_query.the_geom_webmercator)/_cdb_params.res) ${dimensionNames(ctx)} + ${havingClause(ctx)} ) SELECT _cdb_query.* ${aggregateColumnNames(ctx)} @@ -196,6 +306,7 @@ const aggregationQueryTemplates = { Floor(ST_X(_cdb_query.the_geom_webmercator)/_cdb_params.res), Floor(ST_Y(_cdb_query.the_geom_webmercator)/_cdb_params.res) ${dimensionNames(ctx)} + ${havingClause(ctx)} `, 'point-grid': ctx => ` @@ -214,6 +325,7 @@ const aggregationQueryTemplates = { FROM (${ctx.sourceQuery}) _cdb_query, _cdb_params WHERE the_geom_webmercator && _cdb_params.bbox GROUP BY _cdb_gx, _cdb_gy ${dimensionNames(ctx)} + ${havingClause(ctx)} ) SELECT row_number() over() AS cartodb_id, @@ -241,6 +353,7 @@ const aggregationQueryTemplates = { Floor(ST_X(_cdb_query.the_geom_webmercator)/_cdb_params.res), Floor(ST_Y(_cdb_query.the_geom_webmercator)/_cdb_params.res) ${dimensionNames(ctx)} + ${havingClause(ctx)} ) SELECT _cdb_clusters.cartodb_id, diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js index d0dc24c2..ce038f05 100644 --- a/lib/cartodb/models/aggregation/aggregation-validator.js +++ b/lib/cartodb/models/aggregation/aggregation-validator.js @@ -42,6 +42,38 @@ module.exports.createAggregationColumnsValidator = function (mapconfig, validAgg }; }; +module.exports.createAggregationFiltersValidator = function (mapconfig, validParameters) { + return function validateAggregationFilters (value, key, index) { + const dims = mapconfig.getAggregation(index).dimensions || {}; + const cols = mapconfig.getAggregation(index).columns || {}; + const validKeys = Object.keys(dims).concat(Object.keys(cols)); + Object.keys(value).forEach((filteredName) => { + // filteredName must be the name of either an aggregated column or a dimension in the same layer + if (!validKeys.includes(filteredName)) { + const message = `Invalid filtered column: ${filteredName}`; + throw createLayerError(message, mapconfig, index); + } + // The filter parameters must be valid + let filters = value[filteredName]; + // a single filter or an array of filters (to be OR-combined) are accepted + if (!Array.isArray(filters)) { + filters = [filters]; + } + filters.forEach(params => { + Object.keys(params).forEach(paramName => { + if (!validParameters.includes(paramName)) { + const message = `Invalid filter parameter name: ${paramName}`; + throw createLayerError(message, mapconfig, index); + } + }); + // TODO: check parameter value (params[paramName]) to be of the correct type + }); + // TODO: if multiple parameters within params check the combination is valid, + // i.e. one of the *less* parameters and one of the *greater* parameters. + }); + }; +}; + function createAggregationColumnNamesValidator(mapconfig) { return function validateAggregationColumnNames (value, key, index) { Object.keys(value).forEach((columnName) => { diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index d8870c8a..db03ad8e 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -1475,6 +1475,510 @@ describe('aggregation', function () { done(); }); }); + + + ['centroid', 'point-sample', 'point-grid'].forEach(placement => { + it(`filters should work for ${placement} placement`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + placement: placement , + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: { + greater_than_or_equal_to: 0 + } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value >= 0); + }); + + done(); + }); + }); + }); + + ['centroid', 'point-sample', 'point-grid'].forEach(placement => { + it(`multiple ORed filters should work for ${placement} placement`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + placement: placement , + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: [ + { greater_than: 0 }, + { less_than: -2 } + ] + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value > 0 || row.properties.value < -2); + }); + + done(); + }); + }); + }); + + ['centroid', 'point-sample', 'point-grid'].forEach(placement => { + it(`multiple ANDed filters should work for ${placement} placement`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_2, + aggregation: { + placement: placement , + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + }, + value2: { + aggregate_function: 'sum', + aggregated_column: 'sqrt_value' + } + }, + filters: { + value: { greater_than: 0 }, + value2: { less_than: 9 } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value > 0 && row.properties.value2 < 9); + }); + + done(); + }); + }); + }); + + it(`supports IN filters`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: { in: [1, 3] } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value === 1 || row.properties.value === 3); + }); + + done(); + }); + }); + + it(`supports NOT IN filters`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: { not_in: [1, 3] } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value !== 1 && row.properties.value !== 3); + }); + + done(); + }); + }); + + it(`supports EQUAL filters`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: [{ equal: 1}, { equal: 3}] + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value === 1 || row.properties.value === 3); + }); + + done(); + }); + }); + + it(`supports NOT EQUAL filters`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: { not_equal: 1 } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value !== 1); + }); + + done(); + }); + }); + + it(`supports BETWEEN filters`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: { + greater_than_or_equal_to: -1, + less_than_or_equal_to: 2 + } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value >= -1 || row.properties.value <= 2); + }); + + done(); + }); + }); + + it(`supports RANGE filters`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: { + greater_than: -1, + less_than_or_equal_to: 2 + } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(row => { + assert.ok(row.properties.value > -1 || row.properties.value <= 2); + }); + + done(); + }); + }); + + it(`invalid filters cause errors`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + columns: { + value: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: { + not_a_valid_parameter: 0 + } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + + const options = { + response: { + status: 400 + } + }; + + this.testClient.getLayergroup(options, (err, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, { + errors: [ 'Invalid filter parameter name: not_a_valid_parameter'], + errors_with_context:[{ + type: 'layer', + message: 'Invalid filter parameter name: not_a_valid_parameter', + layer: { + id: "layer0", + index: 0, + type: "mapnik", + } + }] + }); + + done(); + }); + }); + + it(`filters on invalid columns cause errors`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + columns: { + value_sum: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + filters: { + value: { + not_a_valid_parameter: 0 + } + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + + const options = { + response: { + status: 400 + } + }; + + this.testClient.getLayergroup(options, (err, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, { + errors: [ 'Invalid filtered column: value'], + errors_with_context:[{ + type: 'layer', + message: 'Invalid filtered column: value', + layer: { + id: "layer0", + index: 0, + type: "mapnik", + } + }] + }); + + done(); + }); + }); + }); }); }); From ead6fa5f1fdee8f8b5aa88a3ad6faf0e6a35a8f2 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 21 Mar 2018 17:21:05 +0100 Subject: [PATCH 13/62] Document aggregation filters Note that dimension filters remain undocumented --- docs/aggregation.md | 77 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/docs/aggregation.md b/docs/aggregation.md index 2fb361c3..c3e754e1 100644 --- a/docs/aggregation.md +++ b/docs/aggregation.md @@ -185,3 +185,80 @@ This is the minimum number of (estimated) rows in the dataset (query results) fo ] } ``` + +### `filters` + +Aggregated data can be filtered by imposing filtering conditions on the aggregated columns. + +Each condition is represented by one or more parameters: + +* `{ "equal": V }` selects an specific value of the aggregated column. +* `{ "not_equal": V }` selects values different from the one specified. +* `{ "in": [v1, v2, v3] }` selects any value from a list. +* `{ "not_in": [v1, v2, v3] }` selects any value not in a list. +* `{ "less_than": v }` selects values strictly less than the one given. +* `{ "less_than_or_equal_to": v }` selects values less than or equal to the one given. +* `{ "greater_than": v }` selects values strictly greater than the one given. +* `{ "greater_than_or_equal_to": v }` selects values greater than or equal to the one given. + +One of the *less* conditions can be combined with one of the *greater* conditions to select a range of values, for example: +* `{ "greater_than": v1, "less_than": v2 }` +* `{ "greater_than_or_equal_to": v1, "less_than": v2 }` +* `{ "greater_than": v1, "less_than_or_equal_to": v2 }` +* `{ "greater_than_or_equal_to": v1, "less_than_or_equal_to": v2 }` + +For a given column, multiple conditions can be passed in an array; the conditions will logically ORed (any of the conditions have to be verifid for the value to be selected): + +* `"myvalue": [ { "equal": 10 }, { "less_than": 0 }]` will select values of the column `myvalue` which are equal to 10 **or** less than 0. + +In addition, the filters applied to different columns are logically combined with AND (all the conditions have to be satisfied for an element to be selected); for example with the following `filters` parameter we'll select aggregated records which have a `total_value` > 100 **and** a category equal to "a". + +```json +{ + "total_value": { "greater_than": 100 }, + "category": { "equal": "a" } +} +``` + +Note that the filtered columns have to be defined with the `columns` parameter, except for `_cdb_features_count`, which is always implicitly defined and can be filtered too. + +#### Example + +```json +{ + "version": "1.7.0", + "extent": [-20037508.5, -20037508.5, 20037508.5, 20037508.5], + "srid": 3857, + "maxzoom": 18, + "minzoom": 3, + "layers": [ + { + "type": "mapnik", + "options": { + "sql": "select * from table", + "cartocss": "#table { marker-width: [total]; marker-fill: ramp(value, (red, green, blue), jenks); }", + "cartocss_version": "2.3.0", + "aggregation": { + "placement": "centroid", + "columns": { + "total_value": { + "aggregate_function": "sum", + "aggregated_column": "value" + }, + "category": { + "aggregate_function": "mode", + "aggregated_column": "category" + } + }, + "filters" : { + "total_value": { "greater_than": 100 }, + "category": { "equal": "a" } + }, + "resolution": 2, + "threshold": 500000 + } + } + } + ] +} +``` From b9de49d5ab349fde078bcc036cdf6191b9875337 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 21 Mar 2018 17:36:26 +0100 Subject: [PATCH 14/62] Remove superfluous aggregation filter condition The default aggregation doesn't admit filters, so this wasn't necessary. --- lib/cartodb/models/aggregation/aggregation-query.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index bfd2def6..4c89c3e4 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -273,7 +273,6 @@ const defaultAggregationQueryTemplate = ctx => ` Floor(ST_X(_cdb_query.the_geom_webmercator)/_cdb_params.res), Floor(ST_Y(_cdb_query.the_geom_webmercator)/_cdb_params.res) ${dimensionNames(ctx)} - ${havingClause(ctx)} ) SELECT _cdb_query.* ${aggregateColumnNames(ctx)} From b40ed13f477c23c69d05d77e03ab5fbe60f92d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Mar 2018 19:08:37 +0100 Subject: [PATCH 15/62] Do not use step to deal with asyn code --- .../provider/create-layergroup-provider.js | 84 +++++++++---------- .../mapconfig/provider/map-store-provider.js | 81 ++++++++---------- .../mapconfig/provider/named-map-provider.js | 82 ++++++++---------- 3 files changed, 108 insertions(+), 139 deletions(-) diff --git a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js index 0eef42f8..36298eb1 100644 --- a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js +++ b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js @@ -27,7 +27,13 @@ module.exports = CreateLayergroupMapConfigProvider; CreateLayergroupMapConfigProvider.prototype.getMapConfig = function(callback) { var self = this; + + if (this.mapConfig && this.params && this.context) { + return callback(null, this.mapConfig, this.params, this.context); + } + var context = {}; + step( function prepareContextLimits() { self.userLimitsApi.getRenderLimits(self.user, self.params.api_key, this); @@ -35,6 +41,7 @@ CreateLayergroupMapConfigProvider.prototype.getMapConfig = function(callback) { function handleRenderLimits(err, renderLimits) { assert.ifError(err); context.limits = renderLimits; + self.context = context; return null; }, function finish(err) { @@ -52,59 +59,50 @@ CreateLayergroupMapConfigProvider.prototype.filter = MapStoreMapConfigProvider.p CreateLayergroupMapConfigProvider.prototype.createKey = MapStoreMapConfigProvider.prototype.createKey; CreateLayergroupMapConfigProvider.prototype.getAffectedTables = function (callback) { - var self = this; + this.getMapConfig((err, mapConfig) => { + if (err) { + return callback(err); + } - const { dbname } = self.params; - const token = self.mapConfig.id(); + const { dbname } = this.params; + const token = mapConfig.id(); - if (self.affectedTablesCache.hasAffectedTables(dbname, token)) { - const affectedTables = self.affectedTablesCache.get(dbname, token); - return callback(null, affectedTables); - } + if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = this.affectedTablesCache.get(dbname, token); + return callback(null, affectedTables); + } - step( - function getSql() { - const queries = []; + const queries = []; - self.mapConfig.getLayers().forEach(function(layer) { - queries.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(table => { - queries.push(`SELECT * FROM ${table} LIMIT 0`); - }); - } - }); - - const sql = queries.length ? queries.join(';') : null; - - if (!sql) { - return callback(); + this.mapConfig.getLayers().forEach(function(layer) { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(table => { + queries.push(`SELECT * FROM ${table} LIMIT 0`); + }); } + }); - return sql; - }, - function getAffectedTables(err, sql) { - assert.ifError(err); + const sql = queries.length ? queries.join(';') : null; - step( - function getConnection() { - self.pgConnection.getConnection(self.user, this); - }, - function getAffectedTables(err, connection) { - assert.ifError(err); - QueryTables.getAffectedTablesFromQuery(connection, sql, this); - }, - this - ); - }, - function finish(err, affectedTables) { + if (!sql) { + return callback(); + } + + this.pgConnection.getConnection(this.user, (err, connection) => { if (err) { return callback(err); } - self.affectedTablesCache.set(dbname, token, affectedTables); + QueryTables.getAffectedTablesFromQuery(connection, sql, (err, affectedTables) => { + if (err) { + return callback(err); + } - return callback(null, affectedTables); - } - ); + this.affectedTablesCache.set(dbname, token, affectedTables); + + callback(null, affectedTables); + }); + }); + }); }; diff --git a/lib/cartodb/models/mapconfig/provider/map-store-provider.js b/lib/cartodb/models/mapconfig/provider/map-store-provider.js index 2c94de92..ccf1472e 100644 --- a/lib/cartodb/models/mapconfig/provider/map-store-provider.js +++ b/lib/cartodb/models/mapconfig/provider/map-store-provider.js @@ -90,64 +90,51 @@ MapStoreMapConfigProvider.prototype.createKey = function(base) { }; MapStoreMapConfigProvider.prototype.getAffectedTables = function(callback) { - var self = this; + this.getMapConfig((err, mapConfig) => { + if (err) { + return callback(err); + } - const { dbname, token } = self.params; + const { dbname } = this.params; + const token = mapConfig.id(); - if (self.affectedTablesCache.hasAffectedTables(dbname, token)) { - const affectedTables = self.affectedTablesCache.get(dbname, token); + if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = this.affectedTablesCache.get(dbname, token); - return callback(null, affectedTables); - } + return callback(null, affectedTables); + } - step( - function getMapConfig() { - self.getMapConfig(this); - }, - function getSql(err, mapConfig) { - assert.ifError(err); + const queries = []; - const queries = []; - - mapConfig.getLayers().forEach(function(layer) { - queries.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(table => { - queries.push(`SELECT * FROM ${table} LIMIT 0`); - }); - } - }); - - const sql = queries.length ? queries.join(';') : null; - - if (!sql) { - return callback(); + mapConfig.getLayers().forEach(function(layer) { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(table => { + queries.push(`SELECT * FROM ${table} LIMIT 0`); + }); } + }); - return sql; - }, - function getAffectedTables(err, sql) { - assert.ifError(err); + const sql = queries.length ? queries.join(';') : null; - step( - function getConnection() { - self.pgConnection.getConnection(self.user, this); - }, - function getAffectedTables(err, connection) { - assert.ifError(err); - QueryTables.getAffectedTablesFromQuery(connection, sql, this); - }, - this - ); - }, - function finish(err, affectedTables) { + if (!sql) { + return callback(); + } + + this.pgConnection.getConnection(this.user, (err, connection) => { if (err) { return callback(err); } - self.affectedTablesCache.set(dbname, token, affectedTables); + QueryTables.getAffectedTablesFromQuery(connection, sql, (err, affectedTables) => { + if (err) { + return callback(err); + } - return callback(err, affectedTables); - } - ); + this.affectedTablesCache.set(dbname, token, affectedTables); + + callback(err, affectedTables); + }); + }); + }); }; diff --git a/lib/cartodb/models/mapconfig/provider/named-map-provider.js b/lib/cartodb/models/mapconfig/provider/named-map-provider.js index 0fa003e2..50612065 100644 --- a/lib/cartodb/models/mapconfig/provider/named-map-provider.js +++ b/lib/cartodb/models/mapconfig/provider/named-map-provider.js @@ -263,66 +263,50 @@ NamedMapMapConfigProvider.prototype.getTemplateName = function() { }; NamedMapMapConfigProvider.prototype.getAffectedTables = function(callback) { - var self = this; + this.getMapConfig((err, mapConfig) => { + if (err) { + return callback(err); + } - let dbname = null; - let token = null; + const { dbname } = this.rendererParams; + const token = mapConfig.id(); - step( - function getMapConfig() { - self.getMapConfig(this); - }, - function getSql(err, mapConfig) { - assert.ifError(err); + if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = this.affectedTablesCache.get(dbname, token); + return callback(null, affectedTables); + } - dbname = self.rendererParams; - token = mapConfig.id(); + const queries = []; - if (self.affectedTablesCache.hasAffectedTables(dbname, token)) { - const affectedTables = self.affectedTablesCache.get(dbname, token); - return callback(null, affectedTables); + mapConfig.getLayers().forEach(layer => { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(table => { + queries.push(`SELECT * FROM ${table} LIMIT 0`); + }); } + }); - const queries = []; + const sql = queries.length ? queries.join(';') : null; - mapConfig.getLayers().forEach(layer => { - queries.push(layer.options.sql); - if (layer.options.affected_tables) { - layer.options.affected_tables.map(table => { - queries.push(`SELECT * FROM ${table} LIMIT 0`); - }); - } - }); + if (!sql) { + return callback(); + } - const sql = queries.length ? queries.join(';') : null; - - if (!sql) { - return callback(); - } - - return sql; - }, - function getAffectedTables(err, sql) { - assert.ifError(err); - step( - function getConnection() { - self.pgConnection.getConnection(self.owner, this); - }, - function getAffectedTables(err, connection) { - assert.ifError(err); - QueryTables.getAffectedTablesFromQuery(connection, sql, this); - }, - this - ); - }, - function finish(err, affectedTables) { + this.pgConnection.getConnection(this.owner, (err, connection) => { if (err) { return callback(err); } - self.affectedTablesCache.set(dbname, token, affectedTables); + QueryTables.getAffectedTablesFromQuery(connection, sql, (err, affectedTables) => { + if (err) { + return callback(err); + } - return callback(err, affectedTables); - } - ); + this.affectedTablesCache.set(dbname, token, affectedTables); + + callback(err, affectedTables); + }); + }); + }); }; From e542d38ec7f44aa1fcbf686db90946923d907961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 11:38:33 +0100 Subject: [PATCH 16/62] Reorder middleware --- lib/cartodb/controllers/named_maps.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 05dba8f8..26c5e4ad 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -84,11 +84,11 @@ NamedMapsController.prototype.register = function(app) { tileBackend: this.tileBackend, label: 'NAMED_MAP_TILE' }), + setContentTypeHeader(), cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), - setContentTypeHeader(), sendResponse(), vectorError() ); @@ -115,12 +115,12 @@ NamedMapsController.prototype.register = function(app) { }), getStaticImageOptions({ tablesExtentApi: this.tablesExtentApi }), getImage({ previewBackend: this.previewBackend, label: 'STATIC_VIZ_MAP' }), + setContentTypeHeader(), incrementMapViews({ metadataBackend: this.metadataBackend }), cacheControlHeader(), cacheChannelHeader(), surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), - setContentTypeHeader(), sendResponse() ); }; @@ -388,6 +388,14 @@ function getImage({ previewBackend, label }) { }; } +function setContentTypeHeader () { + return function setContentTypeHeaderMiddleware(req, res, next) { + res.set('Content-Type', res.get('content-type') || res.get('Content-Type') || 'image/png'); + + next(); + }; +} + function incrementMapViewsError (ctx) { return `ERROR: failed to increment mapview count for user '${ctx.user}': ${ctx.err}`; } @@ -444,11 +452,3 @@ function templateBounds(view) { } return false; } - -function setContentTypeHeader () { - return function setContentTypeHeaderMiddleware(req, res, next) { - res.set('Content-Type', res.get('content-type') || res.get('Content-Type') || 'image/png'); - - next(); - }; -} From 8ce72ea842d66ca9974a47c6b459e294d3e66cbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 12:30:51 +0100 Subject: [PATCH 17/62] Do not pass `res.locals` to collaborators --- lib/cartodb/backends/analysis-status.js | 28 +++---------------------- lib/cartodb/controllers/analyses.js | 26 +++++------------------ lib/cartodb/controllers/layergroup.js | 7 ++++++- lib/cartodb/utils/database-params.js | 25 ++++++++++++++++++++++ 4 files changed, 39 insertions(+), 47 deletions(-) create mode 100644 lib/cartodb/utils/database-params.js diff --git a/lib/cartodb/backends/analysis-status.js b/lib/cartodb/backends/analysis-status.js index 97f851d2..0060f36b 100644 --- a/lib/cartodb/backends/analysis-status.js +++ b/lib/cartodb/backends/analysis-status.js @@ -5,16 +5,14 @@ function AnalysisStatusBackend() { module.exports = AnalysisStatusBackend; - -AnalysisStatusBackend.prototype.getNodeStatus = function (params, callback) { - var nodeId = params.nodeId; - +AnalysisStatusBackend.prototype.getNodeStatus = function (nodeId, dbParams, callback) { var statusQuery = [ 'SELECT node_id, status, updated_at, last_error_message as error_message', 'FROM cdb_analysis_catalog where node_id = \'' + nodeId + '\'' ].join(' '); - var pg = new PSQL(dbParamsFromReqParams(params)); + var pg = new PSQL(dbParams); + pg.query(statusQuery, function(err, result) { if (err) { return callback(err, result); @@ -36,23 +34,3 @@ AnalysisStatusBackend.prototype.getNodeStatus = function (params, callback) { return callback(null, statusResponse); }, true); // use read-only transaction }; - -function dbParamsFromReqParams(params) { - var dbParams = {}; - if ( params.dbuser ) { - dbParams.user = params.dbuser; - } - if ( params.dbpassword ) { - dbParams.pass = params.dbpassword; - } - if ( params.dbhost ) { - dbParams.host = params.dbhost; - } - if ( params.dbport ) { - dbParams.port = params.dbport; - } - if ( params.dbname ) { - dbParams.dbname = params.dbname; - } - return dbParams; -} diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index f632b394..0da18bf6 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -11,6 +11,7 @@ const rateLimit = require('../middleware/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; const cacheControlHeader = require('../middleware/cache-control-header'); const sendResponse = require('../middleware/send-response'); +const dbParamsFromResLocals = require('../utils/database-params'); function AnalysesController(pgConnection, authApi, userLimitsApi) { this.pgConnection = pgConnection; @@ -46,7 +47,10 @@ AnalysesController.prototype.register = function (app) { function createPGClient () { return function createPGClientMiddleware (req, res, next) { - res.locals.pg = new PSQL(dbParamsFromReqParams(res.locals)); + const dbParams = dbParamsFromResLocals(res.locals); + + res.locals.pg = new PSQL(dbParams); + next(); }; } @@ -146,23 +150,3 @@ var tablesQueryTpl = ctx => ` FROM analysis_tables ORDER BY size DESC `; - -function dbParamsFromReqParams(params) { - var dbParams = {}; - if ( params.dbuser ) { - dbParams.user = params.dbuser; - } - if ( params.dbpassword ) { - dbParams.pass = params.dbpassword; - } - if ( params.dbhost ) { - dbParams.host = params.dbhost; - } - if ( params.dbport ) { - dbParams.port = params.dbport; - } - if ( params.dbname ) { - dbParams.dbname = params.dbname; - } - return dbParams; -} diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index a77a7168..28ac7103 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -17,6 +17,8 @@ const sendResponse = require('../middleware/send-response'); const DataviewBackend = require('../backends/dataview'); const AnalysisStatusBackend = require('../backends/analysis-status'); const MapStoreMapConfigProvider = require('../models/mapconfig/provider/map-store-provider'); +const dbParamsFromResLocals = require('../utils/database-params'); + const SUPPORTED_FORMATS = { grid_json: true, json_torque: true, @@ -383,7 +385,10 @@ function distinguishLayergroupFromStaticRoute () { function analysisNodeStatus (analysisStatusBackend) { return function analysisNodeStatusMiddleware(req, res, next) { - analysisStatusBackend.getNodeStatus(res.locals, (err, nodeStatus, stats = {}) => { + const { nodeId } = req.params; + const dbParams = dbParamsFromResLocals(res.locals); + + analysisStatusBackend.getNodeStatus(nodeId, dbParams, (err, nodeStatus, stats = {}) => { req.profiler.add(stats); if (err) { diff --git a/lib/cartodb/utils/database-params.js b/lib/cartodb/utils/database-params.js new file mode 100644 index 00000000..a2176eaf --- /dev/null +++ b/lib/cartodb/utils/database-params.js @@ -0,0 +1,25 @@ +module.exports = function getDatabaseConnectionParams (params) { + const dbParams = {}; + + if (params.dbuser) { + dbParams.user = params.dbuser; + } + + if (params.dbpassword) { + dbParams.pass = params.dbpassword; + } + + if (params.dbhost) { + dbParams.host = params.dbhost; + } + + if (params.dbport) { + dbParams.port = params.dbport; + } + + if (params.dbname) { + dbParams.dbname = params.dbname; + } + + return dbParams; +}; From 875f3c07b39940cc8e65e1f41e5381f9a2df8605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 17:07:38 +0100 Subject: [PATCH 18/62] Pass only needed params to MapStoreMapConfigProvider --- lib/cartodb/controllers/layergroup.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 28ac7103..614480a6 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -425,9 +425,15 @@ function createMapStoreMapConfigProvider ( forcedFormat = null ) { return function createMapStoreMapConfigProviderMiddleware (req, res, next) { - const { user } = res.locals; + const { user, token, cache_buster, api_key } = res.locals; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + const { layer, z, x, y, scale_factor, format } = req.params; - const params = getRequestParams(res.locals); + const params = { + user, token, cache_buster, api_key, + dbuser, dbname, dbpassword, dbhost, dbport, + layer, z, x, y, scale_factor, format + }; if (forcedFormat) { params.format = forcedFormat; From 1059066c055a90469840ada93173d91861b441bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 17:53:24 +0100 Subject: [PATCH 19/62] Use module to get database parameters --- lib/cartodb/backends/dataview.js | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index c110f328..4eebb2a2 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -1,13 +1,11 @@ var assert = require('assert'); - var _ = require('underscore'); var PSQL = require('cartodb-psql'); var step = require('step'); - var BBoxFilter = require('../models/filter/bbox'); - var DataviewFactory = require('../models/dataview/factory'); var DataviewFactoryWithOverviews = require('../models/dataview/overviews/factory'); +const dbParamsFromReqParams = require('../utils/database-params'); var OverviewsQueryRewriter = require('../utils/overviews_query_rewriter'); var overviewsQueryRewriter = new OverviewsQueryRewriter({ zoom_level: 'CDB_ZoomFromScale(!scale_denominator!)' @@ -48,7 +46,7 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param } var pg = new PSQL(dbParamsFromReqParams(params)); - + var query = getDataviewQuery(dataviewDefinition, ownFilter, noFilters); if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom_webmercator', srid: 3857}, {bbox: params.bbox}); @@ -170,23 +168,3 @@ function getDataviewDefinition(mapConfig, dataviewName) { var dataviews = mapConfig.dataviews || {}; return dataviews[dataviewName]; } - -function dbParamsFromReqParams(params) { - var dbParams = {}; - if ( params.dbuser ) { - dbParams.user = params.dbuser; - } - if ( params.dbpassword ) { - dbParams.pass = params.dbpassword; - } - if ( params.dbhost ) { - dbParams.host = params.dbhost; - } - if ( params.dbport ) { - dbParams.port = params.dbport; - } - if ( params.dbname ) { - dbParams.dbname = params.dbname; - } - return dbParams; -} From 258d76888778ddf62f715b8a3ebeff2f49a80c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 17:54:40 +0100 Subject: [PATCH 20/62] Use upercase for constants --- lib/cartodb/controllers/layergroup.js | 38 +++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 614480a6..6ae62433 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -28,6 +28,21 @@ const SUPPORTED_FORMATS = { mvt: true }; +const ALLOWED_DATAVIEW_QUERY_PARAMS = [ + 'filters', // json + 'own_filter', // 0, 1 + 'no_filters', // 0, 1 + 'bbox', // w,s,e,n + 'start', // number + 'end', // number + 'column_type', // string + 'bins', // number + 'aggregation', //string + 'offset', // number + 'q', // widgets search + 'categories', // number +]; + /** * @param {prepareContext} prepareContext * @param {PgConnection} pgConnection @@ -242,25 +257,10 @@ LayergroupController.prototype.register = function(app) { // Undocumented/non-supported API endpoint methods. // Use at your own peril. - const allowedDataviewQueryParams = [ - 'filters', // json - 'own_filter', // 0, 1 - 'no_filters', // 0, 1 - 'bbox', // w,s,e,n - 'start', // number - 'end', // number - 'column_type', // string - 'bins', // number - 'aggregation', //string - 'offset', // number - 'q', // widgets search - 'categories', // number - ]; - app.get( `${mapConfigBasePath}/:token/dataview/:dataviewName`, cors(), - cleanUpQueryParams(allowedDataviewQueryParams), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), @@ -285,7 +285,7 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/:layer/widget/:dataviewName`, cors(), - cleanUpQueryParams(allowedDataviewQueryParams), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), @@ -310,7 +310,7 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/dataview/:dataviewName/search`, cors(), - cleanUpQueryParams(allowedDataviewQueryParams), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), @@ -335,7 +335,7 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/:layer/widget/:dataviewName/search`, cors(), - cleanUpQueryParams(allowedDataviewQueryParams), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), From 2812a5421009060deb131db3bb29d1397ad8ba64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 17:55:15 +0100 Subject: [PATCH 21/62] Pass only needed params to dataview backend --- lib/cartodb/controllers/layergroup.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 6ae62433..bc7026a7 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -456,7 +456,10 @@ function createMapStoreMapConfigProvider ( function getDataview (dataviewBackend) { return function getDataviewMiddleware (req, res, next) { const { user, mapConfigProvider } = res.locals; - const params = getRequestParams(res.locals); + const { dataviewName } = req.params; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + + const params = Object.assign({ dataviewName, dbuser, dbname, dbpassword, dbhost, dbport }, req.query); dataviewBackend.getDataview(mapConfigProvider, user, params, (err, dataview, stats = {}) => { req.profiler.add(stats); From 81706b8726b59a92858bcc2ceddb66fe064994a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 18:03:38 +0100 Subject: [PATCH 22/62] Pass only needed params to dataview backend (search) --- lib/cartodb/controllers/layergroup.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index bc7026a7..3a967680 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -478,8 +478,11 @@ function getDataview (dataviewBackend) { function dataviewSearch (dataviewBackend) { return function dataviewSearchMiddleware (req, res, next) { - const { user, dataviewName, mapConfigProvider } = res.locals; - const params = getRequestParams(res.locals); + const { user, mapConfigProvider } = res.locals; + const { dataviewName } = req.params; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + + const params = Object.assign({ dbuser, dbname, dbpassword, dbhost, dbport }, req.query); dataviewBackend.search(mapConfigProvider, user, dataviewName, params, (err, searchResult, stats = {}) => { req.profiler.add(stats); From d3cbd700545c10a043c4db83fbfe5ae121d15576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 18:16:41 +0100 Subject: [PATCH 23/62] Pass only needed params to attributes backend backend --- lib/cartodb/controllers/layergroup.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 3a967680..6a807474 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -504,7 +504,15 @@ function getFeatureAttributes (attributesBackend) { req.profiler.start('windshaft.maplayer_attribute'); const { mapConfigProvider } = res.locals; - const params = getRequestParams(res.locals); + const { token } = res.locals; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + const { layer, fid } = req.params; + + const params = { + token, + dbuser, dbname, dbpassword, dbhost, dbport, + layer, fid + }; attributesBackend.getFeatureAttributes(mapConfigProvider, params, false, (err, tile, stats = {}) => { req.profiler.add(stats); From 79955c7fac961976403b2fed3e71a5e4be83ed4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 18:27:40 +0100 Subject: [PATCH 24/62] Pass only needed params to tile backend --- lib/cartodb/controllers/layergroup.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 6a807474..63da4947 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -543,7 +543,10 @@ function getTile (tileBackend, profileLabel = 'tile') { req.profiler.start(`windshaft.${profileLabel}`); const { mapConfigProvider } = res.locals; - const params = getRequestParams(res.locals); + const { token } = res.locals; + const { layer, z, x, y, format } = req.params; + + const params = { token, layer, z, x, y, format }; tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats = {}) => { req.profiler.add(stats); From 8523875349394d9ac98ac7face76dd89902e47e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 18:29:00 +0100 Subject: [PATCH 25/62] Remove function thet is never used --- lib/cartodb/controllers/layergroup.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 63da4947..0930fbed 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -408,15 +408,6 @@ function analysisNodeStatus (analysisStatusBackend) { }; } -function getRequestParams(locals) { - const params = Object.assign({}, locals); - - delete params.mapConfigProvider; - delete params.allowedQueryParams; - - return params; -} - function createMapStoreMapConfigProvider ( mapStore, userLimitsApi, From afc608fc5da5576443ce092a9949c867eb1ff52a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 18:57:26 +0100 Subject: [PATCH 26/62] Pass only needed params to named map map config provider --- lib/cartodb/controllers/map.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index cdb1515f..192479e2 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -242,7 +242,11 @@ function getTemplate ( ) { return function getTemplateMiddleware (req, res, next) { const templateParams = req.body; - const { user } = res.locals; + const { user, dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + const { template_id } = req.params; + const { auth_token } = req.query; + + const params = { dbuser, dbname, dbpassword, dbhost, dbport }; const mapConfigProvider = new NamedMapMapConfigProvider( templateMaps, @@ -252,10 +256,10 @@ function getTemplate ( mapConfigAdapter, affectedTablesCache, user, - req.params.template_id, + template_id, templateParams, - res.locals.auth_token, - res.locals + auth_token, + params ); mapConfigProvider.getMapConfig((err, mapConfig, rendererParams) => { From 4f8c184bc0146fbbe5490a5b15dd28483e9da7d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 19:14:18 +0100 Subject: [PATCH 27/62] Pass only needed params to map config adapter --- lib/cartodb/controllers/map.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 192479e2..46ef911c 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -280,7 +280,10 @@ function getTemplate ( function prepareAdapterMapConfig (mapConfigAdapter) { return function prepareAdapterMapConfigMiddleware(req, res, next) { const requestMapConfig = req.body; - const { user, dbhost, dbport, dbname, dbuser, dbpassword, api_key } = res.locals; + + const { user, api_key } = res.locals; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + const params = Object.assign({ dbuser, dbname, dbpassword, dbhost, dbport }, req.query); const context = { analysisConfiguration: { @@ -299,7 +302,7 @@ function prepareAdapterMapConfig (mapConfigAdapter) { } }; - mapConfigAdapter.getMapConfig(user, requestMapConfig, res.locals, context, (err, requestMapConfig) => { + mapConfigAdapter.getMapConfig(user, requestMapConfig, params, context, (err, requestMapConfig) => { req.profiler.done('anonymous.getMapConfig'); if (err) { return next(err); From 6b7c2675f1d717b5ed873e97ef360d56c46ff1a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 19:20:51 +0100 Subject: [PATCH 28/62] Use database params module --- .../mapconfig/adapter/turbo-carto-adapter.js | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js b/lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js index b9487095..2bd30152 100644 --- a/lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/turbo-carto-adapter.js @@ -11,6 +11,8 @@ var PostgresDatasource = require('../../../backends/turbo-carto-postgres-datasou var MapConfig = require('windshaft').model.MapConfig; +const dbParamsFromReqParams = require('../../../utils/database-params'); + function TurboCartoAdapter() { } @@ -158,23 +160,3 @@ TurboCartoAdapter.prototype.process = function (psql, cartocss, sql, callback) { function shouldParseLayerCartocss(layer) { return layer && layer.options && layer.options.cartocss && layer.options.sql; } - -function dbParamsFromReqParams(params) { - var dbParams = {}; - if ( params.dbuser ) { - dbParams.user = params.dbuser; - } - if ( params.dbpassword ) { - dbParams.pass = params.dbpassword; - } - if ( params.dbhost ) { - dbParams.host = params.dbhost; - } - if ( params.dbport ) { - dbParams.port = params.dbport; - } - if ( params.dbname ) { - dbParams.dbname = params.dbname; - } - return dbParams; -} From d029f8199249b3b1a315c58c6a323ad744f0ee36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 19:36:42 +0100 Subject: [PATCH 29/62] Pass only needed params to create layergroup map config provider --- lib/cartodb/controllers/map.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 46ef911c..ef080e24 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -319,16 +319,26 @@ function prepareAdapterMapConfig (mapConfigAdapter) { function createLayergroup (mapBackend, userLimitsApi, pgConnection, affectedTablesCache) { return function createLayergroupMiddleware (req, res, next) { const requestMapConfig = req.body; - const { context, user } = res.locals; + + const { context } = res.locals; + const { user, cache_buster, api_key } = res.locals; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + + const params = { + cache_buster, api_key, + dbuser, dbname, dbpassword, dbhost, dbport + }; + const datasource = context.datasource || Datasource.EmptyDatasource(); const mapConfig = new MapConfig(requestMapConfig, datasource); + const mapConfigProvider = new CreateLayergroupMapConfigProvider( mapConfig, user, userLimitsApi, pgConnection, affectedTablesCache, - res.locals + params ); res.locals.mapConfig = mapConfig; From 4ff8d6fbc3683181b74b7af43a97c3dd225593a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 19:37:08 +0100 Subject: [PATCH 30/62] Pass only needed params to map backend --- lib/cartodb/controllers/map.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index ef080e24..5eb89b58 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -344,7 +344,9 @@ function createLayergroup (mapBackend, userLimitsApi, pgConnection, affectedTabl res.locals.mapConfig = mapConfig; res.locals.analysesResults = context.analysesResults; - mapBackend.createLayergroup(mapConfig, res.locals, mapConfigProvider, (err, layergroup) => { + const mapParams = { dbuser, dbname, dbpassword, dbhost, dbport }; + + mapBackend.createLayergroup(mapConfig, mapParams, mapConfigProvider, (err, layergroup) => { req.profiler.done('createLayergroup'); if (err) { return next(err); From c31639ebbd03198ecc69161002f6039030793653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 22 Mar 2018 19:38:56 +0100 Subject: [PATCH 31/62] Move assignments --- lib/cartodb/controllers/map.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 5eb89b58..e476daf3 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -341,9 +341,6 @@ function createLayergroup (mapBackend, userLimitsApi, pgConnection, affectedTabl params ); - res.locals.mapConfig = mapConfig; - res.locals.analysesResults = context.analysesResults; - const mapParams = { dbuser, dbname, dbpassword, dbhost, dbport }; mapBackend.createLayergroup(mapConfig, mapParams, mapConfigProvider, (err, layergroup) => { @@ -353,7 +350,9 @@ function createLayergroup (mapBackend, userLimitsApi, pgConnection, affectedTabl } res.body = layergroup; + res.locals.mapConfig = mapConfig; res.locals.mapConfigProvider = mapConfigProvider; + res.locals.analysesResults = context.analysesResults; next(); }); From ebefba9e3255bd3832fba9df0e247825f5ab742e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 10:57:35 +0100 Subject: [PATCH 32/62] Revert: move map-config assignment --- lib/cartodb/controllers/map.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index e476daf3..5eb89b58 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -341,6 +341,9 @@ function createLayergroup (mapBackend, userLimitsApi, pgConnection, affectedTabl params ); + res.locals.mapConfig = mapConfig; + res.locals.analysesResults = context.analysesResults; + const mapParams = { dbuser, dbname, dbpassword, dbhost, dbport }; mapBackend.createLayergroup(mapConfig, mapParams, mapConfigProvider, (err, layergroup) => { @@ -350,9 +353,7 @@ function createLayergroup (mapBackend, userLimitsApi, pgConnection, affectedTabl } res.body = layergroup; - res.locals.mapConfig = mapConfig; res.locals.mapConfigProvider = mapConfigProvider; - res.locals.analysesResults = context.analysesResults; next(); }); From 8be7ea5cc14fc63f17da390d9a780dc29f8b1854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 11:01:36 +0100 Subject: [PATCH 33/62] Pass only needed properties to named map provider cache --- lib/cartodb/controllers/named_maps.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 26c5e4ad..912020f7 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -127,16 +127,22 @@ NamedMapsController.prototype.register = function(app) { function getNamedMapProvider ({ namedMapProviderCache, label, forcedFormat = null }) { return function getNamedMapProviderMiddleware (req, res, next) { - const { user } = res.locals; - const { config, auth_token } = req.query; - const { template_id } = req.params; + const { user, token, cache_buster, api_key } = res.locals; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + const { template_id, layer, z, x, y, format } = req.params; + + const params = { + user, token, cache_buster, api_key, + dbuser, dbname, dbpassword, dbhost, dbport, + template_id, layer, z, x, y, format + }; if (forcedFormat) { - res.locals.format = forcedFormat; - res.locals.layer = res.locals.layer || 'all'; + params.format = forcedFormat; + params.layer = params.layer || 'all'; } - const params = getRequestParams(res.locals); + const { config, auth_token } = req.query; namedMapProviderCache.get(user, template_id, config, auth_token, params, (err, namedMapProvider) => { if (err) { From 10ead27676fdbc15be44ecaa94a9371db0802077 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 11:23:19 +0100 Subject: [PATCH 34/62] Pass only needed properties to named map provider cache (static endpoint) --- lib/cartodb/controllers/named_maps.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 912020f7..40faf214 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -176,8 +176,7 @@ function getTemplate ({ label }) { function prepareLayerFilterFromPreviewLayers ({ namedMapProviderCache, label }) { return function prepareLayerFilterFromPreviewLayersMiddleware (req, res, next) { - const { user, template } = res.locals; - const { template_id } = req.params; + const { template } = res.locals; const { config, auth_token } = req.query; if (!template || !template.view || !template.view.preview_layers) { @@ -197,7 +196,15 @@ function prepareLayerFilterFromPreviewLayers ({ namedMapProviderCache, label }) return next(); } - const params = getRequestParams(res.locals); + const { user, token, cache_buster, api_key } = res.locals; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + const { template_id, format } = req.params; + + const params = { + user, token, cache_buster, api_key, + dbuser, dbname, dbpassword, dbhost, dbport, + template_id, format + }; // overwrites 'all' default filter params.layer = layerVisibilityFilter.join(','); From 97a49fab2f2398c67475bbec6fa00eea6c8c0e3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 11:33:40 +0100 Subject: [PATCH 35/62] Remove function defined but nerver used --- lib/cartodb/controllers/named_maps.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 40faf214..a67befea 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -27,17 +27,6 @@ function numMapper(n) { return +n; } -function getRequestParams(locals) { - const params = Object.assign({}, locals); - - delete params.template; - delete params.affectedTables; - delete params.mapConfigProvider; - delete params.allowedQueryParams; - - return params; -} - function NamedMapsController ( namedMapProviderCache, tileBackend, From d3c9da6d5ff3b0f54d89cbc483630f4a2276a978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 11:57:28 +0100 Subject: [PATCH 36/62] Fix layer filter by query params --- lib/cartodb/controllers/named_maps.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index a67befea..5383581c 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -118,12 +118,13 @@ function getNamedMapProvider ({ namedMapProviderCache, label, forcedFormat = nul return function getNamedMapProviderMiddleware (req, res, next) { const { user, token, cache_buster, api_key } = res.locals; const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; - const { template_id, layer, z, x, y, format } = req.params; + const { template_id, layer: layerFromParams, z, x, y, format } = req.params; + const { layer: layerFromQuery } = req.query; const params = { user, token, cache_buster, api_key, dbuser, dbname, dbpassword, dbhost, dbport, - template_id, layer, z, x, y, format + template_id, layer: (layerFromQuery || layerFromParams), z, x, y, format }; if (forcedFormat) { From 7ba3394508b4970a313fa4909b18b2b0b06652b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 14:10:27 +0100 Subject: [PATCH 37/62] Do not merge req.params and req.query into res.locals (don't use locals middleware in analysis controller) --- lib/cartodb/controllers/analyses.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index 0da18bf6..81b1757a 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -1,7 +1,6 @@ const PSQL = require('cartodb-psql'); const cors = require('../middleware/cors'); const user = require('../middleware/user'); -const locals = require('../middleware/locals'); const cleanUpQueryParams = require('../middleware/clean-up-query-params'); const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); @@ -28,7 +27,6 @@ AnalysesController.prototype.register = function (app) { `${mapconfigBasePath}/analyses/catalog`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS_CATALOG), layergroupToken(), From f76606bc2632df81660fe0762aa2bbe2a0c4195e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 14:13:27 +0100 Subject: [PATCH 38/62] Do not use locals middleware in layergroup controller --- lib/cartodb/api/auth_api.js | 6 ++--- lib/cartodb/controllers/layergroup.js | 31 +++++++++------------- lib/cartodb/middleware/error-middleware.js | 12 +++------ lib/cartodb/middleware/layergroup-token.js | 5 ++-- 4 files changed, 21 insertions(+), 33 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index e9f10262..d27ea3e1 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -25,7 +25,7 @@ module.exports = AuthApi; // null if the request is not signed by anyone // or will be a string cartodb username otherwise. // -AuthApi.prototype.authorizedBySigner = function(res, callback) { +AuthApi.prototype.authorizedBySigner = function(req, res, callback) { if ( ! res.locals.token || ! res.locals.signer ) { return callback(null, false); // no signer requested } @@ -33,7 +33,7 @@ AuthApi.prototype.authorizedBySigner = function(res, callback) { var self = this; var layergroup_id = res.locals.token; - var auth_token = res.locals.auth_token; + var auth_token = req.query.auth_token; this.mapStore.load(layergroup_id, function(err, mapConfig) { if (err) { @@ -180,7 +180,7 @@ AuthApi.prototype.authorize = function(req, res, callback) { }); } - this.authorizedBySigner(res, (err, isAuthorizedBySigner) => { + this.authorizedBySigner(req, res, (err, isAuthorizedBySigner) => { if (err) { return callback(err); } diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 0930fbed..da9189ef 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -1,7 +1,6 @@ const cors = require('../middleware/cors'); const user = require('../middleware/user'); const vectorError = require('../middleware/vector-error'); -const locals = require('../middleware/locals'); const cleanUpQueryParams = require('../middleware/clean-up-query-params'); const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); @@ -91,7 +90,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:z/:x/:y@:scale_factor?x.:format`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), @@ -110,17 +108,16 @@ LayergroupController.prototype.register = function(app) { surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), - sendResponse(), incrementErrorMetrics(global.statsClient), tileError(), - vectorError() + vectorError(), + sendResponse() ); app.get( `${mapConfigBasePath}/:token/:z/:x/:y.:format`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), @@ -139,10 +136,10 @@ LayergroupController.prototype.register = function(app) { surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), - sendResponse(), incrementErrorMetrics(global.statsClient), tileError(), - vectorError() + vectorError(), + sendResponse() ); app.get( @@ -150,7 +147,6 @@ LayergroupController.prototype.register = function(app) { distinguishLayergroupFromStaticRoute(), cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), @@ -169,17 +165,16 @@ LayergroupController.prototype.register = function(app) { surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), lastModifiedHeader(), incrementSuccessMetrics(global.statsClient), - sendResponse(), incrementErrorMetrics(global.statsClient), tileError(), - vectorError() + vectorError(), + sendResponse() ); app.get( `${mapConfigBasePath}/:token/:layer/attributes/:fid`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ATTRIBUTES), layergroupToken(), @@ -206,7 +201,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, cors(), cleanUpQueryParams(['layer']), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), layergroupToken(), @@ -232,7 +226,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, cors(), cleanUpQueryParams(['layer']), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), layergroupToken(), @@ -261,7 +254,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/dataview/:dataviewName`, cors(), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), layergroupToken(), @@ -286,7 +278,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:layer/widget/:dataviewName`, cors(), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), layergroupToken(), @@ -311,7 +302,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/dataview/:dataviewName/search`, cors(), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), layergroupToken(), @@ -336,7 +326,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:layer/widget/:dataviewName/search`, cors(), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), layergroupToken(), @@ -361,7 +350,6 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/analysis/node/:nodeId`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS), layergroupToken(), @@ -521,7 +509,7 @@ function getFeatureAttributes (attributesBackend) { } function getStatusCode(tile, format){ - return tile.length === 0 && format === 'mvt'? 204 : 200; + return tile.length === 0 && format === 'mvt' ? 204 : 200; } function parseFormat (format = '') { @@ -654,6 +642,11 @@ function incrementErrorMetrics (statsClient) { function tileError () { return function tileErrorMiddleware (err, req, res, next) { + if (err.message === 'Tile does not exist' && req.params.format === 'mvt') { + res.statusCode = 204; + return next(); + } + // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 let errMsg = err.message ? ( '' + err.message ) : ( '' + err ); diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index 85f38936..c02974b4 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -15,10 +15,6 @@ module.exports = function errorMiddleware (/* options */) { var statusCode = findStatusCode(err); - if (err.message === 'Tile does not exist' && res.locals.format === 'mvt') { - statusCode = 204; - } - setErrorHeader(allErrors, statusCode, res); debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); @@ -186,15 +182,15 @@ function setErrorHeader(errors, statusCode, res) { subtype: error.subtype }; }); - + res.set('X-Tiler-Errors', stringifyForLogs(errorsLog)); } /** - * Remove problematic nested characters + * Remove problematic nested characters * from object for logs RegEx - * - * @param {Object} object + * + * @param {Object} object */ function stringifyForLogs(object) { Object.keys(object).map(key => { diff --git a/lib/cartodb/middleware/layergroup-token.js b/lib/cartodb/middleware/layergroup-token.js index 797b1b3d..c3fcec30 100644 --- a/lib/cartodb/middleware/layergroup-token.js +++ b/lib/cartodb/middleware/layergroup-token.js @@ -5,13 +5,12 @@ const authErrorMessageTemplate = function (signer, user) { module.exports = function layergroupToken () { return function layergroupTokenMiddleware (req, res, next) { - if (!res.locals.token) { + if (!req.params.token) { return next(); } const user = res.locals.user; - - const layergroupToken = LayergroupToken.parse(res.locals.token); + const layergroupToken = LayergroupToken.parse(req.params.token); res.locals.token = layergroupToken.token; res.locals.cache_buster = layergroupToken.cacheBuster; From 516b1f765ec8af5efc3e118050084d0ed9a90388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 16:08:52 +0100 Subject: [PATCH 39/62] Do not use middleware local in map controller --- lib/cartodb/controllers/map.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 5eb89b58..9b8694f8 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -5,7 +5,6 @@ const Datasource = windshaft.model.Datasource; const ResourceLocator = require('../models/resource-locator'); const cors = require('../middleware/cors'); const user = require('../middleware/user'); -const locals = require('../middleware/locals'); const cleanUpQueryParams = require('../middleware/clean-up-query-params'); const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); @@ -103,7 +102,6 @@ MapController.prototype.composeCreateMapMiddleware = function (endpointGroup, us return [ cors(), cleanUpQueryParams(['aggregation']), - locals(), user(), rateLimit(this.userLimitsApi, endpointGroup), layergroupToken(), @@ -214,7 +212,7 @@ function checkInstantiteLayergroup () { function checkCreateLayergroup () { return function checkCreateLayergroupMiddleware (req, res, next) { if (req.method === 'GET') { - const { config } = res.locals; + const { config } = req.query; if (!config) { return next(new Error('layergroup GET needs a "config" parameter')); From f7a23c094cc82a77b37ec25d17371dd45500b280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 16:16:53 +0100 Subject: [PATCH 40/62] Do not use locals middleware in named maps admin controller --- lib/cartodb/controllers/named_maps_admin.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 136e9df7..d971aa45 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -1,7 +1,6 @@ const { templateName } = require('../backends/template_maps'); const cors = require('../middleware/cors'); const user = require('../middleware/user'); -const locals = require('../middleware/locals'); const credentials = require('../middleware/credentials'); const rateLimit = require('../middleware/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; @@ -27,7 +26,6 @@ NamedMapsAdminController.prototype.register = function (app) { app.post( `${templateBasePath}/`, cors(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_CREATE), credentials(), @@ -40,7 +38,6 @@ NamedMapsAdminController.prototype.register = function (app) { app.put( `${templateBasePath}/:template_id`, cors(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_UPDATE), credentials(), @@ -53,7 +50,6 @@ NamedMapsAdminController.prototype.register = function (app) { app.get( `${templateBasePath}/:template_id`, cors(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_GET), credentials(), @@ -65,7 +61,6 @@ NamedMapsAdminController.prototype.register = function (app) { app.delete( `${templateBasePath}/:template_id`, cors(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_DELETE), credentials(), @@ -77,7 +72,6 @@ NamedMapsAdminController.prototype.register = function (app) { app.get( `${templateBasePath}/`, cors(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_LIST), credentials(), From 5fc801f8a68e5f1faca0c181ba9372ddce5cf5a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 16:38:55 +0100 Subject: [PATCH 41/62] Do not use locals middleware in named maps controller --- lib/cartodb/controllers/named_maps.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 5383581c..b0d4cf23 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -1,6 +1,5 @@ const cors = require('../middleware/cors'); const user = require('../middleware/user'); -const locals = require('../middleware/locals'); const cleanUpQueryParams = require('../middleware/clean-up-query-params'); const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); @@ -58,7 +57,6 @@ NamedMapsController.prototype.register = function(app) { `${templateBasePath}/:template_id/:layer/:z/:x/:y.(:format)`, cors(), cleanUpQueryParams(), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_TILES), layergroupToken(), @@ -86,7 +84,6 @@ NamedMapsController.prototype.register = function(app) { `${mapconfigBasePath}/static/named/:template_id/:width/:height.:format`, cors(), cleanUpQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), - locals(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC_NAMED), layergroupToken(), @@ -215,9 +212,11 @@ function prepareLayerFilterFromPreviewLayers ({ namedMapProviderCache, label }) function getTile ({ tileBackend, label }) { return function getTileMiddleware (req, res, next) { - const { mapConfigProvider, format } = res.locals; + const { mapConfigProvider } = res.locals; + const { layer, z, x, y, format } = req.params; + const params = { layer, z, x, y, format }; - tileBackend.getTile(mapConfigProvider, req.params, (err, tile, headers, stats) => { + tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats) => { req.profiler.add(stats); req.profiler.done('render-' + format); @@ -240,8 +239,10 @@ function getTile ({ tileBackend, label }) { function getStaticImageOptions ({ tablesExtentApi }) { return function getStaticImageOptionsMiddleware(req, res, next) { const { user, mapConfigProvider, template } = res.locals; + const { zoom, lon, lat, bbox } = req.query; + const params = { zoom, lon, lat, bbox }; - const imageOpts = getImageOptions(res.locals, template); + const imageOpts = getImageOptions(params, template); if (imageOpts) { res.locals.imageOpts = imageOpts; From 5bc5c0ae8674bdb76b9e75f9e1e570ef40e858a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 16:53:00 +0100 Subject: [PATCH 42/62] Remove locals middleware --- lib/cartodb/middleware/locals.js | 7 ------- test/unit/cartodb/prepare-context.test.js | 13 ------------- 2 files changed, 20 deletions(-) delete mode 100644 lib/cartodb/middleware/locals.js diff --git a/lib/cartodb/middleware/locals.js b/lib/cartodb/middleware/locals.js deleted file mode 100644 index 2629767e..00000000 --- a/lib/cartodb/middleware/locals.js +++ /dev/null @@ -1,7 +0,0 @@ -module.exports = function locals () { - return function localsMiddleware (req, res, next) { - res.locals = Object.assign({}, req.query, req.params); - - next(); - }; -}; diff --git a/test/unit/cartodb/prepare-context.test.js b/test/unit/cartodb/prepare-context.test.js index 82526fe9..ff311cc6 100644 --- a/test/unit/cartodb/prepare-context.test.js +++ b/test/unit/cartodb/prepare-context.test.js @@ -11,7 +11,6 @@ const cleanUpQueryParamsMiddleware = require('../../../lib/cartodb/middleware/cl const authorizeMiddleware = require('../../../lib/cartodb/middleware/authorize'); const dbConnSetupMiddleware = require('../../../lib/cartodb/middleware/db-conn-setup'); const credentialsMiddleware = require('../../../lib/cartodb/middleware/credentials'); -const localsMiddleware = require('../../../lib/cartodb/middleware/locals'); var windshaft = require('windshaft'); @@ -66,18 +65,6 @@ describe('prepare-context', function() { return res; } - it('res.locals are created', function(done) { - const locals = localsMiddleware(); - let req = {}; - let res = {}; - - locals(prepareRequest(req), prepareResponse(res), function(err) { - if ( err ) { done(err); return; } - assert.ok(res.hasOwnProperty('locals'), 'response has locals'); - done(); - }); - }); - it('cleans up request', function(done){ var req = {headers: { host:'localhost' }, query: {dbuser:'hacker',dbname:'secret'}}; var res = {}; From 3b1fd05940b1c17ca7a77ef53b5affa17a700125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 17:24:56 +0100 Subject: [PATCH 43/62] Use layergroup token middleware where it's actually needed --- lib/cartodb/controllers/analyses.js | 2 -- lib/cartodb/controllers/map.js | 2 -- lib/cartodb/controllers/named_maps.js | 3 --- lib/cartodb/middleware/layergroup-token.js | 4 ---- 4 files changed, 11 deletions(-) diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index 81b1757a..9a48e30f 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -2,7 +2,6 @@ const PSQL = require('cartodb-psql'); const cors = require('../middleware/cors'); const user = require('../middleware/user'); const cleanUpQueryParams = require('../middleware/clean-up-query-params'); -const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const authorize = require('../middleware/authorize'); const dbConnSetup = require('../middleware/db-conn-setup'); @@ -29,7 +28,6 @@ AnalysesController.prototype.register = function (app) { cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS_CATALOG), - layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 9b8694f8..0ec3a160 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -6,7 +6,6 @@ const ResourceLocator = require('../models/resource-locator'); const cors = require('../middleware/cors'); const user = require('../middleware/user'); const cleanUpQueryParams = require('../middleware/clean-up-query-params'); -const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); @@ -104,7 +103,6 @@ MapController.prototype.composeCreateMapMiddleware = function (endpointGroup, us cleanUpQueryParams(['aggregation']), user(), rateLimit(this.userLimitsApi, endpointGroup), - layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index b0d4cf23..f00acb8e 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -1,7 +1,6 @@ const cors = require('../middleware/cors'); const user = require('../middleware/user'); const cleanUpQueryParams = require('../middleware/clean-up-query-params'); -const layergroupToken = require('../middleware/layergroup-token'); const credentials = require('../middleware/credentials'); const dbConnSetup = require('../middleware/db-conn-setup'); const authorize = require('../middleware/authorize'); @@ -59,7 +58,6 @@ NamedMapsController.prototype.register = function(app) { cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_TILES), - layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), @@ -86,7 +84,6 @@ NamedMapsController.prototype.register = function(app) { cleanUpQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC_NAMED), - layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), diff --git a/lib/cartodb/middleware/layergroup-token.js b/lib/cartodb/middleware/layergroup-token.js index c3fcec30..1a32e413 100644 --- a/lib/cartodb/middleware/layergroup-token.js +++ b/lib/cartodb/middleware/layergroup-token.js @@ -5,10 +5,6 @@ const authErrorMessageTemplate = function (signer, user) { module.exports = function layergroupToken () { return function layergroupTokenMiddleware (req, res, next) { - if (!req.params.token) { - return next(); - } - const user = res.locals.user; const layergroupToken = LayergroupToken.parse(req.params.token); From 4cba4c7a1f0cb4d0fd36cc18e61c776f455c6043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 17:37:06 +0100 Subject: [PATCH 44/62] Tidy middlewares up: cleanUpQeuryParams --- lib/cartodb/controllers/analyses.js | 2 +- lib/cartodb/controllers/layergroup.js | 22 +++++++++++----------- lib/cartodb/controllers/map.js | 2 +- lib/cartodb/controllers/named_maps.js | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index 9a48e30f..9ed6e092 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -25,12 +25,12 @@ AnalysesController.prototype.register = function (app) { app.get( `${mapconfigBasePath}/analyses/catalog`, cors(), - cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS_CATALOG), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(), createPGClient(), getDataFromQuery({ queryTemplate: catalogQueryTpl, key: 'catalog' }), getDataFromQuery({ queryTemplate: tablesQueryTpl, key: 'tables' }), diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index da9189ef..fa07f73e 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -89,13 +89,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/:z/:x/:y@:scale_factor?x.:format`, cors(), - cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -117,13 +117,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/:z/:x/:y.:format`, cors(), - cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -146,13 +146,13 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:layer/:z/:x/:y.(:format)`, distinguishLayergroupFromStaticRoute(), cors(), - cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -174,13 +174,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/:layer/attributes/:fid`, cors(), - cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ATTRIBUTES), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -200,13 +200,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, cors(), - cleanUpQueryParams(['layer']), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(['layer']), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -225,13 +225,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, cors(), - cleanUpQueryParams(['layer']), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(['layer']), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -253,13 +253,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/dataview/:dataviewName`, cors(), - cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -277,13 +277,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/:layer/widget/:dataviewName`, cors(), - cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -301,13 +301,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/dataview/:dataviewName/search`, cors(), - cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -325,13 +325,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/:layer/widget/:dataviewName/search`, cors(), - cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), createMapStoreMapConfigProvider( this.mapStore, this.userLimitsApi, @@ -349,13 +349,13 @@ LayergroupController.prototype.register = function(app) { app.get( `${mapConfigBasePath}/:token/analysis/node/:nodeId`, cors(), - cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(), analysisNodeStatus(this.analysisStatusBackend), sendResponse() ); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 0ec3a160..5aea9273 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -100,12 +100,12 @@ MapController.prototype.composeCreateMapMiddleware = function (endpointGroup, us return [ cors(), - cleanUpQueryParams(['aggregation']), user(), rateLimit(this.userLimitsApi, endpointGroup), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(['aggregation']), initProfiler(isTemplateInstantiation), checkJsonContentType(), this.getCreateMapMiddlewares(useTemplate), diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index f00acb8e..37794c6a 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -55,12 +55,12 @@ NamedMapsController.prototype.register = function(app) { app.get( `${templateBasePath}/:template_id/:layer/:z/:x/:y.(:format)`, cors(), - cleanUpQueryParams(), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_TILES), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(), getNamedMapProvider({ namedMapProviderCache: this.namedMapProviderCache, label: 'NAMED_MAP_TILE' @@ -81,12 +81,12 @@ NamedMapsController.prototype.register = function(app) { app.get( `${mapconfigBasePath}/static/named/:template_id/:width/:height.:format`, cors(), - cleanUpQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), user(), rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC_NAMED), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + cleanUpQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), getNamedMapProvider({ namedMapProviderCache: this.namedMapProviderCache, label: 'STATIC_VIZ_MAP', forcedFormat: 'png' From d3e2707fce19339f61abd6f9f105a254f890a689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 17:55:41 +0100 Subject: [PATCH 45/62] Tidy middlewares up: put rate limit middleware after authorization --- lib/cartodb/controllers/analyses.js | 2 +- lib/cartodb/controllers/layergroup.js | 22 ++++++++++----------- lib/cartodb/controllers/map.js | 2 +- lib/cartodb/controllers/named_maps.js | 4 ++-- lib/cartodb/controllers/named_maps_admin.js | 10 +++++----- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index 9ed6e092..2d89a091 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -26,10 +26,10 @@ AnalysesController.prototype.register = function (app) { `${mapconfigBasePath}/analyses/catalog`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS_CATALOG), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS_CATALOG), cleanUpQueryParams(), createPGClient(), getDataFromQuery({ queryTemplate: catalogQueryTpl, key: 'catalog' }), diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index fa07f73e..876630d7 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -90,11 +90,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:z/:x/:y@:scale_factor?x.:format`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), cleanUpQueryParams(), createMapStoreMapConfigProvider( this.mapStore, @@ -118,11 +118,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:z/:x/:y.:format`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), cleanUpQueryParams(), createMapStoreMapConfigProvider( this.mapStore, @@ -147,11 +147,11 @@ LayergroupController.prototype.register = function(app) { distinguishLayergroupFromStaticRoute(), cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), cleanUpQueryParams(), createMapStoreMapConfigProvider( this.mapStore, @@ -175,11 +175,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:layer/attributes/:fid`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ATTRIBUTES), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ATTRIBUTES), cleanUpQueryParams(), createMapStoreMapConfigProvider( this.mapStore, @@ -201,11 +201,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), cleanUpQueryParams(['layer']), createMapStoreMapConfigProvider( this.mapStore, @@ -226,11 +226,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), cleanUpQueryParams(['layer']), createMapStoreMapConfigProvider( this.mapStore, @@ -254,11 +254,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/dataview/:dataviewName`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), createMapStoreMapConfigProvider( this.mapStore, @@ -278,11 +278,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:layer/widget/:dataviewName`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), createMapStoreMapConfigProvider( this.mapStore, @@ -302,11 +302,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/dataview/:dataviewName/search`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), createMapStoreMapConfigProvider( this.mapStore, @@ -326,11 +326,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/:layer/widget/:dataviewName/search`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), createMapStoreMapConfigProvider( this.mapStore, @@ -350,11 +350,11 @@ LayergroupController.prototype.register = function(app) { `${mapConfigBasePath}/:token/analysis/node/:nodeId`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS), layergroupToken(), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS), cleanUpQueryParams(), analysisNodeStatus(this.analysisStatusBackend), sendResponse() diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 5aea9273..11074234 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -101,10 +101,10 @@ MapController.prototype.composeCreateMapMiddleware = function (endpointGroup, us return [ cors(), user(), - rateLimit(this.userLimitsApi, endpointGroup), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, endpointGroup), cleanUpQueryParams(['aggregation']), initProfiler(isTemplateInstantiation), checkJsonContentType(), diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 37794c6a..2cb4f609 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -56,10 +56,10 @@ NamedMapsController.prototype.register = function(app) { `${templateBasePath}/:template_id/:layer/:z/:x/:y.(:format)`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_TILES), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_TILES), cleanUpQueryParams(), getNamedMapProvider({ namedMapProviderCache: this.namedMapProviderCache, @@ -82,10 +82,10 @@ NamedMapsController.prototype.register = function(app) { `${mapconfigBasePath}/static/named/:template_id/:width/:height.:format`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC_NAMED), credentials(), authorize(this.authApi), dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC_NAMED), cleanUpQueryParams(['layer', 'zoom', 'lon', 'lat', 'bbox']), getNamedMapProvider({ namedMapProviderCache: this.namedMapProviderCache, diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index d971aa45..afcaa1e3 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -27,10 +27,10 @@ NamedMapsAdminController.prototype.register = function (app) { `${templateBasePath}/`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_CREATE), credentials(), checkContentType({ action: 'POST', label: 'POST TEMPLATE' }), authorizedByAPIKey({ authApi: this.authApi, action: 'create', label: 'POST TEMPLATE' }), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_CREATE), createTemplate({ templateMaps: this.templateMaps }), sendResponse() ); @@ -39,10 +39,10 @@ NamedMapsAdminController.prototype.register = function (app) { `${templateBasePath}/:template_id`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_UPDATE), credentials(), checkContentType({ action: 'PUT', label: 'PUT TEMPLATE' }), authorizedByAPIKey({ authApi: this.authApi, action: 'update', label: 'PUT TEMPLATE' }), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_UPDATE), updateTemplate({ templateMaps: this.templateMaps }), sendResponse() ); @@ -51,9 +51,9 @@ NamedMapsAdminController.prototype.register = function (app) { `${templateBasePath}/:template_id`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_GET), credentials(), authorizedByAPIKey({ authApi: this.authApi, action: 'get', label: 'GET TEMPLATE' }), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_GET), retrieveTemplate({ templateMaps: this.templateMaps }), sendResponse() ); @@ -62,9 +62,9 @@ NamedMapsAdminController.prototype.register = function (app) { `${templateBasePath}/:template_id`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_DELETE), credentials(), authorizedByAPIKey({ authApi: this.authApi, action: 'delete', label: 'DELETE TEMPLATE' }), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_DELETE), destroyTemplate({ templateMaps: this.templateMaps }), sendResponse() ); @@ -73,9 +73,9 @@ NamedMapsAdminController.prototype.register = function (app) { `${templateBasePath}/`, cors(), user(), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_LIST), credentials(), authorizedByAPIKey({ authApi: this.authApi, action: 'list', label: 'GET TEMPLATE LIST' }), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED_LIST), listTemplates({ templateMaps: this.templateMaps }), sendResponse() ); From c5c8dd7ad7d902858bfa8ccb2c0db7c89002f590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Mar 2018 21:20:37 +0100 Subject: [PATCH 46/62] Split layergroup controllers into small controllers --- lib/cartodb/controllers/layergroup.js | 665 ------------------ .../controllers/layergroup/analysis.js | 75 ++ .../controllers/layergroup/attributes.js | 93 +++ .../controllers/layergroup/dataview.js | 199 ++++++ lib/cartodb/controllers/layergroup/index.js | 114 +++ .../map-store-map-config-provider.js | 37 + lib/cartodb/controllers/layergroup/static.js | 161 +++++ lib/cartodb/controllers/layergroup/tile.js | 230 ++++++ 8 files changed, 909 insertions(+), 665 deletions(-) delete mode 100644 lib/cartodb/controllers/layergroup.js create mode 100644 lib/cartodb/controllers/layergroup/analysis.js create mode 100644 lib/cartodb/controllers/layergroup/attributes.js create mode 100644 lib/cartodb/controllers/layergroup/dataview.js create mode 100644 lib/cartodb/controllers/layergroup/index.js create mode 100644 lib/cartodb/controllers/layergroup/middlewares/map-store-map-config-provider.js create mode 100644 lib/cartodb/controllers/layergroup/static.js create mode 100644 lib/cartodb/controllers/layergroup/tile.js diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js deleted file mode 100644 index 876630d7..00000000 --- a/lib/cartodb/controllers/layergroup.js +++ /dev/null @@ -1,665 +0,0 @@ -const cors = require('../middleware/cors'); -const user = require('../middleware/user'); -const vectorError = require('../middleware/vector-error'); -const cleanUpQueryParams = require('../middleware/clean-up-query-params'); -const layergroupToken = require('../middleware/layergroup-token'); -const credentials = require('../middleware/credentials'); -const dbConnSetup = require('../middleware/db-conn-setup'); -const authorize = require('../middleware/authorize'); -const rateLimit = require('../middleware/rate-limit'); -const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; -const cacheControlHeader = require('../middleware/cache-control-header'); -const cacheChannelHeader = require('../middleware/cache-channel-header'); -const surrogateKeyHeader = require('../middleware/surrogate-key-header'); -const lastModifiedHeader = require('../middleware/last-modified-header'); -const sendResponse = require('../middleware/send-response'); -const DataviewBackend = require('../backends/dataview'); -const AnalysisStatusBackend = require('../backends/analysis-status'); -const MapStoreMapConfigProvider = require('../models/mapconfig/provider/map-store-provider'); -const dbParamsFromResLocals = require('../utils/database-params'); - -const SUPPORTED_FORMATS = { - grid_json: true, - json_torque: true, - torque_json: true, - png: true, - png32: true, - mvt: true -}; - -const ALLOWED_DATAVIEW_QUERY_PARAMS = [ - 'filters', // json - 'own_filter', // 0, 1 - 'no_filters', // 0, 1 - 'bbox', // w,s,e,n - 'start', // number - 'end', // number - 'column_type', // string - 'bins', // number - 'aggregation', //string - 'offset', // number - 'q', // widgets search - 'categories', // number -]; - -/** - * @param {prepareContext} prepareContext - * @param {PgConnection} pgConnection - * @param {MapStore} mapStore - * @param {TileBackend} tileBackend - * @param {PreviewBackend} previewBackend - * @param {AttributesBackend} attributesBackend - * @param {SurrogateKeysCache} surrogateKeysCache - * @param {UserLimitsApi} userLimitsApi - * @param {LayergroupAffectedTables} layergroupAffectedTables - * @param {AnalysisBackend} analysisBackend - * @constructor - */ -function LayergroupController( - pgConnection, - mapStore, - tileBackend, - previewBackend, - attributesBackend, - surrogateKeysCache, - userLimitsApi, - layergroupAffectedTablesCache, - analysisBackend, - authApi -) { - this.pgConnection = pgConnection; - this.mapStore = mapStore; - this.tileBackend = tileBackend; - this.previewBackend = previewBackend; - this.attributesBackend = attributesBackend; - this.surrogateKeysCache = surrogateKeysCache; - this.userLimitsApi = userLimitsApi; - this.layergroupAffectedTablesCache = layergroupAffectedTablesCache; - - this.dataviewBackend = new DataviewBackend(analysisBackend); - this.analysisStatusBackend = new AnalysisStatusBackend(); - this.authApi = authApi; -} - -module.exports = LayergroupController; - -LayergroupController.prototype.register = function(app) { - const { base_url_mapconfig: mapConfigBasePath } = app; - - app.get( - `${mapConfigBasePath}/:token/:z/:x/:y@:scale_factor?x.:format`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), - cleanUpQueryParams(), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache - ), - getTile(this.tileBackend, 'map_tile'), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - incrementSuccessMetrics(global.statsClient), - incrementErrorMetrics(global.statsClient), - tileError(), - vectorError(), - sendResponse() - ); - - app.get( - `${mapConfigBasePath}/:token/:z/:x/:y.:format`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), - cleanUpQueryParams(), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache - ), - getTile(this.tileBackend, 'map_tile'), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - incrementSuccessMetrics(global.statsClient), - incrementErrorMetrics(global.statsClient), - tileError(), - vectorError(), - sendResponse() - ); - - app.get( - `${mapConfigBasePath}/:token/:layer/:z/:x/:y.(:format)`, - distinguishLayergroupFromStaticRoute(), - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), - cleanUpQueryParams(), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache - ), - getTile(this.tileBackend, 'maplayer_tile'), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - incrementSuccessMetrics(global.statsClient), - incrementErrorMetrics(global.statsClient), - tileError(), - vectorError(), - sendResponse() - ); - - app.get( - `${mapConfigBasePath}/:token/:layer/attributes/:fid`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ATTRIBUTES), - cleanUpQueryParams(), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache - ), - getFeatureAttributes(this.attributesBackend), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - sendResponse() - ); - - const forcedFormat = 'png'; - - app.get( - `${mapConfigBasePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), - cleanUpQueryParams(['layer']), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache, - forcedFormat - ), - getPreviewImageByCenter(this.previewBackend), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - sendResponse() - ); - - app.get( - `${mapConfigBasePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), - cleanUpQueryParams(['layer']), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache, - forcedFormat - ), - getPreviewImageByBoundingBox(this.previewBackend), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - sendResponse() - ); - - // Undocumented/non-supported API endpoint methods. - // Use at your own peril. - - app.get( - `${mapConfigBasePath}/:token/dataview/:dataviewName`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), - cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache - ), - getDataview(this.dataviewBackend), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - sendResponse() - ); - - app.get( - `${mapConfigBasePath}/:token/:layer/widget/:dataviewName`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), - cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache - ), - getDataview(this.dataviewBackend), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - sendResponse() - ); - - app.get( - `${mapConfigBasePath}/:token/dataview/:dataviewName/search`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), - cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache - ), - dataviewSearch(this.dataviewBackend), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - sendResponse() - ); - - app.get( - `${mapConfigBasePath}/:token/:layer/widget/:dataviewName/search`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), - cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), - createMapStoreMapConfigProvider( - this.mapStore, - this.userLimitsApi, - this.pgConnection, - this.layergroupAffectedTablesCache - ), - dataviewSearch(this.dataviewBackend), - cacheControlHeader(), - cacheChannelHeader(), - surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), - lastModifiedHeader(), - sendResponse() - ); - - app.get( - `${mapConfigBasePath}/:token/analysis/node/:nodeId`, - cors(), - user(), - layergroupToken(), - credentials(), - authorize(this.authApi), - dbConnSetup(this.pgConnection), - rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS), - cleanUpQueryParams(), - analysisNodeStatus(this.analysisStatusBackend), - sendResponse() - ); -}; - -function distinguishLayergroupFromStaticRoute () { - return function distinguishLayergroupFromStaticRouteMiddleware(req, res, next) { - if (req.params.token === 'static') { - return next('route'); - } - - next(); - }; -} - -function analysisNodeStatus (analysisStatusBackend) { - return function analysisNodeStatusMiddleware(req, res, next) { - const { nodeId } = req.params; - const dbParams = dbParamsFromResLocals(res.locals); - - analysisStatusBackend.getNodeStatus(nodeId, dbParams, (err, nodeStatus, stats = {}) => { - req.profiler.add(stats); - - if (err) { - err.label = 'GET NODE STATUS'; - return next(err); - } - - res.set({ - 'Cache-Control': 'public,max-age=5', - 'Last-Modified': new Date().toUTCString() - }); - - res.body = nodeStatus; - - next(); - }); - }; -} - -function createMapStoreMapConfigProvider ( - mapStore, - userLimitsApi, - pgConnection, - affectedTablesCache, - forcedFormat = null -) { - return function createMapStoreMapConfigProviderMiddleware (req, res, next) { - const { user, token, cache_buster, api_key } = res.locals; - const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; - const { layer, z, x, y, scale_factor, format } = req.params; - - const params = { - user, token, cache_buster, api_key, - dbuser, dbname, dbpassword, dbhost, dbport, - layer, z, x, y, scale_factor, format - }; - - if (forcedFormat) { - params.format = forcedFormat; - params.layer = params.layer || 'all'; - } - - res.locals.mapConfigProvider = new MapStoreMapConfigProvider( - mapStore, - user, - userLimitsApi, - pgConnection, - affectedTablesCache, - params - ); - - next(); - }; -} - -function getDataview (dataviewBackend) { - return function getDataviewMiddleware (req, res, next) { - const { user, mapConfigProvider } = res.locals; - const { dataviewName } = req.params; - const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; - - const params = Object.assign({ dataviewName, dbuser, dbname, dbpassword, dbhost, dbport }, req.query); - - dataviewBackend.getDataview(mapConfigProvider, user, params, (err, dataview, stats = {}) => { - req.profiler.add(stats); - - if (err) { - err.label = 'GET DATAVIEW'; - return next(err); - } - - res.body = dataview; - - next(); - }); - }; -} - -function dataviewSearch (dataviewBackend) { - return function dataviewSearchMiddleware (req, res, next) { - const { user, mapConfigProvider } = res.locals; - const { dataviewName } = req.params; - const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; - - const params = Object.assign({ dbuser, dbname, dbpassword, dbhost, dbport }, req.query); - - dataviewBackend.search(mapConfigProvider, user, dataviewName, params, (err, searchResult, stats = {}) => { - req.profiler.add(stats); - - if (err) { - err.label = 'GET DATAVIEW SEARCH'; - return next(err); - } - - res.body = searchResult; - - next(); - }); - }; -} - -function getFeatureAttributes (attributesBackend) { - return function getFeatureAttributesMiddleware (req, res, next) { - req.profiler.start('windshaft.maplayer_attribute'); - - const { mapConfigProvider } = res.locals; - const { token } = res.locals; - const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; - const { layer, fid } = req.params; - - const params = { - token, - dbuser, dbname, dbpassword, dbhost, dbport, - layer, fid - }; - - attributesBackend.getFeatureAttributes(mapConfigProvider, params, false, (err, tile, stats = {}) => { - req.profiler.add(stats); - - if (err) { - err.label = 'GET ATTRIBUTES'; - return next(err); - } - - res.body = tile; - - next(); - }); - }; -} - -function getStatusCode(tile, format){ - return tile.length === 0 && format === 'mvt' ? 204 : 200; -} - -function parseFormat (format = '') { - const prettyFormat = format.replace('.', '_'); - return SUPPORTED_FORMATS[prettyFormat] ? prettyFormat : 'invalid'; -} - -function getTile (tileBackend, profileLabel = 'tile') { - return function getTileMiddleware (req, res, next) { - req.profiler.start(`windshaft.${profileLabel}`); - - const { mapConfigProvider } = res.locals; - const { token } = res.locals; - const { layer, z, x, y, format } = req.params; - - const params = { token, layer, z, x, y, format }; - - 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 getPreviewImageByCenter (previewBackend) { - return function getPreviewImageByCenterMiddleware (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 - }; - - const format = req.params.format === 'jpg' ? 'jpeg' : 'png'; - const { mapConfigProvider: provider } = res.locals; - - previewBackend.getImage(provider, format, width, height, zoom, center, (err, image, headers, stats = {}) => { - req.profiler.done(`render-${format}`); - req.profiler.add(stats); - - if (err) { - err.label = 'STATIC_MAP'; - return next(err); - } - - if (headers) { - res.set(headers); - } - - res.set('Content-Type', headers['Content-Type'] || `image/${format}`); - - res.body = image; - - next(); - }); - }; -} - -function getPreviewImageByBoundingBox (previewBackend) { - return function getPreviewImageByBoundingBoxMiddleware (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'; - const { mapConfigProvider: provider } = res.locals; - - previewBackend.getImage(provider, 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); - } - - if (headers) { - res.set(headers); - } - - res.set('Content-Type', headers['Content-Type'] || `image/${format}`); - - res.body = image; - - next(); - }); - }; -} - -function incrementSuccessMetrics (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(); - }; -} - -function incrementErrorMetrics (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); - }; -} - -function tileError () { - return function tileErrorMiddleware (err, req, res, next) { - if (err.message === 'Tile does not exist' && req.params.format === 'mvt') { - res.statusCode = 204; - return 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); - }; -} diff --git a/lib/cartodb/controllers/layergroup/analysis.js b/lib/cartodb/controllers/layergroup/analysis.js new file mode 100644 index 00000000..3022bce3 --- /dev/null +++ b/lib/cartodb/controllers/layergroup/analysis.js @@ -0,0 +1,75 @@ +const cors = require('../../middleware/cors'); +const user = require('../../middleware/user'); +const layergroupToken = require('../../middleware/layergroup-token'); +const cleanUpQueryParams = require('../../middleware/clean-up-query-params'); +const credentials = require('../../middleware/credentials'); +const dbConnSetup = require('../../middleware/db-conn-setup'); +const authorize = require('../../middleware/authorize'); +const rateLimit = require('../../middleware/rate-limit'); +const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const sendResponse = require('../../middleware/send-response'); +const dbParamsFromResLocals = require('../../utils/database-params'); + +module.exports = class AnalysisController { + constructor ( + analysisStatusBackend, + pgConnection, + mapStore, + userLimitsApi, + layergroupAffectedTablesCache, + authApi, + surrogateKeysCache + ) { + this.analysisStatusBackend = analysisStatusBackend; + this.pgConnection = pgConnection; + this.mapStore = mapStore; + this.userLimitsApi = userLimitsApi; + this.layergroupAffectedTablesCache = layergroupAffectedTablesCache; + this.authApi = authApi; + this.surrogateKeysCache = surrogateKeysCache; + } + + register (app) { + const { base_url_mapconfig: mapConfigBasePath } = app; + + app.get( + `${mapConfigBasePath}/:token/analysis/node/:nodeId`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ANALYSIS), + cleanUpQueryParams(), + analysisNodeStatus(this.analysisStatusBackend), + sendResponse() + ); + + } +}; + +function analysisNodeStatus (analysisStatusBackend) { + return function analysisNodeStatusMiddleware(req, res, next) { + const { nodeId } = req.params; + const dbParams = dbParamsFromResLocals(res.locals); + + analysisStatusBackend.getNodeStatus(nodeId, dbParams, (err, nodeStatus, stats = {}) => { + req.profiler.add(stats); + + if (err) { + err.label = 'GET NODE STATUS'; + return next(err); + } + + res.set({ + 'Cache-Control': 'public,max-age=5', + 'Last-Modified': new Date().toUTCString() + }); + + res.body = nodeStatus; + + next(); + }); + }; +} diff --git a/lib/cartodb/controllers/layergroup/attributes.js b/lib/cartodb/controllers/layergroup/attributes.js new file mode 100644 index 00000000..07ae5da1 --- /dev/null +++ b/lib/cartodb/controllers/layergroup/attributes.js @@ -0,0 +1,93 @@ +const cors = require('../../middleware/cors'); +const user = require('../../middleware/user'); +const layergroupToken = require('../../middleware/layergroup-token'); +const cleanUpQueryParams = require('../../middleware/clean-up-query-params'); +const credentials = require('../../middleware/credentials'); +const dbConnSetup = require('../../middleware/db-conn-setup'); +const authorize = require('../../middleware/authorize'); +const rateLimit = require('../../middleware/rate-limit'); +const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const createMapStoreMapConfigProvider = require('./middlewares/map-store-map-config-provider'); +const cacheControlHeader = require('../../middleware/cache-control-header'); +const cacheChannelHeader = require('../../middleware/cache-channel-header'); +const surrogateKeyHeader = require('../../middleware/surrogate-key-header'); +const lastModifiedHeader = require('../../middleware/last-modified-header'); +const sendResponse = require('../../middleware/send-response'); + +module.exports = class AttribitesController { + constructor ( + attributesBackend, + pgConnection, + mapStore, + userLimitsApi, + layergroupAffectedTablesCache, + authApi, + surrogateKeysCache + ) { + this.attributesBackend = attributesBackend; + this.pgConnection = pgConnection; + this.mapStore = mapStore; + this.userLimitsApi = userLimitsApi; + this.layergroupAffectedTablesCache = layergroupAffectedTablesCache; + this.authApi = authApi; + this.surrogateKeysCache = surrogateKeysCache; + } + + register (app) { + const { base_url_mapconfig: mapConfigBasePath } = app; + + app.get( + `${mapConfigBasePath}/:token/:layer/attributes/:fid`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.ATTRIBUTES), + cleanUpQueryParams(), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), + getFeatureAttributes(this.attributesBackend), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + sendResponse() + ); + } +}; + +function getFeatureAttributes (attributesBackend) { + return function getFeatureAttributesMiddleware (req, res, next) { + req.profiler.start('windshaft.maplayer_attribute'); + + const { mapConfigProvider } = res.locals; + const { token } = res.locals; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + const { layer, fid } = req.params; + + const params = { + token, + dbuser, dbname, dbpassword, dbhost, dbport, + layer, fid + }; + + attributesBackend.getFeatureAttributes(mapConfigProvider, params, false, (err, tile, stats = {}) => { + req.profiler.add(stats); + + if (err) { + err.label = 'GET ATTRIBUTES'; + return next(err); + } + + res.body = tile; + + next(); + }); + }; +} diff --git a/lib/cartodb/controllers/layergroup/dataview.js b/lib/cartodb/controllers/layergroup/dataview.js new file mode 100644 index 00000000..bca27f1e --- /dev/null +++ b/lib/cartodb/controllers/layergroup/dataview.js @@ -0,0 +1,199 @@ +const cors = require('../../middleware/cors'); +const user = require('../../middleware/user'); +const layergroupToken = require('../../middleware/layergroup-token'); +const cleanUpQueryParams = require('../../middleware/clean-up-query-params'); +const credentials = require('../../middleware/credentials'); +const dbConnSetup = require('../../middleware/db-conn-setup'); +const authorize = require('../../middleware/authorize'); +const rateLimit = require('../../middleware/rate-limit'); +const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const createMapStoreMapConfigProvider = require('./middlewares/map-store-map-config-provider'); +const cacheControlHeader = require('../../middleware/cache-control-header'); +const cacheChannelHeader = require('../../middleware/cache-channel-header'); +const surrogateKeyHeader = require('../../middleware/surrogate-key-header'); +const lastModifiedHeader = require('../../middleware/last-modified-header'); +const sendResponse = require('../../middleware/send-response'); + +const ALLOWED_DATAVIEW_QUERY_PARAMS = [ + 'filters', // json + 'own_filter', // 0, 1 + 'no_filters', // 0, 1 + 'bbox', // w,s,e,n + 'start', // number + 'end', // number + 'column_type', // string + 'bins', // number + 'aggregation', //string + 'offset', // number + 'q', // widgets search + 'categories', // number +]; + +module.exports = class DataviewController { + constructor ( + dataviewBackend, + pgConnection, + mapStore, + userLimitsApi, + layergroupAffectedTablesCache, + authApi, + surrogateKeysCache + ) { + this.dataviewBackend = dataviewBackend; + this.pgConnection = pgConnection; + this.mapStore = mapStore; + this.userLimitsApi = userLimitsApi; + this.layergroupAffectedTablesCache = layergroupAffectedTablesCache; + this.authApi = authApi; + this.surrogateKeysCache = surrogateKeysCache; + } + + register (app) { + const { base_url_mapconfig: mapConfigBasePath } = app; + + // Undocumented/non-supported API endpoint methods. + // Use at your own peril. + + app.get( + `${mapConfigBasePath}/:token/dataview/:dataviewName`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), + getDataview(this.dataviewBackend), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + sendResponse() + ); + + app.get( + `${mapConfigBasePath}/:token/:layer/widget/:dataviewName`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), + getDataview(this.dataviewBackend), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + sendResponse() + ); + + app.get( + `${mapConfigBasePath}/:token/dataview/:dataviewName/search`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), + dataviewSearch(this.dataviewBackend), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + sendResponse() + ); + + app.get( + `${mapConfigBasePath}/:token/:layer/widget/:dataviewName/search`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.DATAVIEW_SEARCH), + cleanUpQueryParams(ALLOWED_DATAVIEW_QUERY_PARAMS), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), + dataviewSearch(this.dataviewBackend), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + sendResponse() + ); + } +}; + +function getDataview (dataviewBackend) { + return function getDataviewMiddleware (req, res, next) { + const { user, mapConfigProvider } = res.locals; + const { dataviewName } = req.params; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + + const params = Object.assign({ dataviewName, dbuser, dbname, dbpassword, dbhost, dbport }, req.query); + + dataviewBackend.getDataview(mapConfigProvider, user, params, (err, dataview, stats = {}) => { + req.profiler.add(stats); + + if (err) { + err.label = 'GET DATAVIEW'; + return next(err); + } + + res.body = dataview; + + next(); + }); + }; +} + +function dataviewSearch (dataviewBackend) { + return function dataviewSearchMiddleware (req, res, next) { + const { user, mapConfigProvider } = res.locals; + const { dataviewName } = req.params; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + + const params = Object.assign({ dbuser, dbname, dbpassword, dbhost, dbport }, req.query); + + dataviewBackend.search(mapConfigProvider, user, dataviewName, params, (err, searchResult, stats = {}) => { + req.profiler.add(stats); + + if (err) { + err.label = 'GET DATAVIEW SEARCH'; + return next(err); + } + + res.body = searchResult; + + next(); + }); + }; +} diff --git a/lib/cartodb/controllers/layergroup/index.js b/lib/cartodb/controllers/layergroup/index.js new file mode 100644 index 00000000..e4c1770c --- /dev/null +++ b/lib/cartodb/controllers/layergroup/index.js @@ -0,0 +1,114 @@ +const DataviewBackend = require('../../backends/dataview'); +const AnalysisStatusBackend = require('../../backends/analysis-status'); + +const TileController = require('./tile'); +const AttributesController = require('./attributes'); +const StaticController = require('./static'); +const DataviewController = require('./dataview'); +const AnalysisController = require('./analysis'); + +/** + * @param {prepareContext} prepareContext + * @param {PgConnection} pgConnection + * @param {MapStore} mapStore + * @param {TileBackend} tileBackend + * @param {PreviewBackend} previewBackend + * @param {AttributesBackend} attributesBackend + * @param {SurrogateKeysCache} surrogateKeysCache + * @param {UserLimitsApi} userLimitsApi + * @param {LayergroupAffectedTables} layergroupAffectedTables + * @param {AnalysisBackend} analysisBackend + * @constructor + */ +function LayergroupController( + pgConnection, + mapStore, + tileBackend, + previewBackend, + attributesBackend, + surrogateKeysCache, + userLimitsApi, + layergroupAffectedTablesCache, + analysisBackend, + authApi +) { + this.pgConnection = pgConnection; + this.mapStore = mapStore; + this.tileBackend = tileBackend; + this.previewBackend = previewBackend; + this.attributesBackend = attributesBackend; + this.surrogateKeysCache = surrogateKeysCache; + this.userLimitsApi = userLimitsApi; + this.layergroupAffectedTablesCache = layergroupAffectedTablesCache; + + this.dataviewBackend = new DataviewBackend(analysisBackend); + this.analysisStatusBackend = new AnalysisStatusBackend(); + this.authApi = authApi; +} + +module.exports = LayergroupController; + +LayergroupController.prototype.register = function(app) { + + const tileController = new TileController( + this.tileBackend, + this.pgConnection, + this.mapStore, + this.userLimitsApi, + this.layergroupAffectedTablesCache, + this.authApi, + this.surrogateKeysCache + ); + + tileController.register(app); + + const attributesController = new AttributesController( + this.attributesBackend, + this.pgConnection, + this.mapStore, + this.userLimitsApi, + this.layergroupAffectedTablesCache, + this.authApi, + this.surrogateKeysCache + ); + + attributesController.register(app); + + const staticController = new StaticController( + this.previewBackend, + this.pgConnection, + this.mapStore, + this.userLimitsApi, + this.layergroupAffectedTablesCache, + this.authApi, + this.surrogateKeysCache + ); + + staticController.register(app); + + const dataviewController = new DataviewController( + this.dataviewBackend, + this.pgConnection, + this.mapStore, + this.userLimitsApi, + this.layergroupAffectedTablesCache, + this.authApi, + this.surrogateKeysCache + ); + + dataviewController.register(app); + + const analysisController = new AnalysisController( + this.analysisStatusBackend, + this.pgConnection, + this.mapStore, + this.userLimitsApi, + this.layergroupAffectedTablesCache, + this.authApi, + this.surrogateKeysCache + ); + + analysisController.register(app); + + +}; diff --git a/lib/cartodb/controllers/layergroup/middlewares/map-store-map-config-provider.js b/lib/cartodb/controllers/layergroup/middlewares/map-store-map-config-provider.js new file mode 100644 index 00000000..7d64cb01 --- /dev/null +++ b/lib/cartodb/controllers/layergroup/middlewares/map-store-map-config-provider.js @@ -0,0 +1,37 @@ +const MapStoreMapConfigProvider = require('../../../models/mapconfig/provider/map-store-provider'); + +module.exports = function createMapStoreMapConfigProvider ( + mapStore, + userLimitsApi, + pgConnection, + affectedTablesCache, + forcedFormat = null +) { + return function createMapStoreMapConfigProviderMiddleware (req, res, next) { + const { user, token, cache_buster, api_key } = res.locals; + const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; + const { layer, z, x, y, scale_factor, format } = req.params; + + const params = { + user, token, cache_buster, api_key, + dbuser, dbname, dbpassword, dbhost, dbport, + layer, z, x, y, scale_factor, format + }; + + if (forcedFormat) { + params.format = forcedFormat; + params.layer = params.layer || 'all'; + } + + res.locals.mapConfigProvider = new MapStoreMapConfigProvider( + mapStore, + user, + userLimitsApi, + pgConnection, + affectedTablesCache, + params + ); + + next(); + }; +}; diff --git a/lib/cartodb/controllers/layergroup/static.js b/lib/cartodb/controllers/layergroup/static.js new file mode 100644 index 00000000..00510780 --- /dev/null +++ b/lib/cartodb/controllers/layergroup/static.js @@ -0,0 +1,161 @@ +const cors = require('../../middleware/cors'); +const user = require('../../middleware/user'); +const layergroupToken = require('../../middleware/layergroup-token'); +const cleanUpQueryParams = require('../../middleware/clean-up-query-params'); +const credentials = require('../../middleware/credentials'); +const dbConnSetup = require('../../middleware/db-conn-setup'); +const authorize = require('../../middleware/authorize'); +const rateLimit = require('../../middleware/rate-limit'); +const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const createMapStoreMapConfigProvider = require('./middlewares/map-store-map-config-provider'); +const cacheControlHeader = require('../../middleware/cache-control-header'); +const cacheChannelHeader = require('../../middleware/cache-channel-header'); +const surrogateKeyHeader = require('../../middleware/surrogate-key-header'); +const lastModifiedHeader = require('../../middleware/last-modified-header'); +const sendResponse = require('../../middleware/send-response'); + +module.exports = class StaticController { + constructor ( + previewBackend, + pgConnection, + mapStore, + userLimitsApi, + layergroupAffectedTablesCache, + authApi, + surrogateKeysCache + ) { + this.previewBackend = previewBackend; + this.pgConnection = pgConnection; + this.mapStore = mapStore; + this.userLimitsApi = userLimitsApi; + this.layergroupAffectedTablesCache = layergroupAffectedTablesCache; + this.authApi = authApi; + this.surrogateKeysCache = surrogateKeysCache; + } + + register (app) { + const { base_url_mapconfig: mapConfigBasePath } = app; + + const forcedFormat = 'png'; + + app.get( + `${mapConfigBasePath}/static/center/:token/:z/:lat/:lng/:width/:height.:format`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), + cleanUpQueryParams(['layer']), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache, + forcedFormat + ), + getPreviewImageByCenter(this.previewBackend), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + sendResponse() + ); + + app.get( + `${mapConfigBasePath}/static/bbox/:token/:west,:south,:east,:north/:width/:height.:format`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.STATIC), + cleanUpQueryParams(['layer']), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache, + forcedFormat + ), + getPreviewImageByBoundingBox(this.previewBackend), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + sendResponse() + ); + } +}; + +function getPreviewImageByCenter (previewBackend) { + return function getPreviewImageByCenterMiddleware (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 + }; + + const format = req.params.format === 'jpg' ? 'jpeg' : 'png'; + const { mapConfigProvider: provider } = res.locals; + + previewBackend.getImage(provider, format, width, height, zoom, center, (err, image, headers, stats = {}) => { + req.profiler.done(`render-${format}`); + req.profiler.add(stats); + + if (err) { + err.label = 'STATIC_MAP'; + return next(err); + } + + if (headers) { + res.set(headers); + } + + res.set('Content-Type', headers['Content-Type'] || `image/${format}`); + + res.body = image; + + next(); + }); + }; +} + +function getPreviewImageByBoundingBox (previewBackend) { + return function getPreviewImageByBoundingBoxMiddleware (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'; + const { mapConfigProvider: provider } = res.locals; + + previewBackend.getImage(provider, 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); + } + + if (headers) { + res.set(headers); + } + + res.set('Content-Type', headers['Content-Type'] || `image/${format}`); + + res.body = image; + + next(); + }); + }; +} diff --git a/lib/cartodb/controllers/layergroup/tile.js b/lib/cartodb/controllers/layergroup/tile.js new file mode 100644 index 00000000..07727236 --- /dev/null +++ b/lib/cartodb/controllers/layergroup/tile.js @@ -0,0 +1,230 @@ +const cors = require('../../middleware/cors'); +const user = require('../../middleware/user'); +const layergroupToken = require('../../middleware/layergroup-token'); +const cleanUpQueryParams = require('../../middleware/clean-up-query-params'); +const credentials = require('../../middleware/credentials'); +const dbConnSetup = require('../../middleware/db-conn-setup'); +const authorize = require('../../middleware/authorize'); +const rateLimit = require('../../middleware/rate-limit'); +const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const createMapStoreMapConfigProvider = require('./middlewares/map-store-map-config-provider'); +const cacheControlHeader = require('../../middleware/cache-control-header'); +const cacheChannelHeader = require('../../middleware/cache-channel-header'); +const surrogateKeyHeader = require('../../middleware/surrogate-key-header'); +const lastModifiedHeader = require('../../middleware/last-modified-header'); +const sendResponse = require('../../middleware/send-response'); +const vectorError = require('../../middleware/vector-error'); + +const SUPPORTED_FORMATS = { + grid_json: true, + json_torque: true, + torque_json: true, + png: true, + png32: true, + mvt: true +}; + +module.exports = class TileController { + constructor ( + tileBackend, + pgConnection, + mapStore, + userLimitsApi, + layergroupAffectedTablesCache, + authApi, + surrogateKeysCache + ) { + this.tileBackend = tileBackend; + this.pgConnection = pgConnection; + this.mapStore = mapStore; + this.userLimitsApi = userLimitsApi; + this.layergroupAffectedTablesCache = layergroupAffectedTablesCache; + this.authApi = authApi; + this.surrogateKeysCache = surrogateKeysCache; + } + + register (app) { + const { base_url_mapconfig: mapConfigBasePath } = app; + + app.get( + `${mapConfigBasePath}/:token/:z/:x/:y@:scale_factor?x.:format`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), + cleanUpQueryParams(), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), + getTile(this.tileBackend, 'map_tile'), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + incrementSuccessMetrics(global.statsClient), + incrementErrorMetrics(global.statsClient), + tileError(), + vectorError(), + sendResponse() + ); + + app.get( + `${mapConfigBasePath}/:token/:z/:x/:y.:format`, + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), + cleanUpQueryParams(), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), + getTile(this.tileBackend, 'map_tile'), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + incrementSuccessMetrics(global.statsClient), + incrementErrorMetrics(global.statsClient), + tileError(), + vectorError(), + sendResponse() + ); + + app.get( + `${mapConfigBasePath}/:token/:layer/:z/:x/:y.(:format)`, + distinguishLayergroupFromStaticRoute(), + cors(), + user(), + layergroupToken(), + credentials(), + authorize(this.authApi), + dbConnSetup(this.pgConnection), + rateLimit(this.userLimitsApi, RATE_LIMIT_ENDPOINTS_GROUPS.TILE), + cleanUpQueryParams(), + createMapStoreMapConfigProvider( + this.mapStore, + this.userLimitsApi, + this.pgConnection, + this.layergroupAffectedTablesCache + ), + getTile(this.tileBackend, 'maplayer_tile'), + cacheControlHeader(), + cacheChannelHeader(), + surrogateKeyHeader({ surrogateKeysCache: this.surrogateKeysCache }), + lastModifiedHeader(), + incrementSuccessMetrics(global.statsClient), + incrementErrorMetrics(global.statsClient), + tileError(), + vectorError(), + sendResponse() + ); + } +}; + +function distinguishLayergroupFromStaticRoute () { + return function distinguishLayergroupFromStaticRouteMiddleware(req, res, next) { + if (req.params.token === 'static') { + return next('route'); + } + + next(); + }; +} + +function parseFormat (format = '') { + const prettyFormat = format.replace('.', '_'); + return SUPPORTED_FORMATS[prettyFormat] ? prettyFormat : 'invalid'; +} + +function getStatusCode(tile, format){ + return tile.length === 0 && format === 'mvt' ? 204 : 200; +} + +function getTile (tileBackend, profileLabel = 'tile') { + return function getTileMiddleware (req, res, next) { + req.profiler.start(`windshaft.${profileLabel}`); + + const { mapConfigProvider } = res.locals; + const { token } = res.locals; + const { layer, z, x, y, format } = req.params; + + const params = { token, layer, z, x, y, format }; + + 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 incrementSuccessMetrics (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(); + }; +} + +function incrementErrorMetrics (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); + }; +} + +function tileError () { + return function tileErrorMiddleware (err, req, res, next) { + if (err.message === 'Tile does not exist' && req.params.format === 'mvt') { + res.statusCode = 204; + return 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 a107ee67fac0a8ae9e30044cdc1957a1ddcff445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 27 Mar 2018 10:32:22 +0200 Subject: [PATCH 47/62] Use arrow function --- .../models/mapconfig/provider/create-layergroup-provider.js | 2 +- lib/cartodb/models/mapconfig/provider/map-store-provider.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js index 36298eb1..b3b3f191 100644 --- a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js +++ b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js @@ -74,7 +74,7 @@ CreateLayergroupMapConfigProvider.prototype.getAffectedTables = function (callba const queries = []; - this.mapConfig.getLayers().forEach(function(layer) { + this.mapConfig.getLayers().forEach(layer => { queries.push(layer.options.sql); if (layer.options.affected_tables) { layer.options.affected_tables.map(table => { diff --git a/lib/cartodb/models/mapconfig/provider/map-store-provider.js b/lib/cartodb/models/mapconfig/provider/map-store-provider.js index ccf1472e..f89a79ce 100644 --- a/lib/cartodb/models/mapconfig/provider/map-store-provider.js +++ b/lib/cartodb/models/mapconfig/provider/map-store-provider.js @@ -106,7 +106,7 @@ MapStoreMapConfigProvider.prototype.getAffectedTables = function(callback) { const queries = []; - mapConfig.getLayers().forEach(function(layer) { + mapConfig.getLayers().forEach(layer => { queries.push(layer.options.sql); if (layer.options.affected_tables) { layer.options.affected_tables.map(table => { From 22fdc3d1bf12751ef2724b8f038c99255c59477b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 28 Mar 2018 15:53:34 +0200 Subject: [PATCH 48/62] Add query params when instantiating template --- lib/cartodb/controllers/map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 11074234..9fd67710 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -242,7 +242,7 @@ function getTemplate ( const { template_id } = req.params; const { auth_token } = req.query; - const params = { dbuser, dbname, dbpassword, dbhost, dbport }; + const params = Object.assign({ dbuser, dbname, dbpassword, dbhost, dbport }, req.query); const mapConfigProvider = new NamedMapMapConfigProvider( templateMaps, From c1da1a8a169d574e248809d095e67cb5955d7573 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 4 Apr 2018 11:05:03 +0200 Subject: [PATCH 49/62] Use unique cartodb_id in aggregated results See #889 FOr centroid and point-grid the cartodb_id wasn't unique across tiles. --- lib/cartodb/models/aggregation/aggregation-query.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index 4c89c3e4..aa417ae0 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -290,7 +290,7 @@ const aggregationQueryTemplates = { !bbox! AS bbox ) SELECT - row_number() over() AS cartodb_id, + MIN(_cdb_query.cartodb_id) AS cartodb_id, ST_SetSRID( ST_MakePoint( AVG(ST_X(_cdb_query.the_geom_webmercator)), @@ -317,6 +317,7 @@ const aggregationQueryTemplates = { ), _cdb_clusters AS ( SELECT + MIN(_cdb_query.cartodb_id) AS cartodb_id, Floor(ST_X(_cdb_query.the_geom_webmercator)/_cdb_params.res)::int AS _cdb_gx, Floor(ST_Y(_cdb_query.the_geom_webmercator)/_cdb_params.res)::int AS _cdb_gy ${dimensionDefs(ctx)} @@ -327,7 +328,7 @@ const aggregationQueryTemplates = { ${havingClause(ctx)} ) SELECT - row_number() over() AS cartodb_id, + _cdb_clusters.cartodb_id AS cartodb_id, ST_SetSRID(ST_MakePoint((_cdb_gx+0.5)*res, (_cdb_gy+0.5)*res), 3857) AS the_geom_webmercator ${dimensionNames(ctx)} ${aggregateColumnNames(ctx)} From 2132960d7c372c3d2bbf0820dd85dc7ceb8b5c98 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 4 Apr 2018 15:25:08 +0200 Subject: [PATCH 50/62] Fix non-default aggregation columns The columns for non-default aggregations were the base columns not the resulting aggregated columns In particular this could cause invalid wrapped SQL code to be passed to ST_AsMVT when the Windshaft pg-mvt renderer was used. --- lib/cartodb/models/aggregation/aggregation-mapconfig.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js index 9306518c..d3666897 100644 --- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js +++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js @@ -173,16 +173,12 @@ module.exports = class AggregationMapConfig extends MapConfig { let aggregatedColumns = []; if (columns) { - aggregatedColumns = Object.keys(columns) - .map(key => columns[key].aggregated_column) - .filter(aggregatedColumn => typeof aggregatedColumn === 'string'); + aggregatedColumns = Object.keys(columns); } let dimensionsColumns = []; if (dimensions) { - dimensionsColumns = Object.keys(dimensions) - .map(key => dimensions[key]) - .filter(dimension => typeof dimension === 'string'); + dimensionsColumns = Object.keys(dimensions); } return removeDuplicates(aggregatedColumns.concat(dimensionsColumns)); From 071b6816e3afad7420c48155c60e65630ddeab66 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 4 Apr 2018 16:28:07 +0200 Subject: [PATCH 51/62] Fix docs typos in _cdb_feature_count --- docs/MapConfig-Aggregation-extension.md | 2 +- docs/aggregation.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/MapConfig-Aggregation-extension.md b/docs/MapConfig-Aggregation-extension.md index 850b179a..bf2172f2 100644 --- a/docs/MapConfig-Aggregation-extension.md +++ b/docs/MapConfig-Aggregation-extension.md @@ -29,7 +29,7 @@ The value of this attribute can be `false` to explicitly disable aggregation for // object, defines the columns of the aggregated datasets. Each property corresponds to a columns name and // should contain an object with two properties: "aggregate_function" (one of "sum", "max", "min", "avg", "mode" or "count"), // and "aggregated_column" (the name of a column of the original layer query or "*") - // A column defined as `"_cdb_features_count": {"aggregate_function": "count", aggregated_column: "*"}` + // A column defined as `"_cdb_feature_count": {"aggregate_function": "count", aggregated_column: "*"}` // is always generated in addition to the defined columns. // The column names `cartodb_id`, `the_geom`, `the_geom_webmercator` and `_cdb_feature_count` cannot be used // for aggregated columns, as they correspond to columns always present in the result. diff --git a/docs/aggregation.md b/docs/aggregation.md index c3e754e1..6920776f 100644 --- a/docs/aggregation.md +++ b/docs/aggregation.md @@ -10,7 +10,7 @@ Aggregation is available only for point geometries. During aggregation the point When no placement or columns are specified a special default aggregation is performed. -This special mode performs only spatial aggregation (using a grid defined by the requested tile and the resolution, parameter, as all the other cases), and returns a _random_ record from each group (grid cell) with all its columns and an additional `_cdb_features_count` with the number of features in the group. +This special mode performs only spatial aggregation (using a grid defined by the requested tile and the resolution, parameter, as all the other cases), and returns a _random_ record from each group (grid cell) with all its columns and an additional `_cdb_feature_count` with the number of features in the group. Regarding the randomness of the sample: currently we use the row with the minimum `cartodb_id` value in each group. @@ -18,7 +18,7 @@ The rationale behind having this special aggregation with all the original colum ### User defined aggregations -When either a explicit placement or columns are requested we no longer use the special, query; we use one determined by the placement (which will default to "centroid"), and it will have as columns only the aggregated columns specified, in addition to `_cdb_features_count`, which is always present. +When either a explicit placement or columns are requested we no longer use the special, query; we use one determined by the placement (which will default to "centroid"), and it will have as columns only the aggregated columns specified, in addition to `_cdb_feature_count`, which is always present. We might decide in the future to allow sampling column values for any of the different placement modes. @@ -220,7 +220,7 @@ In addition, the filters applied to different columns are logically combined wit } ``` -Note that the filtered columns have to be defined with the `columns` parameter, except for `_cdb_features_count`, which is always implicitly defined and can be filtered too. +Note that the filtered columns have to be defined with the `columns` parameter, except for `_cdb_feature_count`, which is always implicitly defined and can be filtered too. #### Example From dc706aeb43c7e88aab1858a5688edec165414499 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 4 Apr 2018 16:29:40 +0200 Subject: [PATCH 52/62] Fix bug with dimension aliases The point-sample aggregation query failed if dimensions had alias different from the base columns --- .../models/aggregation/aggregation-query.js | 2 +- test/acceptance/aggregation.js | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index aa417ae0..cfb9855a 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -358,7 +358,7 @@ const aggregationQueryTemplates = { SELECT _cdb_clusters.cartodb_id, the_geom, the_geom_webmercator - ${dimensionNames(ctx, '_cdb_query')} + ${dimensionNames(ctx, '_cdb_clusters')} ${aggregateColumnNames(ctx, '_cdb_clusters')} FROM _cdb_clusters INNER JOIN (${ctx.sourceQuery}) _cdb_query diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index db03ad8e..84f4a0fd 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -689,6 +689,45 @@ describe('aggregation', function () { }); }); + ['centroid', 'point-sample', 'point-grid'].forEach(placement => { + it(`dimensions with alias should work for ${placement} placement`, function(done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + placement: placement , + threshold: 1, + dimensions: { + value2: "value" + } + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + const options = { + format: 'mvt' + }; + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach( + feature => assert.equal(typeof feature.properties.value2, 'number') + ); + + done(); + }); + }); + }); + + it(`dimensions should trigger non-default aggregation`, function(done) { this.mapConfig = createVectorMapConfig([ { From 3d36802686aa9d618a92dade88f939b13fa2e01f Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 4 Apr 2018 16:58:11 +0200 Subject: [PATCH 53/62] Tests for columns present in aggregation MVTs --- test/acceptance/aggregation.js | 97 ++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index 84f4a0fd..a4cd906d 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -357,6 +357,103 @@ describe('aggregation', function () { }); }); + ['centroid', 'point-sample', 'point-grid'].forEach(placement => { + it('should provide all the requested columns in non-default aggregation ', + function (done) { + const response = { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_2, + aggregation: { + placement: placement, + columns: { + 'first_column': { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + dimensions: { + second_column: 'sqrt_value' + }, + threshold: 1 + }, + cartocss: '#layer { marker-width: [first_column]; line-width: [second_column]; }', + cartocss_version: '2.3.0' + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + this.testClient.getLayergroup({ response }, (err, body) => { + if (err) { + return done(err); + } + + + assert.equal(typeof body.metadata, 'object'); + assert.ok(Array.isArray(body.metadata.layers)); + + body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.mvt)); + body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.png)); + done(); + }); + }); + + it('should provide only the requested columns in non-default aggregation ', + function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_2, + aggregation: { + placement: placement, + columns: { + 'first_column': { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + dimensions: { + second_column: 'sqrt_value' + }, + threshold: 1 + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + + this.testClient.getTile(0, 0, 0, { format: 'mvt' }, function (err, res, mvt) { + if (err) { + return done(err); + } + + const geojsonTile = JSON.parse(mvt.toGeoJSONSync(0)); + let columns = new Set(); + geojsonTile.features.forEach(f => { + Object.keys(f.properties).forEach(p => columns.add(p)); + }); + columns = Array.from(columns); + const expected_columns = [ + '_cdb_feature_count', 'cartodb_id', 'first_column', 'second_column' + ]; + assert.deepEqual(columns.sort(), expected_columns.sort()); + + done(); + }); + }); + }); + it('should skip aggregation to create a layergroup with aggregation defined already', function (done) { const mapConfig = createVectorMapConfig([ { From e8cd6856b52643c145a89e19adcce0dc23d8a96f Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 4 Apr 2018 17:18:56 +0200 Subject: [PATCH 54/62] Add missing aggregation columns to ST_AsMVT Aggregation results always should have the cartodb_id and the feature count --- lib/cartodb/models/aggregation/aggregation-mapconfig.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js index d3666897..548f95d3 100644 --- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js +++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js @@ -171,6 +171,8 @@ module.exports = class AggregationMapConfig extends MapConfig { _getLayerAggregationRequiredColumns (index) { const { columns, dimensions } = this.getAggregation(index); + let finalColumns = ['cartodb_id', '_cdb_feature_count']; + let aggregatedColumns = []; if (columns) { aggregatedColumns = Object.keys(columns); @@ -181,7 +183,7 @@ module.exports = class AggregationMapConfig extends MapConfig { dimensionsColumns = Object.keys(dimensions); } - return removeDuplicates(aggregatedColumns.concat(dimensionsColumns)); + return removeDuplicates(finalColumns.concat(aggregatedColumns).concat(dimensionsColumns)); } doesLayerReachThreshold(index, featureCount) { From 98cb0878d96c3862e1deee111952cf47bd10b0f5 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 5 Apr 2018 12:12:00 +0200 Subject: [PATCH 55/62] Update NEWS Reflect the changes in #913 --- NEWS.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 588166b9..75756417 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,8 +1,15 @@ # Changelog -## 6.0.1 +## 6.1.0 Released 2018-mm-dd +New features: +- Aggreation filters + +Bug Fixes: +- Non-default aggregation selected the wrong columns (e.g. for vector tiles) +- Aggregation dimensions with alias where broken + ## 6.0.0 Released 2018-03-19 Backward incompatible changes: From 44424583f0c222a876dde7cf00070939bcad7a6e Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 5 Apr 2018 12:12:58 +0200 Subject: [PATCH 56/62] Revert "Use unique cartodb_id in aggregated results" This reverts commit c1da1a8a169d574e248809d095e67cb5955d7573. This is reverted for moving the change out of PR #913 into its own PR for clarity. --- lib/cartodb/models/aggregation/aggregation-query.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index cfb9855a..02211a7d 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -290,7 +290,7 @@ const aggregationQueryTemplates = { !bbox! AS bbox ) SELECT - MIN(_cdb_query.cartodb_id) AS cartodb_id, + row_number() over() AS cartodb_id, ST_SetSRID( ST_MakePoint( AVG(ST_X(_cdb_query.the_geom_webmercator)), @@ -317,7 +317,6 @@ const aggregationQueryTemplates = { ), _cdb_clusters AS ( SELECT - MIN(_cdb_query.cartodb_id) AS cartodb_id, Floor(ST_X(_cdb_query.the_geom_webmercator)/_cdb_params.res)::int AS _cdb_gx, Floor(ST_Y(_cdb_query.the_geom_webmercator)/_cdb_params.res)::int AS _cdb_gy ${dimensionDefs(ctx)} @@ -328,7 +327,7 @@ const aggregationQueryTemplates = { ${havingClause(ctx)} ) SELECT - _cdb_clusters.cartodb_id AS cartodb_id, + row_number() over() AS cartodb_id, ST_SetSRID(ST_MakePoint((_cdb_gx+0.5)*res, (_cdb_gy+0.5)*res), 3857) AS the_geom_webmercator ${dimensionNames(ctx)} ${aggregateColumnNames(ctx)} From f7fad736c355d89ffa299cc3f596c1718abb140e Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 5 Apr 2018 16:01:29 +0200 Subject: [PATCH 57/62] Add test for uniqueness of aggregated cartodb_id --- test/acceptance/aggregation.js | 59 ++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index db03ad8e..e17db2a5 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -1979,6 +1979,65 @@ describe('aggregation', function () { }); }); + + ['default', 'centroid', 'point-sample', 'point-grid'].forEach(placement => { + it(`aggregated ids are unique for ${placement} aggregation`, function (done) { + this.mapConfig = { + version: '1.6.0', + buffersize: { 'mvt': 0 }, + layers: [ + { + type: 'cartodb', + + options: { + sql: POINTS_SQL_1, + resolution: 1, + aggregation: { + threshold: 1 + } + } + } + ] + }; + if (placement !== 'default') { + this.mapConfig.layers[0].options.aggregation.placement = placement; + } + + this.testClient = new TestClient(this.mapConfig); + + this.testClient.getTile(1, 0, 1, { format: 'mvt' }, (err, res, mvt) => { + if (err) { + return done(err); + } + + const tile1 = JSON.parse(mvt.toGeoJSONSync(0)); + + assert.ok(Array.isArray(tile1.features)); + assert.ok(tile1.features.length > 0); + + this.testClient.getTile(1, 1, 0, { format: 'mvt' }, (err, res, mvt) => { + if (err) { + return done(err); + } + + const tile2 = JSON.parse(mvt.toGeoJSONSync(0)); + + assert.ok(Array.isArray(tile2.features)); + assert.ok(tile2.features.length > 0); + + const tile1Ids = tile1.features.map(f => f.properties.cartodb_id); + const tile2Ids = tile2.features.map(f => f.properties.cartodb_id); + const repeatedIds = tile1Ids.filter(id => tile2Ids.includes(id)); + assert.equal(repeatedIds.length, 0); + + done(); + }); + + }); + }); + }); + + }); }); }); From ffa3a96f1a56d6cacd584bbf1bdd0b89581523cb Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 4 Apr 2018 11:05:03 +0200 Subject: [PATCH 58/62] Use unique cartodb_id in aggregated results See #889 FOr centroid and point-grid the cartodb_id wasn't unique across tiles. --- lib/cartodb/models/aggregation/aggregation-query.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index 02211a7d..cfb9855a 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -290,7 +290,7 @@ const aggregationQueryTemplates = { !bbox! AS bbox ) SELECT - row_number() over() AS cartodb_id, + MIN(_cdb_query.cartodb_id) AS cartodb_id, ST_SetSRID( ST_MakePoint( AVG(ST_X(_cdb_query.the_geom_webmercator)), @@ -317,6 +317,7 @@ const aggregationQueryTemplates = { ), _cdb_clusters AS ( SELECT + MIN(_cdb_query.cartodb_id) AS cartodb_id, Floor(ST_X(_cdb_query.the_geom_webmercator)/_cdb_params.res)::int AS _cdb_gx, Floor(ST_Y(_cdb_query.the_geom_webmercator)/_cdb_params.res)::int AS _cdb_gy ${dimensionDefs(ctx)} @@ -327,7 +328,7 @@ const aggregationQueryTemplates = { ${havingClause(ctx)} ) SELECT - row_number() over() AS cartodb_id, + _cdb_clusters.cartodb_id AS cartodb_id, ST_SetSRID(ST_MakePoint((_cdb_gx+0.5)*res, (_cdb_gy+0.5)*res), 3857) AS the_geom_webmercator ${dimensionNames(ctx)} ${aggregateColumnNames(ctx)} From 26c5ff1f9362e0bf19370452ff6898272f39eafd Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 5 Apr 2018 16:36:07 +0200 Subject: [PATCH 59/62] Update news --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 75756417..20b8f06a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,7 @@ New features: Bug Fixes: - Non-default aggregation selected the wrong columns (e.g. for vector tiles) - Aggregation dimensions with alias where broken +- cartodb_id was not unique accross aggregated vector tiles ## 6.0.0 Released 2018-03-19 From 233f9698f3e51f665eefd755e53f24b0b8ccec9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 6 Apr 2018 12:59:53 +0200 Subject: [PATCH 60/62] fix affectedtables cache --- .../provider/create-layergroup-provider.js | 25 +++++++++++++----- .../mapconfig/provider/map-store-provider.js | 26 ++++++++++++++----- .../mapconfig/provider/named-map-provider.js | 25 +++++++++++++----- 3 files changed, 57 insertions(+), 19 deletions(-) diff --git a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js index b3b3f191..1f19d395 100644 --- a/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js +++ b/lib/cartodb/models/mapconfig/provider/create-layergroup-provider.js @@ -58,7 +58,7 @@ CreateLayergroupMapConfigProvider.prototype.filter = MapStoreMapConfigProvider.p CreateLayergroupMapConfigProvider.prototype.createKey = MapStoreMapConfigProvider.prototype.createKey; -CreateLayergroupMapConfigProvider.prototype.getAffectedTables = function (callback) { +CreateLayergroupMapConfigProvider.prototype.createAffectedTables = function (callback) { this.getMapConfig((err, mapConfig) => { if (err) { return callback(err); @@ -67,11 +67,6 @@ CreateLayergroupMapConfigProvider.prototype.getAffectedTables = function (callba const { dbname } = this.params; const token = mapConfig.id(); - if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { - const affectedTables = this.affectedTablesCache.get(dbname, token); - return callback(null, affectedTables); - } - const queries = []; this.mapConfig.getLayers().forEach(layer => { @@ -106,3 +101,21 @@ CreateLayergroupMapConfigProvider.prototype.getAffectedTables = function (callba }); }); }; + +CreateLayergroupMapConfigProvider.prototype.getAffectedTables = function (callback) { + this.getMapConfig((err, mapConfig) => { + if (err) { + return callback(err); + } + + const { dbname } = this.params; + const token = mapConfig.id(); + + if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = this.affectedTablesCache.get(dbname, token); + return callback(null, affectedTables); + } + + return this.createAffectedTables(callback); + }); +}; diff --git a/lib/cartodb/models/mapconfig/provider/map-store-provider.js b/lib/cartodb/models/mapconfig/provider/map-store-provider.js index f89a79ce..d9f9da83 100644 --- a/lib/cartodb/models/mapconfig/provider/map-store-provider.js +++ b/lib/cartodb/models/mapconfig/provider/map-store-provider.js @@ -89,7 +89,7 @@ MapStoreMapConfigProvider.prototype.createKey = function(base) { return (base) ? baseKeyTpl(tplValues) : rendererKeyTpl(tplValues); }; -MapStoreMapConfigProvider.prototype.getAffectedTables = function(callback) { +MapStoreMapConfigProvider.prototype.createAffectedTables = function(callback) { this.getMapConfig((err, mapConfig) => { if (err) { return callback(err); @@ -98,12 +98,6 @@ MapStoreMapConfigProvider.prototype.getAffectedTables = function(callback) { const { dbname } = this.params; const token = mapConfig.id(); - if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { - const affectedTables = this.affectedTablesCache.get(dbname, token); - - return callback(null, affectedTables); - } - const queries = []; mapConfig.getLayers().forEach(layer => { @@ -138,3 +132,21 @@ MapStoreMapConfigProvider.prototype.getAffectedTables = function(callback) { }); }); }; + +MapStoreMapConfigProvider.prototype.getAffectedTables = function (callback) { + this.getMapConfig((err, mapConfig) => { + if (err) { + return callback(err); + } + + const { dbname } = this.params; + const token = mapConfig.id(); + + if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = this.affectedTablesCache.get(dbname, token); + return callback(null, affectedTables); + } + + return this.createAffectedTables(callback); + }); +}; diff --git a/lib/cartodb/models/mapconfig/provider/named-map-provider.js b/lib/cartodb/models/mapconfig/provider/named-map-provider.js index 50612065..065d20f4 100644 --- a/lib/cartodb/models/mapconfig/provider/named-map-provider.js +++ b/lib/cartodb/models/mapconfig/provider/named-map-provider.js @@ -262,7 +262,7 @@ NamedMapMapConfigProvider.prototype.getTemplateName = function() { return this.templateName; }; -NamedMapMapConfigProvider.prototype.getAffectedTables = function(callback) { +NamedMapMapConfigProvider.prototype.createAffectedTables = function(callback) { this.getMapConfig((err, mapConfig) => { if (err) { return callback(err); @@ -271,11 +271,6 @@ NamedMapMapConfigProvider.prototype.getAffectedTables = function(callback) { const { dbname } = this.rendererParams; const token = mapConfig.id(); - if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { - const affectedTables = this.affectedTablesCache.get(dbname, token); - return callback(null, affectedTables); - } - const queries = []; mapConfig.getLayers().forEach(layer => { @@ -310,3 +305,21 @@ NamedMapMapConfigProvider.prototype.getAffectedTables = function(callback) { }); }); }; + +NamedMapMapConfigProvider.prototype.getAffectedTables = function (callback) { + this.getMapConfig((err, mapConfig) => { + if (err) { + return callback(err); + } + + const { dbname } = this.params; + const token = mapConfig.id(); + + if (this.affectedTablesCache.hasAffectedTables(dbname, token)) { + const affectedTables = this.affectedTablesCache.get(dbname, token); + return callback(null, affectedTables); + } + + return this.createAffectedTables(callback); + }); +}; From c94d78203748cd552b08b2908a543fb582ba150c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 6 Apr 2018 13:00:12 +0200 Subject: [PATCH 61/62] calling to new createAffectedTables --- lib/cartodb/controllers/map.js | 2 +- lib/cartodb/controllers/named_maps.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 9fd67710..004dc0c1 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -422,7 +422,7 @@ function setLastUpdatedTimeToLayergroup () { const { mapConfigProvider, analysesResults } = res.locals; const layergroup = res.body; - mapConfigProvider.getAffectedTables((err, affectedTables) => { + mapConfigProvider.createAffectedTables((err, affectedTables) => { if (err) { return next(err); } diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 2cb4f609..93e56bbe 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -248,7 +248,7 @@ function getStaticImageOptions ({ tablesExtentApi }) { res.locals.imageOpts = DEFAULT_ZOOM_CENTER; - mapConfigProvider.getAffectedTables((err, affectedTables) => { + mapConfigProvider.createAffectedTables((err, affectedTables) => { if (err) { return next(); } From 25aa967146663679053afd35fb7ba8dee4703d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 6 Apr 2018 15:26:11 +0200 Subject: [PATCH 62/62] Forbid access to named map admin resources for everyone but master --- lib/cartodb/api/auth_api.js | 2 +- lib/cartodb/controllers/named_maps_admin.js | 11 +++++- test/acceptance/auth/authorization.js | 44 ++++++++++++++++++--- test/support/test-client.js | 30 ++++++++++++++ 4 files changed, 79 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/api/auth_api.js b/lib/cartodb/api/auth_api.js index d27ea3e1..ff42c9df 100644 --- a/lib/cartodb/api/auth_api.js +++ b/lib/cartodb/api/auth_api.js @@ -109,7 +109,7 @@ AuthApi.prototype.authorizedByAPIKey = function(user, res, callback) { return callback(error); } - return callback(null, true); + return callback(null, true, apikey); }); }; diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index afcaa1e3..df3ac766 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -102,7 +102,7 @@ function authorizedByAPIKey ({ authApi, action, label }) { return function authorizedByAPIKeyMiddleware (req, res, next) { const { user } = res.locals; - authApi.authorizedByAPIKey(user, res, (err, authenticated) => { + authApi.authorizedByAPIKey(user, res, (err, authenticated, apikey) => { if (err) { return next(err); } @@ -114,6 +114,15 @@ function authorizedByAPIKey ({ authApi, action, label }) { return next(error); } + if (apikey.type !== 'master') { + const error = new Error('Forbidden'); + error.type = 'auth'; + error.subtype = 'api-key-does-not-grant-access'; + error.http_status = 403; + + return next(error); + } + next(); }); }; diff --git a/test/acceptance/auth/authorization.js b/test/acceptance/auth/authorization.js index d7df8b32..02668251 100644 --- a/test/acceptance/auth/authorization.js +++ b/test/acceptance/auth/authorization.js @@ -37,7 +37,7 @@ describe('authorization', function() { }); }); - it('should create and get a named map tile using a regular apikey token', function (done) { + it.skip('should create and get a named map tile using a regular apikey token', function (done) { const apikeyToken = 'regular1'; const mapConfig = { version: '1.7.0', @@ -61,7 +61,7 @@ describe('authorization', function() { testClient.drain(done); }); - }); + }); it('should fail getting a named map tile with default apikey token', function (done) { const apikeyTokenCreate = 'regular1'; @@ -203,7 +203,7 @@ describe('authorization', function() { assert.ok(layergroupResult.hasOwnProperty('errors')); assert.equal(layergroupResult.errors.length, 1); assert.ok(layergroupResult.errors[0].match(/permission denied/), layergroupResult.errors[0]); - + testClient.drain(done); }); }); @@ -257,7 +257,7 @@ describe('authorization', function() { type: 'source', params: { query: 'select * from populated_places_simple_reduced' - } + } } ] }; @@ -354,7 +354,39 @@ describe('authorization', function() { }); }); - it('should create and get a named map tile using a regular apikey token', function (done) { + it('should fail while listing named maps with a regular apikey token', function (done) { + const apikeyToken = 'regular1'; + + const testClient = new TestClient({}, apikeyToken); + + testClient.getNamedMapList({ response: {status: 403 }}, function (err, res, body) { + assert.ifError(err); + + assert.equal(res.statusCode, 403); + + assert.equal(body.errors.length, 1); + assert.ok(body.errors[0].match(/Forbidden/), body.errors[0]); + + testClient.drain(done); + }); + }); + + it('should list named maps with master apikey token', function (done) { + const apikeyToken = 1234; + + const testClient = new TestClient({}, apikeyToken); + + testClient.getNamedMapList({}, function (err, res, body) { + assert.ifError(err); + + assert.equal(res.statusCode, 200); + assert.ok(Array.isArray(body.template_ids)); + + testClient.drain(done); + }); + }); + + it.skip('should create and get a named map tile using a regular apikey token', function (done) { const apikeyToken = 'regular1'; const template = { @@ -391,7 +423,7 @@ describe('authorization', function() { }); }); - it('should fail creating a named map using a regular apikey token and a private table', function (done) { + it.skip('should fail creating a named map using a regular apikey token and a private table', function (done) { const apikeyToken = 'regular1'; const template = { diff --git a/test/support/test-client.js b/test/support/test-client.js index 3e919175..7b068901 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -1254,6 +1254,36 @@ TestClient.prototype.getAnalysesCatalog = function (params, callback) { ); }; +TestClient.prototype.getNamedMapList = function(params, callback) { + const request = { + url: `/api/v1/map/named?${qs.stringify({ api_key: this.apiKey })}`, + method: 'GET', + headers: { + host: 'localhost', + 'Content-Type': 'application/json' + } + }; + + let expectedResponse = { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + if (params.response) { + expectedResponse = Object.assign(expectedResponse, params.response); + } + + assert.response(this.server, request, expectedResponse, (res, err) => { + if (err) { + return callback(err); + } + const body = JSON.parse(res.body); + return callback(null, res, body); + }); +}; + TestClient.prototype.getNamedTile = function (name, z, x, y, format, options, callback) { const { params } = options;