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

bug/regression: latest yams results in incomplete generation of types #565

Open dpasirst opened 1 month ago

dpasirst commented 1 month ago

Description

I'm not exactly sure where this bug is arising from (e.g. swift-openapi-generator or one of its dependencies like OpenAPIKit).

In short, using swift-openapi-generator with when the yams version is 5.0.X, the correct swift code (Types) is generated as part of generating types and client. When yams is 5.1.X, the generated code is missing fields. This is with an OpenAPI 3.1 json.

Reproduction

I have attached an example openapi.json along with the difference in the resulting generated swift code. Specifically, in looking at Components.Schemas.NewAccount and Components.Schemas.Account you will see that the one created with yams 5.0.X contains a phoneNumberPayload as part of each. There is also a defined PhoneNumber struct and curiously it is not linked or used instead of phoneNumberPayload but that is perhaps a different improvement. For now, the phoneNumberPayload is usable with yams 5.0.X.

However, when building with yams 5.1.X, the generated NewAccount and Account do not contain phoneNumberPayload. It is dropped but should be there.

I have attached the example openapi.json and the example generated outputs.

openapi.json generated_types_yams5.0.swift.txt generated_types_yams5.1.swift.txt

I'm not sure if this is related but Yams 5.1.0 release notes the following breaking change:

Change how empty strings are decoded into nullable properties. key: "" previously decoded into struct Value: Codable { let key: String? } as Value(key: nil) whereas after this change it decodes as Value(key: ""). This could be a breaking change if you were relying on the previous semantics.

If it is helpful, the openapi.json was generated via Rust using Aide with Axum.

Rust struct definition ```rust #[derive(Default, Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, JsonSchema)] pub enum Role { #[default] #[serde(rename="user")] User, #[serde(rename="admin")] Admin, } #[derive(Default, Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, JsonSchema)] pub struct PhoneNumber { /// country code must contain only a set of digits 0-9 /// it should be formatted without leading zeros. /// for example: U.S.A. should be `1` without `+` and without `00` e.g. not `001` /// it should be assumed that the `+` would be added by the system #[serde(rename = "countryCode")] pub country_code: String, /// the number should be a set of digits "333555222" /// without parens or dashes. It should not contain the country code /// however it should be formatted such that combining a `+` countryCode number /// would result in a complete functional international phone number #[serde(rename = "number")] pub number: String } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, JsonSchema)] pub struct NewAccount { /// username as A-Za-z0-9 #[serde(rename = "username")] pub username: String, /// email address #[serde(rename = "email")] pub email: Option, /// phone number #[serde(rename = "phoneNumber")] pub phone_number: Option, /// The code (string) for referrals #[serde(rename = "role")] pub role: Role, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, JsonSchema)] pub struct Account { /// internal uuid for the user #[serde(rename = "id")] pub id: Uuid, /// username as A-Za-z0-9 #[serde(rename = "username")] pub username: String, /// email address #[serde(rename = "email")] pub email: Option, /// phone number #[serde(rename = "phoneNumber")] pub phone_number: Option, /// The code (string) for referrals #[serde(rename = "role")] pub role: Role, } ```

Package version(s)

OpenAPIKit 3.1.3 swift-algorithms 1.2.0 swift-argument-parser 1.3.1 swift-collections 1.1.0 swift-http-types 1.0.3 swift-numerics 1.0.2 swift-openapi-generator 1.2.1 swift-openapi-runtime 1.4.0 swift-openapi-urlsession 1.0.1 Yams 5.0.6 or 5.1.0 as described resulting in the problem

Expected behavior

The generated types for Account and NewAccount should contain phoneNumberPayload respectively (or even better if it directly contained PhoneNumber) for any version of Yams.

Environment

$ swift -version
swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: x86_64-apple-macosx14.0

Additional information

No response

czechboy0 commented 1 month ago

Hi @dpasirst,

this sounds very similar to https://github.com/apple/swift-openapi-generator/issues/553.

I looked at your doc, and I think the problem is that we're not handling the case of anyOf where one of the cases is just null. In Swift, it's not necessary to have that explicit null, because the fact that phoneNumber already is omitted from required in the Account schema, we'll already treat it as optional.

So in your case, if you edit:

                    "phoneNumber": {
                        "description": "phone number",
                        "anyOf": [
                            {
                                "$ref": "#/components/schemas/PhoneNumber"
                            },
                            {
                                "type": "null"
                            }
                        ]
                    },

