Allow nested routes sharing the same path parameter matcher
When two routes share the same parameter at the same node position
(e.g. GET /restaurants/${id} and GET /restaurants/${id}/menu),
reuse the existing equivalent matcher instead of raising a conflict.
Non-equivalent matchers (different names, kinds, or glob patterns)
at the same node for the same method still raise ValueError.
This commit is contained in:
@@ -92,6 +92,11 @@ class Tree:
|
|||||||
result = child
|
result = child
|
||||||
key = leaf
|
key = leaf
|
||||||
while key is not None:
|
while key is not None:
|
||||||
|
existing = self._find_equivalent_matcher(result, key)
|
||||||
|
if existing is not None:
|
||||||
|
result = existing
|
||||||
|
key = next(it, None)
|
||||||
|
continue
|
||||||
new_node = self.parse(key, result)
|
new_node = self.parse(key, result)
|
||||||
if isinstance(new_node, Node):
|
if isinstance(new_node, Node):
|
||||||
result.children[key] = new_node
|
result.children[key] = new_node
|
||||||
@@ -109,6 +114,44 @@ class Tree:
|
|||||||
def _supports_method(node: Node | PathMatcher, method: HttpMethod) -> bool:
|
def _supports_method(node: Node | PathMatcher, method: HttpMethod) -> bool:
|
||||||
return None in node.supported_methods or method in node.supported_methods
|
return None in node.supported_methods or method in node.supported_methods
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _matcher_identity(leaf: str) -> Optional[Tuple[str, str]]:
|
||||||
|
start = index_of_with_escape(leaf, '${', '\\', 0)
|
||||||
|
if start >= 0:
|
||||||
|
start += 2
|
||||||
|
end = leaf.index('}', start + 2)
|
||||||
|
definition = leaf[start:end]
|
||||||
|
try:
|
||||||
|
colon = definition.index(':')
|
||||||
|
except ValueError:
|
||||||
|
colon = None
|
||||||
|
if colon is None:
|
||||||
|
name = definition
|
||||||
|
kind = 'str'
|
||||||
|
else:
|
||||||
|
name = definition[:colon]
|
||||||
|
kind = definition[colon + 1:]
|
||||||
|
if kind not in ('str', 'int'):
|
||||||
|
raise ValueError(f"Unknown kind: '{kind}'")
|
||||||
|
return (kind, name)
|
||||||
|
if index_of_with_escape(leaf, '*', '\\', 0) >= 0:
|
||||||
|
return ('glob', leaf)
|
||||||
|
return None
|
||||||
|
|
||||||
|
def _find_equivalent_matcher(self, node: Node | PathMatcher, leaf: str) -> Optional[PathMatcher]:
|
||||||
|
identity = self._matcher_identity(leaf)
|
||||||
|
if identity is None:
|
||||||
|
return None
|
||||||
|
kind, key = identity
|
||||||
|
for existing in node.path_matchers:
|
||||||
|
if kind == 'str' and isinstance(existing, StrMatcher) and existing.name == key:
|
||||||
|
return existing
|
||||||
|
if kind == 'int' and isinstance(existing, IntMatcher) and existing.name == key:
|
||||||
|
return existing
|
||||||
|
if kind == 'glob' and isinstance(existing, GlobMatcher) and existing.pattern == key:
|
||||||
|
return existing
|
||||||
|
return None
|
||||||
|
|
||||||
def _check_matcher_conflict(self, node: Node | PathMatcher, method: Optional[HttpMethod]) -> None:
|
def _check_matcher_conflict(self, node: Node | PathMatcher, method: Optional[HttpMethod]) -> None:
|
||||||
new_is_generic = method is None
|
new_is_generic = method is None
|
||||||
for existing in node.path_matchers:
|
for existing in node.path_matchers:
|
||||||
|
|||||||
@@ -190,3 +190,29 @@ class AsgiTest(unittest.TestCase):
|
|||||||
'employee_id': 101325
|
'employee_id': 101325
|
||||||
}, response)
|
}, response)
|
||||||
|
|
||||||
|
@async_test
|
||||||
|
async def test_nested_param_routes(self):
|
||||||
|
app = KayaApp()
|
||||||
|
|
||||||
|
@app.GET('/restaurants/${id}')
|
||||||
|
async def restaurant(ctx: HttpContext, id: str) -> None:
|
||||||
|
await ctx.send_str(200, f"restaurant:{id}")
|
||||||
|
|
||||||
|
@app.GET('/restaurants/${id}/menu')
|
||||||
|
async def menu(ctx: HttpContext, id: str) -> None:
|
||||||
|
await ctx.send_str(200, f"menu:{id}")
|
||||||
|
|
||||||
|
transport = httpx.ASGITransport(app=app)
|
||||||
|
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://127.0.0.1:80") as client:
|
||||||
|
r = await client.get("/restaurants/42")
|
||||||
|
self.assertEqual(200, r.status_code)
|
||||||
|
self.assertEqual("restaurant:42", r.text)
|
||||||
|
|
||||||
|
r = await client.get("/restaurants/42/menu")
|
||||||
|
self.assertEqual(200, r.status_code)
|
||||||
|
self.assertEqual("menu:42", r.text)
|
||||||
|
|
||||||
|
r = await client.get("/restaurants/42/unknown")
|
||||||
|
self.assertEqual(404, r.status_code)
|
||||||
|
|
||||||
|
|||||||
@@ -84,7 +84,14 @@ class TreeTest(unittest.TestCase):
|
|||||||
tree = Tree()
|
tree = Tree()
|
||||||
tree.add((p for p in ('foo', '*')), None, self.handlers[0])
|
tree.add((p for p in ('foo', '*')), None, self.handlers[0])
|
||||||
with self.assertRaises(ValueError):
|
with self.assertRaises(ValueError):
|
||||||
tree.add((p for p in ('foo', '*')), None, self.handlers[1])
|
tree.add((p for p in ('foo', '*.md')), None, self.handlers[1])
|
||||||
|
|
||||||
|
def test_identical_method_agnostic_matchers_reuse(self):
|
||||||
|
tree = Tree()
|
||||||
|
tree.add((p for p in ('foo', '*')), None, self.handlers[0])
|
||||||
|
tree.add((p for p in ('foo', '*')), None, self.handlers[1])
|
||||||
|
handler = Maybe.of_nullable(tree.get_handler('/foo/bar', HttpMethod.GET)).map(lambda it: it[0]).or_none()
|
||||||
|
self.assertIs(self.handlers[0], handler)
|
||||||
|
|
||||||
def test_two_overlapping_method_specific_matchers_raise(self):
|
def test_two_overlapping_method_specific_matchers_raise(self):
|
||||||
tree = Tree()
|
tree = Tree()
|
||||||
@@ -101,3 +108,51 @@ class TreeTest(unittest.TestCase):
|
|||||||
self.assertIs(self.handlers[0], put_handler)
|
self.assertIs(self.handlers[0], put_handler)
|
||||||
self.assertIs(self.handlers[1], get_handler)
|
self.assertIs(self.handlers[1], get_handler)
|
||||||
|
|
||||||
|
def test_nested_routes_with_same_param_allowed(self):
|
||||||
|
tree = Tree()
|
||||||
|
tree.add((p for p in ('restaurants', '${id}')), HttpMethod.GET, self.handlers[0])
|
||||||
|
tree.add((p for p in ('restaurants', '${id}', 'menu')), HttpMethod.GET, self.handlers[1])
|
||||||
|
h0 = Maybe.of_nullable(tree.get_handler('/restaurants/42', HttpMethod.GET)).map(lambda it: it[0]).or_none()
|
||||||
|
h1 = Maybe.of_nullable(tree.get_handler('/restaurants/42/menu', HttpMethod.GET)).map(lambda it: it[0]).or_none()
|
||||||
|
self.assertIs(self.handlers[0], h0)
|
||||||
|
self.assertIs(self.handlers[1], h1)
|
||||||
|
|
||||||
|
def test_nested_routes_same_param_reverse_order(self):
|
||||||
|
tree = Tree()
|
||||||
|
tree.add((p for p in ('restaurants', '${id}', 'menu')), HttpMethod.GET, self.handlers[1])
|
||||||
|
tree.add((p for p in ('restaurants', '${id}')), HttpMethod.GET, self.handlers[0])
|
||||||
|
h0 = Maybe.of_nullable(tree.get_handler('/restaurants/42', HttpMethod.GET)).map(lambda it: it[0]).or_none()
|
||||||
|
h1 = Maybe.of_nullable(tree.get_handler('/restaurants/42/menu', HttpMethod.GET)).map(lambda it: it[0]).or_none()
|
||||||
|
self.assertIs(self.handlers[0], h0)
|
||||||
|
self.assertIs(self.handlers[1], h1)
|
||||||
|
|
||||||
|
def test_nested_routes_with_same_int_param_allowed(self):
|
||||||
|
tree = Tree()
|
||||||
|
tree.add((p for p in ('restaurants', '${id:int}')), HttpMethod.GET, self.handlers[0])
|
||||||
|
tree.add((p for p in ('restaurants', '${id:int}', 'menu')), HttpMethod.GET, self.handlers[1])
|
||||||
|
h0 = Maybe.of_nullable(tree.get_handler('/restaurants/42', HttpMethod.GET)).map(lambda it: it[0]).or_none()
|
||||||
|
h1 = Maybe.of_nullable(tree.get_handler('/restaurants/42/menu', HttpMethod.GET)).map(lambda it: it[0]).or_none()
|
||||||
|
self.assertIs(self.handlers[0], h0)
|
||||||
|
self.assertIs(self.handlers[1], h1)
|
||||||
|
|
||||||
|
def test_nested_routes_method_agnostic_reuses_matcher(self):
|
||||||
|
tree = Tree()
|
||||||
|
tree.add((p for p in ('restaurants', '${id}')), HttpMethod.GET, self.handlers[0])
|
||||||
|
tree.add((p for p in ('restaurants', '${id}', 'menu')), None, self.handlers[1])
|
||||||
|
h0 = Maybe.of_nullable(tree.get_handler('/restaurants/42', HttpMethod.GET)).map(lambda it: it[0]).or_none()
|
||||||
|
h1 = Maybe.of_nullable(tree.get_handler('/restaurants/42/menu', HttpMethod.POST)).map(lambda it: it[0]).or_none()
|
||||||
|
self.assertIs(self.handlers[0], h0)
|
||||||
|
self.assertIs(self.handlers[1], h1)
|
||||||
|
|
||||||
|
def test_different_param_names_still_raise(self):
|
||||||
|
tree = Tree()
|
||||||
|
tree.add((p for p in ('a', '${id}')), HttpMethod.GET, self.handlers[0])
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
tree.add((p for p in ('a', '${name}', 'x')), HttpMethod.GET, self.handlers[1])
|
||||||
|
|
||||||
|
def test_different_param_kinds_still_raise(self):
|
||||||
|
tree = Tree()
|
||||||
|
tree.add((p for p in ('a', '${id}')), HttpMethod.GET, self.handlers[0])
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
tree.add((p for p in ('a', '${id:int}', 'x')), HttpMethod.GET, self.handlers[1])
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user