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

fix: Allow Proxy Paths to have arbitrary names #505

Merged
merged 3 commits into from
Jul 3, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
29 changes: 22 additions & 7 deletions samcli/local/apigw/path_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,31 @@

import re

FLASK_PATH_PARAMS = "/<path:proxy>"
APIGW_PATH_PARAMS_ESCAPED = r"/{proxy\+}"
APIGW_PATH_PARAMS = "/{proxy+}"
# The regex captures any path information before the {proxy+}. This is to support paths that have other params in
# them. Otherwise the regex will match the first { with the last +} giving incorrect results.
# Example: /id/{id}/user/{proxy+}. g<1> = '/id/{id}/user/' g<2> = 'proxy'
PROXY_PATH_PARAMS_ESCAPED = r"(.*/){(.*)\+}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated example looks better to me :)


# The regex replaces what was captured with PROXY_PATH_PARAMS_ESCAPED to construct the full path with the {proxy+}
# replaces. The first group is anything before {proxy+}, while the second group is the name given to the proxy.
# Example: /id/{id}/user/{resource+}; g<1> = '/id/{id}/user/'; g<2> = 'resource'
FLASK_CAPTURE_ALL_PATH = r"\g<1><path:\g<2>>"

# The regex will replace the first group from FLASK_CAPTURE_ALL_PATH_REGEX into the proxy name part of the APIGW path.
# Example: /<path:resource>; g<1> = 'resource'; output = /{resource+}
PROXY_PATH_PARAMS = r"/{\g<1>+}"

# The regex will capture the name of the path for the APIGW Proxy path.
# Example: /<path:resource> is equivalent to the APIGW path /{resource+}
FLASK_CAPTURE_ALL_PATH_REGEX = r"/<path:(.*)>"

LEFT_BRACKET = "{"
RIGHT_BRACKET = "}"
LEFT_ANGLE_BRACKET = "<"
RIGHT_ANGLE_BRACKET = ">"

APIGW_TO_FLASK_REGEX = re.compile(APIGW_PATH_PARAMS_ESCAPED)
FLASK_TO_APIGW_REGEX = re.compile(FLASK_PATH_PARAMS)
APIGW_TO_FLASK_REGEX = re.compile(PROXY_PATH_PARAMS_ESCAPED)
FLASK_TO_APIGW_REGEX = re.compile(FLASK_CAPTURE_ALL_PATH_REGEX)


class PathConverter(object):
Expand All @@ -31,7 +46,7 @@ def convert_path_to_flask(path):
:param str path: Path to convert to Flask defined path
:return str: Path representing a Flask path
"""
proxy_sub_path = APIGW_TO_FLASK_REGEX.sub(FLASK_PATH_PARAMS, path)
proxy_sub_path = APIGW_TO_FLASK_REGEX.sub(FLASK_CAPTURE_ALL_PATH, path)

# Replace the '{' and '}' with '<' and '>' respectively
return proxy_sub_path.replace(LEFT_BRACKET, LEFT_ANGLE_BRACKET).replace(RIGHT_BRACKET, RIGHT_ANGLE_BRACKET)
Expand All @@ -49,7 +64,7 @@ def convert_path_to_api_gateway(path):
:param str path: Path to convert to Api Gateway defined path
:return str: Path representing an Api Gateway path
"""
proxy_sub_path = FLASK_TO_APIGW_REGEX.sub(APIGW_PATH_PARAMS, path)
proxy_sub_path = FLASK_TO_APIGW_REGEX.sub(PROXY_PATH_PARAMS, path)

# Replace the '<' and '>' with '{' and '}' respectively
return proxy_sub_path.replace(LEFT_ANGLE_BRACKET, LEFT_BRACKET).replace(RIGHT_ANGLE_BRACKET, RIGHT_BRACKET)
23 changes: 22 additions & 1 deletion tests/functional/local/apigw/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ def setUpClass(cls):
list_of_routes = [Route(['POST', 'GET'], cls.function_name, '/something'),
Route(['GET'], cls.function_name, '/'),
Route(['GET', 'PUT'], cls.function_name, '/something/{event}'),
Route(['GET'], cls.function_name, '/proxypath/{proxy+}')
Route(['GET'], cls.function_name, '/proxypath/{proxy+}'),
Route(['GET'], cls.function_name, '/resourceproxypath/{resource+}')
]

cls.service, cls.port, cls.url, cls.scheme = make_service(list_of_routes, cls.mock_function_provider, cls.cwd)
Expand Down Expand Up @@ -285,6 +286,26 @@ def test_calling_proxy_path(self):
self.assertEquals(response.status_code, 200)
self.assertEquals(response.headers.get('Content-Type'), "application/json")

def test_calling_proxy_path_that_has_name_resource(self):
path = '/resourceproxypath/resourcepath/thatshouldbecaught'
expected = make_service_response(self.port,
scheme=self.scheme,
method="GET",
resourcePath="/resourceproxypath/{resource+}",
resolvedResourcePath=path,
pathParameters={"resource": "resourcepath/thatshouldbecaught"},
body=None,
queryParams=None,
headers=None)

response = requests.get(self.url + path)

actual = response.json()

self.assertEquals(actual, expected)
self.assertEquals(response.status_code, 200)
self.assertEquals(response.headers.get('Content-Type'), "application/json")

def test_calling_service_with_body_and_query_and_headers(self):
path = "/something/event1"
body = {"json": "data"}
Expand Down
24 changes: 24 additions & 0 deletions tests/unit/local/apigw/test_path_converter.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from unittest import TestCase

from parameterized import parameterized

from samcli.local.apigw.path_converter import PathConverter


Expand All @@ -19,6 +21,17 @@ def test_proxy_path(self):

self.assertEquals(flask_path, "/<path:proxy>")

@parameterized.expand([("/{resource+}", "/<path:resource>"),
("/a/{id}/b/{resource+}", "/a/<id>/b/<path:resource>"),
("/a/b/{proxy}/{resource+}", "/a/b/<proxy>/<path:resource>"),
("/{id}/{something+}", "/<id>/<path:something>"),
("/{a}/{b}/{c}/{d+}", "/<a>/<b>/<c>/<path:d>")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!

])
def test_proxy_path_with_different_name(self, path, expected_result):
flask_path = PathConverter.convert_path_to_flask(path)

self.assertEquals(flask_path, expected_result)

def test_proxy_with_path_param(self):
path = "/id/{id}/user/{proxy+}"

Expand Down Expand Up @@ -57,6 +70,17 @@ def test_proxy_path(self):

self.assertEquals(flask_path, "/{proxy+}")

@parameterized.expand([("/<path:resource>", "/{resource+}"),
("/a/<id>/b/<path:resource>", "/a/{id}/b/{resource+}"),
("/a/b/<proxy>/<path:resource>", "/a/b/{proxy}/{resource+}"),
("/<id>/<path:something>", "/{id}/{something+}"),
("/<a>/<b>/<c>/<path:d>", "/{a}/{b}/{c}/{d+}")
])
def test_proxy_path_with_different_name(self, path, expected_result):
flask_path = PathConverter.convert_path_to_api_gateway(path)

self.assertEquals(flask_path, expected_result)

def test_proxy_with_path_param(self):
path = "/id/<id>/user/<path:proxy>"

Expand Down