playframework / playframework

The Community Maintained High Velocity Web Framework For Java and Scala.
http://www.playframework.com
Apache License 2.0
12.52k stars 4.1k forks source link

Auto-deserialization and self-documenting API endpoints #5317

Open benmccann opened 8 years ago

benmccann commented 8 years ago

I'm working on a Play-Swagger integration. Swagger exports a description of your API, which lets you automatically generates client libraries for numerous languages. This makes writing an API server in Play much more appealing because you get all the client libraries for free.

One thing that makes supporting tools like this more difficult is that it's not obvious from method signatures what input an API expects in its request body.

Right now, you might have a method that looks like:

public Result save() {
    Pet pet = Json.fromJson(request().body().asJson(), Pet.class);

I'd like to add support for doing something like:

public Result save(Pet pet) {

This has several advantages:

I think we could make it such that a parameter that's not referred to in the routes file can be assumed to be the request body. It's also possible that we could also introduce some new syntax to support this auto-deserialization like an annotation (@Body Pet pet) or certain parameter name (Pet body).

gmethvin commented 8 years ago

One possibility is to change the Java API to be more similar to the Scala API. If your method looked like

public Action<Pet> save() {
  return action(Pet.class).handler((request) -> {
    Pet pet = request.body;
    // do stuff
  });
}

you can easily tell Action accepts a Pet. Though you do need a way to easily retrieve a BodyParser.

I really like the simplicity of actions behaving like functions that you can manually compose, without requiring the "magic" of runtime annotations. From a framework design perspective this seems nice but I know it's more verbose from a Java perspective. I'm wondering if there are any other ways we could make it shorter.

benmccann commented 8 years ago

@gmethvin thanks for taking a look!

I agree. I don't think you can make your example any shorter really. You could change action(Pet.class) to action() and I think the type would be inferred. But I think you're still going to need to specify it for use by a body parser. Perhaps your example was already doing that and is just a more concise way of writing this:

public Action<Pet> save() {
  return action().pojoParser(Pet.class).handler((request) -> {
    Pet pet = request.body;
    // do stuff
  });
}

I'm realizing now that my example is going to have trouble because I don't have a Pet.class anywhere in there and the json deserializer (or xml, etc.) will need that. Hmm...

gmethvin commented 8 years ago

Also we'd need a way to map types to body parsers, and I'm not sure whether that should be global or local to the action or controller. What if you wanted a JSON Pet parser in one place and an XML Pet parser somewhere else?

benmccann commented 8 years ago

I was thinking you'd have a PojoBodyParser that's passed the content-type. If it's application/json then you use the JSON deserializer and if it's application/xml then you use the XML deserializer.

wsargent commented 8 years ago

This seems like it could have some commonalities with the work @godenji did in binding value types -- the difference here is POST.

If you had support for a POST body in routes, you could do something like:

POST /save                      controllers.Application.save(body: Pet)

And then the "RequestBodyBindable" would look at the content type, see "application/json" and try to create a Pet from there. Make sense?

gmethvin commented 8 years ago

@wsargent But how do we know whether to look for a RequestBodyBindable or a QueryStringBindable?

I do think we should start making it easier to create a body parser for specific types. Even a shorthand for being able to manually create a JSON parser for a specific type would be nice, e.g. parse.jsonOf[Pet]

wsargent commented 8 years ago

@gmethvin Right... that whole "auto bind a query parameter" thing that Play routing does (only it won't bind "some-param" as it's not identifiable as a javaKeyword, which is another rant) is an issue.

I would probably put a special prefix for Request Body parameters and wrap the whole thing in back-ticks, something like save(bbody: Pet, qquery:Int) etc.

josephearl commented 8 years ago

To me the example with having support for body in routes makes most sense and is easily understood :+1: This would be a great feature to have, love the idea of having the type of Action/Result in the controller method.

benmccann commented 7 years ago

Can't we just tell the variable name is "body" rather than having to put a special prefix?

domdorn commented 7 years ago

hmm.. when thinking about that, we should also think on how to include RAML additional to swagger (some say RAML is the superior approach). My approach to this before was to generate the swagger file first and use the swagger compilers to generate API stubs, controllers, bodyparsers, models, etc. for me..

Making it the other way around (so from existing code to api spec) seems very hard to do and fragile in the resulting implementation.

angadsalaria commented 6 years ago

Attempting to revive this issue. Highly agree with the proposition and would go further along and say that other request attributes such as headers should also be made available, albeit optionally.

Any current work being done towards this effort?