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 support for greedy catch-all path variables #2012

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions chalice/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
if TYPE_CHECKING:
from chalice.local import LambdaContext

_PARAMS = re.compile(r'{\w+}')
# the optional + at the end is for supporting the special greedy parameter in
# API Gateway (ie. "{proxy+}")
_PARAMS = re.compile(r'{(\w+)\+?}')
MiddlewareFuncType = Callable[[Any, Callable[[Any], Any]], Any]
UserHandlerFuncType = Callable[..., Any]

Expand Down Expand Up @@ -577,9 +579,7 @@ def __init__(self, view_function: Callable[..., Any], view_name: str,
def _parse_view_args(self) -> List[str]:
if '{' not in self.uri_pattern:
return []
# The [1:-1] slice is to remove the braces
# e.g {foobar} -> foobar
results = [r[1:-1] for r in _PARAMS.findall(self.uri_pattern)]
results = _PARAMS.findall(self.uri_pattern)
return results

def __eq__(self, other: object) -> bool:
Expand Down
15 changes: 13 additions & 2 deletions chalice/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,19 @@ def match_route(self, url):
captured = {}
for route_url in self.route_urls:
url_parts = route_url.split('/')
if len(parts) == len(url_parts):
for i, j in zip(parts, url_parts):
parts_copy = parts.copy()

# Handle a greedy catch-all path variable (ie. "proxy+")
if url_parts[-1].endswith('+}'):
pos = len(url_parts) - 1
if len(parts) > pos:
catch_all_param = url_parts[-1][1:-2]
captured[catch_all_param] = '/'.join(parts[pos:])
url_parts = url_parts[:-1]
parts_copy = parts_copy[:pos]

if len(parts_copy) == len(url_parts):
for i, j in zip(parts_copy, url_parts):
if j.startswith('{') and j.endswith('}'):
captured[j[1:-1]] = i
continue
Expand Down
4 changes: 4 additions & 0 deletions tests/aws/test_features.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,10 @@ def test_supports_path_params(smoke_test_app):
assert smoke_test_app.get_json('/path/bar') == {'path': 'bar'}


def test_supports_catch_all_param(smoke_test_app):
assert smoke_test_app.get_json('/catch-all/a/b') == {'proxy': 'a/b'}


def test_path_params_mapped_in_api(smoke_test_app, apig_client):
# Use the API Gateway API to ensure that path parameters
# are modeled as such. Otherwise this will break
Expand Down
5 changes: 5 additions & 0 deletions tests/aws/testapp/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,11 @@ def repr_raw_body():
return {'repr-raw-body': app.current_request.raw_body.decode('utf-8')}


@app.route('/catch-all/{proxy+}', methods=['GET'])
def catch_all(proxy):
return {'proxy': proxy}


SOCKET_MESSAGES = []


Expand Down
14 changes: 14 additions & 0 deletions tests/functional/test_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,17 @@ def test_can_reload_server(unused_tcp_port, basic_app, http_session):
assert http_session.json_get(url) == {'version': 'reloaded'}
finally:
p.terminate()


def test_can_parse_proxy_catch_all_route(
config, local_server_factory):
demo = app.Chalice('app-name')

@demo.route('/{proxy+}')
def proxy_view(proxy):
return proxy

local_server, port = local_server_factory(demo, config)
response = local_server.make_call(requests.get, '/any/thing', port)
assert response.status_code == 200
assert response.text == 'any/thing'
6 changes: 6 additions & 0 deletions tests/unit/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,12 @@ def test_can_parse_route_view_args():
assert entry.view_args == ['bar', 'qux']


def test_can_parse_catch_all_route_view_args():
entry = app.RouteEntry(lambda: {"foo": "bar"}, 'view-name',
'/foo/{proxy+}', method='GET')
assert entry.view_args == ['proxy']


def test_can_route_single_view():
demo = app.Chalice('app-name')

Expand Down
8 changes: 7 additions & 1 deletion tests/unit/test_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -595,12 +595,18 @@ def test_multi_value_header(handler):
('/names/bar/wrong', None),
('/a/z/c', '/a/{capture}/c'),
('/a/b/c', '/a/b/c'),
('/x', None),
('/x/foo', '/x/{proxy+}'),
('/x/foo/bar', '/x/{proxy+}'),
('/y/foo/bar', '/y/{capture}/{proxy+}'),
('/y/foo/bar/baz', '/y/{capture}/{proxy+}'),
])
def test_can_match_exact_route(actual_url, matched_url):
matcher = local.RouteMatcher([
'/foo', '/foo/{capture}', '/foo/bar',
'/names/{capture}',
'/a/{capture}/c', '/a/b/c'
'/a/{capture}/c', '/a/b/c',
'/x/{proxy+}', '/y/{capture}/{proxy+}'
])
if matched_url is not None:
assert matcher.match_route(actual_url).route == matched_url
Expand Down