Add a hint when decoding allowedValues

kean commented 2 years ago

I've tested OpenAPIKit with over 400k lines of OpenAPI specs. The most common issue I've encountered so far has to do with string enums and allowed values and the interplay of OpenAPIKit.AnyCodable, Yams, and Double. Here are a couple of examples:

Issues

Jira

searcherKey:
  type: string
  enum:
    - com.atlassian.jira.plugin.system.customfieldtypes:cascadingselectsearcher
    ...

I get AnyCodable with value 0 of type Int in the allowed values.

It happens because of the sexagesimal support in Yams. When it sees ":" in a value, it assumes it's a number. I think it's supposed to bail out when it sees it has letters, but it doesn't and returns 0.

Sexagesimal is just one of the crazy pre-2009 YAML features and I'm not sure why it's supported by Yams. I'm currently using a fork in my project.

Square

up_to:
  type: string 
  enum:
    - inf

I get AnyCodable with value Double.infinity in the allowed values.

This is a similar problem, but this time it's AnyCodable that causes an issue. YAML doesn't enforce strings to have quotes. So when AnyCodable calls if let double = try? container.decode(Double.self) {, Yams attempts to initialize a Double with the given string. And Double("inf") returns Double.infinity.

Suggested Fix

I suggest to add a hint when decoding allowedValues. When we know that Format.self is JSONTypeFormat.StringFormat.self, we can decode [String].self instead of [AnyCodable].self. It's more reliable and is better from the performance perspective as well.

mattpolzin commented 2 years ago

If you don't mind, can you open a PR for this branch against main? There are a few other things I also plan to port to a new release of OpenAPIKit 2.x as well as releasing with 3.0.0.

kean commented 2 years ago

If you don't mind, can you open a PR for this branch against main?

Yeah, absolutely. Cherry-pick: https://github.com/mattpolzin/OpenAPIKit/pull/246

For future references, here's another common example:

- in: query
  name: closed
  schema:
    type: string
      enum:
        - true
        - false

These values become booleans (AnyCodable with Bool). Same does "on" and "off" (because pre-2009 YAML).

kean commented 2 years ago

And sexagesimals in the current version of Yams become a problem everywhere where the type is determined dynamically, not just in enums.

For example the "example" becomes AnyCodable with value 0 (Int):

webhook-config-url:
  type: string
  description: The URL to which the payloads will be delivered.
  example: https://example.com/webhook
  format: uri

I think we can be more clever about the types in case too. We know the type upfront. It's still going to be a problem with dictionaries though, so I think I'll have to continue using my Yams fork.

mattpolzin / OpenAPIKit