Merge pull request #822 from CartoDB/aggregation-validation

Validate aggregation input params

lib/cartodb/models/aggregation/aggregation-map-config.js

@@ -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');
-    }
-};

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;
+    }
+};

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;

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;
+}

lib/cartodb/models/aggregation/aggregation.js

@@ -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);
-    }
-};

lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js

@@ -1,9 +1,9 @@
-const Aggregation = require('../../aggregation/aggregation');
+const AggregationMapConfig = require('../../aggregation/aggregation-mapconfig');
-const AggregationMapConfig = require('../../aggregation/aggregation-map-config');
 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 ?
+            if (!AggregationMapConfig.supportsGeometryType(result.type)) {
-                layer.options.aggregation.threshold :
+                const message = unsupportedGeometryTypeErrorMessage({ geometryType: result.type });
-                Aggregation.THRESHOLD;
+                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(error);
-                return callback(null, shouldAdapt);
             }
 
-            const geometryType = result.type;
+            if (!mapConfig.doesLayerReachThreshold(index, result.count)) {
-
+                return callback(null, false);
-            if (geometryType !== 'ST_Point') {
-                return callback(new Error(unsupportedGeometryTypeErrorMessage({ geometryType })));
             }
 
-            shouldAdapt = true;
+            callback(null, true);
-
-            callback(null, shouldAdapt);
         });
     }
 

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();
+                });
+            });
+
         });
     });
 });