Repository: gtalarico/pyairtable
Pull request #245 (2023-04-22 09:54:20)

Use `POST /listRecords` instead of `GET` when the URL gets too long
This attempts to resolve the same issue as #222 but without as much refactoring. It was hard for me to wrap my head around everything going on in that branch, so I started by writing a unit test and an integration test and then wrote only what I needed to get them to pass. I'm open to feedback or suggestions, especially if anything else in that branch is a must-have.

codecov-commenter: ## [Codecov](https://codecov.io/gh/gtalarico/pyairtable/pull/245?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) Report
> Merging [#245](https://codecov.io/gh/gtalarico/pyairtable/pull/245?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) (f50c02d) into [main](https://codecov.io/gh/gtalarico/pyairtable/commit/2f6ecd6c75e9591e0bf4e16f65ef0c2b326697ce?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) (2f6ecd6) will **increase** coverage by `0.15%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #245      +/-   ##
==========================================
+ Coverage   87.34%   87.50%   +0.15%     
==========================================
  Files          14       14              
  Lines         640      648       +8     
==========================================
+ Hits          559      567       +8     
  Misses         81       81              
```


| [Impacted Files](https://codecov.io/gh/gtalarico/pyairtable/pull/245?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) | Coverage Δ | |
|---|---|---|
| [pyairtable/api/abstract.py](https://codecov.io/gh/gtalarico/pyairtable/pull/245?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico#diff-cHlhaXJ0YWJsZS9hcGkvYWJzdHJhY3QucHk=) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico)


Patch:
diff --git a/pyairtable/api/abstract.py b/pyairtable/api/abstract.py
index 57c8280..175a600 100644
--- a/pyairtable/api/abstract.py
+++ b/pyairtable/api/abstract.py
@@ -2,7 +2,7 @@ import abc
 import posixpath
 import time
 from functools import lru_cache
-from typing import List, Optional, Tuple
+from typing import Dict, List, Optional, Tuple
 from urllib.parse import quote
 
 import requests
@@ -14,10 +14,16 @@ from .retrying import Retry, _RetryingSession
 TimeoutTuple = Tuple[int, int]
 
 
+#: List of parameter names that cannot be passed via POST, only GET
+#: See https://github.com/gtalarico/pyairtable/pull/210#discussion_r1046014885
+PARAMS_NOT_SUPPORTED_VIA_POST = ("userLocale", "timeZone")
+
+
 class ApiAbstract(metaclass=abc.ABCMeta):
     VERSION = "v0"
     API_LIMIT = 1.0 / 5  # 5 per second
     MAX_RECORDS_PER_REQUEST = 10
+    MAX_URL_LENGTH = 16000
 
     session: Session
     endpoint_url: str
@@ -107,10 +113,55 @@ class ApiAbstract(metaclass=abc.ABCMeta):
         else:
             return response.json()
 
-    def _request(self, method: str, url: str, params=None, json_data=None):
-        response = self.session.request(
-            method, url, params=params, json=json_data, timeout=self.timeout
-        )
+    def _request(
+        self,
+        method: str,
+        url: str,
+        fallback_post_url: Optional[str] = None,
+        params: Optional[Dict] = None,
+        json_data: Optional[Dict] = None,
+    ):
+        """
+        Makes a request to the Airtable API, optionally converting a GET to a POST
+        if the URL exceeds the API's maximum URL length.
+
+        See https://support.airtable.com/docs/enforcement-of-url-length-limit-for-web-api-requests
+
+        Args:
+            method (``str``): HTTP method to use.
+            url (``str``): The URL we're attempting to call.
+
+        Keyword Args:
+            fallback_post_url (``str``, optional): The URL to use if we have to convert a GET to a POST.
+            params (``dict``, optional): The query params to append to the URL.
+            json_data (``dict``, optional): The JSON payload for a POST/PUT/PATCH/DELETE request.
+        """
+        request = requests.Request(method, url=url, params=params, json=json_data)
+        prepared = self.session.prepare_request(request)
+
+        # If our URL is too long, move *most* (not all) query params into the body.
+        if (
+            fallback_post_url
+            and method.upper() == "GET"
+            and len(str(prepared.url)) >= self.MAX_URL_LENGTH
+        ):
+            params = dict(params or {})
+            json_data = {
+                **(json_data or {}),
+                **{
+                    key: params.pop(key)
+                    for key in list(params)
+                    if key not in PARAMS_NOT_SUPPORTED_VIA_POST
+                },
+            }
+            return self._request(
+                method="POST",
+                url=fallback_post_url,
+                params=params,
+                json_data=json_data,
+            )
+
+        response = self.session.send(prepared, timeout=self.timeout)
         return self._process_response(response)
 
     def _get_record(
@@ -127,7 +178,12 @@ class ApiAbstract(metaclass=abc.ABCMeta):
             table_url = self.get_table_url(base_id, table_name)
             if offset:
                 params.update({"offset": offset})
-            data = self._request("get", table_url, params=params)
+            data = self._request(
+                method="get",
+                url=table_url,
+                fallback_post_url=posixpath.join(table_url, "listRecords"),
+                params=params,
+            )
             records = data.get("records", [])
             yield records
             offset = data.get("offset")


Test patch:
diff --git a/tests/integration/test_integration_api.py b/tests/integration/test_integration_api.py
index af84c4c..2e933ae 100644
--- a/tests/integration/test_integration_api.py
+++ b/tests/integration/test_integration_api.py
@@ -29,6 +29,11 @@ def test_integration_table(table, cols):
     assert rec["id"] in [r["id"] for r in records]
     assert cols.NUM in records[0]["fields"]  # col name in "fields"
 
+    # Get all via POST
+    formula = f"{cols.TEXT} != '{'x' * 17000}'"
+    records = table.all(formula=formula)
+    assert rec["id"] in [r["id"] for r in records]
+
     # All `return_by_field_ids`
     records = table.all(return_fields_by_field_id=True)
     assert cols.NUM not in records[0]["fields"]  # fields returns fieldId
diff --git a/tests/test_api_table.py b/tests/test_api_table.py
index 2a83f45..0d34fb7 100644
--- a/tests/test_api_table.py
+++ b/tests/test_api_table.py
@@ -126,6 +126,39 @@ def test_all(table, mock_response_list, mock_records):
         assert dict_equals(resp, mock_records[n])
 
 
+def test_all__long_formula(table: Table):
+    """
+    Tests that we automatically convert a GET into a POST if the URL is too long,
+    but that certain parameters (timeZone and userLocale) remain query params.
+
+    See https://github.com/gtalarico/pyairtable/pull/210#discussion_r1046014885
+    """
+    # Here we construct a URL that is *exactly* over the limit.
+    user_locale = "en-ie"
+    time_zone = "UTC"
+    formula = "x" * (
+        table.MAX_URL_LENGTH
+        - len(table.table_url)
+        - len("?filterByFormula=")
+        - len(f"&userLocale={user_locale}&timeZone={time_zone}")
+    )
+
+    with Mocker() as mock:
+        url = f"{table.table_url}/listRecords"
+        mock.post(url, status_code=200, json={"records": []})
+        result = table.all(
+            formula=formula,
+            user_locale=user_locale,
+            time_zone=time_zone,
+        )
+
+    assert result == []
+    assert mock.call_count == 1
+    assert mock.last_request.method == "POST"
+    assert mock.last_request.qs == {"timezone": ["utc"], "userlocale": ["en-ie"]}
+    assert mock.last_request.json() == {"filterByFormula": formula}
+
+
 def test_iterate(table, mock_response_list, mock_records):
     with Mocker() as mock:
         mock.get(