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

[Support enumUnknownDefaultCase] Is there a way to add default case for generated enums? #608

Open
JeonJiyoun opened this issue Aug 5, 2024 · 5 comments
Labels
kind/feature New feature. status/triage Collecting information required to triage the issue.

Comments

@JeonJiyoun
Copy link

JeonJiyoun commented Aug 5, 2024

Motivation

The current swift-openapi-generator does not provide a built-in way to handle unknown enum cases gracefully. This can lead to decoding errors when the API introduces new enum cases that the client is not aware of. Currently, developers have to implement custom solutions or modify generated code manually to handle this scenario, which is not ideal for maintainability and can be error-prone.

As I know, the openapi-generator project already has an enumUnknownDefaultCase option for Swift code generation, which automatically adds an unknown case to enums and handles decoding gracefully. It would be beneficial if swift-openapi-generator could adopt a similar feature.

Proposed solution

Implement an enumUnknownDefaultCase option in swift-openapi-generator, similar to the one in openapi-generator.
Maybe it can be set at the openapi-generator-config.yaml file.
When enabled, this option would:

  1. Add an unknown case to all generated enums.
  2. Implement a custom init(from:) for Codable conformance that uses this unknown case when an unrecognized value is encountered.

Example of desired output:

enum ExampleEnum: String, Codable, CaseIterable {
    case value1 = "VALUE1"
    case value2 = "VALUE2"
    case unknown = "UNKNOWN_DEFAULT_OPEN_API"

    init(from decoder: Decoder) throws {
        let container = try decoder.singleValueContainer()
        let rawValue = try container.decode(String.self)
        self = ExampleEnum(rawValue: rawValue) ?? .unknown
    }
}

### Alternatives considered

If this feature already exists or if there are other good methods to achieve this, I would greatly appreciate any information or guidance you can provide.

### Additional information

[openapi-generator](https://github.com/OpenAPITools/openapi-generator/blob/master/docs/generators/swift5.md)
@JeonJiyoun JeonJiyoun added kind/feature New feature. status/triage Collecting information required to triage the issue. labels Aug 5, 2024
@czechboy0
Copy link
Collaborator

Hi @JeonJiyoun,

in OpenAPI/JSON Schema, enums are closed by default, so the current behavior of swift-openapi-generator is correct.

If in your API you'd like to express open enums, allowing new cases to be added in the future, we documented that common pattern here: https://swiftpackageindex.com/apple/swift-openapi-generator/1.3.0/documentation/swift-openapi-generator/useful-openapi-patterns#Open-enums-and-oneOfs

@JeonJiyoun
Copy link
Author

JeonJiyoun commented Aug 5, 2024

Thank you for your response. However, I still have a few questions.
image

When writing the yaml file as described above, should we use buttonTypePayload.value1.none and buttonTypePayload.value2 to utilize the generated code? I’m wondering if we really need to use the unnecessary value1 and value2. The enumUnknownDefaultCase option provided by OpenAPI Generator seems more convenient. Could you please help me understand why the open enum approach is followed in this manner?

Like how OpenAPI Generator has set the default option of enumUnknownDefaultCase to false, wouldn’t it be better to keep this option off by default and allow users to control it?

@czechboy0
Copy link
Collaborator

czechboy0 commented Aug 5, 2024

The problem with the simple approach of allowing open enums unless they were requested by the OpenAPI document is that it breaks decoding when e.g. oneOfs are nested within other oneOfs. The OpenAPI specification is very clear about the prescribed behavior, and our generator tries to follow the specification as closely as possible. While it might be more convenient to diverge, we believe that's not the right approach, as it will create incompatibilities between code written in different languages (e.g. one fails on an unknown enum case, another allows it). That's why the specification exists - to prescribe the correct behavior.

Now, regarding the practical terms of working with an open enum (anyOf with a nested enum), here are some tips:

  • When parsing such a value, check value1 first - if it's not nil, you know that the value matches one of the known cases, and you can unwrap the exact case conveniently in a switch statement, for example. Now, when value1 is present, value2 will also be present, it'll just be the raw string of the received value. But you can safely ignore value2 in that case.
  • When value1 is nil, you read value2 and know that it's an unknown case, so treat it accordingly.
  • When constructing a value with a known case, you only need to fill in value1 or value2. The encoder will use the first non-nil value and skip the rest of the cases, so even if you fill in both value1 and value2, it doesn't change the runtime behavior.
  • When contructing a value with an unknown case, keep value1 nil and fill in onlly value2.

@JeaSungLEE
Copy link

JeaSungLEE commented Aug 6, 2024

@czechboy0 Please consider offering the option one more time.

I understand your perspective on using open enums. However, there are several challenges in applying this approach to our current project:

  1. Cross-platform consistency: Our Android app is already handling unknown enum cases using the enumUnknownDefaultCase option. If our iOS app can’t process unknown enum cases according to the server API specifications, it would lead to inconsistent behavior across platforms.
  2. Existing API infrastructure: We already have hundreds of APIs generated on the server side. Implementing open enums for all these APIs would require significant additional effort.
  3. Server-side changes: Requesting the server team to modify the specifications as you suggested might be difficult to justify, as both frontend and Android teams are currently using the APIs without issues. It would be challenging to convince them to invest additional resources for this change.
  4. Migration concerns: This issue is likely to affect many developers who are considering migrating from the openapi-generator to swift-openapi-generator. If migration proves problematic, we might be forced to either continue using the existing openapi-generator or implement our own solution.
    We genuinely want to utilize this library effectively. It would be extremely beneficial if the functionality provided by the existing OpenAPI Generator for handling open enums could be made available in this project as well, perhaps as a plugin or an optional configuration. This flexibility would allow us to use this project more effectively and would likely make it more attractive to other developers facing similar challenges.

@czechboy0
Copy link
Collaborator

Hi @JeaSungLEE,

I empathize with your situation, but I'm genuinely not sure how to do what you're asking without breaking the generator.

If we generate an "unknown" case for all enums, meaning that the enum never fails to parse (regardless of what string you provided it), once you put such an enum into e.g. a oneOf (which must match exactly one sub-schema, not more, not less), it will break the decoding logic. Meaning we will parse the incorrect case, and if you then forward that value to e.g. an upstream service, it'll be incorrect data. That goes directly against one of our three main principles.

What you're describing is that a set of teams (possibly unintentionally) decided to use a modified variant of OpenAPI (which then isn't OpenAPI + JSON Schema anymore), and configured all their tools to follow that specification. But any tool that follows the standard OpenAPI specification will fail to parse such data, defeating the point of having a cross-language specification in the first place.

Now, here's a suggestion that hopefully allows you to achieve what you need without having to ask the other teams to change what they're doing: preprocess the OpenAPI document.

You likely have some sort of process or a script that copies the OpenAPI document from an upstream location into the Sources directory of your package, which then allows Swift OpenAPI Generator to generate the Swift code from the OpenAPI document. So you can write a small script (e.g. using OpenAPIKit, the same library that Swift OpenAPI Generator uses to parse the OpenAPI document) that parses the original document, modifies all enums to be wrapped in anyOf (aka turning them into open enums), and writes the document back out. The modified document is then what you'd feed into Swift OpenAPI Generator, and you should get the best of both worlds. I've taken this approach in the past as well several times, usually when the upstream documents had some bugs that could be fixed up automatically.

Hope this unblocks you!

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

No branches or pull requests

3 participants