to:

                    "phoneNumber": {
                        "description": "phone number",
                        "$ref": "#/components/schemas/PhoneNumber"
                    },

It should start working again. Please let me know if that fixed it πŸ™‚

dpasirst commented 1 month ago

Hi @czechboy0 , To answer your question, it partially fixes it but not super friendly.

With your suggested change and yams 5.1.X, phoneNumber does indeed appear; however, it is of type OpenAPIValueContainer? and not type PhoneNumber as I would have expected.

In the original example (unmodified) with yams 5.0.X, phoneNumber is of type Components.Schemas.NewAccount.phoneNumberPayload? which is not ideal (would be nicest to just be PhoneNumber directly), but still easier to work with than OpenAPIValueContainer?.

Regards.

czechboy0 commented 1 month ago

That's strange, I would expect it to be of the PhoneNumber type. Can you post your updated OpenAPI doc here again please?

dpasirst commented 1 month ago

@czechboy0 - as requested - openapi.json with the change you suggested applied in 2 places: openapi.json

czechboy0 commented 4 weeks ago

@dpasirst Your OpenAPI JSON file seemed malformed, in both places you had:

"phoneNumber": {
    "#/components/schemas/PhoneNumber",
    "description": "phone number"
},

but should be:

"phoneNumber": {
    "$ref": "#/components/schemas/PhoneNumber",
    "description": "phone number"
},

When the generator runs, it emits warning and error diagnostics, can you check if it caught the malformed JSON? It's possible it didn't regenerate, showed an error, so your generated files would still be the old ones.

dpasirst commented 4 weeks ago

@czechboy0 - great catch. I see that was caused by my manual edit to change the json per a few comments back, I seem to have been a little too aggressive in removing the "anyOf"... I was not sure where to look for the generator error diagnostic. With the errored json, the Xcode build screen does not seem to indicate such a diagnostic, instead, it lists "- Diagnostics output path: <none - logs to stderr>".

Indeed, this:

"phoneNumber": {
    "$ref": "#/components/schemas/PhoneNumber",
    "description": "phone number"
},

does appear to work with both yams 5.0.X and 5.1.X.

In this case, the original posted json containing the anyOf is generated from a Rust project using Aide...and as already demonstrated manual editing is error-prone and less than ideal. Is this something that needs to be fixed upstream in that project or here on the client side?

In reviewing, https://github.com/apple/swift-openapi-generator/issues/553 , it seems the Aide generated openapi.json might be valid as it is generating with the "null" as quoted.

mwildehahn commented 3 weeks ago

πŸ‘‹ I think I'm running into something similar with these warnings: Schema "null" is not supported, reason: "schema type", skipping

With something like this schema:

      "Cursors": {
        "properties": {
          "next": {
            "anyOf": [
              {
                "type": "string"
              },
              {
                "type": "null"
              }
            ],
            "title": "Next"
          },
          "previous": {
            "anyOf": [
              {
                "type": "string"
              },
              {
                "type": "null"
              }
            ],
            "title": "Previous"
          }
        },
        "type": "object",
        "required": [
          "next",
          "previous"
        ],
        "title": "Cursors"
      },

I understand from reading a couple issues that the recommendation is to not include {"type": "null"} but I don't have direct control over this. This is a side effect of using pydantic + fastapi and dumping the openapi schema. In this specific case, I'm also relying on returning "null" although I could work around it if need be.

mwildehahn commented 3 weeks ago

I was able to work around this just by post-processing the schema to handle removing those anyOf types πŸ‘

dpasirst commented 3 weeks ago

for me, I can move forward for now thanks to @czechboy0 , but given the openapi.json is server generated, I'm hoping the fix to properly support the schema will not be too complicated and hopefully can be resolved quickly.

IanHoar commented 2 weeks ago

Also having this issue. I'm using the schema directly from a django-ninja backend and it writes nullable types in the anyOf style. I'm getting dozens of warnings like Schema "null" is not supported, reason: "schema type" when generating. I'd like to not have to go trough and rewrite those optionals every time I update the schema.

Update: Forced Yams v5.0.6 in my Package.swift and generation is working now

Update 2: Generation worked, but now the app crashes when trying to parse the values that are defined in that anyOf style instead of the type style:

image_large_url:
  anyOf:
    - type: string
    - type: 'null'

needs to be this :arrow_down:

image_large_url:
  type: [string, 'null']