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

Improve RichTextCodec decoding performance #2512

Merged
merged 4 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 6 additions & 9 deletions zio-http/src/main/scala/zio/http/Header.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import scala.util.{Either, Failure, Success, Try}
import zio._

import zio.http.codec.RichTextCodec
import zio.http.endpoint.openapi.OpenAPI.SecurityScheme.Http
import zio.http.internal.DateEncoding

sealed trait Header {
Expand Down Expand Up @@ -2480,16 +2481,12 @@ object Header {
private val codec: RichTextCodec[ContentType] = {

// char `.` according to BNF not allowed as `token`, but here tolerated
val token = RichTextCodec.filter(_ => true).validate("not a token") {
case ' ' | '(' | ')' | '<' | '>' | '@' | ',' | ';' | ':' | '\\' | '"' | '/' | '[' | ']' | '?' | '=' => false
case _ => true
}
val tokenQuoted = RichTextCodec.filter(_ => true).validate("not a quoted token") {
case ' ' | '"' => false
case _ => true
}
val token = RichTextCodec.charsNot(' ', '(', ')', '<', '>', '@', ',', ';', ':', '\\', '"', '/', '[', ']', '?', '=')

val tokenQuoted = RichTextCodec.charsNot(' ', '"')

val type1 = RichTextCodec.string.collectOrFail("unsupported main type") {
case value if MediaType.mainTypeMap.get(value).isDefined => value
case value if MediaType.mainTypeMap.contains(value) => value
}
val type1x = (RichTextCodec.literalCI("x-") ~ token.repeat.string).transform[String](in => s"${in._1}${in._2}")(in => ("x-", s"${in.substring(2)}"))
val codecType1 = (type1 | type1x).transform[String](_.merge) {
Expand Down
39 changes: 32 additions & 7 deletions zio-http/src/main/scala/zio/http/codec/RichTextCodec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import java.lang.Integer.parseInt
import scala.annotation.tailrec
import scala.collection.immutable.BitSet

import zio.stacktracer.TracingImplicits.disableAutoTrace
import zio.{Chunk, NonEmptyChunk}

/**
Expand Down Expand Up @@ -108,17 +107,25 @@ sealed trait RichTextCodec[A] { self =>
*/
final def encode(value: A): Either[String, String] = RichTextCodec.encode(value, self)

/**
* This method is Right biased merge
*/
final def merge[B](implicit ev: A <:< Either[B, B]): RichTextCodec[B] = {
val codec = self.asInstanceOf[RichTextCodec[Either[B, B]]]
codec.transform[B](_.merge)(Right(_))
}

final def optional(default: A): RichTextCodec[Option[A]] =
self.transform[Option[A]](a => Some(a))(_.fold(default)(identity))

lazy val repeat: RichTextCodec[Chunk[A]] =
((self ~ repeat).transform[NonEmptyChunk[A]](t => NonEmptyChunk(t._1, t._2: _*))(c =>
(c.head, c.tail),
) | RichTextCodec.empty.as(Chunk.empty[A]))
.transform[Chunk[A]](_ match {
.transform[Chunk[A]] {
case Left(nonEmpty) => nonEmpty
case Right(maybeEmpty) => maybeEmpty
})(c => c.nonEmptyOrElse[Either[NonEmptyChunk[A], Chunk[A]]](Right(c))(Left(_)))
}(c => c.nonEmptyOrElse[Either[NonEmptyChunk[A], Chunk[A]]](Right(c))(Left(_)))

final def singleton: RichTextCodec[NonEmptyChunk[A]] =
self.transform(a => NonEmptyChunk(a))(_.head)
Expand Down Expand Up @@ -151,18 +158,24 @@ sealed trait RichTextCodec[A] { self =>
case x if p(x) => x
}

final def withError(errorMessage: String): RichTextCodec[A] =
(self | RichTextCodec.fail[A](errorMessage)).merge

}
object RichTextCodec {
private[codec] case object Empty extends RichTextCodec[Unit]
private[codec] final case class CharIn(set: BitSet) extends RichTextCodec[Char]
private[codec] final case class CharIn(set: BitSet) extends RichTextCodec[Char] {
val errorMessage: Left[String, Nothing] =
Left(s"Expected, but did not find: ${this.describe}")
}
private[codec] final case class TransformOrFail[A, B](
codec: RichTextCodec[A],
to: A => Either[String, B],
from: B => Either[String, A],
) extends RichTextCodec[B]
private[codec] final case class Alt[A, B](left: RichTextCodec[A], right: RichTextCodec[B])
extends RichTextCodec[Either[A, B]]
private[codec] final case class Lazy[A](codec0: () => RichTextCodec[A]) extends RichTextCodec[A] {
private[codec] final case class Lazy[A](codec0: () => RichTextCodec[A]) extends RichTextCodec[A] {
lazy val codec: RichTextCodec[A] = codec0()
}
private[codec] final case class Zip[A, B, C](
Expand All @@ -188,6 +201,12 @@ object RichTextCodec {
*/
def char(c: Char): RichTextCodec[Char] = CharIn(BitSet(c.toInt))

def chars(cs: Char*): RichTextCodec[Char] =
CharIn(BitSet(cs.map(_.toInt): _*))

def charsNot(cs: Char*): RichTextCodec[Char] =
filter(c => !cs.contains(c))

/**
* A codec that describes a digit character.
*/
Expand All @@ -200,13 +219,19 @@ object RichTextCodec {
*/
val empty: RichTextCodec[Unit] = Empty

def fail[A](message: String): RichTextCodec[A] =
empty.transformOrFail(_ => Left(message))(_ => Left(message))

/**
* Defines a new codec for a single character based on the specified
* predicate.
*/
def filter(pred: Char => Boolean): RichTextCodec[Char] =
CharIn(BitSet((Char.MinValue to Char.MaxValue).filter(pred).map(_.toInt): _*))

def filterOrFail(pred: Char => Boolean)(failure: String): RichTextCodec[Char] =
filter(pred).collectOrFail(failure) { case c => c }

/**
* A codec that describes a letter character.
*/
Expand Down Expand Up @@ -528,9 +553,9 @@ object RichTextCodec {
case Empty =>
Right((value, ()))

case CharIn(bitset) =>
case self @ CharIn(bitset) =>
if (value.length == 0 || !bitset.contains(value.charAt(0).toInt))
Left(s"Not found: ${bitset.toArray.map(_.toChar).mkString}")
self.errorMessage
else
Right((value.subSequence(1, value.length), value.charAt(0)))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ object RichTextCodecSpec extends ZIOHttpSpec {
assertTrue(success(123) == codec.decode("123--")) &&
assertTrue(codec.decode("4123").isLeft)
},
test("With error message") {
val codec = RichTextCodec.literal("123").withError("Not 123")
assertTrue(codec.decode("678") == Left("(Expected, but did not find: Paragraph(Code(“1”,Inline)), Not 123)"))
Copy link
Member

Choose a reason for hiding this comment

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

This is rendering poorly, perhaps because describe returns Doc, that should first be properly converted into a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is actually the | operator, that chains error messages this way explicitly.

},
),
)
}
Loading