Skip to content

Commit 51a40ab

Browse files
sufyankhanraosufyankhanrao
committed
resolved PR comments after 1st review
1 parent 0853604 commit 51a40ab

10 files changed

Lines changed: 79 additions & 67 deletions

File tree

‎apimatic_core/pagination/paginated_data.py‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def __init__(self, api_call, paginated_items_converter):
4747
if paginated_items_converter is None:
4848
raise ValueError('paginated_items_converter cannot be None')
4949

50-
self._api_call = copy.deepcopy(api_call)
50+
self._api_call = api_call
5151
self._paginated_items_converter = paginated_items_converter
5252
self._initial_request_builder = api_call.request_builder
5353
self._last_request_builder = None
@@ -67,7 +67,7 @@ def __iter__(self):
6767
"""
6868
Returns a new independent iterator instance for paginated data traversal.
6969
"""
70-
return self._get_new_self_instance()
70+
return self.clone()
7171

7272
def __next__(self):
7373
"""
@@ -98,7 +98,7 @@ def pages(self):
9898
Generator yielding HTTP response objects for each page.
9999
"""
100100
# Create a new instance so the page iteration is independent
101-
paginated_data = self._get_new_self_instance()
101+
paginated_data = self.clone()
102102

103103
while True:
104104
paginated_data._paged_response = paginated_data._fetch_next_page()
@@ -118,7 +118,7 @@ def _fetch_next_page(self):
118118
Returns an empty list if no further pages are available.
119119
120120
Returns:
121-
The processed response object for the next page, or an empty list if pagination is complete.
121+
The processed response object for the next page, or None if no pagination strategy is applicable.
122122
123123
Raises:
124124
Exception: Propagates any exceptions encountered during the API call execution.
@@ -137,15 +137,10 @@ def _fetch_next_page(self):
137137

138138
return None
139139

140-
def _get_new_self_instance(self):
140+
def clone(self):
141141
"""
142-
Creates and returns a new independent PaginatedData instance with the initial
143-
request builder and item converter.
144-
145-
Returns:
146-
PaginatedData: A fresh iterator instance for independent pagination traversal.
142+
Creates and returns a new independent PaginatedData instance
143+
with a cloned API call that resets to the initial request builder.
147144
"""
148-
return PaginatedData(
149-
self._api_call.clone(request_builder=self._initial_request_builder),
150-
self._paginated_items_converter
151-
)
145+
cloned_api_call = self._api_call.clone(request_builder=self._initial_request_builder)
146+
return PaginatedData(cloned_api_call, self._paginated_items_converter)

‎apimatic_core/pagination/pagination_strategy.py‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ class PaginationStrategy(ABC):
1212
and update request builders with new pagination parameters based on JSON pointers.
1313
"""
1414

15+
PATH_PARAMS_IDENTIFIER = "$request.path"
16+
QUERY_PARAMS_IDENTIFIER = "$request.query"
17+
HEADER_PARAMS_IDENTIFIER = "$request.headers"
18+
1519
def __init__(self, metadata_wrapper):
1620
"""
1721
Initializes the PaginationStrategy with the provided metadata wrapper.
@@ -72,13 +76,13 @@ def get_updated_request_builder(request_builder, input_pointer, offset):
7276
query_params = request_builder.query_params
7377
header_params = request_builder.header_params
7478

75-
if path_prefix == "$request.path":
79+
if path_prefix == PaginationStrategy.PATH_PARAMS_IDENTIFIER:
7680
template_params = ApiHelper.update_entry_by_json_pointer(
77-
template_params.copy(), field_path, offset, inplace=True)
78-
elif path_prefix == "$request.query":
81+
template_params.copy(), f"{field_path}/value", offset, inplace=True)
82+
elif path_prefix == PaginationStrategy.QUERY_PARAMS_IDENTIFIER:
7983
query_params = ApiHelper.update_entry_by_json_pointer(
8084
query_params.copy(), field_path, offset, inplace=True)
81-
elif path_prefix == "$request.headers":
85+
elif path_prefix == PaginationStrategy.HEADER_PARAMS_IDENTIFIER:
8286
header_params = ApiHelper.update_entry_by_json_pointer(
8387
header_params.copy(), field_path, offset, inplace=True)
8488

@@ -100,13 +104,14 @@ def _get_initial_request_param_value(request_builder, input_pointer, default=0):
100104
"""
101105
path_prefix, field_path = ApiHelper.split_into_parts(input_pointer)
102106

103-
if path_prefix == "$request.path":
104-
value = ApiHelper.get_value_by_json_pointer(request_builder.template_params, field_path)
107+
if path_prefix == PaginationStrategy.PATH_PARAMS_IDENTIFIER:
108+
value = ApiHelper.get_value_by_json_pointer(
109+
request_builder.template_params, f"{field_path}/value")
105110
return int(value) if value is not None else default
106-
elif path_prefix == "$request.query":
111+
elif path_prefix == PaginationStrategy.QUERY_PARAMS_IDENTIFIER:
107112
value = ApiHelper.get_value_by_json_pointer(request_builder.query_params, field_path)
108113
return int(value) if value is not None else default
109-
elif path_prefix == "$request.headers":
114+
elif path_prefix == PaginationStrategy.HEADER_PARAMS_IDENTIFIER:
110115
value = ApiHelper.get_value_by_json_pointer(request_builder.header_params, field_path)
111116
return int(value) if value is not None else default
112117

‎apimatic_core/pagination/strategies/cursor_pagination.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,13 @@ def _get_initial_cursor_value(request_builder, input_pointer):
9696
"""
9797
path_prefix, field_path = ApiHelper.split_into_parts(input_pointer)
9898

99-
if path_prefix == "$request.path":
100-
return ApiHelper.get_value_by_json_pointer(request_builder.template_params, field_path)
101-
elif path_prefix == "$request.query":
99+
if path_prefix == PaginationStrategy.PATH_PARAMS_IDENTIFIER:
100+
value = ApiHelper.get_value_by_json_pointer(
101+
request_builder.template_params, f"{field_path}/value")
102+
return value if value is not None else None
103+
elif path_prefix == PaginationStrategy.QUERY_PARAMS_IDENTIFIER:
102104
return ApiHelper.get_value_by_json_pointer(request_builder.query_params, field_path)
103-
elif path_prefix == "$request.headers":
105+
elif path_prefix == PaginationStrategy.HEADER_PARAMS_IDENTIFIER:
104106
return ApiHelper.get_value_by_json_pointer(request_builder.header_params, field_path)
105107

106108
return None

‎apimatic_core/utilities/api_helper.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# -*- coding: utf-8 -*-
22
from collections import abc
33
import re
4+
import copy
45
import datetime
56
import calendar
67
import email.utils as eut
@@ -779,7 +780,7 @@ def split_into_parts(json_pointer):
779780
tuple: A tuple containing the path prefix and the field path. Returns None if input is None.
780781
"""
781782
if json_pointer is None or json_pointer == '':
782-
return None
783+
return None, None
783784

784785
pointer_parts = json_pointer.split("#")
785786
path_prefix = pointer_parts[0]
@@ -802,7 +803,6 @@ def update_entry_by_json_pointer(dictionary, pointer, value, inplace=True):
802803
dict: The updated dictionary.
803804
"""
804805
if not inplace:
805-
import copy
806806
dictionary = copy.deepcopy(dictionary)
807807

808808
parts = pointer.strip("/").split("/")

‎tests/apimatic_core/pagination_tests/strategies/test_cursor_pagination.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ def mock_metadata_wrapper(self, mocker):
1515
@pytest.fixture
1616
def mock_request_builder(self, mocker):
1717
rb = mocker.Mock(spec=RequestBuilder)
18-
rb.template_params = {"cursor": "initial_path_cursor"}
18+
rb.template_params = {
19+
"cursor": { "value": "initial_path_cursor", "encode": True}
20+
}
1921
rb.query_params = {"cursor": "initial_query_cursor"}
2022
rb.header_params = {"cursor": "initial_header_cursor"}
2123
return rb
@@ -154,16 +156,20 @@ def test_apply_metadata_wrapper(self, mock_metadata_wrapper, mocker):
154156

