Collect all errors during transaction construction

dynamodb/src/main/scala/zio/dynamodb/DynamoDBExecutorImpl.scala

@@ -78,10 +78,10 @@ import zio.aws.dynamodb.model.{
 }
 import zio.dynamodb.ConsistencyMode.toBoolean
 import zio.dynamodb.DynamoDBQuery.BatchGetItem.TableGet
-import zio.dynamodb.DynamoDBQuery._
+import zio.dynamodb.DynamoDBQuery.{ ValidationTransactionError, _ }
 import zio.dynamodb.SSESpecification.SSEType
 import zio.stream.{ Stream, ZStream }
-import zio.{ Chunk, NonEmptyChunk, ZIO }
+import zio.{ Chunk, ZIO }
 
 import scala.collection.immutable.{ Map => ScalaMap }
 
@@ -358,32 +358,77 @@ case object DynamoDBExecutorImpl {
   ): Either[Throwable, (Chunk[Constructor[Any, A]], TransactionType)] =
     if (actions.isEmpty) Left(EmptyTransaction())
     else {
-      val headConstructor = constructorToTransactionType(actions.head)
-        .map(Right(_))
-        .getOrElse(Left(InvalidTransactionActions(NonEmptyChunk(actions.head)))) // TODO: Grab all that are invalid
+      val headConstructor: Ior[ValidationTransactionErrors, TransactionType] =
+        constructorToTransactionType(actions.head)
+          .map(RightIor(_))
+          .getOrElse(LeftIor(ValidationTransactionErrors.fromAction(actions.head)))
+
       actions
-        .drop(1)                                                                 // dropping 1 because we have the head element as the base case of the fold
-        .foldLeft(headConstructor: Either[Throwable, TransactionType]) {
+        .drop(1) // dropping 1 because we have the head element as the base case of the fold
+        .foldLeft(headConstructor: Ior[ValidationTransactionErrors, TransactionType]) {
           case (acc, constructor) =>
             acc match {
-              case l @ Left(_)            => l // Should also continue collecting other failures
-              case Right(transactionType) => constructorMatch(constructor, transactionType)
+              case LeftIor(validationTransactionErrors) => checkType(constructor, validationTransactionErrors)
+              case RightIor(transactionType)            => checkIfNotErrorsYet(constructor, transactionType)
+              case BothIor(errors, transactionType)     =>
+                checkIfHasTransactionTypeAndErrors(constructor, transactionType, errors)
             }
+        } match {
+        case LeftIor(value) => Left(value)
+        case RightIor(value)    => Right((actions, value))
+        case BothIor(valueA, _) => Left(valueA)
+      }
+    }
+
+  private def checkType[A](
+    constructor: Constructor[Any, A],
+    validationTransactionErrors: ValidationTransactionErrors
+  ): Ior[ValidationTransactionErrors, TransactionType] =
+    constructorToTransactionType(constructor)
+      .map(x => BothIor(validationTransactionErrors, x))
+      .getOrElse(LeftIor(validationTransactionErrors.addInvalidAction(constructor)))
+
+  private def checkIfNotErrorsYet[A](
+    constructor: Constructor[Any, A],
+    transactionType: TransactionType
+  ): Ior[ValidationTransactionErrors, TransactionType] =
+    constructorMatch(constructor, transactionType) match {
+      case Left(error) =>
+        error match {
+          case MixedTransactionTypes()                 =>
+            BothIor(ValidationTransactionErrors.fromMixedTypes, transactionType)
+          case InvalidTransactionAction(invalidAction) =>
+            BothIor(ValidationTransactionErrors.fromAction(invalidAction), transactionType)
+        }
+      case Right(_)    => RightIor(transactionType)
+    }
+
+  private def checkIfHasTransactionTypeAndErrors[A](
+    constructor: Constructor[Any, A],
+    transactionType: TransactionType,
+    errors: ValidationTransactionErrors
+  ) =
+    constructorMatch(constructor, transactionType) match {
+      case Left(error) =>
+        error match {
+          case MixedTransactionTypes()                 => BothIor(errors.addMixedTypes(), transactionType)
+          case InvalidTransactionAction(invalidAction) =>
+            BothIor(errors.addInvalidAction(invalidAction), transactionType)
         }
-        .map(transactionType => (actions, transactionType))
+      case Right(_)    => BothIor(errors, transactionType)
     }
 
   private def constructorMatch[A](
     constructor: Constructor[Any, A],
     transactionType: TransactionType
-  ): Either[Throwable, TransactionType] =
+  ): Either[ValidationTransactionError, TransactionType] =
     constructorToTransactionType(constructor)
       .map(t =>
         if (t == transactionType) Right(transactionType)
         else Left(MixedTransactionTypes())
       )
       .getOrElse(
-        Left(InvalidTransactionActions(NonEmptyChunk(constructor)))
+        Left(InvalidTransactionAction(constructor))
       )
 
   private def constructorToTransactionType[A](constructor: Constructor[_, A]): Option[TransactionType] =
