Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added getAs for QueryParams #2505

Merged
merged 5 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion zio-http/src/main/scala/zio/http/QueryParams.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ package zio.http

import java.nio.charset.Charset

import zio.Chunk
import zio.{Chunk, NonEmptyChunk}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NonEmptyChunk is unused or?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right - I left some unused imports, I would fix it


import zio.http.Charsets
import zio.http.codec.{HttpCodecError, TextCodec}
import zio.http.internal.QueryParamEncoding

/**
Expand Down Expand Up @@ -88,25 +89,62 @@ final case class QueryParams(map: Map[String, Chunk[String]]) {
*/
def getAll(key: String): Option[Chunk[String]] = map.get(key)

/**
* Retrieves all typed query parameter values having the specified name.
*/
def getAllAs[A](key: String)(implicit codec: TextCodec[A]): Either[QueryParamsError, Chunk[A]] =
map.get(key) match {
case Some(params) =>
params
.map(param => codec.decode(param).toRight(QueryParamsError.MalformedQueryParam(key, param, codec)))
.partitionMap(identity) match {
case (errors, _) if errors.nonEmpty => Left(QueryParamsError.MultiMalformedQueryParam(errors))
case (_, typedParams) => Right(typedParams)
}
case None => Left(QueryParamsError.MissingQueryParam(key))
}

/**
* Retrieves the first query parameter value having the specified name.
*/
def get(key: String): Option[String] = getAll(key).flatMap(_.headOption)

/**
* Retrieves the first typed query parameter value having the specified name.
*/
def getAs[A](key: String)(implicit codec: TextCodec[A]): Either[QueryParamsError, A] = for {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering, if a def getAsZIO[A](key: String)(implicit codec: TextCodec[A]): IO[QueryParamsError, A] makes sense too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't wrapping it in ZIO to heavy machinery for this task? I believe calling it like ZIO.fromEither(queryParams.getAs ...) is not much trouble, and current version could be possibly used in some lower level implementations :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there could be a significant gain in readability at the cost of a minor performance overhead. Keeping getAs as is and adding getAsZIO as a variant seems consistent with existing ZIO functions like ZIO.serviceWith and ZIO.serviceWithZIO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining :) You are right, I will add additional functions

param <- get(key).toRight(QueryParamsError.MissingQueryParam(key))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! QueryParamsError.Missing seems more readable to me. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems logical :)

typedParam <- codec.decode(param).toRight(QueryParamsError.MalformedQueryParam(key, param, codec))
} yield typedParam

/**
* Retrieves all query parameter values having the specified name, or else
* uses the default iterable.
*/
def getAllOrElse(key: String, default: => Iterable[String]): Chunk[String] =
getAll(key).getOrElse(Chunk.fromIterable(default))

/**
* Retrieves all query parameter values having the specified name, or else
* uses the default iterable.
*/
def getAllAsOrElse[A](key: String, default: => Iterable[A])(implicit codec: TextCodec[A]): Chunk[A] =
getAllAs[A](key).getOrElse(Chunk.fromIterable(default))

/**
* Retrieves the first query parameter value having the specified name, or
* else uses the default value.
*/
def getOrElse(key: String, default: => String): String =
get(key).getOrElse(default)

/**
* Retrieves the first typed query parameter value having the specified name,
* or else uses the default value.
*/
def getAsOrElse[A](key: String, default: => A)(implicit codec: TextCodec[A]): A =
getAs[A](key).getOrElse(default)

override def hashCode: Int = normalize.map.hashCode

/**
Expand Down
44 changes: 44 additions & 0 deletions zio-http/src/main/scala/zio/http/QueryParamsError.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2021 - 2023 Sporta Technologies PVT LTD & the ZIO HTTP contributors.
*
* 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 zio.http

import java.nio.charset.Charset

import scala.util.control.NoStackTrace

import zio.Chunk

import zio.http.codec.TextCodec
import zio.http.internal.QueryParamEncoding

sealed trait QueryParamsError extends Exception with NoStackTrace {
override def getMessage(): String = message
def message: String
}
object QueryParamsError {
final case class MissingQueryParam(queryParamName: String) extends QueryParamsError {
def message = s"Missing query parameter with name $queryParamName"
}

final case class MalformedQueryParam(name: String, value: String, codec: TextCodec[_]) extends QueryParamsError {
def message = s"Unable to decode query parameter with name $name and value $value using $codec"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt that TextCodec always provides stable toString. codec.describe might be more reliable choice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is true.

}

final case class MultiMalformedQueryParam(chunk: Chunk[MalformedQueryParam]) extends QueryParamsError {
def message: String = chunk.map(_.getMessage()).mkString("; ")
}
}
37 changes: 33 additions & 4 deletions zio-http/src/test/scala/zio/http/QueryParamsSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,39 @@ object QueryParamsSpec extends ZIOHttpSpec {
val default = "default"
val unknown = "non-existent"
val queryParams = QueryParams(name -> "a", name -> "b")
assertTrue(queryParams.get(name).get == "a") &&
assertTrue(queryParams.getOrElse(unknown, default) == default) &&
assertTrue(queryParams.getAll(name).get.length == 2) &&
assertTrue(queryParams.getAllOrElse(unknown, Chunk(default)).length == 1)
assertTrue(
queryParams.get(name).get == "a",
queryParams.get(unknown).isEmpty,
queryParams.getOrElse(name, default) == "a",
queryParams.getOrElse(unknown, default) == default,
queryParams.getAll(name).get.length == 2,
queryParams.getAll(unknown).isEmpty,
queryParams.getAllOrElse(name, Chunk(default)).length == 2,
queryParams.getAllOrElse(unknown, Chunk(default)).length == 1,
)
},
),
suite("getAs - getAllAs")(
test("success") {
val typed = "typed"
val default = 3
val invalidTyped = "invalidTyped"
val unknown = "non-existent"
val queryParams = QueryParams(typed -> "1", typed -> "2", invalidTyped -> "str")
assertTrue(
queryParams.getAs[Int](typed) == Right(1),
queryParams.getAs[Int](invalidTyped).isLeft,
queryParams.getAs[Int](unknown).isLeft,
queryParams.getAsOrElse[Int](typed, default) == 1,
queryParams.getAsOrElse[Int](invalidTyped, default) == default,
queryParams.getAsOrElse[Int](unknown, default) == default,
queryParams.getAllAs[Int](typed).map(_.length) == Right(2),
queryParams.getAllAs[Int](invalidTyped).isLeft,
queryParams.getAllAs[Int](unknown).isLeft,
queryParams.getAllAsOrElse[Int](typed, Chunk(default)).length == 2,
queryParams.getAllAsOrElse[Int](invalidTyped, Chunk(default)).length == 1,
queryParams.getAllAsOrElse[Int](unknown, Chunk(default)).length == 1,
)
},
),
suite("encode - decode")(
Expand Down
Loading