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

Post data #19

Open
arran4 opened this issue Jun 2, 2016 · 19 comments
Open

Post data #19

arran4 opened this issue Jun 2, 2016 · 19 comments
Milestone

Comments

@arran4
Copy link

arran4 commented Jun 2, 2016

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.

@cagataygurturk cagataygurturk added this to the v0.0.5 milestone Jun 2, 2016
@cagataygurturk
Copy link
Owner

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

@cagataygurturk cagataygurturk modified the milestones: v0.0.5, 0.0.6 Jun 5, 2016
@arran4
Copy link
Author

arran4 commented Jun 6, 2016

So no luck I take it?

@cagataygurturk
Copy link
Owner

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

@arran4
Copy link
Author

arran4 commented Jun 6, 2016

Awesome. Thanks!

@arranubels
Copy link

What's needed to make this change?

@arranubels
Copy link

arranubels commented Jun 27, 2016

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)

@cagataygurturk
Copy link
Owner

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.

@DavidBoman
Copy link

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?

@cagataygurturk
Copy link
Owner

cagataygurturk commented Jun 29, 2016

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.

@DavidBoman
Copy link

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?

@cagataygurturk
Copy link
Owner

Hey, please see the latest commits, f0b41bf...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.

@arranubels
Copy link

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
Copy link

Done. Let me know what I need to change.

@cagataygurturk
Copy link
Owner

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
Copy link
Owner

Be aware of 7340ae4 which adds automatic deserialization for post body.

@arranubels
Copy link

I'll rebase and create a new PR.

@arranubels
Copy link

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?

@cagataygurturk
Copy link
Owner

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.

@arranubels
Copy link

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

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

No branches or pull requests

4 participants