Allow nested routes sharing the same path parameter matcher
CI / Build Pip package (push) Successful in 2m32s
CI / Build Pip package (push) Successful in 2m32s
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
|
||||
key = leaf
|
||||
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)
|
||||
if isinstance(new_node, Node):
|
||||
result.children[key] = new_node
|
||||
@@ -109,6 +114,44 @@ class Tree:
|
||||
def _supports_method(node: Node | PathMatcher, method: HttpMethod) -> bool:
|
||||
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:
|
||||
new_is_generic = method is None
|
||||
for existing in node.path_matchers:
|
||||
|
||||
@@ -190,3 +190,29 @@ class AsgiTest(unittest.TestCase):
|
||||
'employee_id': 101325
|
||||
}, 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.add((p for p in ('foo', '*')), None, self.handlers[0])
|
||||
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):
|
||||
tree = Tree()
|
||||
@@ -101,3 +108,51 @@ class TreeTest(unittest.TestCase):
|
||||
self.assertIs(self.handlers[0], put_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