diff --git a/.gitignore b/.gitignore index c44c3ae7..3246a258 100644 --- a/.gitignore +++ b/.gitignore @@ -141,6 +141,9 @@ poetry.lock # Pyenv .python-version +#CLAUDE +CLAUDE.local.md + # .DS_Store files .DS_Store diff --git a/pyproject.toml b/pyproject.toml index 4e4533f9..289f153f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.poetry] name = "sinch" description = "Sinch SDK for Python programming language" -version = "2.0.0" +version = "2.0.1" license = "Apache 2.0" readme = "README.md" authors = [ diff --git a/sinch/__init__.py b/sinch/__init__.py index 2d00736e..a3df16eb 100644 --- a/sinch/__init__.py +++ b/sinch/__init__.py @@ -1,5 +1,5 @@ """ Sinch Python SDK""" -__version__ = "2.0.0" +__version__ = "2.0.1" from sinch.core.clients.sinch_client_sync import SinchClient diff --git a/sinch/core/pagination.py b/sinch/core/pagination.py index abc17741..326a4c9c 100644 --- a/sinch/core/pagination.py +++ b/sinch/core/pagination.py @@ -1,29 +1,8 @@ from abc import ABC, abstractmethod -from typing import Generic +from typing import Generic, Iterator from sinch.core.types import BM -class PageIterator: - def __init__(self, paginator, yield_first_page=False): - self.paginator = paginator - # If yielding the first page, set started to False - self.started = not yield_first_page - - def __iter__(self): - return self - - def __next__(self): - if not self.started: - self.started = True - return self.paginator - - if self.paginator.has_next_page: - self.paginator = self.paginator.next_page() - return self.paginator - else: - raise StopIteration - - class Paginator(ABC, Generic[BM]): """ Pagination response object. @@ -45,7 +24,7 @@ def content(self): # TODO: Make iterator() method abstract in Parent class as we implement in the other domains: # - Refactor pydantic models in other domains to have a content property. - def iterator(self): + def iterator(self) -> Iterator[BM]: pass @abstractmethod @@ -96,19 +75,29 @@ def iterator(self): paginator = next_page_instance def _calculate_next_page(self): - """Calculates if there's a next page based on count, page, and page_size.""" - if hasattr(self.result, 'count') and hasattr(self.result, 'page'): - # Use the requested page_size from the endpoint - request_page_size = self.endpoint.request_data.page_size or 1 - if request_page_size > 0 and hasattr(self.result, 'page_size'): - # Calculate total pages needed using the request page_size - total_pages = (self.result.count + request_page_size - 1) // request_page_size - # Check if current page is less than total pages - 1 (0-indexed) - self.has_next_page = self.result.page < (total_pages - 1) - else: - self.has_next_page = False - else: + """Calculates if there's a next page based on count, page, and effective page_size.""" + count = getattr(self.result, 'count', None) + page = getattr(self.result, 'page', None) + page_size = getattr(self.result, 'page_size', None) + + if count is None or page is None or page_size is None: + self.has_next_page = False + return + + if not self.content(): self.has_next_page = False + return + + # Cache first response page_size when not provided in order to calculate next pages correctly + request_page_size = self.endpoint.request_data.page_size + if request_page_size is None: + if not hasattr(self, '_first_response_page_size'): + self._first_response_page_size = page_size + request_page_size = self._first_response_page_size + + total_pages = (count + request_page_size - 1) // request_page_size + self.has_next_page = page < (total_pages - 1) + @classmethod def _initialize(cls, sinch, endpoint): diff --git a/tests/conftest.py b/tests/conftest.py index d879c2d8..321893a9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,19 +2,14 @@ import os from dataclasses import dataclass from unittest.mock import Mock, MagicMock -from sinch.domains.sms.models.v1.internal import ( - ListDeliveryReportsRequest, - ListDeliveryReportsResponse, -) import pytest - +from typing import Optional from sinch import SinchClient -from sinch.core.models.base_model import SinchRequestBaseModel from sinch.core.models.http_response import HTTPResponse from sinch.domains.authentication.models.v1.authentication import OAuthToken -from sinch.domains.numbers.models.v1.response import ActiveNumber - +from pydantic import BaseModel +from pydantic import Field, StrictInt def parse_iso_datetime(iso_string): """ @@ -27,14 +22,14 @@ def parse_iso_datetime(iso_string): return datetime.fromisoformat(iso_string) -@dataclass -class IntBasedPaginationRequest(SinchRequestBaseModel): - page: int - page_size: int = 0 +class SMSBasePaginationRequest(BaseModel): + page: Optional[StrictInt] = Field( + default=None) + page_size: Optional[StrictInt] = Field( + default=None) -@dataclass -class TokenBasedPaginationRequest(SinchRequestBaseModel): +class TokenBasedPaginationRequest(BaseModel): page_size: int page_token: str = None @@ -150,20 +145,15 @@ def token_based_pagination_request_data(): @pytest.fixture def sms_pagination_request_data(): - return ListDeliveryReportsRequest( + return SMSBasePaginationRequest( page=0, page_size=2 ) - @pytest.fixture -def third_int_based_pagination_response(): - return ListDeliveryReportsResponse( - count=4, - page=2, - page_size=2, - delivery_reports=[] - ) +def sms_pagination_request_data_with_page_and_page_size_none(): + return SMSBasePaginationRequest() + @pytest.fixture @@ -266,14 +256,24 @@ def mock_sinch_client_conversation(): @pytest.fixture def mock_pagination_active_number_responses(): return [ - Mock(content=[ActiveNumber(phone_number="+12345678901"), - ActiveNumber(phone_number="+12345678902")], - next_page_token="token_1"), - Mock(content=[ActiveNumber(phone_number="+12345678903"), - ActiveNumber(phone_number="+12345678904")], - next_page_token="token_2"), - Mock(content=[ActiveNumber(phone_number="+12345678905")], - next_page_token=None) + Mock( + content=[ + Mock(phone_number="+12345678901"), + Mock(phone_number="+12345678902"), + ], + next_page_token="token_1", + ), + Mock( + content=[ + Mock(phone_number="+12345678903"), + Mock(phone_number="+12345678904"), + ], + next_page_token="token_2", + ), + Mock( + content=[Mock(phone_number="+12345678905")], + next_page_token=None, + ), ] @@ -286,50 +286,22 @@ def mock_pagination_expected_phone_numbers_response(): @pytest.fixture def mock_sms_pagination_responses(): - from datetime import datetime - from sinch.domains.sms.models.v1.response import RecipientDeliveryReport - return [ - Mock(content=[ - RecipientDeliveryReport( - at=parse_iso_datetime("2025-10-19T16:45:31.935Z"), - batch_id="01K7YNS82JMYGAKAATHFP0QTB5", - code=400, - recipient="12346836075", - status="DELIVERED", - type="recipient_delivery_report_sms" - ), - RecipientDeliveryReport( - at=parse_iso_datetime("2025-10-19T16:40:26.855Z"), - batch_id="01K7YNFY30DS2KKVQZVBFANHMR", - code=400, - recipient="12346836075", - status="DELIVERED", - type="recipient_delivery_report_sms" - ) - ], - count=4, page=0, page_size=2), - Mock(content=[ - RecipientDeliveryReport( - at=parse_iso_datetime("2025-10-19T16:35:15.123Z"), - batch_id="01K7YNGZ45XW8KKPQRSTUVWXYZ", - code=401, - recipient="34683607595", - status="DISPATCHED", - type="recipient_delivery_report_sms" - ), - RecipientDeliveryReport( - at=parse_iso_datetime("2025-10-19T16:30:10.456Z"), - batch_id="01K7YNHM67YZ3LMNOPQRSTUVWX", - code=402, - recipient="34683607596", - status="FAILED", - type="recipient_delivery_report_sms" - ) - ], - count=4, page=1, page_size=2), - Mock(content=[], - count=4, page=2, page_size=2) + Mock( + content=[ + Mock(batch_id="01K7YNS82JMYGAKAATHFP0QTB5"), + Mock(batch_id="01K7YNFY30DS2KKVQZVBFANHMR"), + ], + count=4, page=0, page_size=2, + ), + Mock( + content=[ + Mock(batch_id="01K7YNGZ45XW8KKPQRSTUVWXYZ"), + Mock(batch_id="01K7YNHM67YZ3LMNOPQRSTUVWX"), + ], + count=4, page=1, page_size=2, + ), + Mock(content=[], count=4, page=2, page_size=0), ] diff --git a/tests/unit/test_pagination.py b/tests/unit/test_pagination.py index 2dfa938b..10a795cd 100644 --- a/tests/unit/test_pagination.py +++ b/tests/unit/test_pagination.py @@ -4,6 +4,7 @@ SMSPaginator, TokenBasedPaginator ) +from tests.conftest import SMSBasePaginationRequest # Helper function to initialize SMS paginator @@ -12,7 +13,7 @@ def initialize_sms_paginator(endpoint_mock, request_data, responses): # Create a mock that returns different responses based on page number def mock_request(endpoint): - page = endpoint.request_data.page + page = endpoint.request_data.page or 0 if page == 0: return responses[0] elif page == 1: @@ -25,6 +26,104 @@ def mock_request(endpoint): return SMSPaginator(sinch=client, endpoint=endpoint_mock) +def test_page_size_is_zero(): + request_data = SMSBasePaginationRequest(page=0) + response = Mock(count=0, page=0, page_size=0, content=[]) + client = Mock() + client.configuration.transport.request.return_value = response + endpoint = Mock(request_data=request_data) + + paginator = SMSPaginator(sinch=client, endpoint=endpoint) + + assert paginator.has_next_page is False + +def test_response_without_page_size(): + request_data = SMSBasePaginationRequest(page=0) + response = Mock(count=1, page=0, page_size=None, content=[Mock()]) + client = Mock() + client.configuration.transport.request.return_value = response + endpoint = Mock(request_data=request_data) + + paginator = SMSPaginator(sinch=client, endpoint=endpoint) + + assert paginator.has_next_page is False + + +def test_partial_last_page_does_not_trigger_extra_call(): + """Regression: when the last page is partial (response.page_size smaller than + the first response's page_size), the paginator must not request a further + empty page after the last real one.""" + request_data = SMSBasePaginationRequest() + responses_by_page = { + None: Mock(content=[Mock()] * 30, count=49, page=0, page_size=30), + 1: Mock(content=[Mock()] * 19, count=49, page=1, page_size=19), + } + client = Mock() + client.configuration.transport.request.side_effect = ( + lambda ep: responses_by_page[ep.request_data.page] + ) + endpoint = Mock(request_data=request_data) + + paginator = SMSPaginator(sinch=client, endpoint=endpoint) + list(paginator.iterator()) + + assert client.configuration.transport.request.call_count == 2 + +def test_stop_on_first_page(): + """Regression: when the first page is already the last one, the paginator must not make an extra call.""" + request_data = SMSBasePaginationRequest() + responses_by_page = { + None: Mock(content=[Mock()] * 15, count=15, page=0, page_size=15), + } + client = Mock() + client.configuration.transport.request.side_effect = ( + lambda ep: responses_by_page[ep.request_data.page] + ) + endpoint = Mock(request_data=request_data) + + paginator = SMSPaginator(sinch=client, endpoint=endpoint) + list(paginator.iterator()) + + assert client.configuration.transport.request.call_count == 1 + + +def test_explicit_page_size_with_mid_stream_start_stops_in_one_call(): + """When page_size is passed explicitly and (page+1)*page_size >= count, the + paginator must stop without making an extra empty call.""" + request_data = SMSBasePaginationRequest(page=1, page_size=30) + response = Mock(content=[Mock()] * 19, count=49, page=1, page_size=19) + client = Mock() + client.configuration.transport.request.return_value = response + endpoint = Mock(request_data=request_data) + + paginator = SMSPaginator(sinch=client, endpoint=endpoint) + + assert paginator.has_next_page is False + assert client.configuration.transport.request.call_count == 1 + + +def test_mid_stream_without_page_size_makes_one_extra_call(): + """Known edge case: starting mid-stream without an explicit page_size can't + distinguish a partial last page from a full small page, so the paginator + makes one extra (empty) request before stopping. This test documents the + inevitable behavior so future changes don't accidentally break it.""" + request_data = SMSBasePaginationRequest(page=1) + responses_by_page = { + 1: Mock(content=[Mock()] * 19, count=49, page=1, page_size=19), + 2: Mock(content=[], count=49, page=2, page_size=0), + } + client = Mock() + client.configuration.transport.request.side_effect = ( + lambda ep: responses_by_page[ep.request_data.page] + ) + endpoint = Mock(request_data=request_data) + + paginator = SMSPaginator(sinch=client, endpoint=endpoint) + list(paginator.iterator()) + + assert client.configuration.transport.request.call_count == 2 + + def test_page_sms_iterator_sync_using_manual_pagination( sms_pagination_request_data,