From 3e261fb3539c59c70b7fc79b2bdc9e96ed0f0863 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Tue, 23 Oct 2018 15:49:34 +0200 Subject: [PATCH 1/4] Going red: fails with 400 due to "Cannot read property 'geom_type' of undefined" --- .../stats/mapnik_stats_layergroup.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/acceptance/stats/mapnik_stats_layergroup.js b/test/acceptance/stats/mapnik_stats_layergroup.js index dd58fd85..2f4e0289 100644 --- a/test/acceptance/stats/mapnik_stats_layergroup.js +++ b/test/acceptance/stats/mapnik_stats_layergroup.js @@ -519,6 +519,31 @@ describe('Create mapnik layergroup', function() { }); }); + it(`should not fail "TypeError: ... 'geom_type' of undefined" for empty results`, function(done) { + var testClient = new TestClient({ + version: '1.8.0', + layers: [ + { + type: 'mapnik', + options: { + sql: 'select * from test_table where false', + metadata: { + geometryType: true + } + } + } + ] + }); + + testClient.getLayergroup(function(err, layergroup) { + assert.ifError(err); + assert.equal(layergroup.metadata.layers[0].id, mapnikBasicLayerId(0)); + assert.equal(layergroup.metadata.layers[0].meta.stats.estimatedFeatureCount, 0); + assert.equal(layergroup.metadata.layers[0].meta.stats.geometryType, undefined); + testClient.drain(done); + }); + }); + it('should provide a sample as optional metadata', function(done) { var testClient = new TestClient({ version: '1.4.0', From 26e4a052766af50efb57eca941bf8bdb1b44a12f Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Tue, 23 Oct 2018 15:50:42 +0200 Subject: [PATCH 2/4] Going green: prevent TypeError for empty tables/results This is the intial step to fix https://github.com/CartoDB/carto-vl/issues/1049. --- lib/cartodb/backends/layer-stats/mapnik-layer-stats.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js index 67d00d90..4de960fe 100644 --- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js +++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js @@ -71,7 +71,7 @@ function _geometryType(ctx) { const geometryColumn = AggregationMapConfig.getAggregationGeometryColumn(); const sqlQuery = _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn)); return queryUtils.queryPromise(ctx.dbConnection, sqlQuery) - .then(res => ({ geometryType: res.rows[0].geom_type })); + .then(res => ({ geometryType: (res.rows[0] || {}).geom_type })); } return Promise.resolve(); } From abd378e5f65986a0c9ef95b4ab50f8219e94c950 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Tue, 23 Oct 2018 15:47:48 +0000 Subject: [PATCH 3/4] Run tests based on PostGIS version --- .../stats/mapnik_stats_layergroup.js | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/acceptance/stats/mapnik_stats_layergroup.js b/test/acceptance/stats/mapnik_stats_layergroup.js index 2f4e0289..368125a6 100644 --- a/test/acceptance/stats/mapnik_stats_layergroup.js +++ b/test/acceptance/stats/mapnik_stats_layergroup.js @@ -2,14 +2,32 @@ require('../../support/test_helper'); var assert = require('../../support/assert'); var TestClient = require('../../support/test-client'); +const serverOptions = require('../../../lib/cartodb/server_options'); + +const suites = [{ + desc: 'mvt (mapnik)', + usePostGIS: false +}]; + +if (process.env.POSTGIS_VERSION >= '20400') { + suites.push({ + desc: 'mvt (postgis)', + usePostGIS: true + }); +} + +suites.forEach(({desc, usePostGIS}) => { +describe(`[${desc}] Create mapnik layergroup`, function() { + const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; -describe('Create mapnik layergroup', function() { before(function() { + serverOptions.renderer.mvt.usePostGIS = usePostGIS; this.layerStatsConfig = global.environment.enabledFeatures.layerStats; global.environment.enabledFeatures.layerStats = true; }); after(function() { + serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; global.environment.enabledFeatures.layerStats = this.layerStatsConfig; }); @@ -614,3 +632,4 @@ describe('Create mapnik layergroup', function() { }); }); +}); From b13ae62d0f032cbb78ed46becf3db0b8f1a8132e Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Tue, 23 Oct 2018 16:02:37 +0000 Subject: [PATCH 4/4] Do not assert the estimated count as it seems to change over pg versions --- test/acceptance/stats/mapnik_stats_layergroup.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/acceptance/stats/mapnik_stats_layergroup.js b/test/acceptance/stats/mapnik_stats_layergroup.js index 368125a6..189b42e9 100644 --- a/test/acceptance/stats/mapnik_stats_layergroup.js +++ b/test/acceptance/stats/mapnik_stats_layergroup.js @@ -556,7 +556,6 @@ describe(`[${desc}] Create mapnik layergroup`, function() { testClient.getLayergroup(function(err, layergroup) { assert.ifError(err); assert.equal(layergroup.metadata.layers[0].id, mapnikBasicLayerId(0)); - assert.equal(layergroup.metadata.layers[0].meta.stats.estimatedFeatureCount, 0); assert.equal(layergroup.metadata.layers[0].meta.stats.geometryType, undefined); testClient.drain(done); });