From a59af6ed1aec4b2a4f80fa4804a4ffd8414198bf Mon Sep 17 00:00:00 2001
From: "Markus D. Herrmann" <hackermd@users.noreply.github.com>
Date: Fri, 21 Oct 2022 13:27:35 -0400
Subject: [PATCH] Fix parsing of query parameters (#74)

* Add unit test for query parameters

* Fix parsing of query parameters

* Update test requirements
---
 requirements_test.txt      |  4 ++--
 src/dicomweb_client/uri.py | 16 ++++++++--------
 tests/test_web.py          | 12 ++++++++++++
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/requirements_test.txt b/requirements_test.txt
index 2e57663..49b87c1 100644
--- a/requirements_test.txt
+++ b/requirements_test.txt
@@ -1,5 +1,5 @@
-mypy==0.960
-pytest==7.1.2
+mypy==0.982
+pytest==7.1.3
 pytest-cov==3.0.0
 pytest-flake8==1.1.1
 pytest-localserver==0.5.0
diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py
index c979875..1d19600 100644
--- a/src/dicomweb_client/uri.py
+++ b/src/dicomweb_client/uri.py
@@ -50,7 +50,7 @@ def build_query_string(params: Optional[Dict[str, Any]] = None) -> str:
         return ''
     components = []
     for key, value in params.items():
-        if isinstance(value, Sequence):
+        if isinstance(value, (list, tuple, set)):
             for v in value:
                 c = '='.join([key, quote_plus(str(v))])
                 components.append(c)
@@ -95,19 +95,19 @@ def parse_query_parameters(
     params: Dict[str, Union[int, str, List[str]]] = {}
     if limit is not None:
         if not isinstance(limit, int):
-            raise TypeError('Parameter "limit" must be an integer.')
+            raise TypeError('Argument "limit" must be an integer.')
         if limit < 0:
-            raise ValueError('Parameter "limit" must not be negative.')
+            raise ValueError('Argument "limit" must not be negative.')
         params['limit'] = limit
     if offset is not None:
         if not isinstance(offset, int):
-            raise TypeError('Parameter "offset" must be an integer.')
+            raise TypeError('Argument "offset" must be an integer.')
         if offset < 0:
-            raise ValueError('Parameter "offset" must not be negative.')
+            raise ValueError('Argument "offset" must not be negative.')
         params['offset'] = offset
     if fuzzymatching is not None:
         if not isinstance(fuzzymatching, bool):
-            raise TypeError('Parameter "fuzzymatching" must be boolean.')
+            raise TypeError('Argument "fuzzymatching" must be boolean.')
         if fuzzymatching:
             params['fuzzymatching'] = 'true'
         else:
@@ -116,14 +116,14 @@ def parse_query_parameters(
         includefields = []
         for field in set(fields):
             if not isinstance(field, str):
-                raise TypeError('Elements of "fields" must be a string.')
+                raise TypeError('Items of argument "fields" must be strings.')
             includefields.append(field)
         params['includefield'] = includefields
     if search_filters is not None:
         for field, criterion in search_filters.items():
             if not isinstance(field, str):
                 raise TypeError(
-                    'Keys of "search_filters" must be strings.'
+                    'Keys of argument "search_filters" must be strings.'
                 )
             # TODO: datetime?
             params[field] = criterion
diff --git a/tests/test_web.py b/tests/test_web.py
index 3b07c55..56240df 100644
--- a/tests/test_web.py
+++ b/tests/test_web.py
@@ -217,6 +217,18 @@ def test_search_for_series(httpserver, client, cache_dir):
     )
 
 
+def test_search_for_series_filter_modality(httpserver, client, cache_dir):
+    cache_filename = str(cache_dir.joinpath('search_for_series.json'))
+    with open(cache_filename, 'r') as f:
+        content = f.read()
+    headers = {'content-type': 'application/dicom+json'}
+    httpserver.serve_content(content=content, code=200, headers=headers)
+    client.search_for_series(search_filters={'Modality': 'SM'})
+    request = httpserver.requests[0]
+    assert request.path == '/series'
+    assert request.query_string.decode() == 'Modality=SM'
+
+
 def test_search_for_series_of_study(httpserver, client, cache_dir):
     cache_filename = str(cache_dir.joinpath('search_for_series.json'))
     with open(cache_filename, 'r') as f: