Add typed body on request and response

[wsargent]

Add typed body to Play WS. After this, Play-WS should not have any dependencies at all on the body -- the response has a body of String / ByteString / Source[ByteString]. If you want to get something, you need to add a BodyReadable or BodyWritable function to transform it.

So for Scala, you use a context bound:

implicit val jsonWritable: BodyWritable[JsValue] = new JsonWritable(objectMapper)

val json: JsValue = ...
request.post(json);

For Java you do:

interface DefaultBodyWritables {
   default BodyWritable<JsonNode> body(JsonNode node) { ... }
}

And then you can use overloading so it's not quite so ugly:

class MyClass extends DefaultBodyWritables {
    request.post(body(json));
}

Although this only works so far, and falls over completely when you use generics, i.e. it can't tell a Source from a Source<Multipart.Part<Source>>, so you have to do things like

request.post(multipartBody(parts));

For readables, it looks the same, only you have to specify the type. So for Scala:

implicit val stringReadable: BodyReadable[String] = ...
val string = response.body[String]

And for Java, it looks like this:

class MyClass extends DefaultBodyReadables {
   JsonNode node = response.body(json());
}

[gmethvin]

Why can't we just have play-ws-standalone-json and not have a dependency on ahc? I suppose it doesn't matter much since AHC is the only real implementation we have, but just wondering.

[schmitch]

well the problem here is probably that we need the underlying response/request type, which is a ahc class.

[gmethvin]

Isn't this just a special case of the streamed body?

[schmitch]

well I think for performance reasons it's good to have both.

[gmethvin]

It would be nice if we could use a common interface for both the Writeable interface used for Play bodies and this one.

[gmethvin]

Not saying that needs to be implemented now but would be interested in getting your thoughts on if that'd be eventually possible to write a single implementation that works for Play action and WS bodies.

[wsargent]

This is tricky as we really need something that does => Source[ByteString] as the base type, and Writable does ByteString.

[gmethvin]

What happens if I try to do this on a non-streamed body?

[wsargent]

Source.single(response.bodyAsBytes) :-)

[schmitch]

Well small changes needed, but besides them it looks really really good.
Hm. The only thing which gmethvin already mentioned is that the play-json/xml readable/writeables need to have a direct dependency on ahc. But I think there is not a lot to go around that.

[schmitch]

for what is this instance? Especially since it's package-private?

[schmitch]

I think we might just inline it:
possiblyModifiedHeaders.set(HttpHeaders.Names.CONTENT_TYPE, Collections.singletonList(contentType)).
I mean the possibility is high that the java compiler will do that, but I think there is no reason to keep a variable around just for passing it a line below

[schmitch]

I think this is incorrect.
Basically you can't statically import default methods.
Well In the Java API I would prefer to have both ways either to have:

import play.libs.ws.ahc.DefaultBodyReadables;
import play.libs.ws.ahc.DefaultBodyWritables;

public class MyClient implements DefaultBodyWritables, DefaultBodyReadables {    
    public CompletionStage<String> doStuff() {
      return client.url("http://example.com").post(body("hello world")).thenApply(response ->
         response.body(string())
       );
     }
 }

and a second version which does not have a implements:

import static play.libs.ws.ahc.DefaultBodyReadables.Static.*;
import static play.libs.ws.ahc.DefaultBodyWritables.Static.*;

public class MyClient implements DefaultBodyWritables, DefaultBodyReadables {    
    public CompletionStage<String> doStuff() {
      return client.url("http://example.com").post(body("hello world")).thenApply(response ->
         response.body(string())
       );
     }
 }

Example:

public interface DefaultBodyWritables {

    DefaultBodyWritables instance = new DefaultBodyWritables() {};

    /**
     * Creates a {@link InMemoryBodyWritable} from a String, setting the content-type to "text/plain".
     *
     * @param s the string to pass in.
     * @return a {@link InMemoryBodyWritable} instance.
     */
    default InMemoryBodyWritable body(String s) {
        return new InMemoryBodyWritable(ByteString.fromString(s), "text/plain");
    }

public final class Static {

static InMemoryBodyWritable body(String s) {
        return instance.body(s);
    }

}

  // ...

I think that's the only way to mimic the Scala object DefaultBodyWritables. Another way would be to declare all methods as static then nobody would need to implement the interfaces, which is probably more java-ish but also harder for backwards compatibility.

[schmitch]

well the problem here is probably that we need the underlying response/request type, which is a ahc class.

[schmitch]

well I think for performance reasons it's good to have both.

[schmitch]

I think this should be removed completly since it's no longer in use (or deprecated)

[wsargent]

Note CONTENT_BASE is the wrong field, so we fix this here

[wsargent]

enable request filters for requests that go through stream rather than execute()

[wsargent]

Only in-memory or source

PR applied changes and now differs from my initial review