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

Update for play-ws with BodyReadable/BodyWritables. #7413

Merged
merged 10 commits into from Jun 19, 2017

Conversation

wsargent
Copy link
Member

@wsargent wsargent commented Jun 4, 2017

PR for playframework/play-ws#124 to add typed BodyReadables and BodyWritables.

All of the existing methods on WSRequest and WSResponse are around, but deprecated.

Updated for documentation.

@wsargent wsargent force-pushed the refactor-for-typed-ws-body branch 3 times, most recently from 6cb9003 to 76816f3 Compare June 14, 2017 23:51
@wsargent wsargent force-pushed the refactor-for-typed-ws-body branch 3 times, most recently from 452c608 to cfad753 Compare June 15, 2017 04:11
import akka.util.ByteString
import play.shaded.ahc.org.asynchttpclient.{ Response => AHCResponse }
import play.api.libs.json.JsValue
import play.api.libs.ws._

import scala.xml.Elem

object AhcWSResponse extends WSBodyReadables
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed


def stream(): Future[StreamedResponse] = underlying.stream()
//override def put(body: java.io.File): Future[Response] = withBody(body).execute("PUT")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove comments

override def post(body: Source[MultipartFormData.Part[Source[ByteString, _]], _]): Future[Response] =
withBody(body).execute("POST")
override def post(body: java.io.File): Future[Response] = withBody(body).execute("POST")
//override def post(body: Source[MultipartFormData.Part[Source[ByteString, _]], _]): Future[Response] =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove comments

@@ -13,6 +13,12 @@
</encoder>
</appender>

<logger name="play.api.libs.ws.ahc" level="DEBUG"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this

package play.libs.ws;

