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

[Swift5] Generate models where optional properties (x-swift-always-encode) can be flagged to explicitly encode nil when empty #10926

Closed
jarrodparkes opened this issue Nov 22, 2021 · 6 comments

Comments

@jarrodparkes
Copy link
Contributor

jarrodparkes commented Nov 22, 2021

Description

For certain API's, you must explicitly set optional properties to "null" to achieve a desired outcome.

For example, consider an API with the endpoint PATCH /appointment/{appointment_id} where the request body can be used to set an appointment's start time to "null" — this might transition the appointment to a "reschedule" state:

{
  "start_time": null
}

As of openapi-generator@5.3.1, optional properties are only encoded if a value is present (encodeIfPresent). For example:

public struct AppointmentPatchPayload: Codable {

    public private(set) var startAt: Date?

    public init(startAt: Date? = nil) {
        self.startAt = startAt
    }

    public enum CodingKeys: String, CodingKey, CaseIterable {
        case startAt = "start_at"
    }

    // Encodable protocol methods

    public func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encodeIfPresent(startAt, forKey: .startAt)
    }
}

This issue is a request to modify the swift5 generator so that optional properties can be flagged to explicitly encode nil when empty.

openapi-generator version

openapi-generator@5.3.1; not a regression

OpenAPI declaration file content or url

{
  "title": "AppointmentPatchPayload",
  "type": "object",
  "description": "Payload for patching an appointment.",
  "properties": {
    "start_at": {
      "type": "string",
      "description": "The new date and time (ISO 8601 format) when the appointment is set to start.",
      "format": "date-time"
    }
}

Command line used for generation

docker run --rm -v "${PWD}:/local" openapitools/openapi-generator-cli:v5.3.1 generate \
    -I /local/spec/out/Aryeo/Aryeo.MERGED.json \
    -g swift5 \
    --additional-properties readonlyProperties=true,hashableModels=false \
    --skip-validate-spec \
    -o /local/sdk

Steps to reproduce

  1. Create a spec containing a model (ex: AppointmentPatchPayload) with an optional/nullable property
  2. Run the swift5 generator
  3. Observe the model

Related issues/PRs

Suggest a fix/enhancement

Support an OAS extension such as x-swift-always-encode to inform the swift5 generator to always encode a value for optional properties. The resulting model might look like this:

public struct AppointmentPatchPayload: Codable {

    public private(set) var startAt: Date?

    public init(startAt: Date? = nil) {
        self.startAt = startAt
    }

    public enum CodingKeys: String, CodingKey, CaseIterable {
        case startAt = "start_at"
    }

    // Encodable protocol methods

    public func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: CodingKeys.self)

        if let startAt = startAt {
            try container.encode(startAt, forKey: .startAt)
        } else {
            try container.encodeNil(forKey: .startAt)
        }
    }
}
@jarrodparkes jarrodparkes changed the title [Swift5] Generate models where optional properties can be flagged to explicitly encode nil when empty [Swift5] Generate models where optional properties (x-swift-always-encode) can be flagged to explicitly encode nil when empty Nov 22, 2021
@jarrodparkes
Copy link
Contributor Author

swift maintainers:

@jgavris @ehyche @Edubits @jaz-ah @4brunu

@spacether
Copy link
Contributor

Why not update your spec to define those request body schemas to be nullable: true?
This is a normal openapi use case

@jarrodparkes
Copy link
Contributor Author

@spacether I'm pretty sure defining the bodies as nullable will not change how the models are generated. currently they are always generated in a way where all nullable/optional properties will not be encoded if the value is null. this means, it is impossible to create a request body like...

{
  "start_time": null
}

Right now, regardless of the nullable property, if the start_time is nil, then it would be encoded as...

{}

@spacether
Copy link
Contributor

spacether commented Nov 29, 2021

Ah, thank you for explaining that. In python we encone None as null so I wasn't aware of this problem. Thanks!
Unassigned optional properties are unset in python

@jarrodparkes
Copy link
Contributor Author

@spacether no problem 👍

I sort of assumed that is how the Swift models would work out-of-the-box too, but I think I can understand the for/against arguments. at any rate, looking forward to getting this to be configurable on Swift side 😎

@jarrodparkes
Copy link
Contributor Author

jarrodparkes commented Nov 30, 2021

After more research, it looks like I identified an error in my understanding (see the discussion here). When a property is both nullable AND required, then it will always be encoded:

{
  "title": "AppointmentPatchPayload",
  "type": "object",
  "description": "Payload for patching an appointment.",
  "properties": {
    "start_at": {
      "type": "string",
      "description": "The new date and time (ISO 8601 format) when the appointment is set to start.",
      "format": "date-time",
      "x-nullable": true
    }
  }
  "required": [
    "start_at"
  ]
}

Becomes

public struct AppointmentPatchPayload: Codable {

    public private(set) var startAt: Date?

    public init(startAt: Date? = nil) {
        self.startAt = startAt
    }

    public enum CodingKeys: String, CodingKey, CaseIterable {
        case startAt = "start_at"
    }

    // Encodable protocol methods

    public func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: CodingKeys.self)

        // will encode `null` if nil, otherwise will encode the date value
        try container.encode(startAt, forKey: .startAt)
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants