Skip to content

Commit a472d24

Browse files
committed
fix: Keep pagination iterators advancing past fully-filtered pages
1 parent af6d0f7 commit a472d24

2 files changed

Lines changed: 95 additions & 12 deletions

File tree

src/apify_client/_pagination.py

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,11 @@ def get_items_iterator(
4141
is used for offset bookkeeping (the Apify API's `count` reflects items scanned, which can exceed items returned when
4242
filters are applied).
4343
44-
Iteration stops when a page returns no items or when the user-requested `limit` is reached. The `total` field is
45-
intentionally not consulted, because it can change between calls.
44+
Iteration stops when a page scans no items (`count` is `0`, or `items` is empty when `count` is absent) or when the
45+
user-requested `limit` is reached. A page can scan items while returning none — filters like `clean` drop items from
46+
`items` but still count toward `count` — so terminating on scanned rather than returned items keeps the iterator
47+
advancing across fully-filtered pages. The `total` field is intentionally not consulted, because it can change
48+
between calls.
4649
4750
Args:
4851
callback: Function returning a single page of items.
@@ -62,9 +65,10 @@ def get_items_iterator(
6265
)
6366
yield from current_page.items
6467

65-
fetched_items += max(getattr(current_page, 'count', 0), len(current_page.items))
68+
page_scanned = max(getattr(current_page, 'count', 0), len(current_page.items))
69+
fetched_items += page_scanned
6670

67-
if not current_page.items or (initial_limit and fetched_items >= initial_limit):
71+
if not page_scanned or (initial_limit and fetched_items >= initial_limit):
6872
break
6973

7074

@@ -92,9 +96,10 @@ async def get_items_iterator_async(
9296
for item in current_page.items:
9397
yield item
9498

95-
fetched_items += max(getattr(current_page, 'count', 0), len(current_page.items))
99+
page_scanned = max(getattr(current_page, 'count', 0), len(current_page.items))
100+
fetched_items += page_scanned
96101

97-
if not current_page.items or (initial_limit and fetched_items >= initial_limit):
102+
if not page_scanned or (initial_limit and fetched_items >= initial_limit):
98103
break
99104

100105

@@ -124,8 +129,8 @@ def get_cursor_iterator(
124129
"""Yield individual items from cursor-paginated API responses.
125130
126131
Cursor pagination is restricted to the two API responses that expose it: `ListOfKeys` (for key-value store keys) and
127-
`ListOfRequests` (for request queue requests). Iteration ends when a page returns no items, the next cursor is
128-
`None`, or the user-requested `limit` is reached.
132+
`ListOfRequests` (for request queue requests). Iteration ends when a page scans no items, the next cursor is `None`,
133+
or the user-requested `limit` is reached.
129134
130135
Args:
131136
callback: Function returning a single page of items. Receives `cursor` and `limit` kwargs.
@@ -144,12 +149,13 @@ def get_cursor_iterator(
144149
)
145150
yield from current_page.items
146151

147-
fetched_items += max(getattr(current_page, 'count', 0), len(current_page.items))
152+
page_scanned = max(getattr(current_page, 'count', 0), len(current_page.items))
153+
fetched_items += page_scanned
148154
cursor = (
149155
current_page.next_exclusive_start_key if isinstance(current_page, ListOfKeys) else current_page.next_cursor
150156
)
151157

152-
if not current_page.items or cursor is None or (initial_limit and fetched_items >= initial_limit):
158+
if not page_scanned or cursor is None or (initial_limit and fetched_items >= initial_limit):
153159
break
154160

155161

@@ -189,12 +195,13 @@ async def get_cursor_iterator_async(
189195
for item in current_page.items:
190196
yield item
191197

192-
fetched_items += max(getattr(current_page, 'count', 0), len(current_page.items))
198+
page_scanned = max(getattr(current_page, 'count', 0), len(current_page.items))
199+
fetched_items += page_scanned
193200
cursor = (
194201
current_page.next_exclusive_start_key if isinstance(current_page, ListOfKeys) else current_page.next_cursor
195202
)
196203

197-
if not current_page.items or cursor is None or (initial_limit and fetched_items >= initial_limit):
204+
if not page_scanned or cursor is None or (initial_limit and fetched_items >= initial_limit):
198205
break
199206

200207

tests/unit/test_client_pagination.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@
1111

1212
from apify_client import ApifyClient, ApifyClientAsync
1313
from apify_client import _models as _models_module
14+
from apify_client._pagination import (
15+
get_cursor_iterator,
16+
get_cursor_iterator_async,
17+
get_items_iterator,
18+
get_items_iterator_async,
19+
)
1420
from apify_client._resource_clients import (
1521
ActorCollectionClient,
1622
ActorCollectionClientAsync,
@@ -617,3 +623,73 @@ async def test_rq_list_requests_iterable_async(
617623
client: RequestQueueClientAsync = _CLIENT_FACTORIES[client_name](_make_async_client(pagination_server))
618624
returned_items = [dict(item) async for item in client.iterate_requests(**inputs)]
619625
assert returned_items == expected_items
626+
627+
628+
class _FakeOffsetPage:
629+
"""Offset-paginated page whose `count` (items scanned) may exceed `len(items)` when filters drop items."""
630+
631+
def __init__(self, items: list[dict[str, int]], count: int) -> None:
632+
self.items = items
633+
self.count = count
634+
635+
636+
class _FakeCursorPage:
637+
"""Cursor-paginated page whose `count` (items scanned) may exceed `len(items)` when filters drop items."""
638+
639+
def __init__(self, items: list[dict[str, int]], count: int, next_cursor: str | None) -> None:
640+
self.items = items
641+
self.count = count
642+
self.next_cursor = next_cursor
643+
644+
645+
def test_items_iterator_continues_past_fully_filtered_page() -> None:
646+
"""A fully-filtered page (`items=[]`, `count>0`) must not stop the offset iterator while more data was scanned."""
647+
pages = {
648+
0: _FakeOffsetPage(items=[], count=1000),
649+
1000: _FakeOffsetPage(items=[{'id': 1}, {'id': 2}], count=2),
650+
}
651+
652+
def _callback(*, limit: int | None = None, offset: int | None = None) -> _FakeOffsetPage: # noqa: ARG001
653+
return pages.get(offset or 0, _FakeOffsetPage(items=[], count=0))
654+
655+
assert list(get_items_iterator(_callback, chunk_size=1000)) == [{'id': 1}, {'id': 2}]
656+
657+
658+
async def test_items_iterator_async_continues_past_fully_filtered_page() -> None:
659+
"""A fully-filtered page (`items=[]`, `count>0`) must not stop the async offset iterator while more was scanned."""
660+
pages = {
661+
0: _FakeOffsetPage(items=[], count=1000),
662+
1000: _FakeOffsetPage(items=[{'id': 1}, {'id': 2}], count=2),
663+
}
664+
665+
async def _callback(*, limit: int | None = None, offset: int | None = None) -> _FakeOffsetPage: # noqa: ARG001
666+
return pages.get(offset or 0, _FakeOffsetPage(items=[], count=0))
667+
668+
assert [item async for item in get_items_iterator_async(_callback, chunk_size=1000)] == [{'id': 1}, {'id': 2}]
669+
670+
671+
def test_cursor_iterator_continues_past_fully_filtered_page() -> None:
672+
"""A fully-filtered page (`items=[]`, `count>0`) with a live cursor must not stop the cursor iterator."""
673+
pages = {
674+
None: _FakeCursorPage(items=[], count=1000, next_cursor='c1'),
675+
'c1': _FakeCursorPage(items=[{'id': 1}, {'id': 2}], count=2, next_cursor=None),
676+
}
677+
678+
def _callback(*, limit: int | None = None, cursor: str | None = None) -> _FakeCursorPage: # noqa: ARG001
679+
return pages[cursor]
680+
681+
assert list(get_cursor_iterator(_callback, chunk_size=1000)) == [{'id': 1}, {'id': 2}] # ty: ignore[no-matching-overload]
682+
683+
684+
async def test_cursor_iterator_async_continues_past_fully_filtered_page() -> None:
685+
"""A fully-filtered page (`items=[]`, `count>0`) with a live cursor must not stop the async cursor iterator."""
686+
pages = {
687+
None: _FakeCursorPage(items=[], count=1000, next_cursor='c1'),
688+
'c1': _FakeCursorPage(items=[{'id': 1}, {'id': 2}], count=2, next_cursor=None),
689+
}
690+
691+
async def _callback(*, limit: int | None = None, cursor: str | None = None) -> _FakeCursorPage: # noqa: ARG001
692+
return pages[cursor]
693+
694+
collected = [item async for item in get_cursor_iterator_async(_callback, chunk_size=1000)] # ty: ignore[no-matching-overload]
695+
assert collected == [{'id': 1}, {'id': 2}]

0 commit comments

Comments
 (0)