disneystreaming / smithy-translate

Other
53 stars 12 forks source link

Issues with OpenApi Specs #212

Closed MoeQuadrat closed 9 months ago

MoeQuadrat commented 9 months ago

Problem 1: Incorrect Date-Time Format in Examples We are currently preferring a Smithy client instead of OpenApi Clients for communicating with APIs. However, while implementing an some external APIs (e.g. https://api.sevdesk.de/), we encountered an issue with date-time examples. It appears that some examples have an incorrect format. Currently, our workaround is replacing the date format with a string. It would be nice if there were options to deactivate examples or ignore errors when the example is not the expected format.

Problem 2: Conflicting URIs Leading to Validation Failures The some API Specs have conflicting URIs, resulting in validation failures during translation. Is there a configuration option to ignore this error?

Problem 3: Selector Issues with Smithy's "httpQuery" Annotation Our third challenge is that some API Specs are using objects in queries. This results in selector issues with Smithy's "httpQuery" annotation, leading to the error: "This trait may only be applied to shapes that match the following selector..." Any guidance on resolving this specific selector-related error would be highly appreciated.

Example to OpenApi that has all 3 Problems is on the linked Website. Its pretty huge so rewriting the entire Spec in Smithy is kinda out of Question. Any help on this would be appreciated

daddykotex commented 9 months ago

Hey @MoeQuadrat , nice to see you here.

So my understanding is that you are interacting with an external api, in this case, let's assume api.sevdesk.de, and they don't have a Smithy specifications, just an Open API specifications. The part I'm confused about is why you're opening the issue in this repository? Is it because, you've used smithy-translate to convert their Open API specification in Smithy and using it leads to the problem you're having, or you manually converted the Smithy specification (from the original Open API) and you're having problems?

Also, is this correct to assume that you're using your Smithy specification along Smithy4s to generate the client code and that's where you're having the problems. If not, then we'd need more info to know how you generate your client.

The following applies if your using Smithy4s, otherwise it may not be accurate. Addressing your problems in order:

  1. if you don't want validation, you should not use Timestamp and @timestampFormat, these types have validation built in the json (de)serialization layer
  2. we're aware of this issue and it's a limitation in the Smithy prelude : https://github.com/smithy-lang/smithy/issues/1029
  3. this would again be a limitation in Smithy prelude, but one that I have not encountered in the past as I've never had to shove structures into query params, let me think about this one and I'll see if there is anything we can do (I'm not sure we can)

To conclude, we would need a snippet of smithy code or some of the client code, or some exception that you get because right now, we're left with questions:

  1. what's the smithy code?
  2. what's the date time value supplied that fails to validate (is it invalid and it's ok to fail, or is it an issue on our side (assuming smithy4s))
  3. what is used for client generation, smithy4s or something else?
MoeQuadrat commented 9 months ago

Hi @daddykotex,

Thanks for your quick response and clarification. Let me provide additional details to address your questions:

Context: We use smithy-translate to convert the Open API specification to Smithy. Issues arise during this conversion process. Our client operates based on the generated smithy4s Scala code, closely resembling the smithy4s client. I'm about 99% confident that it will work seamlessly once the Smithy files and Scala code are generated.

Specifics on the Problems:

  1. Validation is crucial for us. However, the challenge we face is that smithy-translate appears to perform validation or parsing of examples in the OpenAPI spec against the given types, leading to crashes when translating a spec with faulty examples. It would be beneficial if such instances resulted in a warning rather than a crash.
  2. I was aware of the linked issue, but I'm unsure about the validation implementation on your end and if there might be functionality to ignore this or set it to a warning.
  3. As it's officially documented in the Swagger documentation (https://swagger.io/docs/specification/describing-parameters/), this problem might arise in the future. Yes, it makes sense, the next time problems in the Smithy prelude arise, I will try addressing them directly in the Smithy repo.

Regarding your questions: The simplest approach is probably trying to convert the OpenAPI from SevDesk to Smithy. I can also provide the exceptions later in the day if needed.

Thanks again for your assistance.

Baccata commented 9 months ago

Hey there.

@MoeQuadrat I'm gonna close this issue, and ask you to open individual issues reflecting each individual problem, with minimised examples to encourage us to look at it, at least for point 1 and 3, and copy the error messages you get in each case.

A note regarding point 3 specifically : smithy-translate offers a best effort conversion : if openapi semantics can't be mapped to what the smithy language and the official http traits allow for, we're not putting ourselves under any obligation to support those semantics.

We can however provide a way to prevent the validation of smithy models before they are dumped onto the file system, if we identify this as a reasonable mitigation. You'd then have to post-process the files in question, either manually or programmatically, to turn them into valid models that smithy4s will be able to consume.

MoeQuadrat commented 9 months ago

Hey @Baccata

Thanks for the suggestion. I'll open separate issues for each problem as you suggested. My bad for not doing it that way from the start. I'll make sure to include all the details you need.