diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js deleted file mode 100644 index e6524e3d..00000000 --- a/lib/cartodb/models/aggregation/aggregation-map-config.js +++ /dev/null @@ -1,34 +0,0 @@ -const MapConfig = require('windshaft').model.MapConfig; - -module.exports = class AggregationMapConfig extends MapConfig { - constructor (config, datasource) { - super(config, datasource); - } - - isAggregationMapConfig () { - return this.isVectorOnlyMapConfig() || this.hasAnyLayerAggregation(); - } - - isAggregationLayer (index) { - return this.isVectorOnlyMapConfig() || this.hasLayerAggregation(index); - } - - hasAnyLayerAggregation () { - const layers = this.getLayers(); - - for (let index = 0; index < layers.length; index++) { - if (this.hasLayerAggregation(index)) { - return true; - } - } - - return false; - } - - hasLayerAggregation (index) { - const layer = this.getLayer(index); - const { aggregation } = layer.options; - - return aggregation !== undefined && (typeof aggregation === 'object' || typeof aggregation === 'boolean'); - } -}; diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js new file mode 100644 index 00000000..c6dbcad4 --- /dev/null +++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js @@ -0,0 +1,124 @@ +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, + createAggregationColumnsValidator +} = aggregationValidator; + +module.exports = class AggregationMapConfig extends MapConfig { + static get PLACEMENTS () { + return [ + 'centroid', + 'point-grid', + 'point-sample' + ]; + } + + static get PLACEMENT () { + return AggregationMapConfig.PLACEMENTS[0]; + } + + static get THRESHOLD () { + return 1e5; // 100K + } + + static get RESOLUTION () { + return 1; + } + + static get SUPPORTED_GEOMETRY_TYPES () { + return [ + 'ST_Point' + ]; + } + + static supportsGeometryType(geometryType) { + return AggregationMapConfig.SUPPORTED_GEOMETRY_TYPES.includes(geometryType); + } + + constructor (config, datasource) { + super(config, datasource); + + const validate = aggregationValidator(this); + const positiveNumberValidator = createPositiveNumberValidator(this); + const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS); + const aggregationColumnsValidator = createAggregationColumnsValidator(this, SUPPORTED_AGGREGATE_FUNCTIONS); + + validate('resolution', positiveNumberValidator); + validate('placement', includesValidPlacementsValidator); + validate('threshold', positiveNumberValidator); + validate('columns', aggregationColumnsValidator); + } + + getAggregatedQuery (index) { + const { sql_raw, sql } = this.getLayer(index).options; + const { + resolution = AggregationMapConfig.RESOLUTION, + threshold = AggregationMapConfig.THRESHOLD, + placement = AggregationMapConfig.PLACEMENT, + columns = {}, + dimmensions = {} + } = this.getAggregation(index); + + return aggregationQuery({ + query: sql_raw || sql, + resolution, + threshold, + placement, + columns, + dimmensions + }); + } + + isAggregationMapConfig () { + return this.isVectorOnlyMapConfig() || this.hasAnyLayerAggregation(); + } + + isAggregationLayer (index) { + return this.isVectorOnlyMapConfig() || this.hasLayerAggregation(index); + } + + hasAnyLayerAggregation () { + const layers = this.getLayers(); + + for (let index = 0; index < layers.length; index++) { + if (this.hasLayerAggregation(index)) { + return true; + } + } + + return false; + } + + hasLayerAggregation (index) { + const layer = this.getLayer(index); + const { aggregation } = layer.options; + + return aggregation !== undefined && (typeof aggregation === 'object' || typeof aggregation === 'boolean'); + } + + getAggregation (index) { + if (!this.hasLayerAggregation(index)) { + return; + } + + const { aggregation } = this.getLayer(index).options; + + if (typeof aggregation === 'boolean') { + return {}; + } + + return aggregation; + } + + doesLayerReachThreshold(index, featureCount) { + const threshold = this.getAggregation(index) && this.getAggregation(index).threshold ? + this.getAggregation(index).threshold : + AggregationMapConfig.THRESHOLD; + + return featureCount >= threshold; + } +}; diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index 69118021..06858373 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -56,6 +56,8 @@ const SUPPORTED_AGGREGATE_FUNCTIONS = { } }; +module.exports.SUPPORTED_AGGREGATE_FUNCTIONS = Object.keys(SUPPORTED_AGGREGATE_FUNCTIONS); + const sep = (list) => { let expr = list.join(', '); return expr ? ', ' + expr : expr; diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js new file mode 100644 index 00000000..d0dc24c2 --- /dev/null +++ b/lib/cartodb/models/aggregation/aggregation-validator.js @@ -0,0 +1,93 @@ +module.exports = function aggregationValidator (mapconfig) { + return function validateProperty (key, validator) { + for (let index = 0; index < mapconfig.getLayers().length; index++) { + const aggregation = mapconfig.getAggregation(index); + + if (aggregation === undefined || aggregation[key] === undefined) { + continue; + } + + validator(aggregation[key], key, index); + } + }; +}; + +module.exports.createIncludesValueValidator = function (mapconfig, validValues) { + return function validateIncludesValue (value, key, index) { + if (!validValues.includes(value)) { + const message = `Invalid ${key}. Valid values: ${validValues.join(', ')}`; + throw createLayerError(message, mapconfig, index); + } + }; +}; + +module.exports.createPositiveNumberValidator = function (mapconfig) { + return function validatePositiveNumber (value, key, index) { + if (!Number.isFinite(value) || value <= 0) { + const message = `Invalid ${key}, should be a number greather than 0`; + throw createLayerError(message, mapconfig, index); + } + }; +}; + +module.exports.createAggregationColumnsValidator = function (mapconfig, validAggregatedFunctions) { + const validateAggregationColumnNames = createAggregationColumnNamesValidator(mapconfig); + const validateAggregateFunction = createAggregateFunctionValidator(mapconfig, validAggregatedFunctions); + const validateAggregatedColumn = createAggregatedColumnValidator(mapconfig); + + return function validateAggregationColumns (value, key, index) { + validateAggregationColumnNames(value, key, index); + validateAggregateFunction(value, key, index); + validateAggregatedColumn(value, key, index); + }; +}; + +function createAggregationColumnNamesValidator(mapconfig) { + return function validateAggregationColumnNames (value, key, index) { + Object.keys(value).forEach((columnName) => { + if (columnName.length <= 0) { + const message = `Invalid column name, should be a non empty string`; + throw createLayerError(message, mapconfig, index); + } + }); + }; +} + +function createAggregateFunctionValidator (mapconfig, validAggregatedFunctions) { + return function validateAggregateFunction (value, key, index) { + Object.keys(value).forEach((columnName) => { + const { aggregate_function } = value[columnName]; + + if (!validAggregatedFunctions.includes(aggregate_function)) { + const message = `Unsupported aggregation function ${aggregate_function},` + + ` valid ones: ${validAggregatedFunctions.join(', ')}`; + throw createLayerError(message, mapconfig, index); + } + }); + }; +} + +function createAggregatedColumnValidator (mapconfig) { + return function validateAggregatedColumn (value, key, index) { + Object.keys(value).forEach((columnName) => { + const { aggregated_column } = value[columnName]; + + if (typeof aggregated_column !== 'string' || aggregated_column <= 0) { + const message = `Invalid aggregated column, should be a non empty string`; + throw createLayerError(message, mapconfig, index); + } + }); + }; +} + +function createLayerError(message, mapconfig, index) { + const error = new Error(message); + error.type = 'layer'; + error.layer = { + id: mapconfig.getLayerId(index), + index: index, + type: mapconfig.layerType(index) + }; + + return error; +} diff --git a/lib/cartodb/models/aggregation/aggregation.js b/lib/cartodb/models/aggregation/aggregation.js deleted file mode 100644 index 2642898c..00000000 --- a/lib/cartodb/models/aggregation/aggregation.js +++ /dev/null @@ -1,26 +0,0 @@ -const aggregationQuery = require('./aggregation-query'); - -module.exports = class Aggregation { - static get THRESHOLD() { - return 1e5; // 100K - } - - constructor (mapconfig, query, { - resolution = 1, - threshold = Aggregation.THRESHOLD, - placement = 'centroid', - columns = {}, - dimensions = {} - } = {}) { - this.mapconfig = mapconfig; - this.query = query; - this.resolution = resolution; - this.threshold = threshold; - this.placement = placement; - this.columns = columns; - this.dimensions = dimensions; - } - sql () { - return aggregationQuery(this); - } -}; diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js index cbbcd50f..425aade7 100644 --- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js @@ -1,9 +1,9 @@ -const Aggregation = require('../../aggregation/aggregation'); -const AggregationMapConfig = require('../../aggregation/aggregation-map-config'); +const AggregationMapConfig = require('../../aggregation/aggregation-mapconfig'); const queryUtils = require('../../../utils/query-utils'); const unsupportedGeometryTypeErrorMessage = ctx => -`Unsupported geometry type: ${ctx.geometryType}. Aggregation is available only for geometry type: ST_Point`; +`Unsupported geometry type: ${ctx.geometryType}. ` + +`Aggregation is available only for geometry type: ${AggregationMapConfig.SUPPORTED_GEOMETRY_TYPES}`; const invalidAggregationParamValueErrorMessage = ctx => `Invalid value for 'aggregation' query param: ${ctx.value}. Valid ones are 'true' or 'false'`; @@ -14,11 +14,17 @@ module.exports = class AggregationMapConfigAdapter { } getMapConfig (user, requestMapConfig, params, context, callback) { - if (!this._isValidAggregationParam(params)) { + if (!this._isValidAggregationQueryParam(params)) { return callback(new Error(invalidAggregationParamValueErrorMessage({ value: params.aggregation }))); } - const mapConfig = new AggregationMapConfig(requestMapConfig); + let mapConfig; + try { + mapConfig = new AggregationMapConfig(requestMapConfig); + } catch (err) { + return callback(err); + } + if (!this._shouldAdapt(mapConfig, params)) { return callback(null, requestMapConfig); @@ -33,7 +39,7 @@ module.exports = class AggregationMapConfigAdapter { }); } - _isValidAggregationParam (params) { + _isValidAggregationQueryParam (params) { const { aggregation } = params; return aggregation === undefined || aggregation === 'true' || aggregation === 'false'; } @@ -84,11 +90,9 @@ module.exports = class AggregationMapConfigAdapter { } if (shouldAdapt) { - const sql = layer.options.sql_raw ? layer.options.sql_raw : layer.options.sql; - const aggregation = new Aggregation(mapConfig, sql, layer.options.aggregation); const sqlQueryWrap = layer.options.sql_wrap; - let aggregationSql = aggregation.sql(); + let aggregationSql = mapConfig.getAggregatedQuery(index); if (sqlQueryWrap) { aggregationSql = sqlQueryWrap.replace(/<%=\s*sql\s*%>/g, aggregationSql); @@ -103,10 +107,8 @@ module.exports = class AggregationMapConfigAdapter { } _shouldAdaptLayer (connection, mapConfig, layer, index, callback) { - let shouldAdapt = false; - if (!mapConfig.isAggregationLayer(index)) { - return callback(null, shouldAdapt); + return callback(null, false); } const aggregationMetadata = queryUtils.getAggregationMetadata({ @@ -115,29 +117,29 @@ module.exports = class AggregationMapConfigAdapter { connection.query(aggregationMetadata, (err, res) => { if (err) { - return callback(null, shouldAdapt); + return callback(null, false); } const result = res.rows[0] || {}; - const estimatedFeatureCount = result.count; - const threshold = layer.options.aggregation && layer.options.aggregation.threshold ? - layer.options.aggregation.threshold : - Aggregation.THRESHOLD; + if (!AggregationMapConfig.supportsGeometryType(result.type)) { + const message = unsupportedGeometryTypeErrorMessage({ geometryType: result.type }); + const error = new Error(message); + error.type = 'layer'; + error.layer = { + id: mapConfig.getLayerId(index), + index: index, + type: mapConfig.layerType(index) + }; - if (estimatedFeatureCount < threshold) { - return callback(null, shouldAdapt); + return callback(error); } - const geometryType = result.type; - - if (geometryType !== 'ST_Point') { - return callback(new Error(unsupportedGeometryTypeErrorMessage({ geometryType }))); + if (!mapConfig.doesLayerReachThreshold(index, result.count)) { + return callback(null, false); } - shouldAdapt = true; - - callback(null, shouldAdapt); + callback(null, true); }); } diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index 561291dc..73284582 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -456,9 +456,14 @@ describe('aggregation', function () { ' Aggregation is available only for geometry type: ST_Point' ], errors_with_context:[{ - type: 'unknown', + type: 'layer', message: 'Unsupported geometry type: ST_Polygon.' + - ' Aggregation is available only for geometry type: ST_Point' + ' Aggregation is available only for geometry type: ST_Point', + layer: { + id: 'layer0', + index: 0, + type: 'mapnik' + } }] }); @@ -686,6 +691,281 @@ describe('aggregation', function () { done(); }); }); + + it('should fail with bad resolution', function (done) { + this.mapConfig = createVectorMapConfig([ + { + id: 'wadus', + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + resolution: 'wadus', + } + } + } + ]); + + 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 resolution, should be a number greather than 0' ], + errors_with_context:[{ + type: 'layer', + message: 'Invalid resolution, should be a number greather than 0', + layer: { + "id": "wadus", + "index": 0, + "type": "mapnik" + } + }] + }); + done(); + }); + }); + + it('should fail with bad placement', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + placement: 'wadus', + } + } + } + ]); + + 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 placement. Valid values: centroid, point-grid, point-sample'], + errors_with_context:[{ + type: 'layer', + message: 'Invalid placement. Valid values: centroid, point-grid, point-sample', + layer: { + id: "layer0", + index: 0, + type: "mapnik", + } + }] + }); + + done(); + }); + }); + + it('should fail with bad threshold', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 'wadus', + } + } + } + ]); + + 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 threshold, should be a number greather than 0' ], + errors_with_context:[{ + type: 'layer', + message: 'Invalid threshold, should be a number greather than 0', + layer: { + "id": "layer0", + "index": 0, + "type": "mapnik" + } + }] + }); + + 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(); + }); + }); + }); }); });