From a1598bf7e91cd58ddfd579cec06cb7083ab34474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabi=C3=A1n=20Heredia=20Montiel?= Date: Thu, 2 Nov 2023 18:39:18 -0600 Subject: [PATCH] [dsl] Use constrainted constructors for PathVar as per code review --- docs/docs/dsl.md | 8 +++++-- .../main/scala/org/http4s/dsl/Http4sDsl.scala | 9 +++----- .../main/scala/org/http4s/dsl/impl/Path.scala | 21 +++++++++++++----- .../main/scala/org/http4s/dsl/request.scala | 4 +--- .../test/scala/org/http4s/dsl/PathSuite.scala | 22 +++++++++++++++++-- 5 files changed, 46 insertions(+), 18 deletions(-) diff --git a/docs/docs/dsl.md b/docs/docs/dsl.md index 10d19a9cbc6..d76e1c3e541 100644 --- a/docs/docs/dsl.md +++ b/docs/docs/dsl.md @@ -424,13 +424,17 @@ val usersService = HttpRoutes.of[IO] { ``` If you want to extract a variable of type `A`, you can provide a custom extractor -which implements `cast: String => Try[A]`, similar to the way in which `IntVar` does it. +which via one of the following: + +- `PathVar.pure[A](cast: String => A)` +- `PathVar.fromPartialFunction[A](cast: PartialFunction[String, A])` +- `PathVar.fromTry[A](cast: String => Try[A])` ```scala mdoc:silent import java.time.LocalDate import scala.util.Try -val LocalDateVar = PathVar(str => Try(LocalDate.parse(str))) +val LocalDateVar = PathVar.fromTry(str => Try(LocalDate.parse(str))) def getTemperatureForecast(date: LocalDate): IO[Double] = IO(42.23) diff --git a/dsl/src/main/scala/org/http4s/dsl/Http4sDsl.scala b/dsl/src/main/scala/org/http4s/dsl/Http4sDsl.scala index b9310096d9b..b36aaa1da64 100644 --- 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 index 790c0d0e474..831e35f63f4 100644 --- a/dsl/src/main/scala/org/http4s/dsl/impl/Path.scala +++ b/dsl/src/main/scala/org/http4s/dsl/impl/Path.scala @@ -177,19 +177,30 @@ 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: * {{{ @@ -197,7 +208,7 @@ object IntVar extends PathVar(str => Try(str.toInt)) * 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: * {{{ @@ -205,7 +216,7 @@ object LongVar extends PathVar(str => Try(str.toLong)) * 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 index 1d71c68e2a5..cd38104a2c3 100644 --- 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 index 5983fdec805..df4275d92aa 100644 --- 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 }) }