From 0bca3d6f337a6690867d6d92aeb7583802373978 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 13:42:27 +0100
Subject: [PATCH 01/29] Validate placement, threshold and resolution

---
 .../aggregation/aggregation-map-config.js     |  89 ++++++++++++
 lib/cartodb/models/aggregation/aggregation.js |   8 ++
 test/acceptance/aggregation.js                | 129 ++++++++++++++++++
 3 files changed, 226 insertions(+)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index e6524e3d..9b12722a 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -1,8 +1,13 @@
 const MapConfig = require('windshaft').model.MapConfig;
+const Aggregation = require('./aggregation');
 
 module.exports = class AggregationMapConfig extends MapConfig {
     constructor (config, datasource) {
         super(config, datasource);
+
+        this.validateResolution();
+        this.validatePlacement();
+        this.validateThreshold();
     }
 
     isAggregationMapConfig () {
@@ -31,4 +36,88 @@ module.exports = class AggregationMapConfig extends MapConfig {
 
         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;
+    }
+
+    validateResolution () {
+        for (let index = 0; index < this.getLayers().length; index++) {
+            const aggregation = this.getAggregation(index);
+
+            if (aggregation === undefined || aggregation.resolution === undefined) {
+                continue;
+            }
+
+            const resolution = parseInt(aggregation.resolution, 10);
+
+            if (!Number.isFinite(resolution) || resolution <= 0)  {
+                const error = new Error(`Invalid resolution, should be a number greather than 0`);
+                error.type = 'layer';
+                error.layer = {
+                    id: this.getLayerId(index),
+                    index: index,
+                    type: this.layerType(index)
+                };
+
+                throw error;
+            }
+        }
+    }
+
+    validatePlacement () {
+        for (let index = 0; index < this.getLayers().length; index++) {
+            const aggregation = this.getAggregation(index);
+
+            if (aggregation === undefined || aggregation.placement === undefined) {
+                continue;
+            }
+
+            if (!Aggregation.PLACEMENTS.includes(aggregation.placement)) {
+                const error = new Error(`Invalid placement. Valid values: ${Aggregation.PLACEMENTS.join(', ')}`);
+                error.type = 'layer';
+                error.layer = {
+                    id: this.getLayerId(index),
+                    index: index,
+                    type: this.layerType(index)
+                };
+
+                throw error;
+            }
+        }
+    }
+
+    validateThreshold () {
+        for (let index = 0; index < this.getLayers().length; index++) {
+            const aggregation = this.getAggregation(index);
+
+            if (aggregation === undefined || aggregation.threshold === undefined) {
+                continue;
+            }
+
+            const threshold = parseInt(aggregation.threshold, 10);
+
+            if (!Number.isFinite(threshold) || threshold <= 0)  {
+                const error = new Error(`Invalid threshold, should be a number greather than 0`);
+                error.type = 'layer';
+                error.layer = {
+                    id: this.getLayerId(index),
+                    index: index,
+                    type: this.layerType(index)
+                };
+
+                throw error;
+            }
+        }
+    }
 };
diff --git a/lib/cartodb/models/aggregation/aggregation.js b/lib/cartodb/models/aggregation/aggregation.js
index 2642898c..b916ffa2 100644
--- a/lib/cartodb/models/aggregation/aggregation.js
+++ b/lib/cartodb/models/aggregation/aggregation.js
@@ -5,6 +5,14 @@ module.exports = class Aggregation {
         return 1e5; // 100K
     }
 
+    static get PLACEMENTS() {
+        return [
+            'centroid',
+            'point-grid',
+            'point-sample'
+        ];
+    }
+
     constructor (mapconfig, query, {
         resolution = 1,
         threshold = Aggregation.THRESHOLD,
diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js
index 561291dc..42990434 100644
--- a/test/acceptance/aggregation.js
+++ b/test/acceptance/aggregation.js
@@ -686,6 +686,135 @@ 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();
+                });
+            });
         });
     });
 });

From 5c1b1e3214f61c5c89b26a8464b15e6d4e92615c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 14:21:03 +0100
Subject: [PATCH 02/29] Improve validation by applying refactor

