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

Advice regarding nullable refs? #513

Open
groue opened this issue Jan 24, 2024 · 7 comments
Open

Advice regarding nullable refs? #513

groue opened this issue Jan 24, 2024 · 7 comments
Labels
kind/support Adopter support requests. status/triage Collecting information required to triage the issue.

Comments

@groue
Copy link

groue commented Jan 24, 2024

Question

Hello,

I'm writing a client with an openapi.yml that frequently uses nullable ref properties, as below:

// Technique 1
my_field:
  nullable: true
  allof:
    - $ref: '#/components/schemas/MyField'

An alternative technique, AFAIK, is:

// Technique 2
my_field:
  oneOf:
    - $ref: '#/components/schemas/MyField'
    - type: 'null'

With both techniques, the generated code is not quite easy to deal with, both when decoding responses or encoding requests. The generated payload is a struct that has a non-semantic value1 property, or an enum that does not expose any accessor for extracting its non-null value.

// Technique 1
public struct my_fieldPayload: Codable, Hashable, Sendable {
    public var value1: Components.Schemas.MyField
    public init(value1: Components.Schemas.MyField) {
        self.value1 = value1
    }
}

// Technique 2
@frozen public enum my_fieldPayload: Codable, Hashable, Sendable {
    case MyField(Components.Schemas.MyField)
    case case2(OpenAPIRuntime.OpenAPIValueContainer)
}

We're far from the expected Swift optionals. The struct generated by the technique 1 is surprisingly shallow.

Did any member in the community meet the same problem? Is there any workaround? Or a better YAML technique for specifying a nullable ref, while preserving good Swift ergonomics? Maybe I'm just holding it wrong?

@groue groue added kind/support Adopter support requests. status/triage Collecting information required to triage the issue. labels Jan 24, 2024
@czechboy0
Copy link
Contributor

czechboy0 commented Jan 24, 2024

This looks a bit related to a question from just yesterday: #512

What is the use case? What are the HTTP semantics you're trying to express?

Note that nullable is only OpenAPI 3.0 syntax, and adding a null to types is the equivalent in OpenAPI 3.1. So which you use depends on the version of the OpenAPI specification.

@groue
Copy link
Author

groue commented Jan 26, 2024

This looks a bit related to a question from just yesterday: #512

Yes: in both issues the OpenAPI spec describes optionality with a pattern, and the generated code creates the desire to share some remarks.

Two such patterns are described in this issue, and another one in #512. I guess there are others.

What is the use case? What are the HTTP semantics you're trying to express?

The JSON api uses some objects in requests and responses. Those objects are defined in the components/schemas section of the spec, for reusability. Some properties of those objects are defined with $ref to #/components/schemas/Whatever. Some of those properties can be null in the JSON payload. Some of those properties are required, some are not. Nullability and requirement are orthogonal concerns, because requirement describes the presence of the key, and nullability describes the validity of a null value. (I know you know all of this).

Let's consider a first spec that tries to use all four combinations of requirement/nullability:

# Probably not valid OpenAPI syntax
components:
  schemas:
    Parent:
      type: object
      properties:
        child1:
          nullable: true
          $ref: '#/components/schemas/Child'
        child2:
          nullable: true
          $ref: '#/components/schemas/Child'
        child3:
          $ref: '#/components/schemas/Child'
        child4:
          $ref: '#/components/schemas/Child'
      required:
        - child1
        - child3
    Child:
      type: object

OpenAPIGenerator 1.2.0 generates a Parent struct where child1 (required nullable) is not a Swift optional, and child2 can not distinguish between missing and present and null.

public struct Parent: Codable, Hashable, Sendable {
    public var child1: Components.Schemas.Child
    public var child2: Components.Schemas.Child?
    public var child3: Components.Schemas.Child
    public var child4: Components.Schemas.Child?
}

Another attempt, with ONE of the patterns for nullable refs:

components:
  schemas:
    Parent:
      type: object
      properties:
        child1:
          nullable: true
          allOf:
            - $ref: '#/components/schemas/Child'
        child2:
          nullable: true
          allOf:
            - $ref: '#/components/schemas/Child'
        child3:
          $ref: '#/components/schemas/Child'
        child4:
          $ref: '#/components/schemas/Child'
      required:
        - child1
        - child3
    Child:
      type: object

This generates:

public struct Parent: Codable, Hashable, Sendable {
    public struct child1Payload: Codable, Hashable, Sendable {
        public var value1: Components.Schemas.Child
    }
    public struct child2Payload: Codable, Hashable, Sendable {
        public var value1: Components.Schemas.Child
    }
    public var child1: Components.Schemas.Parent.child1Payload?
    public var child2: Components.Schemas.Parent.child2Payload?
    public var child3: Components.Schemas.Child
    public var child4: Components.Schemas.Child?
}

This API is not easy to use because of value1.

It's still not possible to express the difference between a child2 key that is missing, or present with the value null.

Another attempt, with another patterns for nullable refs:

components:
  schemas:
    Parent:
      type: object
      properties:
        child1:
          oneOf:
            - $ref: '#/components/schemas/Child'
            - type: 'null'
        child2:
          oneOf:
            - $ref: '#/components/schemas/Child'
            - type: 'null'
        child3:
          $ref: '#/components/schemas/Child'
        child4:
          $ref: '#/components/schemas/Child'
      required:
        - child1
        - child3
    Child:
      type: object
