Merge pull request #70 from hmrc/API-5891-corrections

API-5891 - private header value is made mandatory, error msg for max headers is corrected
hmrc · Jan 18, 2023 · 1c8ac38 · 1c8ac38
2 parents 78e8979 + ffd162b
commit 1c8ac38
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 18 deletions.
diff --git a/app/uk/gov/hmrc/apiplatformoutboundsoap/controllers/OutboundController.scala b/app/uk/gov/hmrc/apiplatformoutboundsoap/controllers/OutboundController.scala
@@ -43,8 +43,8 @@ class OutboundController @Inject() (cc: ControllerComponents, appConfig: AppConf
       messageRequest.privateHeaders match {
         case Some(privHeaders) =>
           if (privHeaders.length > maxPrivateHeaders) {
-            logger.warn(s"Rejecting request because it has ${privHeaders.length} private headers and the maximum is $maxPrivateHeaders")
-            Future.successful(BadRequest(Json.toJson(ErrorResponse(BAD_REQUEST, "Maximum 5 private headers are allowed in message request"))))
+            logger.warn(s"A maximum of $maxPrivateHeaders private headers are allowed in the message but ${privHeaders.length} were supplied")
+            Future.successful(BadRequest(Json.toJson(ErrorResponse(BAD_REQUEST, s"A maximum of $maxPrivateHeaders private headers are allowed in the message but ${privHeaders.length} were supplied"))))
           } else {
             sendMessage(messageRequest)
           }

diff --git a/app/uk/gov/hmrc/apiplatformoutboundsoap/models/MessageRequest.scala b/app/uk/gov/hmrc/apiplatformoutboundsoap/models/MessageRequest.scala
@@ -34,7 +34,7 @@ case class Addressing(from: String, to: String, replyTo: String, faultTo: String
   validate(from.trim != "", "addressing.from being empty")
 }
 
-case class PrivateHeader(name: String, value: Option[String]) {
-  validate(name.trim.length < 1024, "privateHeaders name is longer than 1024 characters")
-  validate(!(value.isDefined && value.get.trim.length > 1024), "privateHeaders value is longer than 1024 characters")
+case class PrivateHeader(name: String, value: String) {
+  validate(name.trim.length <= 1024, "privateHeaders name is longer than 1024 characters")
+  validate(value.trim.length <= 1024, "privateHeaders value is longer than 1024 characters")
 }
diff --git a/it/uk/gov/hmrc/apiplatformoutboundsoap/repositories/OutboundMessageRepositoryISpec.scala b/it/uk/gov/hmrc/apiplatformoutboundsoap/repositories/OutboundMessageRepositoryISpec.scala
@@ -45,7 +45,7 @@ class OutboundMessageRepositoryISpec extends AnyWordSpec with PlayMongoRepositor
 
   override implicit lazy val app: Application = appBuilder.build()
   val ccnHttpStatus: Int                      = 200
-  val privateHeaders                          = Some(List(PrivateHeader(name = "name1", value = Some("value1")), PrivateHeader(name = "name2", value = Some("value2"))))
+  val privateHeaders                          = Some(List(PrivateHeader(name = "name1", value = "value1"), PrivateHeader(name = "name2", value = "value2")))
   val instantNow: Instant                     = now.truncatedTo(ChronoUnit.MILLIS)
 
   val retryingMessage                     =

diff --git a/test/uk/gov/hmrc/apiplatformoutboundsoap/controllers/OutboundControllerSpec.scala b/test/uk/gov/hmrc/apiplatformoutboundsoap/controllers/OutboundControllerSpec.scala
@@ -58,13 +58,13 @@ class OutboundControllerSpec extends AnyWordSpec with Matchers with MockitoSugar
     val fakeRequest                        = FakeRequest("POST", "/message")
     val addressing                         = Addressing(messageId = "987", to = "AddressedTo", replyTo = "ReplyTo", faultTo = "FaultTo", from = "from")
     val addressingJson                     = Json.toJson(addressing)
-    val privateHeaders                     = List(PrivateHeader(name = "name1", value = Some("value1")), PrivateHeader(name = "name2", value = Some("value2")))
-    val privateHeadersEmptyValue           = List(PrivateHeader(name = "name1", value = None), PrivateHeader(name = "name2", value = Some("value2")))
+    val privateHeaders                     = List(PrivateHeader(name = "name1", value = "value1"), PrivateHeader(name = "name2", value = "value2"))
+    val privateHeadersEmptyValue           = List(PrivateHeader(name = "name1", value = ""), PrivateHeader(name = "name2", value = "value2"))
     val privateHeadersTooMany              = privateHeaders ++ List(
-      PrivateHeader(name = "name3", value = Some("value3")),
-      PrivateHeader(name = "name4", value = Some("value4")),
-      PrivateHeader(name = "name5", value = Some("value5")),
-      PrivateHeader(name = "name6", value = Some("value6"))
+      PrivateHeader(name = "name3", value = "value3"),
+      PrivateHeader(name = "name4", value = "value4"),
+      PrivateHeader(name = "name5", value = "value5"),
+      PrivateHeader(name = "name6", value = "value6")
     )
     val privateHeadersJson                 = Json.toJson(privateHeaders)
     val privateHeadersTooManyJson          = Json.toJson(privateHeadersTooMany)
@@ -128,7 +128,7 @@ class OutboundControllerSpec extends AnyWordSpec with Matchers with MockitoSugar
       val result: Future[Result] = underTest.message()(fakeRequest.withBody(messageWithTooManyPrivateHeaders))
       status(result) shouldBe BAD_REQUEST
       (contentAsJson(result) \ "statusCode").as[Int] shouldBe BAD_REQUEST
-      (contentAsJson(result) \ "message").as[String] shouldBe "Maximum 5 private headers are allowed in message request"
+      (contentAsJson(result) \ "message").as[String] shouldBe "A maximum of 5 private headers are allowed in the message but 6 were supplied"
       verifyZeroInteractions(outboundServiceMock)
     }
 
@@ -153,7 +153,7 @@ class OutboundControllerSpec extends AnyWordSpec with Matchers with MockitoSugar
       ))
 
       status(result) shouldBe BAD_REQUEST
