Support nullable values

[m4p]

Commit hash: f530c906215528a7450fc7602b9f93d9236eb4c0

Context: While testing my application that uses with swift-openapi-generator, I noticed that nullable is not yet supported on Schema Objects values. This leads to a Decoder error for API calls that return null for nullable values.

Steps to reproduce:

1. Use a OpenAPI spec that relies on nullable values
2. Make an api call that returns some null values
3. Expected: Nullable values returned as optionals
4. Actual: DecodingError: valueNotFound String - Expected String value but found null instead.

$ swift --version swift-driver version: 1.75.2 Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100) Target: arm64-apple-macosx13.0

$ uname -a Darwin hayai.lan 22.5.0 Darwin Kernel Version 22.5.0: Mon Apr 24 20:52:24 PDT 2023; root:xnu-8796.121.2~5/RELEASE_ARM64_T6000 arm64

My system has IPv6 enabled.

[czechboy0]

Hi @m4p, thanks for taking the time to file the issue.

Could you attach a short snippet of OpenAPI (ideally YAML) that shows more of the context of how you use nullable values? My understanding is that object properties not being required, and a schema being nullable, are similar in the sense that we want an optional value in Swift. (@mattpolzin can add more background here).

With the OpenAPI snippet, I'm also curious how you think the generated Swift code should change, to help us understand how and whether we need to treat non-required object properties and nullable schemas differently. Thanks! 🙏

[m4p]

Sure thing. I'm working with this third-party spec. I can try making the types non-required and see if that has the desired effects.

[m4p]

Yeah, removing nullable values from required, results in the code working as expected, so it may be an issue with the quality of the spec.

[czechboy0]

I believe it's legal to have a required property marked as nullable, it's just a question of how to represent that well in Swift, where {} and {"foo": null} is treated the same way for a Swift Decodable struct that looks like:

struct Foo: Decodable {
  var foo: String?
}

I suspect this is a genuine thing we should handle in Swift OpenAPI Generator, by not just checking whether a property schema is required, but also whether it's nullable.

Just to confirm my suspicion, the code that emits the error you had in the issue description, is that property generated as foo: String or foo: String?.

[mattpolzin]

Yeah, required nullable (and all 3 other permutations) are all valid. In my own API development I've even used these permutations to drive home the semantics of the API even though a client might handle "null" in the same way as the given property being omitted. For this reason, I've always preferred to distinguish between a property being there or not and a property being null or not in server code that I write even though that isn't particularly natural with Swift when encoding or decoding.

[czechboy0]

Thanks for the confirmation, @mattpolzin. So this is a task on Swift OpenAPI Generator. Presumably, we should create a utility method on an object schema that can tell us whether a property should be generated as an optional type, and it'll return true if either the schema is nullable, or the property name is not included in the required array on the object schema. And then replace all occurrences of checking required by using this utility method.

[m4p]

@czechboy0 It generates public var next_url: Swift.String for the line linked in the spec above. Non-Optional.

[czechboy0]

@m4p Great, that matches my expectations. So if anyone would like to contribute a fix, this comment outlines the rough steps: https://github.com/apple/swift-openapi-generator/issues/82#issuecomment-1598817610

[Kyle-Ye]

I'm experience the same problem.

Consider the following input

groups:
  type: array
  items:
    type: object
    additionalProperties: false
    properties:
      id:
        type: integer
      flair_url:
        type:
        - string
        - 'null'
    required:
    - id
    - flair_url

The current output is the following

public struct groupsPayloadPayload: Codable, Equatable, Hashable, Sendable {
    /// - Remark: Generated from `#/paths/site.json/GET/json/groupsPayload/id`.
    public var id: Swift.Int
    /// - Remark: Generated from `#/paths/site.json/GET/json/groupsPayload/flair_url`.
    public var flair_url: Swift.String
    /// Creates a new `groupsPayloadPayload`.
    ///
    /// - Parameters:
    ///   - id:
    ///   - flair_url:
    public init(
        id: Swift.Int,
        flair_url: Swift.String
    ) {
        self.id = id
        self.flair_url = flair_url
    }
    public enum CodingKeys: String, CodingKey {
        case id
        case flair_url
    }
    public init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        id = try container.decode(Swift.Int.self, forKey: .id)
        flair_url = try container.decode(
            Swift.String.self,
            forKey: .flair_url
        )
        try decoder.ensureNoAdditionalProperties(knownKeys: [
            "id", "flair_url", "flair_bg_color", "flair_color",
        ])
    }
}

The expected output is

public init(from decoder: Decoder) throws {
    let container = try decoder.container(keyedBy: CodingKeys.self)
    id = try container.decode(Swift.Int.self, forKey: .id)
    // ❌ since flair_url is marked as required
    flair_url = try? container.decode(Swift.String.self, forKey: .flair_url)
    // ✅ Only String or nil is accepted
    flair_url = try container.decode(Swift.String?.self, forKey: .flair_url)
    try decoder.ensureNoAdditionalProperties(knownKeys: [
        "id", "flair_url", "flair_bg_color", "flair_color",
    ])
}

