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

could not find implicit value for parameter ev #1502

Closed
djm1329 opened this issue Jun 2, 2022 · 11 comments · Fixed by #1503
Closed

could not find implicit value for parameter ev #1502

djm1329 opened this issue Jun 2, 2022 · 11 comments · Fixed by #1503
Labels
bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past core Pertains to guardrail-core good first issue Does not require deep knowledge of the codebase and is easily testable

Comments

@djm1329
Copy link

djm1329 commented Jun 2, 2022

Awesome library, thank you so much! I am encountering a couple of issues using the latest version.

addSbtPlugin("dev.guardrail" % "sbt-guardrail" % "0.71.0")

Generated client code for Akka HTTP does not compile for the following:

openapi: 3.0.2
servers:
  - url: "http://localhost:1234"
paths:
  /foo:
    post:
      operationId: doFoo
      responses:
        '204':
          description: No response
      parameters:
        - name: bar
          in: query
          required: false
          schema:
            $ref: '#/components/schemas/Bar'
components:
  schemas:
    Bar:
      type: string

gives error

Client.scala:45:84: could not find implicit value for parameter ev: com.example.test1.Implicits.AddArg[Option[io.circe.Json]]
[error]     makeRequest(HttpMethods.POST, host + basePath + "/foo" + "?" + Formatter.addArg("bar", bar), allHeaders, HttpEntity.Empty, HttpProtocols.`HTTP/1.1`).flatMap(req => EitherT(httpClient(req).flatMap(resp => resp.status match {
[error]                                                                                    ^

Similar for

openapi: 3.0.2
servers:
  - url: "http://localhost:1234"
paths:
  /foo:
    post:
      operationId: doFoo
      responses:
        '204':
          description: No response
      parameters:
        - name: bar
          in: query
          required: false
          schema:
            $ref: '#/components/schemas/Bar'
components:
  schemas:
    Bar:
      type: object
      properties:
        baz: 
          type: string

gives error

Client.scala:46:84: could not find implicit value for parameter ev: com.example.test2.Implicits.AddArg[Option[com.example.test2.definitions.Bar]]
[error]     makeRequest(HttpMethods.POST, host + basePath + "/foo" + "?" + Formatter.addArg("bar", bar), allHeaders, HttpEntity.Empty, HttpProtocols.`HTTP/1.1`).flatMap(req => EitherT(httpClient(req).flatMap(resp => resp.status match {
[error]
@blast-hardcheese
Copy link
Member

@djm1329 Thanks, I'm glad you're liking it!

In your first example, it seems as though a type alias is getting lost, and it's falling back to the bare type: object representation.

I can't look at this immediately, but the place where we should be doing this lookup is here, with the Deferred resolution happening here.

If you're interested in poking around you can test out generating a client directly from guardrail's sbt console via:

sbt> cli scala --client --specPath path/to/example.yaml --outputPath /tmp/example-client/src/main --packageName com.example.client --framework akka-http

(replacing --specPath with a path to one of your example supplied specifications, as well as --outputPath to target somewhere useful.)

It's even useful to disable the guardrail plugin and --outputPath directly into your project's src/main/, it makes iterating on bugfixes or new features much more convenient.

Setting expectations, this shouldn't be more than a few lines change, or even just identifying an edge case where we case _ => instead of matching a member of a sealed trait.

Thanks for the report!

@blast-hardcheese blast-hardcheese added bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past good first issue Does not require deep knowledge of the codebase and is easily testable core Pertains to guardrail-core labels Jun 3, 2022
@blast-hardcheese
Copy link
Member

Ah, one follow-up -- in your second example, what would you expect to happen? What does it mean to put a type: object into a query string parameter?

I know OpenAPI 3.x introduces specific field encodings to denote that a QS parameter should be a url-encoded JSON blob, but I don't see that directive as part of your example.

I'd like to make sure I understand fully here, thanks in advance!

@djm1329
Copy link
Author

djm1329 commented Jun 3, 2022

@blast-hardcheese Thanks so much for the detailed suggestions. I am going to try to take a look at this tomorrow unless you beat me to it.

For the second example, you raise an excellent point I did not even think about. I am trying to consume a 3rd party spec so pretty sure the 3rd party didn't think about it either. I assume a URL-encoded JSON blob is what's intended. Looking at the OpenAPI 3.0.x specs it seems the default encoding would be "form", but I could very well be reading it incorrectly. Do you happen to know what setting is needed for JSON blob?

Again, thanks for this library and speedy and helpful responses!

@blast-hardcheese
Copy link
Member

Do you happen to know what setting is needed for JSON blob?

So, this part of the spec includes both style and explode, the Style Values indicates that form should just ignore the name and the constituent fields should be inlined as their own parameters, though there are a whole bunch of variations there.

However, do your question,

For more complex scenarios, the content property can define the media type and schema of the parameter. A parameter MUST contain either a schema property, or a content property, but not both. When example or examples are provided in conjunction with the schema object, the example MUST follow the prescribed serialization strategy for the parameter.

presumably it'd just be something like

      parameters:
        - name: bar
          in: query
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Bar'

but that's not currently supported by guardrail, you'd have to just use type: string for now.

@djm1329
Copy link
Author

djm1329 commented Jun 5, 2022

@blast-hardcheese thanks for the guidance on specifying Json blob, it makes sense. Now I'm thinking the form encoding might be more suitable.

I have been tracing through the code following your suggestions. Now I realize I am not sure what the expected result should be for the first example. In the Client do we want a def doFoo(bar: Option[String], ...) or a def doFoo(bar: Option[Bar], ...) with a suitable definition for Bar emitted?

@blast-hardcheese
Copy link
Member

Definitely the latter, I would have expected that the component's name would have been able to be dereferenced, stored into Deferred, then in the resolver the name was looked up against the list of components to find the true type.

We used to emit and manage type aliases, but it was actually really unpleasant ergonomically. It may have been just the way it was done, trying to render everything into a single package object, but either way -- not great. It's what led to expending the resolver to support stuff like Deferred, DeferredArray, and DeferredMap, which I'm much happier with.


As for the form encoding, this is the first time I've seen an API actually expect that, do you have a sample cURL that approximates what you've confirmed that the API expects? I'm really curious.

I think implementation-wise, I'd have to poke around a bit more before giving advice, but I'm leaning in the way of throwing something similar to case class UrlEncoded(values: (String, A: Showable)*) in the support package, with an instance of Showable for UrlEncoded. That should give us the capacity to support the other encoding styles in the future.
(I think this should make sense, considering your initial error, looking for a Showable[Json], but if something was confusing I'm happy to do a more thorough writeup)

@djm1329
Copy link
Author

djm1329 commented Jun 5, 2022

There's definitely a Deferred(Bar). But ultimately that is given RandomType(Bar,io.circe.Json).

Could the problem be in

def modelTypeAlias[L <: LA, F[_]](clsName: String, abstractModel: Tracker[Schema[_]], components: Tracker[Option[Components]])(implicit
? I don't see a StringSchema handled in there, but tbh I am not super clear yet on what I am looking at.

For the form encoding, I need to check with the 3rd party what they expect or if it is even form they want. Unfortunately I have nothing to cURL yet, just a likely-work-in-progress OpenAPI spec. I'll def get back to you with whatever I learn!

@blast-hardcheese
Copy link
Member

blast-hardcheese commented Jun 5, 2022

I don't see a StringSchema handled in there, but tbh I am not super clear yet on what I am looking at.

You're spot on. The problem is that here, we match the case where m is determined to be a StringSchema, but we then call modelTypeAlias, which only knows how to deal with ObjectSchema.

... and since that'll never be true, we always fold in a bare objectType(None), which is exactly the behaviour you're seeing.

So, just looking at the callers to that modelTypeAlias, we've got ObjectSchema, ComposedSchema, and StringSchema. The other primitive types do not use modelTypeAlias, and even the name doesn't follow, since StringSchema is clearly not a model, so...

I opened #1503 to test just using the standard typeAlias instead of modelTypeAlias, to see what we get. Running that branch against your example spec produced exactly what we expected to find, def doFoo(bar: Option[String] = None, ...), so I'm hopeful.

The only addition I'd make is to expand alias.yaml regression test with a type: string as well as a parameter in the doFoo, which I'll take on before merging.

@djm1329
Copy link
Author

djm1329 commented Jun 5, 2022

Awesome @blast-hardcheese! Thank you!

@blast-hardcheese
Copy link
Member

Alright. sbt-guardrail 0.72.0 is out with this fix (plus a few more). If you have scala-steward applying your upgrades for you, you should also get a scalafix rule that rewrites Iterable to Vector for array parameters. If you need to apply it manually,

scalafix https://raw.githubusercontent.com/guardrail-dev/guardrail-scalafix-rules/master/rules/src/main/scala/fix/GuardrailIterableToVector.scala

should patch things up for you.

When you learn back about the encoding for that query parameter, I'd appreciate a new issue, optionally referencing this one, to avoid this issue just being left open.

Thanks!

@djm1329
Copy link
Author

djm1329 commented Jun 6, 2022

Perfect, thanks @blast-hardcheese, I will definitely come back on the query parameter. Really appreciate your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past core Pertains to guardrail-core good first issue Does not require deep knowledge of the codebase and is easily testable
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants