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

Null property type with quotes not generated #553

Open
TopIvanAbramov opened this issue Mar 26, 2024 · 14 comments
Open

Null property type with quotes not generated #553

TopIvanAbramov opened this issue Mar 26, 2024 · 14 comments
Labels
kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue.

Comments

@TopIvanAbramov
Copy link

TopIvanAbramov commented Mar 26, 2024

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

@TopIvanAbramov TopIvanAbramov added kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels Mar 26, 2024
@czechboy0
Copy link
Contributor

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

@TopIvanAbramov
Copy link
Author

TopIvanAbramov commented Mar 26, 2024

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

updated the comment

@mattpolzin
Copy link

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
Copy link

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
Copy link

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
Copy link
Author

TopIvanAbramov commented Mar 26, 2024

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

also, here is complete list of SPM dependencies

@czechboy0
Copy link
Contributor

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

@mattpolzin
Copy link

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
Copy link
Author

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

@mattpolzin
Copy link

@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
Copy link
Author

TopIvanAbramov commented Mar 27, 2024

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

@mattpolzin
Copy link

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
Copy link

mattpolzin commented Mar 27, 2024

@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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue.
Projects
None yet
Development

No branches or pull requests

3 participants