From aa43eb8953a8ab833dad60ad7e54e52fdd1b5865 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Tue, 12 Dec 2017 20:10:42 +0100
Subject: [PATCH] Remove aggregation validation and use MapConfig validation

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

diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index 8df069c0..fdd70bcf 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -2,8 +2,6 @@ const AggregationProxy = require('../../aggregation/aggregation-proxy');
 const { MapConfig } = require('windshaft').model;
 const queryUtils = require('../../../utils/query-utils');
 
-const MISSING_AGGREGATION_COLUMNS = 'Missing columns in the aggregation. The map-config defines cartocss expressions,'+
-' interactivity fields or attributes that are not present in the aggregation';
 const unsupportedGeometryTypeErrorMessage = ctx =>
 `Unsupported geometry type: ${ctx.geometryType}. Aggregation is available only for geometry type: ST_Point`;
 
@@ -16,24 +14,25 @@ module.exports = class AggregationMapConfigAdapter {
     }
 
     getMapConfig (user, requestMapConfig, params, context, callback) {
-        const mapConfig = new MapConfig(requestMapConfig);
-
         if (!this._isValidAggregationParam(params)) {
             return callback(new Error(invalidAggregationParamValueErrorMessage({ value: params.aggregation })));
         }
 
-        if (!this._shouldAdapt(mapConfig, params)) {
-            return callback(null, requestMapConfig);
-        }
+        const mapConfig = new MapConfig(requestMapConfig);
 
-        if (this._hasMissingColumns(mapConfig)) {
-            const error = new Error(MISSING_AGGREGATION_COLUMNS);
+        try {
+            mapConfig.validateAggregation();
+        } catch (error) {
             error.http_status = 400;
             error.type = 'mapconfig';
 
             return callback(error);
         }
 
+        if (!this._shouldAdapt(mapConfig, params)) {
+            return callback(null, requestMapConfig);
+        }
+
         this.pgConnection.getConnection(user, (err, connection) => {
             if (err) {
                 return callback(err);
@@ -48,40 +47,6 @@ module.exports = class AggregationMapConfigAdapter {
         return aggregation === undefined || aggregation === 'true' || aggregation === 'false';
     }
 
-    _hasMissingColumns (mapConfig) {
-        const layers = mapConfig.getLayers();
-
-        for (let index = 0; index < layers.length; index++) {
-            const { aggregation } = layers[index].options;
-            const aggregationColumns = this._getAggregationColumns(aggregation);
-            const layerColumns = mapConfig.getColumnsByLayer(index);
-
-            if (layerColumns.length === 0) {
-                continue;
-            }
-
-            if (aggregationColumns.length === 0) {
-                return true;
-            }
-
-            if (!this._haveSameColumns(aggregationColumns, layerColumns)) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    _haveSameColumns (aggregationColumns, layerColumns) {
-        if (aggregationColumns.length !== layerColumns.length) {
-            return false;
-        }
-
-        const diff = aggregationColumns.filter(column => !layerColumns.includes(column));
-
-        return !diff.length;
-    }
-
     _shouldAdapt (mapConfig, params) {
         const { aggregation } = params;
 
@@ -194,12 +159,4 @@ module.exports = class AggregationMapConfigAdapter {
 
         return { png: true, mvt: true };
     }
-
-    _getAggregationColumns (aggregation) {
-        const hasAggregationColumns = aggregation !== undefined &&
-            typeof aggregation !== 'boolean' &&
-            typeof aggregation.columns === 'object';
-
-        return hasAggregationColumns ? Object.keys(aggregation.columns) : [];
-    }
 };