From 0bfa3ff367a3b0be063ed86e951269a35b916206 Mon Sep 17 00:00:00 2001 From: MV88 Date: Fri, 29 Nov 2024 18:54:02 +0100 Subject: [PATCH 1/6] Fix #10701 enabling failures for wms requests with exception, for ImageWMS sources --- .../map/openlayers/__tests__/Layer-test.jsx | 31 +++++++++++++++++++ .../map/openlayers/plugins/WMSLayer.js | 10 ++++-- web/client/utils/VectorTileUtils.js | 4 +++ .../utils/__tests__/VectorTileUtils-test.js | 13 +++++++- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/web/client/components/map/openlayers/__tests__/Layer-test.jsx b/web/client/components/map/openlayers/__tests__/Layer-test.jsx index e6f8a26af4..931f33d617 100644 --- a/web/client/components/map/openlayers/__tests__/Layer-test.jsx +++ b/web/client/components/map/openlayers/__tests__/Layer-test.jsx @@ -320,6 +320,37 @@ describe('Openlayers layer', () => { expect(layer).toBeTruthy(); expect(map.getLayers().getLength()).toBe(1); }); + it.only('render wms singleTile layer with error', (done) => { + mockAxios.onGet().reply(r => { + expect(r.url.indexOf('SAMPLE_URL') >= 0 ).toBeTruthy(); + return [200, "\n" + + "\n" + + " \n" + + " msWFSGetFeature(): WFS server error. Invalid GetFeature Request\n" + + " \n" + + ""]; + }); + const options = { + type: 'wms', + visibility: true, + singleTile: true, + url: 'SAMPLE_URL', + name: 'osm:vector_tile' + }; + const layer = ReactDOM.render(, document.getElementById("container")); + expect(layer.layer.getSource()).toBeTruthy(); + layer.layer.getSource().on('imageloaderror', (e)=> { + setTimeout(() => { + expect(e).toBeTruthy(); + done(); + }, 200); + }); + }); it('creates a tiled wms layer for openlayers map with long url', (done) => { let options = { "type": "wms", diff --git a/web/client/components/map/openlayers/plugins/WMSLayer.js b/web/client/components/map/openlayers/plugins/WMSLayer.js index 20d3e3fb5c..f5531088c0 100644 --- a/web/client/components/map/openlayers/plugins/WMSLayer.js +++ b/web/client/components/map/openlayers/plugins/WMSLayer.js @@ -29,7 +29,7 @@ import TileWMS from 'ol/source/TileWMS'; import VectorTileSource from 'ol/source/VectorTile'; import VectorTileLayer from 'ol/layer/VectorTile'; -import { isVectorFormat } from '../../../../utils/VectorTileUtils'; +import { isVectorFormat, isValidResponse } from '../../../../utils/VectorTileUtils'; import { OL_VECTOR_FORMATS, applyStyle } from '../../../../utils/openlayers/VectorTileUtils'; import { proxySource, getWMSURLs, wmsToOpenlayersOptions, toOLAttributions, generateTileGrid } from '../../../../utils/openlayers/WMSUtils'; @@ -74,10 +74,14 @@ const loadFunction = (options, headers) => function(image, src) { headers, responseType: 'blob' }).then(response => { - if (response.status === 200 && response.data) { + if (isValidResponse(response)) { image.getImage().src = URL.createObjectURL(response.data); } else { - console.error("Status code: " + response.status); + // #10701 this is needed to trigger the imageloaderror event + // in ol otherwise this event is not triggered if you assign + // the xml content of the exception to the src attribute + image.getImage().src = null; + console.error("error: " + response.data); } }).catch(e => { console.error(e); diff --git a/web/client/utils/VectorTileUtils.js b/web/client/utils/VectorTileUtils.js index 09cbfc8803..d874e7ac0a 100644 --- a/web/client/utils/VectorTileUtils.js +++ b/web/client/utils/VectorTileUtils.js @@ -13,3 +13,7 @@ export const VECTOR_FORMATS = [ ]; export const isVectorFormat = (format) => VECTOR_FORMATS.indexOf(format) !== -1; + +export const isValidResponse = (response) => { + return response?.status === 200 && response?.data && response?.data?.type !== "text/xml"; +}; diff --git a/web/client/utils/__tests__/VectorTileUtils-test.js b/web/client/utils/__tests__/VectorTileUtils-test.js index 26c0de691a..4ecdf57146 100644 --- a/web/client/utils/__tests__/VectorTileUtils-test.js +++ b/web/client/utils/__tests__/VectorTileUtils-test.js @@ -7,7 +7,10 @@ */ import expect from 'expect'; -import { isVectorFormat } from '../VectorTileUtils'; +import { + isValidResponse, + isVectorFormat +} from '../VectorTileUtils'; describe('VectorTileUtils', () => { it('test isVectorFormat with vector formats', () => { @@ -33,4 +36,12 @@ describe('VectorTileUtils', () => { const GIF = 'image/gif'; expect(isVectorFormat(GIF)).toBe(false); }); + it('test isValidResponse', () => { + // invalid responses + expect(isValidResponse({data: {type: "text/xml"}})).toBeFalsy(); + expect(isValidResponse({data: {type: "blob"}})).toBeFalsy(); + expect(isValidResponse({data: {type: "blob"}, status: 401})).toBeFalsy(); + // valid responses + expect(isValidResponse({data: {type: "blob"}, status: 200})).toBeTruthy(); + }); }); From 453a5f7bce9277238c267722496d2162fe8b86ec Mon Sep 17 00:00:00 2001 From: MV88 Date: Mon, 2 Dec 2024 10:12:26 +0100 Subject: [PATCH 2/6] fix test lint --- web/client/components/map/openlayers/__tests__/Layer-test.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/client/components/map/openlayers/__tests__/Layer-test.jsx b/web/client/components/map/openlayers/__tests__/Layer-test.jsx index 931f33d617..2803d28e26 100644 --- a/web/client/components/map/openlayers/__tests__/Layer-test.jsx +++ b/web/client/components/map/openlayers/__tests__/Layer-test.jsx @@ -320,7 +320,7 @@ describe('Openlayers layer', () => { expect(layer).toBeTruthy(); expect(map.getLayers().getLength()).toBe(1); }); - it.only('render wms singleTile layer with error', (done) => { + it('render wms singleTile layer with error', (done) => { mockAxios.onGet().reply(r => { expect(r.url.indexOf('SAMPLE_URL') >= 0 ).toBeTruthy(); return [200, "\n" + From a574965664f695e84159d1c472525296f315083d Mon Sep 17 00:00:00 2001 From: MV88 Date: Tue, 3 Dec 2024 15:42:49 +0100 Subject: [PATCH 3/6] move isValidResponse to WMSUtils adding docs --- .../components/map/openlayers/plugins/WMSLayer.js | 3 ++- web/client/utils/WMSUtils.js | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/web/client/components/map/openlayers/plugins/WMSLayer.js b/web/client/components/map/openlayers/plugins/WMSLayer.js index f5531088c0..7cc6320cbc 100644 --- a/web/client/components/map/openlayers/plugins/WMSLayer.js +++ b/web/client/components/map/openlayers/plugins/WMSLayer.js @@ -29,7 +29,8 @@ import TileWMS from 'ol/source/TileWMS'; import VectorTileSource from 'ol/source/VectorTile'; import VectorTileLayer from 'ol/layer/VectorTile'; -import { isVectorFormat, isValidResponse } from '../../../../utils/VectorTileUtils'; +import { isVectorFormat } from '../../../../utils/VectorTileUtils'; +import { isValidResponse } from '../../../../utils/WMSUtils'; import { OL_VECTOR_FORMATS, applyStyle } from '../../../../utils/openlayers/VectorTileUtils'; import { proxySource, getWMSURLs, wmsToOpenlayersOptions, toOLAttributions, generateTileGrid } from '../../../../utils/openlayers/WMSUtils'; diff --git a/web/client/utils/WMSUtils.js b/web/client/utils/WMSUtils.js index 81439c2de4..9acd8a510b 100644 --- a/web/client/utils/WMSUtils.js +++ b/web/client/utils/WMSUtils.js @@ -46,6 +46,19 @@ export const isValidGetMapFormat = (format) => { export const isValidGetFeatureInfoFormat = (format) => { return getDefaultSupportedGetFeatureInfoFormats().includes(format); }; + +/** + * Validate GetMap response from WMS ImageWMS. + * "OGC protocol returns status = 200 with Exception in body, + * this function checks if the exception is contained in the response". + * https://docs.geoserver.org/main/en/user/services/wms/reference.html#exceptions + * @param {object} response + * @return {boolean} + */ +export const isValidResponse = (response) => { + return response?.status === 200 && response?.data && response?.data?.type !== "text/xml"; +}; + /** * Parses layer info from capabilities object * @param {object} capabilities capabilities section of the layer as an object from xml2js parsing of the WMS capabilities From 6f625eb7a9fa40d3f53124e1e764b300038f047c Mon Sep 17 00:00:00 2001 From: MV88 Date: Wed, 4 Dec 2024 18:36:59 +0100 Subject: [PATCH 4/6] update test --- web/client/utils/__tests__/VectorTileUtils-test.js | 9 --------- web/client/utils/__tests__/WMSUtils-test.js | 11 ++++++++++- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/web/client/utils/__tests__/VectorTileUtils-test.js b/web/client/utils/__tests__/VectorTileUtils-test.js index 4ecdf57146..b5b9ab6ce4 100644 --- a/web/client/utils/__tests__/VectorTileUtils-test.js +++ b/web/client/utils/__tests__/VectorTileUtils-test.js @@ -8,7 +8,6 @@ import expect from 'expect'; import { - isValidResponse, isVectorFormat } from '../VectorTileUtils'; @@ -36,12 +35,4 @@ describe('VectorTileUtils', () => { const GIF = 'image/gif'; expect(isVectorFormat(GIF)).toBe(false); }); - it('test isValidResponse', () => { - // invalid responses - expect(isValidResponse({data: {type: "text/xml"}})).toBeFalsy(); - expect(isValidResponse({data: {type: "blob"}})).toBeFalsy(); - expect(isValidResponse({data: {type: "blob"}, status: 401})).toBeFalsy(); - // valid responses - expect(isValidResponse({data: {type: "blob"}, status: 200})).toBeTruthy(); - }); }); diff --git a/web/client/utils/__tests__/WMSUtils-test.js b/web/client/utils/__tests__/WMSUtils-test.js index 1784efd574..c72a0f2e5e 100644 --- a/web/client/utils/__tests__/WMSUtils-test.js +++ b/web/client/utils/__tests__/WMSUtils-test.js @@ -12,7 +12,8 @@ import { isValidGetFeatureInfoFormat, getLayerOptions, getTileGridFromLayerOptions, - getCustomTileGridProperties + getCustomTileGridProperties, + isValidResponse } from '../WMSUtils'; describe('Test the WMSUtils', () => { @@ -96,4 +97,12 @@ describe('Test the WMSUtils', () => { } }); }); + it('test isValidResponse', () => { + // invalid responses + expect(isValidResponse({data: {type: "text/xml"}})).toBeFalsy(); + expect(isValidResponse({data: {type: "blob"}})).toBeFalsy(); + expect(isValidResponse({data: {type: "blob"}, status: 401})).toBeFalsy(); + // valid responses + expect(isValidResponse({data: {type: "blob"}, status: 200})).toBeTruthy(); + }); }); From 5ede051417ebd0c8e37e37407a1114f838443a9b Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Wed, 4 Dec 2024 18:39:41 +0100 Subject: [PATCH 5/6] Update web/client/utils/VectorTileUtils.js --- web/client/utils/VectorTileUtils.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/web/client/utils/VectorTileUtils.js b/web/client/utils/VectorTileUtils.js index d874e7ac0a..09cbfc8803 100644 --- a/web/client/utils/VectorTileUtils.js +++ b/web/client/utils/VectorTileUtils.js @@ -13,7 +13,3 @@ export const VECTOR_FORMATS = [ ]; export const isVectorFormat = (format) => VECTOR_FORMATS.indexOf(format) !== -1; - -export const isValidResponse = (response) => { - return response?.status === 200 && response?.data && response?.data?.type !== "text/xml"; -}; From c1ca8313aeb488c0933c5dfab42eceb798843ffb Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Wed, 4 Dec 2024 18:40:20 +0100 Subject: [PATCH 6/6] Apply suggestions from code review --- web/client/utils/__tests__/VectorTileUtils-test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/web/client/utils/__tests__/VectorTileUtils-test.js b/web/client/utils/__tests__/VectorTileUtils-test.js index b5b9ab6ce4..26c0de691a 100644 --- a/web/client/utils/__tests__/VectorTileUtils-test.js +++ b/web/client/utils/__tests__/VectorTileUtils-test.js @@ -7,9 +7,7 @@ */ import expect from 'expect'; -import { - isVectorFormat -} from '../VectorTileUtils'; +import { isVectorFormat } from '../VectorTileUtils'; describe('VectorTileUtils', () => { it('test isVectorFormat with vector formats', () => {