Schema based header codecs, unified with query codecs (#3232) #3270
Conversation
987Nabil
force-pushed
the
header-codec-restructure
branch
7 times, most recently
from
e69771b to
0bb55d3
Compare
Fix for publish CI job
987Nabil
force-pushed
the
header-codec-restructure
branch
from
0bb55d3 to
c9c2c22
Compare
| @@ -2409,7 +2494,34 @@ object HttpCodec extends ContentCodecs with HeaderCodecs with MethodCodecs with | |||
| def index(index: Int): Method[A] = copy(index = index) | |||
| } | |||
|
|
|||
| private[http] final case class Header[A](name: String, textCodec: TextCodec[A], index: Int = 0) | |||
| private[http] final case class HeaderCustom[A](codec: SchemaCodec[A], index: Int = 0) | |||
There was a problem hiding this comment.
Isn't this just the same as name = None?
| import zio.http.Charsets | ||
|
|
||
| object StringCodec { | ||
| type StringCodec[A] = Codec[String, Char, A] |
There was a problem hiding this comment.
I'd rather put this in a final case class StringCodec(codec: Codec[String, Char, A]) than have a type alias for it.
Remind me why we need this?
| } | ||
| } | ||
|
|
||
| implicit def fromSchema[A](implicit schema: Schema[A]): Codec[String, Char, A] = { |
There was a problem hiding this comment.
Can we do this whole thing as Schema#toString(value) and Schema#fromString(string)?? Why does this even belong here?
| case HttpCodec.AtomTag.Content => copy(content = f(content)) | ||
| case HttpCodec.AtomTag.Query => copy(query = f(query)) | ||
| case HttpCodec.AtomTag.Header => copy(header = f(header)) | ||
| case HttpCodec.AtomTag.HeaderCustom => copy(headerCustom = f(header)) |
There was a problem hiding this comment.
It's painful to see Header and HeaderCustom when all the other cases just have a single codec!
| .unsafeQueryParam(name) == "" && !emptyStringIsValue(codec.codec.schema))) && hasDefault | ||
| ) | ||
| default | ||
| else if (!hasParam) |
There was a problem hiding this comment.
This complicated and fragile code suggests one way to refactor this is as a method on the query codec which returns a sealed trait containing the right information.
In other words, instead of, if (isA) then extractA else if (isB) then extractB, you can just have a thing match { case A(a) => ... }
| else decoded | ||
| } | ||
|
|
||
| } else if (codec.isCollection) { |
There was a problem hiding this comment.
Definitely need to adopt the above pattern to deal with this. Some pre-analysis with pre-collection of required information, packaged into a sealed trait.
| } | ||
| validateDecoded(codec, decoded) | ||
| } | ||
| if (optional) { |
Fix for publish CI job
fixes #3232
/claim #3232
fixes #3255
fixes #3240