-
Notifications
You must be signed in to change notification settings - Fork 81
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: support binding Body form-url-encoded to POJO with lambda Apps #1414
Conversation
remove method formUrlEncodedToMap
@@ -0,0 +1,406 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is forked from Netty as we do in core
return Optional.empty(); | ||
} | ||
if (nestedBody(bodyArgument)) { | ||
return Optional.of(formUrlEncodedBodyToConvertibleValues(body)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If @Body("name")
, we are transforming the body into a map and then creating a ConvertibleValues
if (nestedBody(bodyArgument)) { | ||
return Optional.of(formUrlEncodedBodyToConvertibleValues(body)); | ||
} | ||
return bindFormUrlEncoded(bodyArgument, body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if Body is not nested), we are transforming the body into a map and then using BeanPropertyBinder
to bind map to POJO.
...-api-proxy/src/test/groovy/io/micronaut/function/aws/proxy/MicronautLambdaHandlerSpec.groovy
Outdated
Show resolved
Hide resolved
...-api-proxy/src/test/groovy/io/micronaut/function/aws/proxy/MicronautLambdaHandlerSpec.groovy
Outdated
Show resolved
Hide resolved
Sonatype complains mostly about forked from Netty |
Do we need to support multipart (ie a File upload) as well. Do we need a new issue for that? |
There are probably issues with multipart but I think we should target that in a different PR. |
I pushed a test, and some fixes for Sonar |
try { | ||
return Optional.of(beanPropertyBinder.bind(argument.getType(), source)); | ||
} catch (ConversionErrorException e) { | ||
return Optional.empty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swallowing this might be undesirable, what if the error needs to be returned to the user or the developer is trying to understand why binding failed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a log statement 515e34a
Kudos, SonarCloud Quality Gate passed! |
@graemerocher any concern with merging this? |
No description provided.