Repository: gtalarico/pyairtable
Pull request #246 (2023-04-27 05:52:52)

Fix regression introduced by `endpoint_url=`
As @goksan pointed out [here](https://github.com/gtalarico/pyairtable/pull/243#issuecomment-1523935537), there is untested behavior that regressed in my previous commit (0189d9d). When calling `api.get_base()` we need to ensure the version number does not get appended to the URL twice.

Rather than second-guessing and modifying constructor arguments, I introduced `ApiAbstract.build_url()` that inserts the correct version number in front of whatever URL components the caller needs to access. I've added tests to check that `.get_table()` doesn't wind up with `/v0/v0` in the URL again.

(I'm planning to merge this immediately but I'm creating a PR for the paper trail.)


Patch:
diff --git a/pyairtable/api/abstract.py b/pyairtable/api/abstract.py
index 4c90ebb..57c8280 100644
--- a/pyairtable/api/abstract.py
+++ b/pyairtable/api/abstract.py
@@ -21,7 +21,7 @@ class ApiAbstract(metaclass=abc.ABCMeta):
 
     session: Session
     endpoint_url: str
-    tiemout: TimeoutTuple
+    timeout: Optional[TimeoutTuple]
 
     def __init__(
         self,
@@ -35,7 +35,7 @@ class ApiAbstract(metaclass=abc.ABCMeta):
         else:
             self.session = _RetryingSession(retry_strategy)
 
-        self.endpoint_url = posixpath.join(endpoint_url, self.VERSION)
+        self.endpoint_url = endpoint_url
         self.timeout = timeout
         self.api_key = api_key
 
@@ -50,14 +50,20 @@ class ApiAbstract(metaclass=abc.ABCMeta):
         self._update_api_key(value)
         self._api_key = value
 
+    def build_url(self, *components: str) -> str:
+        """
+        Returns a URL to the Airtable API endpoint with the given URL components,
+        including the API version number.
+        """
+        return posixpath.join(self.endpoint_url, self.VERSION, *components)
+
     def _update_api_key(self, api_key: str) -> None:
         self.session.headers.update({"Authorization": "Bearer {}".format(api_key)})
 
     @lru_cache()
     def get_table_url(self, base_id: str, table_name: str):
         url_safe_table_name = quote(table_name, safe="")
-        table_url = posixpath.join(self.endpoint_url, base_id, url_safe_table_name)
-        return table_url
+        return self.build_url(base_id, url_safe_table_name)
 
     @lru_cache()
     def _get_record_url(self, base_id: str, table_name: str, record_id):
diff --git a/pyairtable/api/table.py b/pyairtable/api/table.py
index 810d325..b2e2750 100644
--- a/pyairtable/api/table.py
+++ b/pyairtable/api/table.py
@@ -40,7 +40,6 @@ class Table(ApiAbstract):
         """
         self.base_id = base_id
         self.table_name = table_name
-        self.endpoint_url = endpoint_url
         super().__init__(
             api_key,
             timeout=timeout,
diff --git a/pyairtable/metadata.py b/pyairtable/metadata.py
index 5245cfe..16c8462 100644
--- a/pyairtable/metadata.py
+++ b/pyairtable/metadata.py
@@ -1,4 +1,3 @@
-import posixpath
 from typing import Optional, Union
 
 from pyairtable.api import Api, Base, Table
@@ -29,7 +28,7 @@ def get_api_bases(api: Union[Api, Base]) -> dict:
                 ]
             }
     """
-    base_list_url = posixpath.join(api.endpoint_url, "meta", "bases")
+    base_list_url = api.build_url("meta", "bases")
     return api._request("get", base_list_url)
 
 
@@ -77,9 +76,7 @@ def get_base_schema(base: Union[Base, Table]) -> dict:
                 ]
             }
     """
-    base_schema_url = posixpath.join(
-        base.endpoint_url, "meta", "bases", base.base_id, "tables"
-    )
+    base_schema_url = base.build_url("meta", "bases", base.base_id, "tables")
     return base._request("get", base_schema_url)
 
 


Test patch:
diff --git a/tests/test_api_api.py b/tests/test_api_api.py
index 8749448..5037fb6 100644
--- a/tests/test_api_api.py
+++ b/tests/test_api_api.py
@@ -1,6 +1,6 @@
 from unittest import mock
 
-from pyairtable import Api, Table
+from pyairtable import Api, Base, Table
 from pyairtable.api.abstract import ApiAbstract
 
 
@@ -13,27 +13,36 @@ def test_record_url(api: Api):
     assert rv == ApiAbstract("x")._get_record_url("baseid", "tablename", "rec")
 
 
+def test_get_base(api: Api):
+    rv = api.get_base("appTest")
+    assert isinstance(rv, Base)
+    assert rv.base_id == "appTest"
+    assert rv.endpoint_url == api.endpoint_url
+
+
 def test_get_table(api: Api):
-    rv = api.get_table("x", "y")
+    rv = api.get_table("appTest", "tblTest")
     assert isinstance(rv, Table)
-    assert rv.base_id == "x"
-    assert rv.table_name == "y"
+    assert rv.base_id == "appTest"
+    assert rv.table_name == "tblTest"
+    assert rv.endpoint_url == api.endpoint_url
+    assert rv.table_url == "https://api.airtable.com/v0/appTest/tblTest"
 
 
 def test_default_endpoint_url(api: Api):
-    assert api.endpoint_url == "https://api.airtable.com/" + api.VERSION
+    rv = api.build_url("appTest", "tblTest")
+    assert rv == "https://api.airtable.com/v0/appTest/tblTest"
 
 
 def test_endpoint_url(api_with_endpoint_url: Api):
-    assert (
-        api_with_endpoint_url.endpoint_url
-        == "https://api.example.com/" + api_with_endpoint_url.VERSION
-    )
+    rv = api_with_endpoint_url.build_url("appTest", "tblTest")
+    assert rv == "https://api.example.com/v0/appTest/tblTest"
 
 
 def test_endpoint_url_with_trailing_slash():
     api = Api("apikey", endpoint_url="https://api.example.com/")
-    assert api.endpoint_url == "https://api.example.com/" + api.VERSION
+    rv = api.build_url("appTest", "tblTest")
+    assert rv == "https://api.example.com/v0/appTest/tblTest"
 
 
 @mock.patch.object(ApiAbstract, "_get_record")
diff --git a/tests/test_api_base.py b/tests/test_api_base.py
index 02aac9e..f5aff12 100644
--- a/tests/test_api_base.py
+++ b/tests/test_api_base.py
@@ -10,7 +10,7 @@ def test_repr(base):
 
 def test_record_url(base: Base):
     rv = base.get_record_url("tablename", "rec")
-    assert rv == ApiAbstract("x")._get_record_url(base.base_id, "tablename", "rec")
+    assert rv == f"https://api.airtable.com/v0/{base.base_id}/tablename/rec"
 
 
 def test_get_table(base: Base):
@@ -18,6 +18,7 @@ def test_get_table(base: Base):
     assert isinstance(rv, Table)
     assert rv.base_id == base.base_id
     assert rv.table_name == "tablename"
+    assert rv.table_url == f"https://api.airtable.com/v0/{base.base_id}/tablename"
 
 
 @mock.patch.object(ApiAbstract, "_get_record")
diff --git a/tests/test_api_table.py b/tests/test_api_table.py
index 4791fba..2a83f45 100644
--- a/tests/test_api_table.py
+++ b/tests/test_api_table.py
@@ -21,7 +21,7 @@ def test_repr(table):
 )
 def test_url(base_id, table_name, table_url_suffix):
     table = Table("apikey", base_id, table_name)
-    assert table.table_url == "{0}/{1}".format(table.endpoint_url, table_url_suffix)
+    assert table.table_url == f"https://api.airtable.com/v0/{table_url_suffix}"
 
 
 def test_chunk(table):