Fix key-loop in Tree.add to reuse existing children for literal segments after a matcher boundary
The key-loop unconditionally called self.parse() and overwrote result.children[key] for literal segments after the walk-down loop broke at a parameter. This destroyed pre-existing subtrees (with their method children and handlers) when a second route (e.g. POST) shared the same literal sub-segments after a parameter. Added a children.get(key) reuse check before parse(), mirroring the walk-down loop's child-reuse logic but extended past the boundary where parameters live in path_matchers, not children.
This commit is contained in:
@@ -97,6 +97,11 @@ class Tree:
|
||||
result = existing
|
||||
key = next(it, None)
|
||||
continue
|
||||
child = result.children.get(key)
|
||||
if child is not None:
|
||||
result = child
|
||||
key = next(it, None)
|
||||
continue
|
||||
new_node = self.parse(key, result)
|
||||
if isinstance(new_node, Node):
|
||||
result.children[key] = new_node
|
||||
|
||||
@@ -216,3 +216,37 @@ class AsgiTest(unittest.TestCase):
|
||||
r = await client.get("/restaurants/42/unknown")
|
||||
self.assertEqual(404, r.status_code)
|
||||
|
||||
@async_test
|
||||
async def test_nested_param_routes_multiple_methods(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_get(ctx: HttpContext, id: str) -> None:
|
||||
await ctx.send_str(200, f"menu_get:{id}")
|
||||
|
||||
@app.POST('/restaurants/${id}/menu')
|
||||
async def menu_post(ctx: HttpContext, id: str) -> None:
|
||||
await ctx.send_str(200, f"menu_post:{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_get:42", r.text)
|
||||
|
||||
r = await client.post("/restaurants/42/menu")
|
||||
self.assertEqual(200, r.status_code)
|
||||
self.assertEqual("menu_post:42", r.text)
|
||||
|
||||
r = await client.put("/restaurants/42/menu")
|
||||
self.assertEqual(404, r.status_code)
|
||||
|
||||
|
||||
@@ -156,3 +156,33 @@ class TreeTest(unittest.TestCase):
|
||||
with self.assertRaises(ValueError):
|
||||
tree.add((p for p in ('a', '${id:int}', 'x')), HttpMethod.GET, self.handlers[1])
|
||||
|
||||
def test_nested_routes_different_methods_share_subtree(self):
|
||||
tree = Tree()
|
||||
tree.add((p for p in ('restaurants', '${id}', 'menu')), HttpMethod.GET, self.handlers[0])
|
||||
tree.add((p for p in ('restaurants', '${id}', 'menu')), HttpMethod.POST, self.handlers[1])
|
||||
h0 = Maybe.of_nullable(tree.get_handler('/restaurants/42/menu', 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_nested_routes_different_methods_reverse_order(self):
|
||||
tree = Tree()
|
||||
tree.add((p for p in ('restaurants', '${id}', 'menu')), HttpMethod.POST, self.handlers[1])
|
||||
tree.add((p for p in ('restaurants', '${id}', 'menu')), HttpMethod.GET, self.handlers[0])
|
||||
h0 = Maybe.of_nullable(tree.get_handler('/restaurants/42/menu', 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_nested_routes_combined_detail_and_menu_methods(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])
|
||||
tree.add((p for p in ('restaurants', '${id}', 'menu')), HttpMethod.POST, self.handlers[2])
|
||||
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()
|
||||
h2 = 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)
|
||||
self.assertIs(self.handlers[2], h2)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user