Skip to content

Commit 622517d

Browse files
committed
Fix non-terminating auto_paging_iter; release 0.5.2
The reviews feed returns pageInfo.endCursor as null and hasNextPage as true on every page, including the empty page past the last record. SyncPage trusted has_more alone and then called the fetcher with a None cursor, which dropped the `after` query parameter and silently re-requested the first page. As a result client.reviews.list(...).auto_paging_iter() never terminated: it walked the feed and then restarted it, re-yielding every record indefinitely. Two guards, both on SyncPage so every paginated resource is covered rather than just reviews: - next_page() returns None when there is no cursor to page from. - auto_paging_iter() stops on a page that comes back with no records. Also expose the feed's totalCount as page.total on reviews.list, matching what locations.list already did. Set pythonpath = ["src"] for pytest. An installed build of the package was shadowing src/, so the suite was exercising code that is not in this checkout and reported nine failures because of it. Against the working tree the suite is green: 77 passing.
1 parent 6fffa70 commit 622517d

6 files changed

Lines changed: 88 additions & 6 deletions

File tree

README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,19 @@ page = client.locations.list(first=50)
102102
for loc in page:
103103
print(loc.name)
104104

105+
print(page.total) # total records matching the query
106+
105107
if page.has_more: # manual paging
106108
next_page = page.next_page()
107109

108110
for loc in page.auto_paging_iter(): # every record, all pages
109111
print(loc.name)
110112
```
111113

114+
`auto_paging_iter()` walks to the end of the result set and stops there, so it is
115+
safe to use on large histories. `client.reviews.list(...)` exposes `page.total`
116+
the same way, from the feed's `totalCount`.
117+
112118
Responses are `APIObject`s: dot access (`loc.name`), dict access (`loc["stateIso"]`), and `loc.to_dict()` all work.
113119

114120
## Locations

pyproject.toml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
44

55
[project]
66
name = "listingsapi"
7-
version = "0.5.1"
7+
version = "0.5.2"
88
description = "Python SDK for listingsAPI — listings, reviews, posts, and analytics for local marketing"
99
readme = "README.md"
1010
requires-python = ">=3.9"
@@ -48,3 +48,7 @@ listingsapi = ["py.typed"]
4848

4949
[tool.pytest.ini_options]
5050
testpaths = ["tests"]
51+
# Test the working tree, not whatever version happens to be pip-installed.
52+
# Without this an installed copy shadows src/ and the suite silently reports
53+
# failures for code that is not in this checkout.
54+
pythonpath = ["src"]

src/listingsapi/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
ValidationError,
2424
)
2525

26-
__version__ = "0.5.1"
26+
__version__ = "0.5.2"
2727

2828
__all__ = [
2929
# Client

src/listingsapi/_types.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,14 @@ def __repr__(self) -> str:
117117
return f"SyncPage(items={len(self.data)}, has_more={self.has_more})"
118118

119119
def next_page(self) -> SyncPage | None:
120-
"""Fetch the next page of results, or None if no more pages."""
121-
if not self.has_more or not self._fetch_next:
120+
"""Fetch the next page of results, or None if there is no next page.
121+
122+
Returns None when there is no cursor to page from, not just when
123+
has_more is False. Without a cursor the fetcher would omit `after`
124+
entirely and the API would answer with the *first* page again, so
125+
paging would silently restart instead of ending.
126+
"""
127+
if not self.has_more or not self._fetch_next or self._end_cursor is None:
122128
return None
123129
return self._fetch_next(self._end_cursor)
124130

@@ -132,6 +138,9 @@ def auto_paging_iter(self) -> Generator[APIObject, None, None]:
132138
page: SyncPage | None = self
133139
while page is not None:
134140
yield from page.data
135-
if not page.has_more:
141+
# An empty page means the feed is exhausted. Treat that as
142+
# authoritative: some endpoints keep reporting has_more=True
143+
# past the final record, so has_more alone cannot terminate.
144+
if not page.data or not page.has_more:
136145
break
137146
page = page.next_page()

src/listingsapi/resources/reviews.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ def list(
118118
data=items,
119119
has_more=page_info.get("hasNextPage", False),
120120
end_cursor=end_cursor,
121+
total=result.get("totalCount"),
121122
_fetch_next=lambda cursor: self.list(
122123
location_id,
123124
first=first,

tests/test_listingsapi.py

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,68 @@ def test_sync_page_auto_paging_iter():
198198
assert names == ["a", "b", "c"]
199199

200200

201+
def test_sync_page_next_page_none_when_cursor_missing():
202+
"""Without a cursor, `after` would be omitted and the API would return the
203+
first page again, so paging must stop rather than silently restart."""
204+
page = SyncPage(
205+
data=[{"name": "a"}],
206+
has_more=True,
207+
end_cursor=None,
208+
_fetch_next=lambda cursor: pytest.fail("must not refetch without a cursor"),
209+
)
210+
assert page.next_page() is None
211+
212+
213+
def test_sync_page_auto_paging_iter_stops_on_empty_page():
214+
"""An empty page ends the feed even if the API still claims has_more."""
215+
empty = SyncPage(data=[], has_more=True, end_cursor=None)
216+
page1 = SyncPage(
217+
data=[{"name": "a"}],
218+
has_more=True,
219+
end_cursor="cursor1",
220+
_fetch_next=lambda cursor: empty,
221+
)
222+
assert [item.name for item in page1.auto_paging_iter()] == ["a"]
223+
224+
225+
def test_auto_paging_iter_terminates_when_api_always_reports_has_more(client, monkeypatch):
226+
"""Regression: the reviews feed returns pageInfo.endCursor=null and
227+
hasNextPage=true on every page, including the empty one past the last
228+
record. Paging must still finish exactly once over the data."""
229+
total = 12
230+
page_size = 5
231+
calls = []
232+
233+
def fake_location_get(location_id, path_suffix, params=None):
234+
params = params or {}
235+
assert len(calls) < 10, "auto_paging_iter did not terminate"
236+
after = params.get("after")
237+
start = int(after.split(":")[1]) if after else 0
238+
rows = range(start + 1, min(start + page_size, total) + 1)
239+
calls.append(len(rows))
240+
return {
241+
"data": {
242+
"interactions": {
243+
"edges": [
244+
{"cursor": f"Interaction:{n}", "node": {"interactionId": f"r{n}"}}
245+
for n in rows
246+
],
247+
"totalCount": total,
248+
# verbatim live shape: no usable cursor, has-next never false
249+
"pageInfo": {"hasNextPage": True, "endCursor": None},
250+
}
251+
}
252+
}
253+
254+
monkeypatch.setattr(client, "_location_get", fake_location_get)
255+
256+
ids = [r.interactionId for r in client.reviews.list(1, first=page_size).auto_paging_iter()]
257+
258+
assert ids == [f"r{n}" for n in range(1, total + 1)]
259+
assert len(ids) == len(set(ids)), "no record may be yielded twice"
260+
assert calls == [5, 5, 2, 0]
261+
262+
201263
# --- Exceptions ---
202264

203265
def test_exception_hierarchy():
@@ -388,7 +450,7 @@ def test_analytics_google(client):
388450

389451

390452
def test_version():
391-
assert listingsapi.__version__ == "0.5.1"
453+
assert listingsapi.__version__ == "0.5.2"
392454

393455

394456
# --- locations.add (one-call create with validation) ---

0 commit comments

Comments
 (0)