From ca507b7a517fc74a06039f95753313d0e0952e20 Mon Sep 17 00:00:00 2001 From: Olivia Guyot Date: Mon, 31 Jul 2023 12:46:19 +0200 Subject: [PATCH] fix(map): remove SERVICE & REQUEST params from wms url If these params are given to OL for the WMS layer then they will most likely cause an error. Added a url utility to remove them in a case-insensitive way. Also added a test for WFS layers --- .../lib/map-context/map-context.fixtures.ts | 7 +++- .../map-context/map-context.service.spec.ts | 37 ++++++++++++++++--- .../lib/map-context/map-context.service.ts | 23 +++++++++--- .../lib/map-view/map-view.component.spec.ts | 2 +- .../src/lib/map-view/map-view.component.ts | 2 +- libs/util/shared/src/lib/utils/index.ts | 1 + libs/util/shared/src/lib/utils/url.spec.ts | 14 +++++++ libs/util/shared/src/lib/utils/url.ts | 20 ++++++++++ tsconfig.base.json | 2 +- 9 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 libs/util/shared/src/lib/utils/url.spec.ts create mode 100644 libs/util/shared/src/lib/utils/url.ts diff --git a/libs/feature/map/src/lib/map-context/map-context.fixtures.ts b/libs/feature/map/src/lib/map-context/map-context.fixtures.ts index a58c32e9d8..db93f19bee 100644 --- a/libs/feature/map/src/lib/map-context/map-context.fixtures.ts +++ b/libs/feature/map/src/lib/map-context/map-context.fixtures.ts @@ -14,9 +14,14 @@ export const MAP_CTX_LAYER_XYZ_FIXTURE: MapContextLayerModel = { } export const MAP_CTX_LAYER_WMS_FIXTURE: MapContextLayerModel = { type: MapContextLayerTypeEnum.WMS, - url: 'https://www.geograndest.fr/geoserver/region-grand-est/ows?', + url: 'https://www.geograndest.fr/geoserver/region-grand-est/ows?REQUEST=GetCapabilities&SERVICE=WMS', name: 'commune_actuelle_3857', } +export const MAP_CTX_LAYER_WFS_FIXTURE: MapContextLayerModel = { + type: MapContextLayerTypeEnum.WFS, + url: 'https://www.geograndest.fr/geoserver/region-grand-est/ows?REQUEST=GetCapabilities&SERVICE=WFS&VERSION=1.1.0', + name: 'ms:commune_actuelle_3857', +} export const MAP_CTX_LAYER_GEOJSON_FIXTURE: MapContextLayerGeojsonModel = { type: MapContextLayerTypeEnum.GEOJSON, data: FEATURE_COLLECTION_POLYGON_FIXTURE_4326, diff --git a/libs/feature/map/src/lib/map-context/map-context.service.spec.ts b/libs/feature/map/src/lib/map-context/map-context.service.spec.ts index 03010359db..6923ed4f86 100644 --- a/libs/feature/map/src/lib/map-context/map-context.service.spec.ts +++ b/libs/feature/map/src/lib/map-context/map-context.service.spec.ts @@ -22,6 +22,7 @@ import { MAP_CTX_FIXTURE, MAP_CTX_LAYER_GEOJSON_FIXTURE, MAP_CTX_LAYER_GEOJSON_REMOTE_FIXTURE, + MAP_CTX_LAYER_WFS_FIXTURE, MAP_CTX_LAYER_WMS_FIXTURE, MAP_CTX_LAYER_XYZ_FIXTURE, } from './map-context.fixtures' @@ -61,6 +62,7 @@ describe('MapContextService', () => { describe('#createLayer', () => { let layerModel, layer + describe('XYZ', () => { beforeEach(() => { layerModel = MAP_CTX_LAYER_XYZ_FIXTURE @@ -83,10 +85,11 @@ describe('MapContextService', () => { ) }) }) + describe('WMS', () => { beforeEach(() => { - layerModel = MAP_CTX_LAYER_WMS_FIXTURE - layer = service.createLayer(layerModel) + ;(layerModel = MAP_CTX_LAYER_WMS_FIXTURE), + (layer = service.createLayer(layerModel)) }) it('create a tile layer', () => { expect(layer).toBeTruthy() @@ -101,11 +104,13 @@ describe('MapContextService', () => { const params = source.getParams() expect(params.LAYERS).toBe(layerModel.name) }) - it('set correct url', () => { + it('set correct url without existing REQUEST and SERVICE params', () => { const source = layer.getSource() const urls = source.getUrls() expect(urls.length).toBe(1) - expect(urls[0]).toEqual(layerModel.url) + expect(urls[0]).toBe( + 'https://www.geograndest.fr/geoserver/region-grand-est/ows' + ) }) it('set WMS gutter of 20px', () => { const source = layer.getSource() @@ -114,6 +119,28 @@ describe('MapContextService', () => { }) }) + describe('WFS', () => { + beforeEach(() => { + ;(layerModel = MAP_CTX_LAYER_WFS_FIXTURE), + (layer = service.createLayer(layerModel)) + }) + it('create a vector layer', () => { + expect(layer).toBeTruthy() + expect(layer).toBeInstanceOf(VectorLayer) + }) + it('create a Vector source', () => { + const source = layer.getSource() + expect(source).toBeInstanceOf(VectorSource) + }) + it('set correct url load function', () => { + const source = layer.getSource() + const urlLoader = source.getUrl() + expect(urlLoader([10, 20, 30, 40])).toBe( + 'https://www.geograndest.fr/geoserver/region-grand-est/ows?service=WFS&version=1.1.0&request=GetFeature&outputFormat=application%2Fjson&typename=ms%3Acommune_actuelle_3857&srsname=EPSG%3A3857&bbox=10%2C20%2C30%2C40%2CEPSG%3A3857' + ) + }) + }) + describe('GEOJSON', () => { describe('with inline data', () => { beforeEach(() => { @@ -288,7 +315,7 @@ describe('MapContextService', () => { const layerWMSUrl = (map.getLayers().item(1) as TileLayer) .getSource() .getUrls()[0] - expect(layerWMSUrl).toEqual('https://some-wms-server') + expect(layerWMSUrl).toEqual('https://some-wms-server/') }) it('add one WFS layer from config on top of baselayer', () => { const layerWFSSource = ( diff --git a/libs/feature/map/src/lib/map-context/map-context.service.ts b/libs/feature/map/src/lib/map-context/map-context.service.ts index 02658d53e1..57cc12c6e6 100644 --- a/libs/feature/map/src/lib/map-context/map-context.service.ts +++ b/libs/feature/map/src/lib/map-context/map-context.service.ts @@ -22,6 +22,7 @@ import { LayerConfig, MapConfig } from '@geonetwork-ui/util/app-config' import { FeatureCollection } from 'geojson' import { fromLonLat } from 'ol/proj' import WMTS from 'ol/source/WMTS' +import { removeSearchParams } from '@geonetwork-ui/util/shared' export const DEFAULT_BASELAYER_CONTEXT: MapContextLayerXyzModel = { type: MapContextLayerTypeEnum.XYZ, @@ -80,7 +81,7 @@ export class MapContextService { case MapContextLayerTypeEnum.WMS: return new TileLayer({ source: new TileWMS({ - url: layerModel.url, + url: removeSearchParams(layerModel.url, ['request', 'service']), params: { LAYERS: layerModel.name }, gutter: 20, }), @@ -94,11 +95,21 @@ export class MapContextService { source: new VectorSource({ format: new GeoJSON(), url: function (extent) { - return `${ - layerModel.url - }?service=WFS&version=1.1.0&request=GetFeature&outputFormat=application/json&typename=${ - layerModel.name - }&srsname=EPSG:3857&bbox=${extent.join(',')},EPSG:3857` + const urlObj = new URL( + removeSearchParams(layerModel.url, [ + 'service', + 'version', + 'request', + ]) + ) + urlObj.searchParams.set('service', 'WFS') + urlObj.searchParams.set('version', '1.1.0') + urlObj.searchParams.set('request', 'GetFeature') + urlObj.searchParams.set('outputFormat', 'application/json') + urlObj.searchParams.set('typename', layerModel.name) + urlObj.searchParams.set('srsname', 'EPSG:3857') + urlObj.searchParams.set('bbox', `${extent.join(',')},EPSG:3857`) + return urlObj.toString() }, strategy: bboxStrategy, }), diff --git a/libs/feature/record/src/lib/map-view/map-view.component.spec.ts b/libs/feature/record/src/lib/map-view/map-view.component.spec.ts index aa404174d4..3604e7ce2f 100644 --- a/libs/feature/record/src/lib/map-view/map-view.component.spec.ts +++ b/libs/feature/record/src/lib/map-view/map-view.component.spec.ts @@ -135,7 +135,7 @@ class OpenLayersMapMock { } } -class InteractionsMock implements Collection {} +class InteractionsMock extends Collection {} class mapManagerMock { map = new OpenLayersMapMock() diff --git a/libs/feature/record/src/lib/map-view/map-view.component.ts b/libs/feature/record/src/lib/map-view/map-view.component.ts index 570064e430..8de472cade 100644 --- a/libs/feature/record/src/lib/map-view/map-view.component.ts +++ b/libs/feature/record/src/lib/map-view/map-view.component.ts @@ -190,7 +190,7 @@ export class MapViewComponent implements OnInit, OnDestroy { })) ) } - return throwError('protocol not supported') + return throwError(() => 'protocol not supported') } selectLinkToDisplay(link: number) { diff --git a/libs/util/shared/src/lib/utils/index.ts b/libs/util/shared/src/lib/utils/index.ts index af7e2f398c..6e8eca5ea2 100644 --- a/libs/util/shared/src/lib/utils/index.ts +++ b/libs/util/shared/src/lib/utils/index.ts @@ -3,3 +3,4 @@ export * from './strip-html' export * from './freeze' export * from './geojson' export * from './atomic-operations' +export * from './url' diff --git a/libs/util/shared/src/lib/utils/url.spec.ts b/libs/util/shared/src/lib/utils/url.spec.ts new file mode 100644 index 0000000000..543962ef52 --- /dev/null +++ b/libs/util/shared/src/lib/utils/url.spec.ts @@ -0,0 +1,14 @@ +import { removeSearchParams } from './url' + +describe('URL utils', () => { + describe('removeSearchParams', () => { + it('removes given search params in a case insensitive way', () => { + expect( + removeSearchParams( + 'http://my.org/abc/?arg0=1234&arg1=aaa&Arg1=111&ARG2=&aRG3=fff&arg4=5678', + ['ARG1', 'arg2', 'arg3'] + ) + ).toEqual('http://my.org/abc/?arg0=1234&arg4=5678') + }) + }) +}) diff --git a/libs/util/shared/src/lib/utils/url.ts b/libs/util/shared/src/lib/utils/url.ts new file mode 100644 index 0000000000..8a5ee6c1de --- /dev/null +++ b/libs/util/shared/src/lib/utils/url.ts @@ -0,0 +1,20 @@ +/** + * Removes the given search params from the URL completely; this is case-insensitive + * @param url + * @param searchParams + */ +export function removeSearchParams( + url: string, + searchParams: string[] +): string { + const toDelete = [] + const urlObj = new URL(url, window.location.toString()) + const keysLower = searchParams.map((p) => p.toLowerCase()) + for (const param of urlObj.searchParams.keys()) { + if (keysLower.indexOf(param.toLowerCase()) > -1) { + toDelete.push(param) + } + } + toDelete.map((param) => urlObj.searchParams.delete(param)) + return urlObj.toString() +} diff --git a/tsconfig.base.json b/tsconfig.base.json index 4779b4035f..2e284c8548 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -10,7 +10,7 @@ "importHelpers": true, "target": "es2020", "module": "esnext", - "lib": ["es2019", "dom"], + "lib": ["es2019", "dom", "dom.iterable"], "skipLibCheck": true, "skipDefaultLibCheck": true, "baseUrl": ".",