lambadaframework icon indicating copy to clipboard operation
lambadaframework copied to clipboard

Post data

Open arran4 opened this issue 8 years ago • 19 comments

Post data doesn't seem to deserialize. It complains of wrong number of arguments with this sort of definition:

@POST
@PATH("/post")
public Response(String body)

And the following simply doesn't work:

@POST
@PATH("/post")
public Response(@FormParam("query1") String query)

Sorry for the code examples; I'm away from my desk atm.

arran4 avatar Jun 02 '16 09:06 arran4

Seems that serious bug, I'll try to fix it and include to 0.5 release this week. Thanks!

cagataygurturk avatar Jun 02 '16 14:06 cagataygurturk

So no luck I take it?

arran4 avatar Jun 06 '16 00:06 arran4

I am fixing the issue right now and it'll be released in 0.0.6 in the upcoming days

cagataygurturk avatar Jun 06 '16 06:06 cagataygurturk

Awesome. Thanks!

arran4 avatar Jun 06 '16 06:06 arran4

What's needed to make this change?

arranubels avatar Jun 20 '16 23:06 arranubels

Okay I got some more details. I get the following error:

java.lang.IllegalArgumentException: wrong number of arguments at 
sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at 
java.lang.reflect.Method.invoke(Method.java:498) at 
org.lambadaframework.runtime.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:107) at 
org.lambadaframework.runtime.Handler.handleRequest(Handler.java:62) at 
org.lambadaframework.runtime.Handler.handleRequest(Handler.java:13) at 
lambdainternal.EventHandlerLoader$PojoHandlerAsStreamHandler.handleRequest(EventHandlerLoader.java:370) at lambdainternal.EventHandlerLoader$2.call(EventHandlerLoader.java:972) at 
lambdainternal.AWSLambda.startRuntime(AWSLambda.java:231) at lambdainternal.AWSLambda.<clinit>(AWSLambda.java:59) at java.lang.Class.forName0(Native Method) at 
java.lang.Class.forName(Class.java:348) at 
lambdainternal.LambdaRTEntry.main(LambdaRTEntry.java:93)

Regardless of the permutations of configurations I've tried that seems to be the result.

With the following changes to the boilerplate demo: Added to pom dep:

        <dependency>
            <groupId>com.amazonaws</groupId>
            <artifactId>aws-lambda-java-log4j</artifactId>
            <version>1.0.0</version>
        </dependency>

The whole example controller:

@Path("/")
public class ExampleController {
    static final Logger logger = Logger.getLogger(ExampleController.class);

    public static class Input {
        public String value;

        public Input(String value) {
            this.value = value;
        }
    }

    @POST
    @Path("test/test1")
    @Produces(MediaType.APPLICATION_JSON)
    @Consumes(MediaType.APPLICATION_JSON)
    public Response test1(@FormParam("value") Input input) {

        logger.debug("Request got");
        return Response.status(201)
                .entity(input)
                .build();
    }
}

