From 1c7da2c4b3749e822740419bd0becca2df5833fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 3 Jan 2018 12:00:25 +0100 Subject: [PATCH] Going green: do not fail when map-config is vector-only and a layer doesn't have points --- .../adapter/aggregation-mapconfig-adapter.js | 6 +- test/acceptance/aggregation.js | 87 +++++++++++++++++-- 2 files changed, 84 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 28cbffc9..6aa0eaf2 100644 --- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js @@ -132,7 +132,7 @@ module.exports = class AggregationMapConfigAdapter { const result = res.rows[0] || {}; - if (!AggregationMapConfig.supportsGeometryType(result.type)) { + if (!mapConfig.isVectorOnlyMapConfig() && !AggregationMapConfig.supportsGeometryType(result.type)) { const message = unsupportedGeometryTypeErrorMessage({ geometryType: result.type }); const error = new Error(message); error.type = 'layer'; @@ -145,6 +145,10 @@ module.exports = class AggregationMapConfigAdapter { return callback(error); } + if (mapConfig.isVectorOnlyMapConfig() && !AggregationMapConfig.supportsGeometryType(result.type)) { + return callback(null, false); + } + if (!mapConfig.doesLayerReachThreshold(index, result.count)) { return callback(null, false); } diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index 0e2f9672..28b2c099 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -471,6 +471,8 @@ describe('aggregation', function () { type: 'cartodb', options: { sql: POLYGONS_SQL_1, + cartocss: '#layer { marker-width: [value]; }', + cartocss_version: '2.3.0', aggregation: { threshold: 1 } @@ -1233,7 +1235,9 @@ describe('aggregation', function () { }); }); - it('should skip aggregation w/o failing when is Vector Only MapConfig and layer has polygons', function (done) { + + it('should skip aggregation w/o failing when is Vector Only MapConfig and layer has polygons', + function (done) { this.mapConfig = createVectorMapConfig([ { type: 'cartodb', @@ -1244,23 +1248,90 @@ describe('aggregation', function () { ]); this.testClient = new TestClient(this.mapConfig); - const options = { - format: 'mvt' - }; - this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } - const tileJSON = tile.toJSON(); + assert.equal(typeof body.metadata, 'object'); + assert.ok(Array.isArray(body.metadata.layers)); - tileJSON[0].features.forEach(feature => assert.equal(typeof feature.properties.value, 'number')); + body.metadata.layers.forEach(layer => assert.ok(!layer.meta.aggregation.mvt)); + body.metadata.layers.forEach(layer => assert.ok(!layer.meta.aggregation.png)); - done(); + const options = { + format: 'mvt' + }; + + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(feature => { + assert.equal(typeof feature.properties.value, 'number'); + }); + + done(); + }); }); }); + it('should skip aggregation for polygons (w/o failing) and aggregate when the layer has points', + function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POLYGONS_SQL_1 + } + }, + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1 + } + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + assert.equal(typeof body.metadata, 'object'); + assert.ok(Array.isArray(body.metadata.layers)); + + assert.equal(body.metadata.layers[0].meta.aggregation.mvt, false); + assert.equal(body.metadata.layers[1].meta.aggregation.mvt, true); + + const options = { + format: 'mvt' + }; + + this.testClient.getTile(0, 0, 0, options, (err, res, tile) => { + if (err) { + return done(err); + } + + const tileJSON = tile.toJSON(); + + tileJSON[0].features.forEach(feature => { + assert.equal(typeof feature.properties.value, 'number'); + }); + + done(); + }); + }); + }); }); }); });