Skip to content

Commit 5048bc8

Browse files
committed
refactor: simplify pagination logic
1 parent f597eca commit 5048bc8

File tree

6 files changed

+94
-226
lines changed

6 files changed

+94
-226
lines changed

sinch/core/pagination.py

+6-65
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
from abc import ABC, abstractmethod
2-
from collections import namedtuple
32
from typing import Generic
43
from sinch.core.types import BM
54

@@ -50,16 +49,6 @@ async def __anext__(self):
5049
class Paginator(ABC, Generic[BM]):
5150
"""
5251
Pagination response object.
53-
54-
auto_paging_iter method returns an iterator object that can be used for iterator-based page traversing.
55-
For example:
56-
for page in paginated_response.auto_paging_iter():
57-
...process page object
58-
59-
For manual pagination use has_next_page property with next_page() method.
60-
For example:
61-
if paginated_response.has_next_page:
62-
paginated_response = paginated_response.next_page()
6352
"""
6453
def __init__(self, sinch, endpoint, result: BM):
6554
self._sinch = sinch
@@ -81,11 +70,6 @@ def content(self):
8170
def iterator(self):
8271
pass
8372

84-
# TODO: Make get_content() method abstract in Parent class as we implement in the other domains:
85-
# - Refactor pydantic models in other domains to have a content property.
86-
def get_content(self):
87-
pass
88-
8973
@abstractmethod
9074
def next_page(self):
9175
pass
@@ -145,7 +129,7 @@ async def _initialize(cls, sinch, endpoint):
145129
class TokenBasedPaginator(Paginator[BM]):
146130
"""Base paginator for token-based pagination with explicit page navigation and metadata."""
147131

148-
def __init__(self, sinch, endpoint, yield_first_page=False, result=None):
132+
def __init__(self, sinch, endpoint, result=None):
149133
super().__init__(sinch, endpoint, result or sinch.configuration.transport.request(endpoint))
150134

151135
def content(self) -> list[BM]:
@@ -157,9 +141,9 @@ def next_page(self):
157141
return None
158142

159143
self.endpoint.request_data.page_token = self.result.next_page_token
160-
next_result = self._sinch.configuration.transport.request(self.endpoint)
161-
162-
return self.__class__(self._sinch, self.endpoint, result=next_result)
144+
self.result = self._sinch.configuration.transport.request(self.endpoint)
145+
self._calculate_next_page()
146+
return self
163147

164148
def iterator(self):
165149
"""Iterates over individual items across all pages."""
@@ -172,52 +156,14 @@ def iterator(self):
172156
break
173157
paginator = next_page_instance
174158

175-
def get_content(self):
176-
"""Returns structured pagination metadata along with the first page's content (sync)."""
177-
next_page_instance = self.next_page()
178-
return self._get_content(next_page_instance, sync=True)
179-
180-
def _get_content(self, next_page_instance, sync=True):
181-
"""Core logic for `get_content()`, shared between sync and async versions."""
182-
PagedListResponse = namedtuple(
183-
"PagedResponse", ["result", "has_next_page", "next_page_info", "next_page"]
184-
)
185-
186-
next_page_info = {
187-
"result": self.content(),
188-
"result.next": (
189-
self.content() + (next_page_instance.content() if next_page_instance else [])
190-
),
191-
"has_next_page": self.has_next_page,
192-
"has_next_page.next": bool(next_page_instance and next_page_instance.has_next_page),
193-
}
194-
195-
next_page_wrapper = self._get_next_page_wrapper(next_page_instance, sync)
196-
197-
return PagedListResponse(
198-
result=self.content(),
199-
has_next_page=self.has_next_page,
200-
next_page_info=next_page_info,
201-
next_page=next_page_wrapper
202-
)
203-
204-
def _get_next_page_wrapper(self, next_page_instance, sync):
205-
"""Returns a function for fetching the next page."""
206-
if sync:
207-
return lambda: next_page_instance.get_content() if next_page_instance else None
208-
else:
209-
async def async_next_page_wrapper():
210-
return await next_page_instance.get_content() if next_page_instance else None
211-
return async_next_page_wrapper
212-
213159
def _calculate_next_page(self):
214160
self.has_next_page = bool(getattr(self.result, "next_page_token", None))
215161

