diff --git a/NEWS.md b/NEWS.md index 454b9142..fbce5c7d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,8 +1,16 @@ # Changelog -## 3.9.6 +## 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 + + - Dataviews: support for aggregation in search results #708 + ## 3.9.5 Released 2017-06-27 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/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index 0d576614..77f37723 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -321,6 +321,8 @@ Aggregation.prototype.search = function(psql, userQuery, callback) { var self = this; var _userQuery = psql.escapeLiteral('%' + userQuery + '%'); + var _value = this.aggregation !== 'count' && this.aggregationColumn ? + this.aggregation + '(' + this.aggregationColumn + ')' : 'count(1)'; // TODO unfiltered will be wrong as filters are already applied at this point var query = searchQueryTpl({ @@ -333,7 +335,7 @@ Aggregation.prototype.search = function(psql, userQuery, callback) { _searchFiltered: filterCategoriesQueryTpl({ _query: this.query, _column: this.column, - _value: 'count(1)', + _value: _value, _userQuery: _userQuery }) }); diff --git a/package.json b/package.json index dac5756a..e3239be5 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "3.9.6", + "version": "3.9.7", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" @@ -16,7 +16,8 @@ "contributors": [ "Simon Tokumine ", "Javi Santana ", - "Sandro Santilli " + "Sandro Santilli ", + "Carlos Matallín " ], "dependencies": { "body-parser": "~1.14.0", diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 697004e8..d8d03177 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -145,6 +145,35 @@ describe('aggregations happy cases', function() { }); }); }); + + var widgetSearchExpects = { + 'count': [ { category: 'other_a', value: 3 } ], + 'sum': [ { category: 'other_a', value: 6 } ], + 'avg': [ { category: 'other_a', value: 2 } ], + 'max': [ { category: 'other_a', value: 3 } ], + 'min': [ { category: 'other_a', value: 1 } ] + }; + + Object.keys(operations_and_values).forEach(function (operation) { + var description = 'should search OTHER category using "' + operation + '"'; + + it(description, function (done) { + this.testClient = new TestClient(aggregationOperationMapConfig(operation, query_other, 'cat', 'val')); + this.testClient.widgetSearch('cat', 'other_a', function (err, res, searchResult) { + assert.ifError(err); + + assert.ok(searchResult); + assert.equal(searchResult.type, 'aggregation'); + + assert.equal(searchResult.categories.length, 1); + assert.deepEqual( + searchResult.categories, + widgetSearchExpects[operation] + ); + done(); + }); + }); + }); }); describe('aggregation-dataview: special float values', function() { diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js new file mode 100644 index 00000000..b1ea4455 --- /dev/null +++ b/test/acceptance/mvt.js @@ -0,0 +1,53 @@ +require('../support/test_helper'); + +const assert = require('../support/assert'); +const TestClient = require('../support/test-client'); + +function createMapConfig (sql = TestClient.SQL.ONE_POINT) { + 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', + 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() + } + ]; + + testCases.forEach(function (test) { + it(test.desc, done => { + const testClient = new TestClient(test.mapConfig, 1234); + const { z, x, y } = test.coords; + const { format, status } = test; + + testClient.getTile(z, x, y, { format, status }, (err, res) => { + assert.ifError(err); + + assert.equal(res.statusCode, test.status); + testClient.drain(done); + }); + }); + }); +}); diff --git a/test/acceptance/widgets/ported/aggregation.js b/test/acceptance/widgets/ported/aggregation.js index 9329f039..62c998a5 100644 --- a/test/acceptance/widgets/ported/aggregation.js +++ b/test/acceptance/widgets/ported/aggregation.js @@ -322,6 +322,25 @@ describe('widgets', function() { }); }); }); + + [adm0name].forEach(function(userQuery) { + it('should search with sum aggregation: ' + userQuery, function(done) { + this.testClient = new TestClient(aggregationSumMapConfig); + this.testClient.widgetSearch('adm0name', userQuery, function (err, res, searchResult) { + assert.ok(!err, err); + assert.ok(searchResult); + assert.equal(searchResult.type, 'aggregation'); + + assert.equal(searchResult.categories.length, 1); + assert.deepEqual( + searchResult.categories, + [{ category:"Argentina", value:28015640 }] + ); + + done(); + }); + }); + }); }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index d499e9d5..dbaf466d 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; @@ -525,7 +530,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 +547,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 +571,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);