diff --git a/README.md b/README.md index f8077db..dcad2c0 100644 --- a/README.md +++ b/README.md @@ -3,11 +3,13 @@ This service allows other HMRC services to send messages to external SOAP web services. It has a retry mechanism whereby if the CCN2 SOAP service doesn't return a 2xx response, the request will be retried every 60 seconds for 5 minutes by default assuming that the `retry.enabled` property in `application.conf` is set to `true`. The total duration and the interval are both configurable. The retry logic works as follows: -Initial request to CCN2 results in a 2xx response -> message goes into a sent state. -Initial request to CCN2 results in a non 2xx response -> message goes into a retrying state. -Subsequent request to CCN2 results in a non 2xx response and inside retry duration -> message retried. -Subsequent request to CCN2 results in a 2xx response and inside retry duration -> message goes into a sent state. -Subsequent request to CCN2 results in a non 2xx response and outside retry duration -> message goes into failed state. + * Initial request to CCN2 results in a 2xx response -> message goes into a sent state. + * Initial request to CCN2 results in a 5xx response -> message goes into a retrying state. + * Initial request to CCN2 results in a 1xx, 3xx or 4xx response -> message goes into failed state. + * Subsequent request to CCN2 results in a 1xx, 3xx or 4xx response and inside retry duration -> message goes into failed state. + * Subsequent request to CCN2 results in a 5xx response and inside retry duration -> message retried. + * Subsequent request to CCN2 results in a 2xx response and inside retry duration -> message goes into a sent state. + * Subsequent request to CCN2 results in a non 2xx response and outside retry duration -> message goes into failed state. ## `POST /message` Send a SOAP message for the given operation diff --git a/app/uk/gov/hmrc/apiplatformoutboundsoap/responses.scala b/app/uk/gov/hmrc/apiplatformoutboundsoap/responses.scala index 9122fa1..bcef6f6 100644 --- a/app/uk/gov/hmrc/apiplatformoutboundsoap/responses.scala +++ b/app/uk/gov/hmrc/apiplatformoutboundsoap/responses.scala @@ -26,6 +26,14 @@ object ErrorCode extends Enumeration { val INTERNAL_SERVER_ERROR = Value("INTERNAL_SERVER_ERROR") } +object CcnRequestResult extends Enumeration { + type CcnRequestResult = Value + val UNEXPECTED_SUCCESS = Value("UNEXPECTED_SUCCESS") + val SUCCESS = Value("SUCCESS") + val FAIL_ERROR = Value("FAIL_ERROR") + val RETRYABLE_ERROR = Value("RETRYABLE_ERROR") +} + object JsErrorResponse { def apply(errorCode: ErrorCode.Value, message: JsValueWrapper): JsObject = Json.obj( diff --git a/app/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundService.scala b/app/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundService.scala index 6ee0193..05fb637 100644 --- a/app/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundService.scala +++ b/app/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundService.scala @@ -26,12 +26,14 @@ import org.apache.axiom.soap.SOAPEnvelope import org.apache.axis2.addressing.AddressingConstants.Final.{WSAW_NAMESPACE, WSA_NAMESPACE} import org.apache.axis2.addressing.AddressingConstants._ import org.apache.axis2.wsdl.WSDLUtil -import org.apache.http.HttpStatus import org.joda.time.DateTime import org.joda.time.DateTimeZone.UTC import org.joda.time.format.{DateTimeFormatter, ISODateTimeFormat} import play.api.cache.AsyncCacheApi import play.api.Logging +import play.api.http.Status._ +import uk.gov.hmrc.apiplatformoutboundsoap.CcnRequestResult +import uk.gov.hmrc.apiplatformoutboundsoap.CcnRequestResult._ import uk.gov.hmrc.apiplatformoutboundsoap.config.AppConfig import uk.gov.hmrc.apiplatformoutboundsoap.connectors.{NotificationCallbackConnector, OutboundConnector} import uk.gov.hmrc.apiplatformoutboundsoap.models._ @@ -65,8 +67,9 @@ class OutboundService @Inject()(outboundConnector: OutboundConnector, def sendMessage(message: MessageRequest): Future[OutboundSoapMessage] = { for { soapRequest <- buildSoapRequest(message) - result <- outboundConnector.postMessage(message.addressing.messageId, soapRequest) - outboundSoapMessage = buildOutboundSoapMessage(message, soapRequest, result) + httpStatus <- outboundConnector.postMessage(message.addressing.messageId, soapRequest) + outboundSoapMessage = processSendingResult(message, soapRequest, httpStatus) + _ <- logCcnSendResult(outboundSoapMessage, httpStatus) _ <- outboundMessageRepository.persist(outboundSoapMessage) } yield outboundSoapMessage } @@ -79,45 +82,63 @@ class OutboundService @Inject()(outboundConnector: OutboundConnector, val nextRetryDateTime: DateTime = now.plus(appConfig.retryInterval.toMillis) val globalId = message.globalId val messageId = message.messageId + def updateStatusAndNotify(newStatus: SendingStatus)(implicit hc: HeaderCarrier) = { + outboundMessageRepository.updateSendingStatus(globalId, newStatus) map { updatedMessage => + updatedMessage.map(notificationCallbackConnector.sendNotification) + () + } + } + + def retryDurationExpired = { + message.createDateTime.plus(appConfig.retryDuration.toMillis).isBefore(now.getMillis) + } - outboundConnector.postMessage(messageId, SoapRequest(message.soapMessage, message.destinationUrl)) flatMap { result => - if (is2xx(result)) { - log2xxResult(result, globalId, messageId) - outboundMessageRepository.updateSendingStatus(message.globalId, SendingStatus.SENT) map { updatedMessage => - updatedMessage.map(notificationCallbackConnector.sendNotification) - () - } - } else { - if (message.createDateTime.plus(appConfig.retryDuration.toMillis).isBefore(now.getMillis)) { - logger.error(s"Retried message with global ID ${message.globalId} message ID ${message.messageId} got status code $result " + - s"and failed on last attempt") - outboundMessageRepository.updateSendingStatus(message.globalId, SendingStatus.FAILED).map { updatedMessage => - updatedMessage.map(notificationCallbackConnector.sendNotification) - () + outboundConnector.postMessage(messageId, SoapRequest(message.soapMessage, message.destinationUrl)) flatMap { httpStatus => + mapHttpStatusCode(httpStatus) match { + case SUCCESS => + logSuccess(httpStatus, globalId, messageId) + updateStatusAndNotify(SendingStatus.SENT) + case UNEXPECTED_SUCCESS => + logSuccess(httpStatus, globalId, messageId) + updateStatusAndNotify(SendingStatus.SENT) + case FAIL_ERROR => + logSendingFailure(httpStatus, message.globalId, message.messageId) + updateStatusAndNotify(SendingStatus.FAILED) + case RETRYABLE_ERROR => + if (retryDurationExpired) { + logRetryingTimedOut(httpStatus, globalId, messageId) + updateStatusAndNotify(SendingStatus.FAILED) + } else { + logContinuingRetrying(httpStatus, globalId, messageId) + outboundMessageRepository.updateNextRetryTime(message.globalId, nextRetryDateTime).map(_ => ()) } - } else { - logger.warn(s"Retried message with global ID ${message.globalId} message ID ${message.messageId} got status code $result and will retry") - outboundMessageRepository.updateNextRetryTime(message.globalId, nextRetryDateTime).map(_ => ()) - } + case _ => Future.unit } } } - private def buildOutboundSoapMessage(message: MessageRequest, soapRequest: SoapRequest, result: Int): OutboundSoapMessage = { + private def processSendingResult(message: MessageRequest, soapRequest: SoapRequest, httpStatus: Int): OutboundSoapMessage = { val globalId: UUID = randomUUID val messageId = message.addressing.messageId - def is3xx(result: Int): Boolean = result >= 300 && result < 400 - - if (is2xx(result)) { - log2xxResult(result, globalId, messageId) - SentOutboundSoapMessage(globalId, messageId, soapRequest.soapEnvelope, soapRequest.destinationUrl, now, result, message.notificationUrl) - } else if (is3xx(result)|| is4xx(result)) { - logger.error(s"Message with global ID $globalId message ID $messageId got status code $result and failed") - FailedOutboundSoapMessage(globalId, messageId, soapRequest.soapEnvelope, soapRequest.destinationUrl, now, result, message.notificationUrl) - } else { - logger.warn(s"Message with global ID $globalId message ID $messageId got status code $result and will retry") + + def succeededMessage = { + SentOutboundSoapMessage(globalId, messageId, soapRequest.soapEnvelope, soapRequest.destinationUrl, now, httpStatus, message.notificationUrl) + } + + def failedMessage = { + FailedOutboundSoapMessage(globalId, messageId, soapRequest.soapEnvelope, soapRequest.destinationUrl, now, httpStatus, message.notificationUrl) + } + + def retryingMessage = { RetryingOutboundSoapMessage(globalId, messageId, soapRequest.soapEnvelope, soapRequest.destinationUrl, now, - now.plus(appConfig.retryInterval.toMillis), result, message.notificationUrl, None, None) + now.plus(appConfig.retryInterval.toMillis), httpStatus, message.notificationUrl, None, None) + } + + mapHttpStatusCode(httpStatus) match { + case SUCCESS => succeededMessage + case UNEXPECTED_SUCCESS => succeededMessage + case FAIL_ERROR => failedMessage + case RETRYABLE_ERROR => retryingMessage } } @@ -217,12 +238,48 @@ class OutboundService @Inject()(outboundConnector: OutboundConnector, envelope.getBody.addChild(payload) } - - private def log2xxResult(result: Int, globalId: UUID, messageId: String) = { - if (result != HttpStatus.SC_ACCEPTED) { - logger.warn(s"Message with global ID $globalId message ID $messageId got status code $result and successfully sent") + private def mapHttpStatusCode(httpStatusCode: Int): CcnRequestResult.Value = { + if (isSuccessful(httpStatusCode)){ + httpStatusCode match { + case ACCEPTED => SUCCESS + case _ => UNEXPECTED_SUCCESS + } + } else if (isInformational(httpStatusCode) || isRedirect(httpStatusCode) || isClientError(httpStatusCode)) { + FAIL_ERROR } else { - logger.info(s"Message with global ID $globalId message ID $messageId got status code $result and successfully sent") + RETRYABLE_ERROR + } + } + + private def logSendingFailure(httpStatus: Int, globalId: UUID, messageId: String): Unit = { + logger.error(s"Message with global ID $globalId message ID and $messageId, got status code $httpStatus and failed") + } + + private def logRetryingTimedOut(httpStatus: Int, globalId: UUID, messageId: String): Unit = { + logger.error(s"Retried message with global ID $globalId and message ID $messageId, got status code $httpStatus and failed on last attempt") + } + + private def logContinuingRetrying(httpStatus: Int, globalId: UUID, messageId: String): Unit = { + logger.warn(s"Retried message with global ID $globalId and message ID $messageId, got status code $httpStatus and will retry") + } + + private def logSuccess(httpStatus: Int, globalId: UUID, messageId: String): Unit = { + val successLogMsg: String = s"Message with global ID $globalId and message ID $messageId got status code $httpStatus and successfully sent" + httpStatus match { + case ACCEPTED => logger.info(successLogMsg) + case _ => logger.warn(successLogMsg) + } + } + + private def logCcnSendResult(osm: OutboundSoapMessage, httpStatus: Int): Future[Unit] = { + val globalId = osm.globalId + val messageId = osm.messageId + mapHttpStatusCode(httpStatus) match { + case SUCCESS => logSuccess(httpStatus, globalId, messageId) + case UNEXPECTED_SUCCESS => logSuccess(httpStatus, globalId, messageId) + case FAIL_ERROR => logSendingFailure(httpStatus, globalId, messageId) + case RETRYABLE_ERROR => logContinuingRetrying(httpStatus, globalId, messageId) } + Future.unit } } diff --git a/test/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundServiceSpec.scala b/test/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundServiceSpec.scala index 294ef49..70a1e9c 100644 --- a/test/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundServiceSpec.scala +++ b/test/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundServiceSpec.scala @@ -492,6 +492,60 @@ class OutboundServiceSpec extends AnyWordSpec with Matchers with GuiceOneAppPerS verify(outboundMessageRepositoryMock).updateNextRetryTime(retryingMessage.globalId, expectedCreateDateTime.plus(appConfigMock.retryInterval.toMillis)) } + "retry a message and mark failed when SOAP request received 1xx status" in new Setup { + val retryingMessage = RetryingOutboundSoapMessage(randomUUID, "MessageId-A1", "payload", + "some url", DateTime.now(UTC), DateTime.now(UTC), httpStatus) + when(appConfigMock.parallelism).thenReturn(2) + when(appConfigMock.retryDuration).thenReturn(Duration("5s")) + when(appConfigMock.retryInterval).thenReturn(Duration("5s")) + when(outboundConnectorMock.postMessage(*, *)).thenReturn(successful(CONTINUE)) + when(outboundMessageRepositoryMock.retrieveMessagesForRetry). + thenReturn(fromIterator(() => Seq(retryingMessage).toIterator)) + when(outboundMessageRepositoryMock.updateSendingStatus(*, *)).thenReturn(successful(None)) + await(underTest.retryMessages) + + verify(outboundMessageRepositoryMock, never).updateNextRetryTime(*, *) + verify(outboundMessageRepositoryMock, never).updateSendingStatus(retryingMessage.globalId, SendingStatus.SENT) + verify(outboundMessageRepositoryMock, never).updateSendingStatus(retryingMessage.globalId, SendingStatus.RETRYING) + verify(outboundMessageRepositoryMock).updateSendingStatus(retryingMessage.globalId, SendingStatus.FAILED) + } + + "retry a message and mark failed when SOAP request received 3xx status" in new Setup { + val retryingMessage = RetryingOutboundSoapMessage(randomUUID, "MessageId-A1", "payload", + "some url", DateTime.now(UTC), DateTime.now(UTC), httpStatus) + when(appConfigMock.parallelism).thenReturn(2) + when(appConfigMock.retryDuration).thenReturn(Duration("5s")) + when(appConfigMock.retryInterval).thenReturn(Duration("5s")) + when(outboundConnectorMock.postMessage(*, *)).thenReturn(successful(TEMPORARY_REDIRECT)) + when(outboundMessageRepositoryMock.retrieveMessagesForRetry). + thenReturn(fromIterator(() => Seq(retryingMessage).toIterator)) + when(outboundMessageRepositoryMock.updateSendingStatus(*, *)).thenReturn(successful(None)) + await(underTest.retryMessages) + + verify(outboundMessageRepositoryMock, never).updateNextRetryTime(*, *) + verify(outboundMessageRepositoryMock, never).updateSendingStatus(retryingMessage.globalId, SendingStatus.SENT) + verify(outboundMessageRepositoryMock, never).updateSendingStatus(retryingMessage.globalId, SendingStatus.RETRYING) + verify(outboundMessageRepositoryMock).updateSendingStatus(retryingMessage.globalId, SendingStatus.FAILED) + } + + "retry a message and mark failed when SOAP request received 4xx status" in new Setup { + val retryingMessage = RetryingOutboundSoapMessage(randomUUID, "MessageId-A1", "payload", + "some url", DateTime.now(UTC), DateTime.now(UTC), httpStatus) + when(appConfigMock.parallelism).thenReturn(2) + when(appConfigMock.retryDuration).thenReturn(Duration("5s")) + when(appConfigMock.retryInterval).thenReturn(Duration("5s")) + when(outboundConnectorMock.postMessage(*, *)).thenReturn(successful(NOT_FOUND)) + when(outboundMessageRepositoryMock.retrieveMessagesForRetry). + thenReturn(fromIterator(() => Seq(retryingMessage).toIterator)) + when(outboundMessageRepositoryMock.updateSendingStatus(*, *)).thenReturn(successful(None)) + await(underTest.retryMessages) + + verify(outboundMessageRepositoryMock, never).updateNextRetryTime(*, *) + verify(outboundMessageRepositoryMock, never).updateSendingStatus(retryingMessage.globalId, SendingStatus.SENT) + verify(outboundMessageRepositoryMock, never).updateSendingStatus(retryingMessage.globalId, SendingStatus.RETRYING) + verify(outboundMessageRepositoryMock).updateSendingStatus(retryingMessage.globalId, SendingStatus.FAILED) + } + "set a message's status to FAILED when its retryDuration has expired" in new Setup { val retryDuration: Duration = Duration("30s") val retryingMessage = RetryingOutboundSoapMessage(randomUUID, "MessageId-A1", "payload",