From eccc3597aa68113cf9d69c24caac7f63af8a8f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 17 Jul 2017 19:43:59 +0200 Subject: [PATCH 1/5] Respond with 204 when vector tile is empty --- lib/cartodb/controllers/base.js | 4 +++ test/acceptance/mvt.js | 58 +++++++++++++++++++++++++++++++++ test/support/test-client.js | 16 ++++++--- 3 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 test/acceptance/mvt.js diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index ab6587c2..7cdd0713 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -201,6 +201,10 @@ BaseController.prototype.sendError = function(req, res, err, label) { var statusCode = findStatusCode(err); + if (err.message === 'Tile does not exist' && req.params.format === 'mvt') { + statusCode = 204; + } + debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); // If a callback was requested, force status to 200 diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js new file mode 100644 index 00000000..17653d1d --- /dev/null +++ b/test/acceptance/mvt.js @@ -0,0 +1,58 @@ +require('../support/test_helper'); + +var assert = require('../support/assert'); +var TestClient = require('../support/test-client'); + +function createMapConfig (sql) { + sql = sql || [ + 'select', + ' *', + 'from', + ' populated_places_simple_reduced', + ].join('\n'); + + return { + version: '1.6.0', + layers: [{ + type: "cartodb", + options: { + sql: sql, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0', + interactivity: 'cartodb_id' + } + }] + }; +} + + +describe('mvt', function () { + const testCases = [ + { + desc: 'should get empty mvt with code 204 (no content)', + coords: { z: 0, x: 0, y: 0 }, + format: 'mvt', + mapConfig: createMapConfig('select 1 as cartodb_id, null::geometry as the_geom_webmercator') + } + ]; + + testCases.forEach(function (test) { + it(test.desc, done => { + const testClient = new TestClient(test.mapConfig, 1234); + const { z, x, y } = test.coords; + const options = { + format: test.format, + status: 204 + }; + + testClient.getTile(z, x, y, options, (err, res) => { + assert.ifError(err); + + assert.ifError(err); + assert.equal(res.statusCode, 204); + assert.equal(res.body, ''); + testClient.drain(done); + }); + }); + }); +}); \ No newline at end of file diff --git a/test/support/test-client.js b/test/support/test-client.js index 659d6686..72a6eca9 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -525,7 +525,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { }; var expectedResponse = { - status: 200, + status: params.status || 200, headers: { 'Content-Type': 'application/json; charset=utf-8' } @@ -542,7 +542,12 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { if (isMvt) { request.encoding = 'binary'; - expectedResponse.headers['Content-Type'] = 'application/x-protobuf'; + + if (expectedResponse.status === 200) { + expectedResponse.headers['Content-Type'] = 'application/x-protobuf'; + } else if (expectedResponse.status === 204) { + expectedResponse.headers['Content-Type'] = undefined; + } } var isGeojson = format.match(/geojson$/); @@ -561,15 +566,16 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { assert.response(server, request, expectedResponse, function(res, err) { assert.ifError(err); - var obj; if (isPng) { obj = mapnik.Image.fromBytes(new Buffer(res.body, 'binary')); } else if (isMvt) { - obj = new mapnik.VectorTile(z, x, y); - obj.setDataSync(new Buffer(res.body, 'binary')); + if (res.body) { + obj = new mapnik.VectorTile(z, x, y); + obj.setDataSync(new Buffer(res.body, 'binary')); + } } else { obj = JSON.parse(res.body); From ff1399625568432c2da6eb4e5a54ca42ddb53006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 18 Jul 2017 10:44:27 +0200 Subject: [PATCH 2/5] Add test to check that mvt returns 200 when tile has data --- test/acceptance/mvt.js | 34 +++++++++++++++------------------- test/support/test-client.js | 5 +++++ 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index 17653d1d..db75bb16 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -1,16 +1,9 @@ require('../support/test_helper'); -var assert = require('../support/assert'); -var TestClient = require('../support/test-client'); - -function createMapConfig (sql) { - sql = sql || [ - 'select', - ' *', - 'from', - ' populated_places_simple_reduced', - ].join('\n'); +const assert = require('../support/assert'); +const TestClient = require('../support/test-client'); +function createMapConfig (sql = TestClient.SQL.ONE_POINT) { return { version: '1.6.0', layers: [{ @@ -25,14 +18,21 @@ function createMapConfig (sql) { }; } - describe('mvt', function () { const testCases = [ { desc: 'should get empty mvt with code 204 (no content)', coords: { z: 0, x: 0, y: 0 }, format: 'mvt', - mapConfig: createMapConfig('select 1 as cartodb_id, null::geometry as the_geom_webmercator') + status: 204, + mapConfig: createMapConfig(TestClient.SQL.EMPTY) + }, + { + desc: 'should get mvt tile with code 200 (ok)', + coords: { z: 0, x: 0, y: 0 }, + format: 'mvt', + status: 200, + mapConfig: createMapConfig() } ]; @@ -40,17 +40,13 @@ describe('mvt', function () { it(test.desc, done => { const testClient = new TestClient(test.mapConfig, 1234); const { z, x, y } = test.coords; - const options = { - format: test.format, - status: 204 - }; + const { format, status } = test; - testClient.getTile(z, x, y, options, (err, res) => { + testClient.getTile(z, x, y, { format, status }, (err, res) => { assert.ifError(err); assert.ifError(err); - assert.equal(res.statusCode, 204); - assert.equal(res.body, ''); + assert.equal(res.statusCode, test.status); testClient.drain(done); }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index 72a6eca9..5d0eddff 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -75,6 +75,11 @@ module.exports.CARTOCSS = { ].join('\n') }; +module.exports.SQL = { + EMPTY: 'select 1 as cartodb_id, null::geometry as the_geom_webmercator', + ONE_POINT: 'select 1 as cartodb_id, \'SRID=3857;POINT(0 0)\'::geometry the_geom_webmercator' +} + TestClient.prototype.getWidget = function(widgetName, params, callback) { var self = this; From 0aab434f13fb019e9c23059309fa9941afa6c39f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 18 Jul 2017 10:52:24 +0200 Subject: [PATCH 3/5] Remove duplicated assertion --- test/acceptance/mvt.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index db75bb16..6510541b 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -45,7 +45,6 @@ describe('mvt', function () { testClient.getTile(z, x, y, { format, status }, (err, res) => { assert.ifError(err); - assert.ifError(err); assert.equal(res.statusCode, test.status); testClient.drain(done); }); From 446e2d08020fa51c1077a137b3c3bd6e9d94bf9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 18 Jul 2017 11:05:45 +0200 Subject: [PATCH 4/5] Add empty line at the end of file --- test/acceptance/mvt.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index 6510541b..b1ea4455 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -50,4 +50,4 @@ describe('mvt', function () { }); }); }); -}); \ No newline at end of file +}); From f306c26da61196b7903f94c4ef93a04a6a7fb69e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 18 Jul 2017 11:08:39 +0200 Subject: [PATCH 5/5] Update News --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index dd6b97f4..fbce5c7d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,8 @@ ## 3.9.7 Released 2017-mm-dd + - Respond with 204 (No content) when vector tile has no data #712 + ## 3.9.6 Released 2017-07-11