From a1f0cb3f08d5a94cfac34767f65c8a73b9a3df26 Mon Sep 17 00:00:00 2001 From: Alicia Date: Mon, 13 Dec 2021 10:57:59 +0100 Subject: [PATCH 1/5] added initial fix and test --- app/src/services/geoStoreService.js | 40 ++++++++++++++++++++++++++-- app/test/e2e/v1/geostore-get.spec.js | 26 ++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/app/src/services/geoStoreService.js b/app/src/services/geoStoreService.js index efe81a6..e402f6b 100644 --- a/app/src/services/geoStoreService.js +++ b/app/src/services/geoStoreService.js @@ -151,10 +151,45 @@ class GeoStoreService { } } + /** + * @description: checks if a bbox crosses the antimeridian + * @param {Array} bbox + * @returns {Boolean} + * + */ + static async antimeridian(bbox) { + logger.debug('Checking antimeridian'); + const west = bbox[0]; + const east = bbox[2]; + if (west > east) { + logger.debug('Antimeridian detected'); + return true; + } + return false; + } + /** + * @description: This function translates a bbox that crosses the antimeridian + * @param {Array} bbox + * @returns {Array} bbox with the antimeridian corrected + */ + static async translateBBox(bbox) { + logger.debug('Converting bbox from [-180,180] to [0,360]'); + return [bbox[0], bbox[1], 360 - bbox[2], bbox[3]] + + } + /** + * @description: Calculates a bbox. + * If a bbox that crosses the antimeridian will be transformed its + * latitudes from [-180, 180] to [0, 360] + * @param {geoStore} geoStore + * @returns {geoStore} + * + **/ static async calculateBBox(geoStore) { logger.debug('Calculating bbox'); - geoStore.bbox = turf.bbox(geoStore.geojson); + const bbox = turf.bbox(geoStore.geojson); + geoStore.bbox = this.antimeridian(bbox) ? this.translateBBox(bbox) : bbox; await geoStore.save(); return geoStore; } @@ -214,7 +249,8 @@ class GeoStoreService { hash: geoStore.hash }); if (!geoStore.bbox) { - geoStore.bbox = turf.bbox(geoStore.geojson); + const bbox = turf.bbox(geoStore.geojson); + geoStore.bbox = this.antimeridian(bbox) ? this.translateBBox(bbox) : bbox; } return GeoStore.findOneAndUpdate({ hash: geoStore.hash }, geoStore, { diff --git a/app/test/e2e/v1/geostore-get.spec.js b/app/test/e2e/v1/geostore-get.spec.js index 8c5c4df..a029c5b 100644 --- a/app/test/e2e/v1/geostore-get.spec.js +++ b/app/test/e2e/v1/geostore-get.spec.js @@ -61,6 +61,32 @@ describe('Geostore v1 tests - Get geostores', () => { hash.should.equal(createdGeostore.hash); }); + it('Getting a geostore that crosses the antimeridian should give a bbox [position 0 and 2] from [-180, 180] to [0, 360] (happy case)', async () => { + const createdGeostore = await createGeostore(); + const response = await geostore.get(createdGeostore.hash); + + response.status.should.equal(200); + response.body.should.instanceOf(Object).and.have.property('data'); + + const { data } = response.body; + data.id.should.equal(createdGeostore.hash); + data.type.should.equal('geoStore'); + data.should.have.property('attributes').and.should.instanceOf(Object); + data.attributes.should.instanceOf(Object); + + const { geojson, bbox, hash } = data.attributes; + + const expectedGeojson = { + ...DEFAULT_GEOJSON, + crs: {}, + }; + delete expectedGeojson.features[0].properties; + + geojson.should.deep.equal(expectedGeojson); + bbox.should.instanceOf(Array); + hash.should.equal(createdGeostore.hash); + }); + it('Getting geostore with format esri should return the result with esrijson (happy case)', async () => { const createdGeostore = await createGeostore(); const response = await geostore.get(createdGeostore.hash).query({ format: 'esri' }); From d7060dde10e954a5e27c8c53731926358020edd6 Mon Sep 17 00:00:00 2001 From: Alicia Date: Tue, 14 Dec 2021 11:05:55 +0100 Subject: [PATCH 2/5] updated geostore bbox calculation with antimeridian and e2e tests --- Dockerfile | 11 +++---- app/src/services/geoStoreService.js | 49 ++++++++++++++++++++-------- app/test/e2e/utils/test.constants.js | 37 ++++++++++++++++++++- app/test/e2e/v1/geostore-get.spec.js | 12 +++++-- 4 files changed, 86 insertions(+), 23 deletions(-) diff --git a/Dockerfile b/Dockerfile index 5311f34..38b5ef6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,15 +1,14 @@ -FROM node:12-alpine +FROM node:12-bullseye MAINTAINER info@vizzuality.com ENV NAME gfw-geostore-api ENV USER microservice -RUN apk update && apk upgrade && \ - apk add --no-cache --update bash git openssh python alpine-sdk +RUN apt-get update -y && apt-get upgrade -y && \ + apt-get install -y bash git ssh python3 make -RUN addgroup $USER && adduser -s /bin/bash -D -G $USER $USER - -RUN yarn global add grunt-cli bunyan +RUN addgroup $USER && useradd -ms /bin/bash $USER -g $USER +RUN yarn global add bunyan grunt RUN mkdir -p /opt/$NAME COPY package.json /opt/$NAME/package.json diff --git a/app/src/services/geoStoreService.js b/app/src/services/geoStoreService.js index e402f6b..fda8d48 100644 --- a/app/src/services/geoStoreService.js +++ b/app/src/services/geoStoreService.js @@ -151,11 +151,12 @@ class GeoStoreService { } } + /** * @description: checks if a bbox crosses the antimeridian * @param {Array} bbox * @returns {Boolean} - * + * */ static async antimeridian(bbox) { logger.debug('Checking antimeridian'); @@ -170,26 +171,43 @@ class GeoStoreService { /** * @description: This function translates a bbox that crosses the antimeridian - * @param {Array} bbox + * @param {Array} bbox * @returns {Array} bbox with the antimeridian corrected */ - static async translateBBox(bbox) { + static translateBBox(bbox) { logger.debug('Converting bbox from [-180,180] to [0,360]'); - return [bbox[0], bbox[1], 360 - bbox[2], bbox[3]] - + const newBBox = [bbox[0], bbox[1], 360 + bbox[2], bbox[3]]; + return newBBox; + } + /** * @description: Calculates a bbox. - * If a bbox that crosses the antimeridian will be transformed its + * If a bbox that crosses the antimeridian will be transformed its + * latitudes from [-180, 180] to [0, 360] + * @param {geoStore} geoStore + * @returns {bbox} + * + * */ + static async swapBBox(geoStore) { + const bbox = await turf.bbox(geoStore.geojson); + const antimeridian = await GeoStoreService.antimeridian(bbox); + logger.info(bbox); + return antimeridian ? GeoStoreService.translateBBox(bbox) : bbox; + + } + + /** + * @description: Calculates a bbox. + * If a bbox that crosses the antimeridian will be transformed its * latitudes from [-180, 180] to [0, 360] * @param {geoStore} geoStore * @returns {geoStore} - * - **/ + * + * */ static async calculateBBox(geoStore) { logger.debug('Calculating bbox'); - const bbox = turf.bbox(geoStore.geojson); - geoStore.bbox = this.antimeridian(bbox) ? this.translateBBox(bbox) : bbox; + geoStore.bbox = await GeoStoreService.swapBBox(geoStore); await geoStore.save(); return geoStore; } @@ -238,19 +256,24 @@ class GeoStoreService { logger.debug('Repaired geometry', JSON.stringify(geoStore.geojson)); logger.debug('Make Feature Collection'); + geoStore.geojson = GeoJSONConverter.makeFeatureCollection(geoStore.geojson, props); + logger.debug('Result', JSON.stringify(geoStore.geojson)); logger.debug('Creating hash from geojson md5'); + geoStore.hash = md5(JSON.stringify(geoStore.geojson)); + if (geoStore.areaHa === undefined) { geoStore.areaHa = turf.area(geoStore.geojson) / 10000; // convert to ha2 } await GeoStore.findOne({ hash: geoStore.hash }); + logger.debug('bbox geostore'); + logger.debug('geojson', JSON.stringify(geoStore.bbox)); if (!geoStore.bbox) { - const bbox = turf.bbox(geoStore.geojson); - geoStore.bbox = this.antimeridian(bbox) ? this.translateBBox(bbox) : bbox; + geoStore.bbox = await GeoStoreService.swapBBox(geoStore); } return GeoStore.findOneAndUpdate({ hash: geoStore.hash }, geoStore, { @@ -281,7 +304,7 @@ class GeoStoreService { geoStore.geojson = GeoJSONConverter.makeFeatureCollection(geoStore.geojson); logger.debug('Result', JSON.stringify(geoStore.geojson)); geoStore.areaHa = turf.area(geoStore.geojson) / 10000; // convert to ha2 - geoStore.bbox = turf.bbox(geoStore.geojson); + geoStore.bbox = await GeoStoreService.swapBBox(geoStore); // calculate bbox return geoStore; diff --git a/app/test/e2e/utils/test.constants.js b/app/test/e2e/utils/test.constants.js index 3df2dfa..0891ace 100644 --- a/app/test/e2e/utils/test.constants.js +++ b/app/test/e2e/utils/test.constants.js @@ -23,10 +23,45 @@ const DEFAULT_GEOJSON = { }], }; +const ANTIMERIDIAN_GEOJSON = { + type: 'FeatureCollection', + features: [{ + type: 'Feature', + properties: {}, + geometry: { + type: 'Polygon', + coordinates: [ + [ + [ + 176.55029296875, + -20.11783963049162 + ], + [ + 183.09814453125, + -20.11783963049162 + ], + [ + 183.09814453125, + -14.827991347352068 + ], + [ + 176.55029296875, + -14.827991347352068 + ], + [ + 176.55029296875, + -20.11783963049162 + ] + ] + ] + } + }], +}; + const MOCK_RESULT_CARTODB = [{ geojson: '{"type":"MultiPolygon","coordinates":[[[[7.4134,43.7346],[7.4396,43.7492],[7.4179,43.7226],[7.4095,43.7299],[7.4134,43.7346]]]]}', area_ha: 235.490994944, name: 'Monaco' }]; -module.exports = { DEFAULT_GEOJSON, MOCK_RESULT_CARTODB }; +module.exports = { DEFAULT_GEOJSON, ANTIMERIDIAN_GEOJSON, MOCK_RESULT_CARTODB }; diff --git a/app/test/e2e/v1/geostore-get.spec.js b/app/test/e2e/v1/geostore-get.spec.js index a029c5b..221b77d 100644 --- a/app/test/e2e/v1/geostore-get.spec.js +++ b/app/test/e2e/v1/geostore-get.spec.js @@ -4,7 +4,7 @@ const config = require('config'); const GeoStore = require('models/geoStore'); const { createRequest } = require('../utils/test-server'); -const { DEFAULT_GEOJSON } = require('../utils/test.constants'); +const { DEFAULT_GEOJSON, ANTIMERIDIAN_GEOJSON } = require('../utils/test.constants'); const { getUUID, ensureCorrectError, createGeostore } = require('../utils/utils'); chai.should(); @@ -62,7 +62,7 @@ describe('Geostore v1 tests - Get geostores', () => { }); it('Getting a geostore that crosses the antimeridian should give a bbox [position 0 and 2] from [-180, 180] to [0, 360] (happy case)', async () => { - const createdGeostore = await createGeostore(); + const createdGeostore = await createGeostore({}, ANTIMERIDIAN_GEOJSON); const response = await geostore.get(createdGeostore.hash); response.status.should.equal(200); @@ -77,13 +77,19 @@ describe('Geostore v1 tests - Get geostores', () => { const { geojson, bbox, hash } = data.attributes; const expectedGeojson = { - ...DEFAULT_GEOJSON, + ...ANTIMERIDIAN_GEOJSON, crs: {}, }; delete expectedGeojson.features[0].properties; geojson.should.deep.equal(expectedGeojson); bbox.should.instanceOf(Array); + bbox.should.deep.equal([ + -69.074821, + -19.780273, + -39.587517, + -4.488809 + ]); hash.should.equal(createdGeostore.hash); }); From 1732e0f6019ef313ad8daef68aa50f80c94f83a5 Mon Sep 17 00:00:00 2001 From: Alicia Date: Tue, 14 Dec 2021 11:07:23 +0100 Subject: [PATCH 3/5] updated e2e tests correct bbox --- app/test/e2e/v1/geostore-get.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/test/e2e/v1/geostore-get.spec.js b/app/test/e2e/v1/geostore-get.spec.js index 221b77d..3879c77 100644 --- a/app/test/e2e/v1/geostore-get.spec.js +++ b/app/test/e2e/v1/geostore-get.spec.js @@ -85,10 +85,10 @@ describe('Geostore v1 tests - Get geostores', () => { geojson.should.deep.equal(expectedGeojson); bbox.should.instanceOf(Array); bbox.should.deep.equal([ - -69.074821, - -19.780273, - -39.587517, - -4.488809 + 176.550292969, + -20.11783963, + 183.098144531, + -14.827991347 ]); hash.should.equal(createdGeostore.hash); }); From fca4152d8c812eadbac1b62abb858a9ceefed5d3 Mon Sep 17 00:00:00 2001 From: Alicia Date: Tue, 14 Dec 2021 11:14:19 +0100 Subject: [PATCH 4/5] updated e2e tests correct bbox for geometry --- app/test/e2e/v1/geostore-get.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/test/e2e/v1/geostore-get.spec.js b/app/test/e2e/v1/geostore-get.spec.js index 3879c77..06aa3b8 100644 --- a/app/test/e2e/v1/geostore-get.spec.js +++ b/app/test/e2e/v1/geostore-get.spec.js @@ -85,10 +85,10 @@ describe('Geostore v1 tests - Get geostores', () => { geojson.should.deep.equal(expectedGeojson); bbox.should.instanceOf(Array); bbox.should.deep.equal([ - 176.550292969, - -20.11783963, - 183.098144531, - -14.827991347 + 176.55029296875, + -20.11783963049162, + 183.09814453125, + -14.827991347352068 ]); hash.should.equal(createdGeostore.hash); }); From 9e1a72a2b2164daa0c35dbd57b189b875bf70931 Mon Sep 17 00:00:00 2001 From: Tiago Garcia Date: Tue, 14 Dec 2021 13:59:53 +0100 Subject: [PATCH 5/5] Revert functional changes on rw-105 --- app/src/services/geoStoreService.js | 65 ++--------------------------- 1 file changed, 3 insertions(+), 62 deletions(-) diff --git a/app/src/services/geoStoreService.js b/app/src/services/geoStoreService.js index fda8d48..efe81a6 100644 --- a/app/src/services/geoStoreService.js +++ b/app/src/services/geoStoreService.js @@ -152,62 +152,9 @@ class GeoStoreService { } } - /** - * @description: checks if a bbox crosses the antimeridian - * @param {Array} bbox - * @returns {Boolean} - * - */ - static async antimeridian(bbox) { - logger.debug('Checking antimeridian'); - const west = bbox[0]; - const east = bbox[2]; - if (west > east) { - logger.debug('Antimeridian detected'); - return true; - } - return false; - } - - /** - * @description: This function translates a bbox that crosses the antimeridian - * @param {Array} bbox - * @returns {Array} bbox with the antimeridian corrected - */ - static translateBBox(bbox) { - logger.debug('Converting bbox from [-180,180] to [0,360]'); - const newBBox = [bbox[0], bbox[1], 360 + bbox[2], bbox[3]]; - return newBBox; - - } - - /** - * @description: Calculates a bbox. - * If a bbox that crosses the antimeridian will be transformed its - * latitudes from [-180, 180] to [0, 360] - * @param {geoStore} geoStore - * @returns {bbox} - * - * */ - static async swapBBox(geoStore) { - const bbox = await turf.bbox(geoStore.geojson); - const antimeridian = await GeoStoreService.antimeridian(bbox); - logger.info(bbox); - return antimeridian ? GeoStoreService.translateBBox(bbox) : bbox; - - } - - /** - * @description: Calculates a bbox. - * If a bbox that crosses the antimeridian will be transformed its - * latitudes from [-180, 180] to [0, 360] - * @param {geoStore} geoStore - * @returns {geoStore} - * - * */ static async calculateBBox(geoStore) { logger.debug('Calculating bbox'); - geoStore.bbox = await GeoStoreService.swapBBox(geoStore); + geoStore.bbox = turf.bbox(geoStore.geojson); await geoStore.save(); return geoStore; } @@ -256,24 +203,18 @@ class GeoStoreService { logger.debug('Repaired geometry', JSON.stringify(geoStore.geojson)); logger.debug('Make Feature Collection'); - geoStore.geojson = GeoJSONConverter.makeFeatureCollection(geoStore.geojson, props); - logger.debug('Result', JSON.stringify(geoStore.geojson)); logger.debug('Creating hash from geojson md5'); - geoStore.hash = md5(JSON.stringify(geoStore.geojson)); - if (geoStore.areaHa === undefined) { geoStore.areaHa = turf.area(geoStore.geojson) / 10000; // convert to ha2 } await GeoStore.findOne({ hash: geoStore.hash }); - logger.debug('bbox geostore'); - logger.debug('geojson', JSON.stringify(geoStore.bbox)); if (!geoStore.bbox) { - geoStore.bbox = await GeoStoreService.swapBBox(geoStore); + geoStore.bbox = turf.bbox(geoStore.geojson); } return GeoStore.findOneAndUpdate({ hash: geoStore.hash }, geoStore, { @@ -304,7 +245,7 @@ class GeoStoreService { geoStore.geojson = GeoJSONConverter.makeFeatureCollection(geoStore.geojson); logger.debug('Result', JSON.stringify(geoStore.geojson)); geoStore.areaHa = turf.area(geoStore.geojson) / 10000; // convert to ha2 - geoStore.bbox = await GeoStoreService.swapBBox(geoStore); // calculate bbox + geoStore.bbox = turf.bbox(geoStore.geojson); return geoStore;