Repository: gtalarico/pyairtable
Pull request #247 (2023-04-30 00:27:04)

Use `POST /listRecords` instead of `GET` when the URL gets too long
I tried this in #245 but failed to consider enough of the differences between the formatting of query parameters vs. POST parameters. (I reverted that merge in 090bf68.) This branch adds more tests to ensure that the API accepts each parameter we might generate. 

I'll leave this open for a week or so in case there's any feedback, especially if I've missed tests for any other edge cases.

codecov-commenter: ## [Codecov](https://codecov.io/gh/gtalarico/pyairtable/pull/247?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) Report
> Merging [#247](https://codecov.io/gh/gtalarico/pyairtable/pull/247?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) (0f337d5) into [main](https://codecov.io/gh/gtalarico/pyairtable/commit/090bf68293147a3dc333e5c3da0480b47ca0da5b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) (090bf68) will **decrease** coverage by `3.67%`.
> The diff coverage is `92.85%`.

```diff
@@            Coverage Diff             @@
##             main     #247      +/-   ##
==========================================
- Coverage   87.50%   83.83%   -3.67%     
==========================================
  Files          14       14              
  Lines         640      662      +22     
==========================================
- Hits          560      555       -5     
- Misses         80      107      +27     
```


| [Impacted Files](https://codecov.io/gh/gtalarico/pyairtable/pull/247?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/247?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico#diff-cHlhaXJ0YWJsZS9hcGkvYWJzdHJhY3QucHk=) | `98.52% <85.71%> (-1.48%)` | :arrow_down: |
| [pyairtable/api/params.py](https://codecov.io/gh/gtalarico/pyairtable/pull/247?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico#diff-cHlhaXJ0YWJsZS9hcGkvcGFyYW1zLnB5) | `65.78% <96.42%> (-32.17%)` | :arrow_down: |



Patch:
diff --git a/docs/source/api.rst b/docs/source/api.rst
index f32f723..86edb0a 100644
--- a/docs/source/api.rst
+++ b/docs/source/api.rst
@@ -303,8 +303,8 @@ have a corresponding ``kwargs`` that can be used with fetching methods like :met
    :widths: 25 25 50
    :header-rows: 1
 
-   * - Parameter
-     - Airtable Option
+   * - Keyword Argument
+     - Airtable Parameter
      - Notes
    * - ``max_records``
      - ``maxRecords``
diff --git a/pyairtable/api/abstract.py b/pyairtable/api/abstract.py
index 57c8280..1035e9d 100644
--- a/pyairtable/api/abstract.py
+++ b/pyairtable/api/abstract.py
@@ -2,13 +2,13 @@ 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
 from requests.sessions import Session
 
-from .params import to_params_dict
+from .params import options_to_json_and_params, options_to_params
 from .retrying import Retry, _RetryingSession
 
 TimeoutTuple = Tuple[int, int]
@@ -18,6 +18,7 @@ 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
@@ -71,15 +72,6 @@ class ApiAbstract(metaclass=abc.ABCMeta):
         table_url = self.get_table_url(base_id, table_name)
         return posixpath.join(table_url, record_id)
 
-    def _options_to_params(self, **options):
-        """
-        Process params names or values as needed using filters
-        """
-        params = {}
-        for name, value in options.items():
-            params.update(to_params_dict(name, value))
-        return params
-
     def _chunk(self, iterable, chunk_size):
         """Break iterable into chunks"""
         for i in range(0, len(iterable), chunk_size):
@@ -107,27 +99,84 @@ 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,
+        options: Optional[Dict] = 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.
+            options (``dict``, optional): Airtable-specific query params to use while fetching records.
+            params (``dict``, optional): Additional query params to append to the URL as-is.
+            json_data (``dict``, optional): The JSON payload for a POST/PUT/PATCH/DELETE request.
+        """
+        # Convert Airtable-specific options to query params, but give priority to query params
+        # that are explicitly passed via `params=`. This is to preserve backwards-compatibility for
+        # any library users who might be calling `self._request` directly.
+        request_params = {
+            **options_to_params(options or {}),
+            **(params or {}),
+        }
+
+        # Build a requests.PreparedRequest so we can examine how long the URL is.
+        prepared = self.session.prepare_request(
+            requests.Request(
+                method,
+                url=url,
+                params=request_params,
+                json=json_data,
+            )
         )
+
+        # If our URL is too long, move *most* (not all) query params into a POST body.
+        if (
+            len(str(prepared.url)) >= self.MAX_URL_LENGTH
+            and method.upper() == "GET"
+            and fallback_post_url
+        ):
+            json_data, spare_params = options_to_json_and_params(options or {})
+            return self._request(
+                method="POST",
+                url=fallback_post_url,
+                params={**spare_params, **(params or {})},
+                json_data=json_data,
+            )
+
+        response = self.session.send(prepared, timeout=self.timeout)
         return self._process_response(response)
 
     def _get_record(
         self, base_id: str, table_name: str, record_id: str, **options
     ) -> dict:
         record_url = self._get_record_url(base_id, table_name, record_id)
-        params = self._options_to_params(**options)
-        return self._request("get", record_url, params=params)
+        return self._request("get", record_url, options=options)
 
     def _iterate(self, base_id: str, table_name: str, **options):
         offset = None
-        params = self._options_to_params(**options)
+        table_url = self.get_table_url(base_id, table_name)
         while True:
-            table_url = self.get_table_url(base_id, table_name)
             if offset:
-                params.update({"offset": offset})
-            data = self._request("get", table_url, params=params)
+                options.update({"offset": offset})
+            data = self._request(
+                method="get",
+                url=table_url,
+                fallback_post_url=f"{table_url}/listRecords",
+                options=options,
+            )
             records = data.get("records", [])
             yield records
             offset = data.get("offset")
@@ -154,12 +203,11 @@ class ApiAbstract(metaclass=abc.ABCMeta):
         self, base_id: str, table_name: str, fields: dict, typecast=False, **options
     ):
         table_url = self.get_table_url(base_id, table_name)
-        params = self._options_to_params(**options)
         return self._request(
             "post",
             table_url,
             json_data={"fields": fields, "typecast": typecast},
-            params=params,
+            options=options,
         )
 
     def _batch_create(
@@ -172,14 +220,13 @@ class ApiAbstract(metaclass=abc.ABCMeta):
     ) -> List[dict]:
         table_url = self.get_table_url(base_id, table_name)
         inserted_records = []
-        params = self._options_to_params(**options)
         for chunk in self._chunk(records, self.MAX_RECORDS_PER_REQUEST):
             new_records = self._build_batch_record_objects(chunk)
             response = self._request(
                 "post",
                 table_url,
                 json_data={"records": new_records, "typecast": typecast},
-                params=params,
+                options=options,
             )
             inserted_records += response["records"]
             time.sleep(self.API_LIMIT)
@@ -198,12 +245,11 @@ class ApiAbstract(metaclass=abc.ABCMeta):
         record_url = self._get_record_url(base_id, table_name, record_id)
 
         method = "put" if replace else "patch"
-        params = self._options_to_params(**options)
         return self._request(
             method,
             record_url,
             json_data={"fields": fields, "typecast": typecast},
-            params=params,
+            options=options,
         )
 
     def _batch_update(
@@ -218,14 +264,13 @@ class ApiAbstract(metaclass=abc.ABCMeta):
         updated_records = []
         table_url = self.get_table_url(base_id, table_name)
         method = "put" if replace else "patch"
-        params = self._options_to_params(**options)
         for records in self._chunk(records, self.MAX_RECORDS_PER_REQUEST):
             chunk_records = [{"id": x["id"], "fields": x["fields"]} for x in records]
             response = self._request(
                 method,
                 table_url,
                 json_data={"records": chunk_records, "typecast": typecast},
-                params=params,
+                options=options,
             )
             updated_records += response["records"]
             time.sleep(self.API_LIMIT)
diff --git a/pyairtable/api/api.py b/pyairtable/api/api.py
index 2707ab1..36375ef 100644
--- a/pyairtable/api/api.py
+++ b/pyairtable/api/api.py
@@ -215,7 +215,7 @@ class Api(ApiAbstract):
         (5 per second)
 
         >>> records = [{'Name': 'John'}, {'Name': 'Marc'}]
-        >>> api.batch_insert('base_id', 'table_name', records)
+        >>> api.batch_create('base_id', 'table_name', records)
 
         Args:
             base_id: |arg_base_id|
diff --git a/pyairtable/api/params.py b/pyairtable/api/params.py
index 772dab3..00b9777 100644
--- a/pyairtable/api/params.py
+++ b/pyairtable/api/params.py
@@ -1,5 +1,6 @@
+import warnings
 from collections import OrderedDict
-from typing import Any, Dict, List
+from typing import Any, Dict, List, Tuple
 
 
 class InvalidParamException(ValueError):
@@ -66,6 +67,12 @@ def field_names_to_sorting_dict(field_names: List[str]) -> List[Dict[str, str]]:
 
 def to_params_dict(param_name: str, value: Any):
     """Returns a dictionary for use in Request 'params'"""
+    warnings.warn(
+        "to_params_dict will be removed in 2.0.0",
+        DeprecationWarning,
+        stacklevel=2,
+    )
+
     if param_name == "max_records":
         return {"maxRecords": value}
     elif param_name == "view":
@@ -92,3 +99,85 @@ def to_params_dict(param_name: str, value: Any):
     else:
         msg = "'{0}' is not a supported parameter".format(param_name)
         raise InvalidParamException(msg)
+
+
+#: Mapping of pyairtable option names to Airtable parameter names
+OPTIONS_TO_PARAMETERS = {
+    "cell_format": "cellFormat",
+    "fields": "fields",
+    "formula": "filterByFormula",
+    "max_records": "maxRecords",
+    "offset": "offset",
+    "page_size": "pageSize",
+    "return_fields_by_field_id": "returnFieldsByFieldId",
+    "sort": "sort",
+    "time_zone": "timeZone",
+    "user_locale": "userLocale",
+    "view": "view",
+}
+
+
+def _option_to_param(name: str) -> str:
+    try:
+        return OPTIONS_TO_PARAMETERS[name]
+    except KeyError:
+        raise InvalidParamException(name)
+
+
+#: List of option names that cannot be passed via POST, only GET
+#: See https://github.com/gtalarico/pyairtable/pull/210#discussion_r1046014885
+OPTIONS_NOT_SUPPORTED_VIA_POST = ("user_locale", "time_zone")
+
+
+def options_to_params(options: Dict[str, Any]) -> Dict[str, Any]:
+    """
+    Converts Airtable options to a dict of query params.
+
+    Args:
+        options: A dict of Airtable-specific options. See :ref:`parameters`.
+
+    Returns:
+        A dict of query parameters that can be passed to the ``requests`` library.
+    """
+    params = {_option_to_param(name): value for (name, value) in options.items()}
+
+    if "fields" in params:
+        params["fields[]"] = params.pop("fields")
+    if "returnFieldsByFieldId" in params:
+        params["returnFieldsByFieldId"] = int(params["returnFieldsByFieldId"])
+    if "sort" in params:
+        sorting_dict_list = field_names_to_sorting_dict(params.pop("sort"))
+        params.update(dict_list_to_request_params("sort", sorting_dict_list))
+
+    return params
+
+
+def options_to_json_and_params(
+    options: Dict[str, Any]
+) -> Tuple[Dict[str, Any], Dict[str, Any]]:
+    """
+    Converts Airtable options to a JSON payload with (possibly) leftover query params.
+
+    Args:
+        options: A dict of Airtable-specific options. See :ref:`parameters`.
+
+    Returns:
+        A 2-tuple that contains the POST data and the non-POSTable query parameters.
+    """
+    json = {
+        _option_to_param(name): value
+        for (name, value) in options.items()
+        if name not in OPTIONS_NOT_SUPPORTED_VIA_POST
+    }
+    params = {
+        _option_to_param(name): value
+        for (name, value) in options.items()
+        if name in OPTIONS_NOT_SUPPORTED_VIA_POST
+    }
+
+    if "returnFieldsByFieldId" in json:
+        json["returnFieldsByFieldId"] = bool(json["returnFieldsByFieldId"])
+    if "sort" in json:
+        json["sort"] = field_names_to_sorting_dict(json.pop("sort"))
+
+    return (json, params)


Test patch:
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
index 3eb57c3..819000b 100644
--- a/tests/integration/conftest.py
+++ b/tests/integration/conftest.py
@@ -20,6 +20,7 @@ def cols():
     class Columns:
         # Table should have these Columns
         TEXT = "text"  # Text
+        TEXT_ID = "fldzbVdWW4xJdZ1em"  # for returnFieldsByFieldId
         NUM = "number"  # Number, float
         BOOL = "boolean"  # Boolean
         DATETIME = "datetime"  # Datetime
diff --git a/tests/integration/test_integration_api.py b/tests/integration/test_integration_api.py
index af84c4c..a55d611 100644
--- a/tests/integration/test_integration_api.py
+++ b/tests/integration/test_integration_api.py
@@ -58,6 +58,50 @@ def test_integration_table(table, cols):
     assert len(records) == COUNT
 
 
+@pytest.mark.integration
+def test_get_records_options(table: Table, cols):
+    rec = table.create({cols.TEXT: "abracadabra"})
+
+    # Test that each supported option works via GET
+    # (or, at least, that we don't get complaints from the API)
+    assert table.all(view="view") == [rec]
+    assert table.all(max_records=1) == [rec]
+    assert table.all(page_size=1) == [rec]
+    assert table.all(offset="") == [rec]
+    assert table.all(fields=[cols.TEXT, cols.NUM]) == [rec]
+    assert table.all(cell_format="json") == [rec]
+    assert table.all(sort=[cols.TEXT, cols.NUM]) == [rec]
+    assert table.all(time_zone="utc") == [rec]
+    assert table.all(user_locale="en-ie") == [rec]
+    assert table.all(return_fields_by_field_id=True) == [
+        {
+            "id": rec["id"],
+            "createdTime": rec["createdTime"],
+            "fields": {cols.TEXT_ID: rec["fields"][cols.TEXT]},
+        }
+    ]
+
+    # Test that each supported parameter works with a POST
+    # (or, at least, that we don't get complaints from the API)
+    formula = f"{cols.TEXT} != '{'x' * 17000}'"
+    assert table.all(formula=formula, view="view") == [rec]
+    assert table.all(formula=formula, max_records=1) == [rec]
+    assert table.all(formula=formula, page_size=1) == [rec]
+    assert table.all(formula=formula, offset="") == [rec]
+    assert table.all(formula=formula, fields=[cols.TEXT, cols.NUM]) == [rec]
+    assert table.all(formula=formula, cell_format="json") == [rec]
+    assert table.all(formula=formula, sort=[cols.TEXT, cols.NUM]) == [rec]
+    assert table.all(formula=formula, time_zone="utc") == [rec]
+    assert table.all(formula=formula, user_locale="en-ie") == [rec]
+    assert table.all(formula=formula, return_fields_by_field_id=True) == [
+        {
+            "id": rec["id"],
+            "createdTime": rec["createdTime"],
+            "fields": {cols.TEXT_ID: rec["fields"][cols.TEXT]},
+        }
+    ]
+
+
 @pytest.mark.integration
 def test_integration_field_equals(table: Table, cols):
     TEXT_VALUE = "Test {}".format(uuid4())
diff --git a/tests/test_api_table.py b/tests/test_api_table.py
index 2a83f45..6d74677 100644
--- a/tests/test_api_table.py
+++ b/tests/test_api_table.py
@@ -88,6 +88,22 @@ def test_first(table, mock_response_single):
     assert rv["id"] == mock_response_single["id"]
 
 
+def test_first_via_post(table, mock_response_single):
+    mock_response = {"records": [mock_response_single]}
+    with Mocker() as mock:
+        url = table.table_url + "/listRecords"
+        formula = f"RECORD_ID() != '{'x' * 17000}'"
+        mock_endpoint = mock.post(url, status_code=200, json=mock_response)
+        rv = table.first(formula=formula)
+
+    assert mock_endpoint.last_request.json() == {
+        "filterByFormula": formula,
+        "maxRecords": 1,
+        "pageSize": 1,
+    }
+    assert rv == mock_response_single
+
+
 def test_first_none(table, mock_response_single):
     mock_response = {"records": []}
     with Mocker() as mock:
diff --git a/tests/test_params.py b/tests/test_params.py
index 2d655d1..a81810f 100644
--- a/tests/test_params.py
+++ b/tests/test_params.py
@@ -6,6 +6,8 @@ from pyairtable.api.params import (
     InvalidParamException,
     dict_list_to_request_params,
     field_names_to_sorting_dict,
+    options_to_json_and_params,
+    options_to_params,
     to_params_dict,
 )
 
@@ -110,32 +112,104 @@ def test_params_integration(table, mock_records, mock_response_iterator):
         # ],
     ],
 )
-def test_process_params(option, value, url_params):
+def test_convert_options_to_params(option, value, url_params):
     """Ensure kwargs received build a proper params"""
-    # https://codepen.io/airtable/full/rLKkYB
+    processed_params = options_to_params({option: value})
+    request = requests.Request("get", "https://example.com", params=processed_params)
+    assert request.prepare().url.endswith(url_params)
+
+    # TODO: remove in 2.0.0
+    with pytest.deprecated_call():
+        processed_params = to_params_dict(option, value)
 
-    processed_params = to_params_dict(option, value)
-    request = requests.Request("get", "https://fake.com", params=processed_params)
+    request = requests.Request("get", "https://example.com", params=processed_params)
     assert request.prepare().url.endswith(url_params)
 
 
+@pytest.mark.parametrize(
+    "option,value,expected",
+    [
+        ["view", "SomeView", {"view": "SomeView"}],
+        ["max_records", 5, {"maxRecords": 5}],
+        ["page_size", 5, {"pageSize": 5}],
+        ["formula", "NOT(1)", {"filterByFormula": "NOT(1)"}],
+        ["formula", "NOT(1)", {"filterByFormula": "NOT(1)"}],
+        [
+            "formula",
+            "AND({COLUMN_ID}<=6, {COLUMN_ID}>3)",
+            {"filterByFormula": "AND({COLUMN_ID}<=6, {COLUMN_ID}>3)"},
+        ],
+        ["fields", ["Name"], {"fields": ["Name"]}],
+        [
+            "fields",
+            ["Name", "Phone"],
+            {"fields": ["Name", "Phone"]},
+        ],
+        [
+            "sort",
+            ["Name"],
+            {"sort": [{"field": "Name", "direction": "asc"}]},
+        ],
+        [
+            "sort",
+            ["Name", "Phone"],
+            {
+                "sort": [
+                    {"field": "Name", "direction": "asc"},
+                    {"field": "Phone", "direction": "asc"},
+                ]
+            },
+        ],
+        [
+            "sort",
+            ["Name", "-Phone"],
+            {
+                "sort": [
+                    {"field": "Name", "direction": "asc"},
+                    {"field": "Phone", "direction": "desc"},
+                ]
+            },
+        ],
+        ["cell_format", "string", {"cellFormat": "string"}],
+        ["return_fields_by_field_id", True, {"returnFieldsByFieldId": True}],
+        ["return_fields_by_field_id", 1, {"returnFieldsByFieldId": True}],
+        ["return_fields_by_field_id", False, {"returnFieldsByFieldId": False}],
+        # userLocale and timeZone are not supported via POST, so they return "spare params"
+        ["user_locale", "en-US", ({}, {"userLocale": "en-US"})],
+        ["time_zone", "America/Chicago", ({}, {"timeZone": "America/Chicago"})],
+    ],
+)
+def test_convert_options_to_json(option, value, expected):
+    if isinstance(expected, dict):
+        expected = (expected, {})  # most of the time, this returns empty "spare params"
+    assert options_to_json_and_params({option: value}) == expected
+
+
 def test_process_params_invalid():
     with pytest.raises(InvalidParamException):
-        to_params_dict("ffields", "x")
+        options_to_params({"ffields": "x"})
 
 
 def test_dict_list_to_request_params():
     values = [{"field": "a", "direction": "asc"}, {"field": "b", "direction": "desc"}]
     rv = dict_list_to_request_params("sort", values)
-    assert rv["sort[0][field]"] == "a"
-    assert rv["sort[0][direction]"] == "asc"
-    assert rv["sort[1][field]"] == "b"
-    assert rv["sort[1][direction]"] == "desc"
+    assert rv == {
+        "sort[0][field]": "a",
+        "sort[0][direction]": "asc",
+        "sort[1][field]": "b",
+        "sort[1][direction]": "desc",
+    }
 
 
 def test_field_names_to_sorting_dict():
     rv = field_names_to_sorting_dict(["Name", "-Age"])
-    assert rv[0]["field"] == "Name"
-    assert rv[0]["direction"] == "asc"
-    assert rv[1]["field"] == "Age"
-    assert rv[1]["direction"] == "desc"
+    assert rv == [
+        {
+            "field": "Name",
+            "direction": "asc",
+        },
+        {
+            "field": "Age",
+            "direction": "desc",
+        },
+    ]