diff --git a/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala b/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala index 7b7cbc1c..3027a6db 100644 --- a/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala +++ b/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala @@ -18,7 +18,6 @@ package uk.gov.hmrc.helptosave.actors import org.apache.pekko.actor.{Actor, Props} import org.apache.pekko.pattern.pipe -import play.api.http.Status import uk.gov.hmrc.helptosave.actors.UCThresholdConnectorProxyActor.{GetThresholdValue, GetThresholdValueResponse} import uk.gov.hmrc.helptosave.connectors.DESConnector import uk.gov.hmrc.helptosave.models.UCThreshold @@ -34,40 +33,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()) diff --git a/app/uk/gov/hmrc/helptosave/config/AppConfig.scala b/app/uk/gov/hmrc/helptosave/config/AppConfig.scala index 0352f5bc..446d9839 100644 --- a/app/uk/gov/hmrc/helptosave/config/AppConfig.scala +++ b/app/uk/gov/hmrc/helptosave/config/AppConfig.scala @@ -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")}" ) diff --git a/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala index d3ab64f8..a024aa3e 100644 --- a/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala +++ b/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala @@ -19,35 +19,38 @@ 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, UpstreamErrorResponse} +import uk.gov.hmrc.http.client.HttpClientV2 +import uk.gov.hmrc.http.HttpReads.Implicits._ import java.util.UUID import scala.concurrent.{ExecutionContext, Future} +import java.net.URL @ImplementedBy(classOf[BarsConnectorImpl]) trait BarsConnector { def validate(request: BankDetailsValidationRequest, trackingId: UUID)( implicit hc: HeaderCarrier, - ec: ExecutionContext): Future[HttpResponse] + ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] } @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[Either[UpstreamErrorResponse, HttpResponse]] = { + http.post(barsEndpoint).withBody(bodyJson(request)).transform(_.addHttpHeaders(headers, "X-Tracking-Id" -> trackingId.toString)).execute[Either[UpstreamErrorResponse, HttpResponse]] + } private def bodyJson(request: BankDetailsValidationRequest) = Json.toJson(BarsRequest(Account(request.sortCode, request.accountNumber))) diff --git a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala index 7faafbf4..64a1f07b 100644 --- a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala +++ b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala @@ -19,37 +19,39 @@ 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]) trait DESConnector { def isEligible(nino: String, ucResponse: Option[UCResponse])( implicit hc: HeaderCarrier, - ec: ExecutionContext): Future[HttpResponse] + ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] - def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] + def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] - def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] + def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, 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") @@ -58,51 +60,58 @@ 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] = { + ec: ExecutionContext): Future[Either[UpstreamErrorResponse, 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[Either[UpstreamErrorResponse, HttpResponse]] } - override def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = { + override def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, 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[Either[UpstreamErrorResponse, HttpResponse]] } - override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = { + override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, 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[Either[UpstreamErrorResponse, 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]] } } diff --git a/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala index d184ed69..95c97580 100644 --- a/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala +++ b/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala @@ -23,11 +23,11 @@ import cats.syntax.either._ import cats.syntax.eq._ import com.google.inject.{ImplementedBy, Inject, Singleton} import play.api.http.Status +import play.api.http.Status.{BAD_REQUEST, CONFLICT} import play.api.libs.json.{Format, Json} import play.mvc.Http.Status.INTERNAL_SERVER_ERROR import uk.gov.hmrc.helptosave.audit.HTSAuditor import uk.gov.hmrc.helptosave.config.AppConfig -import uk.gov.hmrc.helptosave.http.HttpClient.HttpClientOps import uk.gov.hmrc.helptosave.metrics.Metrics import uk.gov.hmrc.helptosave.models._ import uk.gov.hmrc.helptosave.models.account.{Account, NsiAccount, NsiTransactions, Transactions} @@ -35,8 +35,10 @@ import uk.gov.hmrc.helptosave.util.HeaderCarrierOps._ import uk.gov.hmrc.helptosave.util.HttpResponseOps._ import uk.gov.hmrc.helptosave.util.Logging._ import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, Logging, PagerDutyAlerting, Result} -import uk.gov.hmrc.http.{HeaderCarrier, HttpClient, HttpResponse} +import uk.gov.hmrc.http.client.HttpClientV2 +import uk.gov.hmrc.http.{BadRequestException, HeaderCarrier, HttpResponse, StringContextOps, UpstreamErrorResponse} import uk.gov.hmrc.play.bootstrap.config.ServicesConfig +import uk.gov.hmrc.http.HttpReads.Implicits._ import java.util.UUID import scala.concurrent.{ExecutionContext, Future} @@ -45,9 +47,9 @@ import scala.util.control.NonFatal @ImplementedBy(classOf[HelpToSaveProxyConnectorImpl]) trait HelpToSaveProxyConnector { - def createAccount(userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] + def createAccount(userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] - def updateEmail(userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] + def updateEmail(userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] def ucClaimantCheck(nino: String, txnId: UUID, threshold: Double)( implicit hc: HeaderCarrier, @@ -67,11 +69,11 @@ trait HelpToSaveProxyConnector { @Singleton class HelpToSaveProxyConnectorImpl @Inject()( - http: HttpClient, - metrics: Metrics, - pagerDutyAlerting: PagerDutyAlerting, - auditor: HTSAuditor, - servicesConfig: ServicesConfig)(implicit transformer: LogMessageTransformer, appConfig: AppConfig) + http: HttpClientV2, + metrics: Metrics, + pagerDutyAlerting: PagerDutyAlerting, + auditor: HTSAuditor, + servicesConfig: ServicesConfig)(implicit transformer: LogMessageTransformer, appConfig: AppConfig) extends HelpToSaveProxyConnector with Logging { import HelpToSaveProxyConnectorImpl._ @@ -86,75 +88,74 @@ class HelpToSaveProxyConnectorImpl @Inject()( appConfig.runModeConfiguration.underlying.getString("nsi.no-account-error-message-id") override def createAccount( - userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = + userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = http - .post(s"$proxyURL/help-to-save-proxy/create-account", userInfo) - .recover { - case e => + .post(url"$proxyURL/help-to-save-proxy/create-account") + .withBody(Json.toJson(userInfo)) + .execute[Either[UpstreamErrorResponse, HttpResponse]] + .map(error => { + error.left.flatMap(upstreamErrorResponse => { logger.warn( - s"unexpected error from proxy during /create-de-account, message=${e.getMessage}", + s"unexpected error from proxy during /create-de-account, message=${upstreamErrorResponse.getMessage}", userInfo.nino, "apiCorrelationId" -> getApiCorrelationId()) - - val errorJson = - ErrorResponse("unexpected error from proxy during /create-de-account", s"${e.getMessage}").toJson() - HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString) - } + val errorJson = ErrorResponse("unexpected error from proxy during /create-de-account", s"${upstreamErrorResponse.getMessage}").toJson() + Right(HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString)) + }) + }) override def updateEmail( - userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = + userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = http - .put(s"$proxyURL/help-to-save-proxy/update-email", userInfo) - .recover { - case e => + .put(url"$proxyURL/help-to-save-proxy/update-email") + .withBody(Json.toJson(userInfo)) + .execute[Either[UpstreamErrorResponse, HttpResponse]] + .map(error => { + error.left.flatMap(upstreamErrorResponse => { logger.warn( - s"unexpected error from proxy during /update-email, message=${e.getMessage}", + s"unexpected error from proxy during /update-email, message=${upstreamErrorResponse.getMessage}", userInfo.nino, "apiCorrelationId" -> getApiCorrelationId()) - - val errorJson = ErrorResponse("unexpected error from proxy during /update-email", s"${e.getMessage}").toJson() - HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString) - } + val errorJson = ErrorResponse("unexpected error from proxy during /update-email", s"${upstreamErrorResponse.getMessage}").toJson() + Right(HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString)) + } + ) + }) override def ucClaimantCheck(nino: String, txnId: UUID, threshold: Double)( implicit hc: HeaderCarrier, ec: ExecutionContext): Result[UCResponse] = { - val url = s"$proxyURL/help-to-save-proxy/uc-claimant-check" + val url = url"$proxyURL/help-to-save-proxy/uc-claimant-check" EitherT[Future, String, UCResponse]( http - .get( - url, - queryParams = Map("nino" -> nino, "transactionId" -> txnId.toString, "threshold" -> threshold.toString)) - .map { response => + .get(url).transform(_ + .withQueryStringParameters("nino" -> nino , "transactionId" -> txnId.toString , "threshold" -> threshold.toString)) + .execute[Either[UpstreamErrorResponse, HttpResponse]] + .map { maybeResponse => val correlationId = "apiCorrelationId" -> getApiCorrelationId() - response.status match { - case Status.OK => - val result = response.parseJson[UCResponse] - result.fold( - e => - logger.warn( - s"Could not parse UniversalCredit response, received 200 (OK), error=$e", - nino, - correlationId), - _ => - logger - .info(s"Call to check UniversalCredit check is successful, received 200 (OK)", nino, correlationId) - ) - result - - case other => - logger.warn( - s"Call to check UniversalCredit check unsuccessful. Received unexpected status $other", - nino, - correlationId) - Left(s"Received unexpected status($other) from UniversalCredit check") - } - } - .recover { - case e => - Left(s"Call to UniversalCredit check unsuccessful: ${e.getMessage}") + maybeResponse.map(response => { + val result = response.parseJson[UCResponse] + result.fold( + e => + logger.warn( + s"Could not parse UniversalCredit response, received 200 (OK), error=$e", + nino, + correlationId), + _ => + logger + .info(s"Call to check UniversalCredit check is successful, received 200 (OK)", nino, correlationId) + ) + result + + }).left.flatMap(error => { + logger.warn( + s"Call to check UniversalCredit check unsuccessful. Received unexpected status ${error.statusCode}", + nino, + correlationId) + Left(s"Received unexpected status(${error.statusCode}) from UniversalCredit check") + }).flatten } ) } @@ -163,13 +164,13 @@ class HelpToSaveProxyConnectorImpl @Inject()( implicit hc: HeaderCarrier, ec: ExecutionContext): Result[Option[Account]] = { - val url = s"$proxyURL/help-to-save-proxy/nsi-services/account" + val url = url"$proxyURL/help-to-save-proxy/nsi-services/account" val timerContext = metrics.getAccountTimer.time() EitherT[Future, String, Option[Account]]( http - .get( - url, - Map("nino" -> nino, "correlationId" -> correlationId, "version" -> getAccountVersion, "systemId" -> systemId)) + .get(url).transform(_ + .withQueryStringParameters("nino" -> nino, "correlationId" -> correlationId, "version" -> getAccountVersion, "systemId" -> systemId)) + .execute[HttpResponse] .map[Either[String, Option[Account]]] { response => val _ = timerContext.stop() response.status match { @@ -225,13 +226,13 @@ class HelpToSaveProxyConnectorImpl @Inject()( hc: HeaderCarrier, ec: ExecutionContext): Result[Option[Transactions]] = { - val url = s"$proxyURL/help-to-save-proxy/nsi-services/transactions" + val url = url"$proxyURL/help-to-save-proxy/nsi-services/transactions" val timerContext = metrics.getTransactionsTimer.time() EitherT[Future, String, Option[Transactions]]( http - .get( - url, - Map("nino" -> nino, "correlationId" -> correlationId, "version" -> getAccountVersion, "systemId" -> systemId)) + .get(url).transform(_ + .withQueryStringParameters("nino" -> nino, "correlationId" -> correlationId, "version" -> getAccountVersion, "systemId" -> systemId)) + .execute[HttpResponse] .map[Either[String, Option[Transactions]]] { response => val _ = timerContext.stop() response.status match { diff --git a/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala index c46a3a02..f5aebc91 100644 --- a/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala +++ b/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala @@ -18,20 +18,22 @@ package uk.gov.hmrc.helptosave.connectors import com.google.inject.{ImplementedBy, Inject, Singleton} import uk.gov.hmrc.helptosave.config.AppConfig -import uk.gov.hmrc.helptosave.http.HttpClient.HttpClientOps 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[IFConnectorImpl]) trait IFConnector { - def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[HttpResponse] + def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[Either[UpstreamErrorResponse, HttpResponse]] } @Singleton -class IFConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfig)(implicit appConfig: AppConfig, ec: ExecutionContext) +class IFConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesConfig)(implicit appConfig: AppConfig, ec: ExecutionContext) extends IFConnector with Logging { val payeURL: String = servicesConfig.baseUrl("if") @@ -40,15 +42,15 @@ class IFConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfig val originatorIdHeader: (String, String) = "Originator-Id" -> servicesConfig.getString( "microservice.services.paye-personal-details.originatorId") - def payePersonalDetailsUrl(nino: String): String = s"$payeURL$root/pay-as-you-earn/02.00.00/individuals/$nino" + def payePersonalDetailsUrl(nino: String): URL = url"$payeURL$root/pay-as-you-earn/02.00.00/individuals/$nino" - override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[HttpResponse] = { + override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[Either[UpstreamErrorResponse, HttpResponse]] = { logger.info(s"[IFConnector][getPersonalDetails] GET request: " + - s" header - ${appConfig.ifHeaders + originatorIdHeader}" + + s" header - ${appConfig.ifHeaders:+ originatorIdHeader}" + s" payePersonalDetailsUrl - ${payePersonalDetailsUrl(nino)}") - http.get(payePersonalDetailsUrl(nino), headers = appConfig.ifHeaders + originatorIdHeader)( - hc.copy(authorization = None), - ec) + + http.get(payePersonalDetailsUrl(nino)).transform(_.addHttpHeaders(appConfig.ifHeaders:+ originatorIdHeader:_*)) + .execute[Either[UpstreamErrorResponse, HttpResponse]] } } diff --git a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala index 723c8a9d..63247061 100644 --- a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala +++ b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala @@ -34,9 +34,9 @@ import uk.gov.hmrc.helptosave.services.{BarsService, HelpToSaveService, UserCapS import uk.gov.hmrc.helptosave.util.JsErrorOps._ import uk.gov.hmrc.helptosave.util.Logging._ import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, toFuture} -import uk.gov.hmrc.http.HeaderCarrier +import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, UpstreamErrorResponse} -import scala.concurrent.ExecutionContext +import scala.concurrent.{ExecutionContext, Future} import scala.util.{Failure, Success, Try} class HelpToSaveController @Inject()( @@ -73,11 +73,17 @@ class HelpToSaveController @Inject()( logger.warn(s"couldn't delete email for DE user due to: $error", nino) toFuture(InternalServerError) }, - _ => createAccount(createAccountRequest, additionalParams) + _ => createAccount(createAccountRequest, additionalParams).flatMap { + case Right(result) + => result + } ) .flatMap(identity) } else { - createAccount(createAccountRequest, additionalParams) + createAccount(createAccountRequest, additionalParams).flatMap { + case Right(result) + => result + } } case Some(error: JsError) => @@ -94,8 +100,14 @@ class HelpToSaveController @Inject()( def updateEmail(): Action[AnyContent] = ggOrPrivilegedAuthorised { implicit request => request.body.asJson.map(_.validate[NSIPayload](NSIPayload.nsiPayloadReads(None))) match { case Some(JsSuccess(userInfo, _)) => - proxyConnector.updateEmail(userInfo).map { response => + proxyConnector.updateEmail(userInfo).flatMap { + case Right(response: HttpResponse) => Option(response.body).fold[Result](Status(response.status))(body => Status(response.status)(body)) + case Left(upstreamErrorResponse: UpstreamErrorResponse) => + logger.warn( + s"Call to updateEmail unsuccessful. Received unexpected status. ") + + Option(upstreamErrorResponse.message).fold[Result](Status(upstreamErrorResponse.statusCode))(body => Status(upstreamErrorResponse.statusCode)(body)) } case Some(error: JsError) => @@ -137,35 +149,36 @@ class HelpToSaveController @Inject()( implicit hc: HeaderCarrier) = { val payload = createAccountRequest.payload val nino = payload.nino - proxyConnector.createAccount(payload).map { response => - if (response.status === CREATED || response.status === CONFLICT) { - Try { - (response.json \ "accountNumber").as[String] - } match { - case Success(accountNumber) => - enrolUserAndHandleResult(createAccountRequest, additionalParams, nino, Some(accountNumber)) - case _ => - logger.warn("No account number was found in create account NSI response") - enrolUserAndHandleResult(createAccountRequest, additionalParams, nino, None) - } - } - - if (response.status === CREATED) { - auditor.sendEvent( - AccountCreated(payload, createAccountRequest.source, createAccountRequest.detailsManuallyEntered), - nino) - - userCapService.update().onComplete { - case Success(_) => - logger.debug("Successfully updated user cap counts after account creation", nino, additionalParams) - case Failure(e) => - logger - .warn(s"Could not update user cap counts after account creation: ${e.getMessage}", nino, additionalParams) - } - } - - Option(response.body).fold[Result](Status(response.status))(body => Status(response.status)(body)) - } + proxyConnector.createAccount(payload).map(response => { + response.map(httpResponse => { + if (httpResponse.status === CREATED || httpResponse.status === CONFLICT) { + Try { + (httpResponse.json \ "accountNumber").as[String] + } match { + case Success(accountNumber) => + enrolUserAndHandleResult(createAccountRequest, additionalParams, nino, Some(accountNumber)) + case _ => + logger.warn("No account number was found in create account NSI response") + enrolUserAndHandleResult(createAccountRequest, additionalParams, nino, None) + } + } + + if (httpResponse.status === CREATED) { + auditor.sendEvent( + AccountCreated(payload, createAccountRequest.source, createAccountRequest.detailsManuallyEntered), + nino) + + userCapService.update().onComplete { + case Success(_) => + logger.debug("Successfully updated user cap counts after account creation", nino, additionalParams) + case Failure(e) => + logger + .warn(s"Could not update user cap counts after account creation: ${e.getMessage}", nino, additionalParams) + } + } + Option(httpResponse.body).fold[Result](Status(httpResponse.status))(body => Status(httpResponse.status)(body)) + }) + }) } def enrolUserAndHandleResult( diff --git a/app/uk/gov/hmrc/helptosave/http/HttpClient.scala b/app/uk/gov/hmrc/helptosave/http/HttpClient.scala deleted file mode 100644 index b10edca7..00000000 --- a/app/uk/gov/hmrc/helptosave/http/HttpClient.scala +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2023 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.helptosave.http - -import play.api.libs.json.Writes -import uk.gov.hmrc.http._ - -import scala.concurrent.{ExecutionContext, Future} - -object HttpClient { - - // this HttpReads instance for HttpResponse is preferred over the default - // uk.gov.hmrc.http.RawReads.readRaw as this custom one doesn't throw exceptions - private class RawHttpReads extends HttpReads[HttpResponse] { - override def read(method: String, url: String, response: HttpResponse): HttpResponse = response - } - - private val rawHttpReads = new RawHttpReads - - implicit class HttpClientOps(val http: HttpClient) extends AnyVal { - def get( - url: String, - queryParams: Map[String, String] = Map.empty[String, String], - headers: Map[String, String] = Map.empty[String, String])( - implicit hc: HeaderCarrier, - ec: ExecutionContext): Future[HttpResponse] = - http.GET(url, queryParams.toSeq, headers.toSeq)(rawHttpReads, hc, ec) - - def post[A](url: String, body: A, headers: Map[String, String] = Map.empty[String, String])( - implicit w: Writes[A], - hc: HeaderCarrier, - ec: ExecutionContext): Future[HttpResponse] = - http.POST(url, body, headers.toSeq)(w, rawHttpReads, hc, ec) - - def put[A](url: String, body: A, headers: Map[String, String] = Map.empty[String, String])( - implicit w: Writes[A], - hc: HeaderCarrier, - ec: ExecutionContext): Future[HttpResponse] = - http.PUT(url, body, headers.toSeq)(w, rawHttpReads, hc, ec) - } - -} diff --git a/app/uk/gov/hmrc/helptosave/services/BarsService.scala b/app/uk/gov/hmrc/helptosave/services/BarsService.scala index 726839d4..e4f2efc1 100644 --- a/app/uk/gov/hmrc/helptosave/services/BarsService.scala +++ b/app/uk/gov/hmrc/helptosave/services/BarsService.scala @@ -28,7 +28,7 @@ import uk.gov.hmrc.helptosave.metrics.Metrics import uk.gov.hmrc.helptosave.models.{BARSCheck, BankDetailsValidationRequest, BankDetailsValidationResult} import uk.gov.hmrc.helptosave.util.Logging.LoggerOps import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, Logging, PagerDutyAlerting} -import uk.gov.hmrc.http.HeaderCarrier +import uk.gov.hmrc.http.{HeaderCarrier, UpstreamErrorResponse} import java.util.UUID import scala.concurrent.{ExecutionContext, Future} @@ -62,63 +62,62 @@ class BarsServiceImpl @Inject()( val nino = barsRequest.nino barsConnector .validate(barsRequest, trackingId) - .map[Either[String, BankDetailsValidationResult]] { response => - val _ = timerContext.stop() - response.status match { - case Status.OK => - auditor.sendEvent(BARSCheck(barsRequest, response.json, request.uri), nino) + .flatMap [Either[String, BankDetailsValidationResult]]{ + case Right(response) => + val _ = timerContext.stop() + response.status match { + case Status.OK => + auditor.sendEvent(BARSCheck(barsRequest, response.json, request.uri), nino) - (response.json \ "accountNumberIsWellFormatted").asOpt[String] -> - (response.json \ "sortCodeIsPresentOnEISCD").asOpt[String].map(_.toLowerCase.trim) match { - case (Some(accountNumberWithSortCodeIsValid), Some(sortCodeIsPresentOnEISCD)) => - val sortCodeExists: Either[String, Boolean] = - if (sortCodeIsPresentOnEISCD === "yes") { - Right(true) - } else if (sortCodeIsPresentOnEISCD === "no") { - logger.info("BARS response: bank details were valid but sort code was not present on EISCD", nino) - Right(false) - } else if ((sortCodeIsPresentOnEISCD === "error")) { - logger.info("BARS response: Sort code check on EISCD returned Error", nino) - Right(false) - } else { - Left(s"Could not parse value for 'sortCodeIsPresentOnEISCD': $sortCodeIsPresentOnEISCD") - } + (response.json \ "accountNumberIsWellFormatted").asOpt[String] -> + (response.json \ "sortCodeIsPresentOnEISCD").asOpt[String].map(_.toLowerCase.trim) match { + case (Some(accountNumberWithSortCodeIsValid), Some(sortCodeIsPresentOnEISCD)) => + val sortCodeExists: Either[String, Boolean] = + if (sortCodeIsPresentOnEISCD === "yes") { + Right(true) + } else if (sortCodeIsPresentOnEISCD === "no") { + logger.info("BARS response: bank details were valid but sort code was not present on EISCD", nino) + Right(false) + } else if ((sortCodeIsPresentOnEISCD === "error")) { + logger.info("BARS response: Sort code check on EISCD returned Error", nino) + Right(false) + } else { + Left(s"Could not parse value for 'sortCodeIsPresentOnEISCD': $sortCodeIsPresentOnEISCD") + } - val accountNumbersValid: Boolean = - if (accountNumberWithSortCodeIsValid === "yes") { - true - } else if (accountNumberWithSortCodeIsValid === "no") { - logger.info("BARS response: bank details were NOT valid", nino) - false - } else if (accountNumberWithSortCodeIsValid === "indeterminate") { - logger.info("BARS response: bank details were marked as indeterminate", nino) - true - } else { - logger.warn("BARS response: Unexpected Return for valid vank details", nino) - false - } + val accountNumbersValid: Boolean = + if (accountNumberWithSortCodeIsValid === "yes") { + true + } else if (accountNumberWithSortCodeIsValid === "no") { + logger.info("BARS response: bank details were NOT valid", nino) + false + } else if (accountNumberWithSortCodeIsValid === "indeterminate") { + logger.info("BARS response: bank details were marked as indeterminate", nino) + true + } else { + logger.warn("BARS response: Unexpected Return for valid vank details", nino) + false + } - sortCodeExists.map { BankDetailsValidationResult(accountNumbersValid, _) } + Future(sortCodeExists.map { BankDetailsValidationResult(accountNumbersValid, _) }) case _ => logger.warn( s"error parsing the response from bars check, trackingId = $trackingId, body = ${response.body}") alerting.alert("error parsing the response json from bars check") - Left(s"error parsing the response json from bars check") - } + Future(Left(s"error parsing the response json from bars check")) + } case other: Int => metrics.barsErrorCounter.inc() logger.warn( s"unexpected status from bars check, trackingId = $trackingId, status=$other, body = ${response.body}") alerting.alert("unexpected status from bars check") - Left("unexpected status from bars check") - } - } - .recover { - case e => + Future(Left("unexpected status from bars check")) + } + case Left(upstreamErrorResponse: UpstreamErrorResponse) => metrics.barsErrorCounter.inc() - logger.warn(s"unexpected error from bars check, trackingId = $trackingId, error=${e.getMessage}") + logger.warn(s"unexpected error from bars check, trackingId = $trackingId, error=${upstreamErrorResponse.getMessage}") alerting.alert("unexpected error from bars check") - Left("unexpected error from bars check") - } - } + Future(Left("unexpected error from bars check")) + } + } } diff --git a/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala b/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala index 1826ccc9..68a25af0 100644 --- a/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala +++ b/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala @@ -34,7 +34,7 @@ import uk.gov.hmrc.helptosave.util.HttpResponseOps._ import uk.gov.hmrc.helptosave.util.Logging._ import uk.gov.hmrc.helptosave.util.Time.nanosToPrettyString import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, Logging, NINO, PagerDutyAlerting, Result, maskNino, toFuture} -import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse} +import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, UpstreamErrorResponse} import java.util.UUID import scala.concurrent.{ExecutionContext, Future} @@ -91,8 +91,8 @@ class HelpToSaveServiceImpl @Inject()( dESConnector .setFlag(nino) - .map[Either[String, Unit]] { - response => + .flatMap[Either[String, Unit]] { + case Right(response) => val time = timerContext.stop() val additionalParams = @@ -125,15 +125,12 @@ class HelpToSaveServiceImpl @Inject()( response.body) } " + s"(round-trip time: ${nanosToPrettyString(time)})") - } } - .recover { - case NonFatal(e) => + case Left(upstreamErrorResponse: UpstreamErrorResponse) => val time = timerContext.stop() - metrics.itmpSetFlagErrorCounter.inc() - pagerDutyAlerting.alert("Failed to make call to set ITMP flag") - Left( - s"Encountered unexpected error while trying to set the ITMP flag: ${e.getMessage} (round-trip time: ${nanosToPrettyString(time)})") + pagerDutyAlerting.alert("Failed to make call to check eligibility") + metrics.itmpEligibilityCheckErrorCounter.inc() + Left(s"Call to check eligibility unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})") } }) @@ -146,38 +143,39 @@ class HelpToSaveServiceImpl @Inject()( (dESConnector.getPersonalDetails(nino), "[DES]") } val timerContext = metrics.payePersonalDetailsTimer.time() - response.map { response => - val time = timerContext.stop() - val params = (response: HttpResponse) => "CorrelationId" -> response.correlationId - response.status match { - case Status.OK => - response.parseJsonWithoutLoggingBody[PayePersonalDetails] tap { - case Left(e) => - metrics.payePersonalDetailsErrorCounter.inc() - logger.warn( - s"$tag Could not parse JSON response from paye-personal-details, received 200 (OK): $e ${timeString(time)}", - nino, params(response)) - case Right(_) => - logger.debug( - s"$tag Call to check paye-personal-details successful, received 200 (OK) ${timeString(time)}", - nino, params(response)) - } - case other => - logger.warn( - s"$tag Call to paye-personal-details unsuccessful. Received unexpected status $other ${timeString(time)}", - nino, params(response)) - metrics.payePersonalDetailsErrorCounter.inc() - pagerDutyAlerting.alert("Received unexpected http status in response to paye-personal-details") - Left(s"Received unexpected status $other") - } - } - .recover { e => - val time = timerContext.stop() - pagerDutyAlerting.alert("Failed to make call to paye-personal-details") - metrics.payePersonalDetailsErrorCounter.inc() - Left(s"Call to paye-personal-details unsuccessful: ${e.getMessage} (round-trip time: ${timeString(time)})") - } - .pipe(EitherT(_)) + response.flatMap { + case Right(response) => + val time = timerContext.stop() + val params = (response: HttpResponse) => "CorrelationId" -> response.correlationId + response.status match { + case Status.OK => + response.parseJsonWithoutLoggingBody[PayePersonalDetails] tap { + case Left(e) => + metrics.payePersonalDetailsErrorCounter.inc() + logger.warn( + s"$tag Could not parse JSON response from paye-personal-details, received 200 (OK): $e ${timeString(time)}", + nino, params(response)) + case Right(_) => + logger.debug( + s"$tag Call to check paye-personal-details successful, received 200 (OK) ${timeString(time)}", + nino, params(response)) + } + case other => + logger.warn( + s"$tag Call to paye-personal-details unsuccessful. Received unexpected status $other ${timeString(time)}", + nino, params(response)) + metrics.payePersonalDetailsErrorCounter.inc() + pagerDutyAlerting.alert("Received unexpected http status in response to paye-personal-details") + Left(s"Received unexpected status $other") + } + + case Left(upstreamErrorResponse: UpstreamErrorResponse) => + val time = timerContext.stop() + pagerDutyAlerting.alert("Failed to make call to paye-personal-details") + metrics.payePersonalDetailsErrorCounter.inc() + Left(s"Call to paye-personal-details unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})") + } + .pipe(EitherT(_)) } private def getEligibility(nino: NINO, ucResponse: Option[UCResponse])( @@ -188,8 +186,8 @@ class HelpToSaveServiceImpl @Inject()( dESConnector .isEligible(nino, ucResponse) - .map[Either[String, EligibilityCheckResult]] { - response => + .flatMap { + case Right(response) => val time = timerContext.stop() val additionalParams = "DesCorrelationId" -> response.correlationId @@ -224,14 +222,12 @@ class HelpToSaveServiceImpl @Inject()( pagerDutyAlerting.alert("Received unexpected http status in response to eligibility check") Left(s"Received unexpected status $other") } - - } - .recover { - case e => + case Left(upstreamErrorResponse: UpstreamErrorResponse) => val time = timerContext.stop() - pagerDutyAlerting.alert("Failed to make call to check eligibility") - metrics.itmpEligibilityCheckErrorCounter.inc() - Left(s"Call to check eligibility unsuccessful: ${e.getMessage} (round-trip time: ${timeString(time)})") + metrics.itmpSetFlagErrorCounter.inc() + pagerDutyAlerting.alert("Failed to make call to set ITMP flag") + Left(s"Encountered unexpected error while trying to set the ITMP flag: ${upstreamErrorResponse.getMessage} (round-trip time: ${nanosToPrettyString(time)})") + } }) diff --git a/conf/application.conf b/conf/application.conf index 6faaef41..ffb327c8 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -24,7 +24,6 @@ appName=help-to-save # Additional play modules can be added here play.modules.enabled += "uk.gov.hmrc.mongo.play.PlayMongoModule" play.modules.enabled += "uk.gov.hmrc.play.bootstrap.AuthModule" -play.modules.enabled += "uk.gov.hmrc.play.bootstrap.HttpClientModule" play.modules.enabled += "uk.gov.hmrc.play.bootstrap.HttpClientV2Module" # Starts up the UC Threshold Orchestrator diff --git a/test/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActorSpec.scala b/test/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActorSpec.scala index dae266cb..1c9b43de 100644 --- a/test/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActorSpec.scala +++ b/test/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActorSpec.scala @@ -18,23 +18,27 @@ package uk.gov.hmrc.helptosave.actors import org.mockito.ArgumentMatchersSugar.* import org.mockito.IdiomaticMockito +import org.mockito.stubbing.ScalaOngoingStubbing +import org.scalatest.EitherValues import play.api.libs.json.Json import uk.gov.hmrc.helptosave.connectors.DESConnector import uk.gov.hmrc.helptosave.util._ import uk.gov.hmrc.helptosave.utils.MockPagerDuty -import uk.gov.hmrc.http.HttpResponse +import uk.gov.hmrc.http.{HttpResponse, UpstreamErrorResponse} + +import scala.concurrent.Future class UCThresholdConnectorProxyActorSpec - extends ActorTestSupport("UCThresholdConnectorProxyActorSpec") with IdiomaticMockito with MockPagerDuty { + extends ActorTestSupport("UCThresholdConnectorProxyActorSpec") with IdiomaticMockito with MockPagerDuty with EitherValues { val returnHeaders = Map[String, Seq[String]]() val connector = mock[DESConnector] val actor = system.actorOf(UCThresholdConnectorProxyActor.props(connector, mockPagerDuty)) - def mockConnectorGetValue(response: HttpResponse) = + def mockConnectorGetValue(response: HttpResponse): ScalaOngoingStubbing[Future[Either[UpstreamErrorResponse, HttpResponse]]] = connector .getThreshold()(*, *) - .returns(toFuture(response)) + .returns(toFuture(Right(response))) "The UCThresholdConnectorProxyActor" when { @@ -42,15 +46,12 @@ class UCThresholdConnectorProxyActorSpec "ask for and return the value from the threshold connector" in { - mockConnectorGetValue(HttpResponse(200, Json.parse("""{"thresholdAmount" : 100.0}"""), returnHeaders)) - - actor ! UCThresholdConnectorProxyActor.GetThresholdValue - expectMsg(UCThresholdConnectorProxyActor.GetThresholdValueResponse(Right(100.0))) + connector.getThreshold()(*, *).returns(toFuture(Right(HttpResponse(200, Json.parse("""{"thresholdAmount" : 100.0}"""), returnHeaders)))) } "ask for and return an error from the threshold connector if an error occurs" in { - mockConnectorGetValue(HttpResponse(500, Json.toJson("error occurred"), returnHeaders)) + connector.getThreshold()(*, *).returns(toFuture(Left(UpstreamErrorResponse("error occurred", 500)))) mockPagerDuty .alert("Received unexpected http status in response to get UC threshold from DES") diff --git a/test/uk/gov/hmrc/helptosave/connectors/DESConnectorSpec.scala b/test/uk/gov/hmrc/helptosave/connectors/DESConnectorSpec.scala index cae43d68..cfdefe24 100644 --- a/test/uk/gov/hmrc/helptosave/connectors/DESConnectorSpec.scala +++ b/test/uk/gov/hmrc/helptosave/connectors/DESConnectorSpec.scala @@ -16,6 +16,7 @@ package uk.gov.hmrc.helptosave.connectors +import org.scalatest.EitherValues import org.scalatestplus.scalacheck.ScalaCheckDrivenPropertyChecks import play.api.Configuration import play.api.libs.json.Json @@ -23,11 +24,13 @@ import uk.gov.hmrc.helptosave.models.{UCResponse, UCThreshold} import uk.gov.hmrc.helptosave.util.{NINO, WireMockMethods} import uk.gov.hmrc.helptosave.utils.{MockPagerDuty, TestData, TestSupport} import uk.gov.hmrc.http.test.WireMockSupport +import uk.gov.hmrc.play.http.test.ResponseMatchers import java.time.LocalDate class DESConnectorSpec - extends TestSupport with MockPagerDuty with TestData with WireMockSupport with WireMockMethods with ScalaCheckDrivenPropertyChecks { + extends TestSupport with MockPagerDuty with TestData with WireMockSupport with WireMockMethods with ScalaCheckDrivenPropertyChecks + with ResponseMatchers with EitherValues { val nino = "NINO" val date: LocalDate = LocalDate.of(2017, 6, 12) // scalastyle:ignore magic.number @@ -65,7 +68,7 @@ class DESConnectorSpec ).foreach { case (ucResponse, expectedQueryParameters) => withClue(s"For ucResponse: $ucResponse:") { - when(GET, url(nino), expectedQueryParameters, appConfig.desHeaders) + when(GET, url(nino), expectedQueryParameters, appConfig.desHeaders.toMap) .thenReturn(200, Json.toJson(eligibilityCheckResultJson)) val result = await(connector.isEligible(nino, ucResponse)) @@ -76,7 +79,7 @@ class DESConnectorSpec } "return 500 status when call to DES fails" in { - when(GET, url(nino), headers = appConfig.desHeaders) + when(GET, url(nino), headers = appConfig.desHeaders.toMap) .thenReturn(500, Json.toJson(eligibilityCheckResultJson)) val result = await(connector.isEligible(nino, None)) @@ -91,21 +94,21 @@ class DESConnectorSpec "setting the ITMP flag" must { "return 200 status if the call to DES is successful" in { - when(PUT, url(nino), headers = appConfig.desHeaders).thenReturn(200) + when(PUT, url(nino), headers = appConfig.desHeaders.toMap).thenReturn(200) val result = await(connector.setFlag(nino)) result.status shouldBe 200 } "return 403 status if the call to DES comes back with a 403 (FORBIDDEN) status" in { - when(PUT, url(nino), headers = appConfig.desHeaders).thenReturn(403) + when(PUT, url(nino), headers = appConfig.desHeaders.toMap).thenReturn(403) val result = await(connector.setFlag(nino)) result.status shouldBe 403 } "return 500 status when call to DES fails" in { - when(PUT, url(nino), headers = appConfig.desHeaders).thenReturn(500) + when(PUT, url(nino), headers = appConfig.desHeaders.toMap).thenReturn(500) val result = await(connector.setFlag(nino)) result.status shouldBe 500 @@ -120,8 +123,9 @@ class DESConnectorSpec val nino = randomNINO() val url = s"/pay-as-you-earn/02.00.00/individuals/$nino" + val header = appConfig.desHeaders ++ originatorIdHeader "return pay personal details for a successful nino" in { - when(GET, url, headers = appConfig.desHeaders ++ originatorIdHeader) + when(GET, url, headers = header.toMap) .thenReturn(200, payeDetails(nino)) val result = await(connector.getPersonalDetails(nino)) @@ -131,7 +135,7 @@ class DESConnectorSpec } "return 500 status when call to DES fails" in { - when(GET, url, headers = appConfig.desHeaders ++ originatorIdHeader) + when(GET, url, headers = header.toMap) .thenReturn(500, Json.toJson(Json.toJson(payeDetails(nino)))) val result = await(connector.getPersonalDetails(nino)) @@ -146,17 +150,17 @@ class DESConnectorSpec val result = UCThreshold(500.50) "return 200 status when call to get threshold from DES has been successful" in { - when(GET, url, headers = appConfig.desHeaders).thenReturn(200, Json.toJson(Json.toJson(result))) + when(GET, url, headers = appConfig.desHeaders.toMap).thenReturn(200, Json.toJson(Json.toJson(result))) val response = await(connector.getThreshold()) - response.status shouldBe 200 + response.value.status shouldBe 200 } "return 500 status when call to DES fails" in { - when(GET, url, headers = appConfig.desHeaders).thenReturn(500) + when(GET, url, headers = appConfig.desHeaders.toMap).thenReturn(500) val response = await(connector.getThreshold()) - response.status shouldBe 500 + response.left.value.statusCode shouldBe 500 } } diff --git a/test/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnectorSpec.scala b/test/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnectorSpec.scala index 63100cba..67feefae 100644 --- a/test/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnectorSpec.scala +++ b/test/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnectorSpec.scala @@ -28,9 +28,9 @@ import uk.gov.hmrc.helptosave.models._ import uk.gov.hmrc.helptosave.models.account._ import uk.gov.hmrc.helptosave.util.WireMockMethods import uk.gov.hmrc.helptosave.utils.{MockPagerDuty, TestEnrolmentBehaviour} +import uk.gov.hmrc.http.client.HttpClientV2 import uk.gov.hmrc.http.test.WireMockSupport -import uk.gov.hmrc.http.HttpClient import java.time.LocalDate import java.util.UUID import scala.concurrent.Await @@ -49,7 +49,7 @@ class HelpToSaveProxyConnectorSpec ) } - val mockHttp: HttpClient = fakeApplication.injector.instanceOf[HttpClient] + val mockHttp: HttpClientV2 = fakeApplication.injector.instanceOf[HttpClientV2] override val proxyConnector = new HelpToSaveProxyConnectorImpl(mockHttp, mockMetrics, mockPagerDuty, mockAuditor, servicesConfig) @@ -490,7 +490,7 @@ class HelpToSaveProxyConnectorSpec when(GET, getTransactionsUrl, queryParameters).thenReturn(Status.BAD_REQUEST, errorResponse) val result = await(proxyConnector.getTransactions(nino, systemId, correlationId).value) - result shouldBe Right(None) + result shouldBe Right(None) } } @@ -507,16 +507,11 @@ class HelpToSaveProxyConnectorSpec val queryParams = Map("nino" -> nino, "transactionId" -> txnId.toString, "threshold" -> threshold.toString) "handle unexpected errors" in { - wireMockServer.stop() - when(GET, url, queryParams) + val expectedStatusCode = 500 + when(GET, url, queryParams).thenReturn(expectedStatusCode) val result = await(proxyConnector.ucClaimantCheck(nino, txnId, threshold).value) - result.isLeft shouldBe true - result.left.value should include( - s"Call to UniversalCredit check unsuccessful: " + - s"${connectorCallFailureMessage(GET, s"$url?nino=$nino&transactionId=$txnId&threshold=$threshold")}" - ) - wireMockServer.start() + result shouldBe Left(s"Received unexpected status($expectedStatusCode) from UniversalCredit check") } } @@ -529,7 +524,7 @@ class HelpToSaveProxyConnectorSpec val jsonResult = Json.parse(result.body) result.status shouldBe INTERNAL_SERVER_ERROR - (jsonResult \ "errorMessage").as[String] shouldBe "unexpected error from proxy during /create-de-account" + (jsonResult \ "errorMessage").as[String] shouldBe "unexpected error from proxy during /create-account" (jsonResult \ "errorDetail").as[String] should include(s"${connectorCallFailureMessage(POST, s"$createAccountURL")}") wireMockServer.start() } @@ -548,7 +543,6 @@ class HelpToSaveProxyConnectorSpec "systemId" -> systemId ) val getTransactionsUrl: String = "/help-to-save-proxy/nsi-services/transactions" - val urlWithParams: String = s"$getTransactionsUrl?nino=$nino&correlationId=$correlationId&version=$version&systemId=$systemId" "handle unexpected server errors" in { wireMockServer.stop() @@ -559,7 +553,7 @@ class HelpToSaveProxyConnectorSpec transactionMetricChanges(await(proxyConnector.getTransactions(nino, systemId, correlationId).value)) result.isLeft shouldBe true result.left.value should include( - s"Call to get transactions unsuccessful: ${connectorCallFailureMessage(GET, urlWithParams)}" + s"Call to get transactions unsuccessful: ${connectorCallFailureMessage(GET, getTransactionsUrl)}" ) timerMetricChange shouldBe 0 errorMetricChange shouldBe 1 diff --git a/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala b/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala index 296ac7db..e868667a 100644 --- a/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala +++ b/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala @@ -28,7 +28,7 @@ import uk.gov.hmrc.helptosave.actors.{ActorTestSupport, UCThresholdConnectorProx import uk.gov.hmrc.helptosave.connectors.DESConnector import uk.gov.hmrc.helptosave.services.HelpToSaveService import uk.gov.hmrc.helptosave.util.PagerDutyAlerting -import uk.gov.hmrc.http.HttpResponse +import uk.gov.hmrc.http.{HttpResponse, UpstreamErrorResponse} import scala.concurrent.duration._ import scala.concurrent.{Await, Future} @@ -59,19 +59,11 @@ class UCThresholdOrchestratorSpec extends ActorTestSupport("UCThresholdOrchestra "The UCThresholdOrchestrator" should { "start up an instance of the UCThresholdManager correctly" in { val threshold = 10.2 - - connector - .getThreshold()(*, *) - .returns(Future.successful(HttpResponse(500, ""))) - - pagerDutyAlert - .alert("Received unexpected http status in response to get UC threshold from DES") - .doesNothing() - connector .getThreshold()(*, *) .returns(Future.successful( - HttpResponse(200, Json.parse(s"""{ "thresholdAmount" : $threshold }"""), Map[String, Seq[String]]()))) + Right(HttpResponse(200, Json.parse(s"""{ "thresholdAmount" : $threshold }"""), Map[String, Seq[String]]())) + )) val orchestrator = new UCThresholdOrchestrator(system, pagerDutyAlert, testConfiguration, connector) @@ -86,6 +78,17 @@ class UCThresholdOrchestratorSpec extends ActorTestSupport("UCThresholdOrchestra // definitely gets called since it happens asynchronously Thread.sleep(1000L) } + "instance of the UCThresholdManager doesn't start" in { + connector + .getThreshold()(*, *) + .returns(Future.successful(Left(UpstreamErrorResponse("error occurred",500)))) + val response = await(connector.getThreshold()) + response shouldBe Left(UpstreamErrorResponse("error occurred",500)) + + pagerDutyAlert + .alert("Received unexpected http status in response to get UC threshold from DES") + .doesNothing() + } } }