Added test for nested controller and module

eadwinCode · eadwinCode · commit 4d97c68c20f1 · 2024-06-27T10:49:19.000+01:00
diff --git a/ellar/common/models/controller.py b/ellar/common/models/controller.py
@@ -29,7 +29,9 @@ def full_view_name(cls, name: str) -> str:
         return f"{cls.controller_class_name()}/{name}"
 
     def add_router(
-        cls, router: t.Union["ControllerBase", "ModuleRouter"], prefix: str
+        cls,
+        router: t.Union["ControllerBase", "ModuleRouter"],
+        prefix: t.Optional[str] = None,
     ) -> None:
         reflect.define_metadata(
             NESTED_ROUTERS_KEY, [NestedRouterInfo(prefix=prefix, router=router)], cls
diff --git a/ellar/common/operations/router.py b/ellar/common/operations/router.py
@@ -50,7 +50,9 @@ def control_type(self) -> t.Type[t.Any]:
         return self._control_type
 
     def add_router(
-        self, router: t.Union["ModuleRouter", "ControllerBase"], prefix: t.Optional[str]
+        self,
+        router: t.Union["ModuleRouter", "ControllerBase"],
+        prefix: t.Optional[str] = None,
     ) -> None:
         reflect.define_metadata(
             NESTED_ROUTERS_KEY,
diff --git a/ellar/core/router_builders/controller.py b/ellar/core/router_builders/controller.py
@@ -56,7 +56,6 @@ def process_controller_routes(controller: t.Type[ControllerBase]) -> t.List[Base
                 controller,
             )
             res.append(operation)
-    reflect.define_metadata(CONTROLLER_METADATA.PROCESSED, True, controller)
     return res
 
 
@@ -101,6 +100,7 @@ def build(
             control_type=controller_type,
             **kwargs,
         )
+        reflect.define_metadata(CONTROLLER_METADATA.PROCESSED, True, controller_type)
         return router
 
     @classmethod
diff --git a/ellar/core/router_builders/module_router.py b/ellar/core/router_builders/module_router.py
@@ -2,7 +2,7 @@
 
 from ellar.common import ModuleRouter
 from ellar.common.compatible import AttributeDict
-from ellar.common.constants import CONTROLLER_OPERATION_HANDLER_KEY
+from ellar.common.constants import CONTROLLER_METADATA, CONTROLLER_OPERATION_HANDLER_KEY
 from ellar.common.shortcuts import normalize_path
 from ellar.core.routing import EllarMount
 from ellar.reflect import reflect
@@ -21,15 +21,17 @@ def build(
         base_route_type: t.Type[t.Union[EllarMount, T_]] = EllarMount,
         **kwargs: t.Any,
     ) -> t.Union[T_, EllarMount]:
-        if controller_type.get_pre_build_routes():
+        if reflect.get_metadata(
+            CONTROLLER_METADATA.PROCESSED, controller_type.control_type
+        ):
+            routes = reflect.get_metadata(
+                CONTROLLER_OPERATION_HANDLER_KEY, controller_type.control_type
+            )
+        else:
             routes = build_route_parameters(
                 controller_type.get_pre_build_routes(), controller_type.control_type
             )
             routes.extend(process_nested_routes(controller_type.control_type))
-        else:
-            routes = reflect.get_metadata(
-                CONTROLLER_OPERATION_HANDLER_KEY, controller_type.control_type
-            )
 
         init_kwargs = AttributeDict(controller_type.get_mount_init())
 
@@ -38,7 +40,12 @@ def build(
             init_kwargs.path = normalize_path(f"{prefix}/{init_kwargs.path}")
 
         controller_type.clear_pre_build_routes()
-        return base_route_type(**init_kwargs, routes=routes)  # type:ignore[call-arg]
+        router = base_route_type(**init_kwargs, routes=routes)  # type:ignore[call-arg]
+
+        reflect.define_metadata(
+            CONTROLLER_METADATA.PROCESSED, True, controller_type.control_type
+        )
+        return router
 
     @classmethod
     def check_type(cls, controller_type: t.Union[t.Type, t.Any]) -> None:
diff --git a/ellar/core/router_builders/utils.py b/ellar/core/router_builders/utils.py
@@ -10,6 +10,7 @@
     OPERATION_ENDPOINT_KEY,
     ROUTE_OPERATION_PARAMETERS,
 )
+from ellar.common.exceptions import ImproperConfiguration
 from ellar.common.logging import logger
 from ellar.common.models.controller import NestedRouterInfo
 from ellar.common.operations import RouteParameters, WsRouteParameters
@@ -26,6 +27,8 @@
 
 T_ = t.TypeVar("T_")
 
+_stack_cycle: t.Tuple = ()
+
 
 def get_route_functions(
     klass: t.Type, key: str = OPERATION_ENDPOINT_KEY
@@ -36,11 +39,20 @@ def get_route_functions(
 
 
 def process_nested_routes(controller: t.Type[t.Any]) -> t.List[BaseRoute]:
+    global _stack_cycle
+
     res: t.List[BaseRoute] = []
 
     if reflect.get_metadata(CONTROLLER_METADATA.PROCESSED, controller):
         return []
 
+    if controller in _stack_cycle:
+        raise ImproperConfiguration(
+            "Circular Nested router detected: %s -> %s"
+            % (" -> ".join(map(str, _stack_cycle)), controller)
+        )
+
+    _stack_cycle += (controller,)
     nested_routers: t.List[NestedRouterInfo] = (
         reflect.get_metadata(NESTED_ROUTERS_KEY, controller) or []
     )
@@ -59,6 +71,8 @@ def process_nested_routes(controller: t.Type[t.Any]) -> t.List[BaseRoute]:
         )
 
         res.append(operation)
+
+    _stack_cycle = tuple(_stack_cycle[:-1])
     return res
 
 
diff --git a/tests/test_controller/test_nested_controllers.py b/tests/test_controller/test_nested_controllers.py
@@ -0,0 +1,53 @@
+import pytest
+from ellar.common import Controller, get
+from ellar.common.exceptions import ImproperConfiguration
+from ellar.testing import Test
+
+
+@Controller
+class Cat1Controller:
+    @get("/create")
+    async def create_cat(self):
+        return {"message": f"created from {self.__class__.__name__}"}
+
+
+@Controller
+class Cat2Controller:
+    @get("/create")
+    async def create_cat(self):
+        return {"message": f"created from {self.__class__.__name__}"}
+
+
+@Controller
+class Cat3Controller:
+    @get("/create")
+    async def create_cat(self):
+        return {"message": f"created from {self.__class__.__name__}"}
+
+
+def test_can_reach_controllers():
+    Cat2Controller.add_router(Cat1Controller)
+    Cat3Controller.add_router(Cat2Controller)
+
+    tm = Test.create_test_module(controllers=[Cat3Controller])
+
+    client = tm.get_test_client()
+    res = client.get("cat3/cat2/create")
+    assert res.status_code == 200
+    assert res.json() == {"message": "created from Cat2Controller"}
+
+    res = client.get("cat3/cat2/cat1/create")
+    assert res.status_code == 200
+    assert res.json() == {"message": "created from Cat1Controller"}
+
+    res = client.get("cat3/create")
+    assert res.status_code == 200
+    assert res.json() == {"message": "created from Cat3Controller"}
+
+
+def test_circular_exception_works():
+    Cat2Controller.add_router(Cat1Controller)
+    Cat1Controller.add_router(Cat2Controller)
+
+    with pytest.raises(ImproperConfiguration, match="Circular Nested router"):
+        Test.create_test_module(controllers=[Cat2Controller]).create_application()
diff --git a/tests/test_openapi/test_nested_mount.py b/tests/test_openapi/test_nested_mount.py
@@ -0,0 +1,100 @@
+from ellar.common import Controller, get, serialize_object
+from ellar.openapi import OpenAPIDocumentBuilder
+from ellar.testing import Test
+
+
+@Controller
+class Cat1Controller:
+    @get("/create")
+    async def create_cat(self):
+        return {"message": "created"}
+
+
+@Controller
+class Cat2Controller:
+    @get("/create")
+    async def create_cat(self):
+        return {"message": "created"}
+
+
+Cat2Controller.add_router(Cat1Controller)
+
+
+tm = Test.create_test_module(controllers=[Cat2Controller])
+
+
+def test_nested_route_openapi_schema():
+    app = tm.create_application()
+    document = serialize_object(OpenAPIDocumentBuilder().build_document(app))
+    assert document == NESTED_SCHEMA
+
+
+NESTED_SCHEMA = {
+    "openapi": "3.1.0",
+    "info": {"title": "Ellar API Docs", "version": "1.0.0"},
+    "paths": {
+        "/cat2/cat1/create": {
+            "get": {
+                "tags": ["cat1"],
+                "operationId": "create_cat_create_get__cat1",
+                "responses": {
+                    "200": {
+                        "description": "Successful Response",
+                        "content": {
+                            "application/json": {
+                                "schema": {"type": "object", "title": "Response Model"}
+                            }
+                        },
+                    }
+                },
+            }
+        },
+        "/cat2/create": {
+            "get": {
+                "tags": ["cat2"],
+                "operationId": "create_cat_create_get__cat2",
+                "responses": {
+                    "200": {
+                        "description": "Successful Response",
+                        "content": {
+                            "application/json": {
+                                "schema": {"type": "object", "title": "Response Model"}
+                            }
+                        },
+                    }
+                },
+            }
+        },
+    },
+    "components": {
+        "schemas": {
+            "HTTPValidationError": {
+                "properties": {
+                    "detail": {
+                        "items": {"$ref": "#/components/schemas/ValidationError"},
+                        "type": "array",
+                        "title": "Details",
+                    }
+                },
+                "type": "object",
+                "required": ["detail"],
+                "title": "HTTPValidationError",
+            },
+            "ValidationError": {
+                "properties": {
+                    "loc": {
+                        "items": {"type": "string"},
+                        "type": "array",
+                        "title": "Location",
+                    },
+                    "msg": {"type": "string", "title": "Message"},
+                    "type": {"type": "string", "title": "Error Type"},
+                },
+                "type": "object",
+                "required": ["loc", "msg", "type"],
+                "title": "ValidationError",
+            },
+        }
+    },
+    "tags": [{"name": "cat1"}, {"name": "cat2"}],
+}
diff --git a/tests/test_routing/test_nested_router.py b/tests/test_routing/test_nested_router.py
@@ -0,0 +1,55 @@
+import pytest
+from ellar.common import ModuleRouter
+from ellar.common.exceptions import ImproperConfiguration
+from ellar.testing import Test
+
+router1 = ModuleRouter("/cat1")
+
+
+@router1.get("/create")
+async def create_cat():
+    return {"message": "created from ModuleRouter1"}
+
+
+router2 = ModuleRouter("/cat2")
+
+
+@router2.get("/create")
+async def create_cat_2():
+    return {"message": "created from ModuleRouter2"}
+
+
+router3 = ModuleRouter("/cat3")
+
+
+@router3.get("/create")
+async def create_cat_3():
+    return {"message": "created from ModuleRouter3"}
+
+
+def test_can_reach_routers():
+    router2.add_router(router1)
+    router3.add_router(router2)
+
+    tm = Test.create_test_module(routers=[router3])
+
+    client = tm.get_test_client()
+    res = client.get("cat3/cat2/create")
+    assert res.status_code == 200
+    assert res.json() == {"message": "created from ModuleRouter2"}
+
+    res = client.get("cat3/cat2/cat1/create")
+    assert res.status_code == 200
+    assert res.json() == {"message": "created from ModuleRouter1"}
+
+    res = client.get("cat3/create")
+    assert res.status_code == 200
+    assert res.json() == {"message": "created from ModuleRouter3"}
+
+
+def test_circular_exception_works_router_reference():
+    router2.add_router(router1)
+    router1.add_router(router2)
+
+    with pytest.raises(ImproperConfiguration, match="Circular Nested router"):
+        Test.create_test_module(routers=[router2]).create_application()

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,6 @@ def process_controller_routes(controller: t.Type[ControllerBase]) -> t.List[Base`
`56`	`56`	`controller,`
`57`	`57`	`)`
`58`	`58`	`res.append(operation)`
`59`		`- reflect.define_metadata(CONTROLLER_METADATA.PROCESSED, True, controller)`
`60`	`59`	`return res`
`61`	`60`
`62`	`61`
`@@ -101,6 +100,7 @@ def build(`
`101`	`100`	`control_type=controller_type,`
`102`	`101`	`**kwargs,`
`103`	`102`	`)`
	`103`	`+ reflect.define_metadata(CONTROLLER_METADATA.PROCESSED, True, controller_type)`
`104`	`104`	`return router`
`105`	`105`
`106`	`106`	`@classmethod`