155157
# Test _get_initial_cursor_value
156158
def test_get_initial_cursor_value_path(self, mocker, mock_request_builder):
157-
mock_split_into_parts = mocker.patch.object(ApiHelper, 'split_into_parts', return_value=("$request.path", "/path_cursor"))
158-
mock_get_value_by_json_pointer = mocker.patch.object(ApiHelper, 'get_value_by_json_pointer', return_value="initial_path_cursor")
159+
mock_split_into_parts = mocker.patch.object(
160+
ApiHelper, 'split_into_parts', return_value=(PaginationStrategy.PATH_PARAMS_IDENTIFIER, "/path_cursor"))
161+
mock_get_value_by_json_pointer = mocker.patch.object(
162+
ApiHelper, 'get_value_by_json_pointer', return_value="initial_path_cursor")
159163

160-
result = CursorPagination._get_initial_cursor_value(mock_request_builder, "$request.path#/cursor")
164+
result = CursorPagination._get_initial_cursor_value(
165+
mock_request_builder, "$request.path#/cursor")
161166
mock_split_into_parts.assert_called_once_with("$request.path#/cursor")
162-
mock_get_value_by_json_pointer.assert_called_once_with(mock_request_builder.template_params, "/path_cursor")
167+
mock_get_value_by_json_pointer.assert_called_once_with(
168+
mock_request_builder.template_params, "/path_cursor/value")
163169
assert result == "initial_path_cursor"
164170

165171
def test_get_initial_cursor_value_query(self, mocker, mock_request_builder):
166-
mock_split_into_parts = mocker.patch.object(ApiHelper, 'split_into_parts', return_value=("$request.query", "/cursor"))
172+
mock_split_into_parts = mocker.patch.object(ApiHelper, 'split_into_parts', return_value=(PaginationStrategy.QUERY_PARAMS_IDENTIFIER, "/cursor"))
167173
mock_get_value_by_json_pointer = mocker.patch.object(ApiHelper, 'get_value_by_json_pointer', return_value="initial_query_cursor")
168174

169175
result = CursorPagination._get_initial_cursor_value(mock_request_builder, "$request.query#/cursor")
@@ -172,7 +178,7 @@ def test_get_initial_cursor_value_query(self, mocker, mock_request_builder):
172178
assert result == "initial_query_cursor"
173179

174180
def test_get_initial_cursor_value_headers(self, mocker, mock_request_builder):
175-
mock_split_into_parts = mocker.patch.object(ApiHelper, 'split_into_parts', return_value=("$request.headers", "/cursor"))
181+
mock_split_into_parts = mocker.patch.object(ApiHelper, 'split_into_parts', return_value=(PaginationStrategy.HEADER_PARAMS_IDENTIFIER, "/cursor"))
176182
mock_get_value_by_json_pointer = mocker.patch.object(ApiHelper, 'get_value_by_json_pointer', return_value="initial_header_cursor")
177183

178184
result = CursorPagination._get_initial_cursor_value(mock_request_builder, "$request.headers#/cursor")

‎tests/apimatic_core/pagination_tests/strategies/test_offset_pagination.py‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22

33
from apimatic_core.pagination.paginated_data import PaginatedData
4+
from apimatic_core.pagination.pagination_strategy import PaginationStrategy
45
from apimatic_core.utilities.api_helper import ApiHelper
56
from apimatic_core.request_builder import RequestBuilder
67
from apimatic_core.pagination.strategies.offset_pagination import OffsetPagination
@@ -47,7 +48,7 @@ def clone_with(self, **kwargs):
4748
return new_rb
4849