/**
*
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add documentation

### Submitting multipart/form data

The easiest way to post multipart/form data is to use a `Source<Http.MultipartFormData.Part<Source<ByteString>, ?>, ?>`
The easiest way to post multipart/form data is to use a `Source<Http.MultipartFormData.Part<Source<ByteString>, ?>, ?>` using `multipartBody()`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From where?

### Streaming data
### Submitting JSON data

The easiest way to post JSON data is to use the [[JSON library|JavaJsonActions]] with the WSBodyWritables `body(JsonNode)` method.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provide link to WSBodyWritables

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And show that you can pass in an object mapper


`WS` lets you consume the response's body incrementally by using an Akka Streams `Sink`. The `stream()` method on `WSRequest` returns a `CompletionStage<StreamedResponse>`. A `StreamedResponse` is a simple container holding together the response's headers and body.
You can consume the response's body incrementally by using an [Akka Streams](http://doc.akka.io/docs/akka/current/java/stream/stream-flows-and-basics.html) `Sink`. The `stream()` method on `WSRequest` returns a `CompletionStage<WSResponse>`, where the `WSResponse` contains a `bodyAsSource` method that provides a `Source<ByteString, ?>`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a link to the stream method and bodyAsSource method.

### Processing a response as XML

Similarly, you can process the response as XML by calling `response.asXml()`.
Similarly, you can process the response as XML by calling `response.getBody(xml())` with the `xml()` method provided by `WSBodyReadables`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add link to xml and WSBodyReadables

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

say XML Document

@@ -246,10 +257,6 @@ You can get access to the underlying shaded [AsyncHttpClient](http://static.java

@[ws-underlying-client](code/javaguide/ws/JavaWS.java)

This is important in a couple of cases. The WS library has a couple of limitations that require access to the underlying client:

* `WS` does not support streaming body upload. In this case, you should use the [`FeedableBodyGenerator`](http://static.javadoc.io/org.asynchttpclient/async-http-client/2.0.0/org/asynchttpclient/request/body/generator/FeedableBodyGenerator.html) provided by AsyncHttpClient.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was invalidated by request.setBody(Source) method that @schmitch added

@@ -283,8 +290,6 @@ The following advanced settings can be configured on the underlying AsyncHttpCli

Please refer to the [AsyncHttpClientConfig Documentation](http://static.javadoc.io/org.asynchttpclient/async-http-client/2.0.0/org/asynchttpclient/DefaultAsyncHttpClientConfig.Builder.html) for more information.

> **Note:** `allowPoolingConnection` and `allowSslConnectionPool` are combined in AsyncHttpClient 2.0 into a single `keepAlive` variable. As such, `play.ws.ning.allowPoolingConnection` and `play.ws.ning.allowSslConnectionPool` are not valid and will throw an exception if configured.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not valid in 2.6.x


@[headers](code/ScalaWSSpec.scala)

If you are sending plain text in a particular format, you may want to define the content type explicitly.

@[content-type](code/ScalaWSSpec.scala)

### Request with cookies

Cookies can be added to the request by using `DefaultWSCookie` or by passing through [`play.api.mvc.Cookie`](api/scala/play/api/mvc/Cookie.html).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a link to DefaultWSCookie


@[query-string](code/ScalaWSSpec.scala)

### Request with additional headers

Headers can be specified as a series of key/value tuples.
Headers can be specified as a series of key/value tuples. Use `addHttpHeaders` to append additional headers, and `withHttpHeaders` to overwrite all headers.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add link to addHttpHeaders

@@ -65,20 +67,26 @@ If an HTTP call results in a 302 or a 301 redirect, you can automatically follow

### Request with query parameters

Parameters can be specified as a series of key/value tuples.
Parameters can be specified as a series of key/value tuples. Use `addQueryStringParameters` to add parameters, and `withQueryStringParameters` to overwrite all query string parameters.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add link to addQueryStringParameters

@@ -51,6 +51,8 @@ You end by calling a method corresponding to the HTTP method you want to use. T

This returns a `Future[WSResponse]` where the [Response](api/scala/play/api/libs/ws/WSResponse.html) contains the data returned from the server.

WSRequest extends [`play.api.libs.ws.WSBodyWritables`](api/scala/play/api/libs/ws/WSBodyWritables.html), which contains type classes for converting body input into a `ByteString`. You can create your own type classes if you would like to post, patch or put a custom type into the request.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discuss putting together a custom BodyWritables


For example, imagine you have executed a database query that is returning a large image, and you would like to forward that data to a different endpoint for further processing. Ideally, if you can send the data as you receive it from the database, you will reduce latency and also avoid problems resulting from loading in memory a large set of data. If your database access library supports [Reactive Streams](http://www.reactive-streams.org/) (for instance, [Slick](http://slick.typesafe.com/) does), here is an example showing how you could implement the described behavior:

@[scalaws-stream-request](code/ScalaWSSpec.scala)

The `largeImageFromDB` in the code snippet above is an Akka Streams `Source[ByteString, _]`.
The `largeImageFromDB` in the code snippet above is a `Source[ByteString, _]`.

### Request Filters

You can do additional processing on a WSRequest by adding a request filter. A request filter is added by extending the `play.api.libs.ws.WSRequestFilter` trait, and then adding it to the request with `request.withRequestFilter(filter)`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add link to WSRequestFilter

@@ -178,15 +184,15 @@ The JSON library has a [[useful feature|ScalaJsonCombinators]] that will map an

### Processing a response as XML

You can process the response as an [XML literal](http://www.scala-lang.org/api/current/index.html#scala.xml.NodeSeq) by calling `response.xml`.
You can process the response as an [XML literal](http://www.scala-lang.org/api/current/index.html#scala.xml.NodeSeq) by calling `response.body[Elem]`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discuss using streaming with a Source for StaX elements


`WS` lets you consume the response's body incrementally by using an Akka Streams `Sink`. The `stream()` method on `WSRequest` returns a `Future[StreamedResponse]`. A `StreamedResponse` is a simple container holding together the response's headers and body.
`WS` lets you consume the response's body incrementally by using an [Akka Streams](http://doc.akka.io/docs/akka/current/scala/stream/stream-flows-and-basics.html) `Sink`. The `stream()` method on `WSRequest` returns a streaming `WSResponse` which contains a `bodyAsSource` method that returns a `Source[ByteString, _]`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add link to bodyAsSource method.

@@ -222,7 +228,7 @@ When making a request from a controller, you can map the response to a `Future[R

### Using WSClient with unreliable networks

If you are calling out to an [unreliable network](https://queue.acm.org/detail.cfm?id=2655736) or doing any blocking work, including any kind of DNS work such as calling [`java.util.URL.equals()`](https://docs.oracle.com/javase/8/docs/api/java/net/URL.html#equals-java.lang.Object-), then you should use a custom execution context as described in [[ThreadPools]], preferably through [`play.api.libs.concurrent.CustomExecutionContext`](api/scala/play/api/libs/concurrent/CustomExecutionContext.html). You should size the pool to leave a safety margin large enough to account for futures, and consider using [`play.api.libs.concurrent.Futures`](api/scala/play/api/libs/concurrent/Futures.html) and a [Failsafe Circuit Breaker](https://github.com/jhalterman/failsafe#circuit-breakers).
If you are doing any blocking work, including any kind of DNS work such as calling [`java.util.URL.equals()`](https://docs.oracle.com/javase/8/docs/api/java/net/URL.html#equals-java.lang.Object-), then you should use a custom execution context as described in [[ThreadPools]], preferably through [`play.api.libs.concurrent.CustomExecutionContext`](api/scala/play/api/libs/concurrent/CustomExecutionContext.html). You should size the pool to leave a safety margin large enough to account for futures, and consider using [`play.api.libs.concurrent.Futures`](api/scala/play/api/libs/concurrent/Futures.html) and a [Failsafe Circuit Breaker](https://github.com/jhalterman/failsafe#circuit-breakers).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention BlockContext / blocking as alternatives?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you've written is good. The BlockContext can prevent deadlock, but it doesn't prevent heavily degraded performance if you get your thread pools wrong in the first place.

@@ -274,10 +280,6 @@ You can get access to the underlying [AsyncHttpClient](http://static.javadoc.io/

@[underlying](code/ScalaWSSpec.scala)

This is important in a couple of cases. WSClient has a couple of limitations that require access to the underlying client:

* `WSClient` does not support streaming body upload. In this case, you should use the `FeedableBodyGenerator` provided by AsyncHttpClient.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added in 2.5.x by @schmitch

@@ -78,9 +78,15 @@ For example, if you are sending plain text in a particular format, you may want

@[ws-header-content-type](code/javaguide/ws/JavaWS.java)

### Request with cookie

You can specify cookies for a request.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention WSCookieBuilder


### Submitting XML data

The easiest way to post XML data is to use Play's XML support:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provide link to play.libs.XML

@@ -115,7 +115,7 @@ import org.specs2.execute.AsResult
def forward(request: WSRequest): BodyParser[WSResponse] = BodyParser { req =>
Accumulator.source[ByteString].mapFuture { source =>
request
.withBody(StreamedBody(source))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add migration note to say that StreamedBody and WSResponseHeaders have been removed

@@ -38,8 +37,8 @@
private final AhcWSClient client;

@Inject
public AhcWSClientProvider(AsyncHttpClient asyncHttpClient, Materializer materializer, ObjectMapper objectMapper) {
client = new AhcWSClient(new StandaloneAhcWSClient(asyncHttpClient, materializer, objectMapper), materializer);
public AhcWSClientProvider(AsyncHttpClient asyncHttpClient, Materializer materializer) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The object mapper is passed in through WSBodyReadables / WSBodyWritables

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return converter.apply(request.addCookie(cookie));
}

@Override
public StandaloneWSRequest addCookies(WSCookie... cookies) {
public WSRequest addCookie(Http.Cookie cookie) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added support of Play's Http.Cookie here

*
* @param logger an SLF4J logger
*/
class AhcCurlRequestLogger(logger: org.slf4j.Logger) extends WSRequestFilter with CurlFormat {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this into play-ws


sequential

"WS@java" should {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broke out WSSpec into JavaWSSpec and ScalaWSSpec

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

import scala.concurrent.duration._
import scala.concurrent.{ Await, Future }

"Web service client" title
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broke out WSSpec into JavaWSSpec and ScalaWSSpec

*/
public interface WSResponse extends StandaloneWSResponse {
// Currently, there's nothing that is Play specific here.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List out the full interface so people don't have to go back to the server and we can have stronger guarantees between projects

*
* @param headers the headers to be used
*/
@deprecated("Use withHttpHeaders or addHttpHeaders", "2.6.0")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List out the methods explicitly from play-ws standalone so it's clear what the API is

`WS` lets you consume the response's body incrementally by using an Akka Streams `Sink`. The `stream()` method on `WSRequest` returns a `CompletionStage<StreamedResponse>`. A `StreamedResponse` is a simple container holding together the response's headers and body.
You can consume the response's body incrementally by using an [Akka Streams](http://doc.akka.io/docs/akka/current/java/stream/stream-flows-and-basics.html) `Sink`. The [`stream()`](api/java/play/libs/ws/WSRequest.html#stream--) method on `WSRequest` returns a `CompletionStage<WSResponse>`, where the `WSResponse` contains a [`getBodyAsStream()`](api/java/play/libs/ws/WSResponse.html#getBodyAsStream--) method that provides a `Source<ByteString, ?>`.

> **Note**: In 2.5.x, a `StreamedResponse` was returned in response to a [`request.stream()`](api/java/play/libs/ws/WSRequest.html#stream--) call. In 2.6.x, a standard [`WSResponse`](api/java/play/libs/ws/WSResponse.html) is returned, and the [`getBodyAsStream()`](api/java/play/libs/ws/WSResponse.html#getBodyAsStream--) method should be used to return the Source.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getBodyAsStream is actually returning a InputStream, not a Source.

public interface URLBodyReadables {
default BodyReadable<java.net.URL> url() {
return response -> {
play.shaded.ahc.org.asynchttpclient.Response ahcResponse =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid dealing with the shaded version here by just using response.getBody() and then parsing it as needed.

Copy link
Member

@richdougherty richdougherty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow lots of work.

  • Is all this stuff in the standalone play-ws project too?
  • It's interesting how the WS BodyReadable/BodyWritable concepts overlap with BodyParser/RequestBody/etc concepts in Play. I don't think you need to change anything in this
    PR—a lot of Play's design is already locked in so it can't be changed—but from an aesthetic point of view it would be nice if we had more unification between Play and Play WS.

### Request with timeout

If you wish to specify a request timeout, you can use `setRequestTimeout` to set a value in milliseconds. A value of `-1` can be used to set an infinite timeout.
If you wish to specify a request timeout, you can use `setRequestTimeout` to set a value in milliseconds. A value of `Duration.ofMillis(Long.MAX_VALUE)` can be used to set an infinite timeout.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

infinite or practically infinite? ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Java Duration API is sadly limited -- I looked at a Duration.Inf but they don't have it. :-(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -222,7 +228,7 @@ When making a request from a controller, you can map the response to a `Future[R

### Using WSClient with unreliable networks

If you are calling out to an [unreliable network](https://queue.acm.org/detail.cfm?id=2655736) or doing any blocking work, including any kind of DNS work such as calling [`java.util.URL.equals()`](https://docs.oracle.com/javase/8/docs/api/java/net/URL.html#equals-java.lang.Object-), then you should use a custom execution context as described in [[ThreadPools]], preferably through [`play.api.libs.concurrent.CustomExecutionContext`](api/scala/play/api/libs/concurrent/CustomExecutionContext.html). You should size the pool to leave a safety margin large enough to account for futures, and consider using [`play.api.libs.concurrent.Futures`](api/scala/play/api/libs/concurrent/Futures.html) and a [Failsafe Circuit Breaker](https://github.com/jhalterman/failsafe#circuit-breakers).
If you are doing any blocking work, including any kind of DNS work such as calling [`java.util.URL.equals()`](https://docs.oracle.com/javase/8/docs/api/java/net/URL.html#equals-java.lang.Object-), then you should use a custom execution context as described in [[ThreadPools]], preferably through [`play.api.libs.concurrent.CustomExecutionContext`](api/scala/play/api/libs/concurrent/CustomExecutionContext.html). You should size the pool to leave a safety margin large enough to account for futures, and consider using [`play.api.libs.concurrent.Futures`](api/scala/play/api/libs/concurrent/Futures.html) and a [Failsafe Circuit Breaker](https://github.com/jhalterman/failsafe#circuit-breakers).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you've written is good. The BlockContext can prevent deadlock, but it doesn't prevent heavily degraded performance if you get your thread pools wrong in the first place.

@@ -38,8 +37,8 @@
private final AhcWSClient client;

@Inject
public AhcWSClientProvider(AsyncHttpClient asyncHttpClient, Materializer materializer, ObjectMapper objectMapper) {
client = new AhcWSClient(new StandaloneAhcWSClient(asyncHttpClient, materializer, objectMapper), materializer);
public AhcWSClientProvider(AsyncHttpClient asyncHttpClient, Materializer materializer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


import java.io.File;
import java.io.InputStream;
import java.lang.annotation.Documented;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: unused import?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️

@@ -254,9 +323,20 @@ public WSRequest setVirtualHost(String virtualHost) {
return converter.apply(request.setVirtualHost(virtualHost));
}

/**
* @deprecated Use {@link #setRequestTimeout(Duration timeout)}
* @param timeout the request timeout in milliseconds. A value of -1 indicates an infinite request timeout.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this is a deprecated method, so I guess it's behaviour is fixed, but isn't infinity signalled by MAX_LONG instead of -1 in another place in the API?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duration.of(MAX_LONG) for duration vs -1 for Int/Long

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@richdougherty -1 is for legacy reasons.


sequential

"WS@java" should {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@wsargent wsargent merged commit b7ee394 into playframework:master Jun 19, 2017
@wsargent wsargent deleted the refactor-for-typed-ws-body branch June 19, 2017 23:26
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

Successfully merging this pull request may close these issues.

None yet

3 participants