apple / swift-openapi-generator

Generate Swift client and server code from an OpenAPI document.
https://swiftpackageindex.com/apple/swift-openapi-generator/documentation
Apache License 2.0
1.21k stars 87 forks source link

Null property type with quotes not generated #553

Open TopIvanAbramov opened 1 month ago

TopIvanAbramov commented 1 month ago

Description

If in YAML I specify null type with quotes generator omits the property. But if I specify null as a type without quotes everything works well

Reproduction

CustomerUpdate:
      properties:
        given_name:
          anyOf:
            - type: string
            - type: 'null'
          title: Given Name

But this works fine:

CustomerUpdate:
      properties:
        given_name:
          anyOf:
            - type: string
            - type: null
          title: Given Name

The same issue with json, generator works only if null type isn't surrounded by quotes

Package version(s)

swift-open-api-genrator 1.2.1 swift-openapi-runtime 1.3.2 swift-openapi-urlsession 1.0.1 Yams 5.1.0 OpenAPIKit 3.1.3

Expected behavior

Generator should parse 'null' - with quotes too

Environment

swift version: 5.9

Additional information

No response

czechboy0 commented 1 month ago

Hmm can you also add the resolved versions of OpenAPIKit and Yams, please? cc @mattpolzin

TopIvanAbramov commented 1 month ago

Hmm can you also add the resolved versions of OpenAPIKit and Yams, please? cc @mattpolzin

updated the comment

mattpolzin commented 1 month ago

Strange, when testing against OpenAPIKit directly, I actually get results that would imply the opposite expected result (I would expect with quotes to work and without quotes to fail):

"""
type: null
"""
----
{}
========================
"""
type: 'null'
"""
----
{"type":"null"}

Above I've decoded the first YAML snippet and ended up with an empty schema and a warning. The second YAML snippet decodes to a schema with type "null".

I'm not yet sure what to make of these results.

mattpolzin commented 1 month ago

Sanity checking further, if I use the whole snippets from the OP, I get a schema that has nullable = true when using the quotes (expected) and I get nullable = false when not using quotes (expected).

mattpolzin commented 1 month ago

I'm not trying to say the bug report is unreproducible, of course, I am just not seeing any obvious problems in OpenAPIKit's handling of the given schemas yet.

TopIvanAbramov commented 1 month ago

@mattpolzin try to generate from the full json https://pastebin.com/G8zS0i9u

also, here is complete list of SPM dependencies

czechboy0 commented 1 month ago

Yams also released 5.1.0 with a fix in this area, could it have caused a regression? cc @jpsim

mattpolzin commented 1 month ago

Yams behavior might have changed meaningfully RE this situation, but the results I saw above seemed to indicate that not only did Yams parse the schema alright but OpenAPIKit also represented the schema the way that I expected it to. I still could be missing something obvious on my end, but I wonder if somewhere in the Swift Generator codebase there is an old workaround for "type": null (i.e. not a valid type, because the type must be a string) that is causing the observed behavior from the OP to be opposite of what I would expect given what OpenAPIKit is producing.

TopIvanAbramov commented 1 month ago

Actually, 3-4 days ago everythin worked fine, I think there is a bug in a new version of some dependency

mattpolzin commented 1 month ago

@TopIvanAbramov do you have time to try locking down Yams to version 5.0.0 and see if that fixes the problem? If it does, at least we know where to look. IMO that could still either indicate a bug in Yams or a bug in OpenAPIKit or the Swift Generator depending on whether we were just using Yams under a buggy assumption from before.

TopIvanAbramov commented 1 month ago

@mattpolzin Yeah, the problem with Yams 5.1.0 version, on 5.0.0 everything works perfectly

mattpolzin commented 1 month ago

I re-ran my small test case directly against OpenAPIKit using Yams 5.0.6 rather than 5.1.0 and (like you) observed a difference.

With the previous release of Yams, I get the same result whether I quote null or not (using the snippets from the OP).

With the new release of Yams, I get the differences I described in my previous message.


That said, the behavior as of the new release is the expected behavior. With the previous release, neither snippet is recognized as nullable. With the new release, the quoted "null" is parsed correctly and the schema containing that type is marked nullable by OpenAPIKit.


All that to say, I think what we have here is a new (correct) case to handle rather than a new bug to work around.

mattpolzin commented 1 month ago

@TopIvanAbramov could you elaborate on the correct and incorrect behavior for this ticket? When things work as expected, what is being generated?

I'm wondering if the desired behavior occurs when an anyOf schema has an empty non-nullable fragment in it and the undesired behavior occurs when that same anyOf schema has an empty nullable fragment in it since that appears to be the only difference I can spot in what OpenAPIKit produces with the new version of Yams.


Put into JSON to remove ambiguity, OpenAPIKit is telling the Swift generator about the following when null is not quoted:

{
  "type": "object",
  "properties": {
    "given_name": {
      "title": "Given Name",
      "anyOf": [
        {
          "type": "string"
        },
        {}
      ]
    }
  }
}

In this case, it tells the Swift generator that given_name is not nullable. This is the situation producing the desired results.

Whereas when null is quoted, it tells the Swift generator about the following:

{
  "type": "object",
  "properties": {
    "given_name": {
      "title": "Given Name",
      "anyOf": [
        {
          "type": "string"
        },
        {
          "type": "null"
        }
      ],
      "type": "null"
    }
  }
}

In this case, it tells the Swift generator that given_name is nullable. This is the situation producing the undesired results.

I do see a problem with the JSON that OpenAPIKit produces there (the "type": "null" living outside the anyOf) but that's just a serialization problem; the representation that the Swift generator gets is simply marked nullable.

czechboy0 commented 1 month ago

@TopIvanAbramov Note that type: needs to be followed by a string (or an array), according to JSON Schema 2020-12, which we use here to parse OpenAPI 3.1. So type being null (as opposed to "null") is not a valid schema, and if it was parsed successfully previously, that'd be considered a bug.

If you property quote it as "null" - do you now get the desired or non-desired behavior? We can debug it from here.