From 6c782b30b70df84ef3f3e3fb499e5bdc34f16625 Mon Sep 17 00:00:00 2001 From: "J.Ironman" Date: Tue, 1 Sep 2026 14:26:30 +0200 Subject: [PATCH 1/5] key the wms service cache by a slug of the full url --- mslib/utils/service_manager.py | 42 ++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/mslib/utils/service_manager.py b/mslib/utils/service_manager.py index 8c35e6f1d..de7624974 100644 --- a/mslib/utils/service_manager.py +++ b/mslib/utils/service_manager.py @@ -23,6 +23,44 @@ See the License for the specific language governing permissions and limitations under the License. """ +import urllib.parse + +from slugify import slugify + +# query parameters which describe a single request instead of the service itself +OGC_REQUEST_PARAMS = ("service", "request") + + +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 OGC_REQUEST_PARAMS] + 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. + """ + scheme, netloc, path, params, query, fragment = urllib.parse.urlparse(strip_request_params(url)) + query = urllib.parse.urlencode(sorted(urllib.parse.parse_qsl(query))) + normalised = urllib.parse.urlunparse( + (scheme.lower(), netloc.lower(), path, params, query, fragment)) + return slugify(normalised, lowercase=False) class WMSServiceManager: @@ -44,7 +82,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 From c3c211aa1ea9f289aaf5997d133616d801c261e0 Mon Sep 17 00:00:00 2001 From: "J.Ironman" Date: Tue, 1 Sep 2026 14:27:14 +0200 Subject: [PATCH 2/5] build the capabilities base url with urllib instead of str.replace --- mslib/msui/wms_control.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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) From 46dd4d0baf5c704c0a6b5d64bf17bed831390c4e Mon Sep 17 00:00:00 2001 From: "J.Ironman" Date: Wed, 2 Sep 2026 13:12:20 +0000 Subject: [PATCH 3/5] key the wms service cache by the normalized url instead of a slug Slugifying the normalized url collapses all url separators into "-", so distinct services map onto the same key, e.g. "http://a.com:1/wms" and "http://a.com/1/wms" both became "http-a-com-1-wms". The same happened for query parameters versus path elements. The normalized url is a fine dict key on its own, so keep it as is. Co-Authored-By: Claude Opus 5 (1M context) --- mslib/utils/service_manager.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/mslib/utils/service_manager.py b/mslib/utils/service_manager.py index de7624974..5430b3fce 100644 --- a/mslib/utils/service_manager.py +++ b/mslib/utils/service_manager.py @@ -25,8 +25,6 @@ """ import urllib.parse -from slugify import slugify - # query parameters which describe a single request instead of the service itself OGC_REQUEST_PARAMS = ("service", "request") @@ -55,12 +53,16 @@ def service_cache_key(url): 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))) - normalised = urllib.parse.urlunparse( + return urllib.parse.urlunparse( (scheme.lower(), netloc.lower(), path, params, query, fragment)) - return slugify(normalised, lowercase=False) class WMSServiceManager: From 55e1bfcda43e1272e46a96f8c8cc3ac57d75e271 Mon Sep 17 00:00:00 2001 From: "J.Ironman" Date: Wed, 2 Sep 2026 13:12:20 +0000 Subject: [PATCH 4/5] add tests for the wms service cache key Cover the urls which collided when the key was slugified (port versus path element, host boundary, query parameters versus path) as well as the urls which have to share a key (GetCapabilities parameters, order of the remaining parameters, case of scheme and host) and the cache lookups of WMSServiceManager built on them. Co-Authored-By: Claude Opus 5 (1M context) --- tests/_test_utils/test_service_manager.py | 124 ++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 tests/_test_utils/test_service_manager.py 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" From 7d4f6b79b8d27babb1df33a68cc23a42a6853062 Mon Sep 17 00:00:00 2001 From: "J.Ironman" Date: Fri, 4 Sep 2026 09:38:24 +0200 Subject: [PATCH 5/5] removed unnecessary constant --- mslib/utils/service_manager.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mslib/utils/service_manager.py b/mslib/utils/service_manager.py index 5430b3fce..20d7d09d7 100644 --- a/mslib/utils/service_manager.py +++ b/mslib/utils/service_manager.py @@ -25,9 +25,6 @@ """ import urllib.parse -# query parameters which describe a single request instead of the service itself -OGC_REQUEST_PARAMS = ("service", "request") - def strip_request_params(url): """Remove the OGC request parameters (service, request) from an url. @@ -37,7 +34,7 @@ def strip_request_params(url): """ 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 OGC_REQUEST_PARAMS] + if key.lower() not in ("service", "request")] return urllib.parse.urlunparse( (scheme, netloc, path, params, urllib.parse.urlencode(kept), fragment))