Repository: softwaremill/elasticmq
Pull request #953 (2024-01-22 08:57:03)

fix: The JSON protocol does not handle empty lists, so use the Option
https://github.com/localstack/localstack/pull/9710/files#diff-afa491974d1d7cb931555d6bfd9bfbf8e4f977f1528242ba8732413a848c15b7R251-R252

Similar fix to a recent fix in Localstack: In JSON Protocol, `BatchResponse` does not seem to return "failed" as a field on successful request. If it did, `SendMessageBatchResponse#hasFailed` would not work correctly. (https://github.com/aws/aws-sdk-java-v2/issues/4759)


Patch:
diff --git a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/BatchRequestsModule.scala b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/BatchRequestsModule.scala
index 82541ab2..619a2212 100644
--- a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/BatchRequestsModule.scala
+++ b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/BatchRequestsModule.scala
@@ -40,8 +40,9 @@ trait BatchRequestsModule {
     Future
       .sequence(result)
       .map(
-        _.foldLeft((List.empty[Failed], List.empty[R])) {
-          case ((failures, successes), Left(failed))   => (failures :+ failed, successes)
+        _.foldLeft(Option.empty[List[Failed]], List.empty[R]) {
+          case ((failures, successes), Left(failed)) =>
+            (failures.map(_ :+ failed).orElse(Some(List(failed))), successes)
           case ((failures, successes), Right(success)) => (failures, successes :+ success)
         }
       )
@@ -83,7 +84,7 @@ case class BatchRequest[M](
     QueueUrl: String
 )
 
-case class BatchResponse[R](Failed: List[Failed], Successful: List[R])
+case class BatchResponse[R](Failed: Option[List[Failed]], Successful: List[R])
 
 object BatchResponse {
   implicit def jsonFormat[R: JsonFormat]: RootJsonFormat[BatchResponse[R]] = jsonFormat2(BatchResponse.apply[R])
diff --git a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/ChangeMessageVisibilityBatchDirectives.scala b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/ChangeMessageVisibilityBatchDirectives.scala
index b2a6ba6b..21318aab 100644
--- a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/ChangeMessageVisibilityBatchDirectives.scala
+++ b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/ChangeMessageVisibilityBatchDirectives.scala
@@ -61,7 +61,7 @@ object BatchChangeMessageVisibilityResponseEntry {
       override def toXml(t: BatchResponse[BatchChangeMessageVisibilityResponseEntry]): Elem =
         <ChangeMessageVisibilityBatchResponse>
           <ChangeMessageVisibilityBatchResult>
-            {t.Successful.map(successSerializer.toXml) ++ t.Failed.map(XmlSerializer[Failed].toXml)}
+            {t.Successful.map(successSerializer.toXml) ++ t.Failed.toList.flatMap(_.map(XmlSerializer[Failed].toXml))}
           </ChangeMessageVisibilityBatchResult>
           <ResponseMetadata>
             <RequestId>
diff --git a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/DeleteMessageBatchDirectives.scala b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/DeleteMessageBatchDirectives.scala
index 7b457895..899e1a58 100644
--- a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/DeleteMessageBatchDirectives.scala
+++ b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/DeleteMessageBatchDirectives.scala
@@ -57,7 +57,7 @@ object BatchDeleteMessageResponseEntry {
       override def toXml(t: BatchResponse[T]): Elem =
         <DeleteMessageBatchResponse>
         <DeleteMessageBatchResult>
-          {t.Successful.map(successSerializer.toXml) ++ t.Failed.map(XmlSerializer[Failed].toXml)}
+          {t.Successful.map(successSerializer.toXml) ++ t.Failed.toList.flatMap(_.map(XmlSerializer[Failed].toXml))}
         </DeleteMessageBatchResult>
         <ResponseMetadata>
           <RequestId>{EmptyRequestId}</RequestId>
diff --git a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/SendMessageBatchDirectives.scala b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/SendMessageBatchDirectives.scala
index 4d6faf3c..40141371 100644
--- a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/SendMessageBatchDirectives.scala
+++ b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/SendMessageBatchDirectives.scala
@@ -132,7 +132,7 @@ object BatchMessageSendResponseEntry {
       override def toXml(t: BatchResponse[BatchMessageSendResponseEntry]): Elem =
         <SendMessageBatchResponse>
           <SendMessageBatchResult>
-            {t.Successful.map(successSerializer.toXml) ++ t.Failed.map(XmlSerializer[Failed].toXml)}
+            {t.Successful.map(successSerializer.toXml) ++ t.Failed.toList.flatMap(_.map(XmlSerializer[Failed].toXml))}
           </SendMessageBatchResult>
           <ResponseMetadata>
             <RequestId>


Test patch:
diff --git a/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/AmazonCliTestSuite.scala b/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/AmazonCliTestSuite.scala
index c026341a..f8b418ea 100644
--- a/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/AmazonCliTestSuite.scala
+++ b/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/AmazonCliTestSuite.scala
@@ -396,8 +396,9 @@ class AmazonCliTestSuite
       val batchMessages = sendMessageBatch(queue.QueueUrl, entries)
 
       // then
-      batchMessages.Failed.size shouldBe 1
-      inside(batchMessages.Failed.head) { case failedMessage =>
+      val failed = batchMessages.Failed.toList.flatten
+      failed.size shouldBe 1
+      inside(failed.head) { case failedMessage =>
         import failedMessage._
         Id shouldBe "2"
         SenderFault shouldBe true