Merge pull request #109 from piercefreeman/feature/request-aware-layouts

Support query parameters and overlapping render signatures between layouts and child pages

ci_webapp/ci_webapp/cli.py

@@ -1,5 +1,5 @@
 from click import command
-from mountaineer.cli import handle_runserver, handle_watch
+from mountaineer.cli import handle_build, handle_runserver, handle_watch
 
 
 @command()
@@ -20,3 +20,10 @@ def watch():
         webcontroller="ci_webapp.app:controller",
         subscribe_to_mountaineer=True,
     )
+
+
+@command()
+def build():
+    handle_build(
+        webcontroller="ci_webapp.app:controller",
+    )

ci_webapp/ci_webapp/controllers/root_layout.py

@@ -4,6 +4,7 @@
 
 class RootLayoutRender(RenderBase):
     layout_value: int
+    layout_arg: int
 
 
 class RootLayoutController(LayoutControllerBase):
@@ -13,9 +14,10 @@ def __init__(self):
         super().__init__()
         self.layout_value = 0
 
-    def render(self) -> RootLayoutRender:
+    def render(self, layout_arg: int | None = None) -> RootLayoutRender:
         return RootLayoutRender(
             layout_value=self.layout_value,
+            layout_arg=layout_arg or 0,
         )
 
     @sideeffect

ci_webapp/ci_webapp/views/app/layout.tsx

