BDOG-1421 Adds withProxy and withUserAgent to HttpClient

CHANGELOG.md

@@ -1,3 +1,52 @@
+
+## Version 13.9.0
+
+| Change | Complexity | Fix  |
+|--------------------------------------------------------------|------------|-----------------------------------------------|
+| WSProxy changes  | Minor  | Optional change |
+| `withUserAgent` added | Minor | Optional change |
+| `withTransformRequest` added | Minor | Optional change |
+
+For the following changes, it is expected that you will be using the `uk.gov.hmrc.PlayHttpClient` implementation of `HttpClient` (which should be provided by bootstrap-play). They may not be supported on custom implementations of `HttpClient`.
+
+### WSProxy
+
+`WSProxy` has been deprecated, the behaviour is available on `HttpClient`.
+
+What this means:
+  * You will **not** require two HttpClient implementations to use a proxy. Instead you can call `withProxy` on the same single HttpClient.
+  * It will still need enabling with `proxy.enabled` in configuration, which by default is false, for development. See `WSProxyConfiguration` for configuration changes below.
+
+```scala
+// without proxy
+httpClient.GET(url)
+
+// with proxy (and `proxy.enabled=true` in configuration)
+httpClient.withProxy.GET(url)
+```
+
+`WSProxyConfiguration.apply` has been deprecated, use `WSProxyConfiguration.buildWsProxyServer` instead.
+
+There are some differences with `WSProxyConfiguration.buildWsProxyServer` (which is used by `httpClient.withProxy`):
+  * configPrefix is fixed to `proxy`.
+  * `proxy.proxyRequiredForThisEnvironment` has been replaced with `proxy.enabled`, but note, it defaults to false (rather than true). This is appropriate for development and tests, but will need explicitly enabling when deployed.
+
+### withUserAgent
+
+The useragent defaults to `appName` from configuration. This can be overridden for a single endpoint with the `withUserAgent` function.
+
+```scala
+httpClient.withUserAgent("new-user-agent").GET(url)
+```
+
+### withTransformRequest
+
+You can modify a request before it is executed with `withTransformRequest`. It is preferrable to use this rather than extending HttpClient to override `def buildRequest(url: String, headers: Seq[(String, String)]): play.api.libs.ws.WSRequest`.
+
+```scala
+httpClient.withTransformRequest(_.withRequestTimeout(60.seconds)).GET(url)
+```
+
 ## Version 13.0.0
 
 | Change | Complexity | Fix  |

http-verbs-common/src/main/resources/reference.conf

@@ -18,3 +18,5 @@ http-verbs.retries {
   intervals = [ "500.millis", "1.second", "2.seconds", "4.seconds", "8.seconds" ]
   ssl-engine-closed-already.enabled = false
 }
+
+proxy.enabled = false

http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpClient.scala

@@ -16,4 +16,17 @@
 
 package uk.gov.hmrc.http
 
-trait HttpClient extends HttpGet with HttpPut with HttpPost with HttpDelete with HttpPatch
+import play.api.libs.ws.{WSRequest => PlayWSRequest}
+
+trait HttpClient extends HttpGet with HttpPut with HttpPost with HttpDelete with HttpPatch {
+  // implementations provided to not break clients (which won't be using the new functions)
+  // e.g. implementations of ProxyHttpClient, and the deprecated DefaultHttpClient in bootstrap.
+  def withUserAgent(userAgent: String): HttpClient =
+    sys.error("Not implemented by your implementation of HttpClient. You can use uk.gov.hmrc.http.PlayHttpClient")
+
+  def withProxy: HttpClient =
+    sys.error("Not implemented by your implementation of HttpClient. You can use uk.gov.hmrc.http.PlayHttpClient")
+
+  def withTransformRequest(transform: PlayWSRequest => PlayWSRequest): HttpClient =
+    sys.error("Your implementation of HttpClient does not implement `withTransformRequest`. You can use uk.gov.hmrc.http.PlayHttpClient")
+}

http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/PlayHttpClient.scala

