Repository: gtalarico/pyairtable
Pull request #336 (2024-02-01 05:24:43)

Add `force=` kwarg to `base()` and `table()` methods
Calling `api.base(..., validate=True)` multiple times in a row causes excessive network traffic. This branch adds a new `force=` kwarg that determines whether we ignore cached metadata when validating an ID/name, and by default leaves that behavior off.

Along the way I noticed a test that was broken (it passed when it really shouldn't). Fixed that too.


Patch:
diff --git a/docs/source/_substitutions.rst b/docs/source/_substitutions.rst
index fc65137..d0fd0b8 100644
--- a/docs/source/_substitutions.rst
+++ b/docs/source/_substitutions.rst
@@ -1,6 +1,6 @@
-.. |arg_base_id| replace:: An Airtable base id.
+.. |arg_base_id| replace:: An Airtable base ID.
 
-.. |arg_record_id| replace:: An Airtable record id.
+.. |arg_record_id| replace:: An Airtable record ID.
 
 .. |kwarg_view| replace:: The name or ID of a view.
     If set, only the records in that view will be returned.
diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
index 4c2f091..004f249 100644
--- a/docs/source/changelog.rst
+++ b/docs/source/changelog.rst
@@ -2,6 +2,18 @@
 Changelog
 =========
 
+2.3.0 (TBD)
+------------------------
+
+* :meth:`Api.base <pyairtable.Api.base>`,
+  :meth:`Api.table <pyairtable.Api.table>`,
+  and :meth:`Base.table <pyairtable.Base.table>`
+  will use cached base metadata when called multiple times with ``validate=True``,
+  unless the caller passes a new keyword argument ``force=True``.
+  This allows callers to validate the IDs/names of many bases or tables at once
+  without having to perform expensive network overhead each time.
+  - `PR #336 <https://github.com/gtalarico/pyairtable/pull/336>`_.
+
 2.2.2 (2023-01-28)
 ------------------------
 
diff --git a/pyairtable/api/api.py b/pyairtable/api/api.py
index 682ff54..33319c4 100644
--- a/pyairtable/api/api.py
+++ b/pyairtable/api/api.py
@@ -105,6 +105,7 @@ class Api:
         base_id: str,
         *,
         validate: bool = False,
+        force: bool = False,
     ) -> "pyairtable.api.base.Base":
         """
         Return a new :class:`Base` instance that uses this instance of :class:`Api`.
@@ -112,13 +113,14 @@ class Api:
         Args:
             base_id: |arg_base_id|
             validate: |kwarg_validate_metadata|
+            force: |kwarg_force_metadata|
 
         Raises:
-            KeyError: if ``fetch=True`` and the given base ID does not exist.
+            KeyError: if ``validate=True`` and the given base ID does not exist.
         """
         if validate:
-            bases = {base.id: base for base in self.bases(force=True)}
-            return bases[base_id]
+            info = self._base_info(force=force).base(base_id)
+            return self._base_from_info(info)
         return pyairtable.api.base.Base(self, base_id)
 
     @cache_unless_forced
@@ -137,6 +139,14 @@ class Api:
             }
         )
 