@@ -6,7 +6,9 @@ const Layout = ({ children }: { children: ReactNode }) => {
 
   return (
     <div className="p-6">
-      <h1>Layout State: {serverState.layout_value}</h1>
+      <h1>
+        Layout State: {serverState.layout_value} : {serverState.layout_arg}
+      </h1>
       <div>{children}</div>
       <div>
         <button

ci_webapp/pyproject.toml

@@ -14,6 +14,7 @@ mountaineer = { path = "../", develop = true }
 [tool.poetry.scripts]
 runserver = "ci_webapp.cli:runserver"
 watch = "ci_webapp.cli:watch"
+build = "ci_webapp.cli:build"
 
 [tool.poetry.group.dev.dependencies]
 types-setuptools = "^69.0.0.20240125"

docs_website/docs/views.md

@@ -239,13 +239,9 @@ app_controller = AppController(...)
 app_controller.register(RootLayoutController())
 ```
 
-In general you can implement layout controllers just like you do for pages. But since they're shared across multiple pages there are a few important differences to keep in mind:
+In general you can implement layout controllers just like you do for pages. But since they're shared across multiple child controllers, make sure the keyword arguments you use in your `render` signature don't have any conflicts. Mountaineer will merge these signatures at runtime and check for duplicate keyword names across the layout's child pages. Arguments are allowed to share the same name _and_ type, in which case they will be resolved to the same value. Arguments with conflicting types will raise a `TypeError`.
 
-- Layout controllers will be rendered in an isolated scope. Sideeffects in one layout controller won't affect the others.
-- Dependency injections are similarly isolated. They are run in an isolated, synthetic context and not with the same dependency injection parameters that the page uses.
-- Layout controllers don't modify the page signature. Query params on layouts won't be extracted, for instance.
-
-As long as you write your layout controllers without directly referencing the page that they might be wrapping, which is the case for most layouts, you should be good to go.
+It's also worth noting that layout controllers will resolve their dependencies in the same scope as the page controllers. So if you need database access within your layout, you'll receive the same underlying transaction as the page controller. This makes dependency injection a powerful way to save on resources, but be careful to not treat them as isolated objects.
 
 ## Typescript Configuration
 

mountaineer/__tests__/actions/test_fields.py

@@ -1,4 +1,4 @@
-from typing import AsyncIterator, Iterator, Optional, Type, cast
+from typing import AsyncIterator, Iterator, Optional, Type
 
 import pytest
 from fastapi.responses import JSONResponse
@@ -124,24 +124,19 @@ def test_fuse_metadata_to_response_typehint(
         metadata, sample_controller, render_model
     )
 
-    assert "ExampleController" in raw_model.model_fields.keys()
-
-    fused_model = cast(
-        BaseModel, raw_model.model_fields["ExampleController"].annotation
-    )
     if expected_sideeffect_fields:
-        assert "sideeffect" in fused_model.model_fields.keys()
-        assert fused_model.model_fields["sideeffect"].annotation
+        assert "sideeffect" in raw_model.model_fields.keys()
+        assert raw_model.model_fields["sideeffect"].annotation
         basic_compare_model_fields(
-            fused_model.model_fields["sideeffect"].annotation.model_fields,
+            raw_model.model_fields["sideeffect"].annotation.model_fields,
             expected_sideeffect_fields,
         )
 
     if expected_passthrough_fields:
-        assert "passthrough" in fused_model.model_fields.keys()
-        assert fused_model.model_fields["passthrough"].annotation
+        assert "passthrough" in raw_model.model_fields.keys()
+        assert raw_model.model_fields["passthrough"].annotation
         basic_compare_model_fields(
-            fused_model.model_fields["passthrough"].annotation.model_fields,
+            raw_model.model_fields["passthrough"].annotation.model_fields,
             expected_passthrough_fields,
         )
 

mountaineer/__tests__/actions/test_passthrough_dec.py

@@ -100,11 +100,9 @@ async def call_passthrough_async(
 
     # The response payload should be the same both both sync and async endpoints
     expected_response = {
-        "TestController": {
-            "passthrough": ExamplePassthroughModel(
-                status="success",
-            )
-        }
+        "passthrough": ExamplePassthroughModel(
+            status="success",
+        )
     }
 
     assert return_value_sync == expected_response

mountaineer/__tests__/actions/test_sideeffect_dec.py

@@ -101,13 +101,11 @@ async def mock_get_render_parameters(*args, **kwargs):
 
         # The response payload should be the same both both sync and async endpoints
         expected_response = {
-            controller.__class__.__name__: {
-                "sideeffect": ExampleRenderModel(
-                    value_a="Hello",
-                    value_b="World",
-                ),
-                "passthrough": None,
-            }
+            "sideeffect": ExampleRenderModel(
+                value_a="Hello",
+                value_b="World",
+            ),
+            "passthrough": None,
         }
 
         assert return_value_sync == expected_response
@@ -279,10 +277,8 @@ def call_sideeffect(self, payload: dict) -> None:
     elapsed = (monotonic_ns() - start) / 1e9
     assert response.status_code == 200
     assert response.json() == {
-        "ExampleController": {
-            "sideeffect": {
-                "value_a": "Hello 1229",
-            }
+        "sideeffect": {
+            "value_a": "Hello 1229",
         }
     }
 

mountaineer/__tests__/test_app.py

@@ -1,14 +1,16 @@
 from contextlib import asynccontextmanager
+from inspect import Parameter, signature
 from pathlib import Path
 from unittest.mock import patch
 
 import pytest
-from fastapi import FastAPI
+from fastapi import APIRouter, FastAPI
+from fastapi.routing import APIRoute
 from fastapi.testclient import TestClient
 from pydantic import BaseModel
 
 from mountaineer.actions import passthrough
-from mountaineer.app import AppController
+from mountaineer.app import AppController, ControllerDefinition
 from mountaineer.client_builder.openapi import OpenAPIDefinition
 from mountaineer.config import ConfigBase
 from mountaineer.controller import ControllerBase
@@ -93,7 +95,6 @@ def test_exception_action(self) -> None:
         "TestExceptionActionResponse",
         "ExampleException",
         "ExampleSubModel",
-        "TestExceptionActionResponseRaw",
     }
 
 
@@ -155,7 +156,6 @@ def test_exception_action(self) -> None:
 
     assert openapi_definition.components.schemas.keys() == {
         "TestExceptionActionResponse",
-        "TestExceptionActionResponseRaw",
         "mountaineer.__tests__.test_1.ExampleException",
         "mountaineer.__tests__.test_2.ExampleException",
     }
@@ -319,3 +319,123 @@ def render(self) -> None:
 
     with pytest.raises(ValueError, match="already registered"):
         app.register(make_controller("/example2")())
+
+
+class TargetController(ControllerBase):
+    url = "/target"
+
+    async def render(self) -> None:
+        pass
+
+
+class ReferenceController(ControllerBase):
+    url = "/reference"
+
+    async def render(self) -> None:
+        pass
+
+
+def test_merge_render_signatures():
+    def target_fn(a: int, b: int):
+        pass
+
+    # Partial overlap with (a) and inclusion of a new variable
+    def reference_fn(a: int, c: int):
+        pass
+
+    app = AppController(view_root=Path(""))
+
+    target_definition = ControllerDefinition(
+        controller=TargetController(),
+        router=APIRouter(),
+        view_route=target_fn,
+        url_prefix="/target_prefix",
+        render_router=APIRouter(),
+    )
+    reference_definition = ControllerDefinition(
+        controller=ReferenceController(),
+        router=APIRouter(),
+        view_route=reference_fn,
+        url_prefix="/reference_prefix",
+        render_router=APIRouter(),
+    )
+
+    initial_routes = [
+        route.path for route in app.app.routes if isinstance(route, APIRoute)
+    ]
+    assert initial_routes == []
+
+    app.merge_render_signatures(
+        target_definition, reference_controller=reference_definition
+    )
+
+    assert list(signature(target_definition.view_route).parameters.values()) == [
+        Parameter("a", Parameter.POSITIONAL_OR_KEYWORD, annotation=int),
+        Parameter("b", Parameter.POSITIONAL_OR_KEYWORD, annotation=int),
+        # Items only in the reference function should be included as kwargs
+        Parameter("c", Parameter.KEYWORD_ONLY, annotation=int, default=Parameter.empty),
+    ]
+
+    # After the merging the signature should be updated, and the app controller should
+    # have a new endpoint (since the merging must re-mount)
+    final_routes = [
+        route.path for route in app.app.routes if isinstance(route, APIRoute)
+    ]
+    assert final_routes == ["/target"]
+
+
+def test_merge_render_signatures_conflicting_types():
+    """
+    If the two functions share a parameter, it must be typehinted with the
+    same type in both functions.
+
+    """
+
+    def target_fn(a: int, b: int):
+        pass
+
+    # Partial overlap with (a) and inclusion of a new variable
+    def reference_fn(a: str, c: int):
+        pass
+
+    app = AppController(view_root=Path(""))
+
+    target_definition = ControllerDefinition(
+        controller=TargetController(),
+        router=APIRouter(),
+        view_route=target_fn,
+        url_prefix="/target_prefix",
+        render_router=APIRouter(),
+    )
+    reference_definition = ControllerDefinition(
+        controller=ReferenceController(),
+        router=APIRouter(),
+        view_route=reference_fn,
+        url_prefix="/reference_prefix",
+        render_router=APIRouter(),
+    )
+
+    with pytest.raises(TypeError, match="Conflicting types"):
+        app.merge_render_signatures(
+            target_definition, reference_controller=reference_definition
+        )
+
+
+def test_get_value_mask_for_signature():
+    def target_fn(a: int, b: str):
+        pass
+
+    values = {
+        "a": 1,
+        "b": "test",
+        "c": "other",
+    }
+
+    app = AppController(view_root=Path(""))
+    assert app.get_value_mask_for_signature(
+        signature(target_fn),
+        values,
+    ) == {
+        "a": 1,
+        "b": "test",
+    }

mountaineer/actions/fields.py

@@ -233,24 +233,14 @@ def fuse_metadata_to_response_typehint(
         )
 
     model: Type[BaseModel] = create_model(
-        base_response_name + "Raw",
-        **base_response_params,  # type: ignore
-    )
-
-    # Each action also includes the controller type in the response
-    # signature so the frontend can differentiate between different controllers
-    wrapper_model: Type[BaseModel] = create_model(
         base_response_name,
-        **{controller.__class__.__name__: (model, FieldInfo())},  # type: ignore
+        **base_response_params,  # type: ignore
     )
 
-    return wrapper_model
+    return model
 
 
-def format_final_action_response(
-    controller: "ControllerBase",
-    dict_payload: dict[str, Any],
-):
+def format_final_action_response(dict_payload: dict[str, Any]):
     """
     Wrapper to allow actions to respond with an explicit JSONResponse, or a dictionary. This lets
     both sideeffects and passthrough payloads to inject header metadata that otherwise can't be captured
@@ -269,19 +259,15 @@ def format_final_action_response(
     if len(responses) > 1:
         raise ValueError(f"Multiple conflicting responses returned: {responses}")
 
-    # The final transformation should include our controller name
-    # This signals to the frontend which state subset we want to update
-    action_root = controller.__class__.__name__
-
     if len(responses) == 0:
-        return {action_root: dict_payload}
+        return dict_payload
 
     response_key, response = responses[0]
     dict_payload[response_key] = json_loads(response.body)
 
     # Now inject the newly formatted response into the response object
     return JSONResponse(
-        content={action_root: dict_payload},
+        content=dict_payload,
         status_code=response.status_code,
         headers={
             key: value

mountaineer/actions/passthrough_dec.py

@@ -157,7 +157,7 @@ async def inner(self: "ControllerBase", *func_args, **func_kwargs):
                 if isasyncgen(response):
                     return wrap_passthrough_generator(response)
 
-                return format_final_action_response(self, dict(passthrough=response))
+                return format_final_action_response(dict(passthrough=response))
 
             metadata = init_function_metadata(inner, FunctionActionType.PASSTHROUGH)
             metadata.passthrough_model = passthrough_model

mountaineer/actions/sideeffect_dec.py

@@ -171,7 +171,6 @@ async def inner(self: "ControllerBase", *func_args, **func_kwargs):
                         server_data = await server_data
 
                     return format_final_action_response(
-                        self,
                         dict(
                             sideeffect=server_data,
                             passthrough=passthrough_values,