diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js index 440b21a2..8df069c0 100644 --- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js @@ -7,6 +7,9 @@ const MISSING_AGGREGATION_COLUMNS = 'Missing columns in the aggregation. The map const unsupportedGeometryTypeErrorMessage = ctx => `Unsupported geometry type: ${ctx.geometryType}. Aggregation is available only for geometry type: ST_Point`; +const invalidAggregationParamValueErrorMessage = ctx => +`Invalid value for 'aggregation' query param: ${ctx.value}. Valid ones are 'true' or 'false'`; + module.exports = class AggregationMapConfigAdapter { constructor (pgConnection) { this.pgConnection = pgConnection; @@ -15,6 +18,10 @@ module.exports = class AggregationMapConfigAdapter { getMapConfig (user, requestMapConfig, params, context, callback) { const mapConfig = new MapConfig(requestMapConfig); + if (!this._isValidAggregationParam(params)) { + return callback(new Error(invalidAggregationParamValueErrorMessage({ value: params.aggregation }))); + } + if (!this._shouldAdapt(mapConfig, params)) { return callback(null, requestMapConfig); } @@ -36,6 +43,11 @@ module.exports = class AggregationMapConfigAdapter { }); } + _isValidAggregationParam (params) { + const { aggregation } = params; + return aggregation === undefined || aggregation === 'true' || aggregation === 'false'; + } + _hasMissingColumns (mapConfig) { const layers = mapConfig.getLayers(); @@ -77,11 +89,7 @@ module.exports = class AggregationMapConfigAdapter { return false; } - if (aggregation === 'true') { - return true; - } - - if (mapConfig.isAggregationMapConfig()) { + if (aggregation === 'true' || mapConfig.isAggregationMapConfig()) { return true; } diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index 9c1fed45..56815ccc 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -291,7 +291,8 @@ describe('aggregation', function () { aggregate_function: 'sum', aggregated_column: 'value' } - } + }, + threshold: 1 } } } @@ -308,7 +309,49 @@ describe('aggregation', function () { assert.equal(typeof body.metadata, 'object'); assert.ok(Array.isArray(body.metadata.layers)); - body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation === undefined)); + body.metadata.layers.forEach(layer => assert.equal(layer.meta.aggregation, undefined)); + + done(); + }); + }); + + it('when the aggregation param is not valid should respond with error', function (done) { + const mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1 + } + } + } + ]); + + this.testClient = new TestClient(mapConfig); + const options = { + response: { + status: 400 + }, + aggregation: 'wadus' + }; + + this.testClient.getLayergroup(options, (err, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, { + errors: [ + "Invalid value for 'aggregation' query param: wadus." + + " Valid ones are 'true' or 'false'" + ], + errors_with_context:[{ + type: 'unknown', + message: "Invalid value for 'aggregation' query param: wadus." + + " Valid ones are 'true' or 'false'" + }] + }); done(); }); @@ -352,7 +395,7 @@ describe('aggregation', function () { }); }); - it('when the layer\'s geometry type is not point should responds with error', function (done) { + it('when the layer\'s geometry type is not point should respond with error', function (done) { const mapConfig = createVectorMapConfig([ { type: 'cartodb', diff --git a/test/support/test-client.js b/test/support/test-client.js index 54581f99..f4437a39 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -625,7 +625,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { queryParams.api_key = self.apiKey; } - if (typeof params.aggregation === 'boolean') { + if (params.aggregation !== undefined) { queryParams.aggregation = params.aggregation; } @@ -801,7 +801,7 @@ TestClient.prototype.getLayergroup = function (params, callback) { queryParams.api_key = self.apiKey; } - if (typeof params.aggregation === 'boolean') { + if (params.aggregation !== undefined) { queryParams.aggregation = params.aggregation; }