Repository: nens/clean-python (language: python)

Issue:
Bug: None values in ApiProvider.request params become string None
When you do an Api request with for instance `params={"foo": None}`, which may happen if you have some optional parameters that internally use None as default, this gets converted into:  `path?foo=None`. Because the string "None" is a Python thing, it doesn't get transformed into None again, but into "None", and unexpected things will happen.

What I would expect is the "foo" query parameter to be completely absent (to avoid confusion with empty strings)


Discussion / hints:

Fix patch:
diff --git a/CHANGES.md b/CHANGES.md
index a920fa5..01bb755 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -7,6 +7,10 @@
 
 - Test against Python 3.13 (bumped asyncpg to `0.30.*` and aiohttp to `3.10.*`).
 
+- Fixed a bug in (Sync)ApiProvider and Gateway: if a query parameter with value `None`
+  is presented, this now hides the query parameter from the generated url. Before,
+  it resulted in `path?foo=None`.
+
 
 ## 0.18.0 (2024-10-21)
 ----------------------
diff --git a/clean_python/api_client/api_provider.py b/clean_python/api_client/api_provider.py
index 6948ebd..3a40c45 100644
--- a/clean_python/api_client/api_provider.py
+++ b/clean_python/api_client/api_provider.py
@@ -76,9 +76,12 @@ def join(url: str, path: str, trailing_slash: bool = False) -> str:
 
 
 def add_query_params(url: str, params: Json | None) -> str:
+    # explicitly filter out None values
     if params is None:
         return url
-    return url + "?" + urlencode(params, doseq=True)
+    params = {k: v for k, v in params.items() if v is not None}
+    query_str = urlencode(params, doseq=True)
+    return url + "?" + query_str if query_str else url
 
 
 class FileFormPost(ValueObject):


Test patch:
diff --git a/tests/api_client/test_api_provider.py b/tests/api_client/test_api_provider.py
index a504212..2d30f2e 100644
--- a/tests/api_client/test_api_provider.py
+++ b/tests/api_client/test_api_provider.py
@@ -112,6 +112,9 @@ async def test_post_json(api_provider: ApiProvider, response, request_m):
         ("bar/", {"a": 2}, "http://testserver/foo/bar?a=2"),
         ("", {"a": [1, 2]}, "http://testserver/foo?a=1&a=2"),
         ("", {"a": 1, "b": "foo"}, "http://testserver/foo?a=1&b=foo"),
+        ("", {"a": None}, "http://testserver/foo"),
+        ("", {"a": ""}, "http://testserver/foo?a="),
+        ("", {"a": []}, "http://testserver/foo"),
     ],
 )
 async def test_url(api_provider: ApiProvider, path, params, expected_url, request_m):
diff --git a/tests/api_client/test_sync_api_provider.py b/tests/api_client/test_sync_api_provider.py
index 59b6a91..eee7aaf 100644
--- a/tests/api_client/test_sync_api_provider.py
+++ b/tests/api_client/test_sync_api_provider.py
@@ -86,6 +86,9 @@ def test_post_json(api_provider: SyncApiProvider, response):
         ("bar/", {"a": 2}, "http://testserver/foo/bar?a=2"),
         ("", {"a": [1, 2]}, "http://testserver/foo?a=1&a=2"),
         ("", {"a": 1, "b": "foo"}, "http://testserver/foo?a=1&b=foo"),
+        ("", {"a": None}, "http://testserver/foo"),
+        ("", {"a": ""}, "http://testserver/foo?a="),
+        ("", {"a": []}, "http://testserver/foo"),
     ],
 )
 def test_url(api_provider: SyncApiProvider, path, params, expected_url):