216162
@classmethod
217163
def _initialize(cls, sinch, endpoint):
218164
"""Creates an instance of the paginator skipping first page."""
219165
result = sinch.configuration.transport.request(endpoint)
220-
return cls(sinch, endpoint, yield_first_page=False, result=result)
166+
return cls(sinch, endpoint, result=result)
221167

222168

223169
class AsyncTokenBasedPaginator(TokenBasedPaginator):
@@ -244,12 +190,7 @@ async def iterator(self):
244190
break
245191
paginator = next_page_instance
246192

247-
async def get_content(self):
248-
"""Returns structured pagination metadata"""
249-
next_page_instance = await self.next_page()
250-
return self._get_content(next_page_instance, sync=False)
251-
252193
@classmethod
253194
async def _initialize(cls, sinch, endpoint):
254195
result = await sinch.configuration.transport.request(endpoint)
255-
return cls(sinch, endpoint, yield_first_page=False, result=result)
196+
return cls(sinch, endpoint, result=result)

sinch/domains/numbers/active_numbers.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ class ActiveNumbersWithAsyncPagination(ActiveNumbers):
136136
async def list(
137137
self,
138138
region_code: StrictStr,
139-
number_type: StrictStr,
139+
number_type: NumberTypeValues,
140140
number_pattern: Optional[StrictStr] = None,
141141
number_search_pattern: Optional[NumberSearchPatternTypeValues] = None,
142142
capabilities: Optional[CapabilityTypeValuesList] = None,

tests/conftest.py

+6-24
Original file line numberDiff line numberDiff line change
@@ -186,30 +186,6 @@ def token_based_pagination_request_data():
186186
)
187187

188188

189-
@pytest.fixture
190-
def first_token_based_pagination_response():
191-
return TokenBasedPaginationResponse(
192-
pig_dogs=["Walaszek", "Połać"],
193-
next_page_token="za30%wsze"
194-
)
195-
196-
197-
@pytest.fixture
198-
def second_token_based_pagination_response():
199-
return TokenBasedPaginationResponse(
200-
pig_dogs=["Bartosz", "Piotr"],
201-
next_page_token="ka#556"
202-
)
203-
204-
205-
@pytest.fixture
206-
def third_token_based_pagination_response():
207-
return TokenBasedPaginationResponse(
208-
pig_dogs=["Madrid", "Spain"],
209-
next_page_token=""
210-
)
211-
212-
213189
@pytest.fixture
214190
def int_based_pagination_request_data():
215191
return IntBasedPaginationRequest(
@@ -334,4 +310,10 @@ def mock_pagination_active_number_responses():
334310
next_page_token="token_2"),
335311
Mock(content=[ActiveNumber(phone_number="+12345678905")],
336312
next_page_token=None)
313+
]
314+
315+
@pytest.fixture
316+
def mock_pagination_expected_phone_numbers_response():
317+
return [
318+
"+12345678901", "+12345678902", "+12345678903", "+12345678904", "+12345678905"
337319
]

tests/e2e/numbers/features/steps/numbers.steps.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,13 @@ def step_when_list_phone_numbers(context):
165165
number_type='LOCAL'
166166
)
167167
# Get the first page
168-
context.response = response.get_content()
168+
context.response = response.content()
169169

170170

171171
@then('the response contains "{count}" phone numbers')
172172
def step_then_response_contains_x_phone_numbers(context, count):
173-
assert len(context.response.result) == int(count), \
174-
f'Expected {count}, got {len(context.response.data)}'
173+
assert len(context.response) == int(count), \
174+
f'Expected {count}, got {len(context.response)}'
175175

176176
@when("I send a request to list all the phone numbers")
177177
def step_when_list_all_phone_numbers(context):

tests/unit/domains/numbers/models/active/requests/test_list_active_numbers_request_model.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def test_list_active_numbers_request_expects_parsed_input():
4343
"number_search_pattern": "START",
4444
"number_pattern": "5678",
4545
"page_token": "abc123",
46-
"order_by": "phoneNumber"
46+
"order_by": "PHONE_NUMBER"
4747
}
4848

4949
request = ListActiveNumbersRequest(**data)

0 commit comments

Comments
 (0)