Modified WSRequest.withRequestTimeout to take Duration as a parameter. …

…Fixes #4846

documentation/manual/working/javaGuide/main/ws/JavaWS.md

@@ -59,9 +59,9 @@ 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 time out
+### Request with timeout
 
-If you wish to specify a request timeout, you can use `setTimeout` to set a value in milliseconds.
+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. 
 
 @[ws-timeout](code/javaguide/ws/JavaWS.java)
 

documentation/manual/working/scalaGuide/main/ws/ScalaWS.md

@@ -71,7 +71,7 @@ A virtual host can be specified as a string.
 
 ### Request with timeout
 
-If you wish to specify a request timeout, you can use `withRequestTimeout` to set a value in milliseconds.
+If you wish to specify a request timeout, you can use `withRequestTimeout` to set a value. An infinite timeout can be set by passing `Duration.Inf`.
 
 @[request-timeout](code/ScalaWSSpec.scala)
 

documentation/manual/working/scalaGuide/main/ws/code/ScalaWSSpec.scala

@@ -13,6 +13,7 @@ import org.specs2.runner.JUnitRunner
 //#dependency
 import javax.inject.Inject
 import scala.concurrent.Future
+import scala.concurrent.duration._
 
 import play.api.mvc._
 import play.api.libs.ws._
