Improved robustness of controllers

Author: forman

chartlets.py/chartlets/controllers/_helpers.py

@@ -0,0 +1,44 @@
+from chartlets import Contribution
+from chartlets import ExtensionContext
+from chartlets import Response
+from chartlets.util.assertions import assert_is_not_empty
+from chartlets.util.assertions import assert_is_instance_of
+
+
+def get_contribution(
+    ext_ctx: ExtensionContext,
+    contrib_point_name: str,
+    contrib_index: int,
+) -> tuple[Contribution, None] | tuple[None, Response]:
+    """Get the contribution for given `contrib_point_name` at `contrib_index`.
+
+    Args:
+        ext_ctx: Extension context.
+        contrib_point_name: Contribution point name.
+        contrib_index: Contribution index.
+    Returns:
+        A pair comprising an optional `Contribution` and optional `Response` object.
+    """
+    assert_is_instance_of("ext_ctx", ext_ctx, ExtensionContext)
+    assert_is_not_empty("contrib_point_name", contrib_point_name)
+    assert_is_instance_of("contrib_index", contrib_index, int)
+
+    try:
+        contributions = ext_ctx.contributions[contrib_point_name]
+    except KeyError:
+        return None, Response.failed(
+            404, f"contribution point {contrib_point_name!r} not found"
+        )
+
+    try:
+        contribution = contributions[contrib_index]
+    except IndexError:
+        return None, Response.failed(
+            404,
+            (
+                f"index range of contribution point {contrib_point_name!r} is"
+                f" 0 to {len(contributions) - 1}, got {contrib_index}"
+            ),
+        )
+
+    return contribution, None

chartlets.py/chartlets/controllers/callback.py

@@ -4,6 +4,7 @@
 from chartlets.response import Response
 from chartlets.util.assertions import assert_is_instance_of
 from chartlets.util.assertions import assert_is_not_none
+from ._helpers import get_contribution
 
 
 # POST /chartlets/callback
@@ -29,22 +30,42 @@ def get_callback_results(
     callback_requests: list[dict] = data.get("callbackRequests") or []
 
     # TODO: assert correctness, set status code on error
-    state_change_requests: list[dict] = []
+    state_change_requests: list[dict[str, Any]] = []
     for callback_request in callback_requests:
         contrib_point_name: str = callback_request["contribPoint"]
         contrib_index: int = callback_request["contribIndex"]
         callback_index: int = callback_request["callbackIndex"]
         input_values: list = callback_request["inputValues"]
 
-        contributions = ext_ctx.contributions[contrib_point_name]
-        contribution = contributions[contrib_index]
-        callback = contribution.callbacks[callback_index]
+        contribution, response = get_contribution(
+            ext_ctx, contrib_point_name, contrib_index
+        )
+        if response is not None:
+            return response
+
+        callbacks = contribution.callbacks
+        if not callbacks:
+            return Response.failed(
+                400, f"contribution {contribution.name!r} has no callbacks"
+            )
+
+        try:
+            callback = callbacks[callback_index]
+        except IndexError:
+            return Response.failed(
+                404,
+                (
+                    f"index range of callbacks of contribution {contribution.name!r} is"
+                    f" 0 to {len(callbacks) - 1}, got {callback_index}"
+                ),
+            )
+
         output_values = callback.invoke(ext_ctx.app_ctx, input_values)
 
         if len(callback.outputs) == 1:
             output_values = (output_values,)
 
-        state_changes: list[dict] = []
+        state_changes: list[dict[str, Any]] = []
         for output_index, output in enumerate(callback.outputs):
             output_value = output_values[output_index]
             state_changes.append(
@@ -59,12 +80,23 @@ def get_callback_results(
                 }
             )
 
-        state_change_requests.append(
-            {
-                "contribPoint": contrib_point_name,
-                "contribIndex": contrib_index,
-                "stateChanges": state_changes,
-            }
-        )
+        existing_scr: dict[str, Any] | None = None
+        for scr in state_change_requests:
+            if (
+                scr["contribPoint"] == contrib_point_name
+                and scr["contribIndex"] == contrib_index
+            ):
+                existing_scr = scr
+                break
+        if existing_scr is not None:
+            existing_scr["stateChanges"].extend(state_changes)
+        else:
+            state_change_requests.append(
+                {
+                    "contribPoint": contrib_point_name,
+                    "contribIndex": contrib_index,
+                    "stateChanges": state_changes,
+                }
+            )
 
     return Response.success(state_change_requests)

chartlets.py/chartlets/controllers/layout.py

@@ -2,9 +2,8 @@
 
 from chartlets.extensioncontext import ExtensionContext
 from chartlets.response import Response
-from chartlets.util.assertions import assert_is_not_none
-from chartlets.util.assertions import assert_is_not_empty
 from chartlets.util.assertions import assert_is_instance_of
+from ._helpers import get_contribution
 
 
 def get_layout(
@@ -28,31 +27,16 @@ def get_layout(
         On success, the response is a dictionary that represents
         a JSON-serialized component tree.
     """
-    assert_is_not_none("ext_ctx", ext_ctx)
-    assert_is_not_empty("contrib_point_name", contrib_point_name)
-    assert_is_instance_of("contrib_index", contrib_index, int)
     assert_is_instance_of("data", data, dict)
 
     # TODO: validate data
     input_values = data.get("inputValues") or []
 
-    try:
-        contributions = ext_ctx.contributions[contrib_point_name]
-    except KeyError:
-        return Response.failed(
-            404, f"contribution point {contrib_point_name!r} not found"
-        )
-
-    try:
-        contribution = contributions[contrib_index]
-    except IndexError:
-        return Response.failed(
-            404,
-            (
-                f"index range of contribution point {contrib_point_name!r} is"
-                f" 0 to {len(contributions) - 1}, got {contrib_index}"
-            ),
-        )
+    contribution, response = get_contribution(
+        ext_ctx, contrib_point_name, contrib_index
+    )
+    if response is not None:
+        return response
 
     callback = contribution.layout_callback
     if callback is None:

chartlets.py/chartlets/util/assertions.py

@@ -30,5 +30,6 @@ def assert_is_instance_of(name: str, value: Any, type_set: Type | tuple[Type, ..
             type_set = (type_set,)
         raise TypeError(
             f"value of {name!r} must be of type"
-            f" {" or ".join(map(lambda t: t.__name__, type_set))}, but was {type(value).__name__}"
+            f" {" or ".join(map(lambda t: t.__name__, type_set))},"
+            f" but was {'None' if value is None else type(value).__name__}"
         )

chartlets.py/tests/components/charts/vega_no_altair_test.py

@@ -10,14 +10,17 @@ def test_no_altair(monkeypatch: pytest.MonkeyPatch):
     of "altair" gracefully.
     """
     project_root = pathlib.Path(__file__).absolute().parent
-    while project_root.parent != project_root and not (project_root / "chartlets" / "__init__.py").exists():
+    while (
+        project_root.parent != project_root
+        and not (project_root / "chartlets" / "__init__.py").exists()
+    ):
         project_root = project_root.parent
 
     # Simulate the absence of the 'altair' module
     print("project_root:", project_root)
     monkeypatch.setattr(sys, "path", [f"{project_root}"])
     if "altair" in sys.modules:
-        monkeypatch.delitem(sys.modules,"altair")
+        monkeypatch.delitem(sys.modules, "altair")
 
     # Import the code that handles the missing "altair" package
     importlib.invalidate_caches()