From 0940158d015d35760840750e3be370a2dab788fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 2 Dec 2019 16:17:55 +0100 Subject: [PATCH 01/11] Implemented tests, happy cases --- test/acceptance/dataviews/filters-test.js | 137 ++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 test/acceptance/dataviews/filters-test.js diff --git a/test/acceptance/dataviews/filters-test.js b/test/acceptance/dataviews/filters-test.js new file mode 100644 index 00000000..8e4743e5 --- /dev/null +++ b/test/acceptance/dataviews/filters-test.js @@ -0,0 +1,137 @@ +'use strict'; + +require('../../support/test-helper'); + +const assert = require('../../support/assert'); +const TestClient = require('../../support/test-client'); + +describe('circle filter', function () { + const mapConfig = { + version: '1.8.0', + layers: [ + { + type: 'cartodb', + options: { + source: { + id: 'a0' + }, + cartocss: '#points { marker-width: 10; marker-fill: red; }', + cartocss_version: '2.3.0' + } + } + ], + dataviews: { + categories: { + source: { + id: 'a0' + }, + type: 'aggregation', + options: { + column: 'cat', + aggregation: 'sum', + aggregationColumn: 'val' + } + } + }, + analyses: [ + { + id: 'a0', + type: 'source', + params: { + query: ` + SELECT + ST_SetSRID(ST_MakePoint(x, x), 4326) as the_geom, + ST_Transform(ST_SetSRID(ST_MakePoint(x, x), 4326), 3857) as the_geom_webmercator, + CASE + WHEN x % 4 = 0 THEN 1 + WHEN x % 4 = 1 THEN 2 + WHEN x % 4 = 2 THEN 3 + ELSE 4 + END AS val, + CASE + WHEN x % 4 = 0 THEN 'category_1' + WHEN x % 4 = 1 THEN 'category_2' + WHEN x % 4 = 2 THEN 'category_3' + ELSE 'category_4' + END AS cat + FROM generate_series(-10, 10) x + ` + } + } + ] + }; + + beforeEach(function (done) { + const apikey = 1234; + this.testClient = new TestClient(mapConfig, apikey); + done(); + }); + + afterEach(function (done) { + if (this.testClient) { + this.testClient.drain(done); + } else { + done(); + } + }); + + const scenarios = [ + { + params: { + }, + expected: { + type: 'aggregation', + aggregation: 'sum', + count: 21, + nulls: 0, + nans: 0, + infinities: 0, + min: 5, + max: 40, + categoriesCount: 4, + categories: [ + { category: 'category_4', value: 40, agg: false }, + { category: 'category_3', value: 9, agg: false }, + { category: 'category_2', value: 6, agg: false }, + { category: 'category_1', value: 5, agg: false } + ] + } + }, + { + params: { + circle: { + lat: 0, + lng: 0, + radius: 5000 + } + }, + expected: { + type: 'aggregation', + aggregation: 'sum', + count: 21, + nulls: 0, + nans: 0, + infinities: 0, + min: 5, + max: 40, + categoriesCount: 4, + categories: [ + { category: 'category_4', value: 40, agg: false }, + { category: 'category_3', value: 9, agg: false }, + { category: 'category_2', value: 6, agg: false }, + { category: 'category_1', value: 5, agg: false } + ] + } + } + ]; + + scenarios.forEach(function (scenario) { + it(`should get aggregation dataview with params: ${JSON.stringify(scenario.params)}`, function (done) { + this.testClient.getDataview('categories', scenario.params, (err, dataview) => { + assert.ifError(err); + assert.deepStrictEqual(dataview, scenario.expected); + done(); + }); + }); + }); +}); From 17f151cd5a136d1f51de2af918d7486409d179ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 2 Dec 2019 18:36:41 +0100 Subject: [PATCH 02/11] Implement circle filter for dataviews --- lib/api/map/dataview-layergroup-controller.js | 1 + lib/backends/dataview.js | 7 +++ lib/models/filter/circle.js | 58 +++++++++++++++++++ test/acceptance/dataviews/filters-test.js | 20 +++---- test/support/test-client.js | 2 +- 5 files changed, 75 insertions(+), 13 deletions(-) create mode 100644 lib/models/filter/circle.js diff --git a/lib/api/map/dataview-layergroup-controller.js b/lib/api/map/dataview-layergroup-controller.js index a10fbc25..446bf8a3 100644 --- a/lib/api/map/dataview-layergroup-controller.js +++ b/lib/api/map/dataview-layergroup-controller.js @@ -18,6 +18,7 @@ const ALLOWED_DATAVIEW_QUERY_PARAMS = [ 'own_filter', // 0, 1 'no_filters', // 0, 1 'bbox', // w,s,e,n + 'circle', // json 'start', // number 'end', // number 'column_type', // string diff --git a/lib/backends/dataview.js b/lib/backends/dataview.js index 080718e1..555f8e79 100644 --- a/lib/backends/dataview.js +++ b/lib/backends/dataview.js @@ -3,6 +3,7 @@ var _ = require('underscore'); var PSQL = require('cartodb-psql'); var BBoxFilter = require('../models/filter/bbox'); +const CircleFilter = require('../models/filter/circle'); var DataviewFactory = require('../models/dataview/factory'); var DataviewFactoryWithOverviews = require('../models/dataview/overviews/factory'); const dbParamsFromReqParams = require('../utils/database-params'); @@ -86,6 +87,9 @@ function getQueryWithFilters (dataviewDefinition, params) { if (params.bbox) { var bboxFilter = new BBoxFilter({ column: 'the_geom_webmercator', srid: 3857 }, { bbox: params.bbox }); query = bboxFilter.sql(query); + } else if (params.circle) { + const circleFilter = new CircleFilter({ column: 'the_geom_webmercator', srid: 3857 }, { circle: params.circle }); + query = circleFilter.sql(query); } return query; @@ -197,6 +201,9 @@ function getQueryWithOwnFilters (dataviewDefinition, params) { if (params.bbox) { var bboxFilter = new BBoxFilter({ column: 'the_geom', srid: 4326 }, { bbox: params.bbox }); query = bboxFilter.sql(query); + } else if (params.circle) { + const circleFilter = new CircleFilter({ column: 'the_geom', srid: 4326 }, { circle: params.circle }); + query = circleFilter.sql(query); } return query; diff --git a/lib/models/filter/circle.js b/lib/models/filter/circle.js new file mode 100644 index 00000000..5dcd35db --- /dev/null +++ b/lib/models/filter/circle.js @@ -0,0 +1,58 @@ +'use strict'; + +const debug = require('debug')('windshaft:filter:circle'); +function filterQueryTpl ({ sql, column, srid, lng, lat, radius } = {}) { + return ` + SELECT + * + FROM (${sql}) _cdb_circle_filter + WHERE + ST_Intersects( + ${column}, + ST_Buffer( + ST_Transform( + ST_SetSRID(ST_Point(${lng},${lat}), 4326), + ${srid} + ), + ${radius} + ) + ) + `; +} + +module.exports = class CircleFilter { + constructor (filterDefinition, filterParams) { + const { circle } = filterParams; + + if (!circle) { + throw new Error('Circle filter expects to have a "circle" param'); + } + + const { lng, lat, radius } = JSON.parse(circle); + + if (!Number.isFinite(lng) || !Number.isFinite(lat) || !Number.isFinite(radius)) { + throw new Error('Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"'); + } + + this.column = filterDefinition.column || 'the_geom_webmercator'; + this.srid = filterDefinition.srid || 3857; + this.lng = lng; + this.lat = lat; + this.radius = radius; + } + + sql (rawSql) { + const circleSql = filterQueryTpl({ + sql: rawSql, + column: this.column, + srid: this.srid, + lng: this.lng, + lat: this.lat, + radius: this.radius + }); + + debug(circleSql); + + return circleSql; + } +}; diff --git a/test/acceptance/dataviews/filters-test.js b/test/acceptance/dataviews/filters-test.js index 8e4743e5..dbb07a0a 100644 --- a/test/acceptance/dataviews/filters-test.js +++ b/test/acceptance/dataviews/filters-test.js @@ -77,8 +77,7 @@ describe('circle filter', function () { const scenarios = [ { - params: { - }, + params: JSON.stringify({}), expected: { type: 'aggregation', aggregation: 'sum', @@ -99,27 +98,24 @@ describe('circle filter', function () { }, { params: { - circle: { + circle: JSON.stringify({ lat: 0, lng: 0, radius: 5000 - } + }) }, expected: { type: 'aggregation', aggregation: 'sum', - count: 21, + count: 1, nulls: 0, nans: 0, infinities: 0, - min: 5, - max: 40, - categoriesCount: 4, + min: 1, + max: 1, + categoriesCount: 1, categories: [ - { category: 'category_4', value: 40, agg: false }, - { category: 'category_3', value: 9, agg: false }, - { category: 'category_2', value: 6, agg: false }, - { category: 'category_1', value: 5, agg: false } + { category: 'category_1', value: 1, agg: false } ] } } diff --git a/test/support/test-client.js b/test/support/test-client.js index 634509c0..b09fb552 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -485,7 +485,7 @@ TestClient.prototype.getDataview = function (dataviewName, params, callback) { urlParams.own_filter = params.own_filter; } - ['bbox', 'bins', 'start', 'end', 'aggregation', 'offset', 'categories'].forEach(function (extraParam) { + ['bbox', 'circle', 'bins', 'start', 'end', 'aggregation', 'offset', 'categories'].forEach(function (extraParam) { if (Object.prototype.hasOwnProperty.call(params, extraParam)) { urlParams[extraParam] = params[extraParam]; } From caf09ac644b9291fe92d27418b3635ed815475f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 3 Dec 2019 10:02:51 +0100 Subject: [PATCH 03/11] Rename file --- .../dataviews/{filters-test.js => circle-filter-test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/acceptance/dataviews/{filters-test.js => circle-filter-test.js} (100%) diff --git a/test/acceptance/dataviews/filters-test.js b/test/acceptance/dataviews/circle-filter-test.js similarity index 100% rename from test/acceptance/dataviews/filters-test.js rename to test/acceptance/dataviews/circle-filter-test.js From c877d0b96406006bcc11d79a8c5bff7cca0c54ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 3 Dec 2019 10:58:55 +0100 Subject: [PATCH 04/11] Implement polygon filter --- lib/api/map/dataview-layergroup-controller.js | 1 + lib/backends/dataview.js | 7 +++ lib/models/filter/polygon.js | 63 +++++++++++++++++++ ...filter-test.js => spatial-filters-test.js} | 45 ++++++++++++- test/support/test-client.js | 2 +- 5 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 lib/models/filter/polygon.js rename test/acceptance/dataviews/{circle-filter-test.js => spatial-filters-test.js} (73%) diff --git a/lib/api/map/dataview-layergroup-controller.js b/lib/api/map/dataview-layergroup-controller.js index 446bf8a3..8f849565 100644 --- a/lib/api/map/dataview-layergroup-controller.js +++ b/lib/api/map/dataview-layergroup-controller.js @@ -19,6 +19,7 @@ const ALLOWED_DATAVIEW_QUERY_PARAMS = [ 'no_filters', // 0, 1 'bbox', // w,s,e,n 'circle', // json + 'polygon', // json 'start', // number 'end', // number 'column_type', // string diff --git a/lib/backends/dataview.js b/lib/backends/dataview.js index 555f8e79..f3d05f73 100644 --- a/lib/backends/dataview.js +++ b/lib/backends/dataview.js @@ -4,6 +4,7 @@ var _ = require('underscore'); var PSQL = require('cartodb-psql'); var BBoxFilter = require('../models/filter/bbox'); const CircleFilter = require('../models/filter/circle'); +const PolygonFilter = require('../models/filter/polygon'); var DataviewFactory = require('../models/dataview/factory'); var DataviewFactoryWithOverviews = require('../models/dataview/overviews/factory'); const dbParamsFromReqParams = require('../utils/database-params'); @@ -90,6 +91,9 @@ function getQueryWithFilters (dataviewDefinition, params) { } else if (params.circle) { const circleFilter = new CircleFilter({ column: 'the_geom_webmercator', srid: 3857 }, { circle: params.circle }); query = circleFilter.sql(query); + } else if (params.polygon) { + const polygonFilter = new PolygonFilter({ column: 'the_geom_webmercator', srid: 3857 }, { polygon: params.polygon }); + query = polygonFilter.sql(query); } return query; @@ -204,6 +208,9 @@ function getQueryWithOwnFilters (dataviewDefinition, params) { } else if (params.circle) { const circleFilter = new CircleFilter({ column: 'the_geom', srid: 4326 }, { circle: params.circle }); query = circleFilter.sql(query); + } else if (params.polygon) { + const polygonFilter = new PolygonFilter({ column: 'the_geom', srid: 4326 }, { polygon: params.polygon }); + query = polygonFilter.sql(query); } return query; diff --git a/lib/models/filter/polygon.js b/lib/models/filter/polygon.js new file mode 100644 index 00000000..1dbdcc83 --- /dev/null +++ b/lib/models/filter/polygon.js @@ -0,0 +1,63 @@ +'use strict'; + +const assert = require('assert'); +const debug = require('debug')('windshaft:filter:polygon'); +function filterQueryTpl ({ sql, column, srid, geojson } = {}) { + return ` + SELECT + * + FROM (${sql}) _cdb_polygon_filter + WHERE + ST_Intersects( + ${column}, + ST_Transform( + ST_SetSRID(ST_GeomFromGeoJSON('${JSON.stringify(geojson)}'), 4326), + ${srid} + ) + ) + `; +} + +module.exports = class PolygonFilter { + constructor (filterDefinition, filterParams) { + const { polygon } = filterParams; + + if (!polygon) { + throw new Error('Polygon filter expects to have a "polygon" param'); + } + + const geojson = JSON.parse(polygon); + + if (geojson.type !== 'Polygon') { + throw new Error('Invalid type of geometry. Valid ones: "Polygon"'); + } + + if (!Array.isArray(geojson.coordinates)) { + throw new Error('Invalid geometry, it must be an array of coordinates (long/lat)'); + } + + try { + const length = geojson.coordinates.length; + assert.deepStrictEqual(geojson.coordinates[0], geojson.coordinates[length - 1]); + } catch (error) { + throw new Error('Invalid geometry, it must be a closed polygon'); + } + + this.column = filterDefinition.column || 'the_geom_webmercator'; + this.srid = filterDefinition.srid || 3857; + this.geojson = geojson; + } + + sql (rawSql) { + const polygonSql = filterQueryTpl({ + sql: rawSql, + column: this.column, + srid: this.srid, + geojson: this.geojson + }); + + debug(polygonSql); + + return polygonSql; + } +}; diff --git a/test/acceptance/dataviews/circle-filter-test.js b/test/acceptance/dataviews/spatial-filters-test.js similarity index 73% rename from test/acceptance/dataviews/circle-filter-test.js rename to test/acceptance/dataviews/spatial-filters-test.js index dbb07a0a..26168b13 100644 --- a/test/acceptance/dataviews/circle-filter-test.js +++ b/test/acceptance/dataviews/spatial-filters-test.js @@ -5,7 +5,7 @@ require('../../support/test-helper'); const assert = require('../../support/assert'); const TestClient = require('../../support/test-client'); -describe('circle filter', function () { +describe('spatial filters', function () { const mapConfig = { version: '1.8.0', layers: [ @@ -118,6 +118,49 @@ describe('circle filter', function () { { category: 'category_1', value: 1, agg: false } ] } + }, + { + params: { + polygon: JSON.stringify({ + type: 'Polygon', + coordinates: [ + [ + [ + -9.312286, + 37.907367 + ], + [ + 11.969604, + 6.487254 + ], + [ + -32.217407, + 6.957781 + ], + [ + -9.312286, + 37.907367 + ] + ] + ] + }) + }, + expected: { + type: 'aggregation', + aggregation: 'sum', + count: 3, + nulls: 0, + nans: 0, + infinities: 0, + min: 1, + max: 4, + categoriesCount: 3, + categories: [ + { category: 'category_4', value: 4, agg: false }, + { category: 'category_2', value: 2, agg: false }, + { category: 'category_1', value: 1, agg: false } + ] + } } ]; diff --git a/test/support/test-client.js b/test/support/test-client.js index b09fb552..6f465295 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -485,7 +485,7 @@ TestClient.prototype.getDataview = function (dataviewName, params, callback) { urlParams.own_filter = params.own_filter; } - ['bbox', 'circle', 'bins', 'start', 'end', 'aggregation', 'offset', 'categories'].forEach(function (extraParam) { + ['bbox', 'circle', 'polygon', 'bins', 'start', 'end', 'aggregation', 'offset', 'categories'].forEach(function (extraParam) { if (Object.prototype.hasOwnProperty.call(params, extraParam)) { urlParams[extraParam] = params[extraParam]; } From ea1f43bec7f531bb2d79aac6d717fcdda92e866f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 11 Dec 2019 11:02:31 +0100 Subject: [PATCH 05/11] Fix query to make the proper transformations --- lib/models/filter/circle.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/models/filter/circle.js b/lib/models/filter/circle.js index 5dcd35db..bfc66aa1 100644 --- a/lib/models/filter/circle.js +++ b/lib/models/filter/circle.js @@ -9,12 +9,15 @@ function filterQueryTpl ({ sql, column, srid, lng, lat, radius } = {}) { WHERE ST_Intersects( ${column}, - ST_Buffer( - ST_Transform( - ST_SetSRID(ST_Point(${lng},${lat}), 4326), - ${srid} - ), - ${radius} + ST_Transform( + ST_Buffer( + ST_Transform( + ST_SetSRID(ST_Point(${lng},${lat}), ${srid}), + 4326 + )::geography, + ${radius} + )::geometry, + ${srid} ) ) `; From 95f66b8c4b41698ebfab2708f9c7c62a1f9b7491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 13 Dec 2019 12:36:13 +0100 Subject: [PATCH 06/11] Transform from 3857 --- lib/models/filter/circle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/filter/circle.js b/lib/models/filter/circle.js index bfc66aa1..329d313e 100644 --- a/lib/models/filter/circle.js +++ b/lib/models/filter/circle.js @@ -12,7 +12,7 @@ function filterQueryTpl ({ sql, column, srid, lng, lat, radius } = {}) { ST_Transform( ST_Buffer( ST_Transform( - ST_SetSRID(ST_Point(${lng},${lat}), ${srid}), + ST_SetSRID(ST_Point(${lng},${lat}), 3857), 4326 )::geography, ${radius} From 1829a634e947e733eec38f1ea1dfa3e31d818ff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 16 Dec 2019 09:28:02 +0100 Subject: [PATCH 07/11] Add formula dataview test --- .../dataviews/spatial-filters-test.js | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/test/acceptance/dataviews/spatial-filters-test.js b/test/acceptance/dataviews/spatial-filters-test.js index 26168b13..6ef98ed2 100644 --- a/test/acceptance/dataviews/spatial-filters-test.js +++ b/test/acceptance/dataviews/spatial-filters-test.js @@ -31,6 +31,16 @@ describe('spatial filters', function () { aggregation: 'sum', aggregationColumn: 'val' } + }, + counter: { + type: 'formula', + source: { + id: 'a0' + }, + options: { + column: 'val', + operation: 'count' + } } }, analyses: [ @@ -77,6 +87,7 @@ describe('spatial filters', function () { const scenarios = [ { + dataview: 'categories', params: JSON.stringify({}), expected: { type: 'aggregation', @@ -97,6 +108,7 @@ describe('spatial filters', function () { } }, { + dataview: 'categories', params: { circle: JSON.stringify({ lat: 0, @@ -120,6 +132,7 @@ describe('spatial filters', function () { } }, { + dataview: 'categories', params: { polygon: JSON.stringify({ type: 'Polygon', @@ -161,12 +174,28 @@ describe('spatial filters', function () { { category: 'category_1', value: 1, agg: false } ] } + }, + { + dataview: 'counter', + params: { + circle: JSON.stringify({ + lat: 0, + lng: 0, + radius: 50000 + }) + }, + expected: { + nulls: 0, + operation: 'count', + result: 1, + type: 'formula' + } } ]; scenarios.forEach(function (scenario) { it(`should get aggregation dataview with params: ${JSON.stringify(scenario.params)}`, function (done) { - this.testClient.getDataview('categories', scenario.params, (err, dataview) => { + this.testClient.getDataview(scenario.dataview, scenario.params, (err, dataview) => { assert.ifError(err); assert.deepStrictEqual(dataview, scenario.expected); done(); From da07d550d28a2f16e586a8f7e670b868fa527fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 16 Dec 2019 12:30:28 +0100 Subject: [PATCH 08/11] Use ST_DWithin() --- lib/models/filter/circle.js | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/models/filter/circle.js b/lib/models/filter/circle.js index 329d313e..e7908d09 100644 --- a/lib/models/filter/circle.js +++ b/lib/models/filter/circle.js @@ -1,24 +1,16 @@ 'use strict'; const debug = require('debug')('windshaft:filter:circle'); -function filterQueryTpl ({ sql, column, srid, lng, lat, radius } = {}) { +function filterQueryTpl ({ sql, column, srid, lng, lat, radiusInMeters } = {}) { return ` SELECT * FROM (${sql}) _cdb_circle_filter WHERE - ST_Intersects( - ${column}, - ST_Transform( - ST_Buffer( - ST_Transform( - ST_SetSRID(ST_Point(${lng},${lat}), 3857), - 4326 - )::geography, - ${radius} - )::geometry, - ${srid} - ) + ST_DWithin( + ${srid === 3857 ? `ST_Transform(${column}, 4326)::geography` : `${column}::geography`}, + ST_SetSRID(ST_Point(${lng}, ${lat}), 4326)::geography, + ${radiusInMeters} ) `; } @@ -51,7 +43,7 @@ module.exports = class CircleFilter { srid: this.srid, lng: this.lng, lat: this.lat, - radius: this.radius + radiusInMeters: this.radius }); debug(circleSql); From 6e455a1205aa4918c5afc9878e835f20e30cf8a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 16 Dec 2019 12:54:17 +0100 Subject: [PATCH 09/11] Better condition --- lib/models/filter/circle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/filter/circle.js b/lib/models/filter/circle.js index e7908d09..45b1b8a5 100644 --- a/lib/models/filter/circle.js +++ b/lib/models/filter/circle.js @@ -8,7 +8,7 @@ function filterQueryTpl ({ sql, column, srid, lng, lat, radiusInMeters } = {}) { FROM (${sql}) _cdb_circle_filter WHERE ST_DWithin( - ${srid === 3857 ? `ST_Transform(${column}, 4326)::geography` : `${column}::geography`}, + ${srid === 4326 ? `${column}::geography` : `ST_Transform(${column}, 4326)::geography`}, ST_SetSRID(ST_Point(${lng}, ${lat}), 4326)::geography, ${radiusInMeters} ) From 726e1a22681618ba7b389bb8d87779b84e1aeb53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 16 Dec 2019 16:12:57 +0100 Subject: [PATCH 10/11] Add test to validate parameters --- lib/models/filter/circle.js | 19 ++- lib/models/filter/polygon.js | 20 ++- .../dataviews/spatial-filters-test.js | 148 ++++++++++++++++++ 3 files changed, 178 insertions(+), 9 deletions(-) diff --git a/lib/models/filter/circle.js b/lib/models/filter/circle.js index 45b1b8a5..b09b6446 100644 --- a/lib/models/filter/circle.js +++ b/lib/models/filter/circle.js @@ -18,15 +18,28 @@ function filterQueryTpl ({ sql, column, srid, lng, lat, radiusInMeters } = {}) { module.exports = class CircleFilter { constructor (filterDefinition, filterParams) { const { circle } = filterParams; + let _circle; if (!circle) { - throw new Error('Circle filter expects to have a "circle" param'); + const error = new Error('Circle filter expects to have a "circle" param'); + error.type = 'filter'; + throw error; } - const { lng, lat, radius } = JSON.parse(circle); + try { + _circle = JSON.parse(circle); + } catch (err) { + const error = new Error('Invalid circle parameter. Expected a valid JSON'); + error.type = 'filter'; + throw error; + } + + const { lng, lat, radius } = _circle; if (!Number.isFinite(lng) || !Number.isFinite(lat) || !Number.isFinite(radius)) { - throw new Error('Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"'); + const error = new Error('Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"'); + error.type = 'filter'; + throw error; } this.column = filterDefinition.column || 'the_geom_webmercator'; diff --git a/lib/models/filter/polygon.js b/lib/models/filter/polygon.js index 1dbdcc83..357f00bf 100644 --- a/lib/models/filter/polygon.js +++ b/lib/models/filter/polygon.js @@ -23,17 +23,25 @@ module.exports = class PolygonFilter { const { polygon } = filterParams; if (!polygon) { - throw new Error('Polygon filter expects to have a "polygon" param'); + const error = new Error('Polygon filter expects to have a "polygon" param'); + error.type = 'filter'; + throw error; } - const geojson = JSON.parse(polygon); + let geojson; - if (geojson.type !== 'Polygon') { - throw new Error('Invalid type of geometry. Valid ones: "Polygon"'); + try { + geojson = JSON.parse(polygon); + } catch (err) { + const error = new Error('Invalid polygon parameter. Expected a valid GeoJSON'); + error.type = 'filter'; + throw error; } - if (!Array.isArray(geojson.coordinates)) { - throw new Error('Invalid geometry, it must be an array of coordinates (long/lat)'); + if (geojson.type !== 'Polygon') { + const error = new Error('Invalid type of geometry. Valid ones: "Polygon"'); + error.type = 'filter'; + throw error; } try { diff --git a/test/acceptance/dataviews/spatial-filters-test.js b/test/acceptance/dataviews/spatial-filters-test.js index 6ef98ed2..829829da 100644 --- a/test/acceptance/dataviews/spatial-filters-test.js +++ b/test/acceptance/dataviews/spatial-filters-test.js @@ -130,6 +130,91 @@ describe('spatial filters', function () { { category: 'category_1', value: 1, agg: false } ] } + }, { + dataview: 'categories', + params: { + circle: JSON.stringify({ + lng: 0, + radius: 5000 + }), + response: { + status: 400 + } + }, + expected: { + errors: [ + 'Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"' + ], + errors_with_context: [ + { + type: 'filter', + message: 'Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"' + } + ] + } + }, { + dataview: 'categories', + params: { + circle: JSON.stringify({ + lat: 0, + radius: 5000 + }), + response: { + status: 400 + } + }, + expected: { + errors: [ + 'Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"' + ], + errors_with_context: [ + { + type: 'filter', + message: 'Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"' + } + ] + } + }, { + dataview: 'categories', + params: { + circle: JSON.stringify({ + lng: 0, + lat: 0 + }), + response: { + status: 400 + } + }, + expected: { + errors: [ + 'Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"' + ], + errors_with_context: [ + { + type: 'filter', + message: 'Missing parameter for Circle Filter, expected: "lng", "lat", and "radius"' + } + ] + } + }, { + dataview: 'categories', + params: { + circle: 'wadus', + response: { + status: 400 + } + }, + expected: { + errors: [ + 'Invalid circle parameter. Expected a valid JSON' + ], + errors_with_context: [ + { + type: 'filter', + message: 'Invalid circle parameter. Expected a valid JSON' + } + ] + } }, { dataview: 'categories', @@ -174,6 +259,69 @@ describe('spatial filters', function () { { category: 'category_1', value: 1, agg: false } ] } + }, { + dataview: 'categories', + params: { + polygon: 'wadus', + response: { + status: 400 + } + }, + expected: { + errors: [ + 'Invalid polygon parameter. Expected a valid GeoJSON' + ], + errors_with_context: [ + { + type: 'filter', + message: 'Invalid polygon parameter. Expected a valid GeoJSON' + } + ] + } + }, { + dataview: 'categories', + params: { + polygon: JSON.stringify({ + type: 'Point', + coordinates: [30, 10] + }), + response: { + status: 400 + } + }, + expected: { + errors: [ + 'Invalid type of geometry. Valid ones: "Polygon"' + ], + errors_with_context: [ + { + type: 'filter', + message: 'Invalid type of geometry. Valid ones: "Polygon"' + } + ] + } + }, { + dataview: 'categories', + params: { + polygon: JSON.stringify({ + type: 'Polygon', + coordinates: [[[]]] + }), + response: { + status: 400 + } + }, + expected: { + errors: [ + 'Too few ordinates in GeoJSON' + ], + errors_with_context: [ + { + type: 'unknown', + message: 'Too few ordinates in GeoJSON' + } + ] + } }, { dataview: 'counter', From 7012e6a66ad7d24c3a028a28168e2858cb150dc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 20 Dec 2019 09:37:27 +0100 Subject: [PATCH 11/11] Update NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index d5ce679c..d3aae5fb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -18,6 +18,7 @@ Announcements: - Remove deprecated coverage tool istanbul, using nyc instead. - Removed unused dockerfiles - Use cartodb schema when using cartodb extension functions and tables. +- Implemented circle and polygon dataview filters. ## 8.0.0 Released 2019-11-13