Repository: vitalik/django-ninja
Pull request #188 (2021-07-30 17:42:16)

Fix add_router() to router already attached to API
After a router is attached to an api, attaching further routers to the attached router fails.


Patch:
diff --git a/ninja/main.py b/ninja/main.py
index 94eea9e..e4fbf7d 100644
--- a/ninja/main.py
+++ b/ninja/main.py
@@ -24,6 +24,7 @@ from ninja.parser import Parser
 from ninja.renderers import BaseRenderer, JSONRenderer
 from ninja.router import Router
 from ninja.types import TCallable
+from ninja.utils import normalize_path
 
 if TYPE_CHECKING:
     from .operation import Operation  # pragma: no cover
@@ -291,11 +292,20 @@ class NinjaAPI:
         *,
         auth: Any = NOT_SET,
         tags: Optional[List[str]] = None,
+        parent_router: Router = None,
     ) -> None:
         if auth != NOT_SET:
             router.auth = auth
         if tags is not None:
             router.tags = tags
+
+        if parent_router:
+            parent_prefix = next(
+                (path for path, r in self._routers if r is parent_router), None
+            )
+            assert parent_prefix is not None
+            prefix = normalize_path("/".join((parent_prefix, prefix))).lstrip("/")
+
         self._routers.extend(router.build_routers(prefix))
         router.set_api_instance(self)
 
diff --git a/ninja/router.py b/ninja/router.py
index 803d02b..0d37cba 100644
--- a/ninja/router.py
+++ b/ninja/router.py
@@ -333,13 +333,21 @@ class Router:
         auth: Any = NOT_SET,
         tags: Optional[List[str]] = None,
     ) -> None:
-        if auth != NOT_SET:
-            router.auth = auth
-        if tags is not None:
-            router.tags = tags
-        self._routers.append((prefix, router))
+        if self.api:
+            # we are already attached to an api
+            self.api.add_router(
+                prefix=prefix, router=router, auth=auth, tags=tags, parent_router=self
+            )
+        else:
+            # we are not attached to an api
+            if auth != NOT_SET:
+                router.auth = auth
+            if tags is not None:
+                router.tags = tags
+            self._routers.append((prefix, router))
 
     def build_routers(self, prefix: str) -> List[Tuple[str, "Router"]]:
+        assert self.api is None
         internal_routes = []
         for inter_prefix, inter_router in self._routers:
             _route = normalize_path("/".join((prefix, inter_prefix))).lstrip("/")


Test patch:
diff --git a/tests/test_inheritance_routers.py b/tests/test_inheritance_routers.py
index bba30d6..98edf9c 100644
--- a/tests/test_inheritance_routers.py
+++ b/tests/test_inheritance_routers.py
@@ -6,6 +6,7 @@ api = NinjaAPI()
 
 
 @api.get("/endpoint")
+# view->api
 def global_op(request):
     return "global"
 
@@ -13,15 +14,17 @@ def global_op(request):
 first_router = Router()
 
 
-@first_router.get("/endpoint")
+@first_router.get("/endpoint_1")
+# view->router, router->api
 def router_op1(request):
-    return "first"
+    return "first 1"
 
 
 second_router_one = Router()
 
 
 @second_router_one.get("endpoint_1")
+# view->router2, router2->router1, router1->api
 def router_op2(request):
     return "second 1"
 
@@ -30,6 +33,7 @@ second_router_two = Router()
 
 
 @second_router_two.get("endpoint_2")
+# view->router2, router2->router1, router1->api
 def router_op3(request):
     return "second 2"
 
@@ -39,6 +43,30 @@ first_router.add_router("/second", second_router_two, tags=["two"])
 api.add_router("/first", first_router, tags=["global"])
 
 
+@first_router.get("endpoint_2")
+# router->api, view->router
+def router_op1(request):
+    return "first 2"
+
+
+@second_router_one.get("endpoint_3")
+# router2->router1, router1->api, view->router2
+def router_op3(request, path_param: int = None):
+    return "second 3" if path_param is None else f"second 3: {path_param}"
+
+
+second_router_three = Router()
+
+
+@second_router_three.get("endpoint_4")
+# router1->api, view->router2, router2->router1
+def router_op3(request, path_param: int = None):
+    return "second 4" if path_param is None else f"second 4: {path_param}"
+
+
+first_router.add_router("/second", second_router_three, tags=["three"])
+
+
 client = TestClient(api)
 
 
@@ -46,9 +74,12 @@ client = TestClient(api)
     "path,expected_status,expected_response",
     [
         ("/endpoint", 200, "global"),
-        ("/first/endpoint", 200, "first"),
+        ("/first/endpoint_1", 200, "first 1"),
+        ("/first/endpoint_2", 200, "first 2"),
         ("/first/second/endpoint_1", 200, "second 1"),
         ("/first/second/endpoint_2", 200, "second 2"),
+        ("/first/second/endpoint_3", 200, "second 3"),
+        ("/first/second/endpoint_4", 200, "second 4"),
     ],
 )
 def test_inheritance_responses(path, expected_status, expected_response):
@@ -60,7 +91,7 @@ def test_inheritance_responses(path, expected_status, expected_response):
 def test_tags():
     schema = api.get_openapi_schema()
     # print(schema)
-    glob = schema["paths"]["/api/first/endpoint"]["get"]
+    glob = schema["paths"]["/api/first/endpoint_1"]["get"]
     assert glob["tags"] == ["global"]
 
     e1 = schema["paths"]["/api/first/second/endpoint_1"]["get"]