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: Value of QueryString in the API event should only be a string #405

Merged
merged 2 commits into from
May 31, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion samcli/local/apigw/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,16 @@ def _construct_event(flask_request, port, binary_types):
event_headers["X-Forwarded-Proto"] = flask_request.scheme
event_headers["X-Forwarded-Port"] = str(port)

# APIGW does not support duplicate query parameters. Flask gives query params as a list so
# we need to convert only grab the first item unless many were given, were we grab the last to be consistent
# with APIGW
query_string_dict = Service._query_string_params(flask_request)

event = ApiGatewayLambdaEvent(http_method=method,
body=request_data,
resource=endpoint,
request_context=context,
query_string_params=flask_request.args,
query_string_params=query_string_dict,
headers=event_headers,
path_parameters=flask_request.view_args,
path=flask_request.path,
Expand All @@ -383,6 +388,37 @@ def _construct_event(flask_request, port, binary_types):
LOG.debug("Constructed String representation of Event to invoke Lambda. Event: %s", event_str)
return event_str

@staticmethod
def _query_string_params(flask_request):
"""
Constructs an APIGW equivalent query string dictionary

Parameters
----------
flask_request request
Request from Flask

Returns dict (str: str)
-------
Empty dict if no query params where in the request otherwise returns a dictionary of key to value

"""
query_string_dict = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Most variables in this method are named query_string_<type> or some flavor of it. This is not descriptive. You don't need the prefix because you are inside a method that processes query string parameters


# Flask returns an ImmutableMultiDict so convert to a dictionary that becomes
# a dict(str: list) then iterate over
for query_string_key, query_string_list in dict(flask_request.args).items():
query_string_value_length = len(query_string_list)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This variable is unnecessary. You can just do if query_string_listto know if the list is empty or not


# if the list is empty, default to empty string
if not query_string_value_length:
query_string_dict[query_string_key] = ""
else:
# APIGW doesn't handle duplicate query string keys, picking the last one in the list
query_string_dict[query_string_key] = query_string_list[-1]

return query_string_dict

@staticmethod
def _should_base64_encode(binary_types, request_mimetype):
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/local/apigw/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def test_calling_service_with_body_and_query_and_headers(self):
resolvedResourcePath=path,
pathParameters={"event": "event1"},
body=json.dumps(body),
queryParams={"key": ["value"]},
queryParams={"key": "value"},
headers={"X-Test": "TestValue", "Content-Length": "16",
"Content-Type": "application/json"})

Expand Down
15 changes: 14 additions & 1 deletion tests/integration/local/start_api/test_start_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,20 @@ def test_request_with_query_params(self):

response_data = response.json()

self.assertEquals(response_data.get("queryStringParameters"), {"key": ["value"]})
self.assertEquals(response_data.get("queryStringParameters"), {"key": "value"})

def test_request_with_list_of_query_params(self):
"""
Query params given should be put into the Event to Lambda
"""
response = requests.get(self.url + "/id/4",
params={"key": ["value", "value2"]})

self.assertEquals(response.status_code, 200)

response_data = response.json()

self.assertEquals(response_data.get("queryStringParameters"), {"key": "value2"})

def test_request_with_path_params(self):
"""
Expand Down
23 changes: 22 additions & 1 deletion tests/unit/local/apigw/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ def setUp(self):
self.request_mock.method = "GET"
self.request_mock.remote_addr = "190.0.0.0"
self.request_mock.data = b"DATA!!!!"
self.request_mock.args = {"query": "params"}
self.request_mock.args = {"query": ["params"]}
self.request_mock.headers = {"Content-Type": "application/json", "X-Test": "Value"}
self.request_mock.view_args = {"path": "params"}
self.request_mock.scheme = "http"
Expand Down Expand Up @@ -489,6 +489,27 @@ def test_construct_event_with_binary_data(self, should_base64_encode_patch):
actual_event_str = Service._construct_event(self.request_mock, 3000, binary_types=[])
self.assertEquals(json.loads(actual_event_str), self.expected_dict)

def test_query_string_params_with_empty_params(self):
request_mock = Mock()
request_mock.args = {}

actual_query_string = Service._query_string_params(request_mock)
self.assertEquals(actual_query_string, {})

def test_query_string_params_with_param_value_being_empty_list(self):
request_mock = Mock()
request_mock.args = {"param": []}

actual_query_string = Service._query_string_params(request_mock)
self.assertEquals(actual_query_string, {"param": ""})

def test_query_string_params_with_param_value_being_non_empty_list(self):
request_mock = Mock()
request_mock.args = {"param": ["a", "b"]}

actual_query_string = Service._query_string_params(request_mock)
self.assertEquals(actual_query_string, {"param": "b"})


class TestService_service_response(TestCase):

Expand Down