4950
rb = MockRequestBuilder(
50-
template_params={"offset": 5},
51+
template_params={"offset": {"value" : 5, "encode": True}},
5152
query_params={"offset": 10, "limit": 20},
5253
header_params={"offset": 15}
5354
)
@@ -143,7 +144,7 @@ def test_apply_metadata_wrapper(self, mock_metadata_wrapper, mocker):
143144
@pytest.mark.parametrize(
144145
"input_pointer, initial_params, expected_value, json_pointer_return_value",
145146
[
146-
("$request.path#/offset", {"offset": 50}, 50, "50"),
147+
("$request.path#/offset", {"offset": {"value" : 5, "encode": True}}, 50, "50"),
147148
("$request.query#/offset", {"offset": 100, "limit": 20}, 100, "100"),
148149
("$request.headers#/offset", {"offset": 200}, 200, "200"),
149150
("$request.query#/offset", {"limit": 20}, 0, None), # No value found
@@ -153,11 +154,11 @@ def test_apply_metadata_wrapper(self, mock_metadata_wrapper, mocker):
153154
def test_get_initial_offset_various_scenarios(self, mocker, mock_request_builder, mock_metadata_wrapper,
154155
input_pointer, initial_params, expected_value, json_pointer_return_value):
155156
# Dynamically set params based on the test case
156-
if "$request.path" in input_pointer:
157+
if PaginationStrategy.PATH_PARAMS_IDENTIFIER in input_pointer:
157158
mock_request_builder._template_params = initial_params
158-
elif "$request.query" in input_pointer:
159+
elif PaginationStrategy.QUERY_PARAMS_IDENTIFIER in input_pointer:
159160
mock_request_builder._query_params = initial_params
160-
elif "$request.headers" in input_pointer:
161+
elif PaginationStrategy.HEADER_PARAMS_IDENTIFIER in input_pointer:
161162
mock_request_builder._header_params = initial_params
162163

163164
# Mocks
@@ -172,17 +173,18 @@ def test_get_initial_offset_various_scenarios(self, mocker, mock_request_builder
172173
mock_split_into_parts.assert_called_once_with(input_pointer)
173174

174175
# Assertions for mock calls based on prefix validity
175-
if input_pointer.startswith(("$request.path", "$request.query", "$request.headers")):
176+
if input_pointer.startswith((PaginationStrategy.PATH_PARAMS_IDENTIFIER, PaginationStrategy.QUERY_PARAMS_IDENTIFIER, PaginationStrategy.HEADER_PARAMS_IDENTIFIER)):
176177
# Determine which params dict was accessed
177-
accessed_params = None
178-
if "$request.path" in input_pointer:
178+
if PaginationStrategy.PATH_PARAMS_IDENTIFIER in input_pointer:
179179
accessed_params = mock_request_builder.template_params
180-
elif "$request.query" in input_pointer:
180+
mock_get_value_by_json_pointer.assert_called_once_with(
181+
accessed_params, f"{input_pointer.split('#')[1]}/value")
182+
elif PaginationStrategy.QUERY_PARAMS_IDENTIFIER in input_pointer:
181183
accessed_params = mock_request_builder.query_params
182-
elif "$request.headers" in input_pointer:
184+
mock_get_value_by_json_pointer.assert_called_once_with(accessed_params, input_pointer.split('#')[1])
185+
elif PaginationStrategy.HEADER_PARAMS_IDENTIFIER in input_pointer:
183186
accessed_params = mock_request_builder.header_params
184-
185-
mock_get_value_by_json_pointer.assert_called_once_with(accessed_params, input_pointer.split('#')[1])
187+
mock_get_value_by_json_pointer.assert_called_once_with(accessed_params, input_pointer.split('#')[1])
186188
else:
187189
mock_get_value_by_json_pointer.assert_not_called()
188190

‎tests/apimatic_core/pagination_tests/strategies/test_page_pagination.py‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22

33
from apimatic_core.pagination.paginated_data import PaginatedData
4+
from apimatic_core.pagination.pagination_strategy import PaginationStrategy
45
from apimatic_core.pagination.strategies.page_pagination import PagePagination
56
from apimatic_core.utilities.api_helper import ApiHelper
67
from apimatic_core.request_builder import RequestBuilder
@@ -167,7 +168,7 @@ def test_apply_metadata_wrapper(self, mock_metadata_wrapper, mocker):
167168
@pytest.mark.parametrize(
168169
"input_pointer, initial_params, expected_value, json_pointer_return_value",
169170
[
170-
("$request.path#/page", {"page": 2}, 2, "2"),
171+
("$request.path#/page", {"page": {"value": 2, "encoded": True}}, 2, "2"),
171172
("$request.query#/page", {"page": 3, "limit": 10}, 3, "3"),
172173
("$request.headers#/page", {"page": 4}, 4, "4"),
173174
("$request.query#/page", {"limit": 10}, 1, None),
@@ -176,11 +177,11 @@ def test_apply_metadata_wrapper(self, mock_metadata_wrapper, mocker):
176177
)
177178
def test_get_initial_page_offset_various_scenarios(self, mocker, mock_request_builder, mock_metadata_wrapper,
178179
input_pointer, initial_params, expected_value, json_pointer_return_value):
179-
if "$request.path" in input_pointer:
180+
if PaginationStrategy.PATH_PARAMS_IDENTIFIER in input_pointer:
180181
mock_request_builder._template_params = initial_params
181-
elif "$request.query" in input_pointer:
182+
elif PaginationStrategy.QUERY_PARAMS_IDENTIFIER in input_pointer:
182183
mock_request_builder._query_params = initial_params
183-
elif "$request.headers" in input_pointer:
184+
elif PaginationStrategy.HEADER_PARAMS_IDENTIFIER in input_pointer:
184185
mock_request_builder._header_params = initial_params
185186

186187
mock_split_into_parts = mocker.patch.object(ApiHelper, 'split_into_parts',
@@ -193,16 +194,17 @@ def test_get_initial_page_offset_various_scenarios(self, mocker, mock_request_bu
193194

194195
mock_split_into_parts.assert_called_once_with(input_pointer)
195196

196-
if input_pointer.startswith(("$request.path", "$request.query", "$request.headers")):
197-
accessed_params = None
198-
if "$request.path" in input_pointer:
197+
if input_pointer.startswith((PaginationStrategy.PATH_PARAMS_IDENTIFIER, PaginationStrategy.QUERY_PARAMS_IDENTIFIER, PaginationStrategy.HEADER_PARAMS_IDENTIFIER)):
198+
if PaginationStrategy.PATH_PARAMS_IDENTIFIER in input_pointer:
199199
accessed_params = mock_request_builder.template_params
200-
elif "$request.query" in input_pointer:
200+
mock_get_value_by_json_pointer.assert_called_once_with(
201+
accessed_params, f"{input_pointer.split('#')[1]}/value")
202+
elif PaginationStrategy.QUERY_PARAMS_IDENTIFIER in input_pointer:
201203
accessed_params = mock_request_builder.query_params
202-
elif "$request.headers" in input_pointer:
204+
mock_get_value_by_json_pointer.assert_called_once_with(accessed_params, input_pointer.split('#')[1])
205+
elif PaginationStrategy.HEADER_PARAMS_IDENTIFIER in input_pointer:
203206
accessed_params = mock_request_builder.header_params
204-
205-
mock_get_value_by_json_pointer.assert_called_once_with(accessed_params, input_pointer.split('#')[1])
207+
mock_get_value_by_json_pointer.assert_called_once_with(accessed_params, input_pointer.split('#')[1])
206208
else:
207209
mock_get_value_by_json_pointer.assert_not_called()
208210

‎tests/apimatic_core/pagination_tests/test_paginated_data.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ def test_pages_yields_paginated_responses(self, mocker, mock_paginated_data_inst
128128
# Mock _get_new_self_instance (private method)
129129
# Create a new mock instance for the independent iterator used by pages()
130130
pages_internal_paginated_data = mocker.Mock(spec=PaginatedData)
131-
mocker.patch.object(PaginatedData, '_get_new_self_instance', return_value=pages_internal_paginated_data)
131+
mocker.patch.object(PaginatedData, 'clone', return_value=pages_internal_paginated_data)
132132

133133
# Configure the *internal* mock instance's _fetch_next_page
134134
pages_internal_paginated_data._fetch_next_page = mocker.Mock(
@@ -170,7 +170,7 @@ def test_pages_returns_empty_generator_if_no_initial_items(self, mocker, mock_pa
170170
mock_http_response_empty_items):
171171
# Mock _get_new_self_instance (private method)
172172
pages_internal_paginated_data = mocker.Mock(spec=PaginatedData)
173-
mocker.patch.object(PaginatedData, '_get_new_self_instance', return_value=pages_internal_paginated_data)
173+
mocker.patch.object(PaginatedData, 'clone', return_value=pages_internal_paginated_data)
174174

175175
# Configure the *internal* mock instance's _fetch_next_page to return an empty items response immediately
176176
pages_internal_paginated_data._fetch_next_page = mocker.Mock(return_value=mock_http_response_empty_items)

0 commit comments

Comments
 (0)