Here we treat null as a special case. But what about the following input? (We have random types combined)

groups:
  type: array
  items:
    type: object
    additionalProperties: false
    properties:
      id:
        type: integer
      flair_url:
        type:
        - string
        - integer
    required:
    - id
    - flair_url

[Kyle-Ye]

Thanks for the confirmation, @mattpolzin. So this is a task on Swift OpenAPI Generator. Presumably, we should create a utility method on an object schema that can tell us whether a property should be generated as an optional type, and it'll return true if either the schema is nullable, or the property name is not included in the required array on the object schema. And then replace all occurrences of checking required by using this utility method.

We need to diff requirement and nullability.

A non-required type can infer nullability, but nullability can't infer requirement info.

Consider the following define and input

// name non-required
define:
items:
  type: object
  properties:
    name:
      type:
      - string
  required:
accepted input:
- { "name": "xx" }
- {}
code:
try decodeIfPresent(String.self, forKey: .name)

// name required & nullable
define:
items:
  type: object
  properties:
    name:
      type:
      - string
      - 'null'
  required:
  - name
accepted input:
- { "name": "xx" }
- { "name": nil }
code:
try decode(String?.self, forKey: .name)
> Or we might make it more general to support any type combination
> try decode(A.self, forKey: .name) and try decode(B.self, forKey: .name)

// name required & nonnull
define:
items:
  type: object
  properties:
    name:
      type:
      - string
  required:
  - name
accepted input:
- { "name": "xx" }
code:
try decode(String.self, forKey: .name)

And I think it was not a good idea to use try? decode which will not throw an error even the type is not match.(eg. Expected Int but actually a String)

[Kyle-Ye]

Thanks for the confirmation, @mattpolzin. So this is a task on Swift OpenAPI Generator. Presumably, we should create a utility method on an object schema that can tell us whether a property should be generated as an optional type, and it'll return true if either the schema is nullable, or the property name is not included in the required array on the object schema. And then replace all occurrences of checking required by using this utility method.

A more detailed hint for anyone who'd like to submit a fix.

The fix will probably involve the following code

https://github.com/apple/swift-openapi-generator/blob/6b11135cccfb0846809f434cf2ad95134b65e945/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateCodable.swift#L142

And you may need to add a fullyQualifiedOptionalSwiftName property for TypeUsage type

/// A string representation of the fully qualified Swift type name, with
/// optional wrapping added.
///
/// For example: `Swift.Int?`.
var fullyQualifiedOptionalSwiftName: String {
    withOptional(true).fullyQualifiedSwiftName
}

[mattdw]

I think I've hit a related problem - this spec (which I don't control), specifies enums like:

"refund_behaviour": {
  "type": "string",
  "nullable": true,
  "description": "How the category's refunds or deductions should be reported on.",
  "enum": [
    "debits_are_deductions",
    "credits_are_refunds",
    null
  ],
  "example": "credits_are_refunds"
},

Which then results in errors like:

Error: Disallowed value for a string enum 'Components.Schemas.Category.refund_behaviourPayload (#/components/schemas/Category/refund_behaviour)': ()

(For the time being I've just vendored and edited the spec, as this is a hobby project just to poke at OpenAPI via Swift.)

[simonjbeaumont]

@mattdw that's interesting. We added support for the empty value in a string enum in #59.

If you have the time, could you let us know if you're using a version with this fix and, if you're still having issues, could you open a new issue so we can investigate? 🙏

[czechboy0]

We added support for the empty value in a string enum in https://github.com/apple/swift-openapi-generator/pull/59.

We did, but this is subtly different. With #59, we now support:

type: string
enum:
  - foo
  - ""

But the above is:

type: string
nullable: true
enum:
  - foo
  - null

We carefully have to design support for nullable types, as they are distinct from (non-)required properties on object schemas; yet, in Swift, they probably make sense to look very similar.

Some additional reading:

• OpenAPI 3.0.1 schema object, which says to go to JSON Schema to learn about enums: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.1.md#schema-object
• The linked JSON Schema section: https://datatracker.ietf.org/doc/html/draft-wright-json-schema-validation-00#section-5.20

We'll need a proposal for this, for sure, and possibly even more than one. I imagine we could first propose to support nullable types as object properties, then as enum values, etc, as they might be handled differently.

The main question I'd ask anyone considering taking this on is: what should the generated code look and behave like? Once we agree on that, we can help guide the implementation.

[Kyle-Ye]

Tested with the following and still failed to decode.

i18n_name:
  type:
  - string
  - 'null'

Since there is some unsolved case for the issue, maybe we should consider reopen it?

[czechboy0]

@Kyle-Ye are you including the nullableSchemas feature flag?

[Kyle-Ye]

@Kyle-Ye are you including the nullableSchemas feature flag?

No. After I enable it, it did solve the problem. Thanks for the clarification.