From a5f985257c8a42f8d96dcf30fe99629d591aff1d Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 18 Jun 2018 13:14:17 +0200 Subject: [PATCH 1/5] Tests for dates_as_numbers with tokens --- test/acceptance/date-wrapping.spec.js | 53 +++++++++++++++++++++++++- test/fixtures/test_mapconfigFactory.js | 4 +- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index 9bb8098f..4ba5bbf5 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -262,4 +262,55 @@ describe('date-wrapping', () => { }); }); }); -}); \ No newline at end of file + + describe('when sql queries use mapnik tokens', () => { + beforeEach(() => { + const mapConfig = mapConfigFactory.getVectorMapConfig({ + layerOptions: [{ + dates_as_numbers: true, + additionalColumns: [ + '!scale_denominator! AS sc' + ] + }] + }); + testClient = new TestClient(mapConfig); + }); + + afterEach(done => testClient.drain(done)); + + it.only('should work', done => { + testClient.getLayergroup(function(err, layergroup) { + assert.ifError(err); + assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers, ['date']); + done(); + }); + + }); + + it.only('should return correct tiles', done => { + testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { + const expected = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { cartodb_id: 0, date: 1527810000, sc: 559082000 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { cartodb_id: 1, date: 1527900000, sc: 559082000 } + } + ]; + const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; + + assert.deepEqual(actual, expected); + done(); + }); + }); + }); + +}); + + diff --git a/test/fixtures/test_mapconfigFactory.js b/test/fixtures/test_mapconfigFactory.js index ff554be5..f1af550c 100644 --- a/test/fixtures/test_mapconfigFactory.js +++ b/test/fixtures/test_mapconfigFactory.js @@ -18,16 +18,18 @@ function _generateLayers(opts) { } function _generateLayerConfig(opts) { + const additionalColumns = opts.additionalColumns ? opts.additionalColumns.join(',')+',' : ''; return { type: 'mapnik', options: { sql: ` SELECT + ${additionalColumns} (DATE '2018-06-01' + x) as date, x as cartodb_id, st_makepoint(x * 10, x * 10) as the_geom, st_makepoint(x * 10, x * 10) as the_geom_webmercator - FROM + FROM generate_series(0, 1) x`, aggregation: { columns: {}, From e0cd1aba2975550b8197fb94511fb60c6e3412ac Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 18 Jun 2018 13:15:45 +0200 Subject: [PATCH 2/5] Refactor: move common token substitutions to query-utils --- .../layer-stats/mapnik-layer-stats.js | 26 ++------------ lib/cartodb/backends/overviews-metadata.js | 19 ++++------ .../aggregation/aggregation-mapconfig.js | 13 ++----- lib/cartodb/utils/query-utils.js | 36 ++++++++++++++++++- 4 files changed, 46 insertions(+), 48 deletions(-) diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js index d5f841b8..39286f79 100644 --- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js +++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js @@ -1,27 +1,5 @@ -var queryUtils = require('../../utils/query-utils'); +const queryUtils = require('../../utils/query-utils'); const AggregationMapConfig = require('../../models/aggregation/aggregation-mapconfig'); -var SubstitutionTokens = require('../../utils/substitution-tokens'); - -// Instantiate a query with tokens for a given zoom level -function queryForZoom(sql, zoom, singleTile=false) { - const tileRes = 256; - const wmSize = 6378137.0*2*Math.PI; - const nTiles = Math.pow(2, zoom); - const tileSize = wmSize / nTiles; - const resolution = tileSize / tileRes; - const scaleDenominator = resolution / 0.00028; - const x0 = -wmSize/2, y0 = -wmSize/2; - let bbox = `ST_MakeEnvelope(${x0}, ${y0}, ${x0+wmSize}, ${y0+wmSize})`; - if (singleTile) { - bbox = `ST_MakeEnvelope(${x0}, ${y0}, ${x0 + tileSize}, ${y0 + tileSize})`; - } - return SubstitutionTokens.replace(sql, { - bbox: bbox, - scale_denominator: scaleDenominator, - pixel_width: resolution, - pixel_height: resolution - }); -} function MapnikLayerStats () { this._types = { @@ -52,7 +30,7 @@ function _getSQL(ctx, query, type='pre', zoom=0) { else { sql = ctx.aggrQuery; } - sql = queryForZoom(sql, zoom || 0); + sql = queryUtils.subsituteTokensForZoom(sql, zoom || 0); return query(sql); } diff --git a/lib/cartodb/backends/overviews-metadata.js b/lib/cartodb/backends/overviews-metadata.js index 31045534..7eb7c889 100644 --- a/lib/cartodb/backends/overviews-metadata.js +++ b/lib/cartodb/backends/overviews-metadata.js @@ -1,4 +1,4 @@ -var SubstitutionTokens = require('../utils/substitution-tokens'); +const queryUtils = require('../utils/query-utils'); function OverviewsMetadataBackend(pgQueryRunner) { this.pgQueryRunner = pgQueryRunner; @@ -6,20 +6,15 @@ function OverviewsMetadataBackend(pgQueryRunner) { module.exports = OverviewsMetadataBackend; -function prepareSql(sql) { - return sql && SubstitutionTokens.replace(sql, { - bbox: 'ST_MakeEnvelope(0,0,0,0)', - scale_denominator: '0', - pixel_width: '1', - pixel_height: '1' - }); -} - OverviewsMetadataBackend.prototype.getOverviewsMetadata = function (username, sql, callback) { // FIXME: Currently using internal function _cdb_schema_name // CDB_Overviews should provide the schema information directly. - var query = 'SELECT *, _cdb_schema_name(base_table)' + - ' FROM CDB_Overviews(CDB_QueryTablesText($windshaft$' + prepareSql(sql) + '$windshaft$))'; + const query = ` + SELECT *, _cdb_schema_name(base_table) + FROM CDB_Overviews( + CDB_QueryTablesText($windshaft$${queryUtils.substituteDummyTokens(sql)}$windshaft$) + ); + `; this.pgQueryRunner.run(username, query, function handleOverviewsRows(err, rows) { if (err){ callback(err); diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js index 548f95d3..98a71962 100644 --- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js +++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js @@ -8,19 +8,10 @@ const { createAggregationFiltersValidator } = aggregationValidator; -const SubstitutionTokens = require('../../utils/substitution-tokens'); +const queryUtils = require('../../utils/query-utils'); const removeDuplicates = arr => [...new Set(arr)]; -function prepareSql(sql) { - return sql && SubstitutionTokens.replace(sql, { - bbox: 'ST_MakeEnvelope(0,0,0,0)', - scale_denominator: '0', - pixel_width: '1', - pixel_height: '1' - }); -} - module.exports = class AggregationMapConfig extends MapConfig { static get AGGREGATIONS () { return aggregationQuery.SUPPORTED_AGGREGATE_FUNCTIONS; @@ -205,7 +196,7 @@ module.exports = class AggregationMapConfig extends MapConfig { } const sql = limitedQuery({ - query: prepareSql(layer.options.sql) + query: queryUtils.substituteDummyTokens(layer.options.sql) }); connection.query(sql, (err, result) => { diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index ed239422..633a02a4 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -1,3 +1,5 @@ +const SubstitutionTokens = require('./substitution-tokens'); + function prepareQuery(sql) { var affectedTableRegexCache = { bbox: /!bbox!/g, @@ -176,5 +178,37 @@ function queryPromise(dbConnection, query) { }); } +function substituteDummyTokens(sql) { + return sql && SubstitutionTokens.replace(sql, { + bbox: 'ST_MakeEnvelope(0,0,0,0)', + scale_denominator: '0', + pixel_width: '1', + pixel_height: '1' + }); +} + +function subsituteTokensForZoom(sql, zoom, singleTile=false) { + const tileRes = 256; + const wmSize = 6378137.0*2*Math.PI; + const nTiles = Math.pow(2, zoom); + const tileSize = wmSize / nTiles; + const resolution = tileSize / tileRes; + const scaleDenominator = resolution / 0.00028; + const x0 = -wmSize/2, y0 = -wmSize/2; + let bbox = `ST_MakeEnvelope(${x0}, ${y0}, ${x0+wmSize}, ${y0+wmSize})`; + if (singleTile) { + bbox = `ST_MakeEnvelope(${x0}, ${y0}, ${x0 + tileSize}, ${y0 + tileSize})`; + } + return SubstitutionTokens.replace(sql, { + bbox: bbox, + scale_denominator: scaleDenominator, + pixel_width: resolution, + pixel_height: resolution + }); +} + module.exports.queryPromise = queryPromise; -module.exports.getQueryLimited = getQueryLimited; \ No newline at end of file +module.exports.getQueryLimited = getQueryLimited; +module.exports.substituteDummyTokens = substituteDummyTokens; +module.exports.subsituteTokensForZoom = subsituteTokensForZoom; + From e89503e1fad77373379a8125e0e4f7ba98d2da40 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 18 Jun 2018 13:16:49 +0200 Subject: [PATCH 3/5] Subsitute tokens to get columns for dates_as_numbers Fixes #980 --- .../models/mapconfig/adapter/vector-mapconfig-adapter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index a707b5d3..bedea8b8 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -46,7 +46,7 @@ class VectorMapConfigAdapter { if (err) { return reject(err); } - const query = queryUtils.getQueryLimited(originalQuery, 0); + const query = queryUtils.getQueryLimited(queryUtils.substituteDummyTokens(originalQuery), 0); queryUtils.queryPromise(connection, query) .then(resolve) .catch(reject); From b05d9a0a7545eecbfebafe824d16a5eb7cad2d3a Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 18 Jun 2018 13:33:08 +0200 Subject: [PATCH 4/5] Remove only from tests --- test/acceptance/date-wrapping.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index 4ba5bbf5..3eac2639 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -278,7 +278,7 @@ describe('date-wrapping', () => { afterEach(done => testClient.drain(done)); - it.only('should work', done => { + it('should work', done => { testClient.getLayergroup(function(err, layergroup) { assert.ifError(err); assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers, ['date']); @@ -287,7 +287,7 @@ describe('date-wrapping', () => { }); - it.only('should return correct tiles', done => { + it('should return correct tiles', done => { testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { const expected = [ { From 863e42691a2c914ae280526d7e6f5f22ffdd818a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 25 Jun 2018 12:51:30 +0200 Subject: [PATCH 5/5] Add a release note to advice that a rake task should be performed --- NEWS.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 6f119d7f..eb9626fe 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,9 @@ ## 6.2.0 Released 2018-mm-dd +Notice: +- This release changes the way that authentication works internally. You'll need to run `bundle exec rake carto:api_key:create_default` in your development environment to keep working. + New features: - CI tests with Ubuntu Xenial + PostgreSQL 10.1 and Ubuntu Precise + PostgreSQL 9.5 - Upgrades Windshaft to [4.8.1](https://github.com/CartoDB/Windshaft/blob/4.8.1/NEWS.md#version-481) which includes: @@ -13,7 +16,7 @@ New features: - Fix a bug with zero length lines not being rendered when using the marker symbolizer. - Upgrades Camshaft to [0.61.11](https://github.com/CartoDB/camshaft/releases/tag/0.61.11): - Use Dollar-Quoted String Constants to avoid Syntax Error while running moran analyses. [0.61.10](https://github.com/CartoDB/camshaft/releases/tag/0.61.10) - - Quote name columns when performing trade area analysis to avoid Syntax Errors. [0.61.11](https://github.com/CartoDB/camshaft/releases/tag/0.61.11) + - Quote name columns when performing trade area analysis to avoid Syntax Errors. [0.61.11](https://github.com/CartoDB/camshaft/releases/tag/0.61.11) - Update other deps: - body-parser: 1.18.3 - cartodb-psql: 0.11.0