Merge pull request #369 from kaschnit/allow-router-path-params

Allow path parameters to be specified at router level

ninja/main.py

@@ -324,8 +324,7 @@ def _get_urls(self) -> List[Union[URLResolver, URLPattern]]:
         result = get_openapi_urls(self)
 
         for prefix, router in self._routers:
-            for path in router.urls_paths(prefix):
-                result.append(path)
+            result.extend(router.urls_paths(prefix))
 
         result.append(get_root_url(self))
         return result

ninja/router.py

@@ -16,7 +16,7 @@
 from ninja.errors import ConfigError
 from ninja.operation import PathView
 from ninja.types import TCallable
-from ninja.utils import normalize_path
+from ninja.utils import normalize_path, replace_path_param_notation
 
 if TYPE_CHECKING:
     from ninja import NinjaAPI  # pragma: no cover
@@ -316,8 +316,9 @@ def set_api_instance(
             router.set_api_instance(api, self)
 
     def urls_paths(self, prefix: str) -> Iterator[URLPattern]:
+        prefix = replace_path_param_notation(prefix)
         for path, path_view in self.path_operations.items():
-            path = path.replace("{", "<").replace("}", ">")
+            path = replace_path_param_notation(path)
             route = "/".join([i for i in (prefix, path) if i])
             # to skip lot of checks we simply treat double slash as a mistake:
             route = normalize_path(route)

ninja/utils.py

@@ -8,6 +8,10 @@
 __all__ = ["check_csrf", "is_debug_server", "normalize_path"]
 
 
+def replace_path_param_notation(path: str) -> str:
+    return path.replace("{", "<").replace("}", ">")
+
+
 def normalize_path(path: str) -> str:
     while "//" in path:
         path = path.replace("//", "/")

tests/test_router_path_params.py

@@ -0,0 +1,83 @@
+import pytest
+
+from ninja import NinjaAPI, Path, Router
+from ninja.testing import TestClient
+
+api = NinjaAPI()
+router_with_path_type = Router()
+router_without_path_type = Router()
+router_with_multiple = Router()
+
+
+@router_with_path_type.get("/metadata")
+def get_item_metadata(request, item_id: int = Path(None)) -> int:
+    return item_id
+
+
+@router_without_path_type.get("/")
+def get_item_metadata_2(request, item_id: str = Path(None)) -> str:
+    return item_id
+
+
+@router_without_path_type.get("/metadata")
+def get_item_metadata_3(request, item_id: str = Path(None)) -> str:
+    return item_id
+
+
+@router_without_path_type.get("/")
+def get_item_metadata_4(request, item_id: str = Path(None)) -> str:
+    return item_id
+
+
+@router_with_multiple.get("/metadata/{kind}")
+def get_item_metadata_5(
+    request, item_id: int = Path(None), name: str = Path(None), kind: str = Path(None)
+) -> str:
+    return f"{item_id} {name} {kind}"
+
+
+api.add_router("/with_type/{int:item_id}", router_with_path_type)
+api.add_router("/without_type/{item_id}", router_without_path_type)
+api.add_router("/with_multiple/{int:item_id}/name/{name}", router_with_multiple)
+
+client = TestClient(api)
+
+
+@pytest.mark.parametrize(
+    "path,expected_status,expected_response",
+    [
+        ("/with_type/1/metadata", 200, 1),
+        ("/without_type/1/metadata", 200, "1"),
+        ("/without_type/abc/metadata", 200, "abc"),
+        ("/with_multiple/99/name/foo/metadata/timestamp", 200, "99 foo timestamp"),
+    ],
+)
+def test_router_with_path_params(path, expected_status, expected_response):
+    response = client.get(path)
+    assert response.status_code == expected_status
+    assert response.json() == expected_response
+
+
+@pytest.mark.parametrize(
+    "path,expected_exception,expect_exception_contains",
+    [
+        ("/with_type/abc/metadata", Exception, "Cannot resolve"),
+        ("/with_type//metadata", Exception, "Cannot resolve"),
+        ("/with_type/null/metadata", Exception, "Cannot resolve"),
+        ("/with_type", Exception, "Cannot resolve"),
+        ("/with_type/", Exception, "Cannot resolve"),
+        ("/with_type//", Exception, "Cannot resolve"),
+        ("/with_type/null", Exception, "Cannot resolve"),
+        ("/with_type/null/", Exception, "Cannot resolve"),
+        ("/without_type", Exception, "Cannot resolve"),
+        ("/without_type/", Exception, "Cannot resolve"),
+        ("/without_type//", Exception, "Cannot resolve"),
+        ("/with_multiple/abc/name/foo/metadata/timestamp", Exception, "Cannot resolve"),
+        ("/with_multiple/99", Exception, "Cannot resolve"),
+    ],
+)
+def test_router_with_path_params_nomatch(
+    path, expected_exception, expect_exception_contains
+):
+    with pytest.raises(expected_exception, match=expect_exception_contains):
+        client.get(path)

tests/test_utils.py

@@ -0,0 +1,18 @@
+import pytest
+
+from ninja.utils import replace_path_param_notation
+
+
+@pytest.mark.parametrize(
+    "input,expected_output",
+    [
+        ("abc/{def}", "abc/<def>"),
+        ("abc/<def>", "abc/<def>"),
+        ("abc", "abc"),
+        ("<abc>", "<abc>"),
+        ("{abc}", "<abc>"),
+        ("{abc}/{def}", "<abc>/<def>"),
+    ],
+)
+def test_replace_path_param_notation(input, expected_output):
+    assert replace_path_param_notation(input) == expected_output