diff --git a/mslib/msui/wms_control.py b/mslib/msui/wms_control.py index aad330f22..144ac2096 100644 --- a/mslib/msui/wms_control.py +++ b/mslib/msui/wms_control.py @@ -53,7 +53,7 @@ from mslib.utils.qt import Worker from mslib.msui.multilayers import Multilayers, Layer import mslib.utils.ogcwms as ogcwms -from mslib.utils.service_manager import WMSServiceManager +from mslib.utils.service_manager import WMSServiceManager, strip_request_params from mslib.utils.time import parse_iso_datetime, parse_iso_duration from mslib.utils.auth import save_password_to_keyring, get_auth_from_url_and_name from mslib.utils.config import modify_config_file @@ -988,11 +988,8 @@ def get_capabilities(self, level=None): def on_success(request): self.cpdlg.setValue(5) - # url shortener url translated - url = request.url - - url = url.replace("?service=WMS", "").replace("&service=WMS", "") \ - .replace("?request=GetCapabilities", "").replace("&request=GetCapabilities", "") + # url shortener url translated, service specific parameters are kept + url = strip_request_params(request.url) logging.debug("requesting capabilities from %s", url) self.initialise_wms(url, None, level=level) diff --git a/mslib/utils/service_manager.py b/mslib/utils/service_manager.py index 8c35e6f1d..20d7d09d7 100644 --- a/mslib/utils/service_manager.py +++ b/mslib/utils/service_manager.py @@ -23,6 +23,43 @@ See the License for the specific language governing permissions and limitations under the License. """ +import urllib.parse + + +def strip_request_params(url): + """Remove the OGC request parameters (service, request) from an url. + + All other query parameters are kept in their original order, so the result + can be used as base url for further requests to the same service. + """ + scheme, netloc, path, params, query, fragment = urllib.parse.urlparse(url) + kept = [(key, value) for key, value in urllib.parse.parse_qsl(query) + if key.lower() not in ("service", "request")] + return urllib.parse.urlunparse( + (scheme, netloc, path, params, urllib.parse.urlencode(kept), fragment)) + + +def service_cache_key(url): + """Build the cache key of a WMS service from its full url. + + In contrast to using only the base url, the query parameters are part of + the key. Services which differ solely in their parameters, e.g. + "https://example.com/wms?dataset=a", therefore get separate cache entries. + + The parameters of the GetCapabilities request itself are dropped and the + remaining ones are sorted, so that the very same service is found again no + matter how the request was spelled. Scheme and host are lower cased because + they are case insensitive, the rest of the url is not. + + The key is the normalized url itself. It must not be reduced any further, + e.g. by slugifying it, because that would collapse the url separators and + map distinct services onto the same key, for example + "http://example.com:1/wms" and "http://example.com/1/wms". + """ + scheme, netloc, path, params, query, fragment = urllib.parse.urlparse(strip_request_params(url)) + query = urllib.parse.urlencode(sorted(urllib.parse.parse_qsl(query))) + return urllib.parse.urlunparse( + (scheme.lower(), netloc.lower(), path, params, query, fragment)) class WMSServiceManager: @@ -44,7 +81,7 @@ def clear_cache(self): self._cache.clear() def get_service(self, url): - return self._cache.get(url) + return self._cache.get(service_cache_key(url)) def cache_service(self, url, wms): - self._cache[url] = wms + self._cache[service_cache_key(url)] = wms diff --git a/tests/_test_utils/test_service_manager.py b/tests/_test_utils/test_service_manager.py new file mode 100644 index 000000000..fb6c279f6 --- /dev/null +++ b/tests/_test_utils/test_service_manager.py @@ -0,0 +1,124 @@ +# -*- coding: utf-8 -*- +""" + + tests._test_utils.test_service_manager + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + This module provides pytest functions to tests mslib.utils.service_manager + + This file is part of MSS. + + :copyright: Copyright 2025 Reimar Bauer + :copyright: Copyright 2025-2026 by the MSS team, see AUTHORS. + :license: APACHE-2.0, see LICENSE for details. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +""" +import pytest + +from mslib.utils.service_manager import WMSServiceManager, service_cache_key, strip_request_params + + +class TestStripRequestParams: + @pytest.mark.parametrize("url,expected", [ + ("http://example.com/wms", "http://example.com/wms"), + ("http://example.com/wms?service=WMS&request=GetCapabilities", "http://example.com/wms"), + ("http://example.com/wms?SERVICE=WMS&REQUEST=GetCapabilities", "http://example.com/wms"), + ("http://example.com/wms?dataset=a&service=WMS", "http://example.com/wms?dataset=a"), + # other parameters keep their original order + ("http://example.com/wms?b=2&a=1", "http://example.com/wms?b=2&a=1"), + ]) + def test_strip_request_params(self, url, expected): + assert strip_request_params(url) == expected + + +class TestServiceCacheKey: + @pytest.mark.parametrize("url,other", [ + # a port is not the same as a path element + ("http://example.com:1/wms", "http://example.com/1/wms"), + ("https://example.com:8080/wms", "https://example.com/8080/wms"), + # nor is a host boundary the same as a path separator + ("http://example.com/wms", "http://example/com/wms"), + # different hosts, same words + ("http://a.example.com/wms", "http://a-example-com/wms"), + # parameters must not collapse into the path either + ("http://example.com/wms?dataset=a", "http://example.com/wms/dataset/a"), + ("http://example.com/wms?dataset=a&layer=b", "http://example.com/wms?dataset=a-layer-b"), + # a parameter value separator is not a parameter separator + ("http://example.com/wms?dataset=a&layer=b", "http://example.com/wms?dataset=a%26layer%3Db"), + ]) + def test_distinct_urls_get_distinct_keys(self, url, other): + assert service_cache_key(url) != service_cache_key(other) + + @pytest.mark.parametrize("url,other", [ + # the GetCapabilities request itself is not part of the service + ("http://example.com/wms", + "http://example.com/wms?service=WMS&request=GetCapabilities"), + ("http://example.com/wms?dataset=a", + "http://example.com/wms?service=WMS&dataset=a&request=GetCapabilities"), + # parameter order does not matter + ("http://example.com/wms?dataset=a&layer=b", + "http://example.com/wms?layer=b&dataset=a"), + # scheme and host are case insensitive + ("http://example.com/wms", "HTTP://Example.COM/wms"), + ("http://example.com:1/wms", "HTTP://Example.COM:1/wms"), + ]) + def test_equivalent_urls_share_a_key(self, url, other): + assert service_cache_key(url) == service_cache_key(other) + + @pytest.mark.parametrize("url,other", [ + # the path is case sensitive + ("http://example.com/wms", "http://example.com/WMS"), + # so are parameter names and values + ("http://example.com/wms?dataset=a", "http://example.com/wms?dataset=A"), + ("http://example.com/wms?dataset=a", "http://example.com/wms?DATASET=a"), + # a scheme is not just a name + ("http://example.com/wms", "https://example.com/wms"), + ]) + def test_case_and_scheme_are_kept(self, url, other): + assert service_cache_key(url) != service_cache_key(other) + + +class TestWMSServiceManager: + @pytest.fixture(autouse=True) + def _cache(self): + manager = WMSServiceManager() + manager.clear_cache() + yield + manager.clear_cache() + + def test_cache_and_get_service(self): + manager = WMSServiceManager() + assert manager.get_service("http://example.com/wms") is None + manager.cache_service("http://example.com/wms", "wms") + assert manager.get_service("http://example.com/wms") == "wms" + + def test_get_service_by_equivalent_url(self): + manager = WMSServiceManager() + manager.cache_service("http://example.com/wms?dataset=a", "wms") + assert manager.get_service( + "http://example.com/wms?request=GetCapabilities&dataset=a&service=WMS") == "wms" + + @pytest.mark.parametrize("url,other", [ + ("http://example.com:1/wms", "http://example.com/1/wms"), + ("http://example.com/wms?dataset=a", "http://example.com/wms?dataset=b"), + ("http://example.com/wms?dataset=a", "http://example.com/wms"), + ]) + def test_distinct_services_do_not_share_an_entry(self, url, other): + manager = WMSServiceManager() + manager.cache_service(url, "wms") + assert manager.get_service(other) is None + + def test_shared_cache_between_instances(self): + WMSServiceManager().cache_service("http://example.com/wms", "wms") + assert WMSServiceManager().get_service("http://example.com/wms") == "wms"