diff --git a/lib/cartodb/backends/cluster.js b/lib/cartodb/backends/cluster.js index cba5b80e..671684e4 100644 --- a/lib/cartodb/backends/cluster.js +++ b/lib/cartodb/backends/cluster.js @@ -38,7 +38,61 @@ module.exports = class ClusterBackend { return callback(error); } - const { aggregation } = params; + let { aggregation } = params; + + if ( aggregation !== undefined) { + try { + aggregation = JSON.parse(aggregation); + } catch (err) { + const error = new Error(`Invalid aggregation input, should be a a valid JSON`); + error.http_status = 400; + error.type = 'layer'; + error.subtype = 'aggregation'; + error.layer = { + index: layerIndex, + type: layer.type + }; + + return callback(error); + } + + const { columns, expressions } = aggregation; + + if (!Array.isArray(columns) || !columns.length) { + const error = new Error( + `Invalid aggregation input, columns should be and array of column names` + ); + error.http_status = 400; + error.type = 'layer'; + error.subtype = 'aggregation'; + error.layer = { + index: layerIndex, + type: layer.type + }; + + return callback(error); + } + + if (expressions !== undefined) { + if (expressions === null || + Array.isArray(expressions) || + ['string', 'number', 'boolean'].includes(typeof expressions)) { + const error = new Error( + `Invalid aggregation input, expressions should be and object with expressions` + ); + error.http_status = 400; + error.type = 'layer'; + error.subtype = 'aggregation'; + error.layer = { + index: layerIndex, + type: layer.type + }; + + return callback(error); + } + } + } + const query = layer.options.sql_raw; const resolution = layer.options.aggregation.resolution || 1; @@ -96,13 +150,12 @@ function getClusterFeatures (pg, zoom, clusterId, columns, query, resolution, ag }); if (aggregation !== undefined) { - aggregation = JSON.parse(aggregation); - const { columns = [], expresions = [] } = aggregation; + const { columns = [], expressions = [] } = aggregation; sql = aggregationQuery({ columns, query: sql, - expresions + expressions }); } @@ -155,7 +208,7 @@ const aggregationQuery = ctx => ` SELECT count(1) as _cdb_feature_count ${ctx.columns.length ? `,${ctx.columns.join(', ')}` : ''} - ${ctx.expresions.length ? `,${ctx.expresions.join(', ')}` : ''} + ${ctx.expressions.length ? `,${ctx.expressions.join(', ')}` : ''} FROM (${ctx.query}) __cdb_aggregation ${ctx.columns.length ? `GROUP BY ${ctx.columns.join(', ')}` : ''} `; diff --git a/test/acceptance/cluster.js b/test/acceptance/cluster.js index 8b6ee5d7..5e856da2 100644 --- a/test/acceptance/cluster.js +++ b/test/acceptance/cluster.js @@ -324,7 +324,7 @@ describe('cluster', function () { }); }); - describe('map-config w/o aggregation', function () { + describe('with aggregation', function () { const suite = [ { zoom: 0, @@ -397,7 +397,6 @@ describe('cluster', function () { } ]; - suite.forEach(({ zoom, cartodb_id, resolution, aggregation, expected }) => { it('should return features aggregated by type', function (done) { const mapConfig = createVectorMapConfig([{ @@ -426,4 +425,160 @@ describe('cluster', function () { }); }); }); + + describe('invalid aggregation', function () { + const expectedColumnsError = { + errors:[ 'Invalid aggregation input, columns should be and array of column names' ], + errors_with_context:[ + { + layer: { + index: '0', + type: 'cartodb' + }, + message: 'Invalid aggregation input, columns should be and array of column names', + subtype: 'aggregation', + type: 'layer' + } + ] + }; + + const expectedExpressionsError = { + errors:[ 'Invalid aggregation input, expressions should be and object with expressions' ], + errors_with_context:[ + { + layer: { + index: '0', + type: 'cartodb' + }, + message: 'Invalid aggregation input, expressions should be and object with expressions', + subtype: 'aggregation', + type: 'layer' + } + ] + }; + + const suite = [ + { + description: 'empty aggregation object should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: {}, + expected: expectedColumnsError + }, + { + description: 'empty aggregation array should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: [], + expected: expectedColumnsError + }, + { + description: 'aggregation as string should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: 'wadus', + expected: expectedColumnsError + }, + { + description: 'empty columns array should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: { columns: [] }, + expected: expectedColumnsError + }, + { + description: 'empty columns object should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: { columns: {} }, + expected: expectedColumnsError + }, + { + description: 'columns as string should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: { columns: 'wadus' }, + expected: expectedColumnsError + }, + { + description: 'columns as null should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: { columns: null }, + expected: expectedColumnsError + }, + { + description: 'empty expressions array should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: { columns: [ 'type' ], expressions: [] }, + expected: expectedExpressionsError + }, + { + description: 'empty expressions number should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: { columns: [ 'type' ], expressions: 1 }, + expected: expectedExpressionsError + }, + { + description: 'expressions as string should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: { columns: [ 'type' ], expressions: 'wadus' }, + expected: expectedExpressionsError + }, + { + description: 'expressions as null should respond with error', + zoom: 0, + cartodb_id: 1, + resolution: 1, + aggregation: { columns: [ 'type' ], expressions: null }, + expected: expectedExpressionsError + } + ]; + + suite.forEach(({ description, zoom, cartodb_id, resolution, aggregation, expected }) => { + it(description, function (done) { + const mapConfig = createVectorMapConfig([{ + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1, + resolution + } + } + }]); + const testClient = new TestClient(mapConfig); + const layerId = 0; + const params = { + response: { + status: 400 + }, + aggregation + }; + + testClient.getClusterFeatures(zoom, cartodb_id, layerId, params, (err, body) => { + if (err) { + return done(err); + } + + assert.deepStrictEqual(body, expected); + + testClient.drain(done); + }); + }); + }); + }); });