diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f2a70f0..ad12ed79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +### Version 14.0.0 + +Improves hook-data model for auditing. + +This should not affect most clients, as long as a compatible library versions are used. It is generally expected that clients only depend on `bootstrap-play` which will transitively provide compatible versions. + + ## Version 13.13.0 ### Auditing max body length diff --git a/build.sbt b/build.sbt index d59dcf5e..792caf41 100644 --- a/build.sbt +++ b/build.sbt @@ -6,13 +6,13 @@ import sbt._ Global / concurrentRestrictions += Tags.limitSum(1, Tags.Test, Tags.Untagged) val scala2_12 = "2.12.15" -val scala2_13 = "2.13.7" +val scala2_13 = "2.13.8" -val silencerVersion = "1.7.7" +val silencerVersion = "1.7.8" lazy val commonSettings = Seq( organization := "uk.gov.hmrc", - majorVersion := 13, + majorVersion := 14, scalaVersion := scala2_12, isPublicArtefact := true, scalacOptions ++= Seq("-feature"), diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpDelete.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpDelete.scala index 955805f0..61f660cb 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpDelete.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpDelete.scala @@ -17,26 +17,38 @@ package uk.gov.hmrc.http import uk.gov.hmrc.http.HttpVerbs.{DELETE => DELETE_VERB} -import uk.gov.hmrc.http.hooks.HttpHooks +import uk.gov.hmrc.http.hooks.{HttpHooks, RequestData, ResponseData} import uk.gov.hmrc.http.logging.ConnectionTracing import scala.concurrent.{ExecutionContext, Future} trait HttpDelete - extends CoreDelete - with DeleteHttpTransport - with HttpVerb - with ConnectionTracing - with HttpHooks - with Retries { + extends CoreDelete + with DeleteHttpTransport + with HttpVerb + with ConnectionTracing + with HttpHooks + with Retries { private lazy val hcConfig = HeaderCarrier.Config.fromConfig(configuration) - override def DELETE[O](url: String, headers: Seq[(String, String)] = Seq.empty)(implicit rds: HttpReads[O], hc: HeaderCarrier, ec: ExecutionContext): Future[O] = + override def DELETE[O]( + url : String, + headers: Seq[(String, String)] = Seq.empty + )(implicit + rds: HttpReads[O], + hc : HeaderCarrier, + ec : ExecutionContext + ): Future[O] = withTracing(DELETE_VERB, url) { - val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version + val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(DELETE_VERB, url)(doDelete(url, allHeaders)) - executeHooks(DELETE_VERB, url"$url", allHeaders, None, httpResponse) + executeHooks( + DELETE_VERB, + url"$url", + RequestData(allHeaders, None), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(DELETE_VERB, url, httpResponse).map(rds.read(DELETE_VERB, url, _)) } } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpGet.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpGet.scala index 2b32713a..243db095 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpGet.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpGet.scala @@ -19,22 +19,30 @@ package uk.gov.hmrc.http import java.net.URLEncoder import uk.gov.hmrc.http.HttpVerbs.{GET => GET_VERB} -import uk.gov.hmrc.http.hooks.HttpHooks +import uk.gov.hmrc.http.hooks.{HttpHooks, RequestData, ResponseData} import uk.gov.hmrc.http.logging.ConnectionTracing import scala.concurrent.{ExecutionContext, Future} -trait HttpGet extends CoreGet with GetHttpTransport with HttpVerb with ConnectionTracing with HttpHooks with Retries { +trait HttpGet + extends CoreGet + with GetHttpTransport + with HttpVerb + with ConnectionTracing + with HttpHooks + with Retries { private lazy val hcConfig = HeaderCarrier.Config.fromConfig(configuration) override def GET[A]( - url: String, + url : String, queryParams: Seq[(String, String)], - headers: Seq[(String, String)])( - implicit rds: HttpReads[A], - hc: HeaderCarrier, - ec: ExecutionContext): Future[A] = { + headers : Seq[(String, String)] + )(implicit + rds: HttpReads[A], + hc : HeaderCarrier, + ec : ExecutionContext + ): Future[A] = { if (queryParams.nonEmpty && url.contains("?")) { throw new UrlValidationException( url, @@ -45,9 +53,14 @@ trait HttpGet extends CoreGet with GetHttpTransport with HttpVerb with Connectio val urlWithQuery = url + makeQueryString(queryParams) withTracing(GET_VERB, urlWithQuery) { - val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version + val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(GET_VERB, urlWithQuery)(doGet(urlWithQuery, headers = allHeaders)) - executeHooks(GET_VERB, url"$url", allHeaders, None, httpResponse) + executeHooks( + GET_VERB, + url"$url", + RequestData(allHeaders, None), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(GET_VERB, urlWithQuery, httpResponse).map(response => rds.read(GET_VERB, urlWithQuery, response)) } } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPatch.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPatch.scala index 34506a6c..f8d9f90e 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPatch.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPatch.scala @@ -18,33 +18,40 @@ package uk.gov.hmrc.http import play.api.libs.json.{Json, Writes} import uk.gov.hmrc.http.HttpVerbs.{PATCH => PATCH_VERB} -import uk.gov.hmrc.http.hooks.{HookData, HttpHooks} +import uk.gov.hmrc.http.hooks.{Body, HookData, HttpHooks, RequestData, ResponseData} import uk.gov.hmrc.http.logging.ConnectionTracing import scala.concurrent.{ExecutionContext, Future} trait HttpPatch - extends CorePatch - with PatchHttpTransport - with HttpVerb - with ConnectionTracing - with HttpHooks - with Retries { + extends CorePatch + with PatchHttpTransport + with HttpVerb + with ConnectionTracing + with HttpHooks + with Retries { private lazy val hcConfig = HeaderCarrier.Config.fromConfig(configuration) override def PATCH[I, O]( - url: String, - body: I, - headers: Seq[(String, String)])( - implicit wts: Writes[I], - rds: HttpReads[O], - hc: HeaderCarrier, - ec: ExecutionContext): Future[O] = + url : String, + body : I, + headers: Seq[(String, String)] + )(implicit + wts: Writes[I], + rds: HttpReads[O], + hc : HeaderCarrier, + ec : ExecutionContext + ): Future[O] = withTracing(PATCH_VERB, url) { - val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version + val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(PATCH_VERB, url)(doPatch(url, body, allHeaders)) - executeHooks(PATCH_VERB, url"$url", allHeaders, Option(HookData.FromString(Json.stringify(wts.writes(body)))), httpResponse) + executeHooks( + PATCH_VERB, + url"$url", + RequestData(allHeaders, Some(Body.Complete(HookData.FromString(Json.stringify(wts.writes(body)))))), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(PATCH_VERB, url, httpResponse).map(response => rds.read(PATCH_VERB, url, response)) } } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPost.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPost.scala index a734fb3e..139d6adb 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPost.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPost.scala @@ -18,74 +18,102 @@ package uk.gov.hmrc.http import play.api.libs.json.{Json, Writes} import uk.gov.hmrc.http.HttpVerbs.{POST => POST_VERB} -import uk.gov.hmrc.http.hooks.{HookData, HttpHooks} +import uk.gov.hmrc.http.hooks.{Body, HookData, HttpHooks, RequestData, ResponseData} import uk.gov.hmrc.http.logging.ConnectionTracing import scala.concurrent.{ExecutionContext, Future} trait HttpPost - extends CorePost - with PostHttpTransport - with HttpVerb - with ConnectionTracing - with HttpHooks - with Retries { + extends CorePost + with PostHttpTransport + with HttpVerb + with ConnectionTracing + with HttpHooks + with Retries { private lazy val hcConfig = HeaderCarrier.Config.fromConfig(configuration) override def POST[I, O]( - url: String, - body: I, - headers: Seq[(String, String)])( - implicit wts: Writes[I], - rds: HttpReads[O], - hc: HeaderCarrier, - ec: ExecutionContext): Future[O] = + url : String, + body : I, + headers: Seq[(String, String)] + )(implicit + wts: Writes[I], + rds: HttpReads[O], + hc : HeaderCarrier, + ec : ExecutionContext + ): Future[O] = withTracing(POST_VERB, url) { val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(POST_VERB, url)(doPost(url, body, allHeaders)) - executeHooks(POST_VERB, url"$url", allHeaders, Option(HookData.FromString(Json.stringify(wts.writes(body)))), httpResponse) + executeHooks( + POST_VERB, + url"$url", + RequestData(allHeaders, Some(Body.Complete(HookData.FromString(Json.stringify(wts.writes(body)))))), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(POST_VERB, url, httpResponse).map(rds.read(POST_VERB, url, _)) } override def POSTString[O]( - url: String, - body: String, - headers: Seq[(String, String)])( - implicit rds: HttpReads[O], - hc: HeaderCarrier, - ec: ExecutionContext): Future[O] = + url : String, + body : String, + headers: Seq[(String, String)] + )(implicit + rds: HttpReads[O], + hc : HeaderCarrier, + ec : ExecutionContext + ): Future[O] = withTracing(POST_VERB, url) { val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(POST_VERB, url)(doPostString(url, body, allHeaders)) - executeHooks(POST_VERB, url"$url", allHeaders, Option(HookData.FromString(body)), httpResponse) + executeHooks( + POST_VERB, + url"$url", + RequestData(allHeaders, Some(Body.Complete(HookData.FromString(body)))), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(POST_VERB, url, httpResponse).map(rds.read(POST_VERB, url, _)) } override def POSTForm[O]( - url: String, - body: Map[String, Seq[String]], - headers: Seq[(String, String)])( - implicit rds: HttpReads[O], - hc: HeaderCarrier, - ec: ExecutionContext): Future[O] = + url : String, + body : Map[String, Seq[String]], + headers: Seq[(String, String)] + )(implicit + rds: HttpReads[O], + hc : HeaderCarrier, + ec : ExecutionContext + ): Future[O] = withTracing(POST_VERB, url) { val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(POST_VERB, url)(doFormPost(url, body, allHeaders)) - executeHooks(POST_VERB, url"$url", allHeaders, Option(HookData.FromMap(body)), httpResponse) + executeHooks( + POST_VERB, + url"$url", + RequestData(allHeaders, Some(Body.Complete(HookData.FromMap(body)))), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(POST_VERB, url, httpResponse).map(rds.read(POST_VERB, url, _)) } override def POSTEmpty[O]( - url: String, - headers: Seq[(String, String)])( - implicit rds: HttpReads[O], - hc: HeaderCarrier, - ec: ExecutionContext): Future[O] = + url : String, + headers: Seq[(String, String)] + )(implicit + rds: HttpReads[O], + hc : HeaderCarrier, + ec : ExecutionContext + ): Future[O] = withTracing(POST_VERB, url) { val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(POST_VERB, url)(doEmptyPost(url, allHeaders)) - executeHooks(POST_VERB, url"$url", allHeaders, None, httpResponse) + executeHooks( + POST_VERB, + url"$url", + RequestData(allHeaders, None), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(POST_VERB, url, httpResponse).map(rds.read(POST_VERB, url, _)) } } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPut.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPut.scala index 6711cf49..93f92cff 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPut.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpPut.scala @@ -18,41 +18,61 @@ package uk.gov.hmrc.http import play.api.libs.json.{Json, Writes} import uk.gov.hmrc.http.HttpVerbs.{PUT => PUT_VERB} -import uk.gov.hmrc.http.hooks.{HookData, HttpHooks} +import uk.gov.hmrc.http.hooks.{Body, HookData, HttpHooks, RequestData, ResponseData} import uk.gov.hmrc.http.logging.ConnectionTracing import scala.concurrent.{ExecutionContext, Future} -trait HttpPut extends CorePut with PutHttpTransport with HttpVerb with ConnectionTracing with HttpHooks with Retries { +trait HttpPut + extends CorePut + with PutHttpTransport + with HttpVerb + with ConnectionTracing + with HttpHooks + with Retries { private lazy val hcConfig = HeaderCarrier.Config.fromConfig(configuration) override def PUT[I, O]( - url: String, - body: I, - headers: Seq[(String, String)])( - implicit wts: Writes[I], - rds: HttpReads[O], - hc: HeaderCarrier, - ec: ExecutionContext): Future[O] = + url : String, + body : I, + headers: Seq[(String, String)] + )(implicit + wts: Writes[I], + rds: HttpReads[O], + hc : HeaderCarrier, + ec : ExecutionContext + ): Future[O] = withTracing(PUT_VERB, url) { val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(PUT_VERB, url)(doPut(url, body, allHeaders)) - executeHooks(PUT_VERB, url"$url", allHeaders, Option(HookData.FromString(Json.stringify(wts.writes(body)))), httpResponse) + executeHooks( + PUT_VERB, + url"$url", + RequestData(allHeaders, Some(Body.Complete(HookData.FromString(Json.stringify(wts.writes(body)))))), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(PUT_VERB, url, httpResponse).map(response => rds.read(PUT_VERB, url, response)) } override def PUTString[O]( - url: String, - body: String, - headers: Seq[(String, String)])( - implicit rds: HttpReads[O], - hc: HeaderCarrier, - ec: ExecutionContext): Future[O] = + url : String, + body : String, + headers: Seq[(String, String)] + )(implicit + rds: HttpReads[O], + hc: HeaderCarrier, + ec: ExecutionContext + ): Future[O] = withTracing(PUT_VERB, url) { val allHeaders = HeaderCarrier.headersForUrl(hcConfig, url, headers) :+ "Http-Client-Version" -> BuildInfo.version val httpResponse = retryOnSslEngineClosed(PUT_VERB, url)(doPutString(url, body, allHeaders)) - executeHooks(PUT_VERB, url"$url", allHeaders, Option(HookData.FromString(body)), httpResponse) + executeHooks( + PUT_VERB, + url"$url", + RequestData(allHeaders, Some(Body.Complete(HookData.FromString(body)))), + httpResponse.map(ResponseData.fromHttpResponse) + ) mapErrors(PUT_VERB, url, httpResponse).map(rds.read(PUT_VERB, url, _)) } } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala index 59d24858..7bb853a9 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala @@ -26,7 +26,7 @@ import play.core.parsers.FormUrlEncodedParser import uk.gov.hmrc.http.{BadGatewayException, BuildInfo, GatewayTimeoutException, HeaderCarrier, HttpReads, HttpResponse, Retries} import uk.gov.hmrc.play.http.BodyCaptor import uk.gov.hmrc.play.http.ws.WSProxyConfiguration -import uk.gov.hmrc.http.hooks.{HookData, HttpHook} +import uk.gov.hmrc.http.hooks.{Body, HookData, HttpHook, RequestData, ResponseData} import uk.gov.hmrc.http.logging.ConnectionTracing import java.net.{ConnectException, URL} @@ -39,7 +39,7 @@ import scala.util.{Failure, Success} trait Executor { def execute[A]( request : WSRequest, - hookDataF: Option[Future[Option[HookData]]], + hookDataF: Option[Future[Option[Body[HookData]]]], isStream : Boolean, r : HttpReads[A] )(implicit @@ -94,7 +94,7 @@ final class RequestBuilderImpl( executor : Executor )( request : WSRequest, - hookDataF: Option[Future[Option[HookData]]] + hookDataF: Option[Future[Option[Body[HookData]]]] )(implicit hc: HeaderCarrier ) extends RequestBuilder { @@ -120,7 +120,7 @@ final class RequestBuilderImpl( override def withProxy: RequestBuilderImpl = transform(request => optProxyServer.foldLeft(request)(_ withProxyServer _)) - private def withHookData(hookDataF: Future[Option[HookData]]): RequestBuilderImpl = + private def withHookData(hookDataF: Future[Option[Body[HookData]]]): RequestBuilderImpl = new RequestBuilderImpl(config, optProxyServer, executor)(request, Some(hookDataF)) // for erasure @@ -134,9 +134,8 @@ final class RequestBuilderImpl( } override def withBody[B : BodyWritable : TypeTag](body: B): RequestBuilderImpl = { - val hookDataP = Promise[Option[HookData]]() + val hookDataP = Promise[Option[Body[HookData]]]() val maxBodyLength = config.get[Int]("http-verbs.auditing.maxBodyLength") - val loggingContext = s"outgoing ${request.method} ${request.url} request" transform { req => val req2 = req.withBody(body) req2.body match { @@ -145,19 +144,18 @@ final class RequestBuilderImpl( case InMemoryBody(bytes) => // we can't guarantee that the default BodyWritables have been used - so rather than relying on content-type alone, we identify form data // by provided body type (Map) or content-type (e.g. form data as a string) (body, req2.header("Content-Type")) match { - case (IsMap(m), _ ) => hookDataP.success(Some(HookData.FromMap(m))) - case (_ , Some("application/x-www-form-urlencoded")) => hookDataP.success(Some(HookData.FromMap(FormUrlEncodedParser.parse(bytes.decodeString("UTF-8"))))) - case _ => val auditedBody = BodyCaptor.bodyUpto(bytes, maxBodyLength, loggingContext, isStream = false).decodeString("UTF-8") - hookDataP.success(Some(HookData.FromString(auditedBody))) + case (IsMap(m), _ ) => hookDataP.success(Some(Body.Complete(HookData.FromMap(m)))) + case (_ , Some("application/x-www-form-urlencoded")) => hookDataP.success(Some(Body.Complete(HookData.FromMap(FormUrlEncodedParser.parse(bytes.utf8String))))) + case _ => val body = BodyCaptor.bodyUpto(bytes, maxBodyLength) + hookDataP.success(Some(body.map(bytes => HookData.FromString(bytes.utf8String)))) } req2 case SourceBody(source) => val src2: Source[ByteString, _] = source .alsoTo( BodyCaptor.sink( - loggingContext = loggingContext, maxBodyLength = maxBodyLength, - withCapturedBody = body => hookDataP.success(Some(HookData.FromString(body.decodeString("UTF-8")))) + withCapturedBody = body => hookDataP.success(Some(body.map(bytes => HookData.FromString(bytes.utf8String)))) ) ).recover { case e => hookDataP.failure(e); throw e @@ -195,14 +193,14 @@ class ExecutorImpl( final def execute[A]( request : WSRequest, - optHookDataF: Option[Future[Option[HookData]]], + optHookDataF: Option[Future[Option[Body[HookData]]]], isStream : Boolean, httpReads : HttpReads[A] )(implicit hc: HeaderCarrier, ec: ExecutionContext ): Future[A] = { - val hookDataF = + val hookDataF: Future[Option[Body[HookData]]] = optHookDataF match { case None if request.body != EmptyBody => sys.error(s"There is no audit data available. Please ensure you call `withBody` on the RequestBuilder rather than `transform(_.withBody)`") @@ -227,55 +225,56 @@ class ExecutorImpl( .map(httpReads.read(request.method, request.url, _)) } - // unfortunate return type - first HttpResponse is the full response, the second HttpResponse is truncated for auditing... private def toHttpResponse( isStream : Boolean, request : WSRequest, responseF: Future[WSResponse] )(implicit ec: ExecutionContext - ): (Future[HttpResponse], Future[HttpResponse]) = { - val auditResponseF = Promise[HttpResponse]() - val loggingContext = s"outgoing ${request.method} ${request.url} response" + ): (Future[HttpResponse], Future[ResponseData]) = { + val auditResponseF = Promise[ResponseData]() + + // play returns scala.collection, but default for Scala 2.13 is scala.collection.immutable + def forScala2_13(m: scala.collection.Map[String, scala.collection.Seq[String]]): Map[String, Seq[String]] = + m.mapValues(_.toSeq).toMap + val httpResponseF = for { response <- responseF status = response.status - headers = response.headers.mapValues(_.toSeq).toMap + headers = forScala2_13(response.headers) } yield { - def httpResponse(body : Either[Source[ByteString, _], String]): HttpResponse = - body match { - case Left(src) => HttpResponse( - status = response.status, - bodyAsSource = src, - headers = response.headers.mapValues(_.toSeq).toMap - ) - case Right(str) => HttpResponse( - status = response.status, - body = str, - headers = response.headers.mapValues(_.toSeq).toMap - ) - } if (isStream) { val source = response.bodyAsSource .alsoTo( BodyCaptor.sink( - loggingContext = loggingContext, maxBodyLength = maxBodyLength, - withCapturedBody = body => auditResponseF.success(httpResponse(Right(body.decodeString("UTF-8")))) + withCapturedBody = body => auditResponseF.success(ResponseData( + body = body.map(_.utf8String), + status = response.status, + headers = headers + )) ) ) .recover { case e => auditResponseF.failure(e); throw e } - httpResponse(Left(source)) + HttpResponse( + status = response.status, + bodyAsSource = source, + headers = headers + ) } else { - auditResponseF.success( - httpResponse(Right( - BodyCaptor.bodyUpto(response.body, maxBodyLength, loggingContext, isStream = false) - )) + auditResponseF.success(ResponseData( + body = BodyCaptor.bodyUpto(ByteString(response.body), maxBodyLength).map(_.utf8String), + status = response.status, + headers = headers + )) + HttpResponse( + status = response.status, + body = response.body, + headers = headers ) - httpResponse(Right(response.body)) } } (httpResponseF, auditResponseF.future) @@ -284,8 +283,8 @@ class ExecutorImpl( private def executeHooks( isStream : Boolean, request : WSRequest, - hookDataF : Future[Option[HookData]], - auditedResponseF: Future[HttpResponse] // play-auditing expects the body to be a String + hookDataF : Future[Option[Body[HookData]]], + auditedResponseF: Future[ResponseData] )(implicit hc: HeaderCarrier, ec: ExecutionContext @@ -293,20 +292,22 @@ class ExecutorImpl( def denormalise(hdrs: Map[String, Seq[String]]): Seq[(String, String)] = hdrs.toList.flatMap { case (k, vs) => vs.map(k -> _) } - def executeHooksWithHookData(hookData: Option[HookData]) = + def executeHooksWithHookData(hookData: Option[Body[HookData]]) = hooks.foreach( _.apply( verb = request.method, url = new URL(request.url), - headers = denormalise(request.headers), - body = hookData, + request = RequestData( + headers = denormalise(request.headers), + body = hookData + ), responseF = auditedResponseF ) ) hookDataF.onComplete { case Success(hookData) => executeHooksWithHookData(hookData) - case Failure(e) => // this is unlikely, but we want best attempt at auditing + case Failure(e) => // this will only happen if we fail to upload a stream. We want best attempt at auditing executeHooksWithHookData(None) } } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHook.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHook.scala index fd21e762..6ea4e0e5 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHook.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHook.scala @@ -24,19 +24,57 @@ import scala.concurrent.{ExecutionContext, Future} trait HttpHook { def apply( - verb: String, - url: URL, - headers : Seq[(String, String)], - body: Option[HookData], - responseF: Future[HttpResponse] - )( - implicit hc: HeaderCarrier, + verb : String, + url : URL, + request : RequestData, + responseF: Future[ResponseData] + )(implicit + hc: HeaderCarrier, ec: ExecutionContext ): Unit } +sealed trait Body[+A] { + final def map[B](f: A => B): Body[B] = + this match { + case Body.Complete(body) => Body.Complete(f(body)) + case Body.Truncated(body) => Body.Truncated(f(body)) + } + + final def isTruncated: Boolean = + this match { + case Body.Complete (b) => false + case Body.Truncated(b) => true + } +} + +object Body { + case class Complete [A](body: A) extends Body[A] + case class Truncated[A](body: A) extends Body[A] +} + +case class ResponseData( + body : Body[String], + status : Int, + headers: Map[String, Seq[String]] +) + +object ResponseData { + def fromHttpResponse(httpResponse: HttpResponse) = + ResponseData( + body = Body.Complete(httpResponse.body), + status = httpResponse.status, + headers = httpResponse.headers + ) +} + +case class RequestData( + headers: Seq[(String, String)], + body : Option[Body[HookData]] +) + sealed trait HookData object HookData { - case class FromString(s: String) extends HookData + case class FromString(s: String) extends HookData case class FromMap(m: Map[String, Seq[String]]) extends HookData } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHooks.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHooks.scala index 1fa09bcc..efa1bdb1 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHooks.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHooks.scala @@ -18,37 +18,23 @@ package uk.gov.hmrc.http.hooks import java.net.URL -import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse} +import uk.gov.hmrc.http.HeaderCarrier import scala.concurrent.{ExecutionContext, Future} trait HttpHooks { val hooks: Seq[HttpHook] - val NoneRequired = Seq( - new HttpHook { - def apply( - verb : String, - url : URL, - headers : Seq[(String, String)], - body : Option[HookData], - responseF: Future[HttpResponse] - )( - implicit hc: HeaderCarrier, - ec: ExecutionContext - ): Unit = {} - } - ) + val NoneRequired = Seq.empty protected def executeHooks( - verb : String, - url : URL, - headers: Seq[(String, String)], - body : Option[HookData], - responseF: Future[HttpResponse] - )( - implicit hc: HeaderCarrier, + verb : String, + url : URL, + request : RequestData, + responseF: Future[ResponseData] + )(implicit + hc: HeaderCarrier, ec: ExecutionContext ): Unit = - hooks.foreach(_.apply(verb, url, headers, body, responseF)) + hooks.foreach(_.apply(verb, url, request, responseF)) } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/BodyCaptor.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/BodyCaptor.scala index 64472732..6dbc6437 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/BodyCaptor.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/BodyCaptor.scala @@ -20,14 +20,13 @@ import akka.stream.{Attributes, FlowShape, Inlet, Outlet} import akka.stream.scaladsl.{Flow, Sink} import akka.stream.stage._ import akka.util.ByteString -import org.slf4j.LoggerFactory +import uk.gov.hmrc.http.hooks.Body // based on play.filters.csrf.CSRFAction#BodyHandler private class BodyCaptorFlow( - loggingContext : String, maxBodyLength : Int, - withCapturedBody: ByteString => Unit + withCapturedBody: Body[ByteString] => Unit ) extends GraphStage[FlowShape[ByteString, ByteString]] { val in = Inlet[ByteString]("BodyCaptorFlow.in") val out = Outlet[ByteString]("BodyCaptorFlow.out") @@ -52,7 +51,7 @@ private class BodyCaptorFlow( } override def onUpstreamFinish(): Unit = { - withCapturedBody(BodyCaptor.bodyUpto(buffer, maxBodyLength, loggingContext, isStream = true)) + withCapturedBody(BodyCaptor.bodyUpto(buffer, maxBodyLength)) completeStage() } } @@ -61,42 +60,25 @@ private class BodyCaptorFlow( } object BodyCaptor { - private val logger = LoggerFactory.getLogger(getClass) - def flow( - loggingContext : String, maxBodyLength : Int, - withCapturedBody: ByteString => Unit // provide a callback since a Materialized value would be not be available until the flow has been run + withCapturedBody: Body[ByteString] => Unit // provide a callback since a Materialized value would be not be available until the flow has been run ): Flow[ByteString, ByteString, akka.NotUsed] = Flow.fromGraph(new BodyCaptorFlow( - loggingContext = loggingContext, maxBodyLength = maxBodyLength, withCapturedBody = withCapturedBody )) def sink( - loggingContext : String, maxBodyLength : Int, - withCapturedBody: ByteString => Unit + withCapturedBody: Body[ByteString] => Unit ): Sink[ByteString, akka.NotUsed] = - flow(loggingContext, maxBodyLength, withCapturedBody) + flow(maxBodyLength, withCapturedBody) .to(Sink.ignore) - def bodyUpto(body: String, maxBodyLength: Int, loggingContext: String, isStream: Boolean): String = - if (body.length > maxBodyLength) { - logger.warn( - s"$loggingContext ${if (isStream) "streamed body" else "body " + body.length} exceeds maxLength $maxBodyLength - truncating" - ) - body.take(maxBodyLength) - } else - body - - def bodyUpto(body: ByteString, maxBodyLength: Int, loggingContext: String, isStream: Boolean): ByteString = - if (body.length > maxBodyLength) { - logger.warn( - s"$loggingContext ${if (isStream) "streamed body" else "body " + body.length} exceeds maxLength $maxBodyLength - truncating" - ) - body.take(maxBodyLength) - } else - body + def bodyUpto(body: ByteString, maxBodyLength: Int): Body[ByteString] = + if (body.length > maxBodyLength) + Body.Truncated(body.take(maxBodyLength)) + else + Body.Complete(body) } diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/HeaderCarrierConverter.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/HeaderCarrierConverter.scala index fa23bf0a..486ec65e 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/HeaderCarrierConverter.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/HeaderCarrierConverter.scala @@ -94,10 +94,8 @@ trait HeaderCarrierConverter { trueClientPort = headers.get(HeaderNames.trueClientPort), gaToken = headers.get(HeaderNames.googleAnalyticTokenId), gaUserId = headers.get(HeaderNames.googleAnalyticUserId), - deviceID = session.fold(headers.get(HeaderNames.deviceID))(_ => - cookies.get(CookieNames.deviceID).map(_.value) - .fold[Option[String]](headers.get(HeaderNames.deviceID))(Some(_)) - ), + deviceID = session.flatMap(_ => cookies.get(CookieNames.deviceID).map(_.value)) + .orElse(headers.get(HeaderNames.deviceID)), akamaiReputation = headers.get(HeaderNames.akamaiReputation).map(AkamaiReputation), otherHeaders = otherHeaders(headers, request) ) diff --git a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpDeleteSpec.scala b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpDeleteSpec.scala index 32933aa9..14781b53 100644 --- a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpDeleteSpec.scala +++ b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpDeleteSpec.scala @@ -25,7 +25,7 @@ import org.scalatest.concurrent.PatienceConfiguration.{Interval, Timeout} import org.scalatest.time.{Millis, Seconds, Span} import org.scalatest.wordspec.AnyWordSpecLike import org.scalatest.matchers.should.Matchers -import uk.gov.hmrc.http.hooks.HttpHook +import uk.gov.hmrc.http.hooks.{Body, HttpHook, RequestData, ResponseData} import scala.concurrent.{ExecutionContext, Future} @@ -143,25 +143,35 @@ class HttpDeleteSpec testDelete.DELETE[HttpResponse](url, Seq("header" -> "foo")).futureValue - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testDelete.configuration) val headers = HeaderCarrier.headersForUrl(config, url, Seq("header" -> "foo")) - verify(testDelete.testHook1).apply(eqTo("DELETE"), eqTo(url"$url"), headerCaptor1, eqTo(None), respArgCaptor1)(any, any) - verify(testDelete.testHook2).apply(eqTo("DELETE"), eqTo(url"$url"), headerCaptor2, eqTo(None), respArgCaptor2)(any, any) + verify(testDelete.testHook1).apply(eqTo("DELETE"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testDelete.testHook2).apply(eqTo("DELETE"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe None + + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe None + + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } } diff --git a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpGetSpec.scala b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpGetSpec.scala index 09840162..c2af5044 100644 --- a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpGetSpec.scala +++ b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpGetSpec.scala @@ -14,22 +14,6 @@ * limitations under the License. */ -/* - * Copyright 2015 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.http import akka.actor.ActorSystem @@ -40,7 +24,7 @@ import org.mockito.scalatest.MockitoSugar import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures} import org.scalatest.wordspec.AnyWordSpecLike import org.scalatest.matchers.should.Matchers -import uk.gov.hmrc.http.hooks.HttpHook +import uk.gov.hmrc.http.hooks.{Body, HttpHook, RequestData, ResponseData} import scala.concurrent.{ExecutionContext, Future} import uk.gov.hmrc.http.HttpReads.Implicits._ @@ -139,25 +123,35 @@ class HttpGetSpec testGet.GET[HttpResponse](url).futureValue - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testGet.configuration) val headers = HeaderCarrier.headersForUrl(config, url) - verify(testGet.testHook1).apply(eqTo("GET"), eqTo(url"$url"), headerCaptor1, eqTo(None), respArgCaptor1)(any, any) - verify(testGet.testHook2).apply(eqTo("GET"), eqTo(url"$url"), headerCaptor2, eqTo(None), respArgCaptor2)(any, any) + verify(testGet.testHook1).apply(eqTo("GET"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testGet.testHook2).apply(eqTo("GET"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) + + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe None - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe None + + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } @@ -249,7 +243,6 @@ class HttpGetSpec .GET[HttpResponse]("http://test.net?should=not=be+here", Seq(("one", "1"))) } - "be able to return plain responses provided already has Query and Header String" in { val response = HttpResponse(200, testBody) val testGet = new StubbedHttpGet(Future.successful(response)) diff --git a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPatchSpec.scala b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPatchSpec.scala index 14af4044..0800cb7e 100644 --- a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPatchSpec.scala +++ b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPatchSpec.scala @@ -24,7 +24,7 @@ import org.mockito.scalatest.MockitoSugar import org.scalatest.wordspec.AnyWordSpecLike import org.scalatest.matchers.should.Matchers import play.api.libs.json.{Json, Writes} -import uk.gov.hmrc.http.hooks.{HookData, HttpHook} +import uk.gov.hmrc.http.hooks.{Body, HookData, HttpHook, RequestData, ResponseData} import scala.concurrent.{ExecutionContext, Future} @@ -149,25 +149,35 @@ class HttpPatchSpec testPatch.PATCH[TestRequestClass, HttpResponse](url, testObject, Seq("header" -> "foo")).futureValue - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testPatch.configuration) val headers = HeaderCarrier.headersForUrl(config, url, Seq("header" -> "foo")) - verify(testPatch.testHook1).apply(eqTo("PATCH"), eqTo(url"$url"), headerCaptor1, eqTo(Some(HookData.FromString(testJson))), respArgCaptor1)(any, any) - verify(testPatch.testHook2).apply(eqTo("PATCH"), eqTo(url"$url"), headerCaptor2, eqTo(Some(HookData.FromString(testJson))), respArgCaptor2)(any, any) + verify(testPatch.testHook1).apply(eqTo("PATCH"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testPatch.testHook2).apply(eqTo("PATCH"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe Some(Body.Complete(HookData.FromString(testJson))) + + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe Some(Body.Complete(HookData.FromString(testJson))) + + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } } diff --git a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPostSpec.scala b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPostSpec.scala index d7a0ae30..43a09418 100644 --- a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPostSpec.scala +++ b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPostSpec.scala @@ -24,7 +24,7 @@ import org.mockito.scalatest.MockitoSugar import org.scalatest.wordspec.AnyWordSpecLike import org.scalatest.matchers.should.Matchers import play.api.libs.json.{Json, Writes} -import uk.gov.hmrc.http.hooks.{HookData, HttpHook} +import uk.gov.hmrc.http.hooks.{Body, HookData, HttpHook, RequestData, ResponseData} import scala.concurrent.{ExecutionContext, Future} @@ -191,25 +191,35 @@ class HttpPostSpec val testJson = Json.stringify(trcreads.writes(testObject)) - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testPost.configuration) val headers = HeaderCarrier.headersForUrl(config, url) - verify(testPost.testHook1).apply(eqTo("POST"), eqTo(url"$url"), headerCaptor1, eqTo(Some(HookData.FromString(testJson))), respArgCaptor1)(any, any) - verify(testPost.testHook2).apply(eqTo("POST"), eqTo(url"$url"), headerCaptor2, eqTo(Some(HookData.FromString(testJson))), respArgCaptor2)(any, any) + verify(testPost.testHook1).apply(eqTo("POST"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testPost.testHook2).apply(eqTo("POST"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe Some(Body.Complete(HookData.FromString(testJson))) + + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe Some(Body.Complete(HookData.FromString(testJson))) + + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } @@ -234,25 +244,35 @@ class HttpPostSpec testPost.POSTForm[HttpResponse](url, Map.empty[String, Seq[String]], Seq.empty).futureValue - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testPost.configuration) val headers = HeaderCarrier.headersForUrl(config, url) - verify(testPost.testHook1).apply(eqTo("POST"), eqTo(url"$url"), headerCaptor1, eqTo(Some(HookData.FromMap(Map()))), respArgCaptor1)(any, any) - verify(testPost.testHook2).apply(eqTo("POST"), eqTo(url"$url"), headerCaptor2, eqTo(Some(HookData.FromMap(Map()))), respArgCaptor2)(any, any) + verify(testPost.testHook1).apply(eqTo("POST"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testPost.testHook2).apply(eqTo("POST"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) + + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe Some(Body.Complete(HookData.FromMap(Map()))) - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe Some(Body.Complete(HookData.FromMap(Map()))) + + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } @@ -275,31 +295,42 @@ class HttpPostSpec } "Invoke any hooks provided" in { - val dummyResponse = HttpResponse(200, """{"foo":"t","bar":10}""") + val testBody = """{"foo":"t","bar":10}""" + val dummyResponse = HttpResponse(200, testBody) val dummyResponseFuture = Future.successful(dummyResponse) val testPost = new StubbedHttpPost(dummyResponseFuture) testPost.POSTString[TestClass](url, testRequestBody).futureValue - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testPost.configuration) val headers = HeaderCarrier.headersForUrl(config, url) - verify(testPost.testHook1).apply(eqTo("POST"), eqTo(url"$url"), headerCaptor1, eqTo(Some(HookData.FromString(testRequestBody))), respArgCaptor1)(any, any) - verify(testPost.testHook2).apply(eqTo("POST"), eqTo(url"$url"), headerCaptor2, eqTo(Some(HookData.FromString(testRequestBody))), respArgCaptor2)(any, any) + verify(testPost.testHook1).apply(eqTo("POST"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testPost.testHook2).apply(eqTo("POST"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe Some(Body.Complete(HookData.FromString(testRequestBody))) + + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe Some(Body.Complete(HookData.FromString(testRequestBody))) + + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } @@ -318,31 +349,42 @@ class HttpPostSpec behave like aTracingHttpCall("POST", "POST", new StubbedHttpPost(defaultHttpResponse)) { _.POSTEmpty[HttpResponse](url) } "Invoke any hooks provided" in { - val dummyResponse = HttpResponse(200, """{"foo":"t","bar":10}""") + val testBody = """{"foo":"t","bar":10}""" + val dummyResponse = HttpResponse(200, testBody) val dummyResponseFuture = Future.successful(dummyResponse) val testPost = new StubbedHttpPost(dummyResponseFuture) testPost.POSTEmpty[TestClass](url).futureValue - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testPost.configuration) val headers = HeaderCarrier.headersForUrl(config, url) - verify(testPost.testHook1).apply(eqTo("POST"), eqTo(url"$url"), headerCaptor1, eqTo(None), respArgCaptor1)(any, any) - verify(testPost.testHook2).apply(eqTo("POST"), eqTo(url"$url"), headerCaptor2, eqTo(None), respArgCaptor2)(any, any) + verify(testPost.testHook1).apply(eqTo("POST"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testPost.testHook2).apply(eqTo("POST"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) + + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe None + + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe None - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } diff --git a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPutSpec.scala b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPutSpec.scala index e76a901e..06ba17b3 100644 --- a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPutSpec.scala +++ b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpPutSpec.scala @@ -24,7 +24,7 @@ import org.mockito.scalatest.MockitoSugar import org.scalatest.wordspec.AnyWordSpecLike import org.scalatest.matchers.should.Matchers import play.api.libs.json.{Json, Writes} -import uk.gov.hmrc.http.hooks.{HookData, HttpHook} +import uk.gov.hmrc.http.hooks.{Body, HookData, HttpHook, RequestData, ResponseData} import scala.concurrent.{ExecutionContext, Future} @@ -167,25 +167,35 @@ class HttpPutSpec testPut.PUT[TestRequestClass, HttpResponse](url, testObject).futureValue - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testPut.configuration) val headers = HeaderCarrier.headersForUrl(config, url) - verify(testPut.testHook1).apply(eqTo("PUT"), eqTo(url"$url"), headerCaptor1, eqTo(Some(HookData.FromString(testJson))), respArgCaptor1)(any, any) - verify(testPut.testHook2).apply(eqTo("PUT"), eqTo(url"$url"), headerCaptor2, eqTo(Some(HookData.FromString(testJson))), respArgCaptor2)(any, any) + verify(testPut.testHook1).apply(eqTo("PUT"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testPut.testHook2).apply(eqTo("PUT"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe Some(Body.Complete(HookData.FromString(testJson))) + + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe Some(Body.Complete(HookData.FromString(testJson))) + + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } @@ -211,31 +221,42 @@ class HttpPutSpec } "Invoke any hooks provided" in { - val dummyResponse = HttpResponse(200, """{"foo":"t","bar":10}""") + val testBody = """{"foo":"t","bar":10}""" + val dummyResponse = HttpResponse(200, testBody) val dummyResponseFuture = Future.successful(dummyResponse) val testPut = new StubbedHttpPut(dummyResponseFuture) testPut.PUTString[TestClass](url, testRequestBody, Seq.empty).futureValue - val respArgCaptor1 = ArgCaptor[Future[HttpResponse]] - val respArgCaptor2 = ArgCaptor[Future[HttpResponse]] + val responseFCaptor1 = ArgCaptor[Future[ResponseData]] + val responseFCaptor2 = ArgCaptor[Future[ResponseData]] - val headerCaptor1 = ArgCaptor[Seq[(String, String)]] - val headerCaptor2 = ArgCaptor[Seq[(String, String)]] + val requestCaptor1 = ArgCaptor[RequestData] + val requestCaptor2 = ArgCaptor[RequestData] val config = HeaderCarrier.Config.fromConfig(testPut.configuration) val headers = HeaderCarrier.headersForUrl(config, url) - verify(testPut.testHook1).apply(eqTo("PUT"), eqTo(url"$url"), headerCaptor1, eqTo(Some(HookData.FromString(testRequestBody))), respArgCaptor1)(any, any) - verify(testPut.testHook2).apply(eqTo("PUT"), eqTo(url"$url"), headerCaptor2, eqTo(Some(HookData.FromString(testRequestBody))), respArgCaptor2)(any, any) + verify(testPut.testHook1).apply(eqTo("PUT"), eqTo(url"$url"), requestCaptor1, responseFCaptor1)(any, any) + verify(testPut.testHook2).apply(eqTo("PUT"), eqTo(url"$url"), requestCaptor2, responseFCaptor2)(any, any) + + val request1 = requestCaptor1.value + request1.headers should contain allElementsOf(headers) + request1.body shouldBe Some(Body.Complete(HookData.FromString(testRequestBody))) + + val request2 = requestCaptor2.value + request2.headers should contain allElementsOf(headers) + request2.body shouldBe Some(Body.Complete(HookData.FromString(testRequestBody))) - // verifying directly without ArgumentCaptor didn't work as Futures were different instances + // verifying directly without ArgCaptor doesn't work since Futures are different instances // e.g. Future.successful(5) != Future.successful(5) - respArgCaptor1.value.futureValue shouldBe dummyResponse - respArgCaptor2.value.futureValue shouldBe dummyResponse + val response1 = responseFCaptor1.value.futureValue + response1.status shouldBe 200 + response1.body shouldBe Body.Complete(testBody) - headerCaptor1.value should contain allElementsOf(headers) - headerCaptor2.value should contain allElementsOf(headers) + val response2 = responseFCaptor2.value.futureValue + response2.status shouldBe 200 + response2.body shouldBe Body.Complete(testBody) } } } diff --git a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/client/HttpClientV2Spec.scala b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/client/HttpClientV2Spec.scala index fe0067f4..da46bebd 100644 --- a/http-verbs-common/src/test/scala/uk/gov/hmrc/http/client/HttpClientV2Spec.scala +++ b/http-verbs-common/src/test/scala/uk/gov/hmrc/http/client/HttpClientV2Spec.scala @@ -32,7 +32,7 @@ import play.api.Configuration import play.api.libs.json.{Json, Reads, Writes} import play.api.libs.ws.ahc.{AhcWSClient, AhcWSClientConfigFactory} import uk.gov.hmrc.http.{HeaderCarrier, HttpReads, HttpReadsInstances, HttpResponse, Retries, StringContextOps, UpstreamErrorResponse} -import uk.gov.hmrc.http.hooks.{HookData, HttpHook} +import uk.gov.hmrc.http.hooks.{Body, HookData, HttpHook, RequestData, ResponseData} import uk.gov.hmrc.http.test.WireMockSupport import java.util.concurrent.atomic.AtomicInteger @@ -76,23 +76,23 @@ class HttpClientV2Spec .withHeader("Http-ClientV2-Version", matching(".*")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("PUT"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromString("\"req\""))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "application/json") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Some(Body.Complete(HookData.FromString("\"req\""))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "application/json") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 200 - auditedResponse.body shouldBe "\"res\"" + auditedResponse.body shouldBe Body.Complete("\"res\"") } "work with streams" in new Setup { @@ -124,23 +124,23 @@ class HttpClientV2Spec .withHeader("User-Agent", equalTo("myapp")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("PUT"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromString(requestBody))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "application/octet-stream") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Some(Body.Complete(HookData.FromString(requestBody))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "application/octet-stream") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 200 - auditedResponse.body shouldBe responseBody + auditedResponse.body shouldBe Body.Complete(responseBody) } "handled failed requests with streams" in new Setup { @@ -176,23 +176,23 @@ class HttpClientV2Spec .withHeader("User-Agent", equalTo("myapp")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("PUT"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromString(requestBody))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "application/octet-stream") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Some(Body.Complete(HookData.FromString(requestBody))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "application/octet-stream") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 500 - auditedResponse.body shouldBe responseBody + auditedResponse.body shouldBe Body.Complete(responseBody) } "truncate stream payloads for auditing if too long" in new Setup { @@ -224,23 +224,23 @@ class HttpClientV2Spec .withHeader("User-Agent", equalTo("myapp")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("PUT"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromString(requestBody.take(maxAuditBodyLength)))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "application/octet-stream") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Some(Body.Truncated(HookData.FromString(requestBody.take(maxAuditBodyLength)))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "application/octet-stream") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 200 - auditedResponse.body shouldBe responseBody.take(maxAuditBodyLength) + auditedResponse.body shouldBe Body.Truncated(responseBody.take(maxAuditBodyLength)) } "truncate strict payloads for auditing if too long" in new Setup { @@ -269,23 +269,23 @@ class HttpClientV2Spec .withHeader("User-Agent", equalTo("myapp")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("PUT"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromString(requestBody.take(maxAuditBodyLength)))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "text/plain") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Some(Body.Truncated(HookData.FromString(requestBody.take(maxAuditBodyLength)))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "text/plain") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 200 - auditedResponse.body shouldBe responseBody.take(maxAuditBodyLength) + auditedResponse.body shouldBe Body.Truncated(responseBody.take(maxAuditBodyLength)) } "work with form data" in new Setup { @@ -317,23 +317,23 @@ class HttpClientV2Spec .withHeader("User-Agent", equalTo("myapp")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("POST"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromMap(body))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "application/x-www-form-urlencoded") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Some(Body.Complete(HookData.FromMap(body))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "application/x-www-form-urlencoded") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 200 - auditedResponse.body shouldBe "\"res\"" + auditedResponse.body shouldBe Body.Complete("\"res\"") } "work with form data - custom writeable for content-type" in new Setup { @@ -379,23 +379,23 @@ class HttpClientV2Spec .withHeader("User-Agent", equalTo("myapp")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("POST"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromMap(body))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "nonstandard/x-www-form-urlencoded") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Some(Body.Complete(HookData.FromMap(body))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "nonstandard/x-www-form-urlencoded") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 200 - auditedResponse.body shouldBe "\"res\"" + auditedResponse.body shouldBe Body.Complete("\"res\"") } "work with form data - custom writeable for map type" in new Setup { @@ -441,23 +441,23 @@ class HttpClientV2Spec .withHeader("User-Agent", equalTo("myapp")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("POST"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromMap(body.toMap))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "application/x-www-form-urlencoded") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Some(Body.Complete(HookData.FromMap(body.toMap))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "application/x-www-form-urlencoded") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 200 - auditedResponse.body shouldBe "\"res\"" + auditedResponse.body shouldBe Body.Complete("\"res\"") } /* Note, using non-form-encoding and a non immutable Map implementation will not be escaped properly @@ -504,23 +504,23 @@ class HttpClientV2Spec .withHeader("User-Agent", equalTo("myapp")) ) - val headersCaptor = ArgCaptor[Seq[(String, String)]] - val responseCaptor = ArgCaptor[Future[HttpResponse]] + val requestCaptor = ArgCaptor[RequestData] + val responseFCaptor = ArgCaptor[Future[ResponseData]] verify(mockHttpHook) .apply( verb = eqTo("POST"), url = eqTo(url"$wireMockUrl/"), - headers = headersCaptor, - body = eqTo(Some(HookData.FromMap(body.toMap))), - responseF = responseCaptor + request = requestCaptor, + responseF = responseFCaptor )(any[HeaderCarrier], any[ExecutionContext]) - headersCaptor.value should contain ("User-Agent" -> "myapp") - headersCaptor.value should contain ("Content-Type" -> "application/x-www-form-urlencoded") - val auditedResponse = responseCaptor.value.futureValue + requestCaptor.value.body shouldBe Body.Complete(Some(HookData.FromMap(body.toMap))) + requestCaptor.value.headers should contain ("User-Agent" -> "myapp") + requestCaptor.value.headers should contain ("Content-Type" -> "application/x-www-form-urlencoded") + val auditedResponse = responseFCaptor.value.futureValue auditedResponse.status shouldBe 200 - auditedResponse.body shouldBe "\"res\"" + auditedResponse.body shouldBe Body.Complete("\"res\"") } */ @@ -597,14 +597,11 @@ class HttpClientV2Spec val maxAuditBodyLength = 30 - val httpClientV2: HttpClientV2 = { + def mkHttpClientV2(configStr: String): HttpClientV2 = { val config = Configuration( - ConfigFactory.parseString( - s"""|appName = myapp - |http-verbs.auditing.maxBodyLength = $maxAuditBodyLength - |""".stripMargin - ).withFallback(ConfigFactory.load()) + ConfigFactory.parseString(configStr) + .withFallback(ConfigFactory.load()) ) new HttpClientV2Impl( wsClient = AhcWSClient(AhcWSClientConfigFactory.forConfig(config.underlying)), @@ -613,6 +610,13 @@ class HttpClientV2Spec hooks = Seq(mockHttpHook), ) } + + val httpClientV2: HttpClientV2 = + mkHttpClientV2( + s"""|appName = myapp + |http-verbs.auditing.maxBodyLength = $maxAuditBodyLength + |""".stripMargin + ) } } diff --git a/http-verbs-test-common/src/main/scala/uk/gov/hmrc/http/test/HttpClient2Support.scala b/http-verbs-test-common/src/main/scala/uk/gov/hmrc/http/test/HttpClientV2Support.scala similarity index 100% rename from http-verbs-test-common/src/main/scala/uk/gov/hmrc/http/test/HttpClient2Support.scala rename to http-verbs-test-common/src/main/scala/uk/gov/hmrc/http/test/HttpClientV2Support.scala diff --git a/http-verbs-test-common/src/test/resources/__files/bankHolidays.json b/http-verbs-test-common/src/test/resources/__files/bankHolidays.json index 8d0d188e..238a8ac7 100644 --- a/http-verbs-test-common/src/test/resources/__files/bankHolidays.json +++ b/http-verbs-test-common/src/test/resources/__files/bankHolidays.json @@ -1,7 +1,7 @@ { "events": [ { - "title": "New Year’s Day", + "title": "New Year's Day", "date": "2017-01-02" }, { @@ -33,4 +33,4 @@ "date": "2017-12-26" } ] -} \ No newline at end of file +} diff --git a/http-verbs-test-common/src/test/resources/__files/bankHolidays.xml b/http-verbs-test-common/src/test/resources/__files/bankHolidays.xml index 9df48d01..c55d9c7b 100644 --- a/http-verbs-test-common/src/test/resources/__files/bankHolidays.xml +++ b/http-verbs-test-common/src/test/resources/__files/bankHolidays.xml @@ -1,7 +1,7 @@ - New Year’s Day + New Year's Day 2017-01-02 @@ -32,4 +32,4 @@ Boxing Day 2017-12-26 - \ No newline at end of file + diff --git a/http-verbs-test-common/src/test/scala/uk/gov/hmrc/http/examples/Examples.scala b/http-verbs-test-common/src/test/scala/uk/gov/hmrc/http/examples/Examples.scala index 7020a5a1..82d1c3b6 100644 --- a/http-verbs-test-common/src/test/scala/uk/gov/hmrc/http/examples/Examples.scala +++ b/http-verbs-test-common/src/test/scala/uk/gov/hmrc/http/examples/Examples.scala @@ -206,7 +206,7 @@ class Examples ) val bankHolidays: BankHolidays = httpClient.GET[BankHolidays](url"$wireMockUrl/bank-holidays.json").futureValue - bankHolidays.events.head shouldBe BankHoliday("New Year’s Day", LocalDate.of(2017, 1, 2)) + bankHolidays.events.head shouldBe BankHoliday("New Year's Day", LocalDate.of(2017, 1, 2)) } "read some json and return a raw http response" in { @@ -241,7 +241,7 @@ class Examples httpClient.GET[Option[BankHolidays]](url"$wireMockUrl/401.json") .recover { - case Upstream4xxResponse(message, upstreamResponseCode, reportAs, headers) => // handle here 4xx errors + case UpstreamErrorResponse.Upstream4xxResponse(e) => // handle here 4xx errors }.futureValue } @@ -255,7 +255,7 @@ class Examples httpClient.GET[Option[BankHolidays]](url"$wireMockUrl/500.json") .recover { - case Upstream5xxResponse(message, upstreamResponseCode, reportAs, headers) => // handle here 5xx errors + case UpstreamErrorResponse.Upstream5xxResponse(e) => // handle here 5xx errors }.futureValue } } @@ -306,6 +306,7 @@ class Examples case Success(data) => Some(data) case Failure(e) => throw new CustomException("Unable to parse response") } + case other => throw new CustomException(s"Unexpected status code $other") } "Return some data when getting a 200 back" in { @@ -317,7 +318,7 @@ class Examples val bankHolidays = httpClient.GET[HttpResponse](url"$wireMockUrl/bank-holidays.xml") .map(responseHandler).futureValue - bankHolidays.get.events.head shouldBe BankHoliday("New Year’s Day", LocalDate.of(2017, 1, 2)) + bankHolidays.get.events.head shouldBe BankHoliday("New Year's Day", LocalDate.of(2017, 1, 2)) } "Fail when the response payload cannot be deserialised" in { @@ -353,7 +354,7 @@ class Examples ) val bankHolidays = httpClient.GET[Option[BankHolidays]](url"$wireMockUrl/bank-holidays.json").futureValue - bankHolidays.get.events.head shouldBe BankHoliday("New Year’s Day", LocalDate.of(2017, 1, 2)) + bankHolidays.get.events.head shouldBe BankHoliday("New Year's Day", LocalDate.of(2017, 1, 2)) } "Fail when the response payload cannot be deserialised" in { diff --git a/project/AppDependencies.scala b/project/AppDependencies.scala index 1ac0ba08..cd4eccfc 100644 --- a/project/AppDependencies.scala +++ b/project/AppDependencies.scala @@ -2,7 +2,7 @@ import sbt._ object AppDependencies { - val play28Version = "2.8.8" + val play28Version = "2.8.15" // Dependencies for http-verbs-common and http-verbs-play-xxx modules def coreCompileCommon(scalaVersion: String) = Seq( @@ -18,7 +18,7 @@ object AppDependencies { }) val coreCompilePlay28 = Seq( - "com.typesafe.play" %% "play-json" % "2.8.1", + "com.typesafe.play" %% "play-json" % "2.8.2", // version provided by play28Version "com.typesafe.play" %% "play-ahc-ws" % play28Version ) diff --git a/project/build.properties b/project/build.properties index 7d87f0b1..8a847661 100644 --- a/project/build.properties +++ b/project/build.properties @@ -14,4 +14,4 @@ # limitations under the License. # -sbt.version=1.4.9 +sbt.version=1.5.8