Skip to content

Conversation

@vkuptcov
Copy link
Contributor

@vkuptcov vkuptcov commented Feb 1, 2017

I've tried to use the Spring EchoApp in the real AWS Lambda and the /echo/query-string GET-request was failed for non-set query parameters. It happened because in this case AwsProxyRequest.queryStringParameters remains uninitialized.

public Enumeration<String> getParameterNames() {
List<String> paramNames = new ArrayList<>();
paramNames.addAll(request.getQueryStringParameters().keySet());
Map<String, String> queryStringParameters = request.getQueryStringParameters();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we reduce this to:
if (request.getQueryStringParameters() != null) {
paramsNames.addAll(request.getQueryStringParameters().keySet());
}


private String getQueryStringParameterCaseInsensitive(String key) {
if (request.getQueryStringParameters() == null) {
Map<String, String> queryStringParameters = request.getQueryStringParameters();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to assign here? Wouldn't this method have worked before?

Copy link
Contributor

@sapessi sapessi left a comment

Choose a reason for hiding this comment

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

Left a couple of comments. I'd like to try and avoid assignments unless necessary.

@sapessi
Copy link
Contributor

sapessi commented Feb 2, 2017

Thanks for the pull request, bugs slipped through the cracks. Left just a couple of comments in the commit.

@vkuptcov
Copy link
Contributor Author

vkuptcov commented Feb 2, 2017

@sapessi I've changed the code according to your comments.

@sapessi sapessi merged commit 78d4663 into aws:master Feb 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants