Skip to content

Commit

Permalink
APID-237: Rename variables. Fail immediately on 3xx and 4xx responses… (
Browse files Browse the repository at this point in the history 
#55)

* APID-237: Rename variables. Fail immediately on 3xx and 4xx responses during retrying

* APID-237: refactoring to remove duplication and break down methods into more manageable parts

* APID-237: Reordering code

* APID-237: updated README with description of retrying behaviour
  • Loading branch information
@worthydolt
worthydolt authored Aug 2, 2021
1 parent 156534d commit b1c843c
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 43 deletions.
12 changes: 7 additions & 5 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions app/uk/gov/hmrc/apiplatformoutboundsoap/responses.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
133 changes: 95 additions & 38 deletions app/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundService.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
}

Expand Down Expand Up @@ -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
}
}
54 changes: 54 additions & 0 deletions test/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundServiceSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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", "<IE4N03>payload</IE4N03>",
"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", "<IE4N03>payload</IE4N03>",
"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", "<IE4N03>payload</IE4N03>",
"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", "<IE4N03>payload</IE4N03>",
Expand Down

0 comments on commit b1c843c

Please sign in to comment.