@@ -77,7 +78,7 @@ class ScalaWSSpec extends PlaySpecification with Results {
       //#complex-holder
       val complexRequest: WSRequest =
         request.withHeaders("Accept" -> "application/json")
-          .withRequestTimeout(10000)
+          .withRequestTimeout(10000.millis)
           .withQueryString("search" -> "play")
       //#complex-holder
 
@@ -149,7 +150,7 @@ class ScalaWSSpec extends PlaySpecification with Results {
     "allow setting the request timeout" in withSimpleServer { ws =>
       val response =
         //#request-timeout
-        ws.url(url).withRequestTimeout(5000).get()
+        ws.url(url).withRequestTimeout(5000.millis).get()
         //#request-timeout
 
       await(response).status must_== 200

framework/src/play-integration-test/src/test/scala/play/it/http/FormFieldOrderSpec.scala

@@ -3,6 +3,8 @@
  */
 package play.it.http
 
+import scala.concurrent.duration._
+
 import play.api.mvc._
 import play.api.test._
 import play.api.libs.ws._
@@ -47,7 +49,7 @@ trait FormFieldOrderSpec extends PlaySpecification with ServerIntegrationSpecifi
 
       val future: Future[WSResponse] = WS.url("http://localhost:" + port + "/").
         withHeaders("Content-Type" -> contentType).
-        withRequestTimeout(10000).post(urlEncoded)
+        withRequestTimeout(10000.millis).post(urlEncoded)
 
       val response = await(future)
       response.status must equalTo(OK)

framework/src/play-java-ws/src/main/java/play/libs/ws/WSRequest.java

@@ -250,7 +250,7 @@ public interface WSRequest {
     /**
      * Sets the request timeout in milliseconds.
      *
-     * @param timeout the request timeout in milliseconds.
+     * @param timeout the request timeout in milliseconds. A value of -1 indicates an infinite request timeout.
      * @return the modified WSRequest.
      */
     WSRequest setRequestTimeout(long timeout);

framework/src/play-java-ws/src/main/java/play/libs/ws/ning/NingWSRequest.java

@@ -175,8 +175,8 @@ public WSRequest setVirtualHost(String virtualHost) {
 
     @Override
     public WSRequest setRequestTimeout(long timeout) {
-        if (timeout < 0 || timeout > Integer.MAX_VALUE) {
-            throw new IllegalArgumentException("Timeout must be between 0 and " + Integer.MAX_VALUE + " inclusive");
+        if (timeout < -1 || timeout > Integer.MAX_VALUE) {
+            throw new IllegalArgumentException("Timeout must be between -1 and " + Integer.MAX_VALUE + " inclusive");
         }
         this.timeout = (int) timeout;
         return this;
@@ -474,7 +474,7 @@ Request buildRequest() {
             throw new IllegalStateException("Impossible body: " + body);
         }
 
-        if (this.timeout > 0) {
+        if (this.timeout == -1 || this.timeout > 0) {
             builder.setRequestTimeout(this.timeout);
         }
 

framework/src/play-java-ws/src/test/scala/play/libs/ws/ning/NingWSRequestSpec.scala

@@ -21,6 +21,27 @@ class NingWSRequestSpec extends Specification with Mockito {
       actual must beEqualTo("foo.com")
     }
 
+    "should support setting a request timeout" in {
+      requestWithTimeout(1000) must beEqualTo(1000)
+    }
+
+    "should support setting an infinite request timeout" in {
+      requestWithTimeout(-1) must beEqualTo(-1)
+    }
+
+    "should not support setting a request timeout < -1" in {
+      requestWithTimeout(-2) must throwA[IllegalArgumentException]
+    }
+
+    "should not support setting a request timeout > Integer.MAX_VALUE" in {
+      requestWithTimeout(Int.MaxValue.toLong + 1) must throwA[IllegalArgumentException]
+    }
   }
 
+  def requestWithTimeout(timeout: Long) = {
+    val client = mock[NingWSClient]
+    val request = new NingWSRequest(client, "http://example.com")
+    request.setRequestTimeout(timeout)
+    request.buildRequest().getRequestTimeout()
+  }
 }

framework/src/play-ws/src/main/scala/play/api/libs/ws/WS.scala

@@ -6,6 +6,7 @@ package play.api.libs.ws
 import java.net.URI
 
 import scala.concurrent.{ Future, ExecutionContext }
+import scala.concurrent.duration.Duration
 
 import java.io.File
 
@@ -363,10 +364,11 @@ trait WSRequest {
   def withFollowRedirects(follow: Boolean): WSRequest
 
   /**
-   * Sets the maximum time in milliseconds you expect the request to take.
-   * Warning: a stream consumption will be interrupted when this time is reached.
+   * Sets the maximum time you expect the request to take.
+   * Use Duration.Inf to set an infinite request timeout.
+   * Warning: a stream consumption will be interrupted when this time is reached unless Duration.Inf is set.
    */
-  def withRequestTimeout(timeout: Long): WSRequest
+  def withRequestTimeout(timeout: Duration): WSRequest
 
   /**
    * Sets the virtual host to use in this request

framework/src/play-ws/src/main/scala/play/api/libs/ws/ning/NingWS.scala

@@ -18,6 +18,7 @@ import play.core.parsers.FormUrlEncodedParser
 import collection.immutable.TreeMap
 
 import scala.concurrent.{ Future, Promise }
+import scala.concurrent.duration.Duration
 
 import play.api.libs.ws._
 import play.api.libs.ws.ssl._
@@ -112,9 +113,16 @@ case class NingWSRequest(client: NingWSClient,
 
   def withFollowRedirects(follow: Boolean): WSRequest = copy(followRedirects = Some(follow))
 
-  def withRequestTimeout(timeout: Long): WSRequest = {
-    require(timeout >= 0 && timeout <= Int.MaxValue, s"Request timeout must be between 0 and ${Int.MaxValue}")
-    copy(requestTimeout = Some(timeout.toInt))
+  def withRequestTimeout(timeout: Duration): WSRequest = {
+    timeout match {
+      case Duration.Inf =>
+        copy(requestTimeout = Some(-1))
+      case d => {
+        val millis = d.toMillis
+        require(millis >= 0 && millis <= Int.MaxValue, s"Request timeout must be between 0 and ${Int.MaxValue} milliseconds")
+        copy(requestTimeout = Some(millis.toInt))
+      }
+    }
   }
 
   def withVirtualHost(vh: String): WSRequest = copy(virtualHost = Some(vh))

framework/src/play-ws/src/test/scala/play/api/libs/ws/ning/NingWSSpec.scala

@@ -8,6 +8,7 @@ import com.ning.http.client
 import com.ning.http.client.cookie.{ Cookie => AHCCookie }
 import com.ning.http.client.{ AsyncHttpClient, FluentCaseInsensitiveStringsMap, Param, Response => AHCResponse }
 import org.specs2.mock.Mockito
+import scala.concurrent.duration._
 
 import play.api.mvc._
 
@@ -165,15 +166,26 @@ object NingWSSpec extends PlaySpecification with Mockito {
       req.getFollowRedirect must beEqualTo(true)
     }
 
-    "support timeout" in new WithApplication {
+    "support finite timeout" in new WithApplication {
       val req: client.Request = WS.url("http://playframework.com/")
-        .withRequestTimeout(1000).asInstanceOf[NingWSRequest]
+        .withRequestTimeout(1000.millis).asInstanceOf[NingWSRequest]
         .buildRequest()
       req.getRequestTimeout must be equalTo 1000
     }
 
-    "not support invalid timeout" in new WithApplication {
-      WS.url("http://playframework.com/").withRequestTimeout(-1) should throwAn[IllegalArgumentException]
+    "support infinite timeout" in new WithApplication {
+      val req: client.Request = WS.url("http://playframework.com/")
+        .withRequestTimeout(Duration.Inf).asInstanceOf[NingWSRequest]
+        .buildRequest()
+      req.getRequestTimeout must be equalTo -1
+    }
+
+    "not support negative timeout" in new WithApplication {
+      WS.url("http://playframework.com/").withRequestTimeout(-1.millis) should throwAn[IllegalArgumentException]
+    }
+
+    "not support a timeout greater than Int.MaxValue" in new WithApplication {
+      WS.url("http://playframework.com/").withRequestTimeout((Int.MaxValue.toLong + 1).millis) should throwAn[IllegalArgumentException]
     }
 
     "support a proxy server" in new WithApplication {