Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DLS-10533] migrating from http client to http client v2 #287

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -34,40 +34,33 @@ class UCThresholdConnectorProxyActor(dESConnector: DESConnector, pagerDutyAlerti

implicit val hc: HeaderCarrier = HeaderCarrier()

def getThreshold()(implicit hc: HeaderCarrier): Future[Either[String, Double]] =
dESConnector
.getThreshold()
.map[Either[String, Double]] { response =>
val additionalParams = "DesCorrelationId" -> response.correlationId

response.status match {
case Status.OK =>
val result = response.parseJson[UCThreshold]
result.fold(
{ e =>
logger.warn(
s"Could not parse JSON response from threshold, received 200 (OK): $e, with additionalParams: $additionalParams")
pagerDutyAlerting.alert("Could not parse JSON in UC threshold response")
},
_ =>
logger.debug(
s"Call to threshold successful, received 200 (OK), with additionalParams: $additionalParams")
)
result.map(_.thresholdAmount)

case other =>
logger.warn(
s"Call to get threshold unsuccessful. Received unexpected status $other. " +
s"AdditionalParams: $additionalParams")
pagerDutyAlerting.alert("Received unexpected http status in response to get UC threshold from DES")
Left(s"Received unexpected status $other")
}
}
.recover {
case e =>
pagerDutyAlerting.alert("Failed to make call to get UC threshold from DES")
Left(s"Call to get threshold unsuccessful: ${e.getMessage}")
}
def getThreshold()(implicit hc: HeaderCarrier): Future[Either[String, Double]] = {
dESConnector.getThreshold()
.map(maybeThreshold => {
maybeThreshold.map(response => {
val additionalParams = "DesCorrelationId" -> response.correlationId
val result = response.parseJson[UCThreshold]
result.fold(
{ e =>
logger.warn(
s"Could not parse JSON response from threshold, received 200 (OK): $e, with additionalParams: $additionalParams")
pagerDutyAlerting.alert("Could not parse JSON in UC threshold response")
},
_ =>
logger.debug(
s"Call to threshold successful, received 200 (OK), with additionalParams: $additionalParams")
)
result.map(_.thresholdAmount)
}).left.map(error => {
val additionalParams = "DesCorrelationId" -> error.headers.getOrElse("CorrelationId", "-")
logger.warn(
s"Call to get threshold unsuccessful. Received unexpected status ${error.statusCode}. " +
s"AdditionalParams: $additionalParams")
pagerDutyAlerting.alert("Received unexpected http status in response to get UC threshold from DES")
s"Received unexpected status ${error.statusCode}"
})
}.flatten)
}

override def receive: Receive = {
case GetThresholdValue => getThreshold().map(GetThresholdValueResponse).pipeTo(sender())
Expand Down
4 changes: 2 additions & 2 deletions app/uk/gov/hmrc/helptosave/config/AppConfig.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ class AppConfig @Inject()(

val appName: String = servicesConfig.getString("appName")
val ifEnabled: Boolean = servicesConfig.getBoolean("feature.if.enabled")
val desHeaders: Map[String, String] = Map(
val desHeaders: Seq[(String, String)] = Seq(
"Environment" -> servicesConfig.getString("microservice.services.des.environment"),
"Authorization" -> s"Bearer ${servicesConfig.getString("microservice.services.des.token")}"
)

val ifHeaders: Map[String, String] = Map(
val ifHeaders: Seq[(String, String)] = Seq(
"Environment" -> servicesConfig.getString("microservice.services.if.environment"),
"Authorization" -> s"Bearer ${servicesConfig.getString("microservice.services.if.token")}"
)
Expand Down
16 changes: 9 additions & 7 deletions app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ package uk.gov.hmrc.helptosave.connectors
import com.google.inject.{ImplementedBy, Inject, Singleton}
import play.api.libs.json.{Format, Json}
import uk.gov.hmrc.helptosave.config.AppConfig
import uk.gov.hmrc.helptosave.http.HttpClient._
import uk.gov.hmrc.helptosave.models.BankDetailsValidationRequest
import uk.gov.hmrc.helptosave.util.Logging
import uk.gov.hmrc.http.{HeaderCarrier, HttpClient, HttpResponse}
import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps}
import uk.gov.hmrc.http.client.HttpClientV2

import java.util.UUID
import scala.concurrent.{ExecutionContext, Future}
import java.net.URL

@ImplementedBy(classOf[BarsConnectorImpl])
trait BarsConnector {
Expand All @@ -36,18 +37,19 @@ trait BarsConnector {
}

@Singleton
class BarsConnectorImpl @Inject()(http: HttpClient)(implicit appConfig: AppConfig) extends BarsConnector with Logging {
class BarsConnectorImpl @Inject()(http: HttpClientV2)(implicit appConfig: AppConfig) extends BarsConnector with Logging {

import uk.gov.hmrc.helptosave.connectors.BarsConnectorImpl._

private val barsEndpoint: String = s"${appConfig.barsUrl}/validate/bank-details"
private val barsEndpoint: URL = url"${appConfig.barsUrl}/validate/bank-details"

private val headers = Map("Content-Type" -> "application/json")
private val headers:(String, String) = "Content-Type" -> "application/json"

override def validate(request: BankDetailsValidationRequest, trackingId: UUID)(
implicit hc: HeaderCarrier,
ec: ExecutionContext): Future[HttpResponse] =
http.post(barsEndpoint, bodyJson(request), headers.+("X-Tracking-Id" -> trackingId.toString))
ec: ExecutionContext): Future[HttpResponse] = {
http.post(barsEndpoint).withBody(bodyJson(request)).transform(_.addHttpHeaders(headers, "X-Tracking-Id" -> trackingId.toString)).execute[HttpResponse]
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, missed this in previous review.
Would it also make sense to capture the return type as Either[UpstreamErrorResponse, HttpResponse] here? This might need tests being tidied up to reflect the change

While doing that, could you also check on other connectors making sure of the return type and error handling in calling code.

}

private def bodyJson(request: BankDetailsValidationRequest) =
Json.toJson(BarsRequest(Account(request.sortCode, request.accountNumber)))
Expand Down
60 changes: 36 additions & 24 deletions app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ package uk.gov.hmrc.helptosave.connectors
import cats.Show
import cats.syntax.show._
import com.google.inject.{ImplementedBy, Inject, Singleton}
import play.api.libs.json.{JsNull, JsValue, Writes}
import play.api.libs.json.{JsNull, JsValue}
import uk.gov.hmrc.helptosave.config.AppConfig
import uk.gov.hmrc.helptosave.http.HttpClient.HttpClientOps
import uk.gov.hmrc.helptosave.models.UCResponse
import uk.gov.hmrc.helptosave.util.{Logging, NINO}
import uk.gov.hmrc.http.{HeaderCarrier, HttpClient, HttpResponse}
import uk.gov.hmrc.http.HttpReads.Implicits._
import uk.gov.hmrc.http.client.HttpClientV2
import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps, UpstreamErrorResponse}
import uk.gov.hmrc.play.bootstrap.config.ServicesConfig

import java.net.URL
import scala.concurrent.{ExecutionContext, Future}

@ImplementedBy(classOf[DESConnectorImpl])
Expand All @@ -39,17 +41,17 @@ trait DESConnector {

def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse]

def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse]
def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
}

@Singleton
class DESConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfig)(implicit appConfig: AppConfig)
class DESConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesConfig)(implicit appConfig: AppConfig)
extends DESConnector with Logging {

val itmpECBaseURL: String = servicesConfig.baseUrl("itmp-eligibility-check")
val itmpEnrolmentURL: String = servicesConfig.baseUrl("itmp-enrolment")
val payeURL: String = servicesConfig.baseUrl("paye-personal-details")
val itmpThresholdURL: String = s"${servicesConfig.baseUrl("itmp-threshold")}/universal-credits/threshold-amount"
val itmpThresholdURL: URL = url"${servicesConfig.baseUrl("itmp-threshold")}/universal-credits/threshold-amount"

implicit val booleanShow: Show[Boolean] = Show.show(if (_) "Y" else "N")

Expand All @@ -58,51 +60,61 @@ class DESConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfi
val originatorIdHeader: (String, String) = "Originator-Id" -> servicesConfig.getString(
"microservice.services.paye-personal-details.originatorId")

def eligibilityCheckUrl(nino: String): String = s"$itmpECBaseURL/help-to-save/eligibility-check/$nino"
def eligibilityCheckUrl(nino: String): URL = url"$itmpECBaseURL/help-to-save/eligibility-check/$nino"

def eligibilityCheckQueryParameters(ucResponse: Option[UCResponse]): Map[String, String] =
def eligibilityCheckQueryParameters(ucResponse: Option[UCResponse]) =
ucResponse match {
case Some(UCResponse(a, Some(b))) => Map("universalCreditClaimant" -> a.show, "withinThreshold" -> b.show)
case Some(UCResponse(a, None)) => Map("universalCreditClaimant" -> a.show)
case _ => Map()
case Some(UCResponse(a, Some(b))) => Seq("universalCreditClaimant" -> a.show, "withinThreshold" -> b.show)
case Some(UCResponse(a, None)) => Seq("universalCreditClaimant" -> a.show)
case _ => Seq()
}

def setFlagUrl(nino: NINO): String = s"$itmpEnrolmentURL/help-to-save/accounts/$nino"
def setFlagUrl(nino: NINO): URL = url"$itmpEnrolmentURL/help-to-save/accounts/$nino"

def payePersonalDetailsUrl(nino: String): String = s"$payeURL/pay-as-you-earn/02.00.00/individuals/$nino"
def payePersonalDetailsUrl(nino: String): URL = url"$payeURL/pay-as-you-earn/02.00.00/individuals/$nino"

override def isEligible(nino: String, ucResponse: Option[UCResponse] = None)(
implicit hc: HeaderCarrier,
ec: ExecutionContext): Future[HttpResponse] = {
logger.info(s"[DESConnector][isEligible] GET request: " +
s" header - ${appConfig.desHeaders}" +
s" eligibilityCheckUrl - ${eligibilityCheckUrl(nino)}")
http.get(eligibilityCheckUrl(nino), eligibilityCheckQueryParameters(ucResponse), appConfig.desHeaders)(
hc.copy(authorization = None),
ec)

http.get(eligibilityCheckUrl(nino))(hc.copy(authorization = None)).transform(_
.withQueryStringParameters(eligibilityCheckQueryParameters(ucResponse):_*)
.addHttpHeaders(appConfig.desHeaders:_*))
.execute[HttpResponse]
}

override def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = {
logger.info(s"[DESConnector][setFlag] PUT request: " +
s" header - ${appConfig.desHeaders} " +
s" setFlagUrl - ${setFlagUrl(nino)}")
http.put(setFlagUrl(nino), body, appConfig.desHeaders)(Writes.jsValueWrites, hc.copy(authorization = None), ec)
http.put(setFlagUrl(nino))(hc.copy(authorization = None)).transform(_
.addHttpHeaders(appConfig.desHeaders: _*))
.withBody(body)
.execute[HttpResponse]
}

override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = {
logger.info(s"[DESConnector][getPersonalDetails] GET request: " +
s" header - ${appConfig.desHeaders+ originatorIdHeader}" +
s" header - ${appConfig.desHeaders:+ originatorIdHeader}" +
s" payePersonalDetailsUrl - ${payePersonalDetailsUrl(nino)}")
http.get(payePersonalDetailsUrl(nino), headers = appConfig.desHeaders + originatorIdHeader)(
hc.copy(authorization = None),
ec)
http.get(payePersonalDetailsUrl(nino))(hc.copy(authorization = None)).transform(_
.addHttpHeaders(appConfig.desHeaders:+ originatorIdHeader:_*))
.execute[HttpResponse]
}

override def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = {
override def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = {
logger.info(s"[DESConnector][getThreshold] GET request: " +
s"itmpThresholdURL - $itmpThresholdURL" +
s" header - ${appConfig.desHeaders + originatorIdHeader}")
http.get(itmpThresholdURL, headers = appConfig.desHeaders)(hc.copy(authorization = None), ec)
s" header - ${appConfig.desHeaders:+ originatorIdHeader}")
http.get(itmpThresholdURL)(hc.copy(authorization = None)).transform(_
.addHttpHeaders(appConfig.desHeaders:_*))
.execute[Either[UpstreamErrorResponse, HttpResponse]]
.map(_.fold(_ => Left(UpstreamErrorResponse("error occurred",500)),
_ => Right(HttpResponse(200)))
)
}

}
Loading