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

Fix issue 290. #291

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@kkreuning
Copy link
Contributor

commented May 10, 2019

Fixes #290

This fix generates implicit query param decoders looking like this:

implicit val `java.time.LocalDateQueryParamDecoder`: QueryParamDecoder[java.time.LocalDate] = (value: QueryParameterValue) => Json.fromString(value.value).as[java.time.LocalDate].leftMap(t => ParseFailure("Query decoding failed", t.getMessage)).toValidatedNel

It's not pretty but it works, then again implicits are not shown to the user and the compiler doesn't care about nice names...

Another option would be to have default QueryParamDecoder for types like java.time.LocalDate (and java.time.LocalDateTime etc.) since they are 'standard' types returned by the Swagger generator. That way only custom types (using x-scala-type) would need specific generated implicits. Any ideas on this?

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

@kkreuning kkreuning force-pushed the kkreuning:master branch from b1cce58 to 5836a22 May 10, 2019

@kkreuning kkreuning force-pushed the kkreuning:master branch from 07657cd to a876ab7 May 10, 2019

@@ -636,7 +636,7 @@ object Http4sServerGenerator {
}
if (!List("Unit", "Boolean", "Double", "Float", "Short", "Int", "Long", "Char", "String").contains(elemType.toString())) {
val queryParamDecoder = q"""
implicit val ${Pat.Var(Term.Name(s"${argName.value}QueryParamDecoder"))}: QueryParamDecoder[$elemType] = (value: QueryParameterValue) =>
implicit val ${Pat.Var(Term.Name(s"${elemType}QueryParamDecoder"))}: QueryParamDecoder[$elemType] = (value: QueryParameterValue) =>

This comment has been minimized.

Copy link
@blast-hardcheese

blast-hardcheese May 12, 2019

Collaborator

These elemTypes are not guaranteed to be fully qualified, as we won't always know that information, consider:

- name: foo
  x-scala-type: Foo
- name: foo2
  x-scala-type: com.example.Foo

given imports=List("com.example._") in build.sbt.

I wonder if

implicit def decoderDecoder[A: Decoder]: QueryParamDecoder[A] =
  Json.fromString(value.value).as[A]
    .leftMap(t => ParseFailure("Query decoding failed", t.getMessage))
    .toValidatedNel

would be more stable, relying on scalac to do the wiring up for us.

One unfortunate aspect here is that Json.fromString prevents non-string query parameters. I'd expect

- name: numeric
  type: integer
  format: int64

would fail to parse anything, see #184 and #292. This is outside of the scope of your library upgrade here, just raising awareness of an issue.

@blast-hardcheese blast-hardcheese force-pushed the twilio:master branch from 6a41c95 to 3f0f265 May 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.