+    def _base_from_info(self, base_info: Bases.Info) -> "pyairtable.api.base.Base":
+        return pyairtable.api.base.Base(
+            self,
+            base_info.id,
+            name=base_info.name,
+            permission_level=base_info.permission_level,
+        )
+
     def bases(self, *, force: bool = False) -> List["pyairtable.api.base.Base"]:
         """
         Retrieve the base's schema and return a list of :class:`Base` instances.
@@ -152,13 +162,7 @@ class Api:
             ]
         """
         return [
-            pyairtable.api.base.Base(
-                self,
-                info.id,
-                name=info.name,
-                permission_level=info.permission_level,
-            )
-            for info in self._base_info(force=force).bases
+            self._base_from_info(info) for info in self._base_info(force=force).bases
         ]
 
     def create_base(
@@ -180,11 +184,25 @@ class Api:
         """
         return self.workspace(workspace_id).create_base(name, tables)
 
-    def table(self, base_id: str, table_name: str) -> "pyairtable.api.table.Table":
+    def table(
+        self,
+        base_id: str,
+        table_name: str,
+        *,
+        validate: bool = False,
+        force: bool = False,
+    ) -> "pyairtable.api.table.Table":
         """
         Build a new :class:`Table` instance that uses this instance of :class:`Api`.
+
+        Args:
+            base_id: |arg_base_id|
+            table_name: The Airtable table's ID or name.
+            validate: |kwarg_validate_metadata|
+            force: |kwarg_force_metadata|
         """
-        return self.base(base_id).table(table_name)
+        base = self.base(base_id, validate=validate, force=force)
+        return base.table(table_name, validate=validate, force=force)
 
     def build_url(self, *components: str) -> str:
         """
diff --git a/pyairtable/api/base.py b/pyairtable/api/base.py
index 881fe43..75bf47a 100644
--- a/pyairtable/api/base.py
+++ b/pyairtable/api/base.py
@@ -59,7 +59,10 @@ class Base:
 
         Args:
             api: An instance of :class:`Api` or an Airtable access token.
-            base_id: |arg_base_id|
+            base_id: An Airtable base ID.
+            name: The name of the Airtable base, if known.
+            permission_level: The permission level the current authenticated user
+                has upon the Airtable base, if known.
         """
         if isinstance(api, str):
             warnings.warn(
@@ -98,6 +101,7 @@ class Base:
         id_or_name: str,
         *,
         validate: bool = False,
+        force: bool = False,
     ) -> "pyairtable.api.table.Table":
         """
         Build a new :class:`Table` instance using this instance of :class:`Base`.
@@ -106,13 +110,14 @@ class Base:
             id_or_name: An Airtable table ID or name. Table name should be unencoded,
                 as shown on browser.
             validate: |kwarg_validate_metadata|
+            force: |kwarg_force_metadata|
 
         Usage:
             >>> base.table('Apartments')
             <Table base='appLkNDICXNqxSDhG' name='Apartments'>
         """
         if validate:
-            schema = self.schema(force=True).table(id_or_name)
+            schema = self.schema(force=force).table(id_or_name)
             return pyairtable.api.table.Table(None, self, schema)
         return pyairtable.api.table.Table(None, self, id_or_name)
 
diff --git a/pyairtable/api/workspace.py b/pyairtable/api/workspace.py
index 04d73e2..fbecf48 100644
--- a/pyairtable/api/workspace.py
+++ b/pyairtable/api/workspace.py
@@ -46,7 +46,7 @@ class Workspace:
         url = self.api.build_url("meta/bases")
         payload = {"name": name, "workspaceId": self.id, "tables": list(tables)}
         response = self.api.request("POST", url, json=payload)
-        return self.api.base(response["id"], validate=True)
+        return self.api.base(response["id"], validate=True, force=True)
 
     # Everything below here requires .info() and is therefore Enterprise-only
 


Test patch:
diff --git a/tests/test_api_api.py b/tests/test_api_api.py
index 6ead168..6e5f32f 100644
--- a/tests/test_api_api.py
+++ b/tests/test_api_api.py
@@ -1,8 +1,15 @@
 from unittest import mock
 
+import pytest
+
 from pyairtable import Api, Base, Table  # noqa
 
 
+@pytest.fixture
+def mock_bases_endpoint(api, requests_mock, sample_json):
+    return requests_mock.get(api.build_url("meta/bases"), json=sample_json("Bases"))
+
+
 def test_repr(api):
     assert "Api" in api.__repr__()
 
@@ -61,19 +68,43 @@ def test_whoami(api, requests_mock):
     assert api.whoami() == payload
 
 
-def test_bases(api, requests_mock, sample_json):
-    m = requests_mock.get(api.build_url("meta/bases"), json=sample_json("Bases"))
+@pytest.mark.parametrize("base_id", ("appLkNDICXNqxSDhG", "Apartment Hunting"))
+def test_base(api, base_id, mock_bases_endpoint):
+    # test behavior without validation
+    base = api.base(base_id)
+    assert base.id == base_id
+    assert base.name is None
+    assert base.permission_level is None
+    assert mock_bases_endpoint.call_count == 0
+
+    # test behavior with validation
+    base = api.base(base_id, validate=True)
+    assert base.id == "appLkNDICXNqxSDhG"
+    assert base.name == "Apartment Hunting"
+    assert base.permission_level == "create"
+    assert mock_bases_endpoint.call_count == 1
+
+    # calling a second time uses cached information...
+    api.base(base_id, validate=True)
+    assert mock_bases_endpoint.call_count == 1
+
+    # ...unless we also pass force=True
+    base = api.base(base_id, validate=True, force=True)
+    assert mock_bases_endpoint.call_count == 2
+
+
+def test_bases(api, mock_bases_endpoint):
     base_ids = [base.id for base in api.bases()]
-    assert m.call_count == 1
+    assert mock_bases_endpoint.call_count == 1
     assert base_ids == ["appLkNDICXNqxSDhG", "appSW9R5uCNmRmfl6"]
 
     # Should not make a second API call...
     assert [base.id for base in api.bases()] == base_ids
-    assert m.call_count == 1
+    assert mock_bases_endpoint.call_count == 1
     # ....unless we force it:
     reloaded = api.bases(force=True)
     assert [base.id for base in reloaded] == base_ids
-    assert m.call_count == 2
+    assert mock_bases_endpoint.call_count == 2
 
 
 def test_iterate_requests(api: Api, requests_mock):
diff --git a/tests/test_api_base.py b/tests/test_api_base.py
index a9e3ab3..4223680 100644
--- a/tests/test_api_base.py
+++ b/tests/test_api_base.py
@@ -7,6 +7,11 @@ from pyairtable import Base, Table
 from pyairtable.testing import fake_id
 
 
+@pytest.fixture
+def mock_tables_endpoint(base, requests_mock, sample_json):
+    return requests_mock.get(base.meta_url("tables"), json=sample_json("BaseSchema"))
+
+
 def test_constructor(api):
     base = Base(api, "base_id")
     assert base.api == api
@@ -57,17 +62,16 @@ def test_url(base):
     assert base.url == "https://api.airtable.com/v0/appJMY16gZDQrMWpA"
 
 
-def test_schema(base: Base, requests_mock, sample_json):
-    m = requests_mock.get(base.meta_url("tables"), json=sample_json("BaseSchema"))
+def test_schema(base: Base, mock_tables_endpoint):
     table_schema = base.schema().table("tbltp8DGLhqbUmjK1")
     assert table_schema.name == "Apartments"
-    assert m.call_count == 1
+    assert mock_tables_endpoint.call_count == 1
 
     # Test that we cache the result unless force=True
     base.schema()
-    assert m.call_count == 1
+    assert mock_tables_endpoint.call_count == 1
     base.schema(force=True)
-    assert m.call_count == 2
+    assert mock_tables_endpoint.call_count == 2
 
 
 def test_table(base: Base, requests_mock):
@@ -79,25 +83,30 @@ def test_table(base: Base, requests_mock):
     assert rv.url == f"https://api.airtable.com/v0/{base.id}/tablename"
 
 
-def test_table_validate(base: Base, requests_mock, sample_json):
+def test_table_validate(base: Base, mock_tables_endpoint):
     """
     Test that Base.table(..., validate=True) allows us to look up a table
     by either ID or name and get the correct properties.
     """
-    m = requests_mock.get(base.meta_url("tables"), json=sample_json("BaseSchema"))
+    # It will reuse the cached schema when validate=True is called multiple times...
     base.table("tbltp8DGLhqbUmjK1", validate=True)
     base.table("Apartments", validate=True)
-    assert m.call_count == 2
-    # ...and will raise an exception if called with an invalid ID/name:
+    assert mock_tables_endpoint.call_count == 1
+    # ...unless we also pass force=True
+    base.table("Apartments", validate=True, force=True)
+    assert mock_tables_endpoint.call_count == 2
+
+
+def test_table__invalid(base, mock_tables_endpoint):
+    # validate=True will raise an exception if called with an invalid ID/name:
     with pytest.raises(KeyError):
         base.table("DoesNotExist", validate=True)
 
 
-def test_tables(base: Base, requests_mock, sample_json):
+def test_tables(base: Base, mock_tables_endpoint):
     """
     Test that Base.tables() returns a list of validated Base instances.
     """
-    requests_mock.get(base.meta_url("tables"), json=sample_json("BaseSchema"))
     result = base.tables()
     assert len(result) == 2
     assert result[0].name == "Apartments"
@@ -199,26 +208,44 @@ def test_name(api, base, requests_mock):
         assert Base("tok", "app", name="Base Name").name == "Base Name"
 
 
-def test_create_table(base, requests_mock, sample_json):
+def test_create_table(base, requests_mock, mock_tables_endpoint):
     """
     Test that Base.create_table() makes two calls, one to create the table,
     and another to re-retrieve the entire base's schema.
     """
-    schema = sample_json("BaseSchema")
-    url = base.meta_url("tables")
-    m = requests_mock.post(url, json={"id": "tbltp8DGLhqbUmjK1"})
-    m_get = requests_mock.get(url + "?include=visibleFieldIds", json=schema)
+    m = requests_mock.post(mock_tables_endpoint._url, json={"id": "tblWasJustCreated"})
+    mock_tables_endpoint._responses[0]._params["json"]["tables"].append(
+        {
+            "id": "tblWasJustCreated",
+            "name": "Table Name",
+            "primaryFieldId": "fldWasJustCreated",
+            "fields": [
+                {
+                    "id": "fldWasJustCreated",
+                    "name": "Whatever",
+                    "type": "singleLineText",
+                }
+            ],
+            "views": [],
+        }
+    )
     table = base.create_table(
-        "Table Name", [{"name": "Whatever"}], description="Description"
+        "Table Name",
+        fields=[{"name": "Whatever"}],
+        description="Description",
     )
-    assert isinstance(table, Table)
-    assert m.call_count == m_get.call_count == 1
+    assert m.call_count == 1
     assert m.request_history[-1].json() == {
         "name": "Table Name",
         "description": "Description",
         "fields": [{"name": "Whatever"}],
     }
 
+    assert isinstance(table, Table)
+    assert table.id == "tblWasJustCreated"
+    assert table.name == "Table Name"
+    assert table.schema().primary_field_id == "fldWasJustCreated"
+
 
 def test_delete(base, requests_mock):
     """