Merge pull request #29 from filosganga/collect-alternatives-errors

Collect-alternatives-errors
SystemFw · May 4, 2023 · 13fa836 · 13fa836
2 parents 8f936a0 + cf5f8b5
commit 13fa836
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 59 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -26,7 +26,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest]
-        scala: [2.13.6, 3.2.0, 2.12.14]
+        scala: [2.13.10, 3.2.2, 2.12.14]
         java: [temurin@11]
     runs-on: ${{ matrix.os }}
     steps:
@@ -59,7 +59,7 @@ jobs:
 
       - run: sbt ++${{ matrix.scala }} ci
 
-      - if: matrix.scala == '2.13.6'
+      - if: matrix.scala == '2.13.10'
         run: sbt ++${{ matrix.scala }} docs/mdoc
 
       - name: Compress target directories
@@ -78,7 +78,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest]
-        scala: [2.13.6]
+        scala: [2.13.10]
         java: [temurin@11]
     runs-on: ${{ matrix.os }}
     steps:
@@ -106,22 +106,22 @@ jobs:
             ~/Library/Caches/Coursier/v1
           key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}
 
-      - name: Download target directories (2.13.6)
+      - name: Download target directories (2.13.10)
         uses: actions/download-artifact@v2
         with:
-          name: target-${{ matrix.os }}-2.13.6-${{ matrix.java }}
+          name: target-${{ matrix.os }}-2.13.10-${{ matrix.java }}
 
-      - name: Inflate target directories (2.13.6)
+      - name: Inflate target directories (2.13.10)
         run: |
           tar xf targets.tar
           rm targets.tar
 
-      - name: Download target directories (3.2.0)
+      - name: Download target directories (3.2.2)
         uses: actions/download-artifact@v2
         with:
-          name: target-${{ matrix.os }}-3.2.0-${{ matrix.java }}
+          name: target-${{ matrix.os }}-3.2.2-${{ matrix.java }}
 
-      - name: Inflate target directories (3.2.0)
+      - name: Inflate target directories (3.2.2)
         run: |
           tar xf targets.tar
           rm targets.tar
@@ -148,26 +148,26 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest]
-        scala: [2.13.6]
+        scala: [2.13.10]
         java: [temurin@11]
     runs-on: ${{ matrix.os }}
     steps:
-      - name: Download target directories (2.13.6)
+      - name: Download target directories (2.13.10)
         uses: actions/download-artifact@v2
         with:
-          name: target-${{ matrix.os }}-2.13.6-${{ matrix.java }}
+          name: target-${{ matrix.os }}-2.13.10-${{ matrix.java }}
 
-      - name: Inflate target directories (2.13.6)
+      - name: Inflate target directories (2.13.10)
         run: |
           tar xf targets.tar
           rm targets.tar
 
-      - name: Download target directories (3.2.0)
+      - name: Download target directories (3.2.2)
         uses: actions/download-artifact@v2
         with:
-          name: target-${{ matrix.os }}-3.2.0-${{ matrix.java }}
+          name: target-${{ matrix.os }}-3.2.2-${{ matrix.java }}
 
-      - name: Inflate target directories (3.2.0)
+      - name: Inflate target directories (3.2.2)
         run: |
           tar xf targets.tar
           rm targets.tar

