From 9cffc8781a82465d50f3f5d8339b42815873303a Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 9 Oct 2018 18:48:44 +0200 Subject: [PATCH 01/17] Sample configs: use PostGIS to generate MVT's --- NEWS.md | 1 + config/environments/development.js.example | 2 +- config/environments/production.js.example | 2 +- config/environments/staging.js.example | 2 +- config/environments/test.js.example | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/NEWS.md b/NEWS.md index 794e849b..7b592964 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,7 @@ Released 2018-mm-dd New features - Aggregation time dimensions +- Update sample configurations to use PostGIS to generate MVT's by default (as in production) - Upgrades Windshaft to [4.11.2](https://github.com/CartoDB/Windshaft/blob/4.11.2/NEWS.md#version-4112) - `pg-mvt`: Use `query-rewriter` to compose the query to render a MVT tile. If not defined, it will use a Default Query Rewriter. - `pg-mvt`: Fix bug while building query and there is no columns defined for the layer. diff --git a/config/environments/development.js.example b/config/environments/development.js.example index 87c98032..be15ca6c 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -130,7 +130,7 @@ var config = { //If enabled, MVTs will be generated with PostGIS directly, instead of using Mapnik, //PostGIS 2.4 is required for this to work //If disabled it will use Mapnik MVT generation - usePostGIS: false + usePostGIS: true }, mapnik: { // The size of the pool of internal mapnik backend diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 8e071acf..c058e27f 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -130,7 +130,7 @@ var config = { //If enabled, MVTs will be generated with PostGIS directly, instead of using Mapnik, //PostGIS 2.4 is required for this to work //If disabled it will use Mapnik MVT generation - usePostGIS: false + usePostGIS: true }, mapnik: { // The size of the pool of internal mapnik backend diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index 60301a94..8d89e43f 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -130,7 +130,7 @@ var config = { //If enabled, MVTs will be generated with PostGIS directly, instead of using Mapnik, //PostGIS 2.4 is required for this to work //If disabled it will use Mapnik MVT generation - usePostGIS: false + usePostGIS: true }, mapnik: { // The size of the pool of internal mapnik backend diff --git a/config/environments/test.js.example b/config/environments/test.js.example index d106995e..184675a7 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -130,7 +130,7 @@ var config = { //If enabled, MVTs will be generated with PostGIS directly, instead of using Mapnik, //PostGIS 2.4 is required for this to work //If disabled it will use Mapnik MVT generation - usePostGIS: false + usePostGIS: true }, mapnik: { // The size of the pool of internal mapnik backend From 2af6486f73f1bdc6ce3ada48d36c66dedeab89cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 10 Oct 2018 11:23:25 +0200 Subject: [PATCH 02/17] new docker tags --- ...=> Dockerfile-nodejs6-xenial-pg101:latest} | 0 ...ockerfile-nodejs6-xenial-pg101:postgis-2.4 | 89 +++++++++++++++++++ ...rfile-nodejs6-xenial-pg101:postgis-2.4.4.5 | 88 ++++++++++++++++++ 3 files changed, 177 insertions(+) rename docker/{Dockerfile-nodejs6-xenial-pg101 => Dockerfile-nodejs6-xenial-pg101:latest} (100%) create mode 100644 docker/Dockerfile-nodejs6-xenial-pg101:postgis-2.4 create mode 100644 docker/Dockerfile-nodejs6-xenial-pg101:postgis-2.4.4.5 diff --git a/docker/Dockerfile-nodejs6-xenial-pg101 b/docker/Dockerfile-nodejs6-xenial-pg101:latest similarity index 100% rename from docker/Dockerfile-nodejs6-xenial-pg101 rename to docker/Dockerfile-nodejs6-xenial-pg101:latest diff --git a/docker/Dockerfile-nodejs6-xenial-pg101:postgis-2.4 b/docker/Dockerfile-nodejs6-xenial-pg101:postgis-2.4 new file mode 100644 index 00000000..d1d70589 --- /dev/null +++ b/docker/Dockerfile-nodejs6-xenial-pg101:postgis-2.4 @@ -0,0 +1,89 @@ +FROM ubuntu:xenial + +# Use UTF8 to avoid encoding problems with pgsql +ENV LANG C.UTF-8 +ENV NPROCS 1 +ENV JOBS 1 +ENV CXX g++-4.9 +ENV PGUSER postgres + +# Add external repos +RUN set -ex \ + && apt-get update \ + && apt-get install -y \ + curl \ + software-properties-common \ + locales \ + && add-apt-repository -y ppa:ubuntu-toolchain-r/test \ + && add-apt-repository -y ppa:cartodb/postgresql-10 \ + && add-apt-repository -y ppa:cartodb/gis \ + && curl -sL https://deb.nodesource.com/setup_6.x | bash \ + && locale-gen en_US.UTF-8 \ + && update-locale LANG=en_US.UTF-8 + +# Install dependencies and PostGIS 2.4 from sources +RUN set -ex \ + && apt-get update \ + && apt-get install -y \ + g++-4.9 \ + gcc-4.9 \ + git \ + libcairo2-dev \ + libgdal-dev \ + libgdal1i \ + libgdal20 \ + libgeos-dev \ + libgif-dev \ + libjpeg8-dev \ + libjson-c-dev \ + libpango1.0-dev \ + libpixman-1-dev \ + libproj-dev \ + libprotobuf-c-dev \ + libxml2-dev \ + gdal-bin \ + make \ + nodejs \ + protobuf-c-compiler \ + pkg-config \ + wget \ + zip \ + postgresql-10 \ + postgresql-10-plproxy \ + postgresql-10-postgis-2.4 \ + postgresql-10-postgis-2.4-scripts \ + postgresql-10-postgis-scripts \ + postgresql-client-10 \ + postgresql-client-common \ + postgresql-common \ + postgresql-contrib \ + postgresql-plpython-10 \ + postgresql-server-dev-10 \ + postgis \ + && wget http://download.redis.io/releases/redis-4.0.8.tar.gz \ + && tar xvzf redis-4.0.8.tar.gz \ + && cd redis-4.0.8 \ + && make \ + && make install \ + && cd .. \ + && rm redis-4.0.8.tar.gz \ + && rm -R redis-4.0.8 \ + && apt-get purge -y wget protobuf-c-compiler \ + && apt-get autoremove -y + +# Configure PostgreSQL +RUN set -ex \ + && echo "listen_addresses='*'" >> /etc/postgresql/10/main/postgresql.conf \ + && echo "local all all trust" > /etc/postgresql/10/main/pg_hba.conf \ + && echo "host all all 0.0.0.0/0 trust" >> /etc/postgresql/10/main/pg_hba.conf \ + && echo "host all all ::1/128 trust" >> /etc/postgresql/10/main/pg_hba.conf \ + && /etc/init.d/postgresql start \ + && createdb template_postgis \ + && createuser publicuser \ + && psql -c "CREATE EXTENSION postgis" template_postgis \ + && /etc/init.d/postgresql stop + +WORKDIR /srv +EXPOSE 5858 + +CMD /etc/init.d/postgresql start diff --git a/docker/Dockerfile-nodejs6-xenial-pg101:postgis-2.4.4.5 b/docker/Dockerfile-nodejs6-xenial-pg101:postgis-2.4.4.5 new file mode 100644 index 00000000..c4d7b091 --- /dev/null +++ b/docker/Dockerfile-nodejs6-xenial-pg101:postgis-2.4.4.5 @@ -0,0 +1,88 @@ +FROM ubuntu:xenial + +# Use UTF8 to avoid encoding problems with pgsql +ENV LANG C.UTF-8 +ENV NPROCS 1 +ENV JOBS 1 +ENV CXX g++-4.9 +ENV PGUSER postgres + +# Add external repos +RUN set -ex \ + && apt-get update \ + && apt-get install -y \ + curl \ + software-properties-common \ + locales \ + && add-apt-repository -y ppa:ubuntu-toolchain-r/test \ + && add-apt-repository -y ppa:cartodb/postgresql-10 \ + && add-apt-repository -y ppa:cartodb/gis \ + && curl -sL https://deb.nodesource.com/setup_6.x | bash \ + && locale-gen en_US.UTF-8 \ + && update-locale LANG=en_US.UTF-8 + +RUN set -ex \ + && apt-get update \ + && apt-get install -y \ + g++-4.9 \ + gcc-4.9 \ + git \ + libcairo2-dev \ + libgdal-dev \ + libgdal1i \ + libgdal20 \ + libgeos-dev \ + libgif-dev \ + libjpeg8-dev \ + libjson-c-dev \ + libpango1.0-dev \ + libpixman-1-dev \ + libproj-dev \ + libprotobuf-c-dev \ + libxml2-dev \ + gdal-bin \ + make \ + nodejs \ + protobuf-c-compiler \ + pkg-config \ + wget \ + zip \ + postgresql-10 \ + postgresql-10-plproxy \ + postgis=2.4.4.5+carto-1 \ + postgresql-10-postgis-2.4=2.4.4.5+carto-1 \ + postgresql-10-postgis-2.4-scripts=2.4.4.5+carto-1 \ + postgresql-10-postgis-scripts=2.4.4.5+carto-1 \ + postgresql-client-10 \ + postgresql-client-common \ + postgresql-common \ + postgresql-contrib \ + postgresql-plpython-10 \ + postgresql-server-dev-10 \ + && wget http://download.redis.io/releases/redis-4.0.8.tar.gz \ + && tar xvzf redis-4.0.8.tar.gz \ + && cd redis-4.0.8 \ + && make \ + && make install \ + && cd .. \ + && rm redis-4.0.8.tar.gz \ + && rm -R redis-4.0.8 \ + && apt-get purge -y wget protobuf-c-compiler \ + && apt-get autoremove -y + +# Configure PostgreSQL +RUN set -ex \ + && echo "listen_addresses='*'" >> /etc/postgresql/10/main/postgresql.conf \ + && echo "local all all trust" > /etc/postgresql/10/main/pg_hba.conf \ + && echo "host all all 0.0.0.0/0 trust" >> /etc/postgresql/10/main/pg_hba.conf \ + && echo "host all all ::1/128 trust" >> /etc/postgresql/10/main/pg_hba.conf \ + && /etc/init.d/postgresql start \ + && createdb template_postgis \ + && createuser publicuser \ + && psql -c "CREATE EXTENSION postgis" template_postgis \ + && /etc/init.d/postgresql stop + +WORKDIR /srv +EXPOSE 5858 + +CMD /etc/init.d/postgresql start From 945b1517129b4552d353c143cfdc83847f2dd1e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 10 Oct 2018 11:23:54 +0200 Subject: [PATCH 03/17] using docker tag postgis-2.4.4.5 in travis --- .travis.yml | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3e0b1daa..1e0952d8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ jobs: services: - docker language: generic - before_install: docker pull carto/nodejs6-xenial-pg101 + before_install: docker pull carto/nodejs6-xenial-pg101:postgis-2.4.4.5 script: npm run docker-test - dist: precise addons: @@ -28,7 +28,7 @@ jobs: - sudo add-apt-repository -y ppa:cartodb/gis-testing - sudo apt-get update - + # Force instalation of libgeos-3.5.0 (presumably needed because of existing version of postgis) - sudo apt-get -y install libgeos-3.5.0=3.5.0-1cdb2 @@ -59,9 +59,9 @@ jobs: - createdb template_postgis - createuser publicuser - psql -c "CREATE EXTENSION postgis" template_postgis - + # install yarn 0.27.5 - - curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 0.27.5 + - curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 0.27.5 - export PATH="$HOME/.yarn/bin:$PATH" # instal redis 4 diff --git a/package.json b/package.json index d958b3bc..47f1c24e 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "preinstall": "make pre-install", "test": "make test-all", "update-internal-deps": "rm -rf node_modules && rm -f yarn.lock && yarn", - "docker-test": "docker run -v `pwd`:/srv carto/nodejs6-xenial-pg101 bash run_tests_docker.sh && docker ps --filter status=dead --filter status=exited -aq | xargs -r docker rm -v", + "docker-test": "docker run -v `pwd`:/srv carto/nodejs6-xenial-pg101:postgis-2.4.4.5 bash run_tests_docker.sh && docker ps --filter status=dead --filter status=exited -aq | xargs -r docker rm -v", "docker-bash": "docker run -it -v `pwd`:/srv carto/nodejs6-xenial-pg101 bash" }, "engines": { From be08fa3bfabd5744196574d777245a40ed9f7b46 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 16 Oct 2018 15:41:34 +0200 Subject: [PATCH 04/17] Tweak id's to test against pg-mvt renderer Actually, the ID's are not generated by ST_AsMVT. They appear as an artifact of testing, when using toGeoJSONSync (implemented in mapnik). --- test/acceptance/date-wrapping.spec.js | 37 +++++++++++++-------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index 3eac2639..abe51ff9 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -21,13 +21,13 @@ describe('date-wrapping', () => { const expected = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } } @@ -65,13 +65,13 @@ describe('date-wrapping', () => { const expected = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 0 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 1 } } @@ -111,13 +111,13 @@ describe('date-wrapping', () => { const expected0 = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } } @@ -125,13 +125,13 @@ describe('date-wrapping', () => { const expected1 = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } } @@ -169,13 +169,13 @@ describe('date-wrapping', () => { const expected0 = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 0 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 1 } } @@ -183,13 +183,13 @@ describe('date-wrapping', () => { const expected1 = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } } @@ -227,13 +227,13 @@ describe('date-wrapping', () => { const expected0 = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 0 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 1 } } @@ -241,13 +241,13 @@ describe('date-wrapping', () => { const expected1 = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 0 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { _cdb_feature_count: 1, cartodb_id: 1 } } @@ -292,19 +292,18 @@ describe('date-wrapping', () => { const expected = [ { type: 'Feature', - id: 1, + id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { cartodb_id: 0, date: 1527810000, sc: 559082000 } }, { type: 'Feature', - id: 2, + id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, properties: { cartodb_id: 1, date: 1527900000, sc: 559082000 } } ]; const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; - assert.deepEqual(actual, expected); done(); }); From 4dba4ef64151a17ae920bbf7c269688b6b365691 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 16 Oct 2018 15:55:59 +0200 Subject: [PATCH 05/17] Tweak the scale denominator for pg-mvt renderer The scale denominator is calculated with float values and more precision, resulting in different (but more accurate) values --- test/acceptance/date-wrapping.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index abe51ff9..ce51d558 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -294,13 +294,13 @@ describe('date-wrapping', () => { type: 'Feature', id: 0, geometry: { type: 'Point', coordinates: [0, 0] }, - properties: { cartodb_id: 0, date: 1527810000, sc: 559082000 } + properties: { cartodb_id: 0, date: 1527810000, sc: 559082264.0287178839788058162356 } }, { type: 'Feature', id: 1, geometry: { type: 'Point', coordinates: [0, 0] }, - properties: { cartodb_id: 1, date: 1527900000, sc: 559082000 } + properties: { cartodb_id: 1, date: 1527900000, sc: 559082264.0287178839788058162356 } } ]; const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; From d474d49ce842b4a73bc105f6b3eb1a2b92774a65 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 16 Oct 2018 17:54:33 +0200 Subject: [PATCH 06/17] Do not use point in world's border --- test/acceptance/rate-limit.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acceptance/rate-limit.test.js b/test/acceptance/rate-limit.test.js index 38c94c5d..412a232a 100644 --- a/test/acceptance/rate-limit.test.js +++ b/test/acceptance/rate-limit.test.js @@ -19,7 +19,7 @@ let layergroupid; const query = ` SELECT - ST_Transform('SRID=4326;POINT(-180 85.05112877)'::geometry, 3857) the_geom_webmercator, + ST_Transform('SRID=4326;POINT(-70 42)'::geometry, 3857) the_geom_webmercator, 1 cartodb_id, 2 val `; @@ -335,7 +335,7 @@ describe('rate limit and vector tiles', function () { }; }; - testClient.getTile(0, 0, 0, tileParams(204, '1', '0', '1'), (err) => { + testClient.getTile(0, 0, 0, tileParams(200, '1', '0', '1'), (err) => { assert.ifError(err); testClient.getTile( From e50d1a10d0c4a33a1364eb12fc2985f16c81edb5 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 16 Oct 2018 18:27:27 +0200 Subject: [PATCH 07/17] Skip tests if they cannot be run If configured with `mvt.usePostGIS` but with no postgis version supporting it, they should be skipped. --- test/acceptance/date-wrapping.spec.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index ce51d558..665b8284 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -2,8 +2,13 @@ const assert = require('assert'); const TestClient = require('../support/test-client'); const mapConfigFactory = require('../fixtures/test_mapconfigFactory'); +const serverOptions = require('../../lib/cartodb/server_options'); -describe('date-wrapping', () => { +const usePgMvtRenderer = serverOptions.renderer.mvt.usePostGIS; +const postgisVersion = process.env.POSTGIS_VERSION; +const describe_mvt = postgisVersion >= '20400' || !usePgMvtRenderer ? describe : describe.skip; + +describe_mvt('date-wrapping', () => { let testClient; describe('when a map instantiation has one single layer', () => { From e1576495710ea7952993605d2440ead6479f408b Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 16 Oct 2018 19:32:55 +0200 Subject: [PATCH 08/17] Use of postgis renderer based on availabilty --- test/acceptance/layergroup-metadata.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/acceptance/layergroup-metadata.js b/test/acceptance/layergroup-metadata.js index 4da60ef4..c3a109be 100644 --- a/test/acceptance/layergroup-metadata.js +++ b/test/acceptance/layergroup-metadata.js @@ -2,8 +2,21 @@ require('../support/test_helper'); const assert = require('../support/assert'); const TestClient = require('../support/test-client'); +const serverOptions = require('../../lib/cartodb/server_options'); describe('layergroup metadata', function () { + + const usePgMvtRenderer = process.env.POSTGIS_VERSION >= '20400'; + const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; + + before(function () { + serverOptions.renderer.mvt.usePostGIS = usePgMvtRenderer; + }); + + after(function () { + serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; + }); + [1234, 'default_public', false].forEach(api_key => { it(`tiles base urls ${api_key ? `with api key: ${api_key}` : 'without api key'}`, function (done) { const mapConfig = { From a42af5e0d58f617b76f8f778cef0c107c109566f Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 16 Oct 2018 19:51:22 +0200 Subject: [PATCH 09/17] Do not run test if ST_AsMvt not avail. --- .../acceptance/user-database-timeout-limit.js | 90 ++++--------------- 1 file changed, 17 insertions(+), 73 deletions(-) diff --git a/test/acceptance/user-database-timeout-limit.js b/test/acceptance/user-database-timeout-limit.js index 611daa6d..1a2a505c 100644 --- a/test/acceptance/user-database-timeout-limit.js +++ b/test/acceptance/user-database-timeout-limit.js @@ -380,7 +380,22 @@ describe('user database timeout limit', function () { }); }); - describe('fetching vector tiles', function () { + const describe_pg = process.env.POSTGIS_VERSION >= '20400' ? describe : describe.skip; + describe('fetching vector tiles via mapnik renderer', () => { testFetchingVectorTiles(false); }); + describe_pg('fetching vector tiles via postgis renderer', () => { testFetchingVectorTiles(true); }); + + function testFetchingVectorTiles(usePostGIS) { + const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; + + before(function () { + serverOptions.renderer.mvt.usePostGIS = usePostGIS; + }); + + after(function () { + serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; + }); + + beforeEach(function (done) { const mapconfig = createMapConfig(); this.testClient = new TestClient(mapconfig, 1234); @@ -442,78 +457,7 @@ describe('user database timeout limit', function () { }); }); - - if (process.env.POSTGIS_VERSION >= '20400') { - describe('fetching vector tiles via PostGIS renderer', function() { - const usePostGIS = true; - const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; - - beforeEach(function (done) { - serverOptions.renderer.mvt.usePostGIS = usePostGIS; - - const mapconfig = createMapConfig(); - this.testClient = new TestClient(mapconfig, 1234); - const expectedResponse = { - status: 200, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }; - - this.testClient.getLayergroup({ response: expectedResponse }, (err, res) => { - if (err) { - return done(err); - } - - this.layergroupid = res.layergroupid; - - done(); - }); - }); - - afterEach(function () { - serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; - }); - - describe('with user\'s timeout of 200 ms', function () { - beforeEach(function (done) { - this.testClient.setUserDatabaseTimeoutLimit(200, done); - }); - - afterEach(function (done) { - this.testClient.setUserDatabaseTimeoutLimit(0, done); - }); - - it('"mvt" fails due to statement timeout', function (done) { - const params = { - layergroupid: this.layergroupid, - format: 'mvt', - layers: [ 0 ], - response: { - status: 429, - headers: { - 'Content-Type': 'application/x-protobuf' - } - }, - cacheBuster: true - }; - - this.testClient.getTile(0, 0, 0, params, (err, res, tile) => { - assert.ifError(err); - - var tileJSON = tile.toJSON(); - assert.equal(Array.isArray(tileJSON), true); - assert.equal(tileJSON.length, 2); - assert.equal(tileJSON[0].name, 'errorTileSquareLayer'); - assert.equal(tileJSON[1].name, 'errorTileStripesLayer'); - - done(); - }); - }); - }); - }); - } - }); + } }); From 376a3743c1a4469db931005d3921945d9c6aabc1 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 17 Oct 2018 10:06:48 +0200 Subject: [PATCH 10/17] Fix buffer size per format tests --- test/acceptance/buffer-size-format.js | 39 +++++++++++++++++++-------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/test/acceptance/buffer-size-format.js b/test/acceptance/buffer-size-format.js index 74e6288c..0fd8d800 100644 --- a/test/acceptance/buffer-size-format.js +++ b/test/acceptance/buffer-size-format.js @@ -54,6 +54,8 @@ function createMapConfig (bufferSize, cartocss) { } describe('buffer size per format', function () { + let testClient; + var testCases = [ { desc: 'should get png tile using buffer-size 0', @@ -126,25 +128,27 @@ describe('buffer size per format', function () { ]; afterEach(function(done) { - if (this.testClient) { - return this.testClient.drain(done); + if (testClient) { + return testClient.drain(done); } return done(); }); const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; + after(function () { + serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; + }); + testCases.forEach(function (test) { - var testFn = (usePostGIS) => { + var testFn = () => { it(test.desc, function (done) { - serverOptions.renderer.mvt.usePostGIS = usePostGIS; - this.testClient = new TestClient(test.mapConfig, 1234); - serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; + testClient = new TestClient(test.mapConfig, 1234); var coords = test.coords; var options = { format: test.format, layers: test.layers }; - this.testClient.getTile(coords.z, coords.x, coords.y, options, function (err, res, tile) { + testClient.getTile(coords.z, coords.x, coords.y, options, function (err, res, tile) { assert.ifError(err); // To generate images use: // tile.save(test.fixturePath); @@ -152,10 +156,23 @@ describe('buffer size per format', function () { }); }); }; - if (process.env.POSTGIS_VERSION >= '20400' && test.format === 'mvt'){ - testFn(true); + if (test.format === 'mvt') { + const describe_pg = process.env.POSTGIS_VERSION >= '20400' ? describe : describe.skip; + describe('using mapnik mvt renderer', function() { + before(function () { + serverOptions.renderer.mvt.usePostGIS = false; + }); + testFn(); + }); + describe_pg('using postgis mvt renderer', function() { + before(function () { + serverOptions.renderer.mvt.usePostGIS = true; + }); + testFn(); + }); + } else { + testFn(); } - testFn(false); }); }); @@ -444,7 +461,7 @@ describe('buffer size per format for named maps w/o placeholders', function () { const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; testCases.forEach(function (test) { var testFn = (usePostGIS) => { - it(test.desc + `(${usePostGIS? 'PostGIS':'mapnik'})`, function (done) { + it(test.desc + `(${usePostGIS? 'PostGIS':'mapnik'})`, function (done) { serverOptions.renderer.mvt.usePostGIS = usePostGIS; test.template.name += '_1'; this.testClient = new TestClient(test.template, 1234); From 8491b86c1754293d29b18ea519b5a757027430da Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 17 Oct 2018 10:08:55 +0200 Subject: [PATCH 11/17] Extract test generation function --- test/acceptance/buffer-size-format.js | 39 ++++++++++++++------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/test/acceptance/buffer-size-format.js b/test/acceptance/buffer-size-format.js index 0fd8d800..705992dc 100644 --- a/test/acceptance/buffer-size-format.js +++ b/test/acceptance/buffer-size-format.js @@ -139,39 +139,40 @@ describe('buffer size per format', function () { serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; }); - testCases.forEach(function (test) { - var testFn = () => { - it(test.desc, function (done) { - testClient = new TestClient(test.mapConfig, 1234); - var coords = test.coords; - var options = { - format: test.format, - layers: test.layers - }; - testClient.getTile(coords.z, coords.x, coords.y, options, function (err, res, tile) { - assert.ifError(err); - // To generate images use: - // tile.save(test.fixturePath); - test.assert(tile, done); - }); + var testFn = (test) => { + it(test.desc, function (done) { + testClient = new TestClient(test.mapConfig, 1234); + var coords = test.coords; + var options = { + format: test.format, + layers: test.layers + }; + testClient.getTile(coords.z, coords.x, coords.y, options, function (err, res, tile) { + assert.ifError(err); + // To generate images use: + // tile.save(test.fixturePath); + test.assert(tile, done); }); - }; + }); + }; + + testCases.forEach(function (test) { if (test.format === 'mvt') { const describe_pg = process.env.POSTGIS_VERSION >= '20400' ? describe : describe.skip; describe('using mapnik mvt renderer', function() { before(function () { serverOptions.renderer.mvt.usePostGIS = false; }); - testFn(); + testFn(test); }); describe_pg('using postgis mvt renderer', function() { before(function () { serverOptions.renderer.mvt.usePostGIS = true; }); - testFn(); + testFn(test); }); } else { - testFn(); + testFn(test); } }); }); From bd17f9f5e17aec499e68a5ed7b5ce2d384081c11 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 17 Oct 2018 10:25:18 +0200 Subject: [PATCH 12/17] A better implementation of mvt suites --- test/acceptance/buffer-size-format.js | 37 +++++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/test/acceptance/buffer-size-format.js b/test/acceptance/buffer-size-format.js index 705992dc..86003952 100644 --- a/test/acceptance/buffer-size-format.js +++ b/test/acceptance/buffer-size-format.js @@ -156,24 +156,27 @@ describe('buffer size per format', function () { }); }; - testCases.forEach(function (test) { - if (test.format === 'mvt') { - const describe_pg = process.env.POSTGIS_VERSION >= '20400' ? describe : describe.skip; - describe('using mapnik mvt renderer', function() { - before(function () { - serverOptions.renderer.mvt.usePostGIS = false; - }); - testFn(test); - }); - describe_pg('using postgis mvt renderer', function() { - before(function () { - serverOptions.renderer.mvt.usePostGIS = true; - }); - testFn(test); - }); - } else { + testCases.filter(test => test.format !== 'mvt').forEach(function (test) { + testFn(test); + }); + + describe('using mapnik mvt renderer', function() { + before(function () { + serverOptions.renderer.mvt.usePostGIS = false; + }); + testCases.filter(test => test.format === 'mvt').forEach(function (test) { testFn(test); - } + }); + }); + + const describe_pg = process.env.POSTGIS_VERSION >= '20400' ? describe : describe.skip; + describe_pg('using postgis mvt renderer', function() { + before(function () { + serverOptions.renderer.mvt.usePostGIS = true; + }); + testCases.filter(test => test.format === 'mvt').forEach(function (test) { + testFn(test); + }); }); }); From bfbd9a8f22dc64e88bc7181579c63ae5bf087976 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 17 Oct 2018 10:33:12 +0200 Subject: [PATCH 13/17] Fix another suite (compat mapnik/pgis) --- test/acceptance/buffer-size-format.js | 79 +++++++++++++++++---------- 1 file changed, 50 insertions(+), 29 deletions(-) diff --git a/test/acceptance/buffer-size-format.js b/test/acceptance/buffer-size-format.js index 86003952..a69a30ed 100644 --- a/test/acceptance/buffer-size-format.js +++ b/test/acceptance/buffer-size-format.js @@ -323,6 +323,8 @@ describe('buffer size per format for named maps', function () { describe('buffer size per format for named maps w/o placeholders', function () { + let testClient; + var testCases = [ { desc: 'should get png tile using buffer-size 0 overriden by template params', @@ -456,39 +458,58 @@ describe('buffer size per format for named maps w/o placeholders', function () { ]; afterEach(function(done) { - if (this.testClient) { - return this.testClient.drain(done); + if (testClient) { + return testClient.drain(done); } return done(); }); const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; - testCases.forEach(function (test) { - var testFn = (usePostGIS) => { - it(test.desc + `(${usePostGIS? 'PostGIS':'mapnik'})`, function (done) { - serverOptions.renderer.mvt.usePostGIS = usePostGIS; - test.template.name += '_1'; - this.testClient = new TestClient(test.template, 1234); - serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; - var coords = test.coords; - var options = { - format: test.format, - placeholders: test.placeholders, - layers: test.layers - }; - this.testClient.getTile(coords.z, coords.x, coords.y, options, function (err, res, tile) { - assert.ifError(err); - // To generate images use: - //tile.save(test.fixturePath); - // require('fs').writeFileSync(test.fixturePath, JSON.stringify(tile)); - // require('fs').writeFileSync(test.fixturePath, tile.getDataSync()); - test.assert(tile, done); - }); - }); - }; - if (process.env.POSTGIS_VERSION >= '20400' && test.format === 'mvt'){ - testFn(true); - } - testFn(false); + after(function () { + serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; + }); + + var testFn = (test) => { + it(test.desc, function (done) { + test.template.name += '_1'; + testClient = new TestClient(test.template, 1234); + var coords = test.coords; + var options = { + format: test.format, + placeholders: test.placeholders, + layers: test.layers + }; + testClient.getTile(coords.z, coords.x, coords.y, options, function (err, res, tile) { + assert.ifError(err); + // To generate images use: + //tile.save(test.fixturePath); + // require('fs').writeFileSync(test.fixturePath, JSON.stringify(tile)); + // require('fs').writeFileSync(test.fixturePath, tile.getDataSync()); + test.assert(tile, done); + }); + }); + }; + + testCases.filter(test => test.format !== 'mvt').forEach(function (test) { + testFn(test); + }); + + describe('using mapnik mvt renderer', function() { + before(function () { + serverOptions.renderer.mvt.usePostGIS = false; + }); + testCases.filter(test => test.format === 'mvt').forEach(function (test) { + testFn(test); + }); + }); + + const describe_pg = process.env.POSTGIS_VERSION >= '20400' ? describe : describe.skip; + describe_pg('using postgis mvt renderer', function() { + before(function () { + serverOptions.renderer.mvt.usePostGIS = true; + }); + testCases.filter(test => test.format === 'mvt').forEach(function (test) { + testFn(test); + }); }); }); From d25e8e97983148751dae81d8d43bfa4c05ba2164 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 17 Oct 2018 11:12:58 +0200 Subject: [PATCH 14/17] Make the test work in dual mode (mapnik/pgis) --- test/acceptance/tilejson.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/test/acceptance/tilejson.js b/test/acceptance/tilejson.js index 0d3bc133..0230b715 100644 --- a/test/acceptance/tilejson.js +++ b/test/acceptance/tilejson.js @@ -2,8 +2,23 @@ require('../support/test_helper'); const assert = require('../support/assert'); const TestClient = require('../support/test-client'); +const serverOptions = require('../../lib/cartodb/server_options'); -describe('tilejson', function() { +const describe_pg = process.env.POSTGIS_VERSION >= '20400' ? describe : describe.skip; +const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; + +describe('tilejson via mapnik renderer', () => { tileJsonSuite(false); }); +describe_pg('tilejson via postgis renderer', () => { tileJsonSuite(true); }); + +function tileJsonSuite(usePostGIS) { + + before(function() { + serverOptions.renderer.mvt.usePostGIS = usePostGIS; + }); + + after(function () { + serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; + }); function tilejsonValidation(tilejson, shouldHaveGrid = false) { assert.equal(tilejson.tilejson, '2.2.0'); @@ -215,5 +230,4 @@ describe('tilejson', function() { }); }); - -}); +} From f26ddef24470188ea6a96be84948d0e2bf9ee699 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 17 Oct 2018 15:09:09 +0200 Subject: [PATCH 15/17] Make rate limit tests work in dual mode --- test/acceptance/rate-limit.test.js | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/test/acceptance/rate-limit.test.js b/test/acceptance/rate-limit.test.js index 412a232a..a6465d1d 100644 --- a/test/acceptance/rate-limit.test.js +++ b/test/acceptance/rate-limit.test.js @@ -7,6 +7,7 @@ const cartodbRedis = require('cartodb-redis'); const TestClient = require('../support/test-client'); const UserLimitsBackend = require('../../lib/cartodb/backends/user-limits'); const rateLimitMiddleware = require('../../lib/cartodb/api/middlewares/rate-limit'); +const serverOptions = require('../../lib/cartodb/server_options'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimitMiddleware; let userLimitsApi; @@ -273,7 +274,21 @@ describe('rate limit middleware', function () { }); }); -describe('rate limit and vector tiles', function () { +const describe_pg = process.env.POSTGIS_VERSION >= '20400' ? describe : describe.skip; +const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; + +describe('rate limit and vector tiles (mapnik)', () => { rateLimitAndVectorTilesTest(false); }); +describe_pg('rate limit and vector tiles (postgis)', () => { rateLimitAndVectorTilesTest(true); }); + +function rateLimitAndVectorTilesTest(usePostGIS) { + + before(function() { + serverOptions.renderer.mvt.usePostGIS = usePostGIS; + }); + + after(function () { + serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; + }); before(function(done) { global.environment.enabledFeatures.rateLimitsEnabled = true; @@ -358,4 +373,4 @@ describe('rate limit and vector tiles', function () { }); }); -}); +} From 83897293c67d90df64d01603ffd899aeae83ec38 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 17 Oct 2018 17:32:51 +0200 Subject: [PATCH 16/17] Fix test by giving redis enough time to delete --- test/acceptance/rate-limit.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/rate-limit.test.js b/test/acceptance/rate-limit.test.js index a6465d1d..2dd478ba 100644 --- a/test/acceptance/rate-limit.test.js +++ b/test/acceptance/rate-limit.test.js @@ -325,7 +325,7 @@ function rateLimitAndVectorTilesTest(usePostGIS) { redisClient.SELECT(5, () => { redisClient.del('user:localhost:mapviews:global'); - done(); + setTimeout(done, 1000); }); }); }); From 5573db2bc1d016b26c84b175cd709904553eda1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 19 Oct 2018 11:57:54 +0200 Subject: [PATCH 17/17] Do not use Object.assign as _.defautls equivalent --- .../mapconfig/provider/map-store-provider.js | 10 ++------ .../mapconfig/provider/named-map-provider.js | 23 ++++++++++--------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/lib/cartodb/models/mapconfig/provider/map-store-provider.js b/lib/cartodb/models/mapconfig/provider/map-store-provider.js index abc9eca2..b3210298 100644 --- a/lib/cartodb/models/mapconfig/provider/map-store-provider.js +++ b/lib/cartodb/models/mapconfig/provider/map-store-provider.js @@ -72,14 +72,8 @@ module.exports = class MapStoreMapConfigProvider extends BaseMapConfigProvider { } createKey (base) { - const tplValues = Object.assign({ - dbname: '', - token: '', - dbuser: '', - format: '', - layer: '', - scale_factor: 1 - }, this.params); + const { dbname = '', token = '', dbuser = '', format = '', layer = '', scale_factor = 1 } = this.params; + const tplValues = { dbname, token, dbuser, format, layer, scale_factor }; return (base) ? baseKeyTpl(tplValues) : rendererKeyTpl(tplValues); } diff --git a/lib/cartodb/models/mapconfig/provider/named-map-provider.js b/lib/cartodb/models/mapconfig/provider/named-map-provider.js index 12a41f43..ee0e0bd2 100644 --- a/lib/cartodb/models/mapconfig/provider/named-map-provider.js +++ b/lib/cartodb/models/mapconfig/provider/named-map-provider.js @@ -232,15 +232,16 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider { } createKey (base) { - const tplValues = Object.assign({ - dbname: '', - user: this.user, - templateName: this.templateName, - authToken: this.authToken || '', - configHash: configHash(this.config), - layer: '', - scale_factor: 1 - }, this.params); + const { + dbname = '', + user = this.user, + templateName = this.templateName, + authToken = this.authToken || '', + configHash = createConfigHash(this.config), + layer = '', + scale_factor = 1 + } = this.params; + const tplValues = { dbname, user, templateName, authToken, configHash, layer, scale_factor }; return (base) ? baseKeyTpl(tplValues) : rendererKeyTpl(tplValues); } @@ -268,7 +269,7 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider { } }; -function configHash(config) { +function createConfigHash(config) { if (!config) { return ''; } @@ -276,4 +277,4 @@ function configHash(config) { return crypto.createHash('md5').update(JSON.stringify(config)).digest('hex').substring(0,8); } -module.exports.configHash = configHash; +module.exports.configHash = createConfigHash;