---
 .../aggregation/aggregation-map-config.js     | 102 +++++++-----------
 1 file changed, 38 insertions(+), 64 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index 9b12722a..d2124ace 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -5,9 +5,9 @@ module.exports = class AggregationMapConfig extends MapConfig {
     constructor (config, datasource) {
         super(config, datasource);
 
-        this.validateResolution();
-        this.validatePlacement();
-        this.validateThreshold();
+        this.validateProperty('resolution', createNumberValidator(this));
+        this.validateProperty('placement', createIncludesValueValidator(this, Aggregation.PLACEMENTS));
+        this.validateProperty('threshold', createNumberValidator(this));
     }
 
     isAggregationMapConfig () {
@@ -51,73 +51,47 @@ module.exports = class AggregationMapConfig extends MapConfig {
         return aggregation;
     }
 
-    validateResolution () {
+    validateProperty (prop, validator) {
         for (let index = 0; index < this.getLayers().length; index++) {
             const aggregation = this.getAggregation(index);
 
-            if (aggregation === undefined || aggregation.resolution === undefined) {
+            if (aggregation === undefined || aggregation[prop] === undefined) {
                 continue;
             }
 
-            const resolution = parseInt(aggregation.resolution, 10);
-
-            if (!Number.isFinite(resolution) || resolution <= 0)  {
-                const error = new Error(`Invalid resolution, should be a number greather than 0`);
-                error.type = 'layer';
-                error.layer = {
-                    id: this.getLayerId(index),
-                    index: index,
-                    type: this.layerType(index)
-                };
-
-                throw error;
-            }
-        }
-    }
-
-    validatePlacement () {
-        for (let index = 0; index < this.getLayers().length; index++) {
-            const aggregation = this.getAggregation(index);
-
-            if (aggregation === undefined || aggregation.placement === undefined) {
-                continue;
-            }
-
-            if (!Aggregation.PLACEMENTS.includes(aggregation.placement)) {
-                const error = new Error(`Invalid placement. Valid values: ${Aggregation.PLACEMENTS.join(', ')}`);
-                error.type = 'layer';
-                error.layer = {
-                    id: this.getLayerId(index),
-                    index: index,
-                    type: this.layerType(index)
-                };
-
-                throw error;
-            }
-        }
-    }
-
-    validateThreshold () {
-        for (let index = 0; index < this.getLayers().length; index++) {
-            const aggregation = this.getAggregation(index);
-
-            if (aggregation === undefined || aggregation.threshold === undefined) {
-                continue;
-            }
-
-            const threshold = parseInt(aggregation.threshold, 10);
-
-            if (!Number.isFinite(threshold) || threshold <= 0)  {
-                const error = new Error(`Invalid threshold, should be a number greather than 0`);
-                error.type = 'layer';
-                error.layer = {
-                    id: this.getLayerId(index),
-                    index: index,
-                    type: this.layerType(index)
-                };
-
-                throw error;
-            }
+            validator(aggregation[prop], prop, index);
         }
     }
 };
+
+function createIncludesValueValidator(mapconfig, validValues) {
+    return function validateIncludesValue (prop, key, index) {
+        if (!validValues.includes(prop)) {
+            const error = new Error(`Invalid ${key}. Valid values: ${validValues.join(', ')}`);
+            error.type = 'layer';
+            error.layer = {
+                id: mapconfig.getLayerId(index),
+                index: index,
+                type: mapconfig.layerType(index)
+            };
+
+            throw error;
+        }
+    };
+}
+
+function createNumberValidator(mapconfig) {
+    return function validateNumber (prop, key, index) {
+        if (!Number.isFinite(prop) || prop <= 0)  {
+            const error = new Error(`Invalid ${key}, should be a number greather than 0`);
+            error.type = 'layer';
+            error.layer = {
+                id: mapconfig.getLayerId(index),
+                index: index,
+                type: mapconfig.layerType(index)
+            };
+
+            throw error;
+        }
+    };
+}

From 153a792fcbd681894796781b8f6ff86e14ceef78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 14:25:44 +0100
Subject: [PATCH 03/29] Improve validation by applying refactor

---
 .../aggregation/aggregation-map-config.js     | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index d2124ace..538681f7 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -5,9 +5,9 @@ module.exports = class AggregationMapConfig extends MapConfig {
     constructor (config, datasource) {
         super(config, datasource);
 
-        this.validateProperty('resolution', createNumberValidator(this));
-        this.validateProperty('placement', createIncludesValueValidator(this, Aggregation.PLACEMENTS));
-        this.validateProperty('threshold', createNumberValidator(this));
+        aggregationValidator(this)('resolution', createNumberValidator(this));
+        aggregationValidator(this)('placement', createIncludesValueValidator(this, Aggregation.PLACEMENTS));
+        aggregationValidator(this)('threshold', createNumberValidator(this));
     }
 
     isAggregationMapConfig () {
@@ -64,6 +64,20 @@ module.exports = class AggregationMapConfig extends MapConfig {
     }
 };
 
+function aggregationValidator (mapconfig) {
+    return function validateProperty (prop, validator) {
+        for (let index = 0; index < mapconfig.getLayers().length; index++) {
+            const aggregation = mapconfig.getAggregation(index);
+
+            if (aggregation === undefined || aggregation[prop] === undefined) {
+                continue;
+            }
+
+            validator(aggregation[prop], prop, index);
+        }
+    };
+}
+
 function createIncludesValueValidator(mapconfig, validValues) {
     return function validateIncludesValue (prop, key, index) {
         if (!validValues.includes(prop)) {

From e81a16ce0de3b6495796f4a28bb11d5bbfc9a6f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 14:31:53 +0100
Subject: [PATCH 04/29] Improve validation by applying refactor

---
 .../aggregation/aggregation-map-config.js      | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index 538681f7..704230fd 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -5,9 +5,7 @@ module.exports = class AggregationMapConfig extends MapConfig {
     constructor (config, datasource) {
         super(config, datasource);
 
-        aggregationValidator(this)('resolution', createNumberValidator(this));
-        aggregationValidator(this)('placement', createIncludesValueValidator(this, Aggregation.PLACEMENTS));
-        aggregationValidator(this)('threshold', createNumberValidator(this));
+        this.validate();
     }
 
     isAggregationMapConfig () {
@@ -51,16 +49,12 @@ module.exports = class AggregationMapConfig extends MapConfig {
         return aggregation;
     }
 
-    validateProperty (prop, validator) {
-        for (let index = 0; index < this.getLayers().length; index++) {
-            const aggregation = this.getAggregation(index);
+    validate () {
+        const validator = aggregationValidator(this);
 
-            if (aggregation === undefined || aggregation[prop] === undefined) {
-                continue;
-            }
-
-            validator(aggregation[prop], prop, index);
-        }
+        validator('resolution', createNumberValidator(this));
+        validator('placement', createIncludesValueValidator(this, Aggregation.PLACEMENTS));
+        validator('threshold', createNumberValidator(this));
     }
 };
 

From d0c88ce21d03a594560a1aa62497a2eb85a78c8b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 17:26:41 +0100
Subject: [PATCH 05/29] Improve naming

---
 .../models/aggregation/aggregation-map-config.js       | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index 704230fd..284ee2a2 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -50,11 +50,13 @@ module.exports = class AggregationMapConfig extends MapConfig {
     }
 
     validate () {
-        const validator = aggregationValidator(this);
+        const validate = aggregationValidator(this);
+        const numberValidator = createNumberValidator(this);
+        const includesValidPlacementsValidator = createIncludesValueValidator(this, Aggregation.PLACEMENTS);
 
-        validator('resolution', createNumberValidator(this));
-        validator('placement', createIncludesValueValidator(this, Aggregation.PLACEMENTS));
-        validator('threshold', createNumberValidator(this));
+        validate('resolution', numberValidator);
+        validate('placement', includesValidPlacementsValidator);
+        validate('threshold', numberValidator);
     }
 };
 

From 170fcc1973643ec85fe20eaa8627cef5a3e2760e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 17:38:16 +0100
Subject: [PATCH 06/29] Move static methods

---
 .../models/aggregation/aggregation-map-config.js  | 15 +++++++++++++--
 lib/cartodb/models/aggregation/aggregation.js     | 15 ++-------------
 .../adapter/aggregation-mapconfig-adapter.js      |  2 +-
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index 284ee2a2..d0e5bffb 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -1,7 +1,18 @@
 const MapConfig = require('windshaft').model.MapConfig;
-const Aggregation = require('./aggregation');
 
 module.exports = class AggregationMapConfig extends MapConfig {
+    static get THRESHOLD () {
+        return 1e5; // 100K
+    }
+
+    static get PLACEMENTS () {
+        return [
+            'centroid',
+            'point-grid',
+            'point-sample'
+        ];
+    }
+
     constructor (config, datasource) {
         super(config, datasource);
 
@@ -52,7 +63,7 @@ module.exports = class AggregationMapConfig extends MapConfig {
     validate () {
         const validate = aggregationValidator(this);
         const numberValidator = createNumberValidator(this);
-        const includesValidPlacementsValidator = createIncludesValueValidator(this, Aggregation.PLACEMENTS);
+        const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS);
 
         validate('resolution', numberValidator);
         validate('placement', includesValidPlacementsValidator);
diff --git a/lib/cartodb/models/aggregation/aggregation.js b/lib/cartodb/models/aggregation/aggregation.js
index b916ffa2..e6c0f9a8 100644
--- a/lib/cartodb/models/aggregation/aggregation.js
+++ b/lib/cartodb/models/aggregation/aggregation.js
@@ -1,21 +1,10 @@
 const aggregationQuery = require('./aggregation-query');
+const AggregationMapConfig = require('./aggregation-map-config');
 
 module.exports = class Aggregation {
-    static get THRESHOLD() {
-        return 1e5; // 100K
-    }
-
-    static get PLACEMENTS() {
-        return [
-            'centroid',
-            'point-grid',
-            'point-sample'
-        ];
-    }
-
     constructor (mapconfig, query, {
         resolution = 1,
-        threshold = Aggregation.THRESHOLD,
+        threshold = AggregationMapConfig.THRESHOLD,
         placement = 'centroid',
         columns = {},
         dimensions = {}
diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index cbbcd50f..73247b42 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -123,7 +123,7 @@ module.exports = class AggregationMapConfigAdapter {
 
             const threshold = layer.options.aggregation && layer.options.aggregation.threshold ?
                 layer.options.aggregation.threshold :
-                Aggregation.THRESHOLD;
+                AggregationMapConfig.THRESHOLD;
 
             if (estimatedFeatureCount < threshold) {
                 return callback(null, shouldAdapt);

From 878f3bd627b6593590d3dc7db855964cd44a061a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 18:17:01 +0100
Subject: [PATCH 07/29] Move .sql() to aggregation-mapconfig

---
 .../aggregation/aggregation-map-config.js     | 21 +++++++++++++++++
 lib/cartodb/models/aggregation/aggregation.js | 23 -------------------
 .../adapter/aggregation-mapconfig-adapter.js  |  5 +---
 3 files changed, 22 insertions(+), 27 deletions(-)
 delete mode 100644 lib/cartodb/models/aggregation/aggregation.js

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index d0e5bffb..b2c3b60e 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -1,4 +1,5 @@
 const MapConfig = require('windshaft').model.MapConfig;
+const aggregationQuery = require('./aggregation-query');
 
 module.exports = class AggregationMapConfig extends MapConfig {
     static get THRESHOLD () {
@@ -19,6 +20,26 @@ module.exports = class AggregationMapConfig extends MapConfig {
         this.validate();
     }
 
+    getAggregatedQuery (index) {
+        const { sql_raw, sql } = this.getLayer(index).options;
+        const {
+            resolution = 1,
+            threshold = AggregationMapConfig.THRESHOLD,
+            placement = 'centroid',
+            columns = {},
+            dimmensions = {}
+        } = this.getAggregation(index);
+
+        return aggregationQuery({
+            query: sql_raw || sql,
+            resolution,
+            threshold,
+            placement,
+            columns,
+            dimmensions
+        });
+    }
+
     isAggregationMapConfig () {
         return this.isVectorOnlyMapConfig() || this.hasAnyLayerAggregation();
     }
diff --git a/lib/cartodb/models/aggregation/aggregation.js b/lib/cartodb/models/aggregation/aggregation.js
deleted file mode 100644
index e6c0f9a8..00000000
--- a/lib/cartodb/models/aggregation/aggregation.js
+++ /dev/null
@@ -1,23 +0,0 @@
-const aggregationQuery = require('./aggregation-query');
-const AggregationMapConfig = require('./aggregation-map-config');
-
-module.exports = class Aggregation {
-    constructor (mapconfig, query, {
-        resolution = 1,
-        threshold = AggregationMapConfig.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 73247b42..2572ecdf 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -1,4 +1,3 @@
-const Aggregation = require('../../aggregation/aggregation');
 const AggregationMapConfig = require('../../aggregation/aggregation-map-config');
 const queryUtils = require('../../../utils/query-utils');
 
@@ -84,11 +83,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);

From 2068861988f443754adac52882fbd8aab90ffab3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 18:24:09 +0100
Subject: [PATCH 08/29] Add PLACEMENT default getter

---
 .../models/aggregation/aggregation-map-config.js   | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index b2c3b60e..2c1781c4 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -2,10 +2,6 @@ const MapConfig = require('windshaft').model.MapConfig;
 const aggregationQuery = require('./aggregation-query');
 
 module.exports = class AggregationMapConfig extends MapConfig {
-    static get THRESHOLD () {
-        return 1e5; // 100K
-    }
-
     static get PLACEMENTS () {
         return [
             'centroid',
@@ -14,6 +10,14 @@ module.exports = class AggregationMapConfig extends MapConfig {
         ];
     }
 
+    static get PLACEMENT () {
+        return AggregationMapConfig.PLACEMENTS[0];
+    }
+
+    static get THRESHOLD () {
+        return 1e5; // 100K
+    }
+
     constructor (config, datasource) {
         super(config, datasource);
 
@@ -25,7 +29,7 @@ module.exports = class AggregationMapConfig extends MapConfig {
         const {
             resolution = 1,
             threshold = AggregationMapConfig.THRESHOLD,
-            placement = 'centroid',
+            placement = AggregationMapConfig.PLACEMENT,
             columns = {},
             dimmensions = {}
         } = this.getAggregation(index);

From 81e0c3a098f1300474c0763dd0d7233e1b446578 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 18:26:08 +0100
Subject: [PATCH 09/29] Add RESOLUTION default getter

---
 lib/cartodb/models/aggregation/aggregation-map-config.js | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index 2c1781c4..26f62b17 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -18,6 +18,10 @@ module.exports = class AggregationMapConfig extends MapConfig {
         return 1e5; // 100K
     }
 
+    static get RESOLUTION () {
+        return 1;
+    }
+
     constructor (config, datasource) {
         super(config, datasource);
 
@@ -27,7 +31,7 @@ module.exports = class AggregationMapConfig extends MapConfig {
     getAggregatedQuery (index) {
         const { sql_raw, sql } = this.getLayer(index).options;
         const {
-            resolution = 1,
+            resolution = AggregationMapConfig.RESOLUTION,
             threshold = AggregationMapConfig.THRESHOLD,
             placement = AggregationMapConfig.PLACEMENT,
             columns = {},

From 47e4b9da0d8e45ce626167f6ab536f95f80f694b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 18:43:05 +0100
Subject: [PATCH 10/29] Encapsulate threshold layer validation in
 aggregation-mapconfig

---
 lib/cartodb/models/aggregation/aggregation-map-config.js  | 8 ++++++++
 .../mapconfig/adapter/aggregation-mapconfig-adapter.js    | 6 +-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index 26f62b17..4411002a 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -89,6 +89,14 @@ module.exports = class AggregationMapConfig extends MapConfig {
         return aggregation;
     }
 
+    doesLayerReachThreshold(index, featureCount) {
+        const threshold = this.getAggregation(index) && this.getAggregation(index).threshold ?
+            this.getAggregation(index).threshold :
+            AggregationMapConfig.THRESHOLD;
+
+        return featureCount >= threshold;
+    }
+
     validate () {
         const validate = aggregationValidator(this);
         const numberValidator = createNumberValidator(this);
diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index 2572ecdf..f3f13828 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -118,11 +118,7 @@ module.exports = class AggregationMapConfigAdapter {
             const result = res.rows[0] || {};
             const estimatedFeatureCount = result.count;
 
-            const threshold = layer.options.aggregation && layer.options.aggregation.threshold ?
-                layer.options.aggregation.threshold :
-                AggregationMapConfig.THRESHOLD;
-
-            if (estimatedFeatureCount < threshold) {
+            if (!mapConfig.doesLayerReachThreshold(index, estimatedFeatureCount)) {
                 return callback(null, shouldAdapt);
             }
 

From 6638ba91c3060f78686ec5456c02f48bd40fb22b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 18:53:44 +0100
Subject: [PATCH 11/29] Refactor supported geometry types

---
 .../models/aggregation/aggregation-map-config.js     | 10 ++++++++++
 .../adapter/aggregation-mapconfig-adapter.js         | 12 +++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-map-config.js b/lib/cartodb/models/aggregation/aggregation-map-config.js
index 4411002a..379d7739 100644
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ b/lib/cartodb/models/aggregation/aggregation-map-config.js
@@ -22,6 +22,16 @@ module.exports = class AggregationMapConfig extends MapConfig {
         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);
 
diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index f3f13828..a99eeaef 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -2,7 +2,8 @@ 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'`;
@@ -116,16 +117,13 @@ module.exports = class AggregationMapConfigAdapter {
             }
 
             const result = res.rows[0] || {};
-            const estimatedFeatureCount = result.count;
 
-            if (!mapConfig.doesLayerReachThreshold(index, estimatedFeatureCount)) {
+            if (!mapConfig.doesLayerReachThreshold(index, result.count)) {
                 return callback(null, shouldAdapt);
             }
 
-            const geometryType = result.type;
-
-            if (geometryType !== 'ST_Point') {
-                return callback(new Error(unsupportedGeometryTypeErrorMessage({ geometryType })));
+            if (!AggregationMapConfig.supportsGeometryType(result.type)) {
+                return callback(new Error(unsupportedGeometryTypeErrorMessage({ geometryType: result.type })));
             }
 
             shouldAdapt = true;

From 800870e783bc502009f1cf3b728ccdea8187a74f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 18:55:32 +0100
Subject: [PATCH 12/29] Remove local variable

---
 .../adapter/aggregation-mapconfig-adapter.js     | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index a99eeaef..3897121a 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -101,10 +101,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({
@@ -113,22 +111,22 @@ module.exports = class AggregationMapConfigAdapter {
 
         connection.query(aggregationMetadata, (err, res) => {
             if (err) {
-                return callback(null, shouldAdapt);
+                return callback(null, false);
             }
 
             const result = res.rows[0] || {};
 
             if (!mapConfig.doesLayerReachThreshold(index, result.count)) {
-                return callback(null, shouldAdapt);
+                return callback(null, false);
             }
 
             if (!AggregationMapConfig.supportsGeometryType(result.type)) {
-                return callback(new Error(unsupportedGeometryTypeErrorMessage({ geometryType: result.type })));
+                return callback(new Error(unsupportedGeometryTypeErrorMessage({
+                    geometryType: result.type
+                })));
             }
 
-            shouldAdapt = true;
-
-            callback(null, shouldAdapt);
+            callback(null, true);
         });
     }
 

From 6a36aa1f1349dbed05b5ad7e55969c3033809847 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 18:56:53 +0100
Subject: [PATCH 13/29] Order checks to validate if a layer should be adapted

---
 .../mapconfig/adapter/aggregation-mapconfig-adapter.js    | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index 3897121a..c3afa7d0 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -116,16 +116,16 @@ module.exports = class AggregationMapConfigAdapter {
 
             const result = res.rows[0] || {};
 
-            if (!mapConfig.doesLayerReachThreshold(index, result.count)) {
-                return callback(null, false);
-            }
-
             if (!AggregationMapConfig.supportsGeometryType(result.type)) {
                 return callback(new Error(unsupportedGeometryTypeErrorMessage({
                     geometryType: result.type
                 })));
             }
 
+            if (!mapConfig.doesLayerReachThreshold(index, result.count)) {
+                return callback(null, false);
+            }
+
             callback(null, true);
         });
     }

From 76b0c94835e277e937d78ce07235795b1c6e038d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 19:05:49 +0100
Subject: [PATCH 14/29] Rename file

---
 .../aggregation/aggregation-map-config.js     | 165 ------------------
 .../adapter/aggregation-mapconfig-adapter.js  |   2 +-
 2 files changed, 1 insertion(+), 166 deletions(-)
 delete mode 100644 lib/cartodb/models/aggregation/aggregation-map-config.js

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 379d7739..00000000
--- a/lib/cartodb/models/aggregation/aggregation-map-config.js
+++ /dev/null
@@ -1,165 +0,0 @@
-const MapConfig = require('windshaft').model.MapConfig;
-const aggregationQuery = require('./aggregation-query');
-
-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);
-
-        this.validate();
-    }
-
-    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;
-    }
-
-    validate () {
-        const validate = aggregationValidator(this);
-        const numberValidator = createNumberValidator(this);
-        const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS);
-
-        validate('resolution', numberValidator);
-        validate('placement', includesValidPlacementsValidator);
-        validate('threshold', numberValidator);
-    }
-};
-
-function aggregationValidator (mapconfig) {
-    return function validateProperty (prop, validator) {
-        for (let index = 0; index < mapconfig.getLayers().length; index++) {
-            const aggregation = mapconfig.getAggregation(index);
-
-            if (aggregation === undefined || aggregation[prop] === undefined) {
-                continue;
-            }
-
-            validator(aggregation[prop], prop, index);
-        }
-    };
-}
-
-function createIncludesValueValidator(mapconfig, validValues) {
-    return function validateIncludesValue (prop, key, index) {
-        if (!validValues.includes(prop)) {
-            const error = new Error(`Invalid ${key}. Valid values: ${validValues.join(', ')}`);
-            error.type = 'layer';
-            error.layer = {
-                id: mapconfig.getLayerId(index),
-                index: index,
-                type: mapconfig.layerType(index)
-            };
-
-            throw error;
-        }
-    };
-}
-
-function createNumberValidator(mapconfig) {
-    return function validateNumber (prop, key, index) {
-        if (!Number.isFinite(prop) || prop <= 0)  {
-            const error = new Error(`Invalid ${key}, should be a number greather than 0`);
-            error.type = 'layer';
-            error.layer = {
-                id: mapconfig.getLayerId(index),
-                index: index,
-                type: mapconfig.layerType(index)
-            };
-
-            throw error;
-        }
-    };
-}
diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index c3afa7d0..1bc13abb 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -1,4 +1,4 @@
-const AggregationMapConfig = require('../../aggregation/aggregation-map-config');
+const AggregationMapConfig = require('../../aggregation/aggregation-mapconfig');
 const queryUtils = require('../../../utils/query-utils');
 
 const unsupportedGeometryTypeErrorMessage = ctx =>

From 8a48b96c53134c75caefb7adf881a22faebb49fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 19:06:01 +0100
Subject: [PATCH 15/29] Rename file

---
 .../aggregation/aggregation-mapconfig.js      | 165 ++++++++++++++++++
 1 file changed, 165 insertions(+)
 create mode 100644 lib/cartodb/models/aggregation/aggregation-mapconfig.js

diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
new file mode 100644
index 00000000..379d7739
--- /dev/null
+++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
@@ -0,0 +1,165 @@
+const MapConfig = require('windshaft').model.MapConfig;
+const aggregationQuery = require('./aggregation-query');
+
+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);
+
+        this.validate();
+    }
+
+    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;
+    }
+
+    validate () {
+        const validate = aggregationValidator(this);
+        const numberValidator = createNumberValidator(this);
+        const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS);
+
+        validate('resolution', numberValidator);
+        validate('placement', includesValidPlacementsValidator);
+        validate('threshold', numberValidator);
+    }
+};
+
+function aggregationValidator (mapconfig) {
+    return function validateProperty (prop, validator) {
+        for (let index = 0; index < mapconfig.getLayers().length; index++) {
+            const aggregation = mapconfig.getAggregation(index);
+
+            if (aggregation === undefined || aggregation[prop] === undefined) {
+                continue;
+            }
+
+            validator(aggregation[prop], prop, index);
+        }
+    };
+}
+
+function createIncludesValueValidator(mapconfig, validValues) {
+    return function validateIncludesValue (prop, key, index) {
+        if (!validValues.includes(prop)) {
+            const error = new Error(`Invalid ${key}. Valid values: ${validValues.join(', ')}`);
+            error.type = 'layer';
+            error.layer = {
+                id: mapconfig.getLayerId(index),
+                index: index,
+                type: mapconfig.layerType(index)
+            };
+
+            throw error;
+        }
+    };
+}
+
+function createNumberValidator(mapconfig) {
+    return function validateNumber (prop, key, index) {
+        if (!Number.isFinite(prop) || prop <= 0)  {
+            const error = new Error(`Invalid ${key}, should be a number greather than 0`);
+            error.type = 'layer';
+            error.layer = {
+                id: mapconfig.getLayerId(index),
+                index: index,
+                type: mapconfig.layerType(index)
+            };
+
+            throw error;
+        }
+    };
+}

From fb03cd34246e00a2afadffc671a432cd0f125c19 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 19:17:43 +0100
Subject: [PATCH 16/29] Move aggregation validation to its own module

---
 .../aggregation/aggregation-mapconfig.js      | 51 ++-----------------
 .../aggregation/aggregation-validator.js      | 47 +++++++++++++++++
 2 files changed, 52 insertions(+), 46 deletions(-)
 create mode 100644 lib/cartodb/models/aggregation/aggregation-validator.js

diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
index 379d7739..6b525def 100644
--- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js
+++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
@@ -1,5 +1,10 @@
 const MapConfig = require('windshaft').model.MapConfig;
 const aggregationQuery = require('./aggregation-query');
+const aggregationValidator = require('./aggregation-validator');
+const {
+    createNumberValidator,
+    createIncludesValueValidator
+ } = aggregationValidator;
 
 module.exports = class AggregationMapConfig extends MapConfig {
     static get PLACEMENTS () {
@@ -117,49 +122,3 @@ module.exports = class AggregationMapConfig extends MapConfig {
         validate('threshold', numberValidator);
     }
 };
-
-function aggregationValidator (mapconfig) {
-    return function validateProperty (prop, validator) {
-        for (let index = 0; index < mapconfig.getLayers().length; index++) {
-            const aggregation = mapconfig.getAggregation(index);
-
-            if (aggregation === undefined || aggregation[prop] === undefined) {
-                continue;
-            }
-
-            validator(aggregation[prop], prop, index);
-        }
-    };
-}
-
-function createIncludesValueValidator(mapconfig, validValues) {
-    return function validateIncludesValue (prop, key, index) {
-        if (!validValues.includes(prop)) {
-            const error = new Error(`Invalid ${key}. Valid values: ${validValues.join(', ')}`);
-            error.type = 'layer';
-            error.layer = {
-                id: mapconfig.getLayerId(index),
-                index: index,
-                type: mapconfig.layerType(index)
-            };
-
-            throw error;
-        }
-    };
-}
-
-function createNumberValidator(mapconfig) {
-    return function validateNumber (prop, key, index) {
-        if (!Number.isFinite(prop) || prop <= 0)  {
-            const error = new Error(`Invalid ${key}, should be a number greather than 0`);
-            error.type = 'layer';
-            error.layer = {
-                id: mapconfig.getLayerId(index),
-                index: index,
-                type: mapconfig.layerType(index)
-            };
-
-            throw error;
-        }
-    };
-}
diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js
new file mode 100644
index 00000000..fc2e9066
--- /dev/null
+++ b/lib/cartodb/models/aggregation/aggregation-validator.js
@@ -0,0 +1,47 @@
+
+
+module.exports = function aggregationValidator (mapconfig) {
+    return function validateProperty (prop, validator) {
+        for (let index = 0; index < mapconfig.getLayers().length; index++) {
+            const aggregation = mapconfig.getAggregation(index);
+
+            if (aggregation === undefined || aggregation[prop] === undefined) {
+                continue;
+            }
+
+            validator(aggregation[prop], prop, index);
+        }
+    };
+};
+
+module.exports.createIncludesValueValidator = function (mapconfig, validValues) {
+    return function validateIncludesValue (prop, key, index) {
+        if (!validValues.includes(prop)) {
+            const error = new Error(`Invalid ${key}. Valid values: ${validValues.join(', ')}`);
+            error.type = 'layer';
+            error.layer = {
+                id: mapconfig.getLayerId(index),
+                index: index,
+                type: mapconfig.layerType(index)
+            };
+
+            throw error;
+        }
+    };
+}
+
+module.exports.createNumberValidator = function (mapconfig) {
+    return function validateNumber (prop, key, index) {
+        if (!Number.isFinite(prop) || prop <= 0)  {
+            const error = new Error(`Invalid ${key}, should be a number greather than 0`);
+            error.type = 'layer';
+            error.layer = {
+                id: mapconfig.getLayerId(index),
+                index: index,
+                type: mapconfig.layerType(index)
+            };
+
+            throw error;
+        }
+    };
+}

From e2bd97eea6bf528a35f9a5c50ca5ab0c4ec9d406 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 19:19:02 +0100
Subject: [PATCH 17/29] Move validation to the constructor

---
 .../models/aggregation/aggregation-mapconfig.js | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
index 6b525def..44a7d408 100644
--- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js
+++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
@@ -40,7 +40,13 @@ module.exports = class AggregationMapConfig extends MapConfig {
     constructor (config, datasource) {
         super(config, datasource);
 
-        this.validate();
+        const validate = aggregationValidator(this);
+        const numberValidator = createNumberValidator(this);
+        const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS);
+
+        validate('resolution', numberValidator);
+        validate('placement', includesValidPlacementsValidator);
+        validate('threshold', numberValidator);
     }
 
     getAggregatedQuery (index) {
@@ -112,13 +118,4 @@ module.exports = class AggregationMapConfig extends MapConfig {
         return featureCount >= threshold;
     }
 
-    validate () {
-        const validate = aggregationValidator(this);
-        const numberValidator = createNumberValidator(this);
-        const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS);
-
-        validate('resolution', numberValidator);
-        validate('placement', includesValidPlacementsValidator);
-        validate('threshold', numberValidator);
-    }
 };

From 2dda0a80da09435d38688a9a33633048781b2fa4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 19:35:12 +0100
Subject: [PATCH 18/29] Improve error context

---
 .../models/aggregation/aggregation-mapconfig.js      |  1 -
 .../adapter/aggregation-mapconfig-adapter.js         | 12 ++++++++++--
 test/acceptance/aggregation.js                       |  9 +++++++--
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
index 44a7d408..a22e8316 100644
--- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js
+++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
@@ -117,5 +117,4 @@ module.exports = class AggregationMapConfig extends MapConfig {
 
         return featureCount >= threshold;
     }
-
 };
diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index 1bc13abb..1c3466d7 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -117,9 +117,17 @@ module.exports = class AggregationMapConfigAdapter {
             const result = res.rows[0] || {};
 
             if (!AggregationMapConfig.supportsGeometryType(result.type)) {
-                return callback(new Error(unsupportedGeometryTypeErrorMessage({
+                const error = new Error(unsupportedGeometryTypeErrorMessage({
                     geometryType: result.type
-                })));
+                }));
+                error.type = 'layer';
+                error.layer = {
+                    id: mapConfig.getLayerId(index),
+                    index: index,
+                    type: mapConfig.layerType(index)
+                };
+
+                return callback(error);
             }
 
             if (!mapConfig.doesLayerReachThreshold(index, result.count)) {
diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js
index 42990434..074d773e 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'
+                            }
                         }]
                     });
 

From 777df6337be80970d1644174071cc12e5e8e16e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 19:47:11 +0100
Subject: [PATCH 19/29] Style typo

---
 lib/cartodb/models/aggregation/aggregation-validator.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js
index fc2e9066..db3eb629 100644
--- a/lib/cartodb/models/aggregation/aggregation-validator.js
+++ b/lib/cartodb/models/aggregation/aggregation-validator.js
@@ -28,7 +28,7 @@ module.exports.createIncludesValueValidator = function (mapconfig, validValues)
             throw error;
         }
     };
-}
+};
 
 module.exports.createNumberValidator = function (mapconfig) {
     return function validateNumber (prop, key, index) {
@@ -44,4 +44,4 @@ module.exports.createNumberValidator = function (mapconfig) {
             throw error;
         }
     };
-}
+};

From c63226cd267f99b71c3ff9a7aa477c0f70365947 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 19:51:55 +0100
Subject: [PATCH 20/29] Improve function naming

---
 lib/cartodb/models/aggregation/aggregation-mapconfig.js | 4 ++--
 lib/cartodb/models/aggregation/aggregation-validator.js | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
index a22e8316..f68417ef 100644
--- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js
+++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
@@ -2,7 +2,7 @@ const MapConfig = require('windshaft').model.MapConfig;
 const aggregationQuery = require('./aggregation-query');
 const aggregationValidator = require('./aggregation-validator');
 const {
-    createNumberValidator,
+    createPositiveNumberValidator,
     createIncludesValueValidator
  } = aggregationValidator;
 
@@ -41,7 +41,7 @@ module.exports = class AggregationMapConfig extends MapConfig {
         super(config, datasource);
 
         const validate = aggregationValidator(this);
-        const numberValidator = createNumberValidator(this);
+        const numberValidator = createPositiveNumberValidator(this);
         const includesValidPlacementsValidator = createIncludesValueValidator(this, AggregationMapConfig.PLACEMENTS);
 
         validate('resolution', numberValidator);
diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js
index db3eb629..2dcc4ad7 100644
--- a/lib/cartodb/models/aggregation/aggregation-validator.js
+++ b/lib/cartodb/models/aggregation/aggregation-validator.js
@@ -30,9 +30,9 @@ module.exports.createIncludesValueValidator = function (mapconfig, validValues)
     };
 };
 
-module.exports.createNumberValidator = function (mapconfig) {
-    return function validateNumber (prop, key, index) {
-        if (!Number.isFinite(prop) || prop <= 0)  {
+module.exports.createPositiveNumberValidator = function (mapconfig) {
+    return function validatePositiveNumber (prop, key, index) {
+        if (!Number.isFinite(prop) || prop <= 0) {
             const error = new Error(`Invalid ${key}, should be a number greather than 0`);
             error.type = 'layer';
             error.layer = {

From fa7140e736fed08abed6544fa65c20110851a751 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 19:52:50 +0100
Subject: [PATCH 21/29] Rename argument

---
 lib/cartodb/models/aggregation/aggregation-validator.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js
index 2dcc4ad7..cfa5e9c8 100644
--- a/lib/cartodb/models/aggregation/aggregation-validator.js
+++ b/lib/cartodb/models/aggregation/aggregation-validator.js
@@ -15,8 +15,8 @@ module.exports = function aggregationValidator (mapconfig) {
 };
 
 module.exports.createIncludesValueValidator = function (mapconfig, validValues) {
-    return function validateIncludesValue (prop, key, index) {
-        if (!validValues.includes(prop)) {
+    return function validateIncludesValue (value, key, index) {
+        if (!validValues.includes(value)) {
             const error = new Error(`Invalid ${key}. Valid values: ${validValues.join(', ')}`);
             error.type = 'layer';
             error.layer = {
@@ -31,8 +31,8 @@ module.exports.createIncludesValueValidator = function (mapconfig, validValues)
 };
 
 module.exports.createPositiveNumberValidator = function (mapconfig) {
-    return function validatePositiveNumber (prop, key, index) {
-        if (!Number.isFinite(prop) || prop <= 0) {
+    return function validatePositiveNumber (value, key, index) {
+        if (!Number.isFinite(value) || value <= 0) {
             const error = new Error(`Invalid ${key}, should be a number greather than 0`);
             error.type = 'layer';
             error.layer = {

From c367743d76ad0b12911608e6bf420039a10674a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 20:06:16 +0100
Subject: [PATCH 22/29] Export SUPPORTED_AGGREGATE_FUNCTIONS

---
 lib/cartodb/models/aggregation/aggregation-query.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index 3fc68c8e..d5b819e3 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;

From bdce2f95f22a53f67dca461c5338c192a2ffb0c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 18 Dec 2017 20:42:26 +0100
Subject: [PATCH 23/29] Add validations for columns

---
 .../aggregation/aggregation-mapconfig.js      |  12 +-
 .../aggregation/aggregation-validator.js      |  53 ++++++-
 test/acceptance/aggregation.js                | 146 ++++++++++++++++++
 3 files changed, 205 insertions(+), 6 deletions(-)

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

From cace6169c01e92eb3c21aea3a8070971710be464 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Tue, 19 Dec 2017 10:25:41 +0100
Subject: [PATCH 24/29] Add function to create layer errors

---
 .../aggregation/aggregation-validator.js      | 73 ++++++-------------
 1 file changed, 23 insertions(+), 50 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js
index d0d71cc7..25c06ec4 100644
--- a/lib/cartodb/models/aggregation/aggregation-validator.js
+++ b/lib/cartodb/models/aggregation/aggregation-validator.js
@@ -15,15 +15,8 @@ module.exports = function aggregationValidator (mapconfig) {
 module.exports.createIncludesValueValidator = function (mapconfig, validValues) {
     return function validateIncludesValue (value, key, index) {
         if (!validValues.includes(value)) {
-            const error = new Error(`Invalid ${key}. Valid values: ${validValues.join(', ')}`);
-            error.type = 'layer';
-            error.layer = {
-                id: mapconfig.getLayerId(index),
-                index: index,
-                type: mapconfig.layerType(index)
-            };
-
-            throw error;
+            const message = `Invalid ${key}. Valid values: ${validValues.join(', ')}`;
+            throw createLayerError(message, mapconfig, index);
         }
     };
 };
@@ -31,15 +24,8 @@ module.exports.createIncludesValueValidator = function (mapconfig, validValues)
 module.exports.createPositiveNumberValidator = function (mapconfig) {
     return function validatePositiveNumber (value, key, index) {
         if (!Number.isFinite(value) || value <= 0) {
-            const error = new Error(`Invalid ${key}, should be a number greather than 0`);
-            error.type = 'layer';
-            error.layer = {
-                id: mapconfig.getLayerId(index),
-                index: index,
-                type: mapconfig.layerType(index)
-            };
-
-            throw error;
+            const message = `Invalid ${key}, should be a number greather than 0`;
+            throw createLayerError(message, mapconfig, index);
         }
     };
 };
@@ -48,49 +34,36 @@ module.exports.createAggregationColumnsValidator = function (mapconfig, validAgg
     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 message = `Invalid column name, should be a non empty string`;
+                throw createLayerError(message, mapconfig, index);
             }
 
             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 message = `Unsupported aggregation function ${aggregate_function},` +
+                                ` valid ones: ${validAggregatedFunctions.join(', ')}`;
+                throw createLayerError(message, mapconfig, index);
             }
 
             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;
+                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;
+}

From 45a663d5ae81299f9326dd527e6fbc99421891c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Tue, 19 Dec 2017 10:43:34 +0100
Subject: [PATCH 25/29] Split columns validator

---
 .../aggregation/aggregation-validator.js      | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js
index 25c06ec4..97f31b27 100644
--- a/lib/cartodb/models/aggregation/aggregation-validator.js
+++ b/lib/cartodb/models/aggregation/aggregation-validator.js
@@ -31,13 +31,31 @@ module.exports.createPositiveNumberValidator = function (mapconfig) {
 };
 
 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)) {
@@ -45,7 +63,13 @@ module.exports.createAggregationColumnsValidator = function (mapconfig, validAgg
                                 ` 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) {
@@ -54,7 +78,7 @@ module.exports.createAggregationColumnsValidator = function (mapconfig, validAgg
             }
         });
     };
-};
+}
 
 function createLayerError(message, mapconfig, index) {
     const error = new Error(message);

From 79b04bbdfddfc147a250ede3398f0216b2cfacd1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Tue, 19 Dec 2017 10:47:53 +0100
Subject: [PATCH 26/29] Rename param

---
 lib/cartodb/models/aggregation/aggregation-validator.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-validator.js b/lib/cartodb/models/aggregation/aggregation-validator.js
index 97f31b27..d0dc24c2 100644
--- a/lib/cartodb/models/aggregation/aggregation-validator.js
+++ b/lib/cartodb/models/aggregation/aggregation-validator.js
@@ -1,13 +1,13 @@
 module.exports = function aggregationValidator (mapconfig) {
-    return function validateProperty (prop, validator) {
+    return function validateProperty (key, validator) {
         for (let index = 0; index < mapconfig.getLayers().length; index++) {
             const aggregation = mapconfig.getAggregation(index);
 
-            if (aggregation === undefined || aggregation[prop] === undefined) {
+            if (aggregation === undefined || aggregation[key] === undefined) {
                 continue;
             }
 
-            validator(aggregation[prop], prop, index);
+            validator(aggregation[key], key, index);
         }
     };
 };

From 34808d61471116d3a1953b66b13451695cf028ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Tue, 19 Dec 2017 10:50:53 +0100
Subject: [PATCH 27/29] Improve naming

---
 .../models/mapconfig/adapter/aggregation-mapconfig-adapter.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index 1c3466d7..f424c0d2 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -14,7 +14,7 @@ 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 })));
         }
 
@@ -33,7 +33,7 @@ module.exports = class AggregationMapConfigAdapter {
         });
     }
 
-    _isValidAggregationParam (params) {
+    _isValidAggregationQueryParam (params) {
         const { aggregation } = params;
         return aggregation === undefined || aggregation === 'true' || aggregation === 'false';
     }

From 326cad2f2cb8a63722a39611fe8b2c99c9a2e8a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Tue, 19 Dec 2017 10:54:20 +0100
Subject: [PATCH 28/29] Typo

---
 lib/cartodb/models/aggregation/aggregation-mapconfig.js      | 2 +-
 .../mapconfig/adapter/aggregation-mapconfig-adapter.js       | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
index a50dae41..c6dbcad4 100644
--- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js
+++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js
@@ -6,7 +6,7 @@ const {
     createPositiveNumberValidator,
     createIncludesValueValidator,
     createAggregationColumnsValidator
- } = aggregationValidator;
+} = aggregationValidator;
 
 module.exports = class AggregationMapConfig extends MapConfig {
     static get PLACEMENTS () {
diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index f424c0d2..a73e99a9 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -117,9 +117,8 @@ module.exports = class AggregationMapConfigAdapter {
             const result = res.rows[0] || {};
 
             if (!AggregationMapConfig.supportsGeometryType(result.type)) {
-                const error = new Error(unsupportedGeometryTypeErrorMessage({
-                    geometryType: result.type
-                }));
+                const message = unsupportedGeometryTypeErrorMessage({ geometryType: result.type });
+                const error = new Error(message);
                 error.type = 'layer';
                 error.layer = {
                     id: mapConfig.getLayerId(index),

From f22216e6d218b8aa3b9371cbd4d19ca58a11ac71 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Tue, 19 Dec 2017 12:23:54 +0100
Subject: [PATCH 29/29] Catch error threw from constructor and follow node
 callback pattern

---
 .../mapconfig/adapter/aggregation-mapconfig-adapter.js    | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index a73e99a9..425aade7 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -18,7 +18,13 @@ module.exports = class AggregationMapConfigAdapter {
             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);