Skip to content

Commit

Permalink
Clarify failure semantics for Param & QueryParam parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
noelwelsh committed Feb 10, 2024
1 parent 37cb9f3 commit fc4b086
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 60 deletions.
4 changes: 2 additions & 2 deletions core/jvm/src/test/scala/krop/route/ParamSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ class ParamSuite extends FunSuite {
using munit.Location
) =
values.foreach { case (str, a) =>
assertEquals(param.parse(str), Success(a))
assertEquals(param.parse(str), Right(a))
}

def paramOneParsesInvalid[A](param: Param.One[A], values: Seq[String])(using
munit.Location
) =
values.foreach { (str) => assert(param.parse(str).isFailure) }
values.foreach { (str) => assert(param.parse(str).isLeft) }

test("Param.one parses valid parameter") {
paramOneParsesValid(
Expand Down
23 changes: 10 additions & 13 deletions core/jvm/src/test/scala/krop/route/QueryParamSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,79 +18,76 @@ package krop.route

import munit.FunSuite

import scala.util.Failure
import scala.util.Success

class QueryParamSuite extends FunSuite {
test("Required QueryParam succeeds if first value parses") {
val qp = QueryParam("id", Param.int)
assertEquals(
qp.parse(Map("id" -> List("1"), "name" -> List("Van Gogh"))),
Success(1)
Right(1)
)
assertEquals(
qp.parse(Map("id" -> List("1", "foobar"), "name" -> List("Van Gogh"))),
Success(1)
Right(1)
)
}

test("Required QueryParam fails if first value fails to parse") {
val qp = QueryParam("id", Param.int)
assertEquals(
qp.parse(Map("id" -> List("abc"))),
Failure(QueryParseException.ValueParsingFailed("id", "abc", Param.int))
Left(QueryParseFailure.ValueParsingFailed("id", "abc", Param.int))
)
}

test("Required QueryParam fails if no values are found for name") {
val qp = QueryParam("id", Param.int)
assertEquals(
qp.parse(Map("id" -> List())),
Failure(QueryParseException.NoValuesForName("id"))
Left(QueryParseFailure.NoValuesForName("id"))
)
}

test("Required QueryParam fails if name does not exist") {
val qp = QueryParam("id", Param.int)
assertEquals(
qp.parse(Map("foo" -> List("1"))),
Failure(QueryParseException.NoParameterWithName("id"))
Left(QueryParseFailure.NoParameterWithName("id"))
)
}

test("Optional QueryParam succeeds if first value parses") {
val qp = QueryParam.optional("id", Param.int)
assertEquals(
qp.parse(Map("id" -> List("1"), "name" -> List("Van Gogh"))),
Success(Some(1))
Right(Some(1))
)
assertEquals(
qp.parse(Map("id" -> List("1", "foobar"), "name" -> List("Van Gogh"))),
Success(Some(1))
Right(Some(1))
)
}

test("Optional QueryParam fails if first value fails to parse") {
val qp = QueryParam.optional("id", Param.int)
assertEquals(
qp.parse(Map("id" -> List("abc"))),
Failure(QueryParseException.ValueParsingFailed("id", "abc", Param.int))
Left(QueryParseFailure.ValueParsingFailed("id", "abc", Param.int))
)
}

test("Optional QueryParam succeeds if no values are found for name") {
val qp = QueryParam.optional("id", Param.int)
assertEquals(
qp.parse(Map("id" -> List())),
Success(None)
Right(None)
)
}

test("Optional QueryParam succeeds if name does not exist") {
val qp = QueryParam.optional("id", Param.int)
assertEquals(
qp.parse(Map("foo" -> List("1"))),
Success(None)
Right(None)
)
}
}
18 changes: 10 additions & 8 deletions core/shared/src/main/scala/krop/route/Param.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import cats.syntax.all.*

import java.util.regex.Pattern
import scala.collection.immutable.ArraySeq
import scala.util.Success
import scala.util.Try

/** A [[package.Param]] is used to extract values from a URI's path or query
* parameters.
Expand Down Expand Up @@ -77,7 +75,7 @@ object Param {
*/
final case class All[A](
name: String,
parse: Seq[String] => Try[A],
parse: Seq[String] => Either[ParamParseFailure, A],
unparse: A => Seq[String]
) extends Param[A] {

Expand All @@ -100,7 +98,7 @@ object Param {
*/
final case class One[A](
name: String,
parse: String => Try[A],
parse: String => Either[ParamParseFailure, A],
unparse: A => String
) extends Param[A] {

Expand All @@ -113,16 +111,20 @@ object Param {

/** A `Param` that matches a single `Int` parameter */
val int: Param.One[Int] =
Param.One("<Int>", str => Try(str.toInt), _.toString)
Param.One(
"<Int>",
str => str.toIntOption.toRight(ParamParseFailure(str, "<Int>")),
_.toString
)

/** A `Param` that matches a single `String` parameter */
val string: Param.One[String] =
Param.One("<String>", Success(_), identity)
Param.One("<String>", Right(_), identity)

/** `Param` that simply accumulates all parameters as a `Seq[String]`.
*/
val seq: Param.All[Seq[String]] =
Param.All("<String>", Success(_), identity)
Param.All("<String>", Right(_), identity)

/** `Param` that matches all parameters and converts them to a `String` by
* adding `separator` between matched elements. The inverse splits on this
Expand All @@ -132,7 +134,7 @@ object Param {
val quotedSeparator = Pattern.quote(separator)
Param.All(
s"<String>${separator}",
seq => Success(seq.mkString(separator)),
seq => Right(seq.mkString(separator)),
string => ArraySeq.unsafeWrapArray(string.split(quotedSeparator))
)
}
Expand Down
19 changes: 19 additions & 0 deletions core/shared/src/main/scala/krop/route/ParamParseFailure.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright 2023 Creative Scala
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package krop.route

final case class ParamParseFailure(value: String, description: String)
10 changes: 4 additions & 6 deletions core/shared/src/main/scala/krop/route/Path.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import org.http4s.Uri.Path as UriPath
import scala.annotation.tailrec
import scala.collection.mutable
import scala.compiletime.constValue
import scala.util.Failure
import scala.util.Success

/** A [[krop.route.Path]] represents a pattern to match against the path
* component of the URI of a request.`Paths` are created by calling the `/`
Expand Down Expand Up @@ -222,8 +220,8 @@ final class Path[P <: Tuple, Q <: Tuple] private (
if pathSegments.isEmpty then None
else
parse(pathSegments(0).decoded()) match {
case Failure(_) => None
case Success(value) =>
case Left(_) => None
case Right(value) =>
loop(matchSegments.tail, pathSegments.tail) match {
case None => None
case Some(tail) => Some(value *: tail)
Expand All @@ -232,8 +230,8 @@ final class Path[P <: Tuple, Q <: Tuple] private (

case Param.All(_, parse, _) =>
parse(pathSegments.map(_.decoded())) match {
case Failure(_) => None
case Success(value) => Some(value *: EmptyTuple)
case Left(_) => None
case Right(value) => Some(value *: EmptyTuple)
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions core/shared/src/main/scala/krop/route/Query.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package krop.route

import cats.syntax.all.*

import scala.util.Try

final case class Query[A <: Tuple](segments: Vector[QueryParam[?]]) {
//
// Combinators ---------------------------------------------------------------
Expand All @@ -38,7 +36,7 @@ final case class Query[A <: Tuple](segments: Vector[QueryParam[?]]) {
// Interpreters --------------------------------------------------------------
//

def parse(params: Map[String, List[String]]): Try[A] =
def parse(params: Map[String, List[String]]): Either[QueryParseFailure, A] =
segments
.traverse(s => s.parse(params))
.map(v => Tuple.fromArray(v.toArray).asInstanceOf[A])
Expand Down
43 changes: 20 additions & 23 deletions core/shared/src/main/scala/krop/route/QueryParam.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,9 @@

package krop.route

import cats.syntax.all.*
import krop.route.Param.One

import scala.util.Failure
import scala.util.Success
import scala.util.Try

/** A [[package.QueryParam]] extracts values from a URI's query parameters. It
* consists of a [[package.Param]], which does the necessary type conversion,
* and the name under which the parameters should be found.
Expand All @@ -37,7 +34,7 @@ import scala.util.Try
* * the `QueryParam` that returns all the query parameters.
*/
enum QueryParam[A] {
import QueryParseException.*
import QueryParseFailure.*

/** Get a human-readable description of this `QueryParam`. */
def describe: String =
Expand All @@ -47,50 +44,50 @@ enum QueryParam[A] {
case All => "all"
}

def parse(params: Map[String, List[String]]): Try[A] =
def parse(params: Map[String, List[String]]): Either[QueryParseFailure, A] =
this match {
case Required(name, param) =>
params.get(name) match {
case Some(values) =>
param match {
case Param.All(_, parse, _) => parse(values)
case Param.All(_, parse, _) =>
parse(values).left.map(f =>
ValueParsingFailed(name, f.value, param)
)
case Param.One(_, parse, _) =>
if values.isEmpty then Failure(NoValuesForName(name))
if values.isEmpty then NoValuesForName(name).asLeft
else {
val hd = values.head
parse(hd).recoverWith(_ =>
Failure(
QueryParseException.ValueParsingFailed(name, hd, param)
)
)
parse(hd).left.map(_ => ValueParsingFailed(name, hd, param))
}
}
case None => Failure(NoParameterWithName(name))
case None => NoParameterWithName(name).asLeft
}

case Optional(name, param) =>
params.get(name) match {
case Some(values) =>
param match {
case Param.All(_, parse, _) => parse(values).map(Some(_))
case Param.All(_, parse, _) =>
parse(values)
.map(Some(_))
.left
.map(f => ValueParsingFailed(name, f.value, param))
case Param.One(_, parse, _) =>
if values.isEmpty then Success(None)
if values.isEmpty then None.asRight
else {
val hd = values.head
parse(hd)
.map(Some(_))
.recoverWith(_ =>
Failure(
QueryParseException.ValueParsingFailed(name, hd, param)
)
)
.left
.map(_ => ValueParsingFailed(name, hd, param))
}
}

case None => Success(None)
case None => None.asRight
}

case All => Success(params)
case All => params.asRight
}

def unparse(a: A): Option[(String, List[String])] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@

package krop.route

/** Exception raised when query parsing fails. */
enum QueryParseException(message: String) extends Exception(message) {
/** Failure raised when query parsing fails. */
enum QueryParseFailure(val message: String) {

/** Query parameter parsing failed because no parameter with the given name
* was found in the query parameters.
*/
case NoParameterWithName(name: String)
extends QueryParseException(
extends QueryParseFailure(
s"There was no query parameter with the name ${name}."
)

Expand All @@ -32,12 +32,12 @@ enum QueryParseException(message: String) extends Exception(message) {
* with any values.
*/
case NoValuesForName(name: String)
extends QueryParseException(
extends QueryParseFailure(
s"There were no values associated with the name ${name}"
)

case ValueParsingFailed(name: String, value: String, param: Param[?])
extends QueryParseException(
extends QueryParseFailure(
s"Parsing the value ${value} as ${param.describe} failed for the query parameter ${name}"
)
}

0 comments on commit fc4b086

Please sign in to comment.