Repository: gtalarico/pyairtable
Pull request #357 (2024-03-22 05:04:05)

Do not call orm.Model.Meta callables during `__init_subclass__`
The whole idea behind allowing Meta values to be callables (#268) was to defer evaluation until after the Model subclass is created. However, these callables still get evaluated when the subclass is created, since we try to validate that base_id, table_name, and api_key are defined.

This branch will validate that those three names _exist_ inside `Meta`, but will defer raising an exception if they are callable and return `None` until the first time they get used.


Patch:
diff --git a/pyairtable/orm/model.py b/pyairtable/orm/model.py
index acfcdaa..461f983 100644
--- a/pyairtable/orm/model.py
+++ b/pyairtable/orm/model.py
@@ -142,13 +142,25 @@ class Model:
             setattr(self, key, value)
 
     @classmethod
-    def _get_meta(cls, name: str, default: Any = None, required: bool = False) -> Any:
+    def _get_meta(
+        cls, name: str, default: Any = None, required: bool = False, call: bool = True
+    ) -> Any:
+        """
+        Retrieves the value of a Meta attribute.
+
+        Args:
+            default: The default value to return if the attribute is not set.
+            required: Raise an exception if the attribute is not set.
+            call: If the value is callable, call it before returning a result.
+        """
         if not hasattr(cls, "Meta"):
             raise AttributeError(f"{cls.__name__}.Meta must be defined")
-        if required and not hasattr(cls.Meta, name):
-            raise ValueError(f"{cls.__name__}.Meta.{name} must be defined")
-        value = getattr(cls.Meta, name, default)
-        if callable(value):
+        if not hasattr(cls.Meta, name):
+            if required:
+                raise ValueError(f"{cls.__name__}.Meta.{name} must be defined")
+            return default
+        value = getattr(cls.Meta, name)
+        if call and callable(value):
             value = value()
         if required and value is None:
             raise ValueError(f"{cls.__name__}.Meta.{name} cannot be None")
@@ -156,10 +168,10 @@ class Model:
 
     @classmethod
     def _validate_class(cls) -> None:
-        # Verify required Meta attributes were set
-        assert cls._get_meta("api_key", required=True)
-        assert cls._get_meta("base_id", required=True)
-        assert cls._get_meta("table_name", required=True)
+        # Verify required Meta attributes were set (but don't call any callables)
+        assert cls._get_meta("api_key", required=True, call=False)
+        assert cls._get_meta("base_id", required=True, call=False)
+        assert cls._get_meta("table_name", required=True, call=False)
 
         model_attributes = [a for a in cls.__dict__.keys() if not a.startswith("__")]
         overridden = set(model_attributes).intersection(Model.__dict__.keys())
@@ -174,17 +186,17 @@ class Model:
     @lru_cache
     def get_api(cls) -> Api:
         return Api(
-            api_key=cls._get_meta("api_key"),
+            api_key=cls._get_meta("api_key", required=True),
             timeout=cls._get_meta("timeout"),
         )
 
     @classmethod
     def get_base(cls) -> Base:
-        return cls.get_api().base(cls._get_meta("base_id"))
+        return cls.get_api().base(cls._get_meta("base_id", required=True))
 
     @classmethod
     def get_table(cls) -> Table:
-        return cls.get_base().table(cls._get_meta("table_name"))
+        return cls.get_base().table(cls._get_meta("table_name", required=True))
 
     @classmethod
     def _typecast(cls) -> bool:


Test patch:
diff --git a/tests/test_orm_model.py b/tests/test_orm_model.py
index a95d522..133e6e2 100644
--- a/tests/test_orm_model.py
+++ b/tests/test_orm_model.py
@@ -51,6 +51,22 @@ def test_model_empty_meta():
             street = f.TextField("Street")
 
 
+def test_model_empty_meta_with_callable():
+    """
+    Test that we throw an exception when a required Meta attribute is
+    defined as a callable which returns None.
+    """
+
+    class Address(Model):
+        Meta = fake_meta(api_key=lambda: None)
+        street = f.TextField("Street")
+
+    with mock.patch("pyairtable.Table.first", return_value=fake_record()) as m:
+        with pytest.raises(ValueError):
+            Address.first()
+        m.assert_not_called()
+
+
 @pytest.mark.parametrize("name", ("exists", "id"))
 def test_model_overlapping(name):
     """
@@ -197,7 +213,8 @@ def test_passthrough(methodname):
 def test_dynamic_model_meta():
     """
     Test that we can provide callables in our Meta class to provide
-    the access token, base ID, and table name at runtime.
+    the access token, base ID, and table name at runtime. Also ensure
+    that callable Meta attributes don't get called until they're needed.
     """
     data = {
         "api_key": "FakeApiKey",
@@ -209,12 +226,12 @@ def test_dynamic_model_meta():
         class Meta:
             api_key = lambda: data["api_key"]  # noqa
             base_id = partial(data.get, "base_id")
-
-            @staticmethod
-            def table_name():
-                return data["table_name"]
+            table_name = mock.Mock(return_value=data["table_name"])
 
     f = Fake()
+    Fake.Meta.table_name.assert_not_called()
+
     assert f._get_meta("api_key") == data["api_key"]
     assert f._get_meta("base_id") == data["base_id"]
     assert f._get_meta("table_name") == data["table_name"]
+    Fake.Meta.table_name.assert_called_once()