public struct Parent: Codable, Hashable, Sendable {
    @frozen public enum child1Payload: Codable, Hashable, Sendable {
        case Child(Components.Schemas.Child)
        case case2(OpenAPIRuntime.OpenAPIValueContainer)
    }
    @frozen public enum child2Payload: Codable, Hashable, Sendable {
        case Child(Components.Schemas.Child)
        case case2(OpenAPIRuntime.OpenAPIValueContainer)
    }
    public var child1: Components.Schemas.Parent.child1Payload
    public var child2: Components.Schemas.Parent.child2Payload?
    public var child3: Components.Schemas.Child
    public var child4: Components.Schemas.Child?
}

This API is not easy to use again.

And it's still not possible to express the difference between a child2 key that is missing, or present with the value null.


In the end:

  • I don't quite care today about the difference between a key that is missing, or present with the value null. But I have to tell about it, because some people may need it.
  • The ergonomics of generated code is not great.

Ideally, I'd have one of:

public struct Parent: Codable, Hashable, Sendable {
    public var child1: Components.Schemas.Child?
    public var child2: Components.Schemas.Child? // still no diff between missing and null
    public var child3: Components.Schemas.Child
    public var child4: Components.Schemas.Child?
}

public struct Parent: Codable, Hashable, Sendable {
    public var child1: Components.Schemas.Child?
    public var child2: Components.Schemas.Child?? // OK-ish
    public var child3: Components.Schemas.Child
    public var child4: Components.Schemas.Child?
}

public struct Parent: Codable, Hashable, Sendable {
    public var child1: Components.Schemas.Child?
    public var child2: OpenAPIRuntime.Presence<Components.Schemas.Child?> // clear
    public var child3: Components.Schemas.Child
    public var child4: Components.Schemas.Child?
}

// enum OpenAPIRuntime.Presence<T> {
//     case missing
//     case present(T)
// }

@groue
Copy link
Author

groue commented Jan 26, 2024

I'm now discovering that changing openapi: 3.0.0 to openapi: 3.1.0 introduces some breaking changes in the generated code with the nullable: true, allOf: - $ref: '...' pattern:

3.0.0

openapi: 3.0.0
...
property:
  nullable: true
  allOf:
    - $ref: '#/components/schemas/Whatever'
// Optionality is there 👍 
struct XXX {
    struct propertyPayload: Codable, Hashable, Sendable {
        var value1: Components.Schemas.Whatever
    }
    var property: propertyPayload?
}

3.1.0

openapi: 3.1.0
...
property:
  nullable: true
  allOf:
    - $ref: '#/components/schemas/Whatever'
// Optionality is lost 😓 
struct XXX {
    struct propertyPayload: Codable, Hashable, Sendable {
        var value1: Components.Schemas.Whatever
    }
    var property: propertyPayload
}

I need to strengthen my knowledge of OpenAPI :-/

@groue
Copy link
Author

groue commented Jan 26, 2024

Ideally, I'd have one of:

My suggestions are such a breaking change that I can not reasonably hope anything like that.

But the ergonomics of the generated code would be enhanced if the generator would recognize frequent patterns, and have the payload types adopt protocols that enhance their ergonomics. This could be done in a non-breaking way. The generator could even be enhanced over time by recognizing more and more patterns, decorating the generated more payload types with more protocols.

@czechboy0
Copy link
Contributor

czechboy0 commented Jan 26, 2024

Thanks @groue for the detail, it really helps me understand what you're trying to do.

Unfortunately, I have some bad news:

Nullability and requirement are orthogonal concerns, because requirement describes the presence of the key, and nullability describes the validity of a null value.

In Swift OpenAPI Generator, we do not make the distinction between a missing value and a null value, both are represented as an optional value. What's more, if a field is both nullable and not required, we condense the two optionals into one, so you never get String??, you only ever get String?.

The main reason we don't have that distinction is because we use the platform JSONDecoder/JSONEncoder and Codable, which don't make that distinction. If we wanted to provide that distinction, it'd basically mean reimplementing all those from scratch in this project, which we decided against.

Note that the nullability syntax changed between OpenAPI 3.0 and 3.1.

OpenAPI 3.0:

Foo:
  type: string
  nullable: true

OpenAPI 3.1:

Foo:
  type:
    - string
    - 'null'

With this in mind, I think you can get some of the distinction with an explicit oneOf? But I haven't tested it myself, just a potential direction of exploration.

@brandonbloom
Copy link

I too am surprised to discovered that the following does not produce an Optional:

oneOf:
  - $ref: '#/components/schemas/Thing'
  - type: 'null'

brandonbloom added a commit to brandonbloom/swift-openapi-generator that referenced this issue Apr 1, 2024
brandonbloom added a commit to brandonbloom/swift-openapi-generator that referenced this issue Apr 1, 2024
@brandonbloom
Copy link

brandonbloom commented Apr 2, 2024

PR #558 contains a proposed improvement for "technique 2" discussed in this thread. If it's accepted, can look into technique 1 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/support Adopter support requests. status/triage Collecting information required to triage the issue.
Projects
None yet
Development

No branches or pull requests

3 participants