refactor Meta to subclass from a dict with added value restrictions and key requirements

Author: BurnzZ

tests/test_page_inputs.py

@@ -1,3 +1,6 @@
+import pytest
+import asyncio
+
 from web_poet.page_inputs import ResponseData, Meta
 
 
@@ -13,21 +16,30 @@ def test_html_response():
     assert response.headers["User-Agent"] == "test agent"
 
 
-def test_meta():
-    meta = Meta(x="hi", y=2.2, z={"k": "v"})
-    assert meta.x == "hi"
-    assert meta.y == 2.2
-    assert meta.z == {"k": "v"}
-    assert meta.not_existing_field is None
+def test_meta_required_data():
+    Meta()  # By default, no fields are required.
+
+    class RequiredMeta(Meta):
+        required_data = {"some_data"}
+
+    with pytest.raises(ValueError):
+        RequiredMeta()
+
+    meta = RequiredMeta(some_data=123)
+    assert meta["some_data"] == 123
+
 
-    del meta.z
-    assert meta.z is None
+def test_meta_restriction():
+    # Any value that conforms with `Meta.restrictions` raises an error
+    with pytest.raises(ValueError) as err:
+        Meta(func=lambda x: x + 1)
 
-    # Deleting non-existing fields should not err out.
-    del meta.no_existing_field
-    assert meta.not_existing_field is None
+    with pytest.raises(ValueError) as err:
+        Meta(class_=ResponseData)
 
-    meta.new_field = "new"
-    assert meta.new_field == "new"
+    # These are allowed though
+    m = Meta(x="hi", y=2.2, z={"k": "v"})
+    m["allowed"] = [1, 2, 3]
 
-    str(meta) == "Meta(x='hi', y=2.2, new='new')"
+    with pytest.raises(ValueError) as err:
+        m["not_allowed"] = asyncio.sleep(1)

web_poet/page_inputs.py

@@ -1,4 +1,5 @@
-from typing import Optional, Dict, Any, ByteString, Union
+import inspect
+from typing import Optional, Dict, Any, ByteString, Union, Set
 from contextlib import suppress
 
 import attr
@@ -32,35 +33,65 @@ class ResponseData:
     headers: Optional[Dict[Union[str, ByteString], Any]] = None
 
 
-class Meta:
-    """Container class that could contain any arbitrary data.
+class Meta(dict):
+    """Container class that could contain any arbitrary data to be passed into
+    a Page Object.
 
-    Using this is more useful to pass things around compared to a ``dict`` due
-    to these following characteristics:
+    This is basically a subclass of a ``dict`` but adds some additional
+    functionalities to ensure consistent and compatible Page Objects across
+    different use cases:
 
-        - You can use Python's "." attribute syntax for it.
-        - Accessing attributes that are not existing won't result in errors.
-          Instead, a ``None`` value will be returned.
-        - The same goes for deleting attributes that don't exist wherein errors
-          will be suppressed.
+    * A class variable named ``required_data`` to ensure consistent
+      arguments. If it's instantiated with missing ``keys`` from
+      ``required_data``, then a ``ValueError`` is raised.
 
-    This makes the code simpler by avoiding try/catch, checking an attribute's
-    existence, using ``get()``, etc.
+    * Ensures that some params with data types that are difficult to
+      provide or pass like ``lambdas`` are checked. Otherwise, a ``ValueError``
+      is raised.
     """
 
-    def __init__(self, **kwargs):
-        object.__setattr__(self, "_data", kwargs)
-
-    def __getattr__(self, key):
-        return self._data.get(key)
-
-    def __delattr__(self, key):
-        with suppress(KeyError):
-            del self._data[key]
-
-    def __setattr__(self, key, value):
-        self._data[key] = value
-
-    def __repr__(self):
-        contents = ", ".join([f"{k}={v!r}" for k, v in self._data.items()])
-        return f"Meta({contents})"
+    # Contains the required "keys" when instantiating and setting attributes.
+    required_data: Set = set()
+
+    # Any "value" that returns True for the functions here are not allowed.
+    restrictions: Dict = {
+        inspect.ismodule: "module",
+        inspect.isclass: "class",
+        inspect.ismethod: "method",
+        inspect.isfunction: "function",
+        inspect.isgenerator: "generator",
+        inspect.isgeneratorfunction: "generator",
+        inspect.iscoroutine: "coroutine",
+        inspect.isawaitable: "awaitable",
+        inspect.istraceback: "traceback",
+        inspect.isframe: "frame",
+    }
+
+    def __init__(self, *args, **kwargs) -> None:
+        missing_required_keys = self.required_data - kwargs.keys()
+        if missing_required_keys:
+            raise ValueError(
+                f"These keys are required for instantiation: {missing_required_keys}"
+            )
+        for val in kwargs.values():
+            self.is_restricted_value(val)
+        super().__init__(*args, **kwargs)
+
+    def __setitem__(self, key: Any, value: Any) -> None:
+        self.is_restricted_value(value)
+        super().__setattr__(key, value)
+
+    def is_restricted_value(self, value: Any) -> None:
+        """Raises an error if a given value isn't allowed inside the meta.
+
+        This behavior can be controlled by tweaking the class variable
+        :meth:`~.web_poet.page_inputs.Meta.restrictions`.
+        """
+        violations = []
+
+        for restrictor, err in self.restrictions.items():
+            if restrictor(value):
+                violations.append(f"{err} is not allowed: {value}")
+
+        if violations:
+            raise ValueError(f"Found these issues: {', '.join(violations)}")