diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js index f68417ef..a50dae41 100644 --- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js +++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js @@ -1,9 +1,11 @@ const MapConfig = require('windshaft').model.MapConfig; const aggregationQuery = require('./aggregation-query'); +const { SUPPORTED_AGGREGATE_FUNCTIONS } = require('./aggregation-query'); const aggregationValidator = require('./aggregation-validator'); const { createPositiveNumberValidator, - createIncludesValueValidator + createIncludesValueValidator, + createAggregationColumnsValidator } = aggregationValidator; module.exports = class AggregationMapConfig extends MapConfig { @@ -41,12 +43,14 @@ module.exports = class AggregationMapConfig extends MapConfig { super(config, datasource); const validate = aggregationValidator(this); - const numberValidator = createPositiveNumberValidator(this); + const positiveNumberValidator = createPositiveNumberValidator(this); const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS); + const aggregationColumnsValidator = createAggregationColumnsValidator(this, SUPPORTED_AGGREGATE_FUNCTIONS); - validate('resolution', numberValidator); + validate('resolution', positiveNumberValidator); validate('placement', includesValidPlacementsValidator); - validate('threshold', numberValidator); + validate('threshold', positiveNumberValidator); + validate('columns', aggregationColumnsValidator); } getAggregatedQuery (index) { diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js index cfa5e9c8..d0d71cc7 100644 --- a/lib/cartodb/models/aggregation/aggregation-validator.js +++ b/lib/cartodb/models/aggregation/aggregation-validator.js @@ -1,5 +1,3 @@ - - module.exports = function aggregationValidator (mapconfig) { return function validateProperty (prop, validator) { for (let index = 0; index < mapconfig.getLayers().length; index++) { @@ -45,3 +43,54 @@ module.exports.createPositiveNumberValidator = function (mapconfig) { } }; }; + +module.exports.createAggregationColumnsValidator = function (mapconfig, validAggregatedFunctions) { + return function validateAggregationColumns (value, key, index) { + Object.keys(value).forEach((columnName) => { + if (columnName.length <= 0) { + const error = new Error(`Invalid column name, should be a non empty string`); + error.type = 'layer'; + error.layer = { + id: mapconfig.getLayerId(index), + index: index, + type: mapconfig.layerType(index) + }; + + throw error; + } + + const { aggregate_function } = value[columnName]; + + if (!validAggregatedFunctions.includes(aggregate_function)) { + const error = new Error( + `Unsupported aggregation function ${aggregate_function},` + + ` valid ones: ${validAggregatedFunctions.join(', ')}` + ); + error.type = 'layer'; + error.layer = { + id: mapconfig.getLayerId(index), + index: index, + type: mapconfig.layerType(index) + }; + + throw error; + } + + const { aggregated_column } = value[columnName]; + + if (typeof aggregated_column !== 'string' || aggregated_column <= 0) { + const error = new Error( + `Invalid aggregated column, should be a non empty string` + ); + error.type = 'layer'; + error.layer = { + id: mapconfig.getLayerId(index), + index: index, + type: mapconfig.layerType(index) + }; + + throw error; + } + }); + }; +}; diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index 074d773e..73284582 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -820,6 +820,152 @@ describe('aggregation', function () { done(); }); }); + + it('should fail with bad column name', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + columns : { + '': { + aggregate_function: 'count', + aggregated_column: 'value', + } + }, + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + + const options = { + response: { + status: 400 + } + }; + + this.testClient.getLayergroup(options, (err, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, { + errors: [ 'Invalid column name, should be a non empty string' ], + errors_with_context:[{ + type: 'layer', + message: 'Invalid column name, should be a non empty string', + layer: { + "id": "layer0", + "index": 0, + "type": "mapnik" + } + }] + }); + + done(); + }); + }); + + it('should fail with bad aggregated function', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + columns : { + 'wadus_function': { + aggregate_function: 'wadus', + aggregated_column: 'value', + } + }, + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + + const options = { + response: { + status: 400 + } + }; + + this.testClient.getLayergroup(options, (err, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, { + errors: [ 'Unsupported aggregation function wadus, ' + + 'valid ones: count, avg, sum, min, max, mode' ], + errors_with_context:[{ + type: 'layer', + message: 'Unsupported aggregation function wadus, ' + + 'valid ones: count, avg, sum, min, max, mode', + layer: { + "id": "layer0", + "index": 0, + "type": "mapnik" + } + }] + }); + + done(); + }); + }); + + it('should fail with bad aggregated columns', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + columns : { + 'total_wadus': { + aggregate_function: 'sum', + aggregated_column: '', + } + }, + } + } + } + ]); + this.testClient = new TestClient(this.mapConfig); + + const options = { + response: { + status: 400 + } + }; + + this.testClient.getLayergroup(options, (err, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, { + errors: [ 'Invalid aggregated column, should be a non empty string' ], + errors_with_context:[{ + type: 'layer', + message: 'Invalid aggregated column, should be a non empty string', + layer: { + "id": "layer0", + "index": 0, + "type": "mapnik" + } + }] + }); + + done(); + }); + }); + }); }); });