mattpolzin / OpenAPIKit

Codable Swift OpenAPI implementation.
MIT License
280 stars 35 forks source link

Incorrect JSON generated from in 3.0 alpha 4 #256

Closed knellr closed 1 year ago

knellr commented 2 years ago

Hello! I recently resurrected a project from last year on 3.0. alpha 1 (which included a fix I needed for handling optionals) and moved to 3.0 alpha 4.

On alpha 4 the following appears to be nesting the allowed values an additional level when serialised:

JSONSchema.string(required: false, allowedValues: ["ONE", "TWO"])

i.e.

    "type" : {
        "enum" : [
          [
            "ONE",
            "TWO"
          ]
        ],
        "type" : "string"
      },

As an aside, I guess 3.0 development is on hold for now? (I appreciate this is open source and have no expectations, I'm just curious!)

knellr commented 2 years ago

Just looked in more detail and it appears that it's being serialised this way in both alpha 1 and 4, but alpha 4 is failing when attempting to read it again with:

Error: Expected `Index 0` value in Document.components.schemas.Asset.properties.type.enum to be parsable as String but it was not.
mattpolzin commented 2 years ago

What it looks like is happening here is actually that Swift is quietly picking the variadic version of the .string() function and interpreting your array of strings as a single value. This is ambiguity I had not noticed before -- it seems to make it so that when supplying the allowed values as a literal, you have no choice but to use the variadic version of the helper function.

The following should get you what you want:

JSONSchema.string(required: false, allowedValues: "ONE", "TWO")

Alternatively, defining the allowed values as a variable ahead of time should give the type system enough information to pick the correct function, but it's possible it still confuses it, which would mean the variadic version is more dangerous than helpful.

This confusing situation is also partly because allowedValues are allowed to be of any type upon construction; in other words, even though the schema's type is "string", the allowed values can be anything, so it is not against the rules to use an array. On the other hand, decoding allowed values uses a hint specifically for strings (this is the thing that changed between alpha 1 and alpha 4) because if it doesn't then values can be decoded entirely incorrectly (see https://github.com/mattpolzin/OpenAPIKit/pull/245 for more on that).

I guess 3.0 development is on hold for now?

It's not on hold per-se, but development is happening sporadically (by me) at infrequent intervals. I've been meaning to tackle a few more of the remaining issues recently but I keep getting distracted or pulled away from it when I get the time! I still fully plan to eventually release version 3.0, but admittedly I am resting on the laurels that OpenAPIKit is relatively complete at version v2 and relatively stable at v3 alpha 4. I'm always interested in new contributors, too!

knellr commented 2 years ago

Ah that makes sense, thank you for the very swift investigation and explanation.

I wish I had time to contribute to the project more directly, but sadly life has other plans!