enforce parameter validation

Author: burnash

dlt/sources/helpers/rest_client/paginators.py

@@ -109,7 +109,8 @@ def __init__(
                 For example, 'page'.
             param_body_path (str): The dot-separated path specifying
                 where to place the numeric parameter in the request JSON body.
-                Defaults to `None`.
+                Defaults to `None`. Either `param_name` or `param_body_path`
+                must be provided, not both.
             initial_value (int): The initial value of the numeric parameter.
             value_step (int): The step size to increment the numeric parameter.
             base_index (int, optional): The index of the initial element.
@@ -142,6 +143,10 @@ def __init__(
                 "One of `total_path`, `maximum_value`, `has_more_path`, or `stop_after_empty_page`"
                 " must be provided."
             )
+
+        if (not param_name and not param_body_path) or (param_name and param_body_path):
+            raise ValueError("Either 'param_name' or 'param_body_path' must be provided, not both.")
+
         self.param_name = param_name
         self.param_body_path = param_body_path
         self.initial_value = initial_value
@@ -240,6 +245,8 @@ def update_request(self, request: Request) -> None:
     def _update_request_with_param_name(
         request: Request, param_name: str, current_value: int
     ) -> None:
+        if not param_name:
+            raise ValueError("`param_name` must not be empty.")
         if request.params is None:
             request.params = {}
         request.params[param_name] = current_value
@@ -248,6 +255,8 @@ def _update_request_with_param_name(
     def _update_request_with_body_path(
         request: Request, body_path: str, current_value: int
     ) -> None:
+        if not body_path:
+            raise ValueError("`body_path` must not be empty.")
         if request.json is None:
             request.json = {}
         jsonpath.set_value_at_path(request.json, body_path, current_value)
@@ -342,7 +351,8 @@ def __init__(
             page (int): The page number for the first request. If not provided,
                 the initial value will be set to `base_page`.
             page_param (str): The query parameter name for the page number.
-                Defaults to 'page'.
+                Defaults to 'page'. Either `page_param` or `page_body_path`
+                must be provided, not both.
             page_body_path (str): A dot-separated path specifying where
                 to place the page number in the request JSON body. Use this instead
                 of `page_param` when sending the page number in the request body.
@@ -363,8 +373,9 @@ def __init__(
         if page_param is None and page_body_path is None:
             page_param = "page"
         # Check that only one of page_param or page_body_path is provided
-        elif page_param is not None and page_body_path is not None:
+        elif (not page_param and not page_body_path) or (page_param and page_body_path):
             raise ValueError("Either 'page_param' or 'page_body_path' must be provided, not both.")
+
         if (
             total_path is None
             and maximum_page is None
@@ -523,15 +534,15 @@ def __init__(
         if offset_param is None and offset_body_path is None:
             offset_param = "offset"
         # Check that only one of offset_param or offset_body_path is provided
-        if offset_param is not None and offset_body_path is not None:
+        elif (not offset_param and not offset_body_path) or (offset_param and offset_body_path):
             raise ValueError(
                 "Either 'offset_param' or 'offset_body_path' must be provided, not both."
             )
 
         if limit_param is None and limit_body_path is None:
             limit_param = "limit"
         # Check that only one of limit_param or limit_body_path is provided
-        if limit_param is not None and limit_body_path is not None:
+        elif (not limit_param and not limit_body_path) or (limit_param and limit_body_path):
             raise ValueError(
                 "Either 'limit_param' or 'limit_body_path' must be provided, not both."
             )

tests/sources/helpers/rest_client/test_paginators.py

@@ -10,6 +10,7 @@
     SinglePagePaginator,
     OffsetPaginator,
     PageNumberPaginator,
+    RangePaginator,
     HeaderLinkPaginator,
     JSONLinkPaginator,
     JSONResponseCursorPaginator,
@@ -264,6 +265,77 @@ def test_client_pagination(self, rest_client):
         assert_pagination(pages, total_pages=1)
 
 
+class TestRangePaginator:
+    def test_init(self) -> None:
+        # param_name provided, param_body_path not provided
+        RangePaginator(
+            param_name="page",
+            initial_value=0,
+            value_step=1,
+            total_path="total",
+        )
+
+        # param_name not provided, param_body_path provided
+        RangePaginator(
+            param_name=None,
+            param_body_path="body.path",
+            initial_value=0,
+            value_step=1,
+            total_path="total",
+        )
+
+        # both param_name and param_body_path provided
+        with pytest.raises(
+            ValueError, match="Either 'param_name' or 'param_body_path' must be provided, not both"
+        ):
+            RangePaginator(
+                param_name="page",
+                param_body_path="body.path",
+                initial_value=0,
+                value_step=1,
+                total_path="total",
+            )
+
+        # both param_name and param_body_path are None or empty strings
+        for param_name, param_body_path in [
+            (None, None),
+            ("", None),
+            (None, ""),
+            ("", ""),
+        ]:
+            with pytest.raises(
+                ValueError,
+                match="Either 'param_name' or 'param_body_path' must be provided, not both",
+            ):
+                RangePaginator(
+                    param_name=param_name,
+                    param_body_path=param_body_path,
+                    initial_value=0,
+                    value_step=1,
+                    total_path="total",
+                )
+
+        with pytest.raises(
+            ValueError, match="Either 'param_name' or 'param_body_path' must be provided, not both"
+        ):
+            RangePaginator(
+                param_name=None,
+                param_body_path=None,
+                initial_value=0,
+                value_step=1,
+                total_path="total",
+            )
+
+    def test_range_paginator_update(self) -> None:
+        request = Mock(Request)
+        request.json = {}
+        with pytest.raises(ValueError, match="param_name.*must not be empty"):
+            RangePaginator._update_request_with_param_name(request, "", 0)
+
+        with pytest.raises(ValueError, match="body_path.*must not be empty"):
+            RangePaginator._update_request_with_body_path(request, "", 0)
+
+
 @pytest.mark.usefixtures("mock_api_server")
 class TestOffsetPaginator:
     def test_update_state(self):
@@ -499,6 +571,60 @@ def test_guarantee_termination(self):
             " must be provided"
         )
 
+    @pytest.mark.parametrize(
+        "offset_param,offset_body_path",
+        [
+            (None, None),
+            ("", None),
+            (None, ""),
+            ("", ""),
+        ],
+    )
+    def test_empty_offset_params(self, offset_param, offset_body_path) -> None:
+        if offset_param is None and offset_body_path is None:
+            # Fallback to default 'offset' param
+            paginator = OffsetPaginator(
+                limit=100, offset_param=offset_param, offset_body_path=offset_body_path
+            )
+            assert paginator.param_name == "offset"
+            assert paginator.param_body_path is None
+        else:
+            with pytest.raises(
+                ValueError, match="Either 'offset_param' or 'offset_body_path' must be provided.*"
+            ):
+                OffsetPaginator(
+                    limit=100,
+                    offset_param=offset_param,
+                    offset_body_path=offset_body_path,
+                )
+
+    @pytest.mark.parametrize(
+        "limit_param,limit_body_path",
+        [
+            (None, None),
+            ("", None),
+            (None, ""),
+            ("", ""),
+        ],
+    )
+    def test_empty_limit_params(self, limit_param, limit_body_path) -> None:
+        if limit_param is None and limit_body_path is None:
+            # Fallback to default 'limit' param
+            paginator = OffsetPaginator(
+                limit=100, limit_param=limit_param, limit_body_path=limit_body_path
+            )
+            assert paginator.limit_param == "limit"
+            assert paginator.limit_body_path is None
+        else:
+            with pytest.raises(
+                ValueError, match="Either 'limit_param' or 'limit_body_path' must be provided.*"
+            ):
+                OffsetPaginator(
+                    limit=100,
+                    limit_param=limit_param,
+                    limit_body_path=limit_body_path,
+                )
+
 
 @pytest.mark.usefixtures("mock_api_server")
 class TestPageNumberPaginator:
@@ -761,6 +887,30 @@ def test_guarantee_termination(self):
             " must be provided"
         )
 
+    @pytest.mark.parametrize(
+        "page_param,page_body_path",
+        [
+            (None, None),
+            ("", None),
+            (None, ""),
+            ("", ""),
+        ],
+    )
+    def test_empty_params(self, page_param, page_body_path) -> None:
+        if page_param is None and page_body_path is None:
+            # Fallback to default 'page' param
+            paginator = PageNumberPaginator(page_param=page_param, page_body_path=page_body_path)
+            assert paginator.param_name == "page"
+            assert paginator.param_body_path is None
+        else:
+            with pytest.raises(
+                ValueError, match="Either 'page_param' or 'page_body_path' must be provided.*"
+            ):
+                PageNumberPaginator(
+                    page_param=page_param,
+                    page_body_path=page_body_path,
+                )
+
 
 @pytest.mark.usefixtures("mock_api_server")
 class TestJSONResponseCursorPaginator: