From 358f4434384f7307c5364f72dacfba5c33128aac Mon Sep 17 00:00:00 2001 From: Florent Gravin Date: Tue, 21 Nov 2023 22:12:59 +0100 Subject: [PATCH 1/4] chore: add geojson-validation package in order to valide geojson, for instance when used in ES search payload --- package-lock.json | 9 +++++++++ package.json | 1 + 2 files changed, 10 insertions(+) diff --git a/package-lock.json b/package-lock.json index dcf4d57768..8b5effd162 100644 --- a/package-lock.json +++ b/package-lock.json @@ -49,6 +49,7 @@ "duration-relativetimeformat": "^2.0.3", "embla-carousel": "^8.0.0-rc14", "express": "^4.17.1", + "geojson-validation": "^1.0.2", "moment": "^2.29.4", "ng-table-virtual-scroll": "^1.4.1", "ngx-chips": "3.0.0", @@ -19198,6 +19199,14 @@ "node": ">=6.9.0" } }, + "node_modules/geojson-validation": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/geojson-validation/-/geojson-validation-1.0.2.tgz", + "integrity": "sha512-K5jrJ4wFvORn2pRKeg181LL0QPYuEKn2KHPvfH1m2QtFlAXFLKdseqt0XwBM3ELOY7kNM1fglRQ6ZwUQZ5S00A==", + "bin": { + "gjv": "bin/gjv" + } + }, "node_modules/geotiff": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/geotiff/-/geotiff-2.0.4.tgz", diff --git a/package.json b/package.json index 9c986c5a71..72a8dc2c56 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "duration-relativetimeformat": "^2.0.3", "embla-carousel": "^8.0.0-rc14", "express": "^4.17.1", + "geojson-validation": "^1.0.2", "moment": "^2.29.4", "ng-table-virtual-scroll": "^1.4.1", "ngx-chips": "3.0.0", From d8f4aa4a01de06a0347f2784994ba8f6a032bcef Mon Sep 17 00:00:00 2001 From: Florent Gravin Date: Tue, 21 Nov 2023 22:14:11 +0100 Subject: [PATCH 2/4] fix(search): handle filter geometry issue ATM if the geometry is not valid, the search crashes as it's passer the ES query. This ensure the geometry is valid, and use null if not --- libs/feature/search/src/lib/state/effects.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/libs/feature/search/src/lib/state/effects.ts b/libs/feature/search/src/lib/state/effects.ts index d175ee3782..8b1fcf7ac0 100644 --- a/libs/feature/search/src/lib/state/effects.ts +++ b/libs/feature/search/src/lib/state/effects.ts @@ -1,7 +1,7 @@ import { Inject, Injectable, Optional } from '@angular/core' import { Actions, createEffect, ofType } from '@ngrx/effects' import { select, Store } from '@ngrx/store' -import { buffer, combineLatestWith, debounceTime, from, of } from 'rxjs' +import { buffer, combineLatestWith, debounceTime, from, of, tap } from 'rxjs' import { catchError, map, @@ -45,6 +45,7 @@ import { FILTER_GEOMETRY } from '../feature-search.module' import { RecordsRepositoryInterface } from '@geonetwork-ui/common/domain/repository/records-repository.interface' import { FavoritesService } from '@geonetwork-ui/api/repository/gn4' import { PlatformServiceInterface } from '@geonetwork-ui/common/domain/platform.service.interface' +import { valid as validGeoJson } from 'geojson-validation' @Injectable() export class SearchEffects { @@ -128,8 +129,17 @@ export class SearchEffects { return of([state, favorites, null]) } return this.filterGeometry$.pipe( + tap((geom) => { + const isValid = validGeoJson(geom) + if (!isValid) { + throw '\nFilter geometry is not a valid GeoJson' + } + }), map((geom) => [state, favorites, geom]), - catchError(() => of([state, favorites, null])) // silently opt out of spatial filter if an error happens + catchError((e) => { + console.warn('The filter geometry cannot be used', e) + return of([state, favorites, null]) + }) // silently opt out of spatial filter if an error happens ) }), switchMap( From 650eab0226183ac5513ea8df958e2318a5d135c6 Mon Sep 17 00:00:00 2001 From: Florent Gravin Date: Wed, 29 Nov 2023 15:56:17 +0100 Subject: [PATCH 3/4] fix(geom-filter): warn user only if the geometry if not valid while previous code was warning even if the geometry service returns an error --- libs/feature/search/src/lib/state/effects.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/libs/feature/search/src/lib/state/effects.ts b/libs/feature/search/src/lib/state/effects.ts index 8b1fcf7ac0..9fc1641b50 100644 --- a/libs/feature/search/src/lib/state/effects.ts +++ b/libs/feature/search/src/lib/state/effects.ts @@ -130,14 +130,21 @@ export class SearchEffects { } return this.filterGeometry$.pipe( tap((geom) => { - const isValid = validGeoJson(geom) - if (!isValid) { - throw '\nFilter geometry is not a valid GeoJson' + try { + const trace = validGeoJson(geom, true) as string[] + if (trace?.length > 0) { + throw new Error(trace.join('\n')) + } + } catch (error) { + console.warn( + 'Error while parsing the geometry filter\n', + error + ) + throw new Error() } }), map((geom) => [state, favorites, geom]), catchError((e) => { - console.warn('The filter geometry cannot be used', e) return of([state, favorites, null]) }) // silently opt out of spatial filter if an error happens ) From 69c9d3ff475716f94105c6241b735e9fd2bfbbf5 Mon Sep 17 00:00:00 2001 From: Florent Gravin Date: Wed, 29 Nov 2023 18:18:40 +0100 Subject: [PATCH 4/4] feat(geom-filter): add tests --- .../search/src/lib/state/effects.spec.ts | 39 +++++++++++++++++++ libs/feature/search/src/lib/state/effects.ts | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/libs/feature/search/src/lib/state/effects.spec.ts b/libs/feature/search/src/lib/state/effects.spec.ts index cce5831230..6ac2e6a211 100644 --- a/libs/feature/search/src/lib/state/effects.spec.ts +++ b/libs/feature/search/src/lib/state/effects.spec.ts @@ -444,6 +444,45 @@ describe('Effects', () => { }) ) }) + describe('when geometry is broken', () => { + beforeEach(() => { + effects['filterGeometry$'] = of({ + type: 'Polygon', + coordinates: [[]], + }) + effects = TestBed.inject(SearchEffects) + actions$ = of(new RequestMoreResults('main')) + }) + it('skips the geometry in the search', async () => { + await firstValueFrom(effects.loadResults$) + expect(repository.search).toHaveBeenCalledWith( + expect.not.objectContaining({ + filterGeometry: { type: 'Polygon', coordinates: [[]] }, + }) + ) + }) + }) + describe('when geometry is invalid', () => { + beforeEach(() => { + effects['filterGeometry$'] = of({ + type: 'Polygon', + coordinates: [ + [0, 1], + [0, 1], + ], + }) as any + effects = TestBed.inject(SearchEffects) + actions$ = of(new RequestMoreResults('main')) + }) + it('skips the geometry in the search', async () => { + await firstValueFrom(effects.loadResults$) + expect(repository.search).toHaveBeenCalledWith( + expect.not.objectContaining({ + filterGeometry: { type: 'Polygon', coordinates: [[]] }, + }) + ) + }) + }) }) describe('when useSpatialFilter is disabled', () => { beforeEach(() => { diff --git a/libs/feature/search/src/lib/state/effects.ts b/libs/feature/search/src/lib/state/effects.ts index 9fc1641b50..e649346dab 100644 --- a/libs/feature/search/src/lib/state/effects.ts +++ b/libs/feature/search/src/lib/state/effects.ts @@ -146,7 +146,7 @@ export class SearchEffects { map((geom) => [state, favorites, geom]), catchError((e) => { return of([state, favorites, null]) - }) // silently opt out of spatial filter if an error happens + }) ) }), switchMap(