Post review changes

14 files changed, 85 insertions, 73 deletions

diff --git a/README.md b/README.md
index 3c615db..53a8567 100644
--- a/README.md
+++ b/README.md
@@ -17,7 +17,7 @@ val query = "http language:scala"
 // `sort` is removed, as the value is not defined
 val request = sttp.get(uri"https://api.github.com/search/repositories?q=$query&sort=$sort")
   
-implicit val handler = HttpURLConnectionHandler
+implicit val handler = HttpURLConnectionHandler()
 val response = request.send()
 
 // response.header(...): Option[String]
@@ -61,7 +61,7 @@ experimenting with sttp by copy-pasting the following:
 ```scala
 import $ivy.`com.softwaremill.sttp::core:0.0.11`
 import com.softwaremill.sttp._
-implicit val handler = HttpURLConnectionHandler
+implicit val handler = HttpURLConnectionHandler()
 sttp.get(uri"http://httpbin.org/ip").send()
 ```
 
@@ -113,7 +113,7 @@ value of type `SttpHandler` needs to be in scope to invoke the `send()` on the
 request: 
 
 ```scala
-implicit val handler = HttpURLConnectionHandler
+implicit val handler = HttpURLConnectionHandler()
 
 val response: Response[String] = request.send()
 ```
@@ -214,7 +214,7 @@ in the identity type constructor, which is equivalent to no wrapper at all.
 To use, add an implicit value:
 
 ```scala
-implicit val sttpHandler = HttpURLConnectionHandler
+implicit val sttpHandler = HttpURLConnectionHandler()
 ```
 
 ### `AkkaHttpHandler`
@@ -409,7 +409,7 @@ a request to decode the response to a specific object.
 import com.softwaremill.sttp._
 import com.softwaremill.sttp.circe._
 
-implicit val handler = HttpURLConnectionHandler
+implicit val handler = HttpURLConnectionHandler()
 
 // Assume that there is an implicit circe encoder in scope
 // for the request Payload, and a decoder for the Response
@@ -454,6 +454,26 @@ There are two type aliases for the request template that are used:
 * `type Request[T, S] = RequestT[Id, T, S]`. A sendable request.
 * `type PartialRequest[T, S] = RequestT[Empty, T, S]`
 
+## Timeouts
+
+Sttp supports read and connection timeouts: 
+ * Connection timeout - can be set globally (30 seconds by default)
+ * Read timeout - can be set per request (1 minute by default)
+
+How to use:
+```scala
+import com.softwaremill.sttp._
+import scala.concurrent.duration._
+
+// all backends provide a constructor that allows users to specify connection timeout
+implicit val handler = HttpURLConnectionHandler(connectionTimeout = 1.minute)
+
+sttp
+  .get(uri"...")
+  .readTimeout(5.minutes) // or Duration.Inf to turn read timeout off
+  .send()
+```
+
 ## Notes
 
 * the encoding for `String`s defaults to `utf-8`.
