Repository: gtalarico/pyairtable (python)

Issue:
Endless 'List records'
Hi, I've encountered an issue while trying to fetch data from Atirable using a filter that exceeds the `MAX_URL_LENGTH`.
As a user, this appears as a long period of no response, although there is network activity happening in the background.

Example code: 
```python
# this is example filter formula, but it represents how I fetching data
formula = """AND(
    OR(
        RECORD_ID()='rec1',
        RECORD_ID()='rec2',
        RECORD_ID()='rec3',
        RECORD_ID()='rec4',
        RECORD_ID()='rec5',
        ...
        RECORD_ID()='rec450'
    )
)"""

fields=['f1', 'f2', 'f3']

table = Table(...)
records: list = table.all(formula=formula, fields=fields)
```

My assumption is that the code is unable to handle pagination correctly and keeps endlessly iterating, sending new requests each time. I'm currently using version 2.3.6, and I hoped this issue might have been fixed in version 3.0.0, but it seems it hasn't been.

---
### Additional questions

1) Temporarily, I worked around this issue by rewriting the data-fetching logic in my code using `api.request`. However, I noticed that the `maxRecords` parameter doesn't seem to have any effect—requests always return 100 records if possible.

2) If I have an `offset` from the first response page, do I need to include the same query parameters (options) along with the `offset`, or is it sufficient to pass only the offset parameter to retrieve the next pages?

```python
response = api.request(
                "get",
                url=table.url,
                options=options,
                params={'maxRecords': 50},
                fallback=("post", f"{table.url}/listRecords"),
            )
```






Pull request description:
Fix for endless loop when requesting records via `POST /listRecords`
This fixes #415. The cause was a silly mistake between `params` and `options` within the Api class, and this branch adds both a unit test and an integration test to ensure this behavior is correct.

Patch:
diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
index 3d934c9..38f7a54 100644
--- a/docs/source/changelog.rst
+++ b/docs/source/changelog.rst
@@ -2,6 +2,14 @@
 Changelog
 =========
 
+3.0.1 (2024-12-06)
+------------------------
+
+* Fix for `#415 <https://github.com/gtalarico/pyairtable/issues/415>`_
+  which caused an endless loop when making a request via `POST /listRecords`.
+  - `PR #416 <https://github.com/gtalarico/pyairtable/pull/416>`_,
+    `PR #417 <https://github.com/gtalarico/pyairtable/pull/417>`_
+
 3.0 (2024-11-15)
 ------------------------
 
@@ -58,6 +66,14 @@ Changelog
 * Rewrite of :mod:`pyairtable.formulas` module. See :ref:`Building Formulas`.
   - `PR #329 <https://github.com/gtalarico/pyairtable/pull/329>`_
 
+2.3.7 (2024-12-06)
+------------------------
+
+* Fix for `#415 <https://github.com/gtalarico/pyairtable/issues/415>`_
+  which caused an endless loop when making a request via `POST /listRecords`.
+  - `PR #416 <https://github.com/gtalarico/pyairtable/pull/416>`_,
+    `PR #417 <https://github.com/gtalarico/pyairtable/pull/417>`_
+
 2.3.6 (2024-11-11)
 ------------------------
 
diff --git a/pyairtable/__init__.py b/pyairtable/__init__.py
index da712f3..00aa957 100644
--- a/pyairtable/__init__.py
+++ b/pyairtable/__init__.py
@@ -1,4 +1,4 @@
-__version__ = "3.0.0"
+__version__ = "3.0.1"
 
 from pyairtable.api import Api, Base, Table
 from pyairtable.api.enterprise import Enterprise
diff --git a/pyairtable/api/api.py b/pyairtable/api/api.py
index ac1d863..3dae473 100644
--- a/pyairtable/api/api.py
+++ b/pyairtable/api/api.py
@@ -394,7 +394,7 @@ class Api:
                 return
             if not (offset := _get_offset_field(response)):
                 return