@@ -451,7 +496,8 @@ case object DynamoDBExecutorImpl {
               (Chunk(s), _ => None.asInstanceOf[A])
             ) // we don't get data back from transactWrites, return None here
           case s: ConditionCheck           => Right((Chunk(s), chunk => chunk(0).asInstanceOf[A]))
-          case s                           => Left(InvalidTransactionActions(NonEmptyChunk(s.asInstanceOf[DynamoDBQuery[Any, Any]])))
+          case s                           =>
+            Left(InvalidTransactionAction(s.asInstanceOf[DynamoDBQuery[Any, Any]]))
         }
       case Zip(left, right, zippable)     =>
         for {
@@ -474,9 +520,7 @@ case object DynamoDBExecutorImpl {
           case (constructors, construct) => (constructors, chunk => mapper.asInstanceOf[Any => A](construct(chunk)))
         }
       case Absolve(query)                 =>
-        Left(
-          InvalidTransactionActions(NonEmptyChunk(query.asInstanceOf[DynamoDBQuery[Any, Any]]))
-        )
+        Left(InvalidTransactionAction(query.asInstanceOf[DynamoDBQuery[Any, Any]]))
     }
 
   private[dynamodb] def constructTransaction[A](

dynamodb/src/main/scala/zio/dynamodb/DynamoDBQuery.scala

@@ -748,9 +748,33 @@ object DynamoDBQuery {
     itemMetrics: ReturnItemCollectionMetrics = ReturnItemCollectionMetrics.None
   ) extends Constructor[Any, A]
 
-  private[dynamodb] final case class MixedTransactionTypes() extends Throwable
-  private[dynamodb] final case class InvalidTransactionActions(invalidActions: NonEmptyChunk[DynamoDBQuery[Any, Any]])
-      extends Throwable
+  sealed trait ValidationTransactionError                    extends Throwable
+  private[dynamodb] final case class MixedTransactionTypes() extends ValidationTransactionError
+
+  private[dynamodb] final case class InvalidTransactionAction(invalidAction: DynamoDBQuery[Any, Any])
+      extends ValidationTransactionError
+  private[dynamodb] final case class ValidationTransactionErrors(
+    invalidActions: Chunk[InvalidTransactionAction],
+    mixedTransactionTypes: Option[MixedTransactionTypes]
+  ) extends Throwable
+
+  object ValidationTransactionErrors {
+    def fromAction(action: DynamoDBQuery[Any, Any]): ValidationTransactionErrors =
+      ValidationTransactionErrors(NonEmptyChunk(InvalidTransactionAction(action)), None)
+
+    def fromMixedTypes: ValidationTransactionErrors =
+      ValidationTransactionErrors(Chunk.empty, Some(MixedTransactionTypes()))
+
+    implicit class ValidationTransactionErrorsOps(validationTransactionErrors: ValidationTransactionErrors) {
+      def addInvalidAction(action: DynamoDBQuery[Any, Any]): ValidationTransactionErrors =
+        validationTransactionErrors.copy(invalidActions =
+          validationTransactionErrors.invalidActions.appended(InvalidTransactionAction(action))
+        )
+
+      def addMixedTypes(): ValidationTransactionErrors =
+        validationTransactionErrors.copy(mixedTransactionTypes = Some(MixedTransactionTypes()))
+    }
+  }
 
   private[dynamodb] final case class BatchWriteItem(
     requestItems: MapOfSet[TableName, BatchWriteItem.Write] = MapOfSet.empty,

dynamodb/src/main/scala/zio/dynamodb/Ior.scala

@@ -0,0 +1,9 @@
+package zio.dynamodb
+
+sealed trait Ior[+A, +B]
+
+case class LeftIor[+A, +B](value: A) extends Ior[A, B]
+
+case class RightIor[+A, +B](value: B) extends Ior[A, B]
+
+case class BothIor[+A, +B](valueA: A, valueB: B) extends Ior[A, B]

dynamodb/src/test/scala/zio/dynamodb/TransactionModelSpec.scala

@@ -2,10 +2,10 @@ package zio.dynamodb
 
 import zio.aws.dynamodb.DynamoDb
 import zio.aws.dynamodb.DynamoDbMock
-import zio.{ Chunk, ULayer }
+import zio.{ Chunk, ULayer, ZIO }
 import zio.dynamodb.DynamoDBQuery._
 import zio.dynamodb.ProjectionExpression.$
-import zio.test.Assertion.{ contains, equalTo, fails, hasField, isSubtype }
+import zio.test.Assertion.{ equalTo, fails, hasField, hasSameElements, isSome, isSubtype }
 import zio.test._
 import zio.mock.Expectation.value
 import zio.aws.dynamodb.model.{ ItemResponse, TransactGetItemsResponse, TransactWriteItemsResponse }
@@ -118,15 +118,30 @@ object TransactionModelSpec extends ZIOSpecDefault {
       .or(putItem)
       .or(batchWriteItem)
 
-  private def invalidTransactionActionsContains(action: DynamoDBQuery[Any, Any]): Assertion[Any] =
-    isSubtype[InvalidTransactionActions](
+  private def invalidTransactionActionContains(action: DynamoDBQuery[Any, Any]): Assertion[Any] =
+    isSubtype[InvalidTransactionAction](
+      hasField(
+        "invalidAction",
+        _.invalidAction,
+        equalTo(action)
+      )
+    )
+
+  private def mixedTransactionTypesError(): Assertion[Any] =
+    isSubtype[ValidationTransactionErrors](
+      hasField(
+        "mixedTransactionTypes",
+        _.mixedTransactionTypes,
+        isSome(equalTo(MixedTransactionTypes()))
+      )
+    )
+
+  private def invalidTransactionActionsContains(actions: Seq[DynamoDBQuery[Any, Any]]): Assertion[Any] =
+    isSubtype[ValidationTransactionErrors](
       hasField(
         "invalidActions",
-        a => {
-          val b: Iterable[DynamoDBQuery[Any, Any]] = a.invalidActions.toList
-          b
-        },
-        contains(action)
+        _.invalidActions.toList.map(_.invalidAction),
+        hasSameElements(actions)
       )
     )
 
@@ -137,19 +152,35 @@ object TransactionModelSpec extends ZIOSpecDefault {
     )
 
   val failureSuite = suite("transaction construction failures")(
-    suite("mixed transaction types")(
-      test("mixing update and get") {
+    suite("mixed transaction types")(test("mixing update and get") {
+      val updateItem = UpdateItem(
+        key = item,
+        tableName = tableName,
+        updateExpression = UpdateExpression($("name").set(""))
+      )
+
+      val getItem                                                       = GetItem(tableName, item)
+      val query: DynamoDBQuery[Any, (Option[AttrMap], Option[AttrMap])] = updateItem.zip(getItem)
+
+      assertZIO(query.transaction.execute.exit)(
+        fails(mixedTransactionTypesError())
+      )
+    }),
+    suite("filterMixedTransactions")(
+      test("mixing update and get and invalid actions describe and scan") {
         val updateItem = UpdateItem(
           key = item,
           tableName = tableName,
           updateExpression = UpdateExpression($("name").set(""))
         )
 
-        val getItem                                                       = GetItem(tableName, item)
-        val query: DynamoDBQuery[Any, (Option[AttrMap], Option[AttrMap])] = updateItem.zip(getItem)
+        val getItem  = GetItem(tableName, item)
+        val describe = DescribeTable(tableName)
+        val scan     = ScanSome(tableName, 4)
+        val query    = Chunk(updateItem, getItem, describe, scan)
 
-        assertZIO(query.transaction.execute.exit)(
-          fails(isSubtype[MixedTransactionTypes](Assertion.anything))
+        assertZIO(ZIO.fromEither(DynamoDBExecutorImpl.filterMixedTransactions(query)).exit)(
+          fails(mixedTransactionTypesError()) && fails(invalidTransactionActionsContains(List(describe, scan)))
         )
       }
     ),
@@ -161,31 +192,31 @@ object TransactionModelSpec extends ZIOSpecDefault {
           attributeDefinitions = NonEmptySet(AttributeDefinition.attrDefnString("name")),
           billingMode = BillingMode.PayPerRequest
         )
-        assertZIO(createTable.transaction.execute.exit)(fails(invalidTransactionActionsContains(createTable)))
+        assertZIO(createTable.transaction.execute.exit)(fails(invalidTransactionActionContains(createTable)))
       },
       test("delete table") {
         val deleteTable = DeleteTable(tableName)
-        assertZIO(deleteTable.transaction.execute.exit)(fails(invalidTransactionActionsContains(deleteTable)))
+        assertZIO(deleteTable.transaction.execute.exit)(fails(invalidTransactionActionContains(deleteTable)))
       },
       test("scan all") {
         val scanAll = ScanAll(tableName)
-        assertZIO(scanAll.transaction.execute.exit)(fails(invalidTransactionActionsContains(scanAll)))
+        assertZIO(scanAll.transaction.execute.exit)(fails(invalidTransactionActionContains(scanAll)))
       },
       test("scan some") {
         val scanSome = ScanSome(tableName, 4)
-        assertZIO(scanSome.transaction.execute.exit)(fails(invalidTransactionActionsContains(scanSome)))
+        assertZIO(scanSome.transaction.execute.exit)(fails(invalidTransactionActionContains(scanSome)))
       },
       test("describe table") {
         val describeTable = DescribeTable(tableName)
-        assertZIO(describeTable.transaction.execute.exit)(fails(invalidTransactionActionsContains(describeTable)))
+        assertZIO(describeTable.transaction.execute.exit)(fails(invalidTransactionActionContains(describeTable)))
       },
       test("query some") {
         val querySome = QuerySome(tableName, 4)
-        assertZIO(querySome.transaction.execute.exit)(fails(invalidTransactionActionsContains(querySome)))
+        assertZIO(querySome.transaction.execute.exit)(fails(invalidTransactionActionContains(querySome)))
       },
       test("query all") {
         val queryAll = QueryAll(tableName)
-        assertZIO(queryAll.transaction.execute.exit)(fails(invalidTransactionActionsContains(queryAll)))
+        assertZIO(queryAll.transaction.execute.exit)(fails(invalidTransactionActionContains(queryAll)))
       }
     )
   )