[dsl] Use constrainted constructors for PathVar as per code review

LibreCybernetics · Nov 3, 2023 · cca7b81 · cca7b81
1 parent 9561c09
commit cca7b81
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 16 deletions.
diff --git a/dsl/src/main/scala/org/http4s/dsl/Http4sDsl.scala b/dsl/src/main/scala/org/http4s/dsl/Http4sDsl.scala
@@ -17,11 +17,8 @@
 package org.http4s.dsl
 
 import cats.arrow.FunctionK
-import org.http4s.Method
-import org.http4s.Uri
-import org.http4s.dsl.impl._
-
-import scala.util.Try
+import org.http4s.dsl.impl.*
+import org.http4s.{Method, Uri}
 
 trait Http4sDsl2[F[_], G[_]] extends RequestDsl with Statuses with Responses[F, G] {
   val Path: Uri.Path.type = Uri.Path
@@ -46,7 +43,7 @@ trait Http4sDsl2[F[_], G[_]] extends RequestDsl with Statuses with Responses[F,
     */
   val → : impl.->.type = impl.->
 
-  def PathVar[A](cast: String => Try[A]): impl.PathVar[A] = new impl.PathVar[A](cast)
+  val PathVar: impl.PathVar.type = impl.PathVar
 
   val IntVar: impl.IntVar.type = impl.IntVar
   val LongVar: impl.LongVar.type = impl.LongVar

diff --git a/dsl/src/main/scala/org/http4s/dsl/impl/Path.scala b/dsl/src/main/scala/org/http4s/dsl/impl/Path.scala
@@ -177,35 +177,45 @@ object /: {
   *     case Root / ColorPath(color) => ...
   * }}}
   */
-class PathVar[A](cast: String => Try[A]) {
+class PathVar[A] private[impl] (cast: String => Option[A]) {
   def unapply(str: String): Option[A] =
-    if (str.nonEmpty) cast(str).toOption
+    if (str.nonEmpty) cast(str)
     else None
 }
 
+object PathVar {
+  def pure[A](cast: String => A): PathVar[A] =
+    new PathVar(str => Option(cast(str)))
+
+  def fromPartialFunction[A](cast: PartialFunction[String, A]): PathVar[A] =
+    new PathVar(str => cast.lift(str))
+  def fromTry[A](cast: String => Try[A]): PathVar[A] =
+    new PathVar(str => cast(str).toOption)
+}
+
 /** Integer extractor of a path variable:
   * {{{
   *   Path("/user/123") match {
   *      case Root / "user" / IntVar(userId) => ...
   * }}}
   */
-object IntVar extends PathVar(str => Try(str.toInt))
+object IntVar extends PathVar(str => Try(str.toInt).toOption)
 
 /** Long extractor of a path variable:
   * {{{
   *   Path("/user/123") match {
   *      case Root / "user" / LongVar(userId) => ...
   * }}}
   */
-object LongVar extends PathVar(str => Try(str.toLong))
+object LongVar extends PathVar(str => Try(str.toLong).toOption)
 
 /** UUID extractor of a path variable:
   * {{{
   *   Path("/user/13251d88-7a73-4fcf-b935-54dfae9f023e") match {
   *      case Root / "user" / UUIDVar(userId) => ...
   * }}}
   */
-object UUIDVar extends PathVar(str => Try(java.util.UUID.fromString(str)))
+object UUIDVar extends PathVar(str => Try(java.util.UUID.fromString(str)).toOption)
 
 /** Matrix path variable extractor
   * For an example see [[https://www.w3.org/DesignIssues/MatrixURIs.html MatrixURIs]]

diff --git a/dsl/src/main/scala/org/http4s/dsl/request.scala b/dsl/src/main/scala/org/http4s/dsl/request.scala
@@ -18,8 +18,6 @@ package org.http4s.dsl
 
 import org.http4s.Uri
 
-import scala.util.Try
-
 object request extends RequestDslBinCompat {
   val Path: Uri.Path.type = Uri.Path
   val Root: Uri.Path.Root.type = Uri.Path.Root
@@ -30,7 +28,7 @@ object request extends RequestDslBinCompat {
   val /: : impl./:.type = impl./:
   val +& : impl.+&.type = impl.+&
 
-  def PathVar[A](cast: String => Try[A]): impl.PathVar[A] = new impl.PathVar[A](cast)
+  val PathVar: impl.PathVar.type = impl.PathVar
 
   val IntVar: impl.IntVar.type = impl.IntVar
   val LongVar: impl.LongVar.type = impl.LongVar

diff --git a/dsl/src/test/scala/org/http4s/dsl/PathSuite.scala b/dsl/src/test/scala/org/http4s/dsl/PathSuite.scala
@@ -141,11 +141,29 @@ class PathSuite extends Http4sSuite with AllSyntax {
         case "Green" => Green
         case "Blue" => Blue
       }
+
+      def partialFunction: PartialFunction[String, Color] = {
+        case "Red" => Red
+        case "Green" => Green
+        case "Blue" => Blue
+      }
     }
 
-    val ColorVar = PathVar(str => Try(Color.valueOf(str)))
+    val ColorVarPure = PathVar.pure(Color.valueOf)
+    assert(path"/Green" match {
+      case Root / ColorVarPure(color) => color == Color.Green
+      case _ => false
+    })
+
+    val ColorVarPF = PathVar.fromPartialFunction(Color.partialFunction)
+    assert(path"/Green" match {
+      case Root / ColorVarPF(color) => color == Color.Green
+      case _ => false
+    })
+
+    val ColorVarTry = PathVar.fromTry(str => Try(Color.valueOf(str)))
     assert(path"/Green" match {
-      case Root / ColorVar(color) => color == Color.Green
+      case Root / ColorVarTry(color) => color == Color.Green
       case _ => false
     })
   }