(I've tried a number of different variants here. @QueryParam, no annotations, of String type, various @Consumes just to be sure.

Log4j.properties:

log = .
log4j.rootLogger = DEBUG, LAMBDA

#Define the LAMBDA appender
log4j.appender.LAMBDA=com.amazonaws.services.lambda.runtime.log4j.LambdaAppender
log4j.appender.LAMBDA.layout=org.apache.log4j.PatternLayout
log4j.appender.LAMBDA.layout.conversionPattern=%d{yyyy-MM-dd HH:mm:ss} <%X{AWSRequestId}> %-5p %c{1}:%L - %m%n

My test: curl -X POST -d @testfiles/data.txt https://xxx.execute-api.eu-west-1.amazonaws.com/production/test/test1 --header "Content-Type: APPLICATION/JSON"

And test file:

{ "value": "testtesttest" }

Also, I'm using the latest code in master. (0.0.6)

arranubels avatar Jun 27 '16 06:06 arranubels

There is basically no support for those annotations, that's why it is not working :)

Supported parameter annotations are listed here:

https://github.com/lambadaframework/lambadaframework/blob/master/lambada-maven-plugin/src/main/java/org/lambadaframework/aws/ApiGateway.java#L507

https://github.com/lambadaframework/lambadaframework/blob/bcf18d50ffa33ad3274fe5cdf749cf80e19c76c7/runtime/src/main/java/org/lambadaframework/runtime/ResourceMethodInvoker.java#L66

This two files should be modified, and maybe there should be another little change. I'll be adding this support as soon as I've some time.

cagataygurturk avatar Jun 27 '16 08:06 cagataygurturk

I'm also in desperate need for this! @arran4 , @cagataygurturk - should we join forces and try to fix it?

@cagataygurturk - can you elaborate on what you think is needed?

DavidBoman avatar Jun 27 '16 11:06 DavidBoman

It's a bit more complicated than I was thinking. API Gateway is designed to be REST-compliant, which means they like to accept POST parameters as JSON body. Still looking for a way to support this feature. Please stay tuned.

As a general practice, if you can avoid post params in your application, I'd recommend to accept request body as JSON instead of form values.

cagataygurturk avatar Jun 29 '16 14:06 cagataygurturk

I'm all fine with JSON. The body of my post-data is a pure JSON-snippet. Perhaps @arranubels FormParams are more difficult in that sense. My problem is that I get the "wrong number of arguments" even with a pure JSON-body. Looking in the code mentioned above I can't see anything that performs unmarshalling of the body to a POJO. Do you have something un-commited in the works or is all body-processing missing?

DavidBoman avatar Jun 30 '16 08:06 DavidBoman

Hey, please see the latest commits, https://github.com/lambadaframework/lambadaframework/compare/f0b41bf7dc86cd9ba17682b0d2b74f8a338352c5...master

It was very quick fix and I did not write any test.

  1. Clone the repo
  2. mvn install -DskipTests to install the development version to your local repository
  3. Change lambada version to 0.0.6 (which is the next release), maven will use the development version from your repo
  4. In your controller use post data like this:
    @POST
    @Consumes(MediaType.APPLICATION_JSON)
    @Path("/resource")
    public Response exampleSecondEndpointPost(
            String requestBody
    ) {

        logger.debug("Request got");
        return Response.status(201)
                .entity(new Entity(requestBody))
                .build();
    }

Consumes annotation is important and parameter type should be String.

I was also planning to add automatic deserialisation of JSON but to do that I've to add Jackson (or other JSON library) to runtime dependencies which would increase JAR file size.

I'll look if Jersey is supporting automatic deserialisation because we've to be full jersey-compliant.

If this feature is working for you guys I'll include it to the next release which will happen this weekend.

cagataygurturk avatar Jun 30 '16 09:06 cagataygurturk

Ok. I have closed my old PR.. Creating a new one with some new changes with regards how post works. I don't understand the Api Gateway side enough to make changes to allow actual url encoded post data to be passed through. My changes should work fine once those changes have been made. I have used extensive use of the jacksons objectmapper.

Unfortunately I have formatted the code slightly differently to how it currently has.. So sorry for the large white space changes.

I will put the PR through in the next couple hours.

arranubels avatar Jul 07 '16 03:07 arranubels

Done. Let me know what I need to change.

arranubels avatar Jul 07 '16 04:07 arranubels

Please see it: https://forums.aws.amazon.com/message.jspa?messageID=668814#668814 As i said it is a bit tricky. Let's see if we can add @FormData as well.

cagataygurturk avatar Jul 07 '16 04:07 cagataygurturk

Be aware of https://github.com/lambadaframework/lambadaframework/commit/7340ae4b4a9b9d472be73e54743a07294c48e7e7 which adds automatic deserialization for post body.

cagataygurturk avatar Jul 07 '16 04:07 cagataygurturk

I'll rebase and create a new PR.

arranubels avatar Jul 13 '16 23:07 arranubels

Are you going to consider using Jackson's ObjectMapper's ReadValue function for a substitute to the toObject function you're currently using, as we have it as a dependency anyway?

arranubels avatar Jul 14 '16 05:07 arranubels

I think it's good option to use that method, instead of our "bad" toObject method. Thanks god Jackson comes as a dependency in Lambda runtime.

cagataygurturk avatar Jul 14 '16 06:07 cagataygurturk

I haven't completed eliminated it in my PR. But removing it should help remove most of the explicit typecasts..

arranubels avatar Aug 01 '16 05:08 arranubels