@@ -0,0 +1,63 @@
+/*
+ * Copyright 2021 HM Revenue & Customs
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package uk.gov.hmrc.play.http
+
+import akka.actor.ActorSystem
+import com.typesafe.config.Config
+import play.api.libs.ws.{WSClient, WSRequest => PlayWSRequest}
+import uk.gov.hmrc.http.HttpClient
+import uk.gov.hmrc.http.hooks.HttpHook
+import uk.gov.hmrc.play.http.ws._
+
+// class is final, since any overrides would be lost in the result of `withPlayWSRequest`
+final class PlayHttpClient (
+  override val configuration   : Config,
+  override val hooks           : Seq[HttpHook],
+  override val wsClient        : WSClient,
+  override val actorSystem     : ActorSystem,
+  override val transformRequest: PlayWSRequest => PlayWSRequest
+) extends HttpClient
+     with WSHttp {
+
+  override def withTransformRequest(transformRequest: PlayWSRequest => PlayWSRequest): PlayHttpClient =
+    new PlayHttpClient(
+      this.configuration,
+      this.hooks,
+      this.wsClient,
+      this.actorSystem,
+      this.transformRequest.andThen(transformRequest)
+    )
+
+  private def replaceHeader(req: PlayWSRequest, header: (String, String)): PlayWSRequest = {
+    def denormalise(hdrs: Map[String, Seq[String]]): Seq[(String, String)] =
+      hdrs.toList.flatMap { case (k, vs) => vs.map(k -> _) }
+    val hdrsWithoutKey = req.headers.filterKeys(!_.equalsIgnoreCase(header._1)) // replace existing header
+    req.withHttpHeaders(denormalise(hdrsWithoutKey) :+ header : _*)
+  }
+
+  override def withUserAgent(userAgent: String): HttpClient =
+    withTransformRequest { req =>
+      replaceHeader(req, "User-Agent" -> userAgent)
+    }
+
+  override def withProxy: HttpClient = {
+    val optProxyServer = WSProxyConfiguration.buildWsProxyServer(configuration)
+    withTransformRequest { req =>
+      optProxyServer.foldLeft(req)(_ withProxyServer _)
+    }
+  }
+}

http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/ws/WSRequest.scala

@@ -16,24 +16,30 @@
 
 package uk.gov.hmrc.play.http.ws
 
+import com.typesafe.config.Config
 import play.api.Configuration
 import play.api.libs.ws.{DefaultWSProxyServer, WSProxyServer, WSRequest => PlayWSRequest}
 
 trait WSRequest extends WSRequestBuilder {
 
-  override def buildRequest[A](
+  protected def transformRequest: PlayWSRequest => PlayWSRequest = identity
+
+  override def buildRequest(
     url    : String,
     headers: Seq[(String, String)]
   ): PlayWSRequest =
-    wsClient.url(url)
-      .withHttpHeaders(headers: _*)
+    transformRequest(
+      wsClient.url(url)
+        .withHttpHeaders(headers: _*)
+    )
 }
 
+@deprecated("WSProxy is not required. Use HttpClient.withProxy instead", "13.9.0")
 trait WSProxy extends WSRequest {
 
   def wsProxyServer: Option[WSProxyServer]
 
-  override def buildRequest[A](url: String, headers: Seq[(String, String)]): PlayWSRequest =
+  override def buildRequest(url: String, headers: Seq[(String, String)]): PlayWSRequest =
     wsProxyServer match {
       case Some(proxy) => super.buildRequest(url, headers).withProxyServer(proxy)
       case None        => super.buildRequest(url, headers)
@@ -42,25 +48,38 @@ trait WSProxy extends WSRequest {
 
 object WSProxyConfiguration {
 
+  @deprecated("Use buildWsProxyServer instead. See docs for differences.", "13.9.0")
   def apply(configPrefix: String, configuration: Configuration): Option[WSProxyServer] = {
     val proxyRequired =
       configuration.getOptional[Boolean](s"$configPrefix.proxyRequiredForThisEnvironment").getOrElse(true)
 
-    if (proxyRequired) Some(parseProxyConfiguration(configPrefix, configuration)) else None
+    if (proxyRequired)
+      Some(
+        DefaultWSProxyServer(
+          protocol  = Some(configuration.get[String](s"$configPrefix.protocol")),
+          host      = configuration.get[String](s"$configPrefix.host"),
+          port      = configuration.get[Int](s"$configPrefix.port"),
+          principal = configuration.getOptional[String](s"$configPrefix.username"),
+          password  = configuration.getOptional[String](s"$configPrefix.password")
+        )
+      )
+    else None
   }
 
-  private def parseProxyConfiguration(configPrefix: String, configuration: Configuration) =
-    DefaultWSProxyServer(
-      protocol = configuration
-        .getOptional[String](s"$configPrefix.protocol")
-        .orElse(throw ProxyConfigurationException("protocol")),
-      host =
-        configuration.getOptional[String](s"$configPrefix.host").getOrElse(throw ProxyConfigurationException("host")),
-      port      = configuration.getOptional[Int](s"$configPrefix.port").getOrElse(throw ProxyConfigurationException("port")),
-      principal = configuration.getOptional[String](s"$configPrefix.username"),
-      password  = configuration.getOptional[String](s"$configPrefix.password")
-    )
+  def buildWsProxyServer(configuration: Config): Option[WSProxyServer] = {
+    def getOptionalString(key: String): Option[String] =
+      if (configuration.hasPath(key)) Some(configuration.getString(key)) else None
 
-  case class ProxyConfigurationException(key: String)
-      extends RuntimeException(s"Missing proxy configuration - key '$key' not found")
+    if (configuration.getBoolean("proxy.enabled"))
+      Some(
+        DefaultWSProxyServer(
+          protocol  = Some(configuration.getString("proxy.protocol")),
+          host      = configuration.getString("proxy.host"),
+          port      = configuration.getInt("proxy.port"),
+          principal = getOptionalString("proxy.username"),
+          password  = getOptionalString("proxy.password")
+        )
+      )
+    else None
+  }
 }

http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/ws/WSRequestBuilder.scala

@@ -16,12 +16,16 @@
 
 package uk.gov.hmrc.play.http.ws
 
+import com.typesafe.config.Config
 import play.api.libs.ws.{WSClient, WSRequest => PlayWSRequest}
 import uk.gov.hmrc.http.Request
 
+
 trait WSRequestBuilder extends Request {
 
+  protected def configuration: Config
+
   protected def wsClient: WSClient
 
-  protected def buildRequest[A](url: String, headers: Seq[(String, String)]): PlayWSRequest
+  protected def buildRequest(url: String, headers: Seq[(String, String)]): PlayWSRequest
 }

http-verbs-common/src/test/scala/uk/gov/hmrc/http/HeaderCarrierSpec.scala

@@ -31,18 +31,18 @@ class HeaderCarrierSpec
 
   "headersForUrl" should {
 
-   val internalUrls = List(
-     "http://test.public.service/bar",
-     "http://test.public.mdtp/bar",
-     "http://localhost:1234/bar"
-   )
-   val externalUrl  = "http://test.me"
-
-   def mkConfig(s: String = ""): HeaderCarrier.Config =
-     HeaderCarrier.Config.fromConfig(
-       ConfigFactory.parseString(s)
-         .withFallback(ConfigFactory.load())
+     val internalUrls = List(
+       "http://test.public.service/bar",
+       "http://test.public.mdtp/bar",
+       "http://localhost:1234/bar"
      )
+     val externalUrl  = "http://test.me"
+
+     def mkConfig(s: String = ""): HeaderCarrier.Config =
+       HeaderCarrier.Config.fromConfig(
+         ConfigFactory.parseString(s)
+           .withFallback(ConfigFactory.load())
+       )
 
     "should contain the values passed in by header-carrier for internal urls" in {
       val hc = HeaderCarrier(