From a957d8240499c43f4e445b0e5c14cc8e93131b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 13:28:37 +0200 Subject: [PATCH 01/45] User ES6 class syntax for MapStore --- lib/storages/mapstore.js | 249 +++++++++++++++++---------------------- 1 file changed, 109 insertions(+), 140 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index d68b405a9..5196c7a26 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -1,166 +1,135 @@ 'use strict'; -// Map configuration storage - -var RedisPool = require('redis-mpool'); -var MapConfig = require('../models/mapconfig'); - -/** - * @constructor - * @type {MapStore} - */ -function MapStore (opts) { - opts = opts || {}; - this._config = Object.assign({ - redis_host: '127.0.0.1', - redis_port: 6379, - redis_db: 0, - redis_key_mapcfg_prefix: 'map_cfg|', - expire_time: 300, // in seconds (7200 is 5 hours; 300 is 5 minutes) - pool_max: 50, - pool_idleTimeout: 10000, // in milliseconds - pool_reapInterval: 1000, // in milliseconds - pool_log: false - }, opts); - this.redis_pool = this._get('pool'); - if (!this.redis_pool) { - var redisOpts = { - host: this._get('redis_host'), - port: this._get('redis_port'), - max: this._get('pool_max'), - idleTimeoutMillis: this._get('pool_idleTimeout'), - reapIntervalMillis: this._get('pool_reapInterval'), - log: this._get('pool_log') - }; - this.redis_pool = new RedisPool(redisOpts); +const RedisPool = require('redis-mpool'); +const MapConfig = require('../models/mapconfig'); + +module.exports = class MapStore { + constructor (opts) { + opts = opts || {}; + this._config = Object.assign({ + redis_host: '127.0.0.1', + redis_port: 6379, + redis_db: 0, + redis_key_mapcfg_prefix: 'map_cfg|', + expire_time: 300, // in seconds (7200 is 5 hours; 300 is 5 minutes) + pool_max: 50, + pool_idleTimeout: 10000, // in milliseconds + pool_reapInterval: 1000, // in milliseconds + pool_log: false + }, opts); + this.redis_pool = this._get('pool'); + if (!this.redis_pool) { + var redisOpts = { + host: this._get('redis_host'), + port: this._get('redis_port'), + max: this._get('pool_max'), + idleTimeoutMillis: this._get('pool_idleTimeout'), + reapIntervalMillis: this._get('pool_reapInterval'), + log: this._get('pool_log') + }; + this.redis_pool = new RedisPool(redisOpts); + } + this.logger = opts.logger; } - this.logger = opts.logger; -} - -var o = MapStore.prototype; -/// Internal method: get configuration item -o._get = function (key) { - return this._config[key]; -}; - -/// Internal method: get redis pool -o._redisPool = function () { - return this.redis_pool; -}; + _get (key) { + return this._config[key]; + } -/// Internal method: run redis command -// -/// @param func - the redis function to execute (uppercase required!) -/// @param args - the arguments for the redis function in an array -/// NOTE: the array will be modified -/// @param callback - function(err,val) function to pass results too. -/// -o._redisCmd = function (func, args, callback) { - const pool = this._redisPool(); - const db = this._get('redis_db'); - - pool.acquire(db, (err, client) => { - if (err) { - this.log(err, 'adquiring client'); - return callback(err); - } + _redisPool () { + return this.redis_pool; + } - client[func](...args, (err, data) => { - pool.release(db, client); + _redisCmd (func, args, callback) { + const pool = this._redisPool(); + const db = this._get('redis_db'); + pool.acquire(db, (err, client) => { if (err) { - this.log(err, func, args); + this.log(err, 'adquiring client'); + return callback(err); } - if (callback) { + client[func](...args, (err, data) => { + pool.release(db, client); + if (err) { - return callback(err); + this.log(err, func, args); } - return callback(null, data); - } + if (callback) { + if (err) { + return callback(err); + } + + return callback(null, data); + } + }); }); - }); -}; + } -/// API: Load a saved MapStore object, renewing expiration time -// -/// Static method -/// -/// @param id the MapStore identifier -/// @param callback function(err, mapConfig) callback function -/// -o.load = function (id, callback) { - const key = this._get('redis_key_mapcfg_prefix') + id; - const exp = this._get('expire_time'); - - this._redisCmd('GET', [key], (err, json) => { - if (err) { - return callback(err); - } + // API: Load a saved MapStore object, renewing expiration time + load (id, callback) { + const key = this._get('redis_key_mapcfg_prefix') + id; + const exp = this._get('expire_time'); - if (!json) { - const mapConfigError = new Error(`Invalid or nonexistent map configuration token '${id}'`); - return callback(mapConfigError); - } + this._redisCmd('GET', [key], (err, json) => { + if (err) { + return callback(err); + } - let mapConfig; - try { - const serializedMapConfig = JSON.parse(json); - mapConfig = MapConfig.create(serializedMapConfig); - } catch (error) { - return callback(error); - } + if (!json) { + const mapConfigError = new Error(`Invalid or nonexistent map configuration token '${id}'`); + return callback(mapConfigError); + } - // Postpone expiration for the key - // not waiting for response - this._redisCmd('EXPIRE', [key, exp]); + let mapConfig; + try { + const serializedMapConfig = JSON.parse(json); + mapConfig = MapConfig.create(serializedMapConfig); + } catch (error) { + return callback(error); + } - return callback(null, mapConfig); - }); -}; + // Postpone expiration for the key + // not waiting for response + this._redisCmd('EXPIRE', [key, exp]); -/// API: store map to redis -// -/// @param map MapConfig to store -/// @param callback function(err, id, known) called when save is completed -/// -o.save = function (map, callback) { - const id = map.id(); - const key = this._get('redis_key_mapcfg_prefix') + id; - const exp = this._get('expire_time'); - - this._redisCmd('SETNX', [key, map.serialize()], (err, wasNew) => { - if (err) { - return callback(err); - } + return callback(null, mapConfig); + }); + } - this._redisCmd('EXPIRE', [key, exp]); // not waiting for response + // API: store map to redis + save (map, callback) { + const id = map.id(); + const key = this._get('redis_key_mapcfg_prefix') + id; + const exp = this._get('expire_time'); - return callback(null, id, !wasNew); - }); -}; + this._redisCmd('SETNX', [key, map.serialize()], (err, wasNew) => { + if (err) { + return callback(err); + } -/// API: delete map from store -// -/// @param map MapConfig to delete from store -/// @param callback function(err, id, known) called when save is completed -/// -o.del = function (id, callback) { - const key = this._get('redis_key_mapcfg_prefix') + id; - this._redisCmd('DEL', [key], callback); -}; + this._redisCmd('EXPIRE', [key, exp]); // not waiting for response -o.log = function (err, command, args = []) { - if (this.logger) { - const log = { - error: err.message, - command, - args - }; - this.logger.error(JSON.stringify(log)); + return callback(null, id, !wasNew); + }); } -}; -module.exports = MapStore; + // API: delete map from store + del (id, callback) { + const key = this._get('redis_key_mapcfg_prefix') + id; + this._redisCmd('DEL', [key], callback); + } + + log (err, command, args = []) { + if (this.logger) { + const log = { + error: err.message, + command, + args + }; + this.logger.error(JSON.stringify(log)); + } + } +}; From 3b3b14a11169d940c7dc858bda0f4fe47e09c003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 13:29:34 +0200 Subject: [PATCH 02/45] Use default arguments value --- lib/storages/mapstore.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index 5196c7a26..8ea315ba6 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -4,8 +4,7 @@ const RedisPool = require('redis-mpool'); const MapConfig = require('../models/mapconfig'); module.exports = class MapStore { - constructor (opts) { - opts = opts || {}; + constructor (opts = {}) { this._config = Object.assign({ redis_host: '127.0.0.1', redis_port: 6379, @@ -17,7 +16,9 @@ module.exports = class MapStore { pool_reapInterval: 1000, // in milliseconds pool_log: false }, opts); + this.redis_pool = this._get('pool'); + if (!this.redis_pool) { var redisOpts = { host: this._get('redis_host'), From 4e663eacbec13654dbdee585eeda76c8295f79a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 13:46:07 +0200 Subject: [PATCH 03/45] Rename argument --- lib/storages/mapstore.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index 8ea315ba6..dfeaca223 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -4,18 +4,18 @@ const RedisPool = require('redis-mpool'); const MapConfig = require('../models/mapconfig'); module.exports = class MapStore { - constructor (opts = {}) { + constructor (options = {}) { this._config = Object.assign({ redis_host: '127.0.0.1', redis_port: 6379, redis_db: 0, redis_key_mapcfg_prefix: 'map_cfg|', - expire_time: 300, // in seconds (7200 is 5 hours; 300 is 5 minutes) + expire_time: 300, // in seconds pool_max: 50, pool_idleTimeout: 10000, // in milliseconds pool_reapInterval: 1000, // in milliseconds pool_log: false - }, opts); + }, options); this.redis_pool = this._get('pool'); @@ -30,7 +30,8 @@ module.exports = class MapStore { }; this.redis_pool = new RedisPool(redisOpts); } - this.logger = opts.logger; + + this.logger = options.logger; } _get (key) { From 7fb0463fb6d269cbbe478dfd9d8a932b01f6c017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 13:48:35 +0200 Subject: [PATCH 04/45] Assign arguments to the instance's root --- lib/storages/mapstore.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index dfeaca223..f97543a01 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -5,7 +5,7 @@ const MapConfig = require('../models/mapconfig'); module.exports = class MapStore { constructor (options = {}) { - this._config = Object.assign({ + Object.assign(this, { redis_host: '127.0.0.1', redis_port: 6379, redis_db: 0, @@ -35,7 +35,7 @@ module.exports = class MapStore { } _get (key) { - return this._config[key]; + return this[key]; } _redisPool () { From 80c3021bf76f7920df79cb91e4d2b0b0d4fcc70f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 13:54:42 +0200 Subject: [PATCH 05/45] Siplify usage of the internal pool --- lib/storages/mapstore.js | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index f97543a01..7353f517c 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -17,18 +17,15 @@ module.exports = class MapStore { pool_log: false }, options); - this.redis_pool = this._get('pool'); - - if (!this.redis_pool) { - var redisOpts = { + if (!this.pool) { + this.pool = new RedisPool({ host: this._get('redis_host'), port: this._get('redis_port'), max: this._get('pool_max'), idleTimeoutMillis: this._get('pool_idleTimeout'), reapIntervalMillis: this._get('pool_reapInterval'), log: this._get('pool_log') - }; - this.redis_pool = new RedisPool(redisOpts); + }); } this.logger = options.logger; @@ -38,22 +35,17 @@ module.exports = class MapStore { return this[key]; } - _redisPool () { - return this.redis_pool; - } - _redisCmd (func, args, callback) { - const pool = this._redisPool(); const db = this._get('redis_db'); - pool.acquire(db, (err, client) => { + this.pool.acquire(db, (err, client) => { if (err) { this.log(err, 'adquiring client'); return callback(err); } client[func](...args, (err, data) => { - pool.release(db, client); + this.pool.release(db, client); if (err) { this.log(err, func, args); From fc636eccb20ac45c4d7bb3b0e0c2865232047556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 14:06:01 +0200 Subject: [PATCH 06/45] Better error handling and logging --- lib/storages/mapstore.js | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index 7353f517c..9135e9d77 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -49,15 +49,10 @@ module.exports = class MapStore { if (err) { this.log(err, func, args); + return callback(err); } - if (callback) { - if (err) { - return callback(err); - } - - return callback(null, data); - } + return callback(null, data); }); }); } @@ -85,11 +80,13 @@ module.exports = class MapStore { return callback(error); } - // Postpone expiration for the key - // not waiting for response - this._redisCmd('EXPIRE', [key, exp]); + this._redisCmd('EXPIRE', [key, exp], (err) => { + if (err) { + this.log(err); + } - return callback(null, mapConfig); + return callback(null, mapConfig); + }); }); } @@ -104,9 +101,13 @@ module.exports = class MapStore { return callback(err); } - this._redisCmd('EXPIRE', [key, exp]); // not waiting for response + this._redisCmd('EXPIRE', [key, exp], (err) => { + if (err) { + this.log(err); + } - return callback(null, id, !wasNew); + return callback(null, id, !wasNew); + }); }); } From ce3e1883579ba8efa89402dc9bd7afe50af49bec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 14:15:48 +0200 Subject: [PATCH 07/45] Return error when error so there is no need to log --- lib/index.js | 3 +-- lib/storages/mapstore.js | 19 ++----------------- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/lib/index.js b/lib/index.js index 7805858c0..c26474be8 100644 --- a/lib/index.js +++ b/lib/index.js @@ -14,8 +14,7 @@ const RendererFactory = require('./renderers/renderer-factory'); function windshaftFactory ({ rendererOptions, redisPool, onTileErrorStrategy, logger }) { const mapStore = new MapStore({ pool: redisPool, - expire_time: rendererOptions.grainstore.default_layergroup_ttl, - logger + expire_time: rendererOptions.grainstore.default_layergroup_ttl }); const rendererFactory = new RendererFactory({ diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index 9135e9d77..4cf9af175 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -27,8 +27,6 @@ module.exports = class MapStore { log: this._get('pool_log') }); } - - this.logger = options.logger; } _get (key) { @@ -40,7 +38,6 @@ module.exports = class MapStore { this.pool.acquire(db, (err, client) => { if (err) { - this.log(err, 'adquiring client'); return callback(err); } @@ -48,7 +45,6 @@ module.exports = class MapStore { this.pool.release(db, client); if (err) { - this.log(err, func, args); return callback(err); } @@ -82,7 +78,7 @@ module.exports = class MapStore { this._redisCmd('EXPIRE', [key, exp], (err) => { if (err) { - this.log(err); + return callback(err); } return callback(null, mapConfig); @@ -103,7 +99,7 @@ module.exports = class MapStore { this._redisCmd('EXPIRE', [key, exp], (err) => { if (err) { - this.log(err); + return callback(err); } return callback(null, id, !wasNew); @@ -116,15 +112,4 @@ module.exports = class MapStore { const key = this._get('redis_key_mapcfg_prefix') + id; this._redisCmd('DEL', [key], callback); } - - log (err, command, args = []) { - if (this.logger) { - const log = { - error: err.message, - command, - args - }; - this.logger.error(JSON.stringify(log)); - } - } }; From ae2f44b8b0700a2f32b248337da3b8e4096983a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 14:51:33 +0200 Subject: [PATCH 08/45] Remove private method --- lib/storages/mapstore.js | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index 4cf9af175..f9bfddbd9 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -19,22 +19,18 @@ module.exports = class MapStore { if (!this.pool) { this.pool = new RedisPool({ - host: this._get('redis_host'), - port: this._get('redis_port'), - max: this._get('pool_max'), - idleTimeoutMillis: this._get('pool_idleTimeout'), - reapIntervalMillis: this._get('pool_reapInterval'), - log: this._get('pool_log') + host: this.redis_host, + port: this.redis_port, + max: this.pool_max, + idleTimeoutMillis: this.pool_idleTimeout, + reapIntervalMillis: this.pool_reapInterval, + log: this.pool_log }); } } - _get (key) { - return this[key]; - } - _redisCmd (func, args, callback) { - const db = this._get('redis_db'); + const db = this.redis_db; this.pool.acquire(db, (err, client) => { if (err) { @@ -55,8 +51,8 @@ module.exports = class MapStore { // API: Load a saved MapStore object, renewing expiration time load (id, callback) { - const key = this._get('redis_key_mapcfg_prefix') + id; - const exp = this._get('expire_time'); + const key = this.redis_key_mapcfg_prefix + id; + const exp = this.expire_time; this._redisCmd('GET', [key], (err, json) => { if (err) { @@ -89,8 +85,8 @@ module.exports = class MapStore { // API: store map to redis save (map, callback) { const id = map.id(); - const key = this._get('redis_key_mapcfg_prefix') + id; - const exp = this._get('expire_time'); + const key = this.redis_key_mapcfg_prefix + id; + const exp = this.expire_time; this._redisCmd('SETNX', [key, map.serialize()], (err, wasNew) => { if (err) { @@ -109,7 +105,7 @@ module.exports = class MapStore { // API: delete map from store del (id, callback) { - const key = this._get('redis_key_mapcfg_prefix') + id; + const key = this.redis_key_mapcfg_prefix + id; this._redisCmd('DEL', [key], callback); } }; From 792c6afbc4a43feeffdcf0451d5fd9ee4d1eb4ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 15:00:10 +0200 Subject: [PATCH 09/45] Add method to compose redis keys --- lib/storages/mapstore.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index f9bfddbd9..b9f72d998 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -29,6 +29,10 @@ module.exports = class MapStore { } } + _key (id) { + return `${this.redis_key_mapcfg_prefix}${id}`; + } + _redisCmd (func, args, callback) { const db = this.redis_db; @@ -51,8 +55,7 @@ module.exports = class MapStore { // API: Load a saved MapStore object, renewing expiration time load (id, callback) { - const key = this.redis_key_mapcfg_prefix + id; - const exp = this.expire_time; + const key = this._key(id); this._redisCmd('GET', [key], (err, json) => { if (err) { @@ -72,7 +75,7 @@ module.exports = class MapStore { return callback(error); } - this._redisCmd('EXPIRE', [key, exp], (err) => { + this._redisCmd('EXPIRE', [key, this.expire_time], (err) => { if (err) { return callback(err); } @@ -85,15 +88,14 @@ module.exports = class MapStore { // API: store map to redis save (map, callback) { const id = map.id(); - const key = this.redis_key_mapcfg_prefix + id; - const exp = this.expire_time; + const key = this._key(id); this._redisCmd('SETNX', [key, map.serialize()], (err, wasNew) => { if (err) { return callback(err); } - this._redisCmd('EXPIRE', [key, exp], (err) => { + this._redisCmd('EXPIRE', [key, this.expire_time], (err) => { if (err) { return callback(err); } @@ -105,7 +107,7 @@ module.exports = class MapStore { // API: delete map from store del (id, callback) { - const key = this.redis_key_mapcfg_prefix + id; + const key = this._key(id); this._redisCmd('DEL', [key], callback); } }; From f64f45301abf5482f988aee020c87149709f1469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 15:07:51 +0200 Subject: [PATCH 10/45] Better defaults --- lib/storages/mapstore.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index b9f72d998..ef5568d7f 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -3,19 +3,22 @@ const RedisPool = require('redis-mpool'); const MapConfig = require('../models/mapconfig'); +const defaultOptions = { + redis_host: '127.0.0.1', + redis_port: 6379, + redis_db: 0, + redis_key_mapcfg_prefix: 'map_cfg|', + expire_time: 300, // in seconds + pool: undefined, // redis pool client + pool_max: 50, + pool_idleTimeout: 10000, // in milliseconds + pool_reapInterval: 1000, // in milliseconds + pool_log: false +}; + module.exports = class MapStore { constructor (options = {}) { - Object.assign(this, { - redis_host: '127.0.0.1', - redis_port: 6379, - redis_db: 0, - redis_key_mapcfg_prefix: 'map_cfg|', - expire_time: 300, // in seconds - pool_max: 50, - pool_idleTimeout: 10000, // in milliseconds - pool_reapInterval: 1000, // in milliseconds - pool_log: false - }, options); + Object.assign(this, defaultOptions, options); if (!this.pool) { this.pool = new RedisPool({ From c37c4365b34ab6318228156ba084bf135140b50c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 15:38:02 +0200 Subject: [PATCH 11/45] One liner --- lib/storages/mapstore.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/storages/mapstore.js b/lib/storages/mapstore.js index ef5568d7f..58d2472b9 100644 --- a/lib/storages/mapstore.js +++ b/lib/storages/mapstore.js @@ -66,8 +66,7 @@ module.exports = class MapStore { } if (!json) { - const mapConfigError = new Error(`Invalid or nonexistent map configuration token '${id}'`); - return callback(mapConfigError); + return callback(new Error(`Invalid or nonexistent map configuration token '${id}'`)); } let mapConfig; From 06fa097c9bd090d8bcf3bb7e6c133e2e3a39f751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 15:38:23 +0200 Subject: [PATCH 12/45] Apply ES6 --- lib/stats/timer.js | 60 ++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/lib/stats/timer.js b/lib/stats/timer.js index 6001cae93..e9b37efc5 100644 --- a/lib/stats/timer.js +++ b/lib/stats/timer.js @@ -1,37 +1,35 @@ 'use strict'; -function Timer () { - this.times = {}; -} - -module.exports = Timer; - -Timer.prototype.start = function (label) { - this.timeIt(label, 'start'); -}; - -Timer.prototype.end = function (label) { - this.timeIt(label, 'end'); -}; - -Timer.prototype.timeIt = function (label, what) { - this.times[label] = this.times[label] || {}; - this.times[label][what] = Date.now(); -}; - -Timer.prototype.getTimes = function () { - var self = this; - var times = {}; - - Object.keys(this.times).forEach(function (label) { - var stat = self.times[label]; - if (stat.start && stat.end) { - var elapsed = stat.end - stat.start; - if (elapsed > 0) { - times[label] = elapsed; +module.exports = class Timer { + constructor () { + this.times = {}; + } + + start (label) { + this.timeIt(label, 'start'); + } + + end (label) { + this.timeIt(label, 'end'); + } + + timeIt (label, what) { + this.times[label] = this.times[label] || {}; + this.times[label][what] = Date.now(); + } + + getTimes () { + const times = {}; + + for (const [label, stat] of Object.entries(this.times)) { + if (stat.start && stat.end) { + const elapsed = stat.end - stat.start; + if (elapsed > 0) { + times[label] = elapsed; + } } } - }); - return times; + return times; + } }; From 180bd645c81d34c25552c6aec29a3a9d3f992a21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 15:58:10 +0200 Subject: [PATCH 13/45] ES6 syntax --- lib/renderers/renderer-factory.js | 196 +++++++++++++++--------------- 1 file changed, 98 insertions(+), 98 deletions(-) diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index fa764d86a..bf3a3cec8 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -1,122 +1,122 @@ 'use strict'; -var HttpRenderer = require('./http'); -var BlendRenderer = require('./blend'); -var TorqueRenderer = require('./torque'); -var MapnikRenderer = require('./mapnik'); -var PlainRenderer = require('./plain'); -var PgMvtRenderer = require('./pg-mvt'); -var layersFilter = require('../utils/layer-filter'); - -function RendererFactory (opts) { - opts.http = opts.http || {}; - opts.mapnik = opts.mapnik || {}; - opts.torque = opts.torque || {}; - opts.mvt = opts.mvt || {}; - if (opts.mapnik.mapnik) { - opts.mvt.limits = opts.mapnik.mapnik.limits; +const HttpRenderer = require('./http'); +const BlendRenderer = require('./blend'); +const TorqueRenderer = require('./torque'); +const MapnikRenderer = require('./mapnik'); +const PlainRenderer = require('./plain'); +const PgMvtRenderer = require('./pg-mvt'); +const layersFilter = require('../utils/layer-filter'); + +module.exports = class RendererFactory { + constructor (opts) { + opts.http = opts.http || {}; + opts.mapnik = opts.mapnik || {}; + opts.torque = opts.torque || {}; + opts.mvt = opts.mvt || {}; + if (opts.mapnik.mapnik) { + opts.mvt.limits = opts.mapnik.mapnik.limits; + } + this.opts = opts; + + const availableFactories = [ + new MapnikRenderer.factory(opts.mapnik), // eslint-disable-line new-cap + new TorqueRenderer.factory(opts.torque), // eslint-disable-line new-cap + new PlainRenderer.factory(), // eslint-disable-line new-cap + new BlendRenderer.factory(this), // eslint-disable-line new-cap + new HttpRenderer.factory( // eslint-disable-line new-cap + opts.http.whitelist, + opts.http.timeout, + opts.http.proxy, + opts.http.fallbackImage + ), + new PgMvtRenderer.factory(opts.mvt) // eslint-disable-line new-cap + ]; + + this.factories = availableFactories.reduce((factories, factory) => { + factories[factory.getName()] = factory; + return factories; + }, {}); + + this.onTileErrorStrategy = opts.onTileErrorStrategy; } - this.opts = opts; - - this.mapnikRendererFactory = new MapnikRenderer.factory(opts.mapnik); // eslint-disable-line new-cap - this.blendRendererFactory = new BlendRenderer.factory(this); // eslint-disable-line new-cap - - var availableFactories = [ - this.mapnikRendererFactory, - new TorqueRenderer.factory(opts.torque), // eslint-disable-line new-cap - new PlainRenderer.factory(), // eslint-disable-line new-cap - this.blendRendererFactory, - new HttpRenderer.factory( // eslint-disable-line new-cap - opts.http.whitelist, - opts.http.timeout, - opts.http.proxy, - opts.http.fallbackImage - ), - new PgMvtRenderer.factory(opts.mvt) // eslint-disable-line new-cap - ]; - this.factories = availableFactories.reduce(function (factories, factory) { - factories[factory.getName()] = factory; - return factories; - }, {}); - - this.onTileErrorStrategy = opts.onTileErrorStrategy; -} -module.exports = RendererFactory; + getFactory (mapConfig, layer, format) { + const factoryName = this.getFactoryName(mapConfig, layer, format); + return this.factories[factoryName]; + } -RendererFactory.prototype.getFactory = function (mapConfig, layer, format) { - var factoryName = this.getFactoryName(mapConfig, layer, format); - return this.factories[factoryName]; -}; + getRenderer (mapConfig, params, context, callback) { + if (Number.isFinite(+params.layer) && !mapConfig.getLayer(params.layer)) { + return callback(new Error("Layer '" + params.layer + "' not found in layergroup")); + } -RendererFactory.prototype.getRenderer = function (mapConfig, params, context, callback) { - if (Number.isFinite(+params.layer) && !mapConfig.getLayer(params.layer)) { - return callback(new Error("Layer '" + params.layer + "' not found in layergroup")); - } + let factory; + try { + factory = this.getFactory(mapConfig, params.layer, params.format); + } catch (err) { + return callback(err); + } - var factory; - try { - factory = this.getFactory(mapConfig, params.layer, params.format); - } catch (err) { - return callback(err); - } + if (!factory) { + return callback(new Error("Type for layer '" + params.layer + "' not supported")); + } - if (!factory) { - return callback(new Error("Type for layer '" + params.layer + "' not supported")); - } + if (!factory.supportsFormat(params.format)) { + return callback(new Error('Unsupported format ' + params.format)); + } - if (!factory.supportsFormat(params.format)) { - return callback(new Error('Unsupported format ' + params.format)); + return this._genericMakeRenderer(factory, mapConfig, params, context, callback); } - return this.genericMakeRenderer(factory, mapConfig, params, context, callback); -}; + _genericMakeRenderer (factory, mapConfig, params, context, callback) { + const format = params.format; + const options = { + params: params, + layer: params.layer, + limits: context.limits || {} + }; + + // waiting for async/await to refactor + try { + factory.getRenderer(mapConfig, format, options, (err, renderer) => { + if (err) { + return callback(err); + } + + const onTileErrorStrategy = context.onTileErrorStrategy || this.onTileErrorStrategy; + + try { + const rendererAdaptor = factory.getAdaptor(renderer, onTileErrorStrategy); + return callback(null, rendererAdaptor); + } catch (error) { + return callback(error); + } + }); + } catch (error) { + return callback(error); + } + } -RendererFactory.prototype.genericMakeRenderer = function (factory, mapConfig, params, context, callback) { - var format = params.format; - var options = { - params: params, - layer: params.layer, - limits: context.limits || {} - }; - var onTileErrorStrategy = context.onTileErrorStrategy || this.onTileErrorStrategy; - - // waiting for async/await to refactor - try { - factory.getRenderer(mapConfig, format, options, function (err, renderer) { - if (err) { - return callback(err); + getFactoryName (mapConfig, layer, format) { + if (isMapnikFactory(mapConfig, layer, format)) { + if (this.opts.mvt.usePostGIS && format === 'mvt') { + return PgMvtRenderer.factory.NAME; } - try { - const rendererAdaptor = factory.getAdaptor(renderer, onTileErrorStrategy); - return callback(null, rendererAdaptor); - } catch (error) { - return callback(error); - } - }); - } catch (error) { - return callback(error); - } -}; + return MapnikRenderer.factory.NAME; + } -RendererFactory.prototype.getFactoryName = function (mapConfig, layer, format) { - if (isMapnikFactory(mapConfig, layer, format)) { - if (this.opts.mvt.usePostGIS && format === 'mvt') { - return PgMvtRenderer.factory.NAME; + if (layersFilter.isSingleLayer(layer)) { + return mapConfig.layerType(layer); } - return MapnikRenderer.factory.NAME; - } - if (layersFilter.isSingleLayer(layer)) { - return mapConfig.layerType(layer); + return BlendRenderer.factory.NAME; } - - return BlendRenderer.factory.NAME; }; function isMapnikFactory (mapConfig, layer) { - var filteredLayers = layersFilter(mapConfig, layer); + const filteredLayers = layersFilter(mapConfig, layer); return filteredLayers .map(index => mapConfig.layerType(index)) .every(type => type === 'mapnik'); From e0313e5ecedcaf0bcc3ffbda566af724fba2a78b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 16:05:49 +0200 Subject: [PATCH 14/45] Same export order among renderers --- lib/renderers/mapnik/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/renderers/mapnik/index.js b/lib/renderers/mapnik/index.js index 2c02b3696..611df7b05 100644 --- a/lib/renderers/mapnik/index.js +++ b/lib/renderers/mapnik/index.js @@ -1,4 +1,4 @@ 'use strict'; -module.exports.adaptor = require('./adaptor'); module.exports.factory = require('./factory'); +module.exports.adaptor = require('./adaptor'); From 41f5935751433fc12c3e6ad51695635b18df108f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 16:14:14 +0200 Subject: [PATCH 15/45] Use object instead of a list of arguments --- lib/renderers/http/factory.js | 12 ++++---- lib/renderers/renderer-factory.js | 7 +---- .../renderers/http-factory-test.js | 28 +++++++++++++------ 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/renderers/http/factory.js b/lib/renderers/http/factory.js index 743fc55b7..3b593fc2c 100644 --- a/lib/renderers/http/factory.js +++ b/lib/renderers/http/factory.js @@ -4,13 +4,11 @@ var Renderer = require('./renderer'); var FallbackRenderer = require('./fallback-renderer'); var BaseAdaptor = require('../base-adaptor'); -function HttpFactory (whitelist, timeout, proxy, fallbackImage) { - this.whitelist = whitelist || []; - - this.timeout = timeout; - this.proxy = proxy; - - this.fallbackImage = fallbackImage; +function HttpFactory (options = {}) { + this.whitelist = options.whitelist || []; + this.timeout = options.timeout; + this.proxy = options.proxy; + this.fallbackImage = options.fallbackImage; } module.exports = HttpFactory; diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index bf3a3cec8..8453d69de 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -24,12 +24,7 @@ module.exports = class RendererFactory { new TorqueRenderer.factory(opts.torque), // eslint-disable-line new-cap new PlainRenderer.factory(), // eslint-disable-line new-cap new BlendRenderer.factory(this), // eslint-disable-line new-cap - new HttpRenderer.factory( // eslint-disable-line new-cap - opts.http.whitelist, - opts.http.timeout, - opts.http.proxy, - opts.http.fallbackImage - ), + new HttpRenderer.factory(opts.http), // eslint-disable-line new-cap new PgMvtRenderer.factory(opts.mvt) // eslint-disable-line new-cap ]; diff --git a/test/integration/renderers/http-factory-test.js b/test/integration/renderers/http-factory-test.js index 96d6be72b..932b2b4e1 100644 --- a/test/integration/renderers/http-factory-test.js +++ b/test/integration/renderers/http-factory-test.js @@ -63,9 +63,12 @@ describe('renderer_http_factory_getRenderer', function () { }); it('getRenderer returns a fallback image renderer for invalid urlTemplate', function (done) { - var factoryWithFallbackImage = new HttpRendererFactory( - whitelistSample, 2000, undefined, 'http://example.com/fallback.png' - ); + var factoryWithFallbackImage = new HttpRendererFactory({ + whitelist: whitelistSample, + timeout: 2000, + proxy: undefined, + fallbackImage: 'http://example.com/fallback.png' + }); var mapConfig = MapConfig.create({ layers: [ { @@ -87,9 +90,12 @@ describe('renderer_http_factory_getRenderer', function () { it('returns a renderer for invalid urlTemplate if whitelist is _open-minded_', function (done) { var whitelistAnyUrl = ['.*']; - var factoryWithFallbackImage = new HttpRendererFactory( - whitelistAnyUrl, 2000, undefined, 'http://example.com/fallback.png' - ); + var factoryWithFallbackImage = new HttpRendererFactory({ + whitelist: whitelistAnyUrl, + timeout: 2000, + proxy: undefined, + fallbackImage: 'http://example.com/fallback.png' + }); var mapConfig = MapConfig.create({ layers: [ { @@ -115,7 +121,10 @@ describe('renderer_http_factory_getRenderer', function () { it('returns a renderer with valid subdomains for a subdomain template', function (done) { var whitelistAnyUrl = ['.*']; - var factoryWithFallbackImage = new HttpRendererFactory(whitelistAnyUrl, 2000); + var factoryWithFallbackImage = new HttpRendererFactory({ + whitelist: whitelistAnyUrl, + timeout: 2000 + }); var mapConfig = MapConfig.create({ layers: [ { @@ -138,7 +147,10 @@ describe('renderer_http_factory_getRenderer', function () { it('returns a renderer with valid subdomains for a NON subdomain template', function (done) { var whitelistAnyUrl = ['.*']; - var factoryWithFallbackImage = new HttpRendererFactory(whitelistAnyUrl, 2000); + var factoryWithFallbackImage = new HttpRendererFactory({ + whitelist: whitelistAnyUrl, + timeout: 2000 + }); var mapConfig = MapConfig.create({ layers: [ { From b73fee83cd0b7406f3021a001e707b63cfdf3ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 16:42:42 +0200 Subject: [PATCH 16/45] Stop using index.js to export renderer factories --- lib/renderers/blend/index.js | 4 ---- lib/renderers/http/index.js | 4 ---- lib/renderers/mapnik/index.js | 4 ---- lib/renderers/pg-mvt/index.js | 4 ---- lib/renderers/plain/index.js | 4 ---- lib/renderers/renderer-factory.js | 39 ++++++++++++++----------------- lib/renderers/torque/index.js | 4 ---- test/unit/torque-test.js | 2 +- 8 files changed, 18 insertions(+), 47 deletions(-) delete mode 100644 lib/renderers/blend/index.js delete mode 100644 lib/renderers/http/index.js delete mode 100644 lib/renderers/mapnik/index.js delete mode 100644 lib/renderers/pg-mvt/index.js delete mode 100644 lib/renderers/plain/index.js delete mode 100644 lib/renderers/torque/index.js diff --git a/lib/renderers/blend/index.js b/lib/renderers/blend/index.js deleted file mode 100644 index 20cd310a8..000000000 --- a/lib/renderers/blend/index.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; - -module.exports.factory = require('./factory'); -module.exports.adaptor = require('../base-adaptor'); diff --git a/lib/renderers/http/index.js b/lib/renderers/http/index.js deleted file mode 100644 index 20cd310a8..000000000 --- a/lib/renderers/http/index.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; - -module.exports.factory = require('./factory'); -module.exports.adaptor = require('../base-adaptor'); diff --git a/lib/renderers/mapnik/index.js b/lib/renderers/mapnik/index.js deleted file mode 100644 index 611df7b05..000000000 --- a/lib/renderers/mapnik/index.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; - -module.exports.factory = require('./factory'); -module.exports.adaptor = require('./adaptor'); diff --git a/lib/renderers/pg-mvt/index.js b/lib/renderers/pg-mvt/index.js deleted file mode 100644 index 20cd310a8..000000000 --- a/lib/renderers/pg-mvt/index.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; - -module.exports.factory = require('./factory'); -module.exports.adaptor = require('../base-adaptor'); diff --git a/lib/renderers/plain/index.js b/lib/renderers/plain/index.js deleted file mode 100644 index 20cd310a8..000000000 --- a/lib/renderers/plain/index.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; - -module.exports.factory = require('./factory'); -module.exports.adaptor = require('../base-adaptor'); diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index 8453d69de..f3fec1829 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -1,11 +1,11 @@ 'use strict'; -const HttpRenderer = require('./http'); -const BlendRenderer = require('./blend'); -const TorqueRenderer = require('./torque'); -const MapnikRenderer = require('./mapnik'); -const PlainRenderer = require('./plain'); -const PgMvtRenderer = require('./pg-mvt'); +const HttpRendererFactory = require('./http/factory'); +const BlendRendererFactory = require('./blend/factory'); +const TorqueRendererFactory = require('./torque/factory'); +const MapnikRendererFactory = require('./mapnik/factory'); +const PlainRendererFactory = require('./plain/factory'); +const PgMvtRendererFactory = require('./pg-mvt/factory'); const layersFilter = require('../utils/layer-filter'); module.exports = class RendererFactory { @@ -19,19 +19,14 @@ module.exports = class RendererFactory { } this.opts = opts; - const availableFactories = [ - new MapnikRenderer.factory(opts.mapnik), // eslint-disable-line new-cap - new TorqueRenderer.factory(opts.torque), // eslint-disable-line new-cap - new PlainRenderer.factory(), // eslint-disable-line new-cap - new BlendRenderer.factory(this), // eslint-disable-line new-cap - new HttpRenderer.factory(opts.http), // eslint-disable-line new-cap - new PgMvtRenderer.factory(opts.mvt) // eslint-disable-line new-cap - ]; - - this.factories = availableFactories.reduce((factories, factory) => { - factories[factory.getName()] = factory; - return factories; - }, {}); + this.factories = { + [MapnikRendererFactory.NAME]: new MapnikRendererFactory(opts.mapnik), + [TorqueRendererFactory.NAME]: new TorqueRendererFactory(opts.torque), + [PlainRendererFactory.NAME]: new PlainRendererFactory(), + [BlendRendererFactory.NAME]: new BlendRendererFactory(this), + [HttpRendererFactory.NAME]: new HttpRendererFactory(opts.http), + [PgMvtRendererFactory.NAME]: new PgMvtRendererFactory(opts.mvt) + }; this.onTileErrorStrategy = opts.onTileErrorStrategy; } @@ -96,17 +91,17 @@ module.exports = class RendererFactory { getFactoryName (mapConfig, layer, format) { if (isMapnikFactory(mapConfig, layer, format)) { if (this.opts.mvt.usePostGIS && format === 'mvt') { - return PgMvtRenderer.factory.NAME; + return PgMvtRendererFactory.NAME; } - return MapnikRenderer.factory.NAME; + return MapnikRendererFactory.NAME; } if (layersFilter.isSingleLayer(layer)) { return mapConfig.layerType(layer); } - return BlendRenderer.factory.NAME; + return BlendRendererFactory.NAME; } }; diff --git a/lib/renderers/torque/index.js b/lib/renderers/torque/index.js deleted file mode 100644 index 20cd310a8..000000000 --- a/lib/renderers/torque/index.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; - -module.exports.factory = require('./factory'); -module.exports.adaptor = require('../base-adaptor'); diff --git a/test/unit/torque-test.js b/test/unit/torque-test.js index 551dff89b..0c5760338 100644 --- a/test/unit/torque-test.js +++ b/test/unit/torque-test.js @@ -3,7 +3,7 @@ require('../support/test-helper'); var assert = require('assert'); -var TorqueFactory = require('../../lib/renderers/torque').factory; +var TorqueFactory = require('../../lib/renderers/torque/factory'); var MapConfig = require('../../lib/models/mapconfig'); function dummyPSQL () { From 2f5fc58f719823503db782fc898b1b9fde9fd387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 17:07:14 +0200 Subject: [PATCH 17/45] Simplify constructor --- lib/renderers/renderer-factory.js | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index f3fec1829..23ae72808 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -9,26 +9,18 @@ const PgMvtRendererFactory = require('./pg-mvt/factory'); const layersFilter = require('../utils/layer-filter'); module.exports = class RendererFactory { - constructor (opts) { - opts.http = opts.http || {}; - opts.mapnik = opts.mapnik || {}; - opts.torque = opts.torque || {}; - opts.mvt = opts.mvt || {}; - if (opts.mapnik.mapnik) { - opts.mvt.limits = opts.mapnik.mapnik.limits; - } - this.opts = opts; - + constructor ({ mapnik = {}, mvt = {}, torque = {}, http = {}, onTileErrorStrategy } = {}) { + this.onTileErrorStrategy = onTileErrorStrategy; + this.usePgMvt = mvt && mvt.usePostGIS; + mvt.limits = mapnik && mapnik.mapnik && mapnik.mapnik.limits; this.factories = { - [MapnikRendererFactory.NAME]: new MapnikRendererFactory(opts.mapnik), - [TorqueRendererFactory.NAME]: new TorqueRendererFactory(opts.torque), + [MapnikRendererFactory.NAME]: new MapnikRendererFactory(mapnik), + [TorqueRendererFactory.NAME]: new TorqueRendererFactory(torque), [PlainRendererFactory.NAME]: new PlainRendererFactory(), [BlendRendererFactory.NAME]: new BlendRendererFactory(this), - [HttpRendererFactory.NAME]: new HttpRendererFactory(opts.http), - [PgMvtRendererFactory.NAME]: new PgMvtRendererFactory(opts.mvt) + [HttpRendererFactory.NAME]: new HttpRendererFactory(http), + [PgMvtRendererFactory.NAME]: new PgMvtRendererFactory(mvt) }; - - this.onTileErrorStrategy = opts.onTileErrorStrategy; } getFactory (mapConfig, layer, format) { @@ -90,7 +82,7 @@ module.exports = class RendererFactory { getFactoryName (mapConfig, layer, format) { if (isMapnikFactory(mapConfig, layer, format)) { - if (this.opts.mvt.usePostGIS && format === 'mvt') { + if (this.usePgMvt && format === 'mvt') { return PgMvtRendererFactory.NAME; } From a13c77c116ca4567ca834b6adf61d6ac4b0831ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 17:10:30 +0200 Subject: [PATCH 18/45] Inline method --- lib/renderers/renderer-factory.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index 23ae72808..e351cc91a 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -48,10 +48,6 @@ module.exports = class RendererFactory { return callback(new Error('Unsupported format ' + params.format)); } - return this._genericMakeRenderer(factory, mapConfig, params, context, callback); - } - - _genericMakeRenderer (factory, mapConfig, params, context, callback) { const format = params.format; const options = { params: params, From b47ccf53fb273e2d5441280c595909581c7ac56f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 17:19:49 +0200 Subject: [PATCH 19/45] ES6 syntax --- lib/renderers/renderer-factory.js | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index e351cc91a..07b5ff4a5 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -29,43 +29,41 @@ module.exports = class RendererFactory { } getRenderer (mapConfig, params, context, callback) { - if (Number.isFinite(+params.layer) && !mapConfig.getLayer(params.layer)) { - return callback(new Error("Layer '" + params.layer + "' not found in layergroup")); + const { format, layer } = params; + + if (Number.isFinite(+layer) && !mapConfig.getLayer(layer)) { + return callback(new Error(`Layer '${layer}' not found in layergroup`)); } let factory; try { - factory = this.getFactory(mapConfig, params.layer, params.format); + factory = this.getFactory(mapConfig, layer, format); } catch (err) { return callback(err); } if (!factory) { - return callback(new Error("Type for layer '" + params.layer + "' not supported")); + return callback(new Error(`Type for layer '${layer}' not supported`)); } - if (!factory.supportsFormat(params.format)) { - return callback(new Error('Unsupported format ' + params.format)); + if (!factory.supportsFormat(format)) { + return callback(new Error(`Unsupported format ${format}`)); } - const format = params.format; - const options = { - params: params, - layer: params.layer, - limits: context.limits || {} - }; - // waiting for async/await to refactor try { + const { limits = {} } = context; + const options = { params, layer, limits }; + factory.getRenderer(mapConfig, format, options, (err, renderer) => { if (err) { return callback(err); } - const onTileErrorStrategy = context.onTileErrorStrategy || this.onTileErrorStrategy; - try { + const { onTileErrorStrategy = this.onTileErrorStrategy } = context; const rendererAdaptor = factory.getAdaptor(renderer, onTileErrorStrategy); + return callback(null, rendererAdaptor); } catch (error) { return callback(error); From 556de801a5cfb4525c66096d83749f9529a0283c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 17:27:19 +0200 Subject: [PATCH 20/45] Remove unneeded try/catch --- lib/renderers/renderer-factory.js | 35 ++++++++++++------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index 07b5ff4a5..f3b7dafb0 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -50,28 +50,19 @@ module.exports = class RendererFactory { return callback(new Error(`Unsupported format ${format}`)); } - // waiting for async/await to refactor - try { - const { limits = {} } = context; - const options = { params, layer, limits }; - - factory.getRenderer(mapConfig, format, options, (err, renderer) => { - if (err) { - return callback(err); - } - - try { - const { onTileErrorStrategy = this.onTileErrorStrategy } = context; - const rendererAdaptor = factory.getAdaptor(renderer, onTileErrorStrategy); - - return callback(null, rendererAdaptor); - } catch (error) { - return callback(error); - } - }); - } catch (error) { - return callback(error); - } + const { limits = {} } = context; + const options = { params, layer, limits }; + + factory.getRenderer(mapConfig, format, options, (err, renderer) => { + if (err) { + return callback(err); + } + + const { onTileErrorStrategy = this.onTileErrorStrategy } = context; + const rendererAdaptor = factory.getAdaptor(renderer, onTileErrorStrategy); + + return callback(null, rendererAdaptor); + }); } getFactoryName (mapConfig, layer, format) { From 1f3fbfa841af987d56875701c9962a88be3f6614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 18:35:41 +0200 Subject: [PATCH 21/45] Use try/catch where is needed --- lib/renderers/renderer-factory.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index f3b7dafb0..9ec3cd1cc 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -35,12 +35,7 @@ module.exports = class RendererFactory { return callback(new Error(`Layer '${layer}' not found in layergroup`)); } - let factory; - try { - factory = this.getFactory(mapConfig, layer, format); - } catch (err) { - return callback(err); - } + const factory = this.getFactory(mapConfig, layer, format); if (!factory) { return callback(new Error(`Type for layer '${layer}' not supported`)); @@ -83,8 +78,12 @@ module.exports = class RendererFactory { }; function isMapnikFactory (mapConfig, layer) { - const filteredLayers = layersFilter(mapConfig, layer); - return filteredLayers - .map(index => mapConfig.layerType(index)) - .every(type => type === 'mapnik'); + try { + const filteredLayers = layersFilter(mapConfig, layer); + return filteredLayers + .map(index => mapConfig.layerType(index)) + .every(type => type === 'mapnik'); + } catch (err) { + return false; + } } From ec972218b077f6dca507ad37e9e37110baa198c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 18:55:41 +0200 Subject: [PATCH 22/45] Use ES6 class syntax --- lib/renderers/http/factory.js | 105 +++++++++++++++++----------------- 1 file changed, 52 insertions(+), 53 deletions(-) diff --git a/lib/renderers/http/factory.js b/lib/renderers/http/factory.js index 3b593fc2c..9d9fd2a52 100644 --- a/lib/renderers/http/factory.js +++ b/lib/renderers/http/factory.js @@ -1,62 +1,69 @@ 'use strict'; -var Renderer = require('./renderer'); -var FallbackRenderer = require('./fallback-renderer'); -var BaseAdaptor = require('../base-adaptor'); - -function HttpFactory (options = {}) { - this.whitelist = options.whitelist || []; - this.timeout = options.timeout; - this.proxy = options.proxy; - this.fallbackImage = options.fallbackImage; -} - -module.exports = HttpFactory; -const NAME = 'http'; -module.exports.NAME = NAME; +const Renderer = require('./renderer'); +const FallbackRenderer = require('./fallback-renderer'); +const BaseAdaptor = require('../base-adaptor'); -HttpFactory.prototype.getName = function () { - return NAME; -}; - -HttpFactory.prototype.supportsFormat = function (format) { - return format === 'png'; -}; +module.exports = class HttpFactory { + static get NAME () { + return 'http'; + } -HttpFactory.prototype.getAdaptor = function (renderer, onTileErrorStrategy) { - return new BaseAdaptor(renderer, onTileErrorStrategy); -}; + static isValidUrlTemplate (urlTemplate, whitelist) { + return whitelist.some((currentValue) => { + return urlTemplate === currentValue || urlTemplate.match(currentValue); + }); + } -HttpFactory.prototype.getRenderer = function (mapConfig, format, options, callback) { - var layerNumber = options.layer; + constructor (options = {}) { + this.whitelist = options.whitelist || []; + this.timeout = options.timeout; + this.proxy = options.proxy; + this.fallbackImage = options.fallbackImage; + } - var layer = mapConfig.getLayer(layerNumber); - var urlTemplate = layer.options.urlTemplate; + getName () { + return HttpFactory.NAME; + } - if (layer.type !== this.getName()) { - return callback(new Error('Layer is not an http layer')); + supportsFormat (format) { + return format === 'png'; } - if (!urlTemplate) { - return callback(new Error('Missing mandatory "urlTemplate" option')); + getAdaptor (renderer, onTileErrorStrategy) { + return new BaseAdaptor(renderer, onTileErrorStrategy); } - if (!isValidUrlTemplate(urlTemplate, this.whitelist)) { - if (this.fallbackImage) { - return callback(null, new FallbackRenderer(this.fallbackImage)); - } else { - return callback(new Error('Invalid "urlTemplate" for http layer')); + getRenderer (mapConfig, format, options, callback) { + const layerNumber = options.layer; + const layer = mapConfig.getLayer(layerNumber); + const urlTemplate = layer.options.urlTemplate; + + if (layer.type !== this.getName()) { + return callback(new Error('Layer is not an http layer')); + } + + if (!urlTemplate) { + return callback(new Error('Missing mandatory "urlTemplate" option')); + } + + if (!HttpFactory.isValidUrlTemplate(urlTemplate, this.whitelist)) { + if (this.fallbackImage) { + return callback(null, new FallbackRenderer(this.fallbackImage)); + } else { + return callback(new Error('Invalid "urlTemplate" for http layer')); + } } - } - var subdomains = getSubdomains(urlTemplate, layer.options); + const subdomains = getSubdomains(urlTemplate, layer.options); + const rendererOptions = { + tms: layer.options.tms || false, + timeout: this.timeout, + proxy: this.proxy + }; - var rendererOptions = { - tms: layer.options.tms || false, - timeout: this.timeout, - proxy: this.proxy - }; - return callback(null, new Renderer(urlTemplate, subdomains, rendererOptions)); + return callback(null, new Renderer(urlTemplate, subdomains, rendererOptions)); + } }; function getSubdomains (urlTemplate, options) { @@ -68,11 +75,3 @@ function getSubdomains (urlTemplate, options) { return subdomains; } - -function isValidUrlTemplate (urlTemplate, whitelist) { - return whitelist.some(function (currentValue) { - return urlTemplate === currentValue || urlTemplate.match(currentValue); - }); -} - -module.exports.isValidUrlTemplate = isValidUrlTemplate; From 2a63969d11c984ec2602c15f4fc6a9e5ece49282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 19:21:02 +0200 Subject: [PATCH 23/45] ES6 class syntax --- lib/renderers/plain/factory.js | 85 +++++++++++++++++----------------- 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/lib/renderers/plain/factory.js b/lib/renderers/plain/factory.js index e5a56d361..bfecc15da 100644 --- a/lib/renderers/plain/factory.js +++ b/lib/renderers/plain/factory.js @@ -1,58 +1,56 @@ 'use strict'; -var mapnik = require('@carto/mapnik'); -var ColorRenderer = require('./color-renderer'); -var ImageRenderer = require('./image-renderer'); -var BaseAdaptor = require('../base-adaptor'); - -var requestImage = require('../http/fetch-image'); - -function PlainFactory () { -} - -module.exports = PlainFactory; -const NAME = 'plain'; -module.exports.NAME = NAME; - -PlainFactory.prototype.getName = function () { - return NAME; -}; +const mapnik = require('@carto/mapnik'); +const ColorRenderer = require('./color-renderer'); +const ImageRenderer = require('./image-renderer'); +const BaseAdaptor = require('../base-adaptor'); +const requestImage = require('../http/fetch-image'); + +module.exports = class PlainFactory { + static get NAME () { + return 'plain'; + } -PlainFactory.prototype.supportsFormat = function (format) { - return format === 'png'; -}; + getName () { + return PlainFactory.NAME; + } -PlainFactory.prototype.getAdaptor = function (renderer, onTileErrorStrategy) { - return new BaseAdaptor(renderer, onTileErrorStrategy); -}; + supportsFormat (format) { + return format === 'png'; + } -PlainFactory.prototype.getRenderer = function (mapConfig, format, options, callback) { - var layerNumber = options.layer; + getAdaptor (renderer, onTileErrorStrategy) { + return new BaseAdaptor(renderer, onTileErrorStrategy); + } - var layer = mapConfig.getLayer(layerNumber); + getRenderer (mapConfig, format, options, callback) { + const layerNumber = options.layer; + const layer = mapConfig.getLayer(layerNumber); - if (layer.type !== this.getName()) { - return callback(new Error('Layer is not a \'plain\' layer')); - } + if (layer.type !== this.getName()) { + return callback(new Error('Layer is not a \'plain\' layer')); + } - var color = layer.options.color; - var imageUrl = layer.options.imageUrl; + const color = layer.options.color; + const imageUrl = layer.options.imageUrl; - if (!color && !imageUrl) { - return callback(new Error('Plain layer: at least one of the options, `color` or `imageUrl`, must be provided.')); - } + if (!color && !imageUrl) { + return callback(new Error('Plain layer: at least one of the options, `color` or `imageUrl`, must be provided.')); + } - if (color) { - colorRenderer(color, layer.options, callback); - } else if (imageUrl) { - imageUrlRenderer(imageUrl, layer.options, callback); - } else { - return callback(new Error('Plain layer unknown error')); + if (color) { + colorRenderer(color, layer.options, callback); + } else if (imageUrl) { + imageUrlRenderer(imageUrl, layer.options, callback); + } else { + return callback(new Error('Plain layer unknown error')); + } } }; function colorRenderer (color, options, callback) { - var mapnikColor; + let mapnikColor; + try { if (Array.isArray(color)) { if (color.length === 3) { @@ -60,14 +58,15 @@ function colorRenderer (color, options, callback) { } else if (color.length === 4) { mapnikColor = new mapnik.Color(color[0], color[1], color[2], color[3]); } else { - return callback(new Error("Invalid color for 'plain' layer: invalid integer array")); + return callback(new Error('Invalid color for \'plain\' layer: invalid integer array')); } } else { mapnikColor = new mapnik.Color(color); } } catch (err) { - return callback(new Error("Invalid color for 'plain' layer: " + err.message)); + return callback(new Error(`Invalid color for 'plain' layer: ${err.message}`)); } + return callback(null, new ColorRenderer(mapnikColor, options)); } From e5858884fed4695f713f674c3f5840c5f7915acb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 19:32:16 +0200 Subject: [PATCH 24/45] ES6 class syntax --- lib/renderers/pg-mvt/factory.js | 58 +++++------- lib/renderers/torque/factory.js | 151 +++++++++++++++----------------- 2 files changed, 94 insertions(+), 115 deletions(-) diff --git a/lib/renderers/pg-mvt/factory.js b/lib/renderers/pg-mvt/factory.js index e090e0472..1b9c1759a 100644 --- a/lib/renderers/pg-mvt/factory.js +++ b/lib/renderers/pg-mvt/factory.js @@ -5,45 +5,33 @@ const RendererParams = require('../renderer-params'); const Renderer = require('./renderer'); const BaseAdaptor = require('../base-adaptor'); -/** - * API: initializes the renderer, it should be called once - * - * @param {Object} options - * - dbPoolParams: database connection pool params - * - size: maximum number of resources to create at any given time - * - idleTimeout: max milliseconds a resource can go unused before it should be destroyed - * - reapInterval: frequency to check for idle resources - */ -function PgMvtFactory (options) { - this.options = options || {}; -} - -module.exports = PgMvtFactory; -const NAME = 'pg-mvt'; -const MVT_FORMAT = 'mvt'; -module.exports.NAME = NAME; +module.exports = class PgMvtFactory { + static get NAME () { + return 'pg-mvt'; + } -PgMvtFactory.prototype = { - /// API: renderer name, use for information purposes - name: NAME, + static get MVT_FORMAT () { + return 'mvt'; + } - /// API: tile formats this module is able to render - supported_formats: [MVT_FORMAT], + constructor (options = {}) { + this.options = options; + } - getName: function () { - return this.name; - }, + getName () { + return PgMvtFactory.NAME; + } - supportsFormat: function (format) { - return format === MVT_FORMAT; - }, + supportsFormat (format) { + return format === PgMvtFactory.MVT_FORMAT; + } - getAdaptor: function (renderer, onTileErrorStrategy) { + getAdaptor (renderer, onTileErrorStrategy) { return new BaseAdaptor(renderer, onTileErrorStrategy); - }, + } - getRenderer: function (mapConfig, format, options, callback) { - if (mapConfig.isVectorOnlyMapConfig() && format !== MVT_FORMAT) { + getRenderer (mapConfig, format, options, callback) { + if (mapConfig.isVectorOnlyMapConfig() && format !== PgMvtFactory.MVT_FORMAT) { const error = new Error(`Unsupported format: 'cartocss' option is missing for ${format}`); error.http_status = 400; error.type = 'tile'; @@ -51,7 +39,7 @@ PgMvtFactory.prototype = { } if (!this.supportsFormat(format)) { - return callback(new Error('format not supported: ' + format)); + return callback(new Error(`format not supported: ${format}`)); } const mapLayers = mapConfig.getLayers(); @@ -68,8 +56,8 @@ PgMvtFactory.prototype = { const dbParams = RendererParams.dbParamsFromReqParams(options.params); Object.assign(dbParams, mapConfig.getLayerDatasource(options.layer)); - if (Number.isFinite(mapConfig.getBufferSize(MVT_FORMAT))) { - this.options.bufferSize = mapConfig.getBufferSize(MVT_FORMAT); + if (Number.isFinite(mapConfig.getBufferSize(PgMvtFactory.MVT_FORMAT))) { + this.options.bufferSize = mapConfig.getBufferSize(PgMvtFactory.MVT_FORMAT); } try { diff --git a/lib/renderers/torque/factory.js b/lib/renderers/torque/factory.js index 9bbc1527d..49fb2be35 100644 --- a/lib/renderers/torque/factory.js +++ b/lib/renderers/torque/factory.js @@ -1,57 +1,18 @@ 'use strict'; -var carto = require('carto'); -var debug = require('debug')('windshaft:renderer:torque_factory'); -var PSQL = require('cartodb-psql'); -var torqueReference = require('torque.js').cartocss_reference; - -var RendererParams = require('../renderer-params'); - -var sql = require('../../utils/sql'); -var Renderer = require('./renderer'); -var PSQLAdaptor = require('./psql-adaptor'); -var PngRenderer = require('./png-renderer'); -var BaseAdaptor = require('../base-adaptor'); - +const carto = require('carto'); +const debug = require('debug')('windshaft:renderer:torque_factory'); +const PSQL = require('cartodb-psql'); +const torqueReference = require('torque.js').cartocss_reference; +const RendererParams = require('../renderer-params'); +const sql = require('../../utils/sql'); +const Renderer = require('./renderer'); +const PSQLAdaptor = require('./psql-adaptor'); +const PngRenderer = require('./png-renderer'); +const BaseAdaptor = require('../base-adaptor'); const SubstitutionTokens = require('cartodb-query-tables').utils.substitutionTokens; -/// API: initializes the renderer, it should be called once -// -/// @param options initialization options. -/// - sqlClass: class used to access postgres, by default is PSQL -/// the class should provide the following interface -/// - constructor(params) where params should contain: -/// host, port, database, user, password. -/// the class is always constructed with dbParams passed to -/// getRender as-is -/// - query(sql, callback(err, data), readonly) -/// gets an SQL query and return a javascript object with -/// the same structure of a JSON format response from -/// CartoDB-SQL-API, for reference see -/// http://github.com/CartoDB/CartoDB-SQL-API/blob/1.8.2/doc/API.md#json -/// The 'readonly' parameter (false by default) requests -/// that running the query should not allowed to change the database. -/// - dbPoolParams: database connection pool params -/// - size: maximum number of resources to create at any given time -/// - idleTimeout: max milliseconds a resource can go unused before it should be destroyed -/// - reapInterval: frequency to check for idle resources -/// -function TorqueFactory (options) { - this.options = options || {}; - this.options = Object.assign({ - sqlClass: PSQLAdaptor(PSQL, options.dbPoolParams) - }, this.options); - - if (this.options.sqlClass) { - this.sqlClass = this.options.sqlClass; - } -} - -module.exports = TorqueFactory; -const NAME = 'torque'; -module.exports.NAME = NAME; - -var formatToRenderer = { +const formatToRenderer = { 'torque.json': Renderer, 'torque.bin': Renderer, 'json.torque': Renderer, @@ -60,48 +21,74 @@ var formatToRenderer = { 'torque.png': PngRenderer }; -TorqueFactory.prototype = { - /// API: renderer name, use for information purposes - name: NAME, +// API: initializes the renderer, it should be called once +// +// @param options initialization options. +// - sqlClass: class used to access postgres, by default is PSQL +// the class should provide the following interface +// - constructor(params) where params should contain: +// host, port, database, user, password. +// the class is always constructed with dbParams passed to +// getRender as-is +// - query(sql, callback(err, data), readonly) +// gets an SQL query and return a javascript object with +// the same structure of a JSON format response from +// CartoDB-SQL-API, for reference see +// http://github.com/CartoDB/CartoDB-SQL-API/blob/1.8.2/doc/API.md#json +// The 'readonly' parameter (false by default) requests +// that running the query should not allowed to change the database. +// - dbPoolParams: database connection pool params +// - size: maximum number of resources to create at any given time +// - idleTimeout: max milliseconds a resource can go unused before it should be destroyed +// - reapInterval: frequency to check for idle resources +module.exports = class TorqueFactory { + static get NAME () { + return 'torque'; + } - /// API: tile formats this module is able to render - // TODO: deprecate 'json.torque' and 'bin.torque' with 1.18.0 - supported_formats: Object.keys(formatToRenderer), + constructor (options = {}) { + this.options = Object.assign({ + sqlClass: PSQLAdaptor(PSQL, options.dbPoolParams) + }, options); + } - getName: function () { - return this.name; - }, + getName () { + return TorqueFactory.NAME; + } - supportsFormat: function (format) { + supportsFormat (format) { return !!formatToRenderer[format]; - }, + } - getAdaptor: function (renderer, onTileErrorStrategy) { + getAdaptor (renderer, onTileErrorStrategy) { return new BaseAdaptor(renderer, onTileErrorStrategy); - }, + } - getRenderer: function (mapConfig, format, options, callback) { - var dbParams = RendererParams.dbParamsFromReqParams(options.params); - var layer = options.layer; + getRenderer (mapConfig, format, options, callback) { + const layer = options.layer; if (layer === undefined) { return callback(new Error('torque renderer only supports a single layer')); } + if (!formatToRenderer[format]) { return callback(new Error('format not supported: ' + format)); } - var layerObj = mapConfig.getLayer(layer); + + const layerObj = mapConfig.getLayer(layer); + if (!layerObj) { return callback(new Error('layer index is greater than number of layers')); } + if (layerObj.type !== 'torque') { return callback(new Error('layer ' + layer + ' is not a torque layer')); } - dbParams = Object.assign(dbParams, mapConfig.getLayerDatasource(layer)); + const dbParams = Object.assign({}, RendererParams.dbParamsFromReqParams(options.params), mapConfig.getLayerDatasource(layer)); + const pgSQL = sql(dbParams, this.options.sqlClass); - var pgSQL = sql(dbParams, this.sqlClass); - fetchMapConfigAttributes(pgSQL, layerObj, function (err, params) { + fetchMapConfigAttributes(pgSQL, layerObj, (err, params) => { if (err) { return callback(err); } @@ -121,7 +108,7 @@ TorqueFactory.prototype = { // @throw Error if required properties are not found // function attrsFromCartoCSS (cartocss, required) { - var attrsKeys = { + const attrsKeys = { '-torque-frame-count': 'steps', '-torque-resolution': 'resolution', '-torque-animation-duration': 'animationDuration', @@ -129,8 +116,10 @@ function attrsFromCartoCSS (cartocss, required) { '-torque-time-attribute': 'column', '-torque-data-aggregation': 'data_aggregation' }; + carto.tree.Reference.setData(torqueReference.version.latest); - var env = { + + const env = { benchmark: false, validation_data: false, effects: [], @@ -139,16 +128,17 @@ function attrsFromCartoCSS (cartocss, required) { this.errors.push(e); } }; - var symbolizers = torqueReference.version.latest.layer; - var root = (carto.Parser(env)).parse(cartocss); - var definitions = root.toList(env); - var rules = getMapProperties(definitions, env); - var attrs = {}; - for (var k in attrsKeys) { + const symbolizers = torqueReference.version.latest.layer; + const root = (carto.Parser(env)).parse(cartocss); + const definitions = root.toList(env); + const rules = getMapProperties(definitions, env); + const attrs = {}; + + for (const k in attrsKeys) { if (rules[k]) { attrs[attrsKeys[k]] = rules[k].value.toString(); - var element = rules[k].value.value[0]; - var type = symbolizers[k].type; + const element = rules[k].value.value[0]; + const type = symbolizers[k].type; if (!checkValidType(element, type)) { throw new Error("Unexpected type for property '" + k + "', expected " + type); } @@ -156,6 +146,7 @@ function attrsFromCartoCSS (cartocss, required) { throw new Error("Missing required property '" + k + "' in torque layer CartoCSS"); } } + return attrs; } From e1724d89f1a436918e93070f3d6b36125ffbf273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 20:43:09 +0200 Subject: [PATCH 25/45] ES6 --- lib/renderers/torque/factory.js | 160 +++++++++++++++----------------- 1 file changed, 76 insertions(+), 84 deletions(-) diff --git a/lib/renderers/torque/factory.js b/lib/renderers/torque/factory.js index 49fb2be35..9a5e4e4c6 100644 --- a/lib/renderers/torque/factory.js +++ b/lib/renderers/torque/factory.js @@ -21,31 +21,31 @@ const formatToRenderer = { 'torque.png': PngRenderer }; -// API: initializes the renderer, it should be called once -// -// @param options initialization options. -// - sqlClass: class used to access postgres, by default is PSQL -// the class should provide the following interface -// - constructor(params) where params should contain: -// host, port, database, user, password. -// the class is always constructed with dbParams passed to -// getRender as-is -// - query(sql, callback(err, data), readonly) -// gets an SQL query and return a javascript object with -// the same structure of a JSON format response from -// CartoDB-SQL-API, for reference see -// http://github.com/CartoDB/CartoDB-SQL-API/blob/1.8.2/doc/API.md#json -// The 'readonly' parameter (false by default) requests -// that running the query should not allowed to change the database. -// - dbPoolParams: database connection pool params -// - size: maximum number of resources to create at any given time -// - idleTimeout: max milliseconds a resource can go unused before it should be destroyed -// - reapInterval: frequency to check for idle resources module.exports = class TorqueFactory { static get NAME () { return 'torque'; } + // API: initializes the renderer, it should be called once + // + // @param options initialization options. + // - sqlClass: class used to access postgres, by default is PSQL + // the class should provide the following interface + // - constructor(params) where params should contain: + // host, port, database, user, password. + // the class is always constructed with dbParams passed to + // getRender as-is + // - query(sql, callback(err, data), readonly) + // gets an SQL query and return a javascript object with + // the same structure of a JSON format response from + // CartoDB-SQL-API, for reference see + // http://github.com/CartoDB/CartoDB-SQL-API/blob/1.8.2/doc/API.md#json + // The 'readonly' parameter (false by default) requests + // that running the query should not allowed to change the database. + // - dbPoolParams: database connection pool params + // - size: maximum number of resources to create at any given time + // - idleTimeout: max milliseconds a resource can go unused before it should be destroyed + // - reapInterval: frequency to check for idle resources constructor (options = {}) { this.options = Object.assign({ sqlClass: PSQLAdaptor(PSQL, options.dbPoolParams) @@ -86,27 +86,62 @@ module.exports = class TorqueFactory { } const dbParams = Object.assign({}, RendererParams.dbParamsFromReqParams(options.params), mapConfig.getLayerDatasource(layer)); - const pgSQL = sql(dbParams, this.options.sqlClass); + const pg = sql(dbParams, this.options.sqlClass); - fetchMapConfigAttributes(pgSQL, layerObj, (err, params) => { + fetchMapConfigAttributes(pg, layerObj, (err, params) => { if (err) { return callback(err); } var RendererClass = formatToRenderer[format]; - callback(null, new RendererClass(layerObj, pgSQL, params, layer)); + callback(null, new RendererClass(layerObj, pg, params, layer)); }); } }; +// returns an array of errors if the mapconfig is not supported or contains errors +// errors is empty is the configuration is ok +// dbParams: host, dbname, user and pass +// layer: optional, if is specified only a layer is checked +function fetchMapConfigAttributes (pg, layer, callback) { + let attrs; + + try { + attrs = checkLayerAttributes(layer); + } catch (e) { + return callback(e); + } + + const layerSql = layer.options.sql; + fetchLayerAttributes(pg, layerSql, attrs, (err, layerAttrs) => { + if (err) { + return callback(err); + } + + callback(null, Object.assign(attrs, layerAttrs)); + }); +} + +// check layer and raise an exception is some error is found // +// @throw Error if missing or malformed CartoCSS +function checkLayerAttributes (layerConfig) { + const checks = ['steps', 'resolution', 'column', 'countby']; + const cartoCSS = layerConfig.options.cartocss; + + if (!cartoCSS) { + throw new Error("cartocss can't be undefined"); + } + + return attrsFromCartoCSS(cartoCSS, checks); +} + // given cartocss return javascript properties // // @param required optional array of required properties // // @throw Error if required properties are not found -// function attrsFromCartoCSS (cartocss, required) { const attrsKeys = { '-torque-frame-count': 'steps', @@ -160,74 +195,34 @@ function checkValidType (e, type) { } else if (type === 'color') { return checkValidColor(e); } + return true; } function checkValidColor (e) { - var expectedArguments = { rgb: 3, hsl: 3, rgba: 4, hsla: 4 }; + const expectedArguments = { rgb: 3, hsl: 3, rgba: 4, hsla: 4 }; return typeof e.rgb !== 'undefined' || expectedArguments[e.name] === e.args; } -// // get rules from Map definition // stores errors in env.error -// function getMapProperties (definitions, env) { - var rules = {}; - definitions.filter(function (r) { - return r.elements.join('') === 'Map'; - }).forEach(function (r) { - for (var i = 0; i < r.rules.length; i++) { - var key = r.rules[i].name; - rules[key] = r.rules[i].ev(env); - } - }); - return rules; -} - -// -// check layer and raise an exception is some error is found -// -// @throw Error if missing or malformed CartoCSS -// -function _checkLayerAttributes (layerConfig) { - var cartoCSS = layerConfig.options.cartocss; - if (!cartoCSS) { - throw new Error("cartocss can't be undefined"); - } - var checks = ['steps', 'resolution', 'column', 'countby']; - return attrsFromCartoCSS(cartoCSS, checks); -} - -// returns an array of errors if the mapconfig is not supported or contains errors -// errors is empty is the configuration is ok -// dbParams: host, dbname, user and pass -// layer: optional, if is specified only a layer is checked -function fetchMapConfigAttributes (sql, layer, callback) { - // check attrs - var attrs; - try { - attrs = _checkLayerAttributes(layer); - } catch (e) { - return callback(e); - } - - // fetch layer attributes to check the query and so on is ok - var layerSql = layer.options.sql; - fetchLayerAttributes(sql, layerSql, attrs, function (err, layerAttrs) { - if (err) { - return callback(err); - } - callback(null, Object.assign(attrs, layerAttrs)); - }); + return definitions + .filter((definition) => definition.elements.join('') === 'Map') + .map((definition) => definition.rules) + .reduce((rules, rule) => rules.concat(rule), []) // flat array 1 level + .reduce((properties, rule) => { + properties[rule.name] = rule.ev(env); + return properties; + }, {}); } -function fetchLayerAttributes (sql, layerSql, attrs, callback) { +function fetchLayerAttributes (pg, layerSql, attrs, callback) { layerSql = SubstitutionTokens.replaceXYZ(layerSql, { x: 0, y: 0, z: 0 }); // first step, fetch the schema to know if torque colum is time column const columnsQuery = `select * from (${layerSql}) __torque_wrap_sql limit 0`; - sql(columnsQuery, function (err, data) { + pg(columnsQuery, (err, data) => { // second step, get time bounds to calculate step if (err) { err.message = 'TorqueRenderer: ' + err.message; @@ -236,22 +231,18 @@ function fetchLayerAttributes (sql, layerSql, attrs, callback) { if (!data) { debug(`ERROR: layer query '${layerSql}' returned no data`); - const noDataError = new Error('TorqueRenderer: Layer query returned no data'); - return callback(noDataError); + return callback(new Error('TorqueRenderer: Layer query returned no data')); } if (!Object.prototype.hasOwnProperty.call(data.fields, attrs.column)) { debug(`ERROR: layer query ${layerSql} does not include time-attribute column '${attrs.column}'`); - const columnError = new Error( - `TorqueRenderer: Layer query did not return the requested time-attribute column '${attrs.column}'` - ); - return callback(columnError); + return callback(new Error(`TorqueRenderer: Layer query did not return the requested time-attribute column '${attrs.column}'`)); } const isTime = data.fields[attrs.column].type === 'date'; const stepQuery = getAttributesStepQuery(layerSql, attrs.column, isTime); - sql(stepQuery, function generateMetadata (err, data) { - // prepare metadata needed to render the tiles + + pg(stepQuery, (err, data) => { if (err) { err.message = 'TorqueRenderer: ' + err.message; return callback(err); @@ -278,6 +269,7 @@ function fetchLayerAttributes (sql, layerSql, attrs, callback) { function getAttributesStepQuery (layerSql, column, isTime) { let maxCol = `max(${column})`; let minCol = `min(${column})`; + if (isTime) { maxCol = `date_part('epoch', ${maxCol})`; minCol = `date_part('epoch', ${minCol})`; From fe6fdfe8ffb66dc3b3a94ed92ee6c19eaeed4431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 20:51:10 +0200 Subject: [PATCH 26/45] ES6 --- lib/renderers/torque/factory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/renderers/torque/factory.js b/lib/renderers/torque/factory.js index 9a5e4e4c6..a61d6f616 100644 --- a/lib/renderers/torque/factory.js +++ b/lib/renderers/torque/factory.js @@ -93,7 +93,7 @@ module.exports = class TorqueFactory { return callback(err); } - var RendererClass = formatToRenderer[format]; + const RendererClass = formatToRenderer[format]; callback(null, new RendererClass(layerObj, pg, params, layer)); }); From 4e53cde14dff9b46ccc559fac0c66d513209061a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 10 Apr 2020 20:51:10 +0200 Subject: [PATCH 27/45] ES6 --- lib/renderers/torque/factory.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/renderers/torque/factory.js b/lib/renderers/torque/factory.js index 9a5e4e4c6..8c300fef3 100644 --- a/lib/renderers/torque/factory.js +++ b/lib/renderers/torque/factory.js @@ -93,7 +93,7 @@ module.exports = class TorqueFactory { return callback(err); } - var RendererClass = formatToRenderer[format]; + const RendererClass = formatToRenderer[format]; callback(null, new RendererClass(layerObj, pg, params, layer)); }); @@ -131,7 +131,7 @@ function checkLayerAttributes (layerConfig) { const cartoCSS = layerConfig.options.cartocss; if (!cartoCSS) { - throw new Error("cartocss can't be undefined"); + throw new Error('cartocss can\'t be undefined'); } return attrsFromCartoCSS(cartoCSS, checks); @@ -175,10 +175,10 @@ function attrsFromCartoCSS (cartocss, required) { const element = rules[k].value.value[0]; const type = symbolizers[k].type; if (!checkValidType(element, type)) { - throw new Error("Unexpected type for property '" + k + "', expected " + type); + throw new Error(`Unexpected type for property '${k}', expected ${type}`); } } else if (required && required.indexOf(attrsKeys[k]) !== -1) { - throw new Error("Missing required property '" + k + "' in torque layer CartoCSS"); + throw new Error(`Missing required property '${k}' in torque layer CartoCSS`); } } From 45a4cccbcb6b5b0a7dbe27966600f91395407ad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sun, 12 Apr 2020 13:10:15 +0200 Subject: [PATCH 28/45] Stop injecting a psql wrapper into TorqueRenderer to be able to mock responses while testing. --- lib/renderers/torque/factory.js | 142 +++++++++------------------ lib/renderers/torque/png-renderer.js | 6 +- lib/renderers/torque/psql-adaptor.js | 64 +++++------- lib/renderers/torque/renderer.js | 11 +-- lib/utils/sql.js | 19 ---- test/acceptance/torque-test.js | 10 +- test/unit/torque-test.js | 123 ++++++++++++----------- 7 files changed, 153 insertions(+), 222 deletions(-) delete mode 100644 lib/utils/sql.js diff --git a/lib/renderers/torque/factory.js b/lib/renderers/torque/factory.js index 8c300fef3..ee9d348db 100644 --- a/lib/renderers/torque/factory.js +++ b/lib/renderers/torque/factory.js @@ -2,12 +2,10 @@ const carto = require('carto'); const debug = require('debug')('windshaft:renderer:torque_factory'); -const PSQL = require('cartodb-psql'); const torqueReference = require('torque.js').cartocss_reference; const RendererParams = require('../renderer-params'); -const sql = require('../../utils/sql'); const Renderer = require('./renderer'); -const PSQLAdaptor = require('./psql-adaptor'); +const PsqlAdaptor = require('./psql-adaptor'); const PngRenderer = require('./png-renderer'); const BaseAdaptor = require('../base-adaptor'); const SubstitutionTokens = require('cartodb-query-tables').utils.substitutionTokens; @@ -29,27 +27,12 @@ module.exports = class TorqueFactory { // API: initializes the renderer, it should be called once // // @param options initialization options. - // - sqlClass: class used to access postgres, by default is PSQL - // the class should provide the following interface - // - constructor(params) where params should contain: - // host, port, database, user, password. - // the class is always constructed with dbParams passed to - // getRender as-is - // - query(sql, callback(err, data), readonly) - // gets an SQL query and return a javascript object with - // the same structure of a JSON format response from - // CartoDB-SQL-API, for reference see - // http://github.com/CartoDB/CartoDB-SQL-API/blob/1.8.2/doc/API.md#json - // The 'readonly' parameter (false by default) requests - // that running the query should not allowed to change the database. // - dbPoolParams: database connection pool params // - size: maximum number of resources to create at any given time // - idleTimeout: max milliseconds a resource can go unused before it should be destroyed // - reapInterval: frequency to check for idle resources constructor (options = {}) { - this.options = Object.assign({ - sqlClass: PSQLAdaptor(PSQL, options.dbPoolParams) - }, options); + this.options = options; } getName () { @@ -86,17 +69,15 @@ module.exports = class TorqueFactory { } const dbParams = Object.assign({}, RendererParams.dbParamsFromReqParams(options.params), mapConfig.getLayerDatasource(layer)); - const pg = sql(dbParams, this.options.sqlClass); + const psql = new PsqlAdaptor({ connectionParams: dbParams, poolParams: this.options.dbPoolParams }); - fetchMapConfigAttributes(pg, layerObj, (err, params) => { - if (err) { - return callback(err); - } - - const RendererClass = formatToRenderer[format]; + fetchLayerAttributes(psql, layerObj) + .then((params) => { + const RendererClass = formatToRenderer[format]; - callback(null, new RendererClass(layerObj, pg, params, layer)); - }); + callback(null, new RendererClass(layerObj, psql, params, layer)); + }) + .catch((err) => callback(err)); } }; @@ -104,23 +85,48 @@ module.exports = class TorqueFactory { // errors is empty is the configuration is ok // dbParams: host, dbname, user and pass // layer: optional, if is specified only a layer is checked -function fetchMapConfigAttributes (pg, layer, callback) { - let attrs; - +async function fetchLayerAttributes (psql, layer) { try { - attrs = checkLayerAttributes(layer); - } catch (e) { - return callback(e); - } + const attrs = checkLayerAttributes(layer); + const layerSql = SubstitutionTokens.replaceXYZ(layer.options.sql, { x: 0, y: 0, z: 0 }); + + // fetch the schema to know if torque colum is time column + const columnsQuery = `select * from (${layerSql}) __torque_wrap_sql limit 0`; + const columns = await psql.query(columnsQuery); - const layerSql = layer.options.sql; - fetchLayerAttributes(pg, layerSql, attrs, (err, layerAttrs) => { - if (err) { - return callback(err); + if (!columns) { + debug(`ERROR: layer query '${layerSql}' returned no data`); + throw new Error('Layer query returned no data'); + } + + if (!Object.prototype.hasOwnProperty.call(columns.fields, attrs.column)) { + debug(`ERROR: layer query ${layerSql} does not include time-attribute column '${attrs.column}'`); + throw new Error(`Layer query did not return the requested time-attribute column '${attrs.column}'`); } - callback(null, Object.assign(attrs, layerAttrs)); - }); + // get time bounds to calculate step + const isTime = columns.fields[attrs.column].type === 'date'; + const stepQuery = getAttributesStepQuery(layerSql, attrs.column, isTime); + + const { rows } = await psql.query(stepQuery); + const data = rows[0]; + + let attributeStep = (data.max_date - data.min_date + 1) / Math.min(attrs.steps, data.num_steps >> 0); + attributeStep = Math.abs(attributeStep) === Infinity ? 0 : attributeStep; + + const attributes = { + start: data.min_date, + end: data.max_date, + step: attributeStep || 1, + data_steps: data.num_steps >> 0, + is_time: isTime + }; + + return Object.assign(attrs, attributes); + } catch (err) { + err.message = `TorqueRenderer: ${err.message}`; + throw err; + } } // check layer and raise an exception is some error is found @@ -217,55 +223,6 @@ function getMapProperties (definitions, env) { }, {}); } -function fetchLayerAttributes (pg, layerSql, attrs, callback) { - layerSql = SubstitutionTokens.replaceXYZ(layerSql, { x: 0, y: 0, z: 0 }); - - // first step, fetch the schema to know if torque colum is time column - const columnsQuery = `select * from (${layerSql}) __torque_wrap_sql limit 0`; - pg(columnsQuery, (err, data) => { - // second step, get time bounds to calculate step - if (err) { - err.message = 'TorqueRenderer: ' + err.message; - return callback(err); - } - - if (!data) { - debug(`ERROR: layer query '${layerSql}' returned no data`); - return callback(new Error('TorqueRenderer: Layer query returned no data')); - } - - if (!Object.prototype.hasOwnProperty.call(data.fields, attrs.column)) { - debug(`ERROR: layer query ${layerSql} does not include time-attribute column '${attrs.column}'`); - return callback(new Error(`TorqueRenderer: Layer query did not return the requested time-attribute column '${attrs.column}'`)); - } - - const isTime = data.fields[attrs.column].type === 'date'; - const stepQuery = getAttributesStepQuery(layerSql, attrs.column, isTime); - - pg(stepQuery, (err, data) => { - if (err) { - err.message = 'TorqueRenderer: ' + err.message; - return callback(err); - } - - data = data.rows[0]; - - let attributeStep = (data.max_date - data.min_date + 1) / Math.min(attrs.steps, data.num_steps >> 0); - attributeStep = Math.abs(attributeStep) === Infinity ? 0 : attributeStep; - - const attributes = { - start: data.min_date, - end: data.max_date, - step: attributeStep || 1, - data_steps: data.num_steps >> 0, - is_time: isTime - }; - - callback(null, attributes); - }); - }); -} - function getAttributesStepQuery (layerSql, column, isTime) { let maxCol = `max(${column})`; let minCol = `min(${column})`; @@ -275,8 +232,5 @@ function getAttributesStepQuery (layerSql, column, isTime) { minCol = `date_part('epoch', ${minCol})`; } - return ` - SELECT count(*) as num_steps, ${maxCol} max_date, ${minCol} min_date - FROM (${layerSql}) __torque_wrap_sql - `; + return `SELECT count(*) as num_steps, ${maxCol} max_date, ${minCol} min_date FROM (${layerSql}) __torque_wrap_sql`; } diff --git a/lib/renderers/torque/png-renderer.js b/lib/renderers/torque/png-renderer.js index bd5bd5b05..2544c78e0 100644 --- a/lib/renderers/torque/png-renderer.js +++ b/lib/renderers/torque/png-renderer.js @@ -9,13 +9,13 @@ const Timer = require('../../stats/timer'); const { promisify } = require('util'); module.exports = class TorquePngRenderer extends TorqueRenderer { - constructor (layer, sql, attrs) { + constructor (layer, psql, attrs) { const cartoCssOptions = torque.common.TorqueLayer.optionsFromCartoCSS(layer.options.cartocss); const rendererOptions = { bufferSize: cartoCssOptions['buffer-size'] !== undefined ? cartoCssOptions['buffer-size'] : 32 }; - super(layer, sql, attrs, rendererOptions); + super(layer, psql, attrs, rendererOptions); this.canvasImages = []; const self = this; @@ -77,7 +77,7 @@ module.exports = class TorquePngRenderer extends TorqueRenderer { const timer = new Timer(); const attrs = Object.assign({ stepSelect: this.step, stepOffset: this.stepOffset }, this.attrs); - const { buffer: rows, stats } = await this.getTileData(this.sql, { x: x, y: y }, z, this.layer.options.sql, attrs); + const { buffer: rows, stats } = await this.getTileData(this.psql, { x: x, y: y }, z, this.layer.options.sql, attrs); timer.start('render'); const canvas = createCanvas(this.tile_size, this.tile_size); diff --git a/lib/renderers/torque/psql-adaptor.js b/lib/renderers/torque/psql-adaptor.js index e4ad583e4..84e127c68 100644 --- a/lib/renderers/torque/psql-adaptor.js +++ b/lib/renderers/torque/psql-adaptor.js @@ -1,42 +1,34 @@ 'use strict'; -function PSQLAdaptor (PSQLClass, poolParams) { - var ctor = function (params) { - this._psql = new PSQLClass(params, poolParams); - }; - ctor.prototype.query = function (sql, callback, readonly) { - this._psql.query(sql, this._handleResult.bind(this, callback), readonly); - }; - ctor.prototype._handleResult = function (callback, err, result) { - if (err) { callback(err); return; } - var formatted = { - fields: this._formatResultFields(result.fields), - rows: result.rows - }; - callback(null, formatted); - }; - ctor.prototype._formatResultFields = function (flds) { - var nfields = {}; - for (var i = 0; i < flds.length; ++i) { - var f = flds[i]; - var cname = this._psql.typeName(f.dataTypeID); - var tname; - if (!cname) { - tname = 'unknown(' + f.dataTypeID + ')'; - } else { - tname = typeName(cname); - } - // console.log('cname:'+cname+' tname:'+tname); - nfields[f.name] = { type: tname }; - } - return nfields; - }; - - return ctor; -} +const Psql = require('cartodb-psql'); + +module.exports = class PSQLAdaptor { + constructor ({ connectionParams, poolParams }) { + this._psql = new Psql(connectionParams, poolParams); + } + + query (sql, readonly = true) { + return new Promise((resolve, reject) => { + this._psql.query(sql, (err, result) => { + if (err) { + return reject(err); + } + + const fields = result.fields.reduce((fields, field) => { + const cname = this._psql.typeName(field.dataTypeID); + const type = cname ? typeName(cname) : `unknown(${field.dataTypeID})`; + fields[field.name] = { type }; + return fields; + }, {}); + + return resolve({ fields, rows: result.rows }); + }, readonly); + }); + } +}; function typeName (cname) { - var tname = cname; + let tname = cname; if (cname.match('bool')) { tname = 'boolean'; @@ -54,5 +46,3 @@ function typeName (cname) { return tname; } - -module.exports = PSQLAdaptor; diff --git a/lib/renderers/torque/renderer.js b/lib/renderers/torque/renderer.js index 5b043aa3f..d396b6aa5 100644 --- a/lib/renderers/torque/renderer.js +++ b/lib/renderers/torque/renderer.js @@ -4,13 +4,12 @@ const format = require('../../utils/format'); const Timer = require('../../stats/timer'); const debug = require('debug')('windshaft:renderer:torque'); const SubstitutionTokens = require('cartodb-query-tables').utils.substitutionTokens; -const { promisify } = require('util'); module.exports = class TorqueRenderer { - constructor (layer, sql, attrs, options) { + constructor (layer, psql, attrs, options) { options = options || {}; - this.sql = promisify(sql); + this.psql = psql; this.attrs = attrs; this.layer = layer; @@ -21,7 +20,7 @@ module.exports = class TorqueRenderer { } async getTile (format, z, x, y) { - const { buffer, headers, stats } = await this.getTileData(this.sql, { x: x, y: y }, z, this.layer.options.sql, this.attrs); + const { buffer, headers, stats } = await this.getTileData(this.psql, { x: x, y: y }, z, this.layer.options.sql, this.attrs); return { buffer, headers, stats }; } @@ -47,7 +46,7 @@ module.exports = class TorqueRenderer { return meta; } - async getTileData (sql, coord, zoom, layerSql, attrs) { + async getTileData (psql, coord, zoom, layerSql, attrs) { let columnConv = attrs.column; if (attrs.is_time) { @@ -119,7 +118,7 @@ module.exports = class TorqueRenderer { const timer = new Timer(); timer.start('query'); - const data = await sql(query); + const data = await psql.query(query); timer.end('query'); return { buffer: data.rows, headers: { 'Content-Type': 'application/json' }, stats: timer.getTimes() }; diff --git a/lib/utils/sql.js b/lib/utils/sql.js deleted file mode 100644 index 718826250..000000000 --- a/lib/utils/sql.js +++ /dev/null @@ -1,19 +0,0 @@ -'use strict'; - -var debug = require('debug')('windshaft:utils'); - -function sql (dbParams, SQLClass) { - // TODO: cache class instances by dbParams/sqlClass - return function (query, callback) { - var pg; - try { - debug('Running query %s with params %s', query, dbParams); - pg = new SQLClass(dbParams); - pg.query(query, callback, true); // ensure read-only transaction - } catch (err) { - callback(err); - } - }; -} - -module.exports = sql; diff --git a/test/acceptance/torque-test.js b/test/acceptance/torque-test.js index e024d3cf5..b35452405 100644 --- a/test/acceptance/torque-test.js +++ b/test/acceptance/torque-test.js @@ -27,7 +27,7 @@ describe('torque', function () { var testClient = new TestClient(createTorqueMapConfig('Map { marker-fill:blue; }')); testClient.createLayergroup(function (err) { assert.ok(err); - assert.equal(err.message, "Missing required property '-torque-frame-count' in torque layer CartoCSS"); + assert.equal(err.message, "TorqueRenderer: Missing required property '-torque-frame-count' in torque layer CartoCSS"); done(); }); }); @@ -36,7 +36,7 @@ describe('torque', function () { var testClient = new TestClient(createTorqueMapConfig('Map { -torque-frame-count: 2; }')); testClient.createLayergroup(function (err) { assert.ok(err); - assert.equal(err.message, "Missing required property '-torque-resolution' in torque layer CartoCSS"); + assert.equal(err.message, "TorqueRenderer: Missing required property '-torque-resolution' in torque layer CartoCSS"); done(); }); }); @@ -49,7 +49,7 @@ describe('torque', function () { assert.ok(err); assert.equal( err.message, - "Missing required property '-torque-aggregation-function' in torque layer CartoCSS" + "TorqueRenderer: Missing required property '-torque-aggregation-function' in torque layer CartoCSS" ); done(); }); @@ -75,7 +75,7 @@ describe('torque', function () { var testClient = new TestClient(layergroup); testClient.createLayergroup(function (err) { assert.ok(err); - assert.equal(err.message, "Unexpected type for property '-torque-aggregation-function', expected string"); + assert.equal(err.message, "TorqueRenderer: Unexpected type for property '-torque-aggregation-function', expected string"); done(); }); }); @@ -250,7 +250,7 @@ describe('torque', function () { var testClient = new TestClient(layergroup); testClient.createLayergroup(function (err) { assert.ok(err); - assert.equal(err.message, "Unexpected type for property '-torque-aggregation-function', expected string"); + assert.equal(err.message, "TorqueRenderer: Unexpected type for property '-torque-aggregation-function', expected string"); done(); }); }); diff --git a/test/unit/torque-test.js b/test/unit/torque-test.js index 0c5760338..ac7a5f88f 100644 --- a/test/unit/torque-test.js +++ b/test/unit/torque-test.js @@ -5,20 +5,18 @@ require('../support/test-helper'); var assert = require('assert'); var TorqueFactory = require('../../lib/renderers/torque/factory'); var MapConfig = require('../../lib/models/mapconfig'); +const PSQLAdaptor = require('../../lib/renderers/torque/psql-adaptor'); -function dummyPSQL () { - PSQLDummy.n = Date.now(); - function PSQLDummy () { - this.query = function (sql, callback) { - var res = PSQLDummy.responses[PSQLDummy.queries.length]; - // console.log("* ", PSQLDummy.n, sql, " => ", res); - PSQLDummy.queries.push(sql); - callback.apply(module, res); - }; - } - PSQLDummy.queries = []; - PSQLDummy.responses = []; - return PSQLDummy; +function mockPSQLAdaptorQuery ({ columnsQueryResult, stepQueryResult, tileQueryResult = {} }) { + PSQLAdaptor.prototype.query = async function (sql) { + if (sql.endsWith('__torque_wrap_sql limit 0')) { + return columnsQueryResult; + } else if (sql.startsWith('SELECT count(*) as num_steps')) { + return stepQueryResult; + } else { + return tileQueryResult; + } + }; } describe('torque', function () { @@ -72,33 +70,38 @@ describe('torque', function () { var mapConfig = MapConfig.create(layergroupConfig()); - var sqlApi = null; var torque = null; + beforeEach(function () { - sqlApi = dummyPSQL(); - torque = new TorqueFactory({ - sqlClass: sqlApi - }); + torque = new TorqueFactory(); + this.originalPSQLAdaptorQueryMethod = PSQLAdaptor.prototype.query; + }); + + afterEach(function () { + PSQLAdaptor.prototype.query = this.originalPSQLAdaptorQueryMethod; }); function rendererOptions (layer) { return { - params: {}, + params: { + dbname: 'windshaft_test' + }, layer: layer }; } + var layerZeroOptions = rendererOptions(0); describe('getRenderer', function () { it('should create a renderer with right parmas', function (done) { - sqlApi.responses = [ - [null, { fields: { date: { type: 'date' } } }], - [null, { + mockPSQLAdaptorQuery({ + columnsQueryResult: { fields: { date: { type: 'date' } } }, + stepQueryResult: { rows: [ { min_date: 0, max_date: 10, num_steps: 1, xmin: 0, xmax: 10, ymin: 0, ymax: 10 } ] - }] - ]; + } + }); torque.getRenderer(mapConfig, 'json.torque', layerZeroOptions, function (err, renderer) { assert.ifError(err); assert.ok(!!renderer); @@ -108,14 +111,14 @@ describe('torque', function () { }); it('should raise an error on missing -torque-frame-count', function (done) { - sqlApi.responses = [ - [null, { fields: { date: { type: 'date' } } }], - [null, { + mockPSQLAdaptorQuery({ + columnsQueryResult: { fields: { date: { type: 'date' } } }, + stepQueryResult: { rows: [ { min_date: 0, max_date: 10, num_steps: 1, xmin: 0, xmax: 10, ymin: 0, ymax: 10 } ] - }] - ]; + } + }); var brokenConfig = MapConfig.create(layergroupConfig(makeCartoCss( [ '-torque-time-attribute: "date";', @@ -127,20 +130,20 @@ describe('torque', function () { torque.getRenderer(brokenConfig, 'json.torque', layerZeroOptions, function (err/*, renderer */) { assert.ok(err !== null); assert.ok(err instanceof Error); - assert.equal(err.message, "Missing required property '-torque-frame-count' in torque layer CartoCSS"); + assert.equal(err.message, "TorqueRenderer: Missing required property '-torque-frame-count' in torque layer CartoCSS"); done(); }); }); it('should raise an error on missing -torque-resolution', function (done) { - sqlApi.responses = [ - [null, { fields: { date: { type: 'date' } } }], - [null, { + mockPSQLAdaptorQuery({ + columnsQueryResult: { fields: { date: { type: 'date' } } }, + stepQueryResult: { rows: [ { min_date: 0, max_date: 10, num_steps: 1, xmin: 0, xmax: 10, ymin: 0, ymax: 10 } ] - }] - ]; + } + }); var brokenConfig = MapConfig.create(layergroupConfig(makeCartoCss( [ '-torque-time-attribute: "date";', @@ -152,20 +155,20 @@ describe('torque', function () { torque.getRenderer(brokenConfig, 'json.torque', layerZeroOptions, function (err/*, renderer */) { assert.ok(err !== null); assert.ok(err instanceof Error); - assert.equal(err.message, "Missing required property '-torque-resolution' in torque layer CartoCSS"); + assert.equal(err.message, "TorqueRenderer: Missing required property '-torque-resolution' in torque layer CartoCSS"); done(); }); }); it('should raise an error on missing -torque-time-attribute', function (done) { - sqlApi.responses = [ - [null, { fields: { date: { type: 'date' } } }], - [null, { + mockPSQLAdaptorQuery({ + columnsQueryResult: { fields: { date: { type: 'date' } } }, + stepQueryResult: { rows: [ { min_date: 0, max_date: 10, num_steps: 1, xmin: 0, xmax: 10, ymin: 0, ymax: 10 } ] - }] - ]; + } + }); var brokenConfig = MapConfig.create(layergroupConfig(makeCartoCss( [ '-torque-aggregation-function: "count(cartodb_id)";', @@ -177,7 +180,7 @@ describe('torque', function () { torque.getRenderer(brokenConfig, 'json.torque', layerZeroOptions, function (err/*, renderer */) { assert.ok(err !== null); assert.ok(err instanceof Error); - assert.equal(err.message, "Missing required property '-torque-time-attribute' in torque layer CartoCSS"); + assert.equal(err.message, "TorqueRenderer: Missing required property '-torque-time-attribute' in torque layer CartoCSS"); done(); }); }); @@ -211,14 +214,14 @@ describe('torque', function () { describe('Renderer', function () { it('should get metadata', function (done) { - sqlApi.responses = [ - [null, { fields: { date: { type: 'date' } } }], - [null, { + mockPSQLAdaptorQuery({ + columnsQueryResult: { fields: { date: { type: 'date' } } }, + stepQueryResult: { rows: [ { min_date: 0, max_date: 10, num_steps: 1, xmin: 0, xmax: 10, ymin: 0, ymax: 10 } ] - }] - ]; + } + }); torque.getRenderer(mapConfig, 'json.torque', layerZeroOptions, function (err, renderer) { assert.ok(err === null); renderer.getMetadata() @@ -233,19 +236,22 @@ describe('torque', function () { }); }); it('should get a tile', function (done) { - sqlApi.responses = [ - [null, { fields: { date: { type: 'date' } } }], - [null, { + mockPSQLAdaptorQuery({ + columnsQueryResult: { + fields: { date: { type: 'date' } } + }, + stepQueryResult: { rows: [ { min_date: 0, max_date: 10, num_steps: 1, xmin: 0, xmax: 10, ymin: 0, ymax: 10 } ] - }], - [null, { + }, + tileQueryResult: { rows: [ { x__uint8: 0, y__uint8: 0, vals__uint8: [0, 1, 2], dates__uint16: [4, 5, 6] } ] - }] - ]; + } + }); + torque.getRenderer(mapConfig, 'json.torque', layerZeroOptions, function (err, renderer) { assert.ifError(err); renderer.getTile('json.torque', 0, 0, 0) @@ -284,10 +290,11 @@ describe('torque', function () { }; var mapConfig = MapConfig.create(layergroup); - sqlApi.responses = [ - [null, { fields: { updated_at: { type: 'date' } } }], - [null, { rows: [{ num_steps: 0, max_date: null, min_date: null }] }] - ]; + mockPSQLAdaptorQuery({ + columnsQueryResult: { fields: { updated_at: { type: 'date' } } }, + stepQueryResult: { rows: [{ num_steps: 0, max_date: null, min_date: null }] } + }); + torque.getRenderer(mapConfig, 'json.torque', layerZeroOptions, function (err, renderer) { assert.ifError(err); assert.equal(renderer.attrs.step, 1, 'Number of steps cannot be Infinity'); From c3defd1eb771e004f4302f25333b91d11025e944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sun, 12 Apr 2020 18:13:08 +0200 Subject: [PATCH 29/45] Use ES6 class syntax --- lib/renderers/mapnik/factory.js | 581 ++++++++++++++++---------------- 1 file changed, 291 insertions(+), 290 deletions(-) diff --git a/lib/renderers/mapnik/factory.js b/lib/renderers/mapnik/factory.js index 05eadf982..db7d03222 100644 --- a/lib/renderers/mapnik/factory.js +++ b/lib/renderers/mapnik/factory.js @@ -3,7 +3,6 @@ const { rendererFactory: mapnikRendererFactory } = require('@carto/cartonik'); const mapnik = require('@carto/mapnik'); const grainstore = require('grainstore'); - const layersFilter = require('../../utils/layer-filter'); const MapnikAdaptor = require('./adaptor'); const DefaultQueryRewriter = require('../../utils/default-query-rewriter'); @@ -14,346 +13,348 @@ const COLUMN_TYPE_DEFAULT = COLUMN_TYPE_GEOMETRY; const DEFAULT_TILE_SIZE = 256; const FORMAT_MVT = 'mvt'; -function MapnikFactory (options) { - this.supportedFormats = { - png: true, - png32: true, - 'grid.json': true, - mvt: true - }; - - this._options = options; - - // Set default mapnik options - this._mapnik_opts = Object.assign({}, { - geometry_field: 'the_geom_webmercator', - - // Metatile is the number of tiles-per-side that are going - // to be rendered at once. If all of them will be requested - // we'd have saved time. If only one will be used, we'd have - // wasted time. - // - // Defaults to 2 as of tilelive-mapnik@0.3.2 - // - // We'll assume an average of a 4x4 viewport - metatile: 4, - - // tilelive-mapnik uses an internal cache to store tiles/grids - // generated when using metatile. This options allow to tune - // the behaviour for that internal cache. - metatileCache: { - // Time an object must stay in the cache until is removed - ttl: 0, - // Whether an object must be removed after the first hit - // Usually you want to use `true` here when ttl>0. - deleteOnHit: false - }, - - // Override metatile behaviour depending on the format - formatMetatile: {}, - - // Buffer size is the tickness in pixel of a buffer - // around the rendered (meta?)tile. - // - // This is important for labels and other marker that overlap tile boundaries. - // Setting to 128 ensures no render artifacts. - // 64 may have artifacts but is faster. - // Less important if we can turn metatiling on. - // - // defaults to 128 as of tilelive-mapnik@0.3.2 - // - bufferSize: 64, - - // Buffer size behaviour depending on the format - formatBufferSize: {}, - - // retina support, which scale factors are supported - scale_factors: [1, 2], - - limits: { - // Time in milliseconds a render request can take before it fails, some notes: - // - 0 means no render limit - // - it considers metatiling, it naive implementation: (render timeout) * (number of tiles in metatile) - render: 0, - // As the render request will finish even if timed out, whether it should be placed in the internal - // cache or it should be fully discarded. When placed in the internal cache another attempt to retrieve - // the same tile will result in an immediate response, however that will use a lot of more application - // memory. If we want to enforce this behaviour we have to implement a cache eviction policy for the - // internal cache. - cacheOnTimeout: true - }, - - // A query-rewriter can be passed to preprocess SQL queries - // before passing them to Mapnik. - // The query rewriter should contain one function: - // - `query(sql, data)` to transform queries using the optional data provided - // The data passed to this function can be provided for eaach layer - // through a `query_rewrite_data` entry in the layer options. - // By default a dummy query rewriter which doesn't alter queries is used. - queryRewriter: new DefaultQueryRewriter(), - - // If enabled Mapnik will reuse the features retrieved from the database - // instead of requesting them once per style inside a layer - 'cache-features': true, - - // Require stats per query to the renderer - metrics: false, - - // Options for markers attributes, ellipses and images caches - markers_symbolizer_caches: { - disabled: false - }, - - // INTERNAL: Render time variables - variables: {} - }, options.mapnik || {}); - - this._options.grainstore = this._options.grainstore ? this._options.grainstore : {}; - this._options.grainstore.mapnik_version = this._options.grainstore.mapnik_version - ? this._options.grainstore.mapnik_version - : mapnik.versions.mapnik; - - bootstrapFonts(this._options.grainstore); - - this.tile_scale_factors = this._mapnik_opts.scale_factors.reduce(function (previousValue, currentValue) { - previousValue[currentValue] = DEFAULT_TILE_SIZE * currentValue; - return previousValue; - }, {}); -} - -function bootstrapFonts (grainstoreOptions) { - // Set carto renderer configuration for MMLStore - grainstoreOptions.carto_env = grainstoreOptions.carto_env || {}; - const cenv = grainstoreOptions.carto_env; - - cenv.validation_data = cenv.validation_data || {}; - if (!cenv.validation_data.fonts) { - mapnik.register_system_fonts(); - mapnik.register_default_fonts(); - cenv.validation_data.fonts = Object.keys(mapnik.fontFiles()); +module.exports = class MapnikFactory { + static get NAME () { + return 'mapnik'; } -} - -module.exports = MapnikFactory; -const NAME = 'mapnik'; -module.exports.NAME = NAME; - -MapnikFactory.prototype.getName = function () { - return NAME; -}; - -MapnikFactory.prototype.supportsFormat = function (format) { - return !!this.supportedFormats[format]; -}; -MapnikFactory.prototype.getAdaptor = function (renderer, onTileErrorStrategy) { - return new MapnikAdaptor(renderer, onTileErrorStrategy); -}; + constructor (options) { + this.supportedFormats = { + png: true, + png32: true, + 'grid.json': true, + mvt: true + }; + + this._options = options; + + // Set default mapnik options + this._mapnik_opts = Object.assign({}, { + geometry_field: 'the_geom_webmercator', + + // Metatile is the number of tiles-per-side that are going + // to be rendered at once. If all of them will be requested + // we'd have saved time. If only one will be used, we'd have + // wasted time. + // + // Defaults to 2 as of tilelive-mapnik@0.3.2 + // + // We'll assume an average of a 4x4 viewport + metatile: 4, + + // tilelive-mapnik uses an internal cache to store tiles/grids + // generated when using metatile. This options allow to tune + // the behaviour for that internal cache. + metatileCache: { + // Time an object must stay in the cache until is removed + ttl: 0, + // Whether an object must be removed after the first hit + // Usually you want to use `true` here when ttl>0. + deleteOnHit: false + }, + + // Override metatile behaviour depending on the format + formatMetatile: {}, + + // Buffer size is the tickness in pixel of a buffer + // around the rendered (meta?)tile. + // + // This is important for labels and other marker that overlap tile boundaries. + // Setting to 128 ensures no render artifacts. + // 64 may have artifacts but is faster. + // Less important if we can turn metatiling on. + // + // defaults to 128 as of tilelive-mapnik@0.3.2 + // + bufferSize: 64, + + // Buffer size behaviour depending on the format + formatBufferSize: {}, + + // retina support, which scale factors are supported + scale_factors: [1, 2], + + limits: { + // Time in milliseconds a render request can take before it fails, some notes: + // - 0 means no render limit + // - it considers metatiling, it naive implementation: (render timeout) * (number of tiles in metatile) + render: 0, + // As the render request will finish even if timed out, whether it should be placed in the internal + // cache or it should be fully discarded. When placed in the internal cache another attempt to retrieve + // the same tile will result in an immediate response, however that will use a lot of more application + // memory. If we want to enforce this behaviour we have to implement a cache eviction policy for the + // internal cache. + cacheOnTimeout: true + }, + + // A query-rewriter can be passed to preprocess SQL queries + // before passing them to Mapnik. + // The query rewriter should contain one function: + // - `query(sql, data)` to transform queries using the optional data provided + // The data passed to this function can be provided for eaach layer + // through a `query_rewrite_data` entry in the layer options. + // By default a dummy query rewriter which doesn't alter queries is used. + queryRewriter: new DefaultQueryRewriter(), + + // If enabled Mapnik will reuse the features retrieved from the database + // instead of requesting them once per style inside a layer + 'cache-features': true, + + // Require stats per query to the renderer + metrics: false, + + // Options for markers attributes, ellipses and images caches + markers_symbolizer_caches: { + disabled: false + }, + + // INTERNAL: Render time variables + variables: {} + }, options.mapnik || {}); + + this._options.grainstore = this._options.grainstore ? this._options.grainstore : {}; + this._options.grainstore.mapnik_version = this._options.grainstore.mapnik_version + ? this._options.grainstore.mapnik_version + : mapnik.versions.mapnik; + + bootstrapFonts(this._options.grainstore); + + this.tile_scale_factors = this._mapnik_opts.scale_factors.reduce(function (previousValue, currentValue) { + previousValue[currentValue] = DEFAULT_TILE_SIZE * currentValue; + return previousValue; + }, {}); + } -MapnikFactory.prototype.defineExpectedParams = function (params) { - if (params['cache-features'] === undefined) { - params['cache-features'] = this._mapnik_opts['cache-features']; + getName () { + return MapnikFactory.NAME; } - if (params.metrics === undefined) { - params.metrics = this._mapnik_opts.metrics; + supportsFormat (format) { + return !!this.supportedFormats[format]; } - if (params.markers_symbolizer_caches === undefined) { - params.markers_symbolizer_caches = this._mapnik_opts.markers_symbolizer_caches; + getAdaptor (renderer, onTileErrorStrategy) { + return new MapnikAdaptor(renderer, onTileErrorStrategy); } -}; -MapnikFactory.prototype.getRenderer = function (mapConfig, format, options, callback) { - const isMvt = format === FORMAT_MVT; + defineExpectedParams (params) { + if (params['cache-features'] === undefined) { + params['cache-features'] = this._mapnik_opts['cache-features']; + } + + if (params.metrics === undefined) { + params.metrics = this._mapnik_opts.metrics; + } - if (mapConfig.isVectorOnlyMapConfig() && !isMvt) { - const error = new Error(`Unsupported format: 'cartocss' option is missing for ${format}`); - error.http_status = 400; - error.type = 'tile'; - return callback(error); + if (params.markers_symbolizer_caches === undefined) { + params.markers_symbolizer_caches = this._mapnik_opts.markers_symbolizer_caches; + } } - this.defineExpectedParams(options.params); + getRenderer (mapConfig, format, options, callback) { + const isMvt = format === FORMAT_MVT; - let mmlBuilderConfig; - try { - mmlBuilderConfig = this.mapConfigToMMLBuilderConfig(mapConfig, this._mapnik_opts.queryRewriter, options); - } catch (error) { - return callback(error); - } + if (mapConfig.isVectorOnlyMapConfig() && !isMvt) { + const error = new Error(`Unsupported format: 'cartocss' option is missing for ${format}`); + error.http_status = 400; + error.type = 'tile'; + return callback(error); + } - const params = Object.assign({}, options.params, mmlBuilderConfig); - const limits = Object.assign({}, this._mapnik_opts.limits, options.limits); - const variables = Object.assign({}, this._mapnik_opts.variables, options.variables); + this.defineExpectedParams(options.params); - // fix layer index - // see https://github.com/CartoDB/Windshaft/blob/0.43.0/lib/backends/map_validator.js#L69-L81 - if (params.layer) { - params.layer = mapConfig.getLayerIndexByType('mapnik', params.layer); - } + let mmlBuilderConfig; + try { + mmlBuilderConfig = this.mapConfigToMMLBuilderConfig(mapConfig, this._mapnik_opts.queryRewriter, options); + } catch (error) { + return callback(error); + } - const scaleFactor = params.scale_factor === undefined ? 1 : +params.scale_factor; - const tileSize = this.tile_scale_factors[scaleFactor]; + const params = Object.assign({}, options.params, mmlBuilderConfig); + const limits = Object.assign({}, this._mapnik_opts.limits, options.limits); + const variables = Object.assign({}, this._mapnik_opts.variables, options.variables); - if (!tileSize) { - var err = new Error('Tile with specified resolution not found'); - err.http_status = 404; - return callback(err); - } + // fix layer index + // see https://github.com/CartoDB/Windshaft/blob/0.43.0/lib/backends/map_validator.js#L69-L81 + if (params.layer) { + params.layer = mapConfig.getLayerIndexByType('mapnik', params.layer); + } - let mmlBuilder; - try { - mmlBuilder = getMMLBuilder(mapConfig, this._options.grainstore, params, format); - } catch (error) { - return callback(error); - } + const scaleFactor = params.scale_factor === undefined ? 1 : +params.scale_factor; + const tileSize = this.tile_scale_factors[scaleFactor]; - mmlBuilder.toXML((err, xml) => { - if (err) { + if (!tileSize) { + var err = new Error('Tile with specified resolution not found'); + err.http_status = 404; return callback(err); } + let mmlBuilder; try { - const renderer = mapnikRendererFactory({ - type: isMvt ? 'vector' : 'raster', - xml: xml, - strict: !!params.strict, - bufferSize: this.getBufferSize(mapConfig, format), - poolSize: this._mapnik_opts.poolSize, - poolMaxWaitingClients: this._mapnik_opts.poolMaxWaitingClients, - tileSize: tileSize, - limits: limits, - metrics: options.params.metrics, - variables: variables, - metatile: this.getMetatile(format), // when raster renderer - metatileCache: this._mapnik_opts.metatileCache, // when raster renderer - scale: scaleFactor, // when raster renderer - gzip: false // when vector renderer - }); + mmlBuilder = getMMLBuilder(mapConfig, this._options.grainstore, params, format); + } catch (error) { + return callback(error); + } - return callback(null, renderer); - } catch (err) { - return callback(err); + mmlBuilder.toXML((err, xml) => { + if (err) { + return callback(err); + } + + try { + const renderer = mapnikRendererFactory({ + type: isMvt ? 'vector' : 'raster', + xml: xml, + strict: !!params.strict, + bufferSize: this.getBufferSize(mapConfig, format), + poolSize: this._mapnik_opts.poolSize, + poolMaxWaitingClients: this._mapnik_opts.poolMaxWaitingClients, + tileSize: tileSize, + limits: limits, + metrics: options.params.metrics, + variables: variables, + metatile: this.getMetatile(format), // when raster renderer + metatileCache: this._mapnik_opts.metatileCache, // when raster renderer + scale: scaleFactor, // when raster renderer + gzip: false // when vector renderer + }); + + return callback(null, renderer); + } catch (err) { + return callback(err); + } + }); + }; + + getMetatile (format) { + var metatile = this._mapnik_opts.metatile; + if (Number.isFinite(this._mapnik_opts.formatMetatile[format])) { + metatile = this._mapnik_opts.formatMetatile[format]; } - }); -}; + return metatile; + } -MapnikFactory.prototype.getMetatile = function (format) { - var metatile = this._mapnik_opts.metatile; - if (Number.isFinite(this._mapnik_opts.formatMetatile[format])) { - metatile = this._mapnik_opts.formatMetatile[format]; + getBufferSizeFromOptions (format, options) { + return options && options.formatBufferSize && Number.isFinite(options.formatBufferSize[format]) + ? options.formatBufferSize[format] + : options.bufferSize; } - return metatile; -}; -MapnikFactory.prototype.getBufferSizeFromOptions = function (format, options) { - return options && options.formatBufferSize && Number.isFinite(options.formatBufferSize[format]) - ? options.formatBufferSize[format] - : options.bufferSize; -}; + getBufferSize (mapConfig, format, options) { + options = options || this._mapnik_opts; -MapnikFactory.prototype.getBufferSize = function (mapConfig, format, options) { - options = options || this._mapnik_opts; + if (Number.isFinite(mapConfig.getBufferSize(format))) { + return mapConfig.getBufferSize(format); + } - if (Number.isFinite(mapConfig.getBufferSize(format))) { - return mapConfig.getBufferSize(format); + return this.getBufferSizeFromOptions(format, options); } - return this.getBufferSizeFromOptions(format, options); -}; + mapConfigToMMLBuilderConfig (mapConfig, queryRewriter, rendererOptions) { + var self = this; + var options = { + ids: [], + sql: [], + originalSql: [], + style: [], + style_version: [], + zooms: [], + interactivity: [], + ttl: 0, + datasource_extend: [], + extra_ds_opts: [], + gcols: [], + 'cache-features': rendererOptions.params['cache-features'] + }; -MapnikFactory.prototype.mapConfigToMMLBuilderConfig = function (mapConfig, queryRewriter, rendererOptions) { - var self = this; - var options = { - ids: [], - sql: [], - originalSql: [], - style: [], - style_version: [], - zooms: [], - interactivity: [], - ttl: 0, - datasource_extend: [], - extra_ds_opts: [], - gcols: [], - 'cache-features': rendererOptions.params['cache-features'] - }; + var layerFilter = rendererOptions.layer; - var layerFilter = rendererOptions.layer; + var filteredLayerIndexes = layersFilter(mapConfig, layerFilter); + filteredLayerIndexes.reduce(function (options, layerIndex) { + var layer = mapConfig.getLayer(layerIndex); - var filteredLayerIndexes = layersFilter(mapConfig, layerFilter); - filteredLayerIndexes.reduce(function (options, layerIndex) { - var layer = mapConfig.getLayer(layerIndex); + validateLayer(mapConfig, layerIndex); - validateLayer(mapConfig, layerIndex); + options.ids.push(mapConfig.getLayerId(layerIndex)); + var lyropt = layer.options; - options.ids.push(mapConfig.getLayerId(layerIndex)); - var lyropt = layer.options; + if (lyropt.cartocss !== undefined && lyropt.cartocss_version !== undefined) { + options.style.push(lyropt.cartocss); + options.style_version.push(lyropt.cartocss_version); + } - if (lyropt.cartocss !== undefined && lyropt.cartocss_version !== undefined) { - options.style.push(lyropt.cartocss); - options.style_version.push(lyropt.cartocss_version); - } + var query = queryRewriter.query(lyropt.sql, lyropt.query_rewrite_data); + var queryOptions = prepareQuery(query, lyropt.geom_column, lyropt.geom_type, self._mapnik_opts); - var query = queryRewriter.query(lyropt.sql, lyropt.query_rewrite_data); - var queryOptions = prepareQuery(query, lyropt.geom_column, lyropt.geom_type, self._mapnik_opts); + options.sql.push(queryOptions.sql); + options.originalSql.push(query); - options.sql.push(queryOptions.sql); - options.originalSql.push(query); + var zoom = {}; + if (Number.isFinite(lyropt.minzoom)) { + zoom.minzoom = lyropt.minzoom; + } + if (Number.isFinite(lyropt.maxzoom)) { + zoom.maxzoom = lyropt.maxzoom; + } + options.zooms.push(zoom); - var zoom = {}; - if (Number.isFinite(lyropt.minzoom)) { - zoom.minzoom = lyropt.minzoom; - } - if (Number.isFinite(lyropt.maxzoom)) { - zoom.maxzoom = lyropt.maxzoom; - } - options.zooms.push(zoom); + options.gcols.push({ + type: queryOptions.geomColumnType, // grainstore allows undefined here + name: queryOptions.geomColumnName + }); - options.gcols.push({ - type: queryOptions.geomColumnType, // grainstore allows undefined here - name: queryOptions.geomColumnName - }); + var extraOpt = {}; + if (Object.prototype.hasOwnProperty.call(lyropt, 'raster_band')) { + extraOpt.band = lyropt.raster_band; + } + options.datasource_extend.push(mapConfig.getLayerDatasource(layerIndex)); + options.extra_ds_opts.push(extraOpt); - var extraOpt = {}; - if (Object.prototype.hasOwnProperty.call(lyropt, 'raster_band')) { - extraOpt.band = lyropt.raster_band; + return options; + }, options); + + if (!options.sql.length) { + throw new Error("No 'mapnik' layers in MapConfig"); } - options.datasource_extend.push(mapConfig.getLayerDatasource(layerIndex)); - options.extra_ds_opts.push(extraOpt); - return options; - }, options); + if (!options.gcols.length) { + options.gcols = undefined; + } - if (!options.sql.length) { - throw new Error("No 'mapnik' layers in MapConfig"); - } + // Grainstore limits interactivity to one layer. If there are more than one layer in layer filter then interactivity + // won't be passed to grainstore (due to format requested is png, only one layer is allowed for grid.json format) + if (filteredLayerIndexes.length === 1) { + var lyrInteractivity = mapConfig.getLayer(filteredLayerIndexes[0]); + var lyropt = lyrInteractivity.options; + + if (lyropt.interactivity) { + // NOTE: interactivity used to be a string as of version 1.0.0 + if (Array.isArray(lyropt.interactivity)) { + lyropt.interactivity = lyropt.interactivity.join(','); + } + + options.interactivity.push(lyropt.interactivity); + // grainstore needs to know the layer index to take the interactivity, forced to be 0. + options.layer = 0; + } + } - if (!options.gcols.length) { - options.gcols = undefined; + return options; } +}; - // Grainstore limits interactivity to one layer. If there are more than one layer in layer filter then interactivity - // won't be passed to grainstore (due to format requested is png, only one layer is allowed for grid.json format) - if (filteredLayerIndexes.length === 1) { - var lyrInteractivity = mapConfig.getLayer(filteredLayerIndexes[0]); - var lyropt = lyrInteractivity.options; - - if (lyropt.interactivity) { - // NOTE: interactivity used to be a string as of version 1.0.0 - if (Array.isArray(lyropt.interactivity)) { - lyropt.interactivity = lyropt.interactivity.join(','); - } +function bootstrapFonts (grainstoreOptions) { + // Set carto renderer configuration for MMLStore + grainstoreOptions.carto_env = grainstoreOptions.carto_env || {}; + const cenv = grainstoreOptions.carto_env; - options.interactivity.push(lyropt.interactivity); - // grainstore needs to know the layer index to take the interactivity, forced to be 0. - options.layer = 0; - } + cenv.validation_data = cenv.validation_data || {}; + if (!cenv.validation_data.fonts) { + mapnik.register_system_fonts(); + mapnik.register_default_fonts(); + cenv.validation_data.fonts = Object.keys(mapnik.fontFiles()); } - - return options; -}; +} function validateLayer (mapConfig, layerIndex) { const layer = mapConfig.getLayer(layerIndex); From 6bc913b6d9bbb331dadb37fffcc9850569b35121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sun, 12 Apr 2020 18:24:48 +0200 Subject: [PATCH 30/45] Use ES6 syntax --- lib/renderers/mapnik/factory.js | 71 +++++++++++++++++---------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/lib/renderers/mapnik/factory.js b/lib/renderers/mapnik/factory.js index db7d03222..b52059600 100644 --- a/lib/renderers/mapnik/factory.js +++ b/lib/renderers/mapnik/factory.js @@ -227,17 +227,13 @@ module.exports = class MapnikFactory { }; getMetatile (format) { - var metatile = this._mapnik_opts.metatile; + let metatile = this._mapnik_opts.metatile; + if (Number.isFinite(this._mapnik_opts.formatMetatile[format])) { metatile = this._mapnik_opts.formatMetatile[format]; } - return metatile; - } - getBufferSizeFromOptions (format, options) { - return options && options.formatBufferSize && Number.isFinite(options.formatBufferSize[format]) - ? options.formatBufferSize[format] - : options.bufferSize; + return metatile; } getBufferSize (mapConfig, format, options) { @@ -247,12 +243,11 @@ module.exports = class MapnikFactory { return mapConfig.getBufferSize(format); } - return this.getBufferSizeFromOptions(format, options); + return getBufferSizeFromOptions(format, options); } mapConfigToMMLBuilderConfig (mapConfig, queryRewriter, rendererOptions) { - var self = this; - var options = { + const options = { ids: [], sql: [], originalSql: [], @@ -267,34 +262,34 @@ module.exports = class MapnikFactory { 'cache-features': rendererOptions.params['cache-features'] }; - var layerFilter = rendererOptions.layer; + const layerFilter = rendererOptions.layer; - var filteredLayerIndexes = layersFilter(mapConfig, layerFilter); - filteredLayerIndexes.reduce(function (options, layerIndex) { - var layer = mapConfig.getLayer(layerIndex); + const filteredLayerIndexes = layersFilter(mapConfig, layerFilter); + filteredLayerIndexes.reduce((options, layerIndex) => { + const layer = mapConfig.getLayer(layerIndex); validateLayer(mapConfig, layerIndex); options.ids.push(mapConfig.getLayerId(layerIndex)); - var lyropt = layer.options; + const layerOptions = layer.options; - if (lyropt.cartocss !== undefined && lyropt.cartocss_version !== undefined) { - options.style.push(lyropt.cartocss); - options.style_version.push(lyropt.cartocss_version); + if (layerOptions.cartocss !== undefined && layerOptions.cartocss_version !== undefined) { + options.style.push(layerOptions.cartocss); + options.style_version.push(layerOptions.cartocss_version); } - var query = queryRewriter.query(lyropt.sql, lyropt.query_rewrite_data); - var queryOptions = prepareQuery(query, lyropt.geom_column, lyropt.geom_type, self._mapnik_opts); + const query = queryRewriter.query(layerOptions.sql, layerOptions.query_rewrite_data); + const queryOptions = prepareQuery(query, layerOptions.geom_column, layerOptions.geom_type, this._mapnik_opts); options.sql.push(queryOptions.sql); options.originalSql.push(query); - var zoom = {}; - if (Number.isFinite(lyropt.minzoom)) { - zoom.minzoom = lyropt.minzoom; + const zoom = {}; + if (Number.isFinite(layerOptions.minzoom)) { + zoom.minzoom = layerOptions.minzoom; } - if (Number.isFinite(lyropt.maxzoom)) { - zoom.maxzoom = lyropt.maxzoom; + if (Number.isFinite(layerOptions.maxzoom)) { + zoom.maxzoom = layerOptions.maxzoom; } options.zooms.push(zoom); @@ -303,9 +298,9 @@ module.exports = class MapnikFactory { name: queryOptions.geomColumnName }); - var extraOpt = {}; - if (Object.prototype.hasOwnProperty.call(lyropt, 'raster_band')) { - extraOpt.band = lyropt.raster_band; + const extraOpt = {}; + if (Object.prototype.hasOwnProperty.call(layerOptions, 'raster_band')) { + extraOpt.band = layerOptions.raster_band; } options.datasource_extend.push(mapConfig.getLayerDatasource(layerIndex)); options.extra_ds_opts.push(extraOpt); @@ -314,7 +309,7 @@ module.exports = class MapnikFactory { }, options); if (!options.sql.length) { - throw new Error("No 'mapnik' layers in MapConfig"); + throw new Error('No \'mapnik\' layers in MapConfig'); } if (!options.gcols.length) { @@ -324,16 +319,16 @@ module.exports = class MapnikFactory { // Grainstore limits interactivity to one layer. If there are more than one layer in layer filter then interactivity // won't be passed to grainstore (due to format requested is png, only one layer is allowed for grid.json format) if (filteredLayerIndexes.length === 1) { - var lyrInteractivity = mapConfig.getLayer(filteredLayerIndexes[0]); - var lyropt = lyrInteractivity.options; + const lyrInteractivity = mapConfig.getLayer(filteredLayerIndexes[0]); + const layerOptions = lyrInteractivity.options; - if (lyropt.interactivity) { + if (layerOptions.interactivity) { // NOTE: interactivity used to be a string as of version 1.0.0 - if (Array.isArray(lyropt.interactivity)) { - lyropt.interactivity = lyropt.interactivity.join(','); + if (Array.isArray(layerOptions.interactivity)) { + layerOptions.interactivity = layerOptions.interactivity.join(','); } - options.interactivity.push(lyropt.interactivity); + options.interactivity.push(layerOptions.interactivity); // grainstore needs to know the layer index to take the interactivity, forced to be 0. options.layer = 0; } @@ -381,6 +376,12 @@ function validateLayer (mapConfig, layerIndex) { } } +function getBufferSizeFromOptions (format, options) { + return options && options.formatBufferSize && Number.isFinite(options.formatBufferSize[format]) + ? options.formatBufferSize[format] + : options.bufferSize; +} + // Wrap SQL requests in mapnik format if sent function prepareQuery (userSql, geomColumnName, geomColumnType, options) { // remove trailing ';' From a7898a9f4961929d6b5dbdeb4a919164cd368f70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sun, 12 Apr 2020 19:03:14 +0200 Subject: [PATCH 31/45] Better scope and naming --- lib/renderers/mapnik/factory.js | 236 ++++++++++----------- test/unit/renderers/mapnik-factory-test.js | 26 +-- 2 files changed, 129 insertions(+), 133 deletions(-) diff --git a/lib/renderers/mapnik/factory.js b/lib/renderers/mapnik/factory.js index b52059600..a272c30f6 100644 --- a/lib/renderers/mapnik/factory.js +++ b/lib/renderers/mapnik/factory.js @@ -26,10 +26,7 @@ module.exports = class MapnikFactory { mvt: true }; - this._options = options; - - // Set default mapnik options - this._mapnik_opts = Object.assign({}, { + this._mapnikOptions = Object.assign({}, { geometry_field: 'the_geom_webmercator', // Metatile is the number of tiles-per-side that are going @@ -112,17 +109,15 @@ module.exports = class MapnikFactory { variables: {} }, options.mapnik || {}); - this._options.grainstore = this._options.grainstore ? this._options.grainstore : {}; - this._options.grainstore.mapnik_version = this._options.grainstore.mapnik_version - ? this._options.grainstore.mapnik_version - : mapnik.versions.mapnik; - - bootstrapFonts(this._options.grainstore); - - this.tile_scale_factors = this._mapnik_opts.scale_factors.reduce(function (previousValue, currentValue) { + this._mapnikOptions.scale_factors = this._mapnikOptions.scale_factors.reduce((previousValue, currentValue) => { previousValue[currentValue] = DEFAULT_TILE_SIZE * currentValue; return previousValue; }, {}); + + this._grainstoreOptions = options.grainstore || {}; + this._grainstoreOptions.mapnik_version = this._grainstoreOptions.mapnik_version || mapnik.versions.mapnik; + + bootstrapFonts(this._grainstoreOptions); } getName () { @@ -137,20 +132,6 @@ module.exports = class MapnikFactory { return new MapnikAdaptor(renderer, onTileErrorStrategy); } - defineExpectedParams (params) { - if (params['cache-features'] === undefined) { - params['cache-features'] = this._mapnik_opts['cache-features']; - } - - if (params.metrics === undefined) { - params.metrics = this._mapnik_opts.metrics; - } - - if (params.markers_symbolizer_caches === undefined) { - params.markers_symbolizer_caches = this._mapnik_opts.markers_symbolizer_caches; - } - } - getRenderer (mapConfig, format, options, callback) { const isMvt = format === FORMAT_MVT; @@ -161,18 +142,18 @@ module.exports = class MapnikFactory { return callback(error); } - this.defineExpectedParams(options.params); + defineExpectedParams(options.params, this._mapnikOptions); let mmlBuilderConfig; try { - mmlBuilderConfig = this.mapConfigToMMLBuilderConfig(mapConfig, this._mapnik_opts.queryRewriter, options); + mmlBuilderConfig = mapConfigToMMLBuilderConfig(mapConfig, options, this._mapnikOptions); } catch (error) { return callback(error); } const params = Object.assign({}, options.params, mmlBuilderConfig); - const limits = Object.assign({}, this._mapnik_opts.limits, options.limits); - const variables = Object.assign({}, this._mapnik_opts.variables, options.variables); + const limits = Object.assign({}, this._mapnikOptions.limits, options.limits); + const variables = Object.assign({}, this._mapnikOptions.variables, options.variables); // fix layer index // see https://github.com/CartoDB/Windshaft/blob/0.43.0/lib/backends/map_validator.js#L69-L81 @@ -181,17 +162,17 @@ module.exports = class MapnikFactory { } const scaleFactor = params.scale_factor === undefined ? 1 : +params.scale_factor; - const tileSize = this.tile_scale_factors[scaleFactor]; + const tileSize = this._mapnikOptions.scale_factors[scaleFactor]; if (!tileSize) { - var err = new Error('Tile with specified resolution not found'); + const err = new Error('Tile with specified resolution not found'); err.http_status = 404; return callback(err); } let mmlBuilder; try { - mmlBuilder = getMMLBuilder(mapConfig, this._options.grainstore, params, format); + mmlBuilder = getMMLBuilder(mapConfig, this._grainstoreOptions, params, format); } catch (error) { return callback(error); } @@ -206,15 +187,15 @@ module.exports = class MapnikFactory { type: isMvt ? 'vector' : 'raster', xml: xml, strict: !!params.strict, - bufferSize: this.getBufferSize(mapConfig, format), - poolSize: this._mapnik_opts.poolSize, - poolMaxWaitingClients: this._mapnik_opts.poolMaxWaitingClients, + bufferSize: this._getBufferSize(mapConfig, format), + poolSize: this._mapnikOptions.poolSize, + poolMaxWaitingClients: this._mapnikOptions.poolMaxWaitingClients, tileSize: tileSize, limits: limits, metrics: options.params.metrics, variables: variables, - metatile: this.getMetatile(format), // when raster renderer - metatileCache: this._mapnik_opts.metatileCache, // when raster renderer + metatile: this._getMetatile(format), // when raster renderer + metatileCache: this._mapnikOptions.metatileCache, // when raster renderer scale: scaleFactor, // when raster renderer gzip: false // when vector renderer }); @@ -224,20 +205,20 @@ module.exports = class MapnikFactory { return callback(err); } }); - }; + } - getMetatile (format) { - let metatile = this._mapnik_opts.metatile; + _getMetatile (format) { + let metatile = this._mapnikOptions.metatile; - if (Number.isFinite(this._mapnik_opts.formatMetatile[format])) { - metatile = this._mapnik_opts.formatMetatile[format]; + if (Number.isFinite(this._mapnikOptions.formatMetatile[format])) { + metatile = this._mapnikOptions.formatMetatile[format]; } return metatile; } - getBufferSize (mapConfig, format, options) { - options = options || this._mapnik_opts; + _getBufferSize (mapConfig, format, options) { + options = options || this._mapnikOptions; if (Number.isFinite(mapConfig.getBufferSize(format))) { return mapConfig.getBufferSize(format); @@ -245,98 +226,113 @@ module.exports = class MapnikFactory { return getBufferSizeFromOptions(format, options); } +}; - mapConfigToMMLBuilderConfig (mapConfig, queryRewriter, rendererOptions) { - const options = { - ids: [], - sql: [], - originalSql: [], - style: [], - style_version: [], - zooms: [], - interactivity: [], - ttl: 0, - datasource_extend: [], - extra_ds_opts: [], - gcols: [], - 'cache-features': rendererOptions.params['cache-features'] - }; +function defineExpectedParams (params, mapnikOptions) { + if (params['cache-features'] === undefined) { + params['cache-features'] = mapnikOptions['cache-features']; + } - const layerFilter = rendererOptions.layer; + if (params.metrics === undefined) { + params.metrics = mapnikOptions.metrics; + } - const filteredLayerIndexes = layersFilter(mapConfig, layerFilter); - filteredLayerIndexes.reduce((options, layerIndex) => { - const layer = mapConfig.getLayer(layerIndex); + if (params.markers_symbolizer_caches === undefined) { + params.markers_symbolizer_caches = mapnikOptions.markers_symbolizer_caches; + } +} - validateLayer(mapConfig, layerIndex); +function mapConfigToMMLBuilderConfig (mapConfig, rendererOptions, mapnikOptions) { + const options = { + ids: [], + sql: [], + originalSql: [], + style: [], + style_version: [], + zooms: [], + interactivity: [], + ttl: 0, + datasource_extend: [], + extra_ds_opts: [], + gcols: [], + 'cache-features': rendererOptions.params['cache-features'] + }; - options.ids.push(mapConfig.getLayerId(layerIndex)); - const layerOptions = layer.options; + const layerFilter = rendererOptions.layer; - if (layerOptions.cartocss !== undefined && layerOptions.cartocss_version !== undefined) { - options.style.push(layerOptions.cartocss); - options.style_version.push(layerOptions.cartocss_version); - } + const filteredLayerIndexes = layersFilter(mapConfig, layerFilter); + filteredLayerIndexes.reduce((options, layerIndex) => { + const layer = mapConfig.getLayer(layerIndex); - const query = queryRewriter.query(layerOptions.sql, layerOptions.query_rewrite_data); - const queryOptions = prepareQuery(query, layerOptions.geom_column, layerOptions.geom_type, this._mapnik_opts); + validateLayer(mapConfig, layerIndex); - options.sql.push(queryOptions.sql); - options.originalSql.push(query); + options.ids.push(mapConfig.getLayerId(layerIndex)); + const layerOptions = layer.options; - const zoom = {}; - if (Number.isFinite(layerOptions.minzoom)) { - zoom.minzoom = layerOptions.minzoom; - } - if (Number.isFinite(layerOptions.maxzoom)) { - zoom.maxzoom = layerOptions.maxzoom; - } - options.zooms.push(zoom); - - options.gcols.push({ - type: queryOptions.geomColumnType, // grainstore allows undefined here - name: queryOptions.geomColumnName - }); + if (layerOptions.cartocss !== undefined && layerOptions.cartocss_version !== undefined) { + options.style.push(layerOptions.cartocss); + options.style_version.push(layerOptions.cartocss_version); + } - const extraOpt = {}; - if (Object.prototype.hasOwnProperty.call(layerOptions, 'raster_band')) { - extraOpt.band = layerOptions.raster_band; - } - options.datasource_extend.push(mapConfig.getLayerDatasource(layerIndex)); - options.extra_ds_opts.push(extraOpt); + const { queryRewriter } = mapnikOptions; + const query = queryRewriter.query(layerOptions.sql, layerOptions.query_rewrite_data); + const queryOptions = prepareQuery(query, layerOptions.geom_column, layerOptions.geom_type, mapnikOptions); - return options; - }, options); + options.sql.push(queryOptions.sql); + options.originalSql.push(query); - if (!options.sql.length) { - throw new Error('No \'mapnik\' layers in MapConfig'); + const zoom = {}; + if (Number.isFinite(layerOptions.minzoom)) { + zoom.minzoom = layerOptions.minzoom; } - - if (!options.gcols.length) { - options.gcols = undefined; + if (Number.isFinite(layerOptions.maxzoom)) { + zoom.maxzoom = layerOptions.maxzoom; } + options.zooms.push(zoom); - // Grainstore limits interactivity to one layer. If there are more than one layer in layer filter then interactivity - // won't be passed to grainstore (due to format requested is png, only one layer is allowed for grid.json format) - if (filteredLayerIndexes.length === 1) { - const lyrInteractivity = mapConfig.getLayer(filteredLayerIndexes[0]); - const layerOptions = lyrInteractivity.options; - - if (layerOptions.interactivity) { - // NOTE: interactivity used to be a string as of version 1.0.0 - if (Array.isArray(layerOptions.interactivity)) { - layerOptions.interactivity = layerOptions.interactivity.join(','); - } - - options.interactivity.push(layerOptions.interactivity); - // grainstore needs to know the layer index to take the interactivity, forced to be 0. - options.layer = 0; - } + options.gcols.push({ + type: queryOptions.geomColumnType, // grainstore allows undefined here + name: queryOptions.geomColumnName + }); + + const extraOpt = {}; + if (Object.prototype.hasOwnProperty.call(layerOptions, 'raster_band')) { + extraOpt.band = layerOptions.raster_band; } + options.datasource_extend.push(mapConfig.getLayerDatasource(layerIndex)); + options.extra_ds_opts.push(extraOpt); return options; + }, options); + + if (!options.sql.length) { + throw new Error('No \'mapnik\' layers in MapConfig'); } -}; + + if (!options.gcols.length) { + options.gcols = undefined; + } + + // Grainstore limits interactivity to one layer. If there are more than one layer in layer filter then interactivity + // won't be passed to grainstore (due to format requested is png, only one layer is allowed for grid.json format) + if (filteredLayerIndexes.length === 1) { + const lyrInteractivity = mapConfig.getLayer(filteredLayerIndexes[0]); + const layerOptions = lyrInteractivity.options; + + if (layerOptions.interactivity) { + // NOTE: interactivity used to be a string as of version 1.0.0 + if (Array.isArray(layerOptions.interactivity)) { + layerOptions.interactivity = layerOptions.interactivity.join(','); + } + + options.interactivity.push(layerOptions.interactivity); + // grainstore needs to know the layer index to take the interactivity, forced to be 0. + options.layer = 0; + } + } + + return options; +} function bootstrapFonts (grainstoreOptions) { // Set carto renderer configuration for MMLStore @@ -383,11 +379,11 @@ function getBufferSizeFromOptions (format, options) { } // Wrap SQL requests in mapnik format if sent -function prepareQuery (userSql, geomColumnName, geomColumnType, options) { +function prepareQuery (userSql, geomColumnName, geomColumnType, mapnikOptions) { // remove trailing ';' userSql = userSql.replace(/;\s*$/, ''); - geomColumnName = geomColumnName || options.geometry_field; + geomColumnName = geomColumnName || mapnikOptions.geometry_field; geomColumnType = geomColumnType || COLUMN_TYPE_DEFAULT; return { diff --git a/test/unit/renderers/mapnik-factory-test.js b/test/unit/renderers/mapnik-factory-test.js index 8d6275138..62264ed1d 100644 --- a/test/unit/renderers/mapnik-factory-test.js +++ b/test/unit/renderers/mapnik-factory-test.js @@ -9,7 +9,7 @@ var MapConfig = require('../../../lib/models/mapconfig'); describe('renderer-mapnik-factory metatile', function () { it('should use default metatile value', function () { var factory = new MapnikRendererFactory({}); - assert.equal(factory.getMetatile('png'), 4); + assert.equal(factory._getMetatile('png'), 4); }); it('should use provided metatile value', function () { @@ -18,7 +18,7 @@ describe('renderer-mapnik-factory metatile', function () { metatile: 1 } }); - assert.equal(factory.getMetatile('png'), 1); + assert.equal(factory._getMetatile('png'), 1); }); it('should use provided formatMetatile value', function () { @@ -30,7 +30,7 @@ describe('renderer-mapnik-factory metatile', function () { } } }); - assert.equal(factory.getMetatile('png'), 4); + assert.equal(factory._getMetatile('png'), 4); }); }); @@ -60,7 +60,7 @@ describe('renderer-mapnik-factory buffer-size', function () { it('should use default buffer-size value', function () { var factory = new MapnikRendererFactory({}); - assert.equal(factory.getBufferSize(mapConfig, 'png'), 64); + assert.equal(factory._getBufferSize(mapConfig, 'png'), 64); }); it('should use provided buffer-size value', function () { @@ -69,7 +69,7 @@ describe('renderer-mapnik-factory buffer-size', function () { bufferSize: 128 } }); - assert.equal(factory.getBufferSize(mapConfig, 'png'), 128); + assert.equal(factory._getBufferSize(mapConfig, 'png'), 128); }); it('should use provided formatBufferSize value', function () { @@ -81,7 +81,7 @@ describe('renderer-mapnik-factory buffer-size', function () { } } }); - assert.equal(factory.getBufferSize(mapConfig, 'png'), 128); + assert.equal(factory._getBufferSize(mapConfig, 'png'), 128); }); it('should use provided buffer-size value', function () { @@ -93,7 +93,7 @@ describe('renderer-mapnik-factory buffer-size', function () { } } }); - assert.equal(factory.getBufferSize(mapConfig, 'mvt'), 64); + assert.equal(factory._getBufferSize(mapConfig, 'mvt'), 64); }); it('should use value provided by mapConfig for png and mvt', function () { @@ -121,9 +121,9 @@ describe('renderer-mapnik-factory buffer-size', function () { } } }); - assert.equal(factory.getBufferSize(mapConfig, 'png'), 128); - assert.equal(factory.getBufferSize(mapConfig, 'mvt'), 0); - assert.equal(factory.getBufferSize(mapConfig, 'grid.json'), 64); + assert.equal(factory._getBufferSize(mapConfig, 'png'), 128); + assert.equal(factory._getBufferSize(mapConfig, 'mvt'), 0); + assert.equal(factory._getBufferSize(mapConfig, 'grid.json'), 64); }); it('should use value provided by mapConfig for png and mvt', function () { @@ -151,8 +151,8 @@ describe('renderer-mapnik-factory buffer-size', function () { } } }); - assert.equal(factory.getBufferSize(mapConfig, 'png'), 128); - assert.equal(factory.getBufferSize(mapConfig, 'mvt'), 128); - assert.equal(factory.getBufferSize(mapConfig, 'grid.json'), 128); + assert.equal(factory._getBufferSize(mapConfig, 'png'), 128); + assert.equal(factory._getBufferSize(mapConfig, 'mvt'), 128); + assert.equal(factory._getBufferSize(mapConfig, 'grid.json'), 128); }); }); From 460b03aa0904a96defc4b0455223c014889a2726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Sun, 12 Apr 2020 19:09:08 +0200 Subject: [PATCH 32/45] Use constants --- lib/renderers/mapnik/factory.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/renderers/mapnik/factory.js b/lib/renderers/mapnik/factory.js index a272c30f6..44d04b3c6 100644 --- a/lib/renderers/mapnik/factory.js +++ b/lib/renderers/mapnik/factory.js @@ -12,6 +12,7 @@ const COLUMN_TYPE_RASTER = 'raster'; const COLUMN_TYPE_DEFAULT = COLUMN_TYPE_GEOMETRY; const DEFAULT_TILE_SIZE = 256; const FORMAT_MVT = 'mvt'; +const SUPPORTED_FORMATS = ['png', 'png32', 'grid.json', FORMAT_MVT]; module.exports = class MapnikFactory { static get NAME () { @@ -19,13 +20,6 @@ module.exports = class MapnikFactory { } constructor (options) { - this.supportedFormats = { - png: true, - png32: true, - 'grid.json': true, - mvt: true - }; - this._mapnikOptions = Object.assign({}, { geometry_field: 'the_geom_webmercator', @@ -125,7 +119,7 @@ module.exports = class MapnikFactory { } supportsFormat (format) { - return !!this.supportedFormats[format]; + return SUPPORTED_FORMATS.includes(format); } getAdaptor (renderer, onTileErrorStrategy) { @@ -349,7 +343,7 @@ function bootstrapFonts (grainstoreOptions) { function validateLayer (mapConfig, layerIndex) { const layer = mapConfig.getLayer(layerIndex); - var layerOptions = layer.options; + const layerOptions = layer.options; if (!mapConfig.isVectorOnlyMapConfig()) { if (!Object.prototype.hasOwnProperty.call(layerOptions, 'cartocss')) { From b1cb1f51a187abdaf2a92fb9fa0f710f4614afc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 13 Apr 2020 12:45:34 +0200 Subject: [PATCH 33/45] Export what needed, a function for this use case --- lib/backends/attributes.js | 4 +-- lib/renderers/pg-mvt/factory.js | 4 +-- lib/renderers/renderer-params.js | 44 +++++++++++++++++--------------- lib/renderers/torque/factory.js | 4 +-- 4 files changed, 29 insertions(+), 27 deletions(-) diff --git a/lib/backends/attributes.js b/lib/backends/attributes.js index c54ae0a2e..e7dce5426 100644 --- a/lib/backends/attributes.js +++ b/lib/backends/attributes.js @@ -2,7 +2,7 @@ const PSQL = require('cartodb-psql'); -const RendererParams = require('../renderers/renderer-params'); +const parseDbParams = require('../renderers/renderer-params'); const Timer = require('../stats/timer'); const SubstitutionTokens = require('cartodb-query-tables').utils.substitutionTokens; @@ -49,7 +49,7 @@ AttributesBackend.prototype.getFeatureAttributes = function (mapConfigProvider, const dbParams = Object.assign( {}, - RendererParams.dbParamsFromReqParams(params), + parseDbParams(params), mapConfig.getLayerDatasource(params.layer) ); diff --git a/lib/renderers/pg-mvt/factory.js b/lib/renderers/pg-mvt/factory.js index 1b9c1759a..3c0b7ac0b 100644 --- a/lib/renderers/pg-mvt/factory.js +++ b/lib/renderers/pg-mvt/factory.js @@ -1,7 +1,7 @@ 'use strict'; const layersFilter = require('../../utils/layer-filter'); -const RendererParams = require('../renderer-params'); +const parseDbParams = require('../renderer-params'); const Renderer = require('./renderer'); const BaseAdaptor = require('../base-adaptor'); @@ -53,7 +53,7 @@ module.exports = class PgMvtFactory { } const layers = filteredLayers.map(layerIndex => mapConfig.getLayer(layerIndex)); - const dbParams = RendererParams.dbParamsFromReqParams(options.params); + const dbParams = parseDbParams(options.params); Object.assign(dbParams, mapConfig.getLayerDatasource(options.layer)); if (Number.isFinite(mapConfig.getBufferSize(PgMvtFactory.MVT_FORMAT))) { diff --git a/lib/renderers/renderer-params.js b/lib/renderers/renderer-params.js index 5fcf814ae..d873a9058 100644 --- a/lib/renderers/renderer-params.js +++ b/lib/renderers/renderer-params.js @@ -1,25 +1,27 @@ 'use strict'; -var RendererParams = { - dbParamsFromReqParams: function (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; +module.exports = function parseDbParams (params) { + const dbParams = {}; + + if (params.dbuser) { + dbParams.user = params.dbuser; + } + + if (params.dbpassword) { + dbParams.pass = params.dbpassword; + } + + if (params.dbhost) { + dbParams.host = params.dbhost; } -}; -module.exports = RendererParams; + if (params.dbport) { + dbParams.port = params.dbport; + } + + if (params.dbname) { + dbParams.dbname = params.dbname; + } + + return dbParams; +}; diff --git a/lib/renderers/torque/factory.js b/lib/renderers/torque/factory.js index ee9d348db..351f305bd 100644 --- a/lib/renderers/torque/factory.js +++ b/lib/renderers/torque/factory.js @@ -3,7 +3,7 @@ const carto = require('carto'); const debug = require('debug')('windshaft:renderer:torque_factory'); const torqueReference = require('torque.js').cartocss_reference; -const RendererParams = require('../renderer-params'); +const parseDbParams = require('../renderer-params'); const Renderer = require('./renderer'); const PsqlAdaptor = require('./psql-adaptor'); const PngRenderer = require('./png-renderer'); @@ -68,7 +68,7 @@ module.exports = class TorqueFactory { return callback(new Error('layer ' + layer + ' is not a torque layer')); } - const dbParams = Object.assign({}, RendererParams.dbParamsFromReqParams(options.params), mapConfig.getLayerDatasource(layer)); + const dbParams = Object.assign({}, parseDbParams(options.params), mapConfig.getLayerDatasource(layer)); const psql = new PsqlAdaptor({ connectionParams: dbParams, poolParams: this.options.dbPoolParams }); fetchLayerAttributes(psql, layerObj) From 25eb375ee475c1ae402fdaf36211e3fba5ec0d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 15:23:55 +0200 Subject: [PATCH 34/45] Same structure for blend factory --- lib/renderers/blend/factory.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/renderers/blend/factory.js b/lib/renderers/blend/factory.js index 632592e06..6324141a5 100644 --- a/lib/renderers/blend/factory.js +++ b/lib/renderers/blend/factory.js @@ -6,15 +6,18 @@ const BaseAdaptor = require('../base-adaptor'); const layersFilter = require('../../utils/layer-filter'); const EMPTY_IMAGE_BUFFER = new mapnik.Image(256, 256).encodeSync('png'); -const NAME = 'blend'; -class BlendFactory { +module.exports = class BlendFactory { + static get NAME () { + return 'blend'; + } + constructor (rendererFactory) { this.rendererFactory = rendererFactory; } getName () { - return NAME; + return BlendFactory.NAME; } supportsFormat (format) { @@ -89,10 +92,7 @@ class BlendFactory { }) .catch(err => callback(err)); } -} - -module.exports = BlendFactory; -module.exports.NAME = NAME; +}; async function onTileErrorStrategyHttpRenderer (err, format) { if (err.code === 'ETIMEDOUT') { From 809b41d550640446b05fade7b5d208b76ce008f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 15:59:05 +0200 Subject: [PATCH 35/45] Use ES6 class syntax --- lib/models/mapconfig.js | 431 +++++++++++++++++++--------------------- 1 file changed, 205 insertions(+), 226 deletions(-) diff --git a/lib/models/mapconfig.js b/lib/models/mapconfig.js index 36ba5a742..32482076d 100644 --- a/lib/models/mapconfig.js +++ b/lib/models/mapconfig.js @@ -1,237 +1,272 @@ 'use strict'; -var Crypto = require('crypto'); -var semver = require('semver'); - -var Datasource = require('./datasource'); - -// Map configuration object +const crypto = require('crypto'); +const semver = require('semver'); +const Datasource = require('./datasource'); + +// see: http://github.com/CartoDB/Windshaft/wiki/MapConfig-specification +module.exports = class MapConfig { + // Factory like method to create MapConfig objects when you are unsure about being + // able to provide all the MapConfig collaborators or you have to create a MapConfig + // object from a serialized version + static create (rawConfig, datasource) { + if (rawConfig.ds) { + return new MapConfig(rawConfig.cfg, new Datasource(rawConfig.ds)); + } + datasource = datasource || Datasource.EmptyDatasource(); + return new MapConfig(rawConfig, datasource); + } -/// API: Create MapConfig from configuration object -// -/// @param obj js MapConfiguration object, see -/// http://github.com/CartoDB/Windshaft/wiki/MapConfig-specification -/// -function MapConfig (config, datasource) { - // TODO: inject defaults ? - this._cfg = config; + static getLayerId (rawMapConfig, layerIndex) { + var layer = rawMapConfig.layers[layerIndex]; + if (layer.id) { + return layer.id; + } - if (!semver.satisfies(this.version(), '>= 1.0.0 <= 1.8.0')) { - throw new Error('Unsupported layergroup configuration version ' + this.version()); + var layerType = getType(layer.type); + var layerId = 'layer' + getLayerIndexByType(rawMapConfig, layerType, layerIndex); + if (layerType !== 'mapnik') { + layerId = layerType + '-' + layerId; + } + return layerId; } - if (!Object.prototype.hasOwnProperty.call(this._cfg, 'layers')) { - throw new Error('Missing layers array from layergroup config'); - } + constructor (config, datasource) { + // TODO: inject defaults ? + this._cfg = config; - this._cfg.layers.forEach(function (layer, i) { - if (!Object.prototype.hasOwnProperty.call(layer, 'options')) { - throw new Error('Missing options from layer ' + i + ' of layergroup config'); + if (!semver.satisfies(this.version(), '>= 1.0.0 <= 1.8.0')) { + throw new Error('Unsupported layergroup configuration version ' + this.version()); } - // NOTE: interactivity used to be a string as of version 1.0.0 - if (Array.isArray(layer.options.interactivity)) { - layer.options.interactivity = layer.options.interactivity.join(','); + + if (!Object.prototype.hasOwnProperty.call(this._cfg, 'layers')) { + throw new Error('Missing layers array from layergroup config'); } - }); - if (this._cfg.buffersize) { - Object.keys(this._cfg.buffersize).forEach(format => { - if (this._cfg.buffersize[format] !== undefined && !Number.isFinite(this._cfg.buffersize[format])) { - throw new Error(`Buffer size of format "${format}" must be a number`); + this._cfg.layers.forEach(function (layer, i) { + if (!Object.prototype.hasOwnProperty.call(layer, 'options')) { + throw new Error('Missing options from layer ' + i + ' of layergroup config'); + } + // NOTE: interactivity used to be a string as of version 1.0.0 + if (Array.isArray(layer.options.interactivity)) { + layer.options.interactivity = layer.options.interactivity.join(','); } }); - } - /** - * @type {Datasource} - */ - this._datasource = datasource; + if (this._cfg.buffersize) { + Object.keys(this._cfg.buffersize).forEach(format => { + if (this._cfg.buffersize[format] !== undefined && !Number.isFinite(this._cfg.buffersize[format])) { + throw new Error(`Buffer size of format "${format}" must be a number`); + } + }); + } - this._id = null; -} + /** + * @type {Datasource} + */ + this._datasource = datasource; -function md5Hash (s) { - return Crypto.createHash('md5').update(s, 'binary').digest('hex'); -} + this._id = null; + } -/// API: Get serialized version of this MapConfig -MapConfig.prototype.serialize = function () { - if (this._datasource.isEmpty()) { - return JSON.stringify(this._cfg); + serialize () { + if (this._datasource.isEmpty()) { + return JSON.stringify(this._cfg); + } + return JSON.stringify({ + cfg: this._cfg, + ds: this._datasource.obj() + }); } - return JSON.stringify({ - cfg: this._cfg, - ds: this._datasource.obj() - }); -}; -/// API: Get identifier for this MapConfig -MapConfig.prototype.id = function () { - if (this._id === null) { - this._id = md5Hash(JSON.stringify(this._cfg)); + id () { + if (this._id === null) { + this._id = md5Hash(JSON.stringify(this._cfg)); + } + + return this._id; } - // debug('MapConfig.id=%s', this._id); - return this._id; -}; -/// API: Get configuration object of this MapConfig -MapConfig.prototype.obj = function () { - return this._cfg; -}; + obj () { + return this._cfg; + } -MapConfig.prototype.version = function () { - return this._cfg.version || '1.0.0'; -}; + version () { + return this._cfg.version || '1.0.0'; + } -MapConfig.prototype.setDbParams = function (dbParams) { - this._cfg.dbparams = dbParams; - this.flush(); -}; + setDbParams (dbParams) { + this._cfg.dbparams = dbParams; + this.flush(); + } -MapConfig.prototype.flush = function () { // flush id so it gets recalculated - this._id = null; -}; + flush () { + this._id = null; + } -/// API: Get type string of given layer -// -/// @param num layer index (0-based) -/// @returns a type string, as read from the layer -/// -MapConfig.prototype.layerType = function (num) { - var lyr = this.getLayer(num); - if (!lyr) { - return undefined; + layerType (layerIndex) { + var lyr = this.getLayer(layerIndex); + if (!lyr) { + return undefined; + } + return this.getType(lyr.type); } - return this.getType(lyr.type); -}; -MapConfig.prototype.getType = function (type) { - return getType(type); -}; + getType (type) { + return getType(type); + } -function getType (type) { - // TODO: check validity of other types ? - return (!type || type === 'cartodb') ? 'mapnik' : type; -} + setBufferSize (bufferSize) { + this._cfg.buffersize = bufferSize; + this.flush(); -MapConfig.prototype.setBufferSize = function (bufferSize) { - this._cfg.buffersize = bufferSize; - this.flush(); - return this; -}; + return this; + } + + getBufferSize (format) { + if (this._cfg.buffersize && isValidBufferSize(this._cfg.buffersize[format])) { + return parseInt(this._cfg.buffersize[format], 10); + } -MapConfig.prototype.getBufferSize = function (format) { - if (this._cfg.buffersize && isValidBufferSize(this._cfg.buffersize[format])) { - return parseInt(this._cfg.buffersize[format], 10); + return undefined; } - return undefined; -}; + hasIncompatibleLayers () { + return !this.isVectorOnlyMapConfig() && this.hasVectorLayer(); + } -function isValidBufferSize (value) { - return Number.isFinite(parseInt(value, 10)); -} + isVectorOnlyMapConfig () { + const layers = this.getLayers(); + let isVectorOnlyMapConfig = false; -MapConfig.prototype.hasIncompatibleLayers = function () { - return !this.isVectorOnlyMapConfig() && this.hasVectorLayer(); -}; + if (!layers.length) { + return isVectorOnlyMapConfig; + } -MapConfig.prototype.isVectorOnlyMapConfig = function () { - const layers = this.getLayers(); - let isVectorOnlyMapConfig = false; + isVectorOnlyMapConfig = true; + + for (let index = 0; index < layers.length; index++) { + if (!this.isVectorLayer(index)) { + isVectorOnlyMapConfig = false; + break; + } + } - if (!layers.length) { return isVectorOnlyMapConfig; } - isVectorOnlyMapConfig = true; + hasVectorLayer () { + const layers = this.getLayers(); + let hasVectorLayer = false; - for (let index = 0; index < layers.length; index++) { - if (!this.isVectorLayer(index)) { - isVectorOnlyMapConfig = false; - break; + for (let index = 0; index < layers.length; index++) { + if (this.isVectorLayer(index)) { + hasVectorLayer = true; + break; + } } + + return hasVectorLayer; } - return isVectorOnlyMapConfig; -}; + isVectorLayer (index) { + const layer = this.getLayer(index); + const type = getType(layer.type); + const sql = this.getLayerOption(index, 'sql'); + const cartocss = this.getLayerOption(index, 'cartocss'); + const cartocssVersion = this.getLayerOption(index, 'cartocss_version'); + + return type === 'mapnik' && typeof sql === 'string' && cartocss === undefined && cartocssVersion === undefined; + } -MapConfig.prototype.hasVectorLayer = function () { - const layers = this.getLayers(); - let hasVectorLayer = false; + /***************************************************************************** + * Layers + ****************************************************************************/ - for (let index = 0; index < layers.length; index++) { - if (this.isVectorLayer(index)) { - hasVectorLayer = true; - break; - } + getLayerId (layerIndex) { + return MapConfig.getLayerId(this._cfg, layerIndex); } - return hasVectorLayer; -}; + getIndexByLayerId (layerId) { + var layers = this.getLayers(); -MapConfig.prototype.isVectorLayer = function (index) { - const layer = this.getLayer(index); - const type = getType(layer.type); - const sql = this.getLayerOption(index, 'sql'); - const cartocss = this.getLayerOption(index, 'cartocss'); - const cartocssVersion = this.getLayerOption(index, 'cartocss_version'); + for (var i = 0; i < layers.length; i++) { + if (layers[i].id === layerId) { + return i; + } + } - return type === 'mapnik' && typeof sql === 'string' && cartocss === undefined && cartocssVersion === undefined; -}; + return -1; + } -/***************************************************************************** - * Layers - ****************************************************************************/ + getLayer (layerIndex) { + return this._cfg.layers[layerIndex]; + } -MapConfig.prototype.getLayerId = function (layerIndex) { - return getLayerId(this._cfg, layerIndex); -}; + getLayers () { + return this._cfg.layers.map(function (_layer, layerIndex) { + return this.getLayer(layerIndex); + }.bind(this)); + } -function getLayerId (rawMapConfig, layerIndex) { - var layer = rawMapConfig.layers[layerIndex]; - if (layer.id) { - return layer.id; + getLayerIndexByType (type, mapConfigLayerIdx) { + return getLayerIndexByType(this._cfg, type, mapConfigLayerIdx); } - var layerType = getType(layer.type); - var layerId = 'layer' + getLayerIndexByType(rawMapConfig, layerType, layerIndex); - if (layerType !== 'mapnik') { - layerId = layerType + '-' + layerId; + getLayerOption (layerIndex, optionName, defaultValue) { + var layerOption = defaultValue; + var layer = this.getLayer(layerIndex); + if (layer && Object.prototype.hasOwnProperty.call(layer.options, optionName)) { + layerOption = layer.options[optionName]; + } + return layerOption; } - return layerId; -} -MapConfig.prototype.getIndexByLayerId = function (layerId) { - var layers = this.getLayers(); + /***************************************************************************** + * Datasource + ****************************************************************************/ - for (var i = 0; i < layers.length; i++) { - if (layers[i].id === layerId) { - return i; + getLayerDatasource (layerIndex) { + var datasource = this._datasource.getLayerDatasource(layerIndex) || {}; + + var layerSrid = this.getLayerOption(layerIndex, 'srid'); + if (layerSrid) { + datasource.srid = layerSrid; } + + return datasource; } - return -1; -}; + /***************************************************************************** + * MVT + ****************************************************************************/ -/// API: Get layer by index -// -/// @returns undefined on invalid index -/// -MapConfig.prototype.getLayer = function (layerIndex) { - return this._cfg.layers[layerIndex]; -}; + getMVTExtents () { + const layers = this.getLayers(); + const extent = getTileExtent(layers); + const simplifyExtent = getSimplifyExtent(layers, extent); -MapConfig.prototype.getLayers = function () { - return this._cfg.layers.map(function (_layer, layerIndex) { - return this.getLayer(layerIndex); - }.bind(this)); + return { extent: extent || DEFAULT_EXTENT, simplify_extent: simplifyExtent || DEFAULT_SIMPLIFY_EXTENT }; + } }; -MapConfig.prototype.getLayerIndexByType = function (type, mapConfigLayerIdx) { - return getLayerIndexByType(this._cfg, type, mapConfigLayerIdx); -}; +function md5Hash (s) { + return crypto.createHash('md5').update(s, 'binary').digest('hex'); +} + +function getType (type) { + // TODO: check validity of other types ? + return (!type || type === 'cartodb') ? 'mapnik' : type; +} + +function isValidBufferSize (value) { + return Number.isFinite(parseInt(value, 10)); +} + +/***************************************************************************** + * Layers + ****************************************************************************/ function getLayerIndexByType (rawMapConfig, type, mapConfigLayerIdx) { var typeLayerIndex = 0; @@ -246,45 +281,6 @@ function getLayerIndexByType (rawMapConfig, type, mapConfigLayerIdx) { return mapConfigToTypeLayers[mapConfigLayerIdx]; } -MapConfig.prototype.getLayerOption = function (layerIndex, optionName, defaultValue) { - var layerOption = defaultValue; - var layer = this.getLayer(layerIndex); - if (layer && Object.prototype.hasOwnProperty.call(layer.options, optionName)) { - layerOption = layer.options[optionName]; - } - return layerOption; -}; - -/***************************************************************************** - * Datasource - ****************************************************************************/ - -MapConfig.prototype.getLayerDatasource = function (layerIndex) { - var datasource = this._datasource.getLayerDatasource(layerIndex) || {}; - - var layerSrid = this.getLayerOption(layerIndex, 'srid'); - if (layerSrid) { - datasource.srid = layerSrid; - } - - return datasource; -}; - -/** - * À la Factory method - * - * @param {Object} rawConfig - * @param {Datasource} [datasource=Datasource.EmptyDatasource()] - * @returns {MapConfig} - */ -function create (rawConfig, datasource) { - if (rawConfig.ds) { - return new MapConfig(rawConfig.cfg, new Datasource(rawConfig.ds)); - } - datasource = datasource || Datasource.EmptyDatasource(); - return new MapConfig(rawConfig, datasource); -} - /***************************************************************************** * MVT ****************************************************************************/ @@ -360,20 +356,3 @@ function getTileExtent (layers) { return extent; } - -// Returns an object with the extents needed for MVTs. Throws on error -MapConfig.prototype.getMVTExtents = function () { - const layers = this.getLayers(); - const extent = getTileExtent(layers); - const simplifyExtent = getSimplifyExtent(layers, extent); - - return { extent: extent || DEFAULT_EXTENT, simplify_extent: simplifyExtent || DEFAULT_SIMPLIFY_EXTENT }; -}; - -module.exports = MapConfig; -// Factory like method to create MapConfig objects when you are unsure about being -// able to provide all the MapConfig collaborators or you have to create a MapConfig -// object from a serialized version -module.exports.create = create; - -module.exports.getLayerId = getLayerId; From a4f373c86ccd6695a59258e35c56c79f8457001e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 16:32:29 +0200 Subject: [PATCH 36/45] ES2020 sugar --- lib/models/datasource.js | 80 ++++++++++++++++------------------- lib/models/mapconfig.js | 91 ++++++++++++++++------------------------ 2 files changed, 71 insertions(+), 100 deletions(-) diff --git a/lib/models/datasource.js b/lib/models/datasource.js index e69b2402b..a75d8dc2f 100644 --- a/lib/models/datasource.js +++ b/lib/models/datasource.js @@ -1,58 +1,50 @@ 'use strict'; -/** - * @param {Array} layersDbParams - * @constructor - */ -function Datasource (layersDbParams) { - layersDbParams = layersDbParams || []; - if (!Array.isArray(layersDbParams)) { - throw new Error('layersDbParams must be an Array'); - } - this.layersDbParams = layersDbParams; -} - -module.exports = Datasource; +class Datasource { + constructor (layersDbParams = []) { + if (!Array.isArray(layersDbParams)) { + throw new Error('layersDbParams must be an Array'); + } -Datasource.prototype.getLayerDatasource = function (layerIndex) { - return this.layersDbParams[layerIndex] || {}; -}; - -Datasource.prototype.obj = function () { - return this.layersDbParams; -}; + this.layersDbParams = layersDbParams; + } -Datasource.prototype.isEmpty = function () { - return this.layersDbParams.filter(function (layerDbParams) { - return !!layerDbParams; - }).length === 0; -}; + getLayerDatasource (layerIndex) { + return this.layersDbParams[layerIndex] || {}; + } -Datasource.prototype.clone = function () { - return new Datasource(this.layersDbParams.slice()); -}; + obj () { + return this.layersDbParams; + } -// ------------------ EmptyDatasource ------------------ + isEmpty () { + return this.layersDbParams.filter(layerDbParams => !!layerDbParams).length === 0; + } -function emptyDatasource () { - return new Datasource([]); + clone () { + return new Datasource(this.layersDbParams.slice()); + } } -module.exports.EmptyDatasource = emptyDatasource; +class Builder { + constructor () { + this.layersDbParams = []; + } -// ------------------ Builder ------------------ + withLayerDatasource (layerIndex, dbParams) { + this.layersDbParams[layerIndex] = dbParams; + return this; + } -function Builder () { - this.layersDbParams = []; + build () { + return new Datasource(this.layersDbParams); + } } -module.exports.Builder = Builder; - -Builder.prototype.withLayerDatasource = function (layerIndex, dbParams) { - this.layersDbParams[layerIndex] = dbParams; - return this; -}; +function emptyDatasource () { + return new Datasource([]); +} -Builder.prototype.build = function () { - return new Datasource(this.layersDbParams); -}; +module.exports = Datasource; +module.exports.Builder = Builder; +module.exports.EmptyDatasource = emptyDatasource; diff --git a/lib/models/mapconfig.js b/lib/models/mapconfig.js index 32482076d..93d6d4d85 100644 --- a/lib/models/mapconfig.js +++ b/lib/models/mapconfig.js @@ -18,35 +18,41 @@ module.exports = class MapConfig { } static getLayerId (rawMapConfig, layerIndex) { - var layer = rawMapConfig.layers[layerIndex]; + const layer = rawMapConfig.layers[layerIndex]; + if (layer.id) { return layer.id; } - var layerType = getType(layer.type); - var layerId = 'layer' + getLayerIndexByType(rawMapConfig, layerType, layerIndex); + const layerType = getType(layer.type); + + let layerId = `layer${getLayerIndexByType(rawMapConfig, layerType, layerIndex)}`; if (layerType !== 'mapnik') { - layerId = layerType + '-' + layerId; + layerId = `${layerType}-${layerId}`; } + return layerId; } constructor (config, datasource) { // TODO: inject defaults ? + this._id = null; this._cfg = config; + this._datasource = datasource; if (!semver.satisfies(this.version(), '>= 1.0.0 <= 1.8.0')) { - throw new Error('Unsupported layergroup configuration version ' + this.version()); + throw new Error(`Unsupported layergroup configuration version ${this.version()}`); } if (!Object.prototype.hasOwnProperty.call(this._cfg, 'layers')) { throw new Error('Missing layers array from layergroup config'); } - this._cfg.layers.forEach(function (layer, i) { + this._cfg.layers.forEach((layer, index) => { if (!Object.prototype.hasOwnProperty.call(layer, 'options')) { - throw new Error('Missing options from layer ' + i + ' of layergroup config'); + throw new Error(`Missing options from layer ${index} of layergroup config`); } + // NOTE: interactivity used to be a string as of version 1.0.0 if (Array.isArray(layer.options.interactivity)) { layer.options.interactivity = layer.options.interactivity.join(','); @@ -60,19 +66,13 @@ module.exports = class MapConfig { } }); } - - /** - * @type {Datasource} - */ - this._datasource = datasource; - - this._id = null; } serialize () { if (this._datasource.isEmpty()) { return JSON.stringify(this._cfg); } + return JSON.stringify({ cfg: this._cfg, ds: this._datasource.obj() @@ -106,11 +106,13 @@ module.exports = class MapConfig { } layerType (layerIndex) { - var lyr = this.getLayer(layerIndex); - if (!lyr) { + const layer = this.getLayer(layerIndex); + + if (!layer) { return undefined; } - return this.getType(lyr.type); + + return this.getType(layer.type); } getType (type) { @@ -180,20 +182,14 @@ module.exports = class MapConfig { return type === 'mapnik' && typeof sql === 'string' && cartocss === undefined && cartocssVersion === undefined; } - /***************************************************************************** - * Layers - ****************************************************************************/ - getLayerId (layerIndex) { return MapConfig.getLayerId(this._cfg, layerIndex); } getIndexByLayerId (layerId) { - var layers = this.getLayers(); - - for (var i = 0; i < layers.length; i++) { - if (layers[i].id === layerId) { - return i; + for (const [index, layer] of this.getLayers().entries()) { + if (layer.id === layerId) { + return index; } } @@ -205,32 +201,28 @@ module.exports = class MapConfig { } getLayers () { - return this._cfg.layers.map(function (_layer, layerIndex) { - return this.getLayer(layerIndex); - }.bind(this)); + return this._cfg.layers.map((_layer, layerIndex) => this.getLayer(layerIndex)); } - getLayerIndexByType (type, mapConfigLayerIdx) { - return getLayerIndexByType(this._cfg, type, mapConfigLayerIdx); + getLayerIndexByType (type, mapConfigLayerIndex) { + return getLayerIndexByType(this._cfg, type, mapConfigLayerIndex); } getLayerOption (layerIndex, optionName, defaultValue) { - var layerOption = defaultValue; - var layer = this.getLayer(layerIndex); + const layer = this.getLayer(layerIndex); + let layerOption = defaultValue; + if (layer && Object.prototype.hasOwnProperty.call(layer.options, optionName)) { layerOption = layer.options[optionName]; } + return layerOption; } - /***************************************************************************** - * Datasource - ****************************************************************************/ - getLayerDatasource (layerIndex) { - var datasource = this._datasource.getLayerDatasource(layerIndex) || {}; + const datasource = this._datasource.getLayerDatasource(layerIndex) || {}; + const layerSrid = this.getLayerOption(layerIndex, 'srid'); - var layerSrid = this.getLayerOption(layerIndex, 'srid'); if (layerSrid) { datasource.srid = layerSrid; } @@ -238,10 +230,6 @@ module.exports = class MapConfig { return datasource; } - /***************************************************************************** - * MVT - ****************************************************************************/ - getMVTExtents () { const layers = this.getLayers(); const extent = getTileExtent(layers); @@ -264,10 +252,6 @@ function isValidBufferSize (value) { return Number.isFinite(parseInt(value, 10)); } -/***************************************************************************** - * Layers - ****************************************************************************/ - function getLayerIndexByType (rawMapConfig, type, mapConfigLayerIdx) { var typeLayerIndex = 0; var mapConfigToTypeLayers = {}; @@ -281,10 +265,6 @@ function getLayerIndexByType (rawMapConfig, type, mapConfigLayerIdx) { return mapConfigToTypeLayers[mapConfigLayerIdx]; } -/***************************************************************************** - * MVT - ****************************************************************************/ - const DEFAULT_EXTENT = 4096; const DEFAULT_SIMPLIFY_EXTENT = 256; // Accepted values between 1 and 2^31 -1 (DEFAULT_MAX_EXTENT) @@ -309,7 +289,7 @@ function getSimplifyExtent (layers, vectorExtent) { }))]; if (extents.length > 1) { - throw new Error('Multiple simplify extent values in mapConfig (' + extents + ')'); + throw new Error(`Multiple simplify extent values in mapConfig (${extents})`); } if (undef === layers.length) { @@ -321,8 +301,7 @@ function getSimplifyExtent (layers, vectorExtent) { // Accepted values between 1 and max_extent const simplifyExtent = parseInt(extents[0]); if (!checkRange(simplifyExtent, DEFAULT_MIN_EXTENT, maxExtent)) { - throw new Error('Invalid vector_simplify_extent (' + simplifyExtent + '). ' + - 'Must be between 1 and vector_extent [' + maxExtent + ']'); + throw new Error(`Invalid vector_simplify_extent (${simplifyExtent}). Must be between 1 and vector_extent [${maxExtent}]`); } return simplifyExtent; @@ -342,7 +321,7 @@ function getTileExtent (layers) { }))]; if (layerExtents.length > 1) { - throw new Error('Multiple extent values in mapConfig (' + layerExtents + ')'); + throw new Error(`Multiple extent values in mapConfig (${layerExtents})`); } if (undef === layers.length) { @@ -351,7 +330,7 @@ function getTileExtent (layers) { const extent = parseInt(layerExtents[0]); if (!checkRange(extent, DEFAULT_MIN_EXTENT, DEFAULT_MAX_EXTENT)) { - throw new Error('Invalid vector_extent. Must be between 1 and ' + DEFAULT_MAX_EXTENT); + throw new Error(`Invalid vector_extent. Must be between 1 and ${DEFAULT_MAX_EXTENT}`); } return extent; From 63f068b84a2743340d5668454451d5c3ace3b94a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 17:16:29 +0200 Subject: [PATCH 37/45] ES2020 syntax sugar for mapconfig providers --- .../providers/dummy-mapconfig-provider.js | 22 ++--- .../providers/mapconfig-provider-proxy.js | 47 +++++----- .../providers/mapstore-mapconfig-provider.js | 86 ++++++++++--------- 3 files changed, 80 insertions(+), 75 deletions(-) diff --git a/lib/models/providers/dummy-mapconfig-provider.js b/lib/models/providers/dummy-mapconfig-provider.js index c089ddf62..b8cb8d1b6 100644 --- a/lib/models/providers/dummy-mapconfig-provider.js +++ b/lib/models/providers/dummy-mapconfig-provider.js @@ -1,18 +1,14 @@ 'use strict'; -var util = require('util'); -var MapStoreMapConfigProvider = require('./mapstore-mapconfig-provider'); +const MapStoreMapConfigProvider = require('./mapstore-mapconfig-provider'); -function DummyMapConfigProvider (mapConfig, params) { - MapStoreMapConfigProvider.call(this, undefined, params); +module.exports = class DummyMapConfigProvider extends MapStoreMapConfigProvider { + constructor (mapConfig, params) { + super(undefined, params); + this.mapConfig = mapConfig; + } - this.mapConfig = mapConfig; -} - -util.inherits(DummyMapConfigProvider, MapStoreMapConfigProvider); - -module.exports = DummyMapConfigProvider; - -DummyMapConfigProvider.prototype.getMapConfig = function (callback) { - return callback(null, this.mapConfig, this.params, {}); + getMapConfig (callback) { + return callback(null, this.mapConfig, this.params, {}); + } }; diff --git a/lib/models/providers/mapconfig-provider-proxy.js b/lib/models/providers/mapconfig-provider-proxy.js index fe20af8a8..8fc8b71d5 100644 --- a/lib/models/providers/mapconfig-provider-proxy.js +++ b/lib/models/providers/mapconfig-provider-proxy.js @@ -1,31 +1,34 @@ 'use strict'; -function MapConfigProviderProxy (mapConfigProvider, params) { - this.mapConfigProvider = mapConfigProvider; - this.params = params; -} +module.exports = class MapConfigProviderProxy { + constructor (mapConfigProvider, params) { + this.mapConfigProvider = mapConfigProvider; + this.params = params; + } -module.exports = MapConfigProviderProxy; + getMapConfig (callback) { + this.mapConfigProvider.getMapConfig((err, mapConfig, params, context) => { + if (err) { + return callback(err); + } -MapConfigProviderProxy.prototype.getMapConfig = function (callback) { - var self = this; - this.mapConfigProvider.getMapConfig(function (err, mapConfig, params, context) { - return callback(err, mapConfig, Object.assign({}, params, self.params), context); - }); -}; + return callback(null, mapConfig, Object.assign({}, params, this.params), context); + }); + } -MapConfigProviderProxy.prototype.getKey = function () { - return this.mapConfigProvider.getKey.apply(this); -}; + getKey () { + return this.mapConfigProvider.getKey.apply(this); + } -MapConfigProviderProxy.prototype.getCacheBuster = function () { - return this.mapConfigProvider.getCacheBuster.apply(this); -}; + getCacheBuster () { + return this.mapConfigProvider.getCacheBuster.apply(this); + } -MapConfigProviderProxy.prototype.filter = function () { - return this.mapConfigProvider.filter.apply(this, arguments); -}; + filter () { + return this.mapConfigProvider.filter.apply(this, arguments); + } -MapConfigProviderProxy.prototype.createKey = function () { - return this.mapConfigProvider.createKey.apply(this, arguments); + createKey () { + return this.mapConfigProvider.createKey.apply(this, arguments); + } }; diff --git a/lib/models/providers/mapstore-mapconfig-provider.js b/lib/models/providers/mapstore-mapconfig-provider.js index 40f9a1777..4d5b8f040 100644 --- a/lib/models/providers/mapstore-mapconfig-provider.js +++ b/lib/models/providers/mapstore-mapconfig-provider.js @@ -1,49 +1,55 @@ 'use strict'; -function MapStoreMapConfigProvider (mapStore, params) { - this.mapStore = mapStore; - this.params = params; - this.token = params.token; - this.cacheBuster = params.cache_buster || 0; -} - -module.exports = MapStoreMapConfigProvider; - -MapStoreMapConfigProvider.prototype.getMapConfig = function (callback) { - var self = this; - this.mapStore.load(this.token, function (err, mapConfig) { - return callback(err, mapConfig, self.params, {}); - }); -}; - -MapStoreMapConfigProvider.prototype.getKey = function () { - return createKey(this.params); -}; - -MapStoreMapConfigProvider.prototype.getCacheBuster = function () { - return this.cacheBuster; -}; - -MapStoreMapConfigProvider.prototype.filter = function (key) { - var regex = new RegExp('^' + createKey(this.params, true) + '.*'); - return key && key.match(regex); +module.exports = class MapStoreMapConfigProvider { + static createKey (...args) { + return createKey(...args); + } + + constructor (mapStore, params) { + this.mapStore = mapStore; + this.params = params; + this.token = params.token; + this.cacheBuster = params.cache_buster || 0; + } + + getMapConfig (callback) { + this.mapStore.load(this.token, (err, mapConfig) => { + if (err) { + return callback(err); + } + + return callback(null, mapConfig, this.params, {}); + }); + } + + getKey () { + return createKey(this.params); + } + + getCacheBuster () { + return this.cacheBuster; + } + + filter (key) { + const regex = new RegExp(`^${createKey(this.params, true)}.*`); + return key && key.match(regex); + } }; // Configure bases for cache keys suitable for string interpolation const baseKey = ctx => `${ctx.dbname}:${ctx.token}`; const renderKey = ctx => `${baseKey(ctx)}:${ctx.dbuser}:${ctx.format}:${ctx.layer}:${ctx.scale_factor}`; // Create a string ID/key from a set of params -function createKey (params, base) { - const ctx = Object.assign({}, { - dbname: '', - token: '', - dbuser: '', - format: '', - layer: '', - scale_factor: 1 - }, params); - - return base ? baseKey(ctx) : renderKey(ctx); -} +const defaultParams = { + dbname: '', + token: '', + dbuser: '', + format: '', + layer: '', + scale_factor: 1 +}; -module.exports.createKey = createKey; +function createKey (params, useBaseKey) { + const ctx = Object.assign({}, defaultParams, params); + return useBaseKey ? baseKey(ctx) : renderKey(ctx); +} From 5c3903961043677aadc83dceeaf982109dac46a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 17:18:38 +0200 Subject: [PATCH 38/45] Typo --- lib/models/providers/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/providers/README.md b/lib/models/providers/README.md index 24f826df8..ff50134a6 100644 --- a/lib/models/providers/README.md +++ b/lib/models/providers/README.md @@ -39,6 +39,6 @@ Returns a number representing the last modification time of the MapConfig so it' must be recreated or not. ```javascript -getKey() +getCacheBuster() ``` - @return `{Number}` the last modified time for the MapConfig, aka buster From d2d0a74469a43ef7d8b2b0244edc18065d850631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 17:32:05 +0200 Subject: [PATCH 39/45] ES2020 syntax sugar for layers metadata --- lib/metadata/empty-layer-metadata.js | 26 +++---- lib/metadata/index.js | 14 ++-- lib/metadata/layer-metadata.js | 106 +++++++++++++------------- lib/metadata/mapnik-layer-metadata.js | 26 +++---- lib/metadata/torque-layer-metadata.js | 71 +++++++++-------- 5 files changed, 120 insertions(+), 123 deletions(-) diff --git a/lib/metadata/empty-layer-metadata.js b/lib/metadata/empty-layer-metadata.js index 1f7b874a3..f5eb32924 100644 --- a/lib/metadata/empty-layer-metadata.js +++ b/lib/metadata/empty-layer-metadata.js @@ -1,17 +1,15 @@ 'use strict'; -function EmptyLayerMetadata (types) { - this._types = types || {}; -} - -EmptyLayerMetadata.prototype.is = function (type) { - return this._types[type] ? this._types[type] : false; -}; - -EmptyLayerMetadata.prototype.getMetadata = function (mapConfig, layer, layerId, params, rendererCache, callback) { - process.nextTick(function () { - callback(null, {}); - }); +module.exports = class EmptyLayerMetadata { + constructor (types) { + this._types = types || {}; + } + + is (type) { + return this._types[type] ? this._types[type] : false; + } + + getMetadata (mapConfig, layer, layerId, params, rendererCache, callback) { + process.nextTick(() => callback(null, {})); + } }; - -module.exports = EmptyLayerMetadata; diff --git a/lib/metadata/index.js b/lib/metadata/index.js index 969888657..f4f45fd49 100644 --- a/lib/metadata/index.js +++ b/lib/metadata/index.js @@ -1,14 +1,16 @@ 'use strict'; -var LayerMetadata = require('./layer-metadata'); -var EmptyLayerMetadata = require('./empty-layer-metadata'); -var MapnikLayerMetadata = require('./mapnik-layer-metadata'); -var TorqueLayerMetadata = require('./torque-layer-metadata'); +const LayerMetadata = require('./layer-metadata'); +const EmptyLayerMetadata = require('./empty-layer-metadata'); +const MapnikLayerMetadata = require('./mapnik-layer-metadata'); +const TorqueLayerMetadata = require('./torque-layer-metadata'); + +module.exports = function layerMetadataFactory () { + const layerMetadataIterator = []; -module.exports = function LayerMetadataFactory () { - var layerMetadataIterator = []; layerMetadataIterator.push(new EmptyLayerMetadata({ http: true, plain: true })); layerMetadataIterator.push(new MapnikLayerMetadata()); layerMetadataIterator.push(new TorqueLayerMetadata()); + return new LayerMetadata(layerMetadataIterator); }; diff --git a/lib/metadata/layer-metadata.js b/lib/metadata/layer-metadata.js index 077fa8257..d0dd3c6bf 100644 --- a/lib/metadata/layer-metadata.js +++ b/lib/metadata/layer-metadata.js @@ -1,70 +1,70 @@ 'use strict'; -function LayerMetadata (layerMetadataIterator) { - this.layerMetadataIterator = layerMetadataIterator; -} +module.exports = class LayerMetadata { + constructor (layerMetadataIterator) { + this.layerMetadataIterator = layerMetadataIterator; + } -LayerMetadata.prototype.getLayerMetadataFn = function (mapConfig, layerId) { - const layerType = mapConfig.layerType(layerId); - let getMetadadaFn; + getLayerMetadataFn (mapConfig, layerId) { + const layerType = mapConfig.layerType(layerId); + let getMetadadaFn; - for (const layerMetadata of this.layerMetadataIterator) { - if (layerMetadata.is(layerType)) { - getMetadadaFn = layerMetadata.getMetadata.bind(layerMetadata); - break; + for (const layerMetadata of this.layerMetadataIterator) { + if (layerMetadata.is(layerType)) { + getMetadadaFn = layerMetadata.getMetadata.bind(layerMetadata); + break; + } } - } - return getMetadadaFn; -}; + return getMetadadaFn; + } -LayerMetadata.prototype.getMetadata = function (rendererCache, params, mapConfig, callback) { - const metadataParams = mapConfig.getLayers() - .map((layer, layerId) => { - const getMetadata = this.getLayerMetadataFn(mapConfig, layerId); - return getMetadata ? { getMetadata, mapConfig, layer, layerId, params, rendererCache } : null; - }) - .filter(metadataParam => metadataParam !== null); + getMetadata (rendererCache, params, mapConfig, callback) { + const metadataParams = mapConfig.getLayers() + .map((layer, layerId) => { + const getMetadata = this.getLayerMetadataFn(mapConfig, layerId); + return getMetadata ? { getMetadata, mapConfig, layer, layerId, params, rendererCache } : null; + }) + .filter(metadataParam => metadataParam !== null); - if (!metadataParams.length) { - return callback(null, []); - } + if (!metadataParams.length) { + return callback(null, []); + } - return Promise.all(metadataParams.map(({ getMetadata, mapConfig, layer, layerId, params, rendererCache }) => { - return new Promise((resolve, reject) => { - getMetadata(mapConfig, layer, layerId, params, rendererCache, (err, metadata) => { - if (err) { - return reject(err); - } + return Promise.all(metadataParams.map(({ getMetadata, mapConfig, layer, layerId, params, rendererCache }) => { + return new Promise((resolve, reject) => { + getMetadata(mapConfig, layer, layerId, params, rendererCache, (err, metadata) => { + if (err) { + return reject(err); + } - return resolve(metadata); + return resolve(metadata); + }); }); - }); - })) - .then(results => { - if (!results.length) { - return callback(null, null); - } + })) + .then(results => { + if (!results.length) { + return callback(null, null); + } - const metadata = []; + const metadata = []; - mapConfig.getLayers().forEach(function (layer, layerIndex) { - var layerType = mapConfig.layerType(layerIndex); + mapConfig.getLayers().forEach(function (layer, layerIndex) { + var layerType = mapConfig.layerType(layerIndex); - metadata[layerIndex] = { - type: layerType, - id: mapConfig.getLayerId(layerIndex), - meta: results[layerIndex] - }; + metadata[layerIndex] = { + type: layerType, + id: mapConfig.getLayerId(layerIndex), + meta: results[layerIndex] + }; - if (layer.options.cartocss && metadata[layerIndex].meta) { - metadata[layerIndex].meta.cartocss = layer.options.cartocss; - } - }); + if (layer.options.cartocss && metadata[layerIndex].meta) { + metadata[layerIndex].meta.cartocss = layer.options.cartocss; + } + }); - return callback(null, metadata); - }) - .catch(err => callback(err)); + return callback(null, metadata); + }) + .catch(err => callback(err)); + } }; - -module.exports = LayerMetadata; diff --git a/lib/metadata/mapnik-layer-metadata.js b/lib/metadata/mapnik-layer-metadata.js index 2e885d30e..57812fafe 100644 --- a/lib/metadata/mapnik-layer-metadata.js +++ b/lib/metadata/mapnik-layer-metadata.js @@ -1,18 +1,18 @@ 'use strict'; -function MapnikLayerMetadata () { - this._types = { - mapnik: true, - cartodb: true - }; -} +module.exports = class MapnikLayerMetadata { + constructor () { + this._types = { + mapnik: true, + cartodb: true + }; + } -MapnikLayerMetadata.prototype.is = function (type) { - return this._types[type] ? this._types[type] : false; -}; + is (type) { + return this._types[type] ? this._types[type] : false; + } -MapnikLayerMetadata.prototype.getMetadata = function (mapConfig, layer, layerId, params, rendererCache, callback) { - return callback(null, { cartocss: layer.options.cartocss }); + getMetadata (mapConfig, layer, layerId, params, rendererCache, callback) { + return callback(null, { cartocss: layer.options.cartocss }); + } }; - -module.exports = MapnikLayerMetadata; diff --git a/lib/metadata/torque-layer-metadata.js b/lib/metadata/torque-layer-metadata.js index 073a47005..75b2c41e2 100644 --- a/lib/metadata/torque-layer-metadata.js +++ b/lib/metadata/torque-layer-metadata.js @@ -1,43 +1,40 @@ 'use strict'; -var DummyMapConfigProvider = require('../models/providers/dummy-mapconfig-provider'); - -function TorqueLayerMetadata () { - this._types = { - torque: true - }; -} - -TorqueLayerMetadata.prototype.is = function (type) { - return this._types[type] ? this._types[type] : false; -}; - -TorqueLayerMetadata.prototype.getMetadata = function (mapConfig, layer, layerId, params, rendererCache, callback) { - params = Object.assign({}, params, { - token: mapConfig.id(), - format: 'json.torque', - layer: layerId - }); - - const dummyMapConfigProvider = new DummyMapConfigProvider(mapConfig, params); - rendererCache.getRenderer(dummyMapConfigProvider, function (err, renderer) { - if (err) { - if (renderer) { - renderer.release(); - } - - return callback(err); - } - - renderer.getMetadata() - .then(meta => callback(null, meta)) - .catch(err => callback(err)) - .finally(() => { +const DummyMapConfigProvider = require('../models/providers/dummy-mapconfig-provider'); + +module.exports = class TorqueLayerMetadata { + constructor () { + this._types = { + torque: true + }; + } + + is (type) { + return this._types[type] ? this._types[type] : false; + } + + getMetadata (mapConfig, layer, layerId, params, rendererCache, callback) { + params = Object.assign({}, params, { + token: mapConfig.id(), + format: 'json.torque', + layer: layerId + }); + + const dummyMapConfigProvider = new DummyMapConfigProvider(mapConfig, params); + + rendererCache.getRenderer(dummyMapConfigProvider, (err, renderer) => { + if (err) { if (renderer) { renderer.release(); } - }); - }); -}; -module.exports = TorqueLayerMetadata; + return callback(err); + } + + renderer.getMetadata() + .then(meta => callback(null, meta)) + .catch(err => callback(err)) + .finally(() => renderer ? renderer.release() : undefined); + }); + } +}; From f8493540b01fa3c2a8e198a78419c899e5dffbbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 18:05:55 +0200 Subject: [PATCH 40/45] ES2020 syntax sugar for renderer cache --- lib/cache/cache-entry.js | 214 ++++++++++++++-------------- lib/cache/renderer-cache.js | 268 +++++++++++++++++------------------- 2 files changed, 237 insertions(+), 245 deletions(-) diff --git a/lib/cache/cache-entry.js b/lib/cache/cache-entry.js index c51e949ea..7a7ffd998 100644 --- a/lib/cache/cache-entry.js +++ b/lib/cache/cache-entry.js @@ -1,131 +1,131 @@ 'use strict'; const debug = require('debug')('windshaft:cache-entry'); - -function CacheEntry (cacheBuster, key = 'key-not-provided') { - this.ready = false; - this.err = null; - this.renderer = null; - this.ctime = this.atime = Date.now(); // last access time - this.cb = []; - this._refcount = 0; - this.cacheBuster = cacheBuster; - this.key = key; -} - -CacheEntry.prototype = Object.create(require('events').EventEmitter.prototype); - -CacheEntry.prototype.pushCallback = function (cb) { - this._addRef(); - if (this.ready) { - cb(this.err, this, true); - this.setLastUsed(Date.now()); // update last access time; - } else { - this.cb.push(cb); +const { EventEmitter } = require('events'); + +module.exports = class CacheEntry extends EventEmitter { + constructor (cacheBuster, key = 'key-not-provided') { + super(); + this.ready = false; + this.err = null; + this.renderer = null; + this.ctime = this.atime = Date.now(); // last access time + this.cb = []; + this._refcount = 0; + this.cacheBuster = cacheBuster; + this.key = key; } -}; - -// Get one tile from the service -CacheEntry.prototype.getTile = async function (format, z, x, y) { - return this.renderer.getTile(format, z, x, y); -}; - -CacheEntry.prototype.getMetadata = async function () { - return this.renderer.getMetadata(); -}; -// Get the contained entry -CacheEntry.prototype.get = function () { - return this.renderer.get(); -}; - -CacheEntry.prototype.setReady = function (err, renderer) { - // consistency check - if (this.ready) { - debug('Invalid call to CacheEntry.setReady on an ready entry'); - return; + pushCallback (callback) { + this._addRef(); + if (this.ready) { + callback(this.err, this, true); + this.setLastUsed(Date.now()); // update last access time; + } else { + this.cb.push(callback); + } } - this.ready = true; - this.err = err; - this.renderer = renderer; - - var cached = false; - var cb = this.cb.shift(); - while (cb) { - cb(err, this, cached); - cached = true; - cb = this.cb.shift(); + // Get one tile from the service + async getTile (format, z, x, y) { + return this.renderer.getTile(format, z, x, y); } - // TODO: update last access time here ? - this.emit('ready', this); - - if (err) { - this.emit('error', err); + async getMetadata () { + return this.renderer.getMetadata(); } -}; -// Call this as soon as you get a reference to the object -// if you plan to use it for longer than the current tick. -// Call dropRef when finished with it. -CacheEntry.prototype._addRef = function () { - ++this._refcount; -}; - -CacheEntry.prototype.release = function () { - this._dropRef(); -}; - -CacheEntry.prototype._dropRef = function () { - if (!--this._refcount) { - this._destroy(); + // Get the contained entry + get () { + return this.renderer.get(); } -}; -CacheEntry.prototype.setLastUsed = function (t) { - this.atime = t; -}; + setReady (err, renderer) { + // consistency check + if (this.ready) { + debug('Invalid call to CacheEntry.setReady on an ready entry'); + return; + } + + this.ready = true; + this.err = err; + this.renderer = renderer; + + let cached = false; + let callback = this.cb.shift(); + while (callback) { + callback(err, this, cached); + cached = true; + callback = this.cb.shift(); + } + + // TODO: update last access time here ? + this.emit('ready', this); + + if (err) { + this.emit('error', err); + } + } -CacheEntry.prototype.timeSinceLastAccess = function (now) { - return now - this.atime; -}; + // Call this as soon as you get a reference to the object + // if you plan to use it for longer than the current tick. + // Call dropRef when finished with it. + _addRef () { + ++this._refcount; + } -// Destroy (now or ASAP) a CacheEntry -// -// TODO: accept a callback ? -// -CacheEntry.prototype._destroy = function () { - if (this.cb.length) { - debug(`CacheEntry._destroy was called while still having ${this.cb.length} callbacks pending`); - return; + release () { + this._dropRef(); } - if (this._refcount) { - debug(`CacheEntry._destroy was called while still having ${this._refcount} references`); - return; + _dropRef () { + if (!--this._refcount) { + this._destroy(); + } } - if (!this.ready) { - // not ready yet, try later - debug(`cache_entry ${this.key} isn't ready yet, scheduling destruction on 'ready' event`); - this.on('ready', this._destroy.bind(this)); - return; + setLastUsed (t) { + this.atime = t; } - if (!this.renderer) { - debug(`Cache entry ${this.key} is ready but has no renderer, so nothing to do`); - return; + timeSinceLastAccess (now) { + return now - this.atime; } - // TODO: fixme: we can't modify the renderer here because - // tilelive-mapnik is keeping an internal cache and - // so modifying an object we'd be modifying something - // which will possibly be reused later. - // See https://github.com/mapbox/tilelive-mapnik/issues/47 - this.renderer.close(() => { - debug(`Mapnik Renderer ${this.key} closed due to its cache entry was released and it doesn't have references`); - }); + // Destroy (now or ASAP) a CacheEntry + // + // TODO: accept a callback ? + // + _destroy () { + if (this.cb.length) { + debug(`CacheEntry._destroy was called while still having ${this.cb.length} callbacks pending`); + return; + } + + if (this._refcount) { + debug(`CacheEntry._destroy was called while still having ${this._refcount} references`); + return; + } + + if (!this.ready) { + // not ready yet, try later + debug(`cache_entry ${this.key} isn't ready yet, scheduling destruction on 'ready' event`); + this.on('ready', this._destroy.bind(this)); + return; + } + + if (!this.renderer) { + debug(`Cache entry ${this.key} is ready but has no renderer, so nothing to do`); + return; + } + + // TODO: fixme: we can't modify the renderer here because + // tilelive-mapnik is keeping an internal cache and + // so modifying an object we'd be modifying something + // which will possibly be reused later. + // See https://github.com/mapbox/tilelive-mapnik/issues/47 + this.renderer.close(() => { + debug(`Mapnik Renderer ${this.key} closed due to its cache entry was released and it doesn't have references`); + }); + } }; - -module.exports = CacheEntry; diff --git a/lib/cache/renderer-cache.js b/lib/cache/renderer-cache.js index 0829bbed9..9a9b6c844 100644 --- a/lib/cache/renderer-cache.js +++ b/lib/cache/renderer-cache.js @@ -5,172 +5,164 @@ // - Purges the renderer objects after `{Number} options.timeout` ms of inactivity since the last cache entry access // Renderer objects are encapsulated inside a {CacheEntry} that tracks the last access time for each renderer -var EventEmitter = require('events').EventEmitter; -var util = require('util'); -var CacheEntry = require('./cache-entry'); -var debug = require('debug')('windshaft:renderercache'); - -function RendererCache (rendererFactory, options) { - if (!(this instanceof RendererCache)) { - return new RendererCache(rendererFactory, options); +const { EventEmitter } = require('events'); +const CacheEntry = require('./cache-entry'); +const debug = require('debug')('windshaft:renderercache'); + +module.exports = class RendererCache extends EventEmitter { + constructor (rendererFactory, options = {}) { + super(); + + this.renderers = {}; + this.timeout = options.timeout || options.ttl || 60000; + this.gcRun = 0; + this.rendererFactory = rendererFactory; + + setInterval(() => { + const now = Date.now(); + for (const [key, cacheEntry] of Object.entries(this.renderers)) { + if (cacheEntry.timeSinceLastAccess(now) > this.timeout) { + this.del(key); + } + } + }, this.timeout); } - EventEmitter.call(this); - - options = options || {}; + // If renderer cache entry exists at req-derived key, return it, + // else generate a new one and save at key. + // + // Caches lifetime is driven by the timeout passed at RendererCache + // construction time. + // + // @param callback will be called with (err, renderer) + // If `err` is null the renderer should be + // ready for you to use (calling getTile or getGrid). + // Note that the object is a proxy to the actual TileStore + // so you won't get the whole TileLive interface available. + // If you need that, use the .get() function. + // In order to reduce memory usage call renderer.release() + // when you're sure you won't need it anymore. + getRenderer (mapConfigProvider, callback) { + const cacheBuster = this.getCacheBusterValue(mapConfigProvider.getCacheBuster()); + const key = mapConfigProvider.getKey(); + let cacheEntry = this.renderers[key]; + + if (this.shouldRecreateRenderer(cacheEntry, cacheBuster)) { + cacheEntry = this.renderers[key] = new CacheEntry(cacheBuster, key); + cacheEntry._addRef(); // we add another ref for this.renderers[key] + + cacheEntry.on('error', (err) => { + debug('Removing RendererCache ' + key + ' on error ' + err); + this.emit('err', err); + this.del(key); + }); - this.renderers = {}; - this.timeout = options.timeout || options.ttl || 60000; + mapConfigProvider.getMapConfig((err, mapConfig, params, context) => { + if (err) { + this.del(key); + return callback(err); + } - this.gcRun = 0; + this.rendererFactory.getRenderer(mapConfig, params, context, cacheEntry.setReady.bind(cacheEntry)); + }); + } - this.rendererFactory = rendererFactory; + cacheEntry.pushCallback(callback); + }; - setInterval(function () { - var now = Date.now(); - Object.keys(this.renderers).forEach(function (key) { - var cacheEntry = this.renderers[key]; - if (cacheEntry.timeSinceLastAccess(now) > this.timeout) { - this.del(key); - } - }.bind(this)); - }.bind(this), this.timeout); -} - -util.inherits(RendererCache, EventEmitter); - -module.exports = RendererCache; - -// If renderer cache entry exists at req-derived key, return it, -// else generate a new one and save at key. -// -// Caches lifetime is driven by the timeout passed at RendererCache -// construction time. -// -// -// @param callback will be called with (err, renderer) -// If `err` is null the renderer should be -// ready for you to use (calling getTile or getGrid). -// Note that the object is a proxy to the actual TileStore -// so you won't get the whole TileLive interface available. -// If you need that, use the .get() function. -// In order to reduce memory usage call renderer.release() -// when you're sure you won't need it anymore. -RendererCache.prototype.getRenderer = function (mapConfigProvider, callback) { - var cacheBuster = this.getCacheBusterValue(mapConfigProvider.getCacheBuster()); - - // setup - var key = mapConfigProvider.getKey(); - - var cacheEntry = this.renderers[key]; - - if (this.shouldRecreateRenderer(cacheEntry, cacheBuster)) { - cacheEntry = this.renderers[key] = new CacheEntry(cacheBuster, key); - cacheEntry._addRef(); // we add another ref for this.renderers[key] - - var self = this; - - cacheEntry.on('error', function (err) { - debug('Removing RendererCache ' + key + ' on error ' + err); - self.emit('err', err); - self.del(key); - }); - - mapConfigProvider.getMapConfig(function makeRenderer (err, mapConfig, params, context) { - if (err) { - self.del(key); - return callback(err); - } - self.rendererFactory.getRenderer(mapConfig, params, context, cacheEntry.setReady.bind(cacheEntry)); - }); - } + getCacheBusterValue (cacheBuster) { + if (cacheBuster === undefined) { + return 0; + } - cacheEntry.pushCallback(callback); -}; + if (Number.isFinite(cacheBuster)) { + return Math.min(this._getMaxCacheBusterValue(), cacheBuster); + } -RendererCache.prototype.getCacheBusterValue = function (cacheBuster) { - if (cacheBuster === undefined) { - return 0; + return cacheBuster; } - if (Number.isFinite(cacheBuster)) { - return Math.min(this._getMaxCacheBusterValue(), cacheBuster); + + _getMaxCacheBusterValue () { + return Date.now(); } - return cacheBuster; -}; -RendererCache.prototype._getMaxCacheBusterValue = function () { - return Date.now(); -}; + shouldRecreateRenderer (cacheEntry, cacheBuster) { + if (cacheEntry) { + const entryCacheBuster = parseFloat(cacheEntry.cacheBuster); + const requestCacheBuster = parseFloat(cacheBuster); -RendererCache.prototype.shouldRecreateRenderer = function (cacheEntry, cacheBuster) { - if (cacheEntry) { - var entryCacheBuster = parseFloat(cacheEntry.cacheBuster); - var requestCacheBuster = parseFloat(cacheBuster); + if (isNaN(entryCacheBuster) || isNaN(requestCacheBuster)) { + return cacheEntry.cacheBuster !== cacheBuster; + } - if (isNaN(entryCacheBuster) || isNaN(requestCacheBuster)) { - return cacheEntry.cacheBuster !== cacheBuster; + return requestCacheBuster > entryCacheBuster; } - return requestCacheBuster > entryCacheBuster; + + return true; } - return true; -}; -// delete all renderers in cache -RendererCache.prototype.purge = function () { - Object.keys(this.renderers).forEach(this.del.bind(this)); -}; + // delete all renderers in cache + purge () { + for (const key of Object.keys(this.renderers)) { + this.del(key); + } + } -// Clears out all renderers related to a given database+token, regardless of other arguments -RendererCache.prototype.reset = function (mapConfigProvider) { - Object.keys(this.renderers) - .filter(mapConfigProvider.filter.bind(mapConfigProvider)) - .forEach(this.del.bind(this)); -}; + // Clears out all renderers related to a given database+token, regardless of other arguments + reset (mapConfigProvider) { + for (const key of Object.keys(this.renderers)) { + if (mapConfigProvider.filter(key)) { + this.del(key); + } + } + } + + // drain render pools, remove renderer and associated timeout calls + del (id) { + const cacheEntry = this.renderers[id]; -// drain render pools, remove renderer and associated timeout calls -RendererCache.prototype.del = function (id) { - var cacheEntry = this.renderers[id]; - if (cacheEntry) { - delete this.renderers[id]; - cacheEntry.release(); + if (cacheEntry) { + delete this.renderers[id]; + cacheEntry.release(); + } } -}; -RendererCache.prototype.getStats = function () { - const stats = new Map(); - const rendererCacheEntries = Object.entries(this.renderers); + getStats () { + const stats = new Map(); + const rendererCacheEntries = Object.entries(this.renderers); - stats.set('rendercache.count', rendererCacheEntries.length); + stats.set('rendercache.count', rendererCacheEntries.length); - return rendererCacheEntries.reduce((accumulatedStats, [cacheKey, cacheEntry]) => { - let format = cacheKey.split(':')[3]; // [dbname, token, dbuser, format, layer, scale] + return rendererCacheEntries.reduce((accumulatedStats, [cacheKey, cacheEntry]) => { + let format = cacheKey.split(':')[3]; // [dbname, token, dbuser, format, layer, scale] - format = format === '' ? 'no-format' : format; - format = format === 'grid.json' ? 'grid' : format.replace('.', '-'); + format = format === '' ? 'no-format' : format; + format = format === 'grid.json' ? 'grid' : format.replace('.', '-'); - const key = `rendercache.format.${format}`; + const key = `rendercache.format.${format}`; - if (accumulatedStats.has(key)) { - accumulatedStats.set(key, accumulatedStats.get(key) + 1); - } else { - accumulatedStats.set(key, 1); - } + if (accumulatedStats.has(key)) { + accumulatedStats.set(key, accumulatedStats.get(key) + 1); + } else { + accumulatedStats.set(key, 1); + } - // it might a cacheEntry has been released & removed from the cache during this process - const rendererStats = cacheEntry.renderer && cacheEntry.renderer.getStats && cacheEntry.renderer.getStats(); + // it might a cacheEntry has been released & removed from the cache during this process + const rendererStats = cacheEntry.renderer && cacheEntry.renderer.getStats && cacheEntry.renderer.getStats(); - if (!(rendererStats instanceof Map)) { - return accumulatedStats; - } + if (!(rendererStats instanceof Map)) { + return accumulatedStats; + } - for (const [stat, value] of rendererStats) { - if (accumulatedStats.has(stat)) { - accumulatedStats.set(stat, accumulatedStats.get(stat) + value); - } else { - accumulatedStats.set(stat, value); + for (const [stat, value] of rendererStats) { + if (accumulatedStats.has(stat)) { + accumulatedStats.set(stat, accumulatedStats.get(stat) + value); + } else { + accumulatedStats.set(stat, value); + } } - } - return accumulatedStats; - }, stats); + return accumulatedStats; + }, stats); + } }; From 30c48dfa078d5d76c5d9b8c7426576dc514658fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 18:33:49 +0200 Subject: [PATCH 41/45] ES2020 syntax sugar for backends --- lib/backends/README.md | 7 - lib/backends/attributes.js | 143 ++++++++++--------- lib/backends/index.js | 9 -- lib/backends/map-validator.js | 251 +++++++++++++++++----------------- lib/backends/map.js | 162 +++++++++++----------- lib/backends/preview.js | 68 ++++----- lib/backends/tile.js | 99 +++++++------- 7 files changed, 356 insertions(+), 383 deletions(-) delete mode 100644 lib/backends/README.md delete mode 100644 lib/backends/index.js diff --git a/lib/backends/README.md b/lib/backends/README.md deleted file mode 100644 index 0b1e44cbc..000000000 --- a/lib/backends/README.md +++ /dev/null @@ -1,7 +0,0 @@ -# About backend - -Backend implementations found here should not know anything about request/response objects, instead basic types should -be the parameters when working with any backend. Collaborators will be provided to the backend. - -The current state is not like that as for know we are moving code to where we think it fits better, removing that kind -of dependencies should be addressed in the future. \ No newline at end of file diff --git a/lib/backends/attributes.js b/lib/backends/attributes.js index e7dce5426..0098a2bea 100644 --- a/lib/backends/attributes.js +++ b/lib/backends/attributes.js @@ -1,94 +1,89 @@ 'use strict'; const PSQL = require('cartodb-psql'); - const parseDbParams = require('../renderers/renderer-params'); const Timer = require('../stats/timer'); const SubstitutionTokens = require('cartodb-query-tables').utils.substitutionTokens; -function AttributesBackend () {} - -module.exports = AttributesBackend; - -/// Gets attributes for a given layer feature -// -/// Calls req2params, then expects parameters: -/// -/// * token - MapConfig identifier -/// * layer - Layer number -/// * fid - Feature identifier -/// -/// The referenced layer must have been configured -/// to allow for attributes fetching. -/// See https://github.com/CartoDB/Windshaft/wiki/MapConfig-1.1.0 -/// -/// @param testMode if true generates a call returning requested -/// columns plus the fid column of the first record -/// it is only meant to check validity of configuration -/// -AttributesBackend.prototype.getFeatureAttributes = function (mapConfigProvider, params, testMode, callback) { - const timer = new Timer(); - - mapConfigProvider.getMapConfig((err, mapConfig) => { - if (err) { - return callback(err); - } - - const layer = mapConfig.getLayer(params.layer); - if (!layer) { - const error = new Error(`Map ${params.token} has no layer number ${params.layer}`); - return callback(error); - } +module.exports = class AttributesBackend { + // Gets attributes for a given layer feature + // + // * token - MapConfig identifier + // * layer - Layer number + // * fid - Feature identifier + // + // The referenced layer must have been configured + // to allow for attributes fetching. + // See https://github.com/CartoDB/Windshaft/wiki/MapConfig-1.1.0 + // + // @param testMode if true generates a call returning requested + // columns plus the fid column of the first record + // it is only meant to check validity of configuration + // + getFeatureAttributes (mapConfigProvider, params, testMode, callback) { + const timer = new Timer(); + + mapConfigProvider.getMapConfig((err, mapConfig) => { + if (err) { + return callback(err); + } - timer.start('getAttributes'); - const attributes = layer.options.attributes; - if (!attributes) { - const error = new Error(`Layer ${params.layer} has no exposed attributes`); - return callback(error); - } + const layer = mapConfig.getLayer(params.layer); + if (!layer) { + const error = new Error(`Map ${params.token} has no layer number ${params.layer}`); + return callback(error); + } - const dbParams = Object.assign( - {}, - parseDbParams(params), - mapConfig.getLayerDatasource(params.layer) - ); - - let pg; - try { - pg = new PSQL(dbParams); - } catch (error) { - return callback(error); - } + timer.start('getAttributes'); + const attributes = layer.options.attributes; + if (!attributes) { + const error = new Error(`Layer ${params.layer} has no exposed attributes`); + return callback(error); + } - const sql = getSQL(attributes, pg, layer, params, testMode); + const dbParams = Object.assign( + {}, + parseDbParams(params), + mapConfig.getLayerDatasource(params.layer) + ); + + let pg; + try { + pg = new PSQL(dbParams); + } catch (error) { + return callback(error); + } - pg.query(sql, (err, data) => { - timer.end('getAttributes'); + const sql = getSQL(attributes, pg, layer, params, testMode); - if (err) { - return callback(err); - } + pg.query(sql, (err, data) => { + timer.end('getAttributes'); - if (testMode) { - return callback(null, null, timer.getTimes()); - } + if (err) { + return callback(err); + } - const featureAttributes = extractFeatureAttributes(data.rows); + if (testMode) { + return callback(null, null, timer.getTimes()); + } - if (!featureAttributes) { - const rowsLengthError = new Error( - `Multiple features (${data.rows.length}) identified by ` + - `'${attributes.id}' = ${params.fid} in layer ${params.layer}` - ); - if (!data.rows.length) { - rowsLengthError.http_status = 404; + const featureAttributes = extractFeatureAttributes(data.rows); + + if (!featureAttributes) { + const rowsLengthError = new Error( + `Multiple features (${data.rows.length}) identified by ` + + `'${attributes.id}' = ${params.fid} in layer ${params.layer}` + ); + if (!data.rows.length) { + rowsLengthError.http_status = 404; + } + return callback(rowsLengthError); } - return callback(rowsLengthError); - } - return callback(null, featureAttributes, timer.getTimes()); - }, true); // use read-only transaction - }); + return callback(null, featureAttributes, timer.getTimes()); + }, true); // use read-only transaction + }); + } }; function getSQL (attributes, pg, layer, params, testMode) { diff --git a/lib/backends/index.js b/lib/backends/index.js deleted file mode 100644 index ddbd0d7e0..000000000 --- a/lib/backends/index.js +++ /dev/null @@ -1,9 +0,0 @@ -'use strict'; - -module.exports = { - Attributes: require('./attributes'), - Map: require('./map'), - MapValidator: require('./map-validator'), - Preview: require('./preview'), - Tile: require('./tile') -}; diff --git a/lib/backends/map-validator.js b/lib/backends/map-validator.js index bb5293dae..a8cb67709 100644 --- a/lib/backends/map-validator.js +++ b/lib/backends/map-validator.js @@ -5,106 +5,153 @@ const INCOMPATIBLE_LAYERS_ERROR = 'The `mapnik` or `cartodb` layers must be cons const MapConfigProviderProxy = require('../models/providers/mapconfig-provider-proxy'); -/** - * @param {TileBackend} tileBackend - * @param {AttributesBackend} attributesBackend - * @constructor - * @type {MapValidatorBackend} - */ -function MapValidatorBackend (tileBackend, attributesBackend) { - this.tileBackend = tileBackend; - this.attributesBackend = attributesBackend; -} +module.exports = class MapValidatorBackend { + constructor (tileBackend, attributesBackend) { + this.tileBackend = tileBackend; + this.attributesBackend = attributesBackend; + } -module.exports = MapValidatorBackend; + validate (mapConfigProvider, callback) { + mapConfigProvider.getMapConfig((err, mapConfig, params) => { + if (err) { + return callback(err, false); + } -MapValidatorBackend.prototype.validate = function (mapConfigProvider, callback) { - mapConfigProvider.getMapConfig((err, mapConfig, params) => { - if (err) { - return callback(err, false); - } + const token = mapConfig.id(); - var token = mapConfig.id(); + if (mapConfig.isVectorOnlyMapConfig()) { + return this.validateVectorLayergroup(mapConfigProvider, params, token) + .then(() => callback(null, true)) + .catch(err => callback(err, false)); + } - if (mapConfig.isVectorOnlyMapConfig()) { - return this.validateVectorLayergroup(mapConfigProvider, params, token) - .then(() => callback(null, true)) - .catch(err => callback(err, false)); - } + if (mapConfig.hasIncompatibleLayers()) { + const error = new Error(INCOMPATIBLE_LAYERS_ERROR); + error.type = 'mapconfig'; + error.http_status = 400; + return callback(error); + } - if (mapConfig.hasIncompatibleLayers()) { - const error = new Error(INCOMPATIBLE_LAYERS_ERROR); - error.type = 'mapconfig'; - error.http_status = 400; - return callback(error); - } + const mapnikLayersIds = getMapnikLayerIds(mapConfig); - var mapnikLayersIds = getMapnikLayerIds(mapConfig); + if (!mapnikLayersIds.length) { + return this._validateRemaningFormatLayers(mapConfigProvider, mapConfig, params) + .then(() => callback(null, true)) + .catch(err => callback(err, false)); + } - if (!mapnikLayersIds.length) { - return this._validateRemaningFormatLayers(mapConfigProvider, mapConfig, params) + let allLayers; // if layer is undefined then it fetchs all layers + this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'png', allLayers) + .then(() => this._validateRemaningFormatLayers(mapConfigProvider, mapConfig, params)) .then(() => callback(null, true)) - .catch(err => callback(err, false)); - } - - let allLayers; // if layer is undefined then it fetchs all layers - this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'png', allLayers) - .then(() => this._validateRemaningFormatLayers(mapConfigProvider, mapConfig, params)) - .then(() => callback(null, true)) - .catch(errAllLayers => { - const checkFailingMapnikTileLayerFns = mapnikLayersIds.map(layerId => - this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'png', layerId) - ); - - if (!checkFailingMapnikTileLayerFns.length) { - return callback(errAllLayers, !errAllLayers); - } + .catch(errAllLayers => { + const checkFailingMapnikTileLayerFns = mapnikLayersIds.map(layerId => + this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'png', layerId) + ); + + if (!checkFailingMapnikTileLayerFns.length) { + return callback(errAllLayers, !errAllLayers); + } + + return Promise.all(checkFailingMapnikTileLayerFns) + .then(() => callback(errAllLayers, false)) + .catch(err => callback(err, false)); + }); + }); + } - return Promise.all(checkFailingMapnikTileLayerFns) - .then(() => callback(errAllLayers, false)) - .catch(err => callback(err, false)); - }); - }); -}; + _validateRemaningFormatLayers (mapConfigProvider, mapConfig, params) { + const token = mapConfig.id(); + const validateRemainingFormatLayerPromises = []; -MapValidatorBackend.prototype._validateRemaningFormatLayers = function (mapConfigProvider, mapConfig, params) { - const token = mapConfig.id(); - const validateRemainingFormatLayerPromises = []; + mapConfig.getLayers().forEach((layer, layerId) => { + const layerOptions = layer.options; + const layerType = mapConfig.layerType(layerId); - mapConfig.getLayers().forEach((layer, layerId) => { - var lyropt = layer.options; - var layerType = mapConfig.layerType(layerId); + if (layerType === 'mapnik') { + if (layerOptions.interactivity) { + validateRemainingFormatLayerPromises.push( + this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'grid.json', layerId) + ); + } + } else if (layerType === 'torque') { + validateRemainingFormatLayerPromises.push( + this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'json.torque', layerId) + ); + } - if (layerType === 'mapnik') { - if (lyropt.interactivity) { + // both 'cartodb' or 'torque' types can have attributes + // attribute validation is usually very inefficient when sql_raw if present so we skip it + if (layerOptions.attributes && !layerOptions.sql_raw) { validateRemainingFormatLayerPromises.push( - this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'grid.json', layerId) + this.tryFetchFeatureAttributes(mapConfigProvider, params, token, layerId) ); } - } else if (layerType === 'torque') { - validateRemainingFormatLayerPromises.push( - this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'json.torque', layerId) - ); - } + }); - // both 'cartodb' or 'torque' types can have attributes - // attribute validation is usually very inefficient when sql_raw if present so we skip it - if (lyropt.attributes && !lyropt.sql_raw) { - validateRemainingFormatLayerPromises.push( - this.tryFetchFeatureAttributes(mapConfigProvider, params, token, layerId) - ); + if (!validateRemainingFormatLayerPromises.length) { + return Promise.resolve(); } - }); - if (!validateRemainingFormatLayerPromises.length) { - return Promise.resolve(); + return Promise.all(validateRemainingFormatLayerPromises); } - return Promise.all(validateRemainingFormatLayerPromises); + tryFetchTileOrGrid (mapConfigProvider, _params, token, format, layerId) { + const params = Object.assign({}, _params); + params.token = token; + params.format = format; + params.layer = layerId; + params.x = 0; + params.y = 0; + params.z = 30; + + return new Promise((resolve, reject) => { + const mapConfigProviderProxy = new MapConfigProviderProxy(mapConfigProvider, params); + + this.tileBackend.getTile(mapConfigProviderProxy, params, (err) => { + if (err) { + // Grainstore returns styles error indicating layer index, since validation is performed + // one by one instead of all blended layers of MapConfig, this fixes the error message to + // show the right layer index. + if (err.message.match(/^style0: CartoCSS is empty/) && layerId > 0) { + err.message = err.message.replace(/style0/i, `style${layerId}`); + } + err.layerIndex = layerId; + + return reject(err); + } + + return resolve(); + }); + }); + } + + tryFetchFeatureAttributes (mapConfigProvider, _params, token, layernum) { + const params = Object.assign({}, _params); + params.token = token; + params.layer = layernum; + + const proxyProvider = new MapConfigProviderProxy(mapConfigProvider, params); + + return new Promise((resolve, reject) => { + this.attributesBackend.getFeatureAttributes(proxyProvider, params, true, err => { + if (err) { + err.layerIndex = layernum; + return reject(err); + } + resolve(); + }); + }); + } + + validateVectorLayergroup (mapConfigProvider, params, token) { + let allLayers; // if layer is undefined then it fetchs all layers + return this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'mvt', allLayers); + } }; function getMapnikLayerIds (mapConfig) { - var mapnikLayerIds = []; + const mapnikLayerIds = []; mapConfig.getLayers().forEach(function (layer, layerId) { if (mapConfig.layerType(layerId) === 'mapnik') { @@ -114,53 +161,3 @@ function getMapnikLayerIds (mapConfig) { return mapnikLayerIds; } - -MapValidatorBackend.prototype.tryFetchTileOrGrid = function (mapConfigProvider, _params, token, format, layerId) { - const params = Object.assign({}, _params); - params.token = token; - params.format = format; - params.layer = layerId; - params.x = 0; - params.y = 0; - params.z = 30; - - return new Promise((resolve, reject) => { - this.tileBackend.getTile(new MapConfigProviderProxy(mapConfigProvider, params), params, function (err) { - if (err) { - // Grainstore returns styles error indicating layer index, since validation is performed - // one by one instead of all blended layers of MapConfig, this fixes the error message to - // show the right layer index. - if (err.message.match(/^style0: CartoCSS is empty/) && layerId > 0) { - err.message = err.message.replace(/style0/i, 'style' + layerId); - } - err.layerIndex = layerId; - - return reject(err); - } - return resolve(); - }); - }); -}; - -MapValidatorBackend.prototype.tryFetchFeatureAttributes = function (mapConfigProvider, _params, token, layernum) { - const params = Object.assign({}, _params); - params.token = token; - params.layer = layernum; - - const proxyProvider = new MapConfigProviderProxy(mapConfigProvider, params); - - return new Promise((resolve, reject) => { - this.attributesBackend.getFeatureAttributes(proxyProvider, params, true, err => { - if (err) { - err.layerIndex = layernum; - return reject(err); - } - resolve(); - }); - }); -}; - -MapValidatorBackend.prototype.validateVectorLayergroup = function (mapConfigProvider, params, token) { - let allLayers; // if layer is undefined then it fetchs all layers - return this.tryFetchTileOrGrid(mapConfigProvider, params, token, 'mvt', allLayers); -}; diff --git a/lib/backends/map.js b/lib/backends/map.js index ff9e7683f..3c177c344 100644 --- a/lib/backends/map.js +++ b/lib/backends/map.js @@ -4,113 +4,107 @@ const debug = require('debug')('windshaft:backend:map'); const Timer = require('../stats/timer'); const layerMetadataFactory = require('../metadata'); -/** - * @param {RendererCache} rendererCache - * @param {MapStore} mapStore - * @param {MapValidator} mapValidator - * @constructor - */ -function MapBackend (rendererCache, mapStore, mapValidator) { - this._rendererCache = rendererCache; - this._mapStore = mapStore; - this._mapValidator = mapValidator; - this._layerMetadata = layerMetadataFactory(); -} +module.exports = class MapBackend { + constructor (rendererCache, mapStore, mapValidator) { + this._rendererCache = rendererCache; + this._mapStore = mapStore; + this._mapValidator = mapValidator; + this._layerMetadata = layerMetadataFactory(); + } -module.exports = MapBackend; + createLayergroup (mapConfig, params, validatorMapConfigProvider, callback) { + const timer = new Timer(); + timer.start('createLayergroup'); + + // Inject db parameters into the configuration + // to ensure getting different identifiers for + // maps created against different databases + // or users. See + // https://github.com/CartoDB/Windshaft/issues/163 + mapConfig.setDbParams({ + name: params.dbname, + user: params.dbuser + }); -MapBackend.prototype.createLayergroup = function (mapConfig, params, validatorMapConfigProvider, callback) { - const timer = new Timer(); - timer.start('createLayergroup'); + timer.start('mapSave'); + // will save only if successful + this._mapStore.save(mapConfig, (err, mapConfigId, known) => { + timer.end('mapSave'); - // Inject db parameters into the configuration - // to ensure getting different identifiers for - // maps created against different databases - // or users. See - // https://github.com/CartoDB/Windshaft/issues/163 - mapConfig.setDbParams({ - name: params.dbname, - user: params.dbuser - }); + if (err) { + return callback(err); + } - timer.start('mapSave'); - // will save only if successful - this._mapStore.save(mapConfig, (err, mapConfigId, known) => { - timer.end('mapSave'); + if (known) { + return this._getLayergroupMetadata(params, mapConfig, timer, callback); + } - if (err) { - return callback(err); - } + timer.start('validate'); + this._mapValidator.validate(validatorMapConfigProvider, (err, isValid) => { + timer.end('validate'); - if (known) { - return this._getLayergroupMetadata(params, mapConfig, timer, callback); - } + if (err || !isValid) { + return this._deleteMapConfigAfterError(err, mapConfig.id(), callback); + } - timer.start('validate'); - this._mapValidator.validate(validatorMapConfigProvider, (err, isValid) => { - timer.end('validate'); + return this._getLayergroupMetadata(params, mapConfig, timer, callback); + }); + }); + } - if (err || !isValid) { + _getLayergroupMetadata (params, mapConfig, timer, callback) { + timer.start('layer-metadata'); + return this._getLayersMetadata(params, mapConfig, (err, layergroupData) => { + timer.end('layer-metadata'); + + if (err) { return this._deleteMapConfigAfterError(err, mapConfig.id(), callback); } - return this._getLayergroupMetadata(params, mapConfig, timer, callback); + timer.end('createLayergroup'); + return callback(null, layergroupData, timer.getTimes()); }); - }); -}; - -MapBackend.prototype._getLayergroupMetadata = function (params, mapConfig, timer, callback) { - timer.start('layer-metadata'); - return this._getLayersMetadata(params, mapConfig, (err, layergroupData) => { - timer.end('layer-metadata'); - - if (err) { - return this._deleteMapConfigAfterError(err, mapConfig.id(), callback); - } - - timer.end('createLayergroup'); - return callback(null, layergroupData, timer.getTimes()); - }); -}; + } -MapBackend.prototype._getLayersMetadata = function (params, mapConfig, callback) { - const layergroupData = { - layergroupid: mapConfig.id() - }; + _getLayersMetadata (params, mapConfig, callback) { + const layergroupData = { + layergroupid: mapConfig.id() + }; - this._layerMetadata.getMetadata(this._rendererCache, params, mapConfig, (err, layersMetadata) => { - if (err) { - return callback(err); - } + this._layerMetadata.getMetadata(this._rendererCache, params, mapConfig, (err, layersMetadata) => { + if (err) { + return callback(err); + } - if (layersMetadata) { - layergroupData.metadata = { - layers: layersMetadata - }; + if (layersMetadata) { + layergroupData.metadata = { + layers: layersMetadata + }; - // backwards compatibility for torque - const torqueMetadata = getTorqueMetadata(layersMetadata); - if (torqueMetadata) { - layergroupData.metadata.torque = torqueMetadata; + // backwards compatibility for torque + const torqueMetadata = getTorqueMetadata(layersMetadata); + if (torqueMetadata) { + layergroupData.metadata.torque = torqueMetadata; + } } - } - return callback(null, layergroupData); - }); -}; + return callback(null, layergroupData); + }); + } -MapBackend.prototype._deleteMapConfigAfterError = function (err, mapConfigId, callback) { - this._mapStore.del(mapConfigId, function (delErr) { - if (delErr) { - debug(`Failed to delete MapConfig '${mapConfigId}' after: ${err.message}`); - } + _deleteMapConfigAfterError (err, mapConfigId, callback) { + this._mapStore.del(mapConfigId, function (delErr) { + if (delErr) { + debug(`Failed to delete MapConfig '${mapConfigId}' after: ${err.message}`); + } - return callback(err); - }); + return callback(err); + }); + } }; function getTorqueMetadata (layersMetadata) { - const torqueMetadata = layersMetadata.reduce(function (acc, layer, layerId) { + const torqueMetadata = layersMetadata.reduce((acc, layer, layerId) => { if (layer.type === 'torque') { acc[layerId] = layer.meta; } diff --git a/lib/backends/preview.js b/lib/backends/preview.js index 5d6de1a37..3dd4fdd14 100644 --- a/lib/backends/preview.js +++ b/lib/backends/preview.js @@ -2,40 +2,40 @@ const { preview } = require('@carto/cartonik'); -function PreviewBackend (rendererCache, options) { - this._rendererCache = rendererCache; - this._options = options || {}; -} - -module.exports = PreviewBackend; - -PreviewBackend.prototype.getImage = function (options, callback) { - const { mapConfigProvider, format, width, height, zoom, center, bbox } = options; - - this._rendererCache.getRenderer(mapConfigProvider, (err, renderer) => { - if (err) { - if (renderer) { - renderer.release(); +module.exports = class PreviewBackend { + constructor (rendererCache, options = {}) { + this._rendererCache = rendererCache; + this._options = options; + } + + getImage (options, callback) { + const { mapConfigProvider, format, width, height, zoom, center, bbox } = options; + + this._rendererCache.getRenderer(mapConfigProvider, (err, renderer) => { + if (err) { + if (renderer) { + renderer.release(); + } + + return callback(err); } - return callback(err); - } - - const options = { - zoom: zoom, - scale: 1, - center: center, - dimensions: { width, height }, - bbox: bbox, - format: format, - getTile: renderer.getTile.bind(renderer), - limit: (this._options.imageSizeLimit || 8192) + 1, - concurrency: this._options.concurrency - }; - - preview(options) - .then(({ image, stats }) => callback(null, image, stats)) - .catch((err) => callback(err)) - .finally(() => renderer.release()); - }); + const options = { + zoom: zoom, + scale: 1, + center: center, + dimensions: { width, height }, + bbox: bbox, + format: format, + getTile: renderer.getTile.bind(renderer), + limit: (this._options.imageSizeLimit || 8192) + 1, + concurrency: this._options.concurrency + }; + + preview(options) + .then(({ image, stats }) => callback(null, image, stats)) + .catch((err) => callback(err)) + .finally(() => renderer.release()); + }); + } }; diff --git a/lib/backends/tile.js b/lib/backends/tile.js index f6e6f0a4a..c5113b9c1 100644 --- a/lib/backends/tile.js +++ b/lib/backends/tile.js @@ -2,60 +2,63 @@ const Timer = require('../stats/timer'); -function TileBackend (rendererCache) { - this._rendererCache = rendererCache; -} - -module.exports = TileBackend; - -// Gets a tile for a given set of tile ZXY coords. (OSM style) -// Call with .png for images, or .grid.json for UTFGrid tiles -// -// query string arguments: -// -// * sql - use SQL to filter displayed results or perform operations pre-render -// * style - assign a per tile style using carto -// * interactivity - specify which columns to represent in the UTFGrid -// * cache_buster - specify to ensure a new renderer is used -// * geom_type - specify default style to use if no style present -TileBackend.prototype.getTile = function (mapConfigProvider, params, callback) { - if (params.format === 'grid.json' && !params.interactivity) { - if (!params.token) { // token embeds interactivity - return callback(new Error('Missing interactivity parameter')); - } +module.exports = class TileBackend { + constructor (rendererCache) { + this._rendererCache = rendererCache; } - const timer = new Timer(); - const extraHeaders = {}; - timer.start('getTileOrGrid'); - timer.start('getRenderer'); - this._rendererCache.getRenderer(mapConfigProvider, function (err, renderer, isCached) { - timer.end('getRenderer'); - if (err) { - if (renderer) { - renderer.release(); - } - - return callback(err); - } + // Gets a tile for a given set of tile ZXY coords. (OSM style) + // Call with .png for images, or .grid.json for UTFGrid tiles + // + // query string arguments: + // + // * sql - use SQL to filter displayed results or perform operations pre-render + // * style - assign a per tile style using carto + // * interactivity - specify which columns to represent in the UTFGrid + // * cache_buster - specify to ensure a new renderer is used + // * geom_type - specify default style to use if no style present + getTile (mapConfigProvider, params, callback) { + const { format, interactivity, token, z, x, y } = params; - if (isCached) { - extraHeaders['X-Windshaft-Cache'] = Date.now() - renderer.ctime; + // TODO: review if needed see cartonik's validations + if (format === 'grid.json' && !interactivity) { + if (!token) { // token embeds interactivity + return callback(new Error('Missing interactivity parameter')); + } } - timer.start('render-' + params.format.replace('.', '-')); - renderer.getTile(params.format, +params.z, +params.x, +params.y) - .then(({ buffer, headers, stats }) => { - timer.end('render-' + params.format.replace('.', '-')); - timer.end('getTileOrGrid'); - - return callback(null, buffer, Object.assign(extraHeaders, headers), Object.assign(timer.getTimes(), stats)); - }) - .catch((err) => callback(err)) - .finally(() => { + const timer = new Timer(); + const extraHeaders = {}; + timer.start('getTileOrGrid'); + timer.start('getRenderer'); + this._rendererCache.getRenderer(mapConfigProvider, (err, renderer, isCached) => { + timer.end('getRenderer'); + if (err) { if (renderer) { renderer.release(); } - }); - }); + + return callback(err); + } + + if (isCached) { + extraHeaders['X-Windshaft-Cache'] = Date.now() - renderer.ctime; + } + + timer.start('render-' + format.replace('.', '-')); + renderer.getTile(format, +z, +x, +y) + .then(({ buffer, headers, stats }) => { + timer.end('render-' + format.replace('.', '-')); + timer.end('getTileOrGrid'); + + return callback(null, buffer, Object.assign(extraHeaders, headers), Object.assign(timer.getTimes(), stats)); + }) + .catch((err) => callback(err)) + .finally(() => { + if (renderer) { + renderer.release(); + } + }); + }); + } }; From 03fd10908e9773cf47c1928e33ef415169cece41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 19:09:00 +0200 Subject: [PATCH 42/45] ES2020 syntax sugar for utilities --- lib/renderers/renderer-factory.js | 6 ++- lib/utils/default-query-rewriter.js | 18 ++----- lib/utils/format.js | 18 +++---- lib/utils/layer-columns.js | 50 ------------------ lib/utils/layer-filter.js | 68 ++++++++++++------------ test/unit/utils/layer-columns-test.js | 75 --------------------------- 6 files changed, 52 insertions(+), 183 deletions(-) delete mode 100644 lib/utils/layer-columns.js delete mode 100644 test/unit/utils/layer-columns-test.js diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index 9ec3cd1cc..431de69e6 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -69,7 +69,7 @@ module.exports = class RendererFactory { return MapnikRendererFactory.NAME; } - if (layersFilter.isSingleLayer(layer)) { + if (isSingleLayer(layer)) { return mapConfig.layerType(layer); } @@ -87,3 +87,7 @@ function isMapnikFactory (mapConfig, layer) { return false; } } + +function isSingleLayer (layerFilter) { + return Number.isFinite(+layerFilter); +} diff --git a/lib/utils/default-query-rewriter.js b/lib/utils/default-query-rewriter.js index 8bae41d6d..4bf5e250a 100644 --- a/lib/utils/default-query-rewriter.js +++ b/lib/utils/default-query-rewriter.js @@ -1,17 +1,7 @@ 'use strict'; -// Dummy query-rewriter which doesn't alter queries - -// This class implements the Windshaft query rewriting API: -// -// * query(sql, data) // transform SQL query, with additional data -// -function DefaultQueryRewriter () { -} - -module.exports = DefaultQueryRewriter; - -DefaultQueryRewriter.prototype.query = function (query, data) { - // Not using data parameter, but declared here for documentation purposes - return query; +module.exports = class DefaultQueryRewriter { + query (query) { + return query; + } }; diff --git a/lib/utils/format.js b/lib/utils/format.js index 0d6acfa78..37d368470 100644 --- a/lib/utils/format.js +++ b/lib/utils/format.js @@ -1,15 +1,13 @@ 'use strict'; -function format (str) { - var replacements = Array.prototype.slice.call(arguments, 1); +module.exports = function format (str) { + const replacements = Array.prototype.slice.call(arguments, 1); - replacements.forEach(function (attrs) { - Object.keys(attrs).forEach(function (attr) { - str = str.replace(new RegExp('\\{' + attr + '\\}', 'g'), attrs[attr]); - }); - }); + for (const attrs of replacements) { + for (const [key, attr] of Object.entries(attrs)) { + str = str.replace(new RegExp(`\\{${key}\\}`, 'g'), attr); + } + } return str; -} - -module.exports = format; +}; diff --git a/lib/utils/layer-columns.js b/lib/utils/layer-columns.js deleted file mode 100644 index 50f3e9e77..000000000 --- a/lib/utils/layer-columns.js +++ /dev/null @@ -1,50 +0,0 @@ -'use strict'; - -var cartocssUtils = require('./cartocss-utils'); - -var EXCLUDE_PROPERTIES = { - 'mapnik::geometry_type': true, - 'mapnik-geometry-type': true -}; - -module.exports.getColumns = function (layerOptions) { - var columns = []; - - if (Array.isArray(layerOptions.columns)) { - columns = layerOptions.columns; - } - - if (typeof layerOptions.cartocss === 'string') { - columns = columns.concat(cartocssUtils.getColumnNamesFromCartoCSS(layerOptions.cartocss)); - } - - columns = columns.concat(getColumnNamesFromInteractivity(layerOptions.interactivity)); - - // filter out repeated ones and non string values - columns = columns - .filter(function (item) { - return typeof item === 'string' && item.length > 0; - }) - .filter(function (item) { - return !Object.prototype.hasOwnProperty.call(EXCLUDE_PROPERTIES, item); - }) - .filter(function (item, pos, self) { - return self.indexOf(item) === pos; - }); - - return columns; -}; - -function getColumnNamesFromInteractivity (interactivity) { - interactivity = interactivity || []; - - var columnNameFromInteractivity = []; - - if (Array.isArray(interactivity)) { - columnNameFromInteractivity = columnNameFromInteractivity.concat(interactivity); - } else { - columnNameFromInteractivity = columnNameFromInteractivity.concat(interactivity.split(',')); - } - - return columnNameFromInteractivity; -} diff --git a/lib/utils/layer-filter.js b/lib/utils/layer-filter.js index 6da253da6..735dd5275 100644 --- a/lib/utils/layer-filter.js +++ b/lib/utils/layer-filter.js @@ -1,5 +1,29 @@ 'use strict'; +module.exports = function layersFilter (mapConfig, layerFilter) { + layerFilter = defaultLayerFilter(layerFilter); + + let filteredLayers = []; + + if (typeof layerFilter === 'string') { + filteredLayers = resolveStringFilter(mapConfig, layerFilter); + } else if (!Array.isArray(layerFilter)) { + filteredLayers = [layerFilter]; + } else { + filteredLayers = layerFilter; + } + + if (!filteredLayers.every(Number.isFinite)) { + throw new Error('Invalid layer filtering'); + } + + filteredLayers = filteredLayers.sort((a, b) => a - b); + + checkLayerBounds(mapConfig, filteredLayers); + + return filteredLayers; +}; + const layerAliases = { all: true, mapnik: true, @@ -8,10 +32,6 @@ const layerAliases = { plain: true }; -function isAlias (layerFilter) { - return Object.prototype.hasOwnProperty.call(layerAliases, layerFilter); -} - function defaultLayerFilter (layerFilter) { if (layerFilter === undefined) { return 'mapnik'; @@ -23,16 +43,20 @@ function resolveAlias (mapConfig, alias) { if (Number.isFinite(+alias)) { return [+alias]; } + if (alias === 'all') { return mapConfig.getLayers().map((_, i) => i); } + if (!Object.prototype.hasOwnProperty.call(layerAliases, alias)) { throw new Error('Invalid layer filtering'); } + return mapConfig.getLayers().reduce((filteredLayers, _, index) => { if (mapConfig.layerType(index) === alias) { filteredLayers.push(index); } + return filteredLayers; }, []); } @@ -42,6 +66,7 @@ function resolveIds (mapConfig, layerIds) { if (layerIdsToNumber.every(Number.isFinite)) { return layerIdsToNumber; } + if (layerIdsToNumber.every(Number.isNaN)) { return layerIds.map(mapConfig.getIndexByLayerId.bind(mapConfig)); } @@ -50,8 +75,8 @@ function resolveIds (mapConfig, layerIds) { } function checkLayerBounds (mapConfig, filteredLayers) { - var uppermostLayerIdx = filteredLayers[filteredLayers.length - 1]; - var lowestLayerIdx = filteredLayers[0]; + const uppermostLayerIdx = filteredLayers[filteredLayers.length - 1]; + const lowestLayerIdx = filteredLayers[0]; if (lowestLayerIdx < 0 || uppermostLayerIdx >= mapConfig.getLayers().length) { throw new Error('Invalid layer filtering'); @@ -62,33 +87,10 @@ function resolveStringFilter (mapConfig, layerFilter) { if (isAlias(layerFilter)) { return resolveAlias(mapConfig, layerFilter); } + return resolveIds(mapConfig, layerFilter.split(',')); } -module.exports = function filterLayer (mapConfig, layerFilter) { - layerFilter = defaultLayerFilter(layerFilter); - - var filteredLayers = []; - - if (typeof layerFilter === 'string') { - filteredLayers = resolveStringFilter(mapConfig, layerFilter); - } else if (!Array.isArray(layerFilter)) { - filteredLayers = [layerFilter]; - } else { - filteredLayers = layerFilter; - } - - if (!filteredLayers.every(Number.isFinite)) { - throw new Error('Invalid layer filtering'); - } - - filteredLayers = filteredLayers.sort(function (a, b) { return a - b; }); - - checkLayerBounds(mapConfig, filteredLayers); - - return filteredLayers; -}; - -module.exports.isSingleLayer = function isSingleLayer (layerFilter) { - return Number.isFinite(+layerFilter); -}; +function isAlias (layerFilter) { + return Object.prototype.hasOwnProperty.call(layerAliases, layerFilter); +} diff --git a/test/unit/utils/layer-columns-test.js b/test/unit/utils/layer-columns-test.js deleted file mode 100644 index 14fb981c6..000000000 --- a/test/unit/utils/layer-columns-test.js +++ /dev/null @@ -1,75 +0,0 @@ -'use strict'; - -require('../../support/test-helper'); -var LayerColumns = require('../../../lib/utils/layer-columns'); -var assert = require('assert'); - -describe('mvt-utils', function () { - function createOptions (interactivity, columns) { - var options = { - sql: 'select * from populated_places_simple_reduced', - cartocss: ['#layer0 {', - 'marker-fill: red;', - 'marker-width: 10;', - 'text-name: [name];', - '[pop_max>100000] { marker-fill: black; } ', - '}'].join('\n'), - cartocss_version: '2.3.0' - }; - - if (interactivity) { - options.interactivity = interactivity; - } - - if (columns) { - options.columns = columns; - } - - return options; - } - - it('should not duplicate column names', function () { - var columns = LayerColumns.getColumns(createOptions()); - assert.deepEqual(columns, ['pop_max', 'name']); - }); - - it('should handle interactivity strings', function () { - var columns = LayerColumns.getColumns(createOptions('cartodb_id,pop_max')); - assert.deepEqual(columns, ['pop_max', 'name', 'cartodb_id']); - }); - - it('should handle interactivity array', function () { - var columns = LayerColumns.getColumns(createOptions(['cartodb_id', 'pop_max'])); - assert.deepEqual(columns, ['pop_max', 'name', 'cartodb_id']); - }); - - it('should handle columns array', function () { - var columns = LayerColumns.getColumns(createOptions(null, ['cartodb_id', 'pop_min'])); - assert.deepEqual(columns, ['cartodb_id', 'pop_min', 'pop_max', 'name']); - }); - - it('should handle empty columns array', function () { - var columns = LayerColumns.getColumns(createOptions(null, [])); - assert.deepEqual(columns, ['pop_max', 'name']); - }); - - it('should ignore no-string values', function () { - var columns = LayerColumns.getColumns(createOptions(null, ['cartodb_id', 'pop_min', 1])); - assert.deepEqual(columns, ['cartodb_id', 'pop_min', 'pop_max', 'name']); - }); - - it('should ignore null values', function () { - var columns = LayerColumns.getColumns(createOptions(null, ['cartodb_id', 'pop_min', null])); - assert.deepEqual(columns, ['cartodb_id', 'pop_min', 'pop_max', 'name']); - }); - - it('should ignore undefined values', function () { - var columns = LayerColumns.getColumns(createOptions(null, ['cartodb_id', undefined, 'pop_min'])); - assert.deepEqual(columns, ['cartodb_id', 'pop_min', 'pop_max', 'name']); - }); - - it('should ignore empty values', function () { - var columns = LayerColumns.getColumns(createOptions(null, ['cartodb_id', '', 'pop_min'])); - assert.deepEqual(columns, ['cartodb_id', 'pop_min', 'pop_max', 'name']); - }); -}); From 5f7e4fe199ead036a713d4bc30e88e4a3633a7b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 19:10:48 +0200 Subject: [PATCH 43/45] Rename module --- lib/renderers/blend/factory.js | 2 +- lib/renderers/mapnik/factory.js | 2 +- lib/renderers/pg-mvt/factory.js | 2 +- lib/renderers/renderer-factory.js | 2 +- lib/utils/{layer-filter.js => layers-filter.js} | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename lib/utils/{layer-filter.js => layers-filter.js} (100%) diff --git a/lib/renderers/blend/factory.js b/lib/renderers/blend/factory.js index 6324141a5..bdc86d05b 100644 --- a/lib/renderers/blend/factory.js +++ b/lib/renderers/blend/factory.js @@ -3,7 +3,7 @@ const mapnik = require('@carto/mapnik'); const Renderer = require('./renderer'); const BaseAdaptor = require('../base-adaptor'); -const layersFilter = require('../../utils/layer-filter'); +const layersFilter = require('../../utils/layers-filter'); const EMPTY_IMAGE_BUFFER = new mapnik.Image(256, 256).encodeSync('png'); diff --git a/lib/renderers/mapnik/factory.js b/lib/renderers/mapnik/factory.js index 44d04b3c6..9089dac18 100644 --- a/lib/renderers/mapnik/factory.js +++ b/lib/renderers/mapnik/factory.js @@ -3,7 +3,7 @@ const { rendererFactory: mapnikRendererFactory } = require('@carto/cartonik'); const mapnik = require('@carto/mapnik'); const grainstore = require('grainstore'); -const layersFilter = require('../../utils/layer-filter'); +const layersFilter = require('../../utils/layers-filter'); const MapnikAdaptor = require('./adaptor'); const DefaultQueryRewriter = require('../../utils/default-query-rewriter'); diff --git a/lib/renderers/pg-mvt/factory.js b/lib/renderers/pg-mvt/factory.js index 3c0b7ac0b..ac58a525b 100644 --- a/lib/renderers/pg-mvt/factory.js +++ b/lib/renderers/pg-mvt/factory.js @@ -1,6 +1,6 @@ 'use strict'; -const layersFilter = require('../../utils/layer-filter'); +const layersFilter = require('../../utils/layers-filter'); const parseDbParams = require('../renderer-params'); const Renderer = require('./renderer'); const BaseAdaptor = require('../base-adaptor'); diff --git a/lib/renderers/renderer-factory.js b/lib/renderers/renderer-factory.js index 431de69e6..1112b7110 100644 --- a/lib/renderers/renderer-factory.js +++ b/lib/renderers/renderer-factory.js @@ -6,7 +6,7 @@ const TorqueRendererFactory = require('./torque/factory'); const MapnikRendererFactory = require('./mapnik/factory'); const PlainRendererFactory = require('./plain/factory'); const PgMvtRendererFactory = require('./pg-mvt/factory'); -const layersFilter = require('../utils/layer-filter'); +const layersFilter = require('../utils/layers-filter'); module.exports = class RendererFactory { constructor ({ mapnik = {}, mvt = {}, torque = {}, http = {}, onTileErrorStrategy } = {}) { diff --git a/lib/utils/layer-filter.js b/lib/utils/layers-filter.js similarity index 100% rename from lib/utils/layer-filter.js rename to lib/utils/layers-filter.js From f650d37ebe84bc994249077d33bb03012491ea48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 24 Apr 2020 19:21:41 +0200 Subject: [PATCH 44/45] ES2020 syntax sugar for utilities --- lib/metadata/layer-metadata.js | 2 +- lib/models/mapconfig.js | 4 +- lib/renderers/base-adaptor.js | 2 +- lib/renderers/http/factory.js | 2 +- lib/renderers/http/renderer.js | 2 +- lib/renderers/mapnik/adaptor.js | 2 +- lib/utils/cartocss-utils.js | 67 ++++++++++++++------------------- 7 files changed, 36 insertions(+), 45 deletions(-) diff --git a/lib/metadata/layer-metadata.js b/lib/metadata/layer-metadata.js index d0dd3c6bf..f76527d14 100644 --- a/lib/metadata/layer-metadata.js +++ b/lib/metadata/layer-metadata.js @@ -50,7 +50,7 @@ module.exports = class LayerMetadata { const metadata = []; mapConfig.getLayers().forEach(function (layer, layerIndex) { - var layerType = mapConfig.layerType(layerIndex); + const layerType = mapConfig.layerType(layerIndex); metadata[layerIndex] = { type: layerType, diff --git a/lib/models/mapconfig.js b/lib/models/mapconfig.js index 93d6d4d85..92013be6d 100644 --- a/lib/models/mapconfig.js +++ b/lib/models/mapconfig.js @@ -253,8 +253,8 @@ function isValidBufferSize (value) { } function getLayerIndexByType (rawMapConfig, type, mapConfigLayerIdx) { - var typeLayerIndex = 0; - var mapConfigToTypeLayers = {}; + let typeLayerIndex = 0; + const mapConfigToTypeLayers = {}; rawMapConfig.layers.forEach(function (layer, layerIdx) { if (getType(layer.type) === type) { diff --git a/lib/renderers/base-adaptor.js b/lib/renderers/base-adaptor.js index a7c9d6635..46ef00679 100644 --- a/lib/renderers/base-adaptor.js +++ b/lib/renderers/base-adaptor.js @@ -1,7 +1,7 @@ 'use strict'; function wrap (x, z) { - var limit = (1 << z); + const limit = (1 << z); return ((x % limit) + limit) % limit; } diff --git a/lib/renderers/http/factory.js b/lib/renderers/http/factory.js index 9d9fd2a52..0d8a794b4 100644 --- a/lib/renderers/http/factory.js +++ b/lib/renderers/http/factory.js @@ -67,7 +67,7 @@ module.exports = class HttpFactory { }; function getSubdomains (urlTemplate, options) { - var subdomains = options.subdomains; + let subdomains = options.subdomains; if (!subdomains) { subdomains = urlTemplate.match(/\{ *([s]+) *\}/g) ? ['a', 'b', 'c'] : []; diff --git a/lib/renderers/http/renderer.js b/lib/renderers/http/renderer.js index d823ecfcf..0405e22dc 100644 --- a/lib/renderers/http/renderer.js +++ b/lib/renderers/http/renderer.js @@ -92,7 +92,7 @@ const templateRe = /\{ *([\w_]+) *\}/g; // super-simple templating facility, used for TileLayer URLs function template (str, data) { return str.replace(templateRe, function (str, key) { - var value = data[key]; + let value = data[key]; if (value === undefined) { throw new Error('No value provided for variable ' + str); diff --git a/lib/renderers/mapnik/adaptor.js b/lib/renderers/mapnik/adaptor.js index acdbcf3e3..7d48261f9 100644 --- a/lib/renderers/mapnik/adaptor.js +++ b/lib/renderers/mapnik/adaptor.js @@ -7,7 +7,7 @@ const METRIC_PREFIX = 'Mk_'; function parseMapnikMetrics (stats) { if (stats && Object.prototype.hasOwnProperty.call(stats, 'Mapnik')) { Object.keys(stats.Mapnik).forEach(function (key) { - var metric = stats.Mapnik[key]; + const metric = stats.Mapnik[key]; if (metric.constructor === Object) { if (Object.prototype.hasOwnProperty.call(metric, 'Time (us)')) { stats[METRIC_PREFIX + key] = Math.floor(metric['Time (us)'] / 1000); diff --git a/lib/utils/cartocss-utils.js b/lib/utils/cartocss-utils.js index 74b38973b..3147340c7 100644 --- a/lib/utils/cartocss-utils.js +++ b/lib/utils/cartocss-utils.js @@ -1,27 +1,28 @@ 'use strict'; -var carto = require('carto'); -var torqueReference = require('torque.js').cartocss_reference; +const carto = require('carto'); +const torqueReference = require('torque.js').cartocss_reference; +const MAP_ATTRIBUTES = ['buffer-size']; +carto.tree.Reference.setData(torqueReference.version.latest); -module.exports.getColumnNamesFromCartoCSS = function (cartocss) { +module.exports.getColumnNamesFromCartoCSS = function getColumnNamesFromCartoCSS (cartocss) { return selectors(cartocss); }; -carto.tree.Reference.setData(torqueReference.version.latest); - -var MAP_ATTRIBUTES = ['buffer-size']; -module.exports.optionsFromCartoCSS = function (cartocss) { - var shader = new carto.RendererJS().render(cartocss); - var mapConfig = shader.findLayer({ name: 'Map' }); +module.exports.optionsFromCartoCSS = function optionsFromCartoCSS (cartocss) { + const shader = new carto.RendererJS().render(cartocss); + const mapConfig = shader.findLayer({ name: 'Map' }); - var options = {}; + const options = {}; if (mapConfig) { - MAP_ATTRIBUTES.reduce(function (opts, attributeName) { - var v = mapConfig.eval(attributeName); + MAP_ATTRIBUTES.reduce((opts, attributeName) => { + const v = mapConfig.eval(attributeName); + if (v !== undefined) { opts[attributeName] = v; } + return opts; }, options); } @@ -30,7 +31,7 @@ module.exports.optionsFromCartoCSS = function (cartocss) { }; function selectors (cartocss) { - var env = { + const env = { benchmark: false, validation_data: false, effects: [], @@ -39,11 +40,11 @@ function selectors (cartocss) { this.errors.push(e); } }; - var parser = carto.Parser(env); - var tree = parser.parse(cartocss); - var definitions = tree.toList(env); + const parser = carto.Parser(env); + const tree = parser.parse(cartocss); + const definitions = tree.toList(env); - var allSelectors = {}; + const allSelectors = {}; appendFiltersKeys(definitions, allSelectors); appendRulesFields(definitions, allSelectors); @@ -52,9 +53,9 @@ function selectors (cartocss) { function appendFiltersKeys (definitions, allSelectors) { definitions - .reduce(function (filters, r) { + .reduce((filters, r) => { if (r.filters && r.filters.filters) { - Object.keys(r.filters.filters).forEach(function (filterId) { + Object.keys(r.filters.filters).forEach((filterId) => { allSelectors[r.filters.filters[filterId].key.value] = true; }); } @@ -66,22 +67,12 @@ function appendFiltersKeys (definitions, allSelectors) { function appendRulesFields (definitions, allSelectors) { definitions - .map(function (r) { - return r.rules; - }) - .reduce(function (allRules, rules) { - return allRules.concat(rules); - }, []) - .reduce(function (allValues, rule) { - return values(rule.value, allValues); - }, []) - .filter(function (values) { - return values.is === 'field'; - }) - .map(function (rule) { - return rule.value; - }) - .reduce(function (keys, field) { + .map((r) => r.rules) + .reduce((allRules, rules) => allRules.concat(rules), []) + .reduce((allValues, rule) => values(rule.value, allValues), []) + .filter((values) => values.is === 'field') + .map((rule) => rule.value) + .reduce((keys, field) => { keys[field] = true; return keys; }, allSelectors); @@ -91,16 +82,16 @@ function appendRulesFields (definitions, allSelectors) { function values (value, allValues) { allValues = allValues || []; + if (value.is === 'value' || value.is === 'expression') { if (Array.isArray(value.value)) { - value.value.forEach(function (_value) { - values(_value, allValues); - }); + value.value.forEach((_value) => values(_value, allValues)); } else { values(value.value, allValues); } } else { allValues.push(value); } + return allValues; } From f4a6e63f95720ea488cafd8db7a7e380a02e2f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 1 May 2020 15:34:17 +0200 Subject: [PATCH 45/45] Update NEWS --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index 70ba9e7c0..0785c311d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,9 @@ # Version 6.0.1 2020-mm-dd +Announcements: +- Apply ES2018 syntax uniformly (#724) +- Better error messages for TorqueRenderer. # Version 6.0.0 2020-04-05