From 4a63fed943b76518fca84f8f4e0bd7970f636088 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 13 Dec 2017 12:35:17 +0100 Subject: [PATCH] Simplify Aggregation classes We're using the same aggregation queries for the Raster and Vector cases, so we don't need the class hierarchies used to handled them differently. AggregationProxy has been renamed to Aggregation --- .../models/aggregation/aggregation-proxy.js | 66 ------------------- ...tion-templates.js => aggregation-query.js} | 26 +++++++- lib/cartodb/models/aggregation/aggregation.js | 24 +++++++ .../models/aggregation/base-aggregation.js | 12 ---- .../models/aggregation/raster-aggregation.js | 15 ----- .../models/aggregation/vector-aggregation.js | 15 ----- .../adapter/aggregation-mapconfig-adapter.js | 6 +- 7 files changed, 51 insertions(+), 113 deletions(-) delete mode 100644 lib/cartodb/models/aggregation/aggregation-proxy.js rename lib/cartodb/models/aggregation/{aggregation-templates.js => aggregation-query.js} (87%) create mode 100644 lib/cartodb/models/aggregation/aggregation.js delete mode 100644 lib/cartodb/models/aggregation/base-aggregation.js delete mode 100644 lib/cartodb/models/aggregation/raster-aggregation.js delete mode 100644 lib/cartodb/models/aggregation/vector-aggregation.js diff --git a/lib/cartodb/models/aggregation/aggregation-proxy.js b/lib/cartodb/models/aggregation/aggregation-proxy.js deleted file mode 100644 index d5c64c18..00000000 --- a/lib/cartodb/models/aggregation/aggregation-proxy.js +++ /dev/null @@ -1,66 +0,0 @@ -const RasterAggregation = require('./raster-aggregation'); -const VectorAggregation = require('./vector-aggregation'); -const RASTER_AGGREGATION = 'RasterAggregation'; -const VECTOR_AGGREGATION = 'VectorAggregation'; - -module.exports = class AggregationProxy { - static get THRESHOLD() { - return 1e5; // 100K - } - - constructor (mapconfig, query, { - resolution = 256, - threshold = AggregationProxy.THRESHOLD, - placement = 'centroid', - columns = {} - } = {}) { - this.mapconfig = mapconfig; - this.query = query; - this.resolution = resolution; - this.threshold = threshold; - this.placement = placement; - this.columns = columns; - this.implementation = this._getAggregationImplementation(); - } - - _getAggregationImplementation () { - let implementation = null; - - switch (this._getAggregationType()) { - case VECTOR_AGGREGATION: - implementation = new VectorAggregation( - this.query, - this.resolution, - this.threshold, - this.placement, - this.columns - ); - break; - case RASTER_AGGREGATION: - implementation = new RasterAggregation( - this.query, - this.resolution, - this.threshold, - this.placement, - this.columns - ); - break; - default: - throw new Error('Unsupported aggregation type'); - } - - return implementation; - } - - _getAggregationType () { - if (this.mapconfig.isVectorOnlyMapConfig()) { - return VECTOR_AGGREGATION; - } - - return RASTER_AGGREGATION; - } - - sql () { - return this.implementation.sql(); - } -}; diff --git a/lib/cartodb/models/aggregation/aggregation-templates.js b/lib/cartodb/models/aggregation/aggregation-query.js similarity index 87% rename from lib/cartodb/models/aggregation/aggregation-templates.js rename to lib/cartodb/models/aggregation/aggregation-query.js index eb7d6019..7dd55b3c 100644 --- a/lib/cartodb/models/aggregation/aggregation-templates.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -1,7 +1,14 @@ /** - * Returns template function (function that accepts template parameters and returns a string) + * Returns a template function (function that accepts template parameters and returns a string) + * to generate an aggregation query. + * Valid options to define the query template are: + * - placement + * The query template parameters taken by the result template function are: + * - sourceQuery + * - res + * - columns */ -module.exports = (options) => { +const templateForOptions = (options) => { let templateFn = aggregationQueryTemplates[options.placement]; if (!templateFn) { throw new Error("Invalid Aggregation placement: '" + options.placement + "'"); @@ -9,6 +16,21 @@ module.exports = (options) => { return templateFn; }; +/** + * Generates an aggregation query given the aggregation options: + * - query + * - resolution + * - columns + * - placement + */ +const queryForOptions = (options) => templateForOptions(options)({ + sourceQuery: options.query, + res: options.resolution, + columns: options.columns +}); + +module.exports = queryForOptions; + const SUPPORTED_AGGREGATE_FUNCTIONS = { 'count': { sql: (column_name, params) => `count(${params.aggregated_column || '*'})` diff --git a/lib/cartodb/models/aggregation/aggregation.js b/lib/cartodb/models/aggregation/aggregation.js new file mode 100644 index 00000000..447740b4 --- /dev/null +++ b/lib/cartodb/models/aggregation/aggregation.js @@ -0,0 +1,24 @@ +const aggregationQuery = require('./aggregation-query'); + +module.exports = class Aggregation { + static get THRESHOLD() { + return 1e5; // 100K + } + + constructor (mapconfig, query, { + resolution = 256, + threshold = Aggregation.THRESHOLD, + placement = 'centroid', + columns = {} + } = {}) { + this.mapconfig = mapconfig; + this.query = query; + this.resolution = resolution; + this.threshold = threshold; + this.placement = placement; + this.columns = columns; + } + sql () { + return aggregationQuery(this); + } +}; diff --git a/lib/cartodb/models/aggregation/base-aggregation.js b/lib/cartodb/models/aggregation/base-aggregation.js deleted file mode 100644 index 0e54689d..00000000 --- a/lib/cartodb/models/aggregation/base-aggregation.js +++ /dev/null @@ -1,12 +0,0 @@ -module.exports = class BaseAggregation { - constructor(query, resolution, threshold, placement, columns) { - this.query = query; - this.resolution = resolution; - this.threshold = threshold; - this.placement = placement; - this.columns = columns; - } - sql () { - throw new Error('Unimplemented method'); - } -}; diff --git a/lib/cartodb/models/aggregation/raster-aggregation.js b/lib/cartodb/models/aggregation/raster-aggregation.js deleted file mode 100644 index 7314b805..00000000 --- a/lib/cartodb/models/aggregation/raster-aggregation.js +++ /dev/null @@ -1,15 +0,0 @@ -const BaseAggregation = require('./base-aggregation'); -const aggregationTemplate = require('./aggregation-templates'); - -module.exports = class RasterAggregation extends BaseAggregation { - constructor () { - super(...arguments); - } - sql () { - return aggregationTemplate(this)({ - sourceQuery: this.query, - res: this.resolution, - columns: this.columns - }); - } -}; diff --git a/lib/cartodb/models/aggregation/vector-aggregation.js b/lib/cartodb/models/aggregation/vector-aggregation.js deleted file mode 100644 index 8cd4c859..00000000 --- a/lib/cartodb/models/aggregation/vector-aggregation.js +++ /dev/null @@ -1,15 +0,0 @@ -const BaseAggregation = require('./base-aggregation'); -const aggregationTemplate = require('./aggregation-templates'); - -module.exports = class VectorAggregation extends BaseAggregation { - constructor () { - super(...arguments); - } - sql () { - return aggregationTemplate(this)({ - sourceQuery: this.query, - res: this.resolution, - columns: this.columns - }); - } -}; diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js index fdd70bcf..b92c0563 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 AggregationProxy = require('../../aggregation/aggregation-proxy'); +const Aggregation = require('../../aggregation/aggregation'); const { MapConfig } = require('windshaft').model; const queryUtils = require('../../../utils/query-utils'); @@ -93,7 +93,7 @@ module.exports = class AggregationMapConfigAdapter { } if (shouldAdapt) { - const aggregation = new AggregationProxy(mapConfig, layer.options.sql, layer.options.aggregation); + const aggregation = new Aggregation(mapConfig, layer.options.sql, layer.options.aggregation); const sqlQueryWrap = layer.options.sql_wrap; let aggregationSql = aggregation.sql(layer.options); @@ -130,7 +130,7 @@ module.exports = class AggregationMapConfigAdapter { const threshold = layer.options.aggregation && layer.options.aggregation.threshold ? layer.options.aggregation.threshold : - AggregationProxy.THRESHOLD; + Aggregation.THRESHOLD; if (estimatedFeatureCount < threshold) { return callback(null, shouldAdapt);