diff --git a/build.sbt b/build.sbt
@@ -14,10 +14,10 @@ ThisBuild / scmInfo := Some(
 ThisBuild / startYear := Some(2020)
 Global / excludeLintKeys += scmInfo
 
-val Scala213 = "2.13.6"
+val Scala213 = "2.13.10"
 ThisBuild / spiewakMainBranches := Seq("main")
 
-ThisBuild / crossScalaVersions := Seq(Scala213, "3.2.0", "2.12.14")
+ThisBuild / crossScalaVersions := Seq(Scala213, "3.2.2", "2.12.14")
 ThisBuild / versionIntroduced := Map("3.0.0" -> "0.3.0")
 ThisBuild / scalaVersion := (ThisBuild / crossScalaVersions).value.head
 ThisBuild / initialCommands := """

diff --git a/modules/core/shared/src/main/scala/internal/decoding.scala b/modules/core/shared/src/main/scala/internal/decoding.scala
@@ -17,7 +17,7 @@
 package dynosaur
 package internal
 
-import cats.{~>, Monoid, MonoidK}
+import cats.{~>, Monoid}
 import cats.syntax.all._
 import alleycats.std.map._
 import cats.free.FreeApplicative
@@ -147,16 +147,29 @@ object decoding {
 
   def decodeSum[A](cases: Chain[Alt[A]]): DynamoValue => Res[A] = {
 
-    implicit def orElse[T]: Monoid[Option[T]] =
-      MonoidK[Option].algebra
+    implicit def orElsev[T]: Monoid[Either[List[Schema.ReadError], T]] =
+      Monoid.instance(
+        Left(List.empty),
+        {
+          case (Left(l), Left(r)) => Left(l ++ r)
+          case (Left(_), r @ Right(_)) => r
+          case (l @ Right(_), Left(_)) => l
+          case (l @ Right(_), Right(_)) => l
+        }
+      )
 
     cases
       .foldMap { alt => (v: DynamoValue) =>
-        alt.caseSchema.read(v).map(alt.prism.inject).toOption
+        alt.caseSchema.read(v).map(alt.prism.inject).leftMap(e => List(e))
+      }
+      .andThen { res =>
+        res.leftMap { errors =>
+          val errorsDetails = errors.map(_.message).mkString("[", ",", "]")
+          ReadError(
+            s"value doesn't match any of the alternatives: $errorsDetails"
+          )
+        }
       }
-      .andThen(
-        _.toRight(ReadError("value doesn't match any of the alternatives"))
-      )
   }
 
   def decodeIsos[V](xmap: XMap[V], v: DynamoValue): Res[V] =

diff --git a/modules/core/shared/src/test/scala/SchemaSuite.scala b/modules/core/shared/src/test/scala/SchemaSuite.scala
@@ -99,6 +99,39 @@ class SchemaSuite extends ScalaCheckSuite {
     assertEquals(roundTrip, data.some)
   }
 
+  val statusSchema: Schema[Status] = Schema.oneOf { alt =>
+    val error = Schema
+      .record[Error] { field =>
+        field.const("type", "error") *> (
+          field("msg", _.msg),
+          field("cause", _.cause)
+        ).mapN(Error.apply)
+      }
+
+    val warning = Schema
+      .record[Warning] { field =>
+        field.const("type", "warning") *> (
+          field("msg", _.msg),
+          field("cause", _.cause)
+        ).mapN(Warning.apply)
+      }
+
+    val unknown =
+      Schema.record[Unknown.type](
+        _.const("type", "unknown").as(Unknown)
+      )
+
+    val successful = Schema
+      .record[Successful] { field =>
+        field.const("type", "successful") *> (
+          field("link", _.link),
+          field("expires", _.expires)
+        ).mapN(Successful.apply)
+      }
+
+    alt(error) |+| alt(warning) |+| alt(unknown) |+| alt(successful)
+  }
+
   test("id") {
     forAllNoShrink { (dv: V) =>
       val expected = dv
@@ -756,43 +789,11 @@ class SchemaSuite extends ScalaCheckSuite {
 
   test("ADTs, via a discriminator field") {
     val schema: Schema[Upload] = {
-      val error = Schema
-        .record[Error] { field =>
-          field.const("type", "error") *> (
-            field("msg", _.msg),
-            field("cause", _.cause)
-          ).mapN(Error.apply)
-        }
-
-      val warning = Schema
-        .record[Warning] { field =>
-          field.const("type", "warning") *> (
-            field("msg", _.msg),
-            field("cause", _.cause)
-          ).mapN(Warning.apply)
-        }
-
-      val unknown =
-        Schema.record[Unknown.type](
-          _.const("type", "unknown").as(Unknown)
-        )
-
-      val successful = Schema
-        .record[Successful] { field =>
-          field.const("type", "successful") *> (
-            field("link", _.link),
-            field("expires", _.expires)
-          ).mapN(Successful.apply)
-        }
 
       Schema.record[Upload] { field =>
         (
           field("id", _.id),
-          field("status", _.status) {
-            Schema.oneOf { alt =>
-              alt(error) |+| alt(warning) |+| alt(unknown) |+| alt(successful)
-            }
-          }
+          field("status", _.status)(statusSchema)
         ).mapN(Upload.apply)
       }
     }
@@ -962,6 +963,18 @@ class SchemaSuite extends ScalaCheckSuite {
     check(schema, text, expected)
   }
 
+  test("ADTs, aggregated errors") {
+    val errorMessage = statusSchema.read(DynamoValue.n(5)).leftMap(_.message)
+    assert(
+      errorMessage.swap.exists(msg =>
+        // Don't want to test the full message, I just care that there is the list of failures
+        msg.contains("alternatives: [") && msg
+          .contains("]")
+      ),
+      errorMessage
+    )
+  }
+
   val compileTimeInferenceSpec = {
     val userSchema: Schema[User] = Schema.record { field =>
       (

diff --git a/project/build.properties b/project/build.properties
@@ -1 +1 @@
-sbt.version=1.7.2
+sbt.version=1.8.2