Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add better exception msg for duplicated param names #1177

Merged
merged 1 commit into from May 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions starlette/routing.py
Expand Up @@ -107,6 +107,7 @@ def compile_path(
"""
path_regex = "^"
path_format = ""
duplicated_params = set()

idx = 0
param_convertors = {}
Expand All @@ -124,10 +125,18 @@ def compile_path(
path_format += path[idx : match.start()]
path_format += "{%s}" % param_name

if param_name in param_convertors:
duplicated_params.add(param_name)

param_convertors[param_name] = convertor

idx = match.end()

if duplicated_params:
names = ", ".join(sorted(duplicated_params))
ending = "s" if len(duplicated_params) > 1 else ""
raise ValueError(f"Duplicated param name{ending} {names} at path {path}")

path_regex += re.escape(path[idx:]) + "$"
path_format += path[idx:]

Expand Down
14 changes: 14 additions & 0 deletions tests/test_routing.py
Expand Up @@ -622,3 +622,17 @@ def test_partial_async_endpoint():
cls_method_response = test_client.get("/cls")
assert cls_method_response.status_code == 200
assert cls_method_response.json() == {"arg": "foo"}


def test_duplicated_param_names():
with pytest.raises(
ValueError,
match="Duplicated param name id at path /{id}/{id}",
):
Route("/{id}/{id}", user)

with pytest.raises(
ValueError,
match="Duplicated param names id, name at path /{id}/{name}/{id}/{name}",
):
Route("/{id}/{name}/{id}/{name}", user)