From 8be83045122ed190baab576d6946fb2989b74fdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milan=20P=C3=A1nik?= <5015486+mpanik@users.noreply.github.com> Date: Fri, 27 Feb 2026 13:03:02 -0500 Subject: [PATCH] [86b8jt0u9] Fix pydantic model being passed directly to requests --- dnastack/cli/commands/utils.py | 2 +- dnastack/client/collections/client.py | 2 +- dnastack/client/workbench/base_client.py | 2 +- .../workbench/test_workbench_result_loader.py | 98 +++++++++++++++++++ 4 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 tests/unit/client/workbench/test_workbench_result_loader.py diff --git a/dnastack/cli/commands/utils.py b/dnastack/cli/commands/utils.py index eb08c78c..8b1e7af4 100644 --- a/dnastack/cli/commands/utils.py +++ b/dnastack/cli/commands/utils.py @@ -17,6 +17,6 @@ PAGINATION_PAGE_SIZE_ARG = ArgumentSpec( name='page_size', arg_names=['--page-size'], - help='Set the page size returned by the server.', + help='Set the page size returned by the server. Note: results are iterated through all pages unless --max-results is also specified.', type=int, ) diff --git a/dnastack/client/collections/client.py b/dnastack/client/collections/client.py index ebe5cbbd..4c05a82c 100644 --- a/dnastack/client/collections/client.py +++ b/dnastack/client/collections/client.py @@ -91,7 +91,7 @@ def load(self) -> List[any]: if not self.__next_page_url: response = session.get(current_url, - params=self.__list_options, + params=self.__list_options.model_dump(exclude_none=True), trace_context=self.__trace) else: current_url = self.__next_page_url diff --git a/dnastack/client/workbench/base_client.py b/dnastack/client/workbench/base_client.py index e760d83d..44475857 100644 --- a/dnastack/client/workbench/base_client.py +++ b/dnastack/client/workbench/base_client.py @@ -107,7 +107,7 @@ def load(self) -> List[any]: if not self.__next_page_url: response = session.get(current_url, - params=self.__list_options, + params=self.__list_options.model_dump(exclude_none=True), trace_context=self.__trace) else: current_url = self.__next_page_url diff --git a/tests/unit/client/workbench/test_workbench_result_loader.py b/tests/unit/client/workbench/test_workbench_result_loader.py new file mode 100644 index 00000000..492dd568 --- /dev/null +++ b/tests/unit/client/workbench/test_workbench_result_loader.py @@ -0,0 +1,98 @@ +"""Unit tests for WorkbenchResultLoader parameter serialization""" +import unittest +from unittest.mock import Mock, MagicMock + +from dnastack.client.workbench.base_client import WorkbenchResultLoader +from dnastack.client.workbench.models import BaseListOptions, PaginatedResource, Pagination + + +class StubPaginatedResource(PaginatedResource): + """PaginatedResource subclass that returns an empty items list.""" + data: list = [] + + def items(self): + return self.data + + +class StubResultLoader(WorkbenchResultLoader): + """Concrete subclass for testing, since WorkbenchResultLoader.extract_api_response is abstract-like.""" + + def extract_api_response(self, response_body: dict) -> PaginatedResource: + return StubPaginatedResource( + pagination=Pagination( + next_page_url=response_body.get('next_page_url'), + total_elements=response_body.get('total_elements', 0), + ), + data=response_body.get('items', []), + ) + + +class TestWorkbenchResultLoaderParams(unittest.TestCase): + """Test that WorkbenchResultLoader excludes None values from query params on the first request.""" + + def _make_mock_session(self, response_body: dict): + mock_session = MagicMock() + mock_response = Mock() + mock_response.status_code = 200 + mock_response.text = '{}' + mock_response.json.return_value = response_body + mock_session.__enter__ = Mock(return_value=mock_session) + mock_session.__exit__ = Mock(return_value=False) + mock_session.get.return_value = mock_response + return mock_session + + def test_first_request_excludes_none_params(self): + """params dict sent to session.get() must not contain any None values.""" + mock_session = self._make_mock_session({'next_page_url': None, 'total_elements': 0}) + + list_options = BaseListOptions(page=2, page_size=5) + loader = StubResultLoader( + service_url='https://example.com/api/runs', + http_session=mock_session, + trace=None, + list_options=list_options, + max_results=10, + ) + + loader.load() + + mock_session.get.assert_called_once() + call_kwargs = mock_session.get.call_args + params = call_kwargs.kwargs.get('params') or call_kwargs[1].get('params') + + # Must be a plain dict, not a Pydantic model + self.assertIsInstance(params, dict) + + # No None values allowed + none_keys = [k for k, v in params.items() if v is None] + self.assertEqual(none_keys, [], f"params contains None values for keys: {none_keys}") + + # The explicitly-set values must be present + self.assertEqual(params['page'], 2) + self.assertEqual(params['page_size'], 5) + + # page_token was not set, so it must be absent entirely + self.assertNotIn('page_token', params) + + def test_default_options_produce_empty_params(self): + """When no options are explicitly set, params dict should be empty (all defaults are None).""" + mock_session = self._make_mock_session({'next_page_url': None, 'total_elements': 0}) + + loader = StubResultLoader( + service_url='https://example.com/api/runs', + http_session=mock_session, + trace=None, + list_options=BaseListOptions(), + max_results=10, + ) + + loader.load() + + call_kwargs = mock_session.get.call_args + params = call_kwargs.kwargs.get('params') or call_kwargs[1].get('params') + self.assertIsInstance(params, dict) + self.assertEqual(params, {}) + + +if __name__ == '__main__': + unittest.main()