-      contentAsString(result) shouldBe "Invalid MessageRequest payload: List((/privateHeaders(0)/name,List(JsonValidationError(List(error.path.missing),List()))))"
+      contentAsString(result) shouldBe "Invalid MessageRequest payload: List((/privateHeaders(0)/name,List(JsonValidationError(List(error.path.missing),List()))), (/privateHeaders(0)/value,List(JsonValidationError(List(error.path.missing),List()))))"
       verifyZeroInteractions(outboundServiceMock)
     }
 

diff --git a/test/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundServiceSpec.scala b/test/uk/gov/hmrc/apiplatformoutboundsoap/services/OutboundServiceSpec.scala
@@ -87,9 +87,10 @@ class OutboundServiceSpec extends AnyWordSpec with Matchers with GuiceOneAppPerS
     val messageId                         = "123"
     val to                                = "CCN2"
     val from                              = "HMRC"
-    val longPrivateHeaderValue            =
-      "value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1value1valuevlue1value1value1"
-    val privateHeaders                    = Some(List(PrivateHeader(name = "name1", Some(value = "value1")), PrivateHeader(name = "name2", Some(value = "value2"))))
+    val longPrivateHeaderValue1023Length = "".padTo(1023, 'a')
+    val longPrivateHeaderValue1024Length = longPrivateHeaderValue1023Length ++ "a"
+    val longPrivateHeaderValue1025Length  = longPrivateHeaderValue1024Length ++ "a"
+    val privateHeaders                    = Some(List(PrivateHeader(name = "name1", value = "value1"), PrivateHeader(name = "name2", value = "value2")))
     val addressing                        = Addressing(from, to, "ReplyTo", "FaultTo", messageId, Some("RelatesTo"))
     // mixin refers to mandatory and default addressing fields
     val addressingMixinFields             = Addressing(from = from, to = to, replyTo = "ReplyTo", faultTo = "FaultTo", messageId = messageId)
@@ -464,11 +465,35 @@ class OutboundServiceSpec extends AnyWordSpec with Matchers with GuiceOneAppPerS
 
       val exception: IllegalArgumentException = intercept[IllegalArgumentException] {
         await(underTest.sendMessage(messageRequestWithPrivateHeaders.copy(privateHeaders =
-          Some(List(PrivateHeader(name = "name1", value = Some(longPrivateHeaderValue)), PrivateHeader(name = "name2", value = Some("value2"))))
+          Some(List(PrivateHeader(name = "name1", value = longPrivateHeaderValue1025Length), PrivateHeader(name = "name2", value = "value2")))
         )))
       }
       exception.getMessage should include("privateHeaders value is longer than 1024 characters")
     }
+
+    "persist message when the private headers field parameters size is equals to 1024" in new Setup {
+      when(wsSecurityServiceMock.addUsernameToken(*)).thenReturn(expectedSoapEnvelope(allAddressingHeaders))
+      when(outboundConnectorMock.postMessage(*, *)).thenReturn(successful(expectedStatus))
+      when(appConfigMock.retryInterval).thenReturn(Duration("1s"))
+      val messageCaptor: ArgumentCaptor[OutboundSoapMessage] = ArgumentCaptor.forClass(classOf[OutboundSoapMessage])
+      when(outboundMessageRepositoryMock.persist(messageCaptor.capture())).thenReturn(Future(InsertOneResult.acknowledged(BsonNumber(1))))
+      await(underTest.sendMessage(messageRequestWithPrivateHeaders.copy(privateHeaders =
+        Some(List(PrivateHeader(name = "name1", value = longPrivateHeaderValue1024Length), PrivateHeader(name = "name2", value = "value2")))
+      )))
+      messageCaptor.getValue.messageId shouldBe messageId
+    }
+
+    "persist message when the private headers field parameters size is less than 1024" in new Setup {
+      when(wsSecurityServiceMock.addUsernameToken(*)).thenReturn(expectedSoapEnvelope(allAddressingHeaders))
+      when(outboundConnectorMock.postMessage(*, *)).thenReturn(successful(expectedStatus))
+      when(appConfigMock.retryInterval).thenReturn(Duration("1s"))
+      val messageCaptor: ArgumentCaptor[OutboundSoapMessage] = ArgumentCaptor.forClass(classOf[OutboundSoapMessage])
+      when(outboundMessageRepositoryMock.persist(messageCaptor.capture())).thenReturn(Future(InsertOneResult.acknowledged(BsonNumber(1))))
+      await(underTest.sendMessage(messageRequestWithPrivateHeaders.copy(privateHeaders =
+        Some(List(PrivateHeader(name = "name1", value = longPrivateHeaderValue1023Length), PrivateHeader(name = "name2", value = "value2")))
+      )))
+      messageCaptor.getValue.messageId shouldBe messageId
+    }
   }
 
   "retryMessages" should {