@@ -486,3 +506,4 @@ and pick a task you'd like to work on!
 * [Omar Alejandro Mainegra Sarduy](https://github.com/omainegra)
 * [Bjørn Madsen](https://github.com/aeons)
 * [Piotr Buda](https://github.com/pbuda)
+* [Piotr Gabara](https://github.com/bhop)
diff --git a/akka-http-handler/src/main/scala/com/softwaremill/sttp/akkahttp/AkkaHttpHandler.scala b/akka-http-handler/src/main/scala/com/softwaremill/sttp/akkahttp/AkkaHttpHandler.scala
index da538a1..c6b0a2c 100644
--- a/akka-http-handler/src/main/scala/com/softwaremill/sttp/akkahttp/AkkaHttpHandler.scala
+++ b/akka-http-handler/src/main/scala/com/softwaremill/sttp/akkahttp/AkkaHttpHandler.scala
@@ -37,21 +37,22 @@ class AkkaHttpHandler private (actorSystem: ActorSystem,
   private implicit val as: ActorSystem = actorSystem
   private implicit val materializer: ActorMaterializer = ActorMaterializer()
 
+  private val connectionSettings = ClientConnectionSettings(actorSystem)
+    .withConnectingTimeout(connectionTimeout)
+
+  private val connectionPoolSettings = ConnectionPoolSettings(actorSystem)
+
   override def send[T](r: Request[T, S]): Future[Response[T]] = {
     implicit val ec: ExecutionContext = this.ec
 
-    val connectionSettings = ClientConnectionSettings(actorSystem)
-      .withIdleTimeout(r.readTimeout)
-      .withConnectingTimeout(connectionTimeout)
-
-    val connectionPoolSettings = ConnectionPoolSettings(actorSystem)
-      .withConnectionSettings(connectionSettings)
+    val settings = connectionPoolSettings
+      .withConnectionSettings(
+        connectionSettings.withIdleTimeout(r.options.readTimeout))
 
     requestToAkka(r)
       .flatMap(setBodyOnAkka(r, r.body, _))
       .toFuture
-      .flatMap(req =>
-        Http().singleRequest(req, settings = connectionPoolSettings))
+      .flatMap(req => Http().singleRequest(req, settings = settings))
       .flatMap { hr =>
         val code = hr.status.intValue()
 
@@ -285,7 +286,8 @@ object AkkaHttpHandler {
   private def apply(actorSystem: ActorSystem,
                     ec: ExecutionContext,
                     terminateActorSystemOnClose: Boolean,
-                    connectionTimeout: FiniteDuration): SttpHandler[Future, Source[ByteString, Any]] =
+                    connectionTimeout: FiniteDuration)
+    : SttpHandler[Future, Source[ByteString, Any]] =
     new FollowRedirectsHandler(
       new AkkaHttpHandler(actorSystem,
                           ec,
@@ -297,7 +299,8 @@ object AkkaHttpHandler {
     *           e.g. mapping responses. Defaults to the global execution
     *           context.
     */
-  def apply(connectionTimeout: FiniteDuration = SttpHandler.DefaultConnectionTimeout)(
+  def apply(connectionTimeout: FiniteDuration =
+              SttpHandler.DefaultConnectionTimeout)(
       implicit ec: ExecutionContext = ExecutionContext.Implicits.global)
     : SttpHandler[Future, Source[ByteString, Any]] =
     AkkaHttpHandler(ActorSystem("sttp"),
@@ -313,7 +316,8 @@ object AkkaHttpHandler {
     *           context.
     */
   def usingActorSystem(actorSystem: ActorSystem,
-                       connectionTimeout: FiniteDuration = SttpHandler.DefaultConnectionTimeout)(
+                       connectionTimeout: FiniteDuration =
+                         SttpHandler.DefaultConnectionTimeout)(
       implicit ec: ExecutionContext = ExecutionContext.Implicits.global)
     : SttpHandler[Future, Source[ByteString, Any]] =
     AkkaHttpHandler(actorSystem,
diff --git a/async-http-client-handler/cats/src/main/scala/com/softwaremill/sttp/asynchttpclient/cats/AsyncHttpClientCatsHandler.scala b/async-http-client-handler/cats/src/main/scala/com/softwaremill/sttp/asynchttpclient/cats/AsyncHttpClientCatsHandler.scala
index 906f090..9948d42 100644
--- a/async-http-client-handler/cats/src/main/scala/com/softwaremill/sttp/asynchttpclient/cats/AsyncHttpClientCatsHandler.scala
+++ b/async-http-client-handler/cats/src/main/scala/com/softwaremill/sttp/asynchttpclient/cats/AsyncHttpClientCatsHandler.scala
@@ -50,8 +50,7 @@ object AsyncHttpClientCatsHandler {
   def apply[F[_]: Async](connectionTimeout: FiniteDuration = SttpHandler.DefaultConnectionTimeout)
     : SttpHandler[F, Nothing] =
     AsyncHttpClientCatsHandler(
-      new DefaultAsyncHttpClient(
-        AsyncHttpClientHandler.withConnectionTimeout(connectionTimeout)),
+      AsyncHttpClientHandler.defaultClient(connectionTimeout.toMillis.toInt),
       closeClient = true)
 
   def usingConfig[F[_]: Async](
diff --git a/async-http-client-handler/fs2/src/main/scala/com/softwaremill/sttp/asynchttpclient/fs2/AsyncHttpClientFs2Handler.scala b/async-http-client-handler/fs2/src/main/scala/com/softwaremill/sttp/asynchttpclient/fs2/AsyncHttpClientFs2Handler.scala
index 8d38f43..a33932b 100644
--- a/async-http-client-handler/fs2/src/main/scala/com/softwaremill/sttp/asynchttpclient/fs2/AsyncHttpClientFs2Handler.scala
+++ b/async-http-client-handler/fs2/src/main/scala/com/softwaremill/sttp/asynchttpclient/fs2/AsyncHttpClientFs2Handler.scala
@@ -68,8 +68,7 @@ object AsyncHttpClientFs2Handler {
       implicit ec: ExecutionContext = ExecutionContext.Implicits.global)
     : SttpHandler[F, Stream[F, ByteBuffer]] =
     AsyncHttpClientFs2Handler[F](
-      new DefaultAsyncHttpClient(
-        AsyncHttpClientHandler.withConnectionTimeout(connectionTimeout)),
+      AsyncHttpClientHandler.defaultClient(connectionTimeout.toMillis.toInt),
       closeClient = true)
 
   /**
diff --git a/async-http-client-handler/future/src/main/scala/com/softwaremill/sttp/asynchttpclient/future/AsyncHttpClientFutureHandler.scala b/async-http-client-handler/future/src/main/scala/com/softwaremill/sttp/asynchttpclient/future/AsyncHttpClientFutureHandler.scala
index bbcc9c2..6e0a22c 100644
--- a/async-http-client-handler/future/src/main/scala/com/softwaremill/sttp/asynchttpclient/future/AsyncHttpClientFutureHandler.scala
+++ b/async-http-client-handler/future/src/main/scala/com/softwaremill/sttp/asynchttpclient/future/AsyncHttpClientFutureHandler.scala
@@ -49,8 +49,7 @@ object AsyncHttpClientFutureHandler {
       implicit ec: ExecutionContext = ExecutionContext.Implicits.global)
     : SttpHandler[Future, Nothing] =
     AsyncHttpClientFutureHandler(
-      new DefaultAsyncHttpClient(
-        AsyncHttpClientHandler.withConnectionTimeout(connectionTimeout)),
+      AsyncHttpClientHandler.defaultClient(connectionTimeout.toMillis.toInt),
       closeClient = true)
 
   /**
diff --git a/async-http-client-handler/monix/src/main/scala/com/softwaremill/sttp/asynchttpclient/monix/AsyncHttpClientMonixHandler.scala b/async-http-client-handler/monix/src/main/scala/com/softwaremill/sttp/asynchttpclient/monix/AsyncHttpClientMonixHandler.scala
index 457726a..7cfcb43 100644
--- a/async-http-client-handler/monix/src/main/scala/com/softwaremill/sttp/asynchttpclient/monix/AsyncHttpClientMonixHandler.scala
+++ b/async-http-client-handler/monix/src/main/scala/com/softwaremill/sttp/asynchttpclient/monix/AsyncHttpClientMonixHandler.scala
@@ -66,8 +66,7 @@ object AsyncHttpClientMonixHandler {
       implicit s: Scheduler = Scheduler.Implicits.global)
     : SttpHandler[Task, Observable[ByteBuffer]] =
     AsyncHttpClientMonixHandler(
-      new DefaultAsyncHttpClient(
-        AsyncHttpClientHandler.withConnectionTimeout(connectionTimeout)),
+      AsyncHttpClientHandler.defaultClient(connectionTimeout.toMillis.toInt),
       closeClient = true)
 
   /**
diff --git a/async-http-client-handler/scalaz/src/main/scala/com/softwaremill/sttp/asynchttpclient/scalaz/AsyncHttpClientScalazHandler.scala b/async-http-client-handler/scalaz/src/main/scala/com/softwaremill/sttp/asynchttpclient/scalaz/AsyncHttpClientScalazHandler.scala
index 88555fd..0e80a29 100644
--- a/async-http-client-handler/scalaz/src/main/scala/com/softwaremill/sttp/asynchttpclient/scalaz/AsyncHttpClientScalazHandler.scala
+++ b/async-http-client-handler/scalaz/src/main/scala/com/softwaremill/sttp/asynchttpclient/scalaz/AsyncHttpClientScalazHandler.scala
@@ -46,8 +46,7 @@ object AsyncHttpClientScalazHandler {
   def apply(connectionTimeout: FiniteDuration = SttpHandler.DefaultConnectionTimeout)
     : SttpHandler[Task, Nothing] =
     AsyncHttpClientScalazHandler(
-      new DefaultAsyncHttpClient(
-        AsyncHttpClientHandler.withConnectionTimeout(connectionTimeout)),
+      AsyncHttpClientHandler.defaultClient(connectionTimeout.toMillis.toInt),
       closeClient = true)
 
   def usingConfig(cfg: AsyncHttpClientConfig): SttpHandler[Task, Nothing] =
diff --git a/async-http-client-handler/src/main/scala/com/softwaremill/sttp/asynchttpclient/AsyncHttpClientHandler.scala b/async-http-client-handler/src/main/scala/com/softwaremill/sttp/asynchttpclient/AsyncHttpClientHandler.scala
index 8467399..984ecf6 100644
--- a/async-http-client-handler/src/main/scala/com/softwaremill/sttp/asynchttpclient/AsyncHttpClientHandler.scala
+++ b/async-http-client-handler/src/main/scala/com/softwaremill/sttp/asynchttpclient/AsyncHttpClientHandler.scala
@@ -18,6 +18,7 @@ import org.asynchttpclient.{
   AsyncHandler,
   AsyncHttpClient,
   DefaultAsyncHttpClientConfig,
+  DefaultAsyncHttpClient,
   HttpResponseBodyPart,
   HttpResponseHeaders,
   HttpResponseStatus,
@@ -29,7 +30,6 @@ import org.asynchttpclient.{
 import org.reactivestreams.{Publisher, Subscriber, Subscription}
 
 import scala.collection.JavaConverters._
-import scala.concurrent.duration.FiniteDuration
 import scala.language.higherKinds
 import scala.util.{Failure, Try}
 
@@ -157,10 +157,11 @@ abstract class AsyncHttpClientHandler[R[_], S](asyncHttpClient: AsyncHttpClient,
   }
 
   private def requestToAsync(r: Request[_, S]): AsyncRequest = {
+    val readTimeout = r.options.readTimeout
     val rb = new RequestBuilder(r.method.m)
       .setUrl(r.uri.toString)
       .setRequestTimeout(
-        if (r.readTimeout.isFinite()) r.readTimeout.toMillis.toInt else -1)
+        if (readTimeout.isFinite()) readTimeout.toMillis.toInt else -1)
     r.headers.foreach { case (k, v) => rb.setHeader(k, v) }
     setBody(r, r.body, rb)
     rb.build()
@@ -296,11 +297,12 @@ abstract class AsyncHttpClientHandler[R[_], S](asyncHttpClient: AsyncHttpClient,
 
 object AsyncHttpClientHandler {
 
-  private[asynchttpclient] def withConnectionTimeout(t: FiniteDuration) = {
-    new DefaultAsyncHttpClientConfig.Builder()
-      .setConnectTimeout(t.toMillis.toInt)
-      .build()
-  }
+  private[asynchttpclient] def defaultClient(connectionTimeout: Int): AsyncHttpClient =
+    new DefaultAsyncHttpClient(
+      new DefaultAsyncHttpClientConfig.Builder()
+        .setConnectTimeout(connectionTimeout)
+        .build()
+    )
 }
 
 object EmptyPublisher extends Publisher[ByteBuffer] {
diff --git a/core/src/main/scala/com/softwaremill/sttp/HttpURLConnectionHandler.scala b/core/src/main/scala/com/softwaremill/sttp/HttpURLConnectionHandler.scala
index 24c81c7..9b73298 100644
--- a/core/src/main/scala/com/softwaremill/sttp/HttpURLConnectionHandler.scala
+++ b/core/src/main/scala/com/softwaremill/sttp/HttpURLConnectionHandler.scala
@@ -21,7 +21,7 @@ class HttpURLConnectionHandler private (connectionTimeout: FiniteDuration)
     c.setRequestMethod(r.method.m)
     r.headers.foreach { case (k, v) => c.setRequestProperty(k, v) }
     c.setDoInput(true)
-    c.setReadTimeout(timeout(r.readTimeout))
+    c.setReadTimeout(timeout(r.options.readTimeout))
     c.setConnectTimeout(timeout(connectionTimeout))
 
     // redirects are handled in SttpHandler
diff --git a/core/src/main/scala/com/softwaremill/sttp/RequestT.scala b/core/src/main/scala/com/softwaremill/sttp/RequestT.scala
index b9b49f4..e4fd421 100644
--- a/core/src/main/scala/com/softwaremill/sttp/RequestT.scala
+++ b/core/src/main/scala/com/softwaremill/sttp/RequestT.scala
@@ -34,7 +34,6 @@ case class RequestT[U[_], T, +S](
     body: RequestBody[S],
     headers: Seq[(String, String)],
     response: ResponseAs[T, S],
-    readTimeout: Duration,
     options: RequestOptions,
     tags: Map[String, Any]
 ) {
@@ -218,7 +217,7 @@ case class RequestT[U[_], T, +S](
     copy[U, T, S2](body = StreamBody(b))
 
   def readTimeout(t: Duration): RequestT[U, T, S] =
-    copy(readTimeout = t)
+    this.copy(options = options.copy(readTimeout = t))
 
   def response[T2, S2 >: S](ra: ResponseAs[T2, S2]): RequestT[U, T2, S2] =
     this.copy(response = ra)
@@ -280,4 +279,4 @@ class SpecifyAuthScheme[U[_], T, +S](hn: String, rt: RequestT[U, T, S]) {
     rt.header(hn, s"Bearer $token")
 }
 
-case class RequestOptions(followRedirects: Boolean)
+case class RequestOptions(followRedirects: Boolean, readTimeout: Duration)
diff --git a/core/src/main/scala/com/softwaremill/sttp/package.scala b/core/src/main/scala/com/softwaremill/sttp/package.scala
index 4ec1331..a9950be 100644
--- a/core/src/main/scala/com/softwaremill/sttp/package.scala
+++ b/core/src/main/scala/com/softwaremill/sttp/package.scala
@@ -58,14 +58,14 @@ package object sttp {
     * An empty request with no headers.
     */
   val emptyRequest: RequestT[Empty, String, Nothing] =
-    RequestT[Empty, String, Nothing](None,
-                                     None,
-                                     NoBody,
-                                     Vector(),
-                                     asString,
-                                     DefaultReadTimeout,
-                                     RequestOptions(followRedirects = true),
-                                     Map())
+    RequestT[Empty, String, Nothing](
+      None,
+      None,
+      NoBody,
+      Vector(),
+      asString,
+      RequestOptions(followRedirects = true, readTimeout = DefaultReadTimeout),
+      Map())
 
   /**
     * A starting request, with the following modifications comparing to
diff --git a/core/src/test/scala/com/softwaremill/sttp/RequestTests.scala b/core/src/test/scala/com/softwaremill/sttp/RequestTests.scala
index 6332467..e62112a 100644
--- a/core/src/test/scala/com/softwaremill/sttp/RequestTests.scala
+++ b/core/src/test/scala/com/softwaremill/sttp/RequestTests.scala
@@ -68,6 +68,6 @@ class RequestTests extends FlatSpec with Matchers {
   }
 
   "request timeout" should "use default if not overridden" in {
-    sttp.readTimeout should be(DefaultReadTimeout)
+    sttp.options.readTimeout should be(DefaultReadTimeout)
   }
 }
diff --git a/okhttp-handler/monix/src/main/scala/com/softwaremill/sttp/okhttp/monix/OkHttpMonixHandler.scala b/okhttp-handler/monix/src/main/scala/com/softwaremill/sttp/okhttp/monix/OkHttpMonixHandler.scala
index 608d499..8d20bad 100644
--- a/okhttp-handler/monix/src/main/scala/com/softwaremill/sttp/okhttp/monix/OkHttpMonixHandler.scala
+++ b/okhttp-handler/monix/src/main/scala/com/softwaremill/sttp/okhttp/monix/OkHttpMonixHandler.scala
@@ -1,7 +1,7 @@
 package com.softwaremill.sttp.okhttp.monix
 
 import java.nio.ByteBuffer
-import java.util.concurrent.{ArrayBlockingQueue, TimeUnit}
+import java.util.concurrent.ArrayBlockingQueue
 
 import com.softwaremill.sttp.{SttpHandler, _}
 import com.softwaremill.sttp.okhttp.{OkHttpAsyncHandler, OkHttpHandler}
@@ -89,13 +89,8 @@ object OkHttpMonixHandler {
       implicit s: Scheduler = Scheduler.Implicits.global)
     : SttpHandler[Task, Observable[ByteBuffer]] =
     OkHttpMonixHandler(
-      OkHttpHandler
-        .defaultBuilder()
-        .connectTimeout(connectionTimeout.toMillis, TimeUnit.MILLISECONDS)
-        .readTimeout(SttpHandler.DefaultConnectionTimeout.toMillis, TimeUnit.MILLISECONDS)
-        .build(),
-      closeClient = true
-    )(s)
+      OkHttpHandler.defaultClient(DefaultReadTimeout.toMillis, connectionTimeout.toMillis),
+      closeClient = true)(s)
 
   def usingClient(client: OkHttpClient)(implicit s: Scheduler =
                                           Scheduler.Implicits.global)
diff --git a/okhttp-handler/src/main/scala/com/softwaremill/sttp/okhttp/OkHttpClientHandler.scala b/okhttp-handler/src/main/scala/com/softwaremill/sttp/okhttp/OkHttpClientHandler.scala
index d670b62..3e57930 100644
--- a/okhttp-handler/src/main/scala/com/softwaremill/sttp/okhttp/OkHttpClientHandler.scala
+++ b/okhttp-handler/src/main/scala/com/softwaremill/sttp/okhttp/OkHttpClientHandler.scala
@@ -144,20 +144,24 @@ abstract class OkHttpHandler[R[_], S](client: OkHttpClient,
 }
 
 object OkHttpHandler {
-  def defaultBuilder(): OkHttpClient.Builder =
+
+  private[okhttp] def defaultClient(readTimeout: Long,
+                                    connectionTimeout: Long): OkHttpClient =
     new OkHttpClient.Builder()
       .followRedirects(false)
       .followSslRedirects(false)
-
-  def updateClientIfCustomReadTimeout[T, S](
-      r: Request[T, S],
-      client: OkHttpClient): OkHttpClient = {
-    if (r.readTimeout == SttpHandler.DefaultConnectionTimeout) client
+      .connectTimeout(connectionTimeout, TimeUnit.MILLISECONDS)
+      .readTimeout(readTimeout, TimeUnit.MILLISECONDS)
+      .build()
+
+  private[okhttp] def updateClientIfCustomReadTimeout[T, S](r: Request[T, S],
+                                                            client: OkHttpClient): OkHttpClient = {
+    val readTimeout = r.options.readTimeout
+    if (readTimeout == DefaultReadTimeout) client
     else
       client
         .newBuilder()
-        .readTimeout(if (r.readTimeout.isFinite()) r.readTimeout.toMillis
-                     else 0,
+        .readTimeout(if (readTimeout.isFinite()) readTimeout.toMillis else 0,
                      TimeUnit.MILLISECONDS)
         .build()
 
@@ -188,10 +192,7 @@ object OkHttpSyncHandler {
       connectionTimeout: FiniteDuration = SttpHandler.DefaultConnectionTimeout)
     : SttpHandler[Id, Nothing] =
     OkHttpSyncHandler(
-      OkHttpHandler
-        .defaultBuilder()
-        .connectTimeout(connectionTimeout.toMillis, TimeUnit.MILLISECONDS)
-        .build(),
+      OkHttpHandler.defaultClient(DefaultReadTimeout.toMillis, connectionTimeout.toMillis),
       closeClient = true)
 
   def usingClient(client: OkHttpClient): SttpHandler[Id, Nothing] =
@@ -242,13 +243,8 @@ object OkHttpFutureHandler {
       implicit ec: ExecutionContext = ExecutionContext.Implicits.global)
     : SttpHandler[Future, Nothing] =
     OkHttpFutureHandler(
-      OkHttpHandler
-        .defaultBuilder()
-        .connectTimeout(connectionTimeout.toMillis, TimeUnit.MILLISECONDS)
-        .readTimeout(SttpHandler.DefaultConnectionTimeout.toMillis, TimeUnit.MILLISECONDS)
-        .build(),
-      closeClient = true
-    )
+      OkHttpHandler.defaultClient(DefaultReadTimeout.toMillis, connectionTimeout.toMillis),
+      closeClient = true)
 
   def usingClient(client: OkHttpClient)(implicit ec: ExecutionContext =
                                           ExecutionContext.Implicits.global)