From b56d2ec30b584c0751ac469b700426524c5a7b44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 3 Aug 2017 12:24:05 +0200 Subject: [PATCH] Validate aggregation value --- lib/cartodb/backends/dataview.js | 14 +-- lib/cartodb/models/dataview/histogram.js | 17 ++++ test/acceptance/dataviews/histogram.js | 121 +++++++++++++++++++++++ yarn.lock | 12 +-- 4 files changed, 146 insertions(+), 18 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 0c9a30fe..29dcd903 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -20,17 +20,6 @@ function DataviewBackend(analysisBackend) { this.analysisBackend = analysisBackend; } -var DATE_AGGREGATIONS = { - 'auto': true, - 'minute': true, - 'hour': true, - 'day': true, - 'week': true, - 'month': true, - 'quarter': true, - 'year': true -}; - module.exports = DataviewBackend; DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, params, callback) { @@ -116,7 +105,8 @@ function getOverrideParams(params, ownFilter) { {ownFilter: ownFilter} ); - if (params.aggregation && DATE_AGGREGATIONS.hasOwnProperty(params.aggregation)) { + // validation will be delegated to the proper dataview + if (params.aggregation !== undefined) { overrideParams.aggregation = params.aggregation; } diff --git a/lib/cartodb/models/dataview/histogram.js b/lib/cartodb/models/dataview/histogram.js index 4c01cc46..13a9d998 100644 --- a/lib/cartodb/models/dataview/histogram.js +++ b/lib/cartodb/models/dataview/histogram.js @@ -434,12 +434,29 @@ Histogram.prototype._shouldOverrideBins = function (override) { return override && _.has(override, 'bins'); }; +var DATE_AGGREGATIONS = { + 'auto': true, + 'minute': true, + 'hour': true, + 'day': true, + 'week': true, + 'month': true, + 'quarter': true, + 'year': true +}; + Histogram.prototype._buildDateHistogramQuery = function (psql, override, callback) { var _column = this.column; var _query = this.query; var _aggregation = override && override.aggregation ? override.aggregation : this.aggregation; var _offset = override && Number.isFinite(override.offset) ? override.offset : this.offset; + if (!DATE_AGGREGATIONS.hasOwnProperty(_aggregation)) { + return callback(new Error('Invalid aggregation value. Valid ones: ' + + Object.keys(DATE_AGGREGATIONS).join(', ') + )); + } + if (_aggregation === 'auto') { this.getAutomaticAggregation(psql, function (err, aggregation) { if (err || aggregation === 'none') { diff --git a/test/acceptance/dataviews/histogram.js b/test/acceptance/dataviews/histogram.js index 4d48ab33..51808d67 100644 --- a/test/acceptance/dataviews/histogram.js +++ b/test/acceptance/dataviews/histogram.js @@ -766,3 +766,124 @@ describe('histogram-dataview: special float valuer', function() { }); }); }); + +describe('histogram-dates: aggregation input value', function() { + + afterEach(function(done) { + if (this.testClient) { + this.testClient.drain(done); + } else { + done(); + } + }); + + var mapConfig = createMapConfig( + [ + { + type: "cartodb", + options: { + source: { + id: "a0" + }, + cartocss: "#points { marker-width: 10; marker-fill: red; }", + cartocss_version: "2.3.0" + } + } + ], + { + agg_value_histogram: { + source: { + id: 'a0' + }, + type: 'histogram', + options: { + column: 'd', + aggregation: 'day' + } + }, + bad_agg_value_histogram: { + source: { + id: 'a0' + }, + type: 'histogram', + options: { + column: 'd', + aggregation: 'wadus' + } + } + }, + [ + { + id: 'a0', + type: 'source', + params: { + query: [ + 'select null::geometry the_geom_webmercator, date AS d', + 'from generate_series(', + '\'2007-02-15 01:00:00\'::timestamp,', + '\'2008-04-09 01:00:00\'::timestamp,', + ' \'1 day\'::interval', + ') date' + ].join(' ') + } + } + ] + ); + + it('should fail when aggregation values is not valid while instantiating the map', function(done) { + this.testClient = new TestClient(mapConfig, 1234); + const override = { + response: { + status: 400 + } + }; + + this.testClient.getDataview('bad_agg_value_histogram', override, function(err, dataviewError) { + assert.ifError(err); + + assert.deepEqual(dataviewError, { + errors: [ + 'Invalid aggregation value. Valid ones: auto, minute, hour, day, week, month, quarter, year' + ], + errors_with_context: [{ + type: 'unknown', + message: [ + 'Invalid aggregation value. ', + 'Valid ones: auto, minute, hour, day, week, month, quarter, year' + ].join('') + }] + }); + + done(); + }); + }); + + it('should fail when aggregation values is not valid while fetching dataview result', function(done) { + this.testClient = new TestClient(mapConfig, 1234); + const override = { + aggregation: 'wadus', + response: { + status: 400 + } + }; + + this.testClient.getDataview('agg_value_histogram', override, function(err, dataviewError) { + assert.ifError(err); + + assert.deepEqual(dataviewError, { + errors: [ + 'Invalid aggregation value. Valid ones: auto, minute, hour, day, week, month, quarter, year' + ], + errors_with_context: [{ + type: 'unknown', + message: [ + 'Invalid aggregation value. ', + 'Valid ones: auto, minute, hour, day, week, month, quarter, year' + ].join('') + }] + }); + + done(); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index a835b0e0..49cbc74e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,7 +2,7 @@ # yarn lockfile v1 -"abaculus@github:cartodb/abaculus#2.0.3-cdb1": +abaculus@cartodb/abaculus#2.0.3-cdb1: version "2.0.3-cdb1" resolved "https://codeload.github.com/cartodb/abaculus/tar.gz/f5f34e1c80cdd8d49edd1d6fe3b2220ab2e23aaf" dependencies: @@ -205,7 +205,7 @@ camshaft@0.55.6: dot "^1.0.3" request "^2.69.0" -"canvas@github:cartodb/node-canvas#1.6.2-cdb2": +canvas@cartodb/node-canvas#1.6.2-cdb2: version "1.6.2-cdb2" resolved "https://codeload.github.com/cartodb/node-canvas/tar.gz/8acf04557005c633f9e68524488a2657c04f3766" dependencies: @@ -231,7 +231,7 @@ carto@CartoDB/carto#0.15.1-cdb1: optimist "~0.6.0" underscore "~1.6.0" -"carto@github:cartodb/carto#0.15.1-cdb3": +carto@cartodb/carto#0.15.1-cdb3: version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" dependencies: @@ -1288,7 +1288,7 @@ mocha@~3.4.1: mkdirp "0.5.1" supports-color "3.1.2" -moment@^2.10.6, moment@^2.18.1: +moment@^2.10.6, moment@~2.18.1: version "2.18.1" resolved "https://registry.yarnpkg.com/moment/-/moment-2.18.1.tgz#c36193dd3ce1c2eed2adb7c802dbbc77a81b1c0f" @@ -2103,7 +2103,7 @@ through@2: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" -"tilelive-bridge@github:cartodb/tilelive-bridge#2.3.1-cdb3": +tilelive-bridge@cartodb/tilelive-bridge#2.3.1-cdb3: version "2.3.1-cdb3" resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/bde83c8dcf4ada40c7c0eb1b477f212e75399d23" dependencies: @@ -2111,7 +2111,7 @@ through@2: mapnik-pool "~0.1.3" sphericalmercator "1.0.x" -"tilelive-mapnik@github:cartodb/tilelive-mapnik#0.6.18-cdb2": +tilelive-mapnik@cartodb/tilelive-mapnik#0.6.18-cdb2: version "0.6.18-cdb2" resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/46f1adefee90f3f46c0ede5e0833f8522634a858" dependencies: