diff --git a/CHANGELOG.md b/CHANGELOG.md index 6aebfd05..b3785f9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,9 @@ # Change Log +## [4.63.0](https://github.com/plivo/plivo-python/tree/v4.63.0) (2026-09-23) +**Bug Fix - Auto-pagination under cursor pagination** +- Fixed `__iter__` looping forever on Call and Recording list endpoints when the API ignores a client-supplied `offset`. Iteration now follows `meta.next`, reading whichever of `cursor=` or `offset=` it carries, and falls back to the offset walk for responses without `meta`. +- Added optional `cursor` parameter to `calls.list()` and `recordings.list()`. Appended as a trailing optional argument; existing `offset` callers are unaffected. + ## [4.62.0](https://github.com/plivo/plivo-python/tree/v4.62.0) (2026-07-28) **Feature - Toll-free verification terms, privacy, opt-in and help fields** - Added optional `terms_and_conditions_link`, `privacy_policy_link`, `optin_message` and `help_message` parameters to the toll-free verification create and update methods diff --git a/plivo/base.py b/plivo/base.py index 393fe809..f929c42e 100644 --- a/plivo/base.py +++ b/plivo/base.py @@ -7,6 +7,76 @@ from plivo.exceptions import InvalidRequestError +try: + from urllib.parse import parse_qs, urlparse +except ImportError: + from urlparse import parse_qs, urlparse + +DEFAULT_PAGE_LIMIT = 20 + +# Outcomes of _next_page_params() that aren't a params dict. +_END_OF_PAGES = object() # meta says there is no next page +_NO_PAGE_HINT = object() # response carried no meta to follow + + +def _attr(obj, key): + """Read a key off a response fragment, dict or ResponseObject alike.""" + if isinstance(obj, dict): + return obj.get(key) + # getattr, not []: ResponseObject.__getitem__ falls back to + # self.objects on a miss, which is not what we want here. + return getattr(obj, key, None) + + +def _next_page_params(response, limit): + """Derive the next page's request params from a list response's meta. + + meta.next is a relative URL whose query string carries either an + opaque cursor or a plain offset, depending on which pagination mode + the deployment happens to be serving: + + '/v1/.../Call/?limit=20&cursor=' cursor / hybrid + '/v1/.../Call/?limit=20&offset=20' offset (legacy) + + Reading whichever param is present is what lets one walk stay correct + in all three modes without the SDK knowing which one is live. + + Returns a params dict, or _END_OF_PAGES / _NO_PAGE_HINT. + """ + meta = _attr(response, 'meta') + if meta is None: + return _NO_PAGE_HINT + + next_url = _attr(meta, 'next') + if not next_url: + return _END_OF_PAGES + + query = parse_qs(urlparse(str(next_url)).query) + + next_limit = query.get('limit', [None])[0] + if next_limit is not None: + try: + limit = int(next_limit) + except ValueError: + pass + + cursor = query.get('cursor', [None])[0] + if cursor: + # A cursor supersedes offset, and offset must not ride along with + # it: under hybrid the server still applies a supplied offset, + # which would pin the walk to page 1 forever. + return {'limit': limit, 'cursor': cursor, 'offset': None} + + offset = query.get('offset', [None])[0] + if offset is not None: + try: + return {'limit': limit, 'offset': int(offset)} + except ValueError: + return _END_OF_PAGES + + # meta.next is there but carries neither param we understand. + return _NO_PAGE_HINT + class Meta: def __init__(self): @@ -338,20 +408,50 @@ def __init__(self, client, **kwargs): self.client = client def __iter__(self): - if not getattr(self, 'list') or not self.__class__._iterable: + """Walks every page of this resource, one record at a time. + + Follows meta.next rather than computing its own offsets, so the + walk stays correct whether the deployment applies offset, ignores + it in favour of cursors, or serves the hybrid of the two. Falls + back to incrementing offset only for responses that carry no meta. + """ + if not getattr(self, 'list', None) or not self.__class__._iterable: raise NotImplementedError( 'list is not supported for this resource') def gen(): - limit = 20 - offset = 0 + limit = DEFAULT_PAGE_LIMIT + params = {'limit': limit, 'offset': 0} + while True: - response = self.list(limit=limit, offset=offset) - if not response.objects: + response = self.list(**params) + if not _attr(response, 'objects'): return for item in response: yield item - offset += limit + + next_params = _next_page_params(response, limit) + + if next_params is _END_OF_PAGES: + return + + if next_params is _NO_PAGE_HINT: + # No usable meta. If we were walking by offset we can + # carry on and stop on the first empty page; if we + # were following a cursor there is nothing to follow. + if params.get('offset') is None: + return + next_params = dict(params) + next_params['offset'] += limit + + if next_params == params: + # Never re-request the page we just consumed. A server + # that ignores our paging params would otherwise hand + # back page 1 forever. + return + + params = next_params + limit = params.get('limit') or limit return gen() diff --git a/plivo/resources/calls.py b/plivo/resources/calls.py index 55d3972e..ec558de0 100644 --- a/plivo/resources/calls.py +++ b/plivo/resources/calls.py @@ -239,6 +239,7 @@ def create(self, ], callback_url=[optional(is_url())], callback_method=[optional(of_type(six.text_type))], + cursor=[optional(of_type(six.text_type))], ) def list(self, subaccount=None, @@ -262,7 +263,8 @@ def list(self, hangup_cause_code=None, hangup_source=None, callback_url=None, - callback_method=None + callback_method=None, + cursor=None ): # Adding if else block because if we are fetching response without callback_url then response will be of type # ListResponseObject, if passing callback_url then will be of type diff --git a/plivo/resources/recordings.py b/plivo/resources/recordings.py index 07d82ef3..2db5542e 100644 --- a/plivo/resources/recordings.py +++ b/plivo/resources/recordings.py @@ -49,6 +49,7 @@ class Recordings(PlivoResourceInterface): ], callback_url=[optional(is_url())], callback_method=[optional(of_type(six.text_type))], + cursor=[optional(of_type(six.text_type))], ) def list(self, subaccount=None, @@ -71,7 +72,8 @@ def list(self, conference_name=None, mpc_name=None, conference_uuid=None, - mpc_uuid=None): + mpc_uuid=None, + cursor=None): if subaccount: if isinstance(subaccount, Subaccount): diff --git a/plivo/version.py b/plivo/version.py index 02e7a3e3..7c7f4bf3 100644 --- a/plivo/version.py +++ b/plivo/version.py @@ -1,2 +1,2 @@ # -*- coding: utf-8 -*- -__version__ = '4.62.0' +__version__ = '4.63.0' diff --git a/setup.py b/setup.py index ecc1a5c0..d6328ca9 100644 --- a/setup.py +++ b/setup.py @@ -10,7 +10,7 @@ setup( name='plivo', - version='4.62.0', + version='4.63.0', description='A Python SDK to make voice calls & send SMS using Plivo and to generate Plivo XML', long_description=long_description, url='https://github.com/plivo/plivo-python', diff --git a/tests/resources/test_pagination.py b/tests/resources/test_pagination.py new file mode 100644 index 00000000..2d4c7182 --- /dev/null +++ b/tests/resources/test_pagination.py @@ -0,0 +1,265 @@ +# -*- coding: utf-8 -*- +""" +Auto-pagination tests for PlivoResourceInterface.__iter__. + +The Call and Recording list endpoints are moving to cursor (keyset) +pagination behind two deployment-wide switches, so at any given moment a +deployment is serving one of three shapes: + + OFFSET (legacy) offset applied; meta.next -> '?limit=20&offset=20' + HYBRID offset applied; meta.next -> '?limit=20&cursor=' + CURSOR offset IGNORED (page 1 returned instead, HTTP 200); + meta.next -> '?limit=20&cursor=' + +meta.total_count is already absent from these responses. Cursors are +opaque and limit still caps at 20. + +These tests never touch a real server: FakeListBackend stands in for +client.calls.list / client.recordings.list. That is deliberate -- QA is +currently on HYBRID, where offset is still applied and iteration looks +perfectly healthy, so a live QA call cannot reproduce the CURSOR-mode +defect. +""" + +import base64 + +from plivo.base import ListResponseObject, ResponseObject +from tests.base import PlivoResourceTestCase + +try: + from urllib.parse import urlencode +except ImportError: + from urllib import urlencode + + +# How many list() calls we let a walk make before declaring it runaway. +# Every scenario below needs 3, so 10 is generous headroom. +RUNAWAY_THRESHOLD = 10 + + +class PaginationRunaway(Exception): + """Raised instead of letting a non-terminating walk hang the suite.""" + + +class FakeListBackend(object): + """A stand-in for a list endpoint, in any of the three pagination modes.""" + + def __init__(self, + client, + mode, + total=45, + page_size=20, + path='/v1/Account/MAXXXXXXXXXXXXXXXXXX/Call/', + id_field='call_uuid'): + self.client = client + self.mode = mode + self.total = total + self.page_size = page_size + self.path = path + self.id_field = id_field + self.call_log = [] + + # -- opaque cursor tokens ------------------------------------------------ + def _encode(self, index): + token = base64.urlsafe_b64encode(str(index).encode('utf-8')) + return token.decode('utf-8').rstrip('=') + + def _decode(self, token): + padded = token + '=' * (-len(token) % 4) + return int(base64.urlsafe_b64decode(padded.encode('utf-8'))) + + def _next_url(self, next_index, limit): + if self.mode == 'offset': + query = [('limit', limit), ('offset', next_index)] + else: + query = [('limit', limit), ('cursor', self._encode(next_index))] + return self.path + '?' + urlencode(query) + + def list(self, limit=20, offset=0, cursor=None, **kwargs): + self.call_log.append({'limit': limit, 'offset': offset, + 'cursor': cursor}) + if len(self.call_log) > RUNAWAY_THRESHOLD: + raise PaginationRunaway( + 'list() called {0} times walking {1} records in {2} mode -- ' + 'iteration is not terminating'.format( + len(self.call_log), self.total, self.mode)) + + limit = min(limit or 20, self.page_size) + + if self.mode == 'cursor': + # A client-supplied offset is ignored outright: page 1 comes + # back instead. HTTP 200, well-formed body, wrong rows. + start = self._decode(cursor) if cursor else 0 + elif offset is not None: + # OFFSET and HYBRID both still apply offset. When an offset + # arrives alongside a cursor the offset is what takes effect, + # so a client that sends offset=0 with its cursor is pinned to + # page 1 -- which is exactly why the walk must omit offset once + # it is following a cursor. + start = offset + else: + start = self._decode(cursor) if cursor else 0 + + objects = [{self.id_field: 'id-{0:03d}'.format(i)} + for i in range(start, min(start + limit, self.total))] + next_index = start + limit + + meta = { + 'limit': limit, + 'offset': start if self.mode != 'cursor' else None, + 'previous': None, + # total_count is already gone from these responses. + 'next': (self._next_url(next_index, limit) + if next_index < self.total else None), + } + + return ListResponseObject(self.client, { + 'api_id': 'fake-api-id', + 'meta': ResponseObject(meta), + 'objects': [ResponseObject(o) for o in objects], + }) + + +class MetaLessListBackend(object): + """A list endpoint whose responses carry no meta at all. + + Not every resource that inherits __iter__ returns meta, so the walk + still has to handle its absence by incrementing offset and stopping + on the first empty page. + """ + + def __init__(self, client, total=45, page_size=20): + self.client = client + self.total = total + self.page_size = page_size + self.call_log = [] + self.mode = 'offset (no meta)' + self.id_field = 'call_uuid' + + def list(self, limit=20, offset=0, cursor=None, **kwargs): + self.call_log.append({'limit': limit, 'offset': offset, + 'cursor': cursor}) + if len(self.call_log) > RUNAWAY_THRESHOLD: + raise PaginationRunaway( + 'list() called {0} times walking {1} records with no meta -- ' + 'iteration is not terminating'.format( + len(self.call_log), self.total)) + limit = min(limit or 20, self.page_size) + start = offset or 0 + objects = [{self.id_field: 'id-{0:03d}'.format(i)} + for i in range(start, min(start + limit, self.total))] + return ListResponseObject(self.client, { + 'api_id': 'fake-api-id', + 'objects': [ResponseObject(o) for o in objects], + }) + + +class AutoPaginationTest(PlivoResourceTestCase): + def _walk(self, interface, backend): + interface.list = backend.list + return [item for item in interface] + + def _assert_full_clean_walk(self, interface, backend): + ids = [obj[backend.id_field] for obj in self._walk(interface, + backend)] + expected = ['id-{0:03d}'.format(i) for i in range(backend.total)] + self.assertEqual( + len(ids), len(set(ids)), + 'walk yielded duplicate records in {0} mode'.format(backend.mode)) + self.assertEqual( + ids, expected, + 'walk did not yield every record exactly once in {0} ' + 'mode'.format(backend.mode)) + + # -- the defect --------------------------------------------------------- + def test_calls_iteration_terminates_in_cursor_mode(self): + backend = FakeListBackend(self.client, mode='cursor') + try: + self._assert_full_clean_walk(self.client.calls, backend) + except PaginationRunaway as exc: + self.fail(str(exc)) + + def test_recordings_iteration_terminates_in_cursor_mode(self): + backend = FakeListBackend( + self.client, + mode='cursor', + path='/v1/Account/MAXXXXXXXXXXXXXXXXXX/Recording/', + id_field='recording_id') + try: + self._assert_full_clean_walk(self.client.recordings, backend) + except PaginationRunaway as exc: + self.fail(str(exc)) + + # -- the other two live modes ------------------------------------------ + def test_calls_iteration_in_hybrid_mode(self): + backend = FakeListBackend(self.client, mode='hybrid') + try: + self._assert_full_clean_walk(self.client.calls, backend) + except PaginationRunaway as exc: + self.fail(str(exc)) + + def test_calls_iteration_in_offset_mode(self): + backend = FakeListBackend(self.client, mode='offset') + self._assert_full_clean_walk(self.client.calls, backend) + + def test_no_offset_is_sent_alongside_a_cursor(self): + backend = FakeListBackend(self.client, mode='cursor') + try: + self._walk(self.client.calls, backend) + except PaginationRunaway as exc: + self.fail(str(exc)) + cursor_calls = [c for c in backend.call_log if c['cursor']] + self.assertTrue(cursor_calls, 'walk never followed a cursor') + for call in cursor_calls: + self.assertIsNone( + call['offset'], + 'offset={0!r} was sent alongside cursor={1!r}; under HYBRID ' + 'that offset is still applied and pins the walk to page ' + '1'.format(call['offset'], call['cursor'])) + + def test_iteration_stops_when_meta_next_is_null(self): + """One exact page: meta.next is null, so there is nothing to follow.""" + backend = FakeListBackend(self.client, mode='cursor', total=20) + self.client.calls.list = backend.list + try: + self.assertEqual(len([item for item in self.client.calls]), 20) + except PaginationRunaway as exc: + self.fail(str(exc)) + + def test_iteration_falls_back_when_response_has_no_meta(self): + """Meta-less list responses keep the plain offset walk.""" + backend = MetaLessListBackend(self.client, total=45) + self.client.calls.list = backend.list + try: + ids = [obj['call_uuid'] for obj in self.client.calls] + except PaginationRunaway as exc: + self.fail(str(exc)) + self.assertEqual(ids, ['id-{0:03d}'.format(i) for i in range(45)]) + + +class CursorParamTest(PlivoResourceTestCase): + def test_calls_list_accepts_cursor(self): + self.client.set_expected_response(200, {'api_id': 'x', 'meta': {}, + 'objects': []}) + self.client.calls.list(cursor='b3BhcXVl') + self.assertIn('cursor=b3BhcXVl', self.client.current_request.url) + + def test_recordings_list_accepts_cursor(self): + self.client.set_expected_response(200, {'api_id': 'x', 'meta': {}, + 'objects': []}) + self.client.recordings.list(cursor='b3BhcXVl') + self.assertIn('cursor=b3BhcXVl', self.client.current_request.url) + + def test_calls_list_still_accepts_offset(self): + self.client.set_expected_response(200, {'api_id': 'x', 'meta': {}, + 'objects': []}) + self.client.calls.list(limit=10, offset=30) + self.assertIn('offset=30', self.client.current_request.url) + self.assertIn('limit=10', self.client.current_request.url) + + def test_recordings_list_still_accepts_offset(self): + self.client.set_expected_response(200, {'api_id': 'x', 'meta': {}, + 'objects': []}) + self.client.recordings.list(limit=10, offset=30) + self.assertIn('offset=30', self.client.current_request.url) + self.assertIn('limit=10', self.client.current_request.url)