-            params = {**params, offset_field: offset}
+            options = {**options, offset_field: offset}
 
     def chunked(self, iterable: Sequence[T]) -> Iterator[Sequence[T]]:
         """
diff --git a/pyairtable/api/params.py b/pyairtable/api/params.py
index 07c7a2b..48aab06 100644
--- a/pyairtable/api/params.py
+++ b/pyairtable/api/params.py
@@ -74,6 +74,9 @@ OPTIONS_TO_PARAMETERS = {
     # get webhook payloads
     "limit": "limit",
     "cursor": "cursor",
+    # get audit log events
+    "next": "next",
+    "previous": "previous",
 }
 
 


Test patch:
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
index b9b3e81..68db160 100644
--- a/tests/integration/conftest.py
+++ b/tests/integration/conftest.py
@@ -10,20 +10,21 @@ def valid_img_url():
     return "https://github.com/gtalarico/pyairtable/raw/9f243cb0935ad7112859f990434612efdaf49c67/docs/source/_static/logo.png"
 
 
+class Columns:
+    # Table should have these Columns
+    TEXT = "text"  # Text
+    TEXT_ID = "fldzbVdWW4xJdZ1em"  # for returnFieldsByFieldId
+    NUM = "number"  # Number, float
+    NUM_ID = "fldFLyuxGuWobyMV2"  # for returnFieldsByFieldId
+    BOOL = "boolean"  # Boolean
+    DATETIME = "datetime"  # Datetime
+    ATTACHMENT = "attachment"  # attachment
+    ATTACHMENT_ID = "fld5VP9oPeCpvIumr"  # for upload_attachment
+
+
 @pytest.fixture
-def cols():
-    class Columns:
-        # Table should have these Columns
-        TEXT = "text"  # Text
-        TEXT_ID = "fldzbVdWW4xJdZ1em"  # for returnFieldsByFieldId
-        NUM = "number"  # Number, float
-        NUM_ID = "fldFLyuxGuWobyMV2"  # for returnFieldsByFieldId
-        BOOL = "boolean"  # Boolean
-        DATETIME = "datetime"  # Datetime
-        ATTACHMENT = "attachment"  # attachment
-        ATTACHMENT_ID = "fld5VP9oPeCpvIumr"  # for upload_attachment
-
-    return Columns
+def cols() -> Columns:
+    return Columns()
 
 
 @pytest.fixture
diff --git a/tests/integration/test_integration_api.py b/tests/integration/test_integration_api.py
index 66656f4..b5ee0e6 100644
--- a/tests/integration/test_integration_api.py
+++ b/tests/integration/test_integration_api.py
@@ -6,7 +6,7 @@ import pytest
 import requests
 
 from pyairtable import Table
-from pyairtable import formulas as fo
+from pyairtable.formulas import AND, EQ, FIND, OR, RECORD_ID, Field, match
 from pyairtable.utils import date_to_iso_str, datetime_to_iso_str
 
 pytestmark = [pytest.mark.integration]
@@ -150,17 +150,17 @@ def test_integration_field_equals(table: Table, cols):
     rv_create = table.create(values)
 
     # match all - finds
-    rv_first = table.first(formula=fo.match(values))
+    rv_first = table.first(formula=match(values))
     assert rv_first and rv_first["id"] == rv_create["id"]
 
     # match all - does not find
     values = {cols.TEXT: TEXT_VALUE, cols.NUM: 0}
-    rv_first = table.first(formula=fo.match(values))
+    rv_first = table.first(formula=match(values))
     assert rv_first is None
 
     # match all w/ match_any=True - does not find
     values = {cols.TEXT: TEXT_VALUE, cols.NUM: 0}
-    rv_first = table.first(formula=fo.match(values, match_any=True))
+    rv_first = table.first(formula=match(values, match_any=True))
     assert rv_first and rv_first["id"] == rv_create["id"]
 
 
@@ -226,7 +226,7 @@ def test_batch_upsert(table: Table, cols):
 
 def test_integration_formula_datetime(table: Table, cols):
     now = datetime.now(timezone.utc)
-    formula = fo.match({cols.DATETIME: now})
+    formula = match({cols.DATETIME: now})
     rv_create = table.create({cols.DATETIME: datetime_to_iso_str(now)})
     rv_first = table.first(formula=formula)
     assert rv_first and rv_first["id"] == rv_create["id"]
@@ -243,7 +243,7 @@ def test_integration_formula_date_filter(table: Table, cols):
         rec = table.create({cols.DATETIME: dt_str})
         created.append(rec)
 
-    formula = fo.FIND(date_str, fo.Field(cols.DATETIME))
+    formula = FIND(date_str, Field(cols.DATETIME))
     rv_all = table.all(formula=formula)
     print("repr", repr(formula), "\nstr", str(formula))
     assert rv_all
@@ -253,12 +253,12 @@ def test_integration_formula_date_filter(table: Table, cols):
 def test_integration_field_equals_with_quotes(table: Table, cols):
     VALUE = "Contact's Name {}".format(uuid4())
     rv_create = table.create({cols.TEXT: VALUE})
-    rv_first = table.first(formula=fo.match({cols.TEXT: VALUE}))
+    rv_first = table.first(formula=match({cols.TEXT: VALUE}))
     assert rv_first and rv_first["id"] == rv_create["id"]
 
     VALUE = 'Some "Quote"  {}'.format(uuid4())
     rv_create = table.create({cols.TEXT: VALUE})
-    rv_first = table.first(formula=fo.match({cols.TEXT: VALUE}))
+    rv_first = table.first(formula=match({cols.TEXT: VALUE}))
     assert rv_first and rv_first["id"] == rv_create["id"]
 
 
@@ -268,10 +268,10 @@ def test_integration_formula_composition(table: Table, cols):
     bool_ = True
     rv_create = table.create({cols.TEXT: text, cols.NUM: num, cols.BOOL: bool_})
 
-    formula = fo.AND(
-        fo.EQ(fo.Field(cols.TEXT), text),
-        fo.EQ(fo.Field(cols.NUM), num),
-        fo.EQ(fo.Field(cols.BOOL), bool_),  # not needs to be int()
+    formula = AND(
+        EQ(Field(cols.TEXT), text),
+        EQ(Field(cols.NUM), num),
+        EQ(Field(cols.BOOL), bool_),  # not needs to be int()
     )
     rv_first = table.first(formula=formula)
 
@@ -362,3 +362,42 @@ def test_integration_comments(api, table: Table, cols):
 
     # Test that we can delete the comment
     comments[0].delete()
+
+
+def test_pagination(cols, api, table):
+    """
+    Test that we can paginate through records as expected.
+    """
+    # Start by creating 500 unique records
+    created = table.batch_create([{cols.TEXT: f"Record {i}"} for i in range(500)])
+    formula = OR(RECORD_ID().eq(record["id"]) for record in created[:-1])
+
+    # The formula ought to be longer than the maximum URL length,
+    # so we know we'll convert the request to a POST.
+    assert len(str(formula)) > api.MAX_URL_LENGTH
+    assert created[-1]["id"] not in str(formula)
+
+    for page_size in [10, 50]:
+        paginator = table.iterate(formula=formula, page_size=page_size)
+
+        # Test that each page is the expected size
+        assert len(page1 := next(paginator)) == page_size
+        assert len(page2 := next(paginator)) == page_size
+
+        # Test that we don't keep getting the same records
+        page1_ids = {record["id"] for record in page1}
+        page2_ids = {record["id"] for record in page2}
+        assert page1_ids != page2_ids
+
+    for max_records in [10, 50]:
+        # Test that max_records actually limits the number of records returned,
+        # not just the size of each page of records.
+        records = table.all(formula=formula, max_records=max_records)
+        assert len(records) == max_records
+
+    # Test the combination of each.
+    paginator = table.iterate(formula=formula, page_size=10, max_records=25)
+    pages = list(paginator)
+    ids = {record["id"] for page in pages for record in page}
+    assert [len(page) for page in pages] == [10, 10, 5]
+    assert len(ids) == 25
diff --git a/tests/test_api_api.py b/tests/test_api_api.py
index 458497b..541804d 100644
--- a/tests/test_api_api.py
+++ b/tests/test_api_api.py
@@ -116,6 +116,42 @@ def test_iterate_requests(api: Api, requests_mock):
     assert responses == [response["json"] for response in response_list]
 
 
+def test_iterate_requests__post(api: Api, requests_mock):
+    url = "https://example.com"
+    # prepare a few pages of dummy responses
+    response_list = [
+        {"json": {"page": 0, "offset": 1}},
+        {"json": {"page": 1, "offset": 2}},
+        {"json": {"page": 2}},
+    ]
+    m = requests_mock.post(url, response_list=response_list)
+    # construct a request that will get converted from GET to POST
+    formula = "X" * (api.MAX_URL_LENGTH + 1)
+    pages = list(
+        api.iterate_requests(
+            "GET",
+            url=url,
+            fallback=("POST", url),
+            options={"formula": formula},
+        )
+    )
+    # ensure we got the responses we expected
+    assert pages == [
+        {"page": 0, "offset": 1},
+        {"page": 1, "offset": 2},
+        {"page": 2},
+    ]
+    # ensure we made three POST requests
+    assert m.call_count == 3
+    assert len(pages) == 3
+    requests = [r.json() for r in m.request_history]
+    assert requests == [
+        {"filterByFormula": formula},
+        {"filterByFormula": formula, "offset": "1"},
+        {"filterByFormula": formula, "offset": "2"},
+    ]
+
+
 def test_iterate_requests__invalid_type(api: Api, requests_mock):
     url = "https://example.com"
     response_list = [{"json": {"page": n, "offset": n + 1}} for n in range(1, 3)]