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.26k stars 91 forks source link

Handling of enum unknown cases #428

Closed groue closed 6 months ago

groue commented 6 months ago

Hello,

Unless I'm mistaken, Swift OpenAPI Generator does not generate code that is able to handle unknown enum cases in responses.

The scenario is very classic:

  1. The openapi.yml file defines an enum with a limited number of known cases.
  2. Code is generated against this specification.
  3. The server outputs a value that is not listed in the known cases.
  4. How is this handled by the generated code?

As far as I know, the sanction is immediate: a DecodingError.dataCorrupted is thrown. It is impossible for the client to decide how unrecognized cases should be handled.


Of course, adding a new enum case is, strictly speaking, a breaking change. So a server that adds an undocumented enum case is wrong.

A certain amount of pragmatism can help sorting things out, though. Not all server apis have reached "version 1.0". It is not unreasonable for the client to request the ability to decide what to do with unrecognized cases, just in case the server evolves in an unexpected way:

The OpenAPI format does not allow to distinguish between "frozen" and "not frozen" enums. It's impossible for the author of the OpenAPI spec to express the intent. When all enums are considered "frozen", the only way to avoid errors from overly strict OpenAPI generators is to discard all non-frozen enums and replace them with the type of their raw value. But this creates new problems on its own:

Finally, a Google search reveals that most OpenAPI generators repositories have an issue similar to this one. Most support unknown cases. Apple/swift-protobuf supports "unrecognized" cases as well.


Looking a what swift-protobuf does, I find this examples in the tests:

// SwiftProtobug.Enum is found at
// <https://github.com/apple/swift-protobuf/blob/0d922f013e900fccea5d083dedc691202ea8d399/Sources/SwiftProtobuf/Enum.swift#L19>
// public protocol Enum: RawRepresentable, Hashable, CaseIterable, Sendable { ... }
enum SwiftProtoTesting_UnknownEnum_Proto3_MyEnum: SwiftProtobuf.Enum {
  typealias RawValue = Int
  case foo // = 0
  case bar // = 1
  case baz // = 2
  case UNRECOGNIZED(Int)

  init() {
    self = .foo
  }

  init?(rawValue: Int) {
    switch rawValue {
    case 0: self = .foo
    case 1: self = .bar
    case 2: self = .baz
    default: self = .UNRECOGNIZED(rawValue)
    }
  }

  var rawValue: Int {
    switch self {
    case .foo: return 0
    case .bar: return 1
    case .baz: return 2
    case .UNRECOGNIZED(let i): return i
    }
  }

  // The compiler won't synthesize support with the UNRECOGNIZED case.
  static let allCases: [SwiftProtoTesting_UnknownEnum_Proto3_MyEnum] = [
    .foo,
    .bar,
    .baz,
  ]
}

I don't like the init() at all (maybe it fits ProtoBuf), but otherwise I find it pretty much satisfying.

I like the CaseIterable support that is a nice touch.

I also like the big scary UNRECOGNIZED that helps the user avoid producing this case when feeding requests.

groue commented 6 months ago

At the risk of sounding empathetic, I consider this issue as quite serious.

It's not a blocker because I happen to be able to replace enums with their raw values in my openapi.yml (with the bad consequence on requests described above).

So I'm able to make my client app able to decide what to do with unknown cases, which is a major need.

But the cost is a damaged openapi.yml.

I hope this issue will find your ears!

czechboy0 commented 6 months ago

Hi @groue, Swift OpenAPI Generator used to always generate an case unknown(String), but it was removed because it completely breaks composition and other features of JSON Schema, like oneOf (because the failure to decode a value is used as signal by the caller).

Instead, we documented this pattern back then for those who explicitly want to allow "open enums" here: https://swiftpackageindex.com/apple/swift-openapi-generator/0.3.5/documentation/swift-openapi-generator/useful-openapi-patterns#Open-enums-and-oneOfs

groue commented 6 months ago

Thank you @czechboy0 for your answer. You have unblocked my personal case, so it's OK to close this issue 👍

The recommended technique assumes that the OpenAPI generator user can freely modify openapi.yml. You now that I think this is not a tenable position. This just does not scale with the size and complexity of user projects. And this leads to a weird inversion: the library fits the needs of its maintainers, instead of fitting the needs of its users. I urge the team to reconsider the assumption that people can freely modify the openapi.yml file.

czechboy0 commented 6 months ago

I appreciate what you're saying, but this project is not an OpenAPI linter. There are plenty of such tools in the wider ecosystem that adopters can use to get an OpenAPI document that's to their liking and represents the backend API.

Swift OpenAPI Generator uses the input OpenAPI document as the source of truth, and focuses on generating Swift code that faithfully represents that API contract. If people want to change the API contract for whatever reason, they need to do that before they pass the document to the generator.

This rationale follows the pattern of small, single-purpose, focused tools that can be freely composed; rather than large monolithic tools that do several things. I understand that this might not be everyone's preference, but it is how this project was designed, and this is documented from the beginning in https://swiftpackageindex.com/apple/swift-openapi-generator/0.3.5/documentation/swift-openapi-generator/project-scope-and-goals.

groue commented 6 months ago

Apologies for sounding harsher than needed. After all, it could be argued that the recommended anyOf technique is a good way, for an OpenAPI spec author, to specify a "non-frozen" enum. Yet I still feel for fellow developers who happen to consume an openapi.yml that was not given that amount of care.

groue commented 6 months ago

I appreciate what you're saying, but this project is not an OpenAPI linter. There are plenty of such tools in the wider ecosystem that adopters can use to get an OpenAPI document that's to their liking and represents the backend API.

No linter can decide if an enum is frozen or not. This issue is not a request for a linter feature.

groue commented 6 months ago

So I use the recommended technique based on anyOf. I'm not sure I'm rewarded for my virtue.

Here is the boilerplate code that I have to write, for each and every non-frozen enum, with no possibility of code reuse. Also note how the generated type is not very handy: since both value1 and value2 are optionals, code has to deal with an unreachable path.

// The public enum that was needed in the first place
public enum MyEnum {
   case known1
   case known2
   case UNRECOGNIZED(String)
}

extension MyEnum {
    // components:
    //   schemas:
    //     MyEnum:
    //       anyOf:
    //         - type: string
    //           enum:
    //             - known1
    //             - known2
    //         - type: string
    init(payload: Components.Schemas.MyEnum) {
        if let value1 = payload.value1 {
            switch value1 {
            case .known1:
                self = .known1
            case .known2:
                self = .known2
            }
        } else if let value2 = payload.value2 {
            self = .UNRECOGNIZED(value2)
        } else {
            preconditionFailure("This can't happen, right?")
        }
    }
}

This makes me want a generator on top of the generator 🤣 So, no, definitely, this is not a request for a linter feature 🙂

czechboy0 commented 6 months ago

After all, it could be argued that the recommended anyOf technique is a good way, for an OpenAPI spec author, to specify a "non-frozen" enum. Yet I still feel for fellow developers who happen to consume an openapi.yml that was not given that amount of care.

In JSON Schema 2020-12, used by OpenAPI 3.1.0, enums are closed by default: https://json-schema.org/draft/2020-12/json-schema-validation#name-enum

We need to respect that as an OpenAPI generator. We can't offer an option to make enums open, because it would break deserialization of enums within other types like oneOfs.

If you disagree with enums being closed by default, I think (not being snarky here, genuinely might be a good idea) you could engage with the JSON Schema community, they're super nice and welcoming. If some future JSON Schema specification version supports open enums out of the box without breaking nested structures, and it's used by a future OpenAPI version, then we'd have no choice but to support it 🙂

But we can't go against the specification, especially where it's so explicit. When the specification is silent or ambiguous, we can make a judgement call, but that's not the case here. And we learned that from experience, as again, we used to have open enums initially, and it made it impossible to implement features that compose JSON schemas.

Here is the boilerplate code that I have to write

If you do this often, you could write a member macro that generates the matching type with the extra case for you, pretty similar to the official examples: https://github.com/apple/swift-syntax/tree/main/Examples/Tests/MacroExamples/Implementation/Member

groue commented 6 months ago

It is normal that you list all my possible workarounds 👍

You will note that all of them require a non-minor amount of work, with no guarantee of success.

The idea of jumping in the JSON Schema community is honest, but my own honesty answers that the maintainers of an OpenAPI generator are in a much more favorable position for arguing in this arena. Your cold support for my request does not make me optimistic. At best, I could attempt at asking whether the linked spec could be extended with a sentence like:

A dedicated 'unknown' instance MAY validate successfully this keyword if its value is not equal to any of the elements in this keyword's array value".

I can not hope more than a MAY. This one would at least match the reality, because some generators already support this feature. A SHOULD is already controversial, and a MUST is completely out of reach for a nobody like I am. And even with a SHOULD, Swift OpenAPI Generator could keep on being strict, for convenience reasons ("composition and other features of JSON Schema, like oneOf, etc.")

In the end, not only me, but all Swift OpenAPI Generator users that happen to meet this problem will have to do the extra work described above. It could be argued that this is a user-hostile situation that needs a fix.

Anyway. This also makes Swift OpenAPI generator an outlier. A Google search for "openapi generator unknown enum case" reveals just how frequent is the implemented support for the requested feature.

groue commented 6 months ago

Interestingly, Protocol Buffers have evolved:

Prior to the introduction of syntax = "proto3" all enums were closed. Proto3 introduced open enums specifically because of the unexpected behavior that closed enums cause.

"Unexpected", you bet!

This same page lists "known issues" that are very funny:

  • All known C++ releases are out of conformance. When a proto2 file imports an enum defined in a proto3 file, C++ treats that field as a closed enum.
  • All known C# releases are out of conformance. C# treats all enums as open.
  • All known Java releases are out of conformance. When a proto2 file imports an enum defined in a proto3 file, Java treats that field as a closed enum.
  • All known Kotlin releases are out of conformance. When a proto2 file imports an enum defined in a proto3 file, Kotlin treats that field as a closed enum.
  • All known Go releases are out of conformance. Go treats all enums as open.
  • All known Ruby releases are out of conformance. Ruby treats all enums as open.
  • ...

Frequently the non-conformances tend to favor open enums 👿

Nagging aside (sorry), I really have no hope at all to have the slightest influence on this topic in the JSON Schema community. This is one of the most radioactive topics.

czechboy0 commented 6 months ago

I hear your concerns, but let me reiterate - this is not a decision that we made. We simply implement the JSON Schema and OpenAPI specifications, which are unambiguous on this topic.

We literally cannot offer open enums without breaking decoding of composite types, even if we wanted to diverge from the specification in this one place. But diverging from the specification defeats the purpose of having language-agnostic specifications in the first place.

Many other generators don't even support oneOf and other type composition, which is why I suspect they might be able to offer open enums. But we can't, without breaking functionality that adopters already rely on.

czechboy0 commented 6 months ago

And the open enum pattern is pretty well know, for example see OpenAI's API here, using this to either provide a well-known model name, or a freeform string: https://github.com/openai/openai-openapi/blob/master/openapi.yaml#L5307-L5322

groue commented 6 months ago

We literally cannot offer open enums without breaking decoding of composite types.

I think I understand what you mean.

In the example below, FooOrBarExclusively would never match anything if the Foo and Bar enums would be open. Indeed 1. both enums would match any input (due to their openness), but 2. oneOf requires exclusive matching. This is a case where making enums open would introduce a breaking change:

components:
  schemas:
    Foo:
      type: string
      enum:
      - foo
    Bar:
      type: string
      enum:
      - bar
    FooOrBarExclusively:
      oneOf:
        - $ref: '#/components/schemas/Foo'
        - $ref: '#/components/schemas/Bar'

There are probably other similar scenarios which would break if enums were open.

We simply implement the JSON Schema and OpenAPI specifications, which are unambiguous on this topic.

And this is good. Nobody wants things to be different. Maybe the "simply" could be amended, based on the feedback of users who describe their pain implementing their own needs on top of Swift OpenAPI generator. It would not be good for the library to be identified as "risky" ("risky" as in: everything works fine until you hit a wall that is expensive to work around, due to the library insisting on being "simple").

I'll continue thinking about this, in parallel with #375. It is valuable for the library user to be able to opt in for some specific behaviors, even if they derail from the specification, as long as the consequences are well understood.

During my morning shower I was imagining that openapi-generator-config.yaml could allow the user to control the generation of component types:

generate:
  - types
  - client
components:
  schemas:
    Foo:
      # A way to opt out from generation.
      # Module has to define its own `Components.Schemas.Foo`,
      # according to some documented constraints
      # (such as Codable conformance, etc, whatever is needed)
      generate: off
    Bar:
      # A way to opt in for an open enum, assuming all
      # consequences are well understood.
      generate: open_enum

Side note: the macro approach mentioned above can not work, because the macro does not have access to the enum members in the openapi.yml file.

czechboy0 commented 6 months ago

the macro approach mentioned above can not work, because the macro does not have access to the enum members in the openapi.yml file

Right, it'd require #383, which we're open to doing/accepting a PR for, because it seems to possibly solve a bunch of customization needs.

czechboy0 commented 6 months ago

Regarding excluding some schemas from generation, please file a separate issue for it and describe the use case more there. It was briefly considered when the filtering feature was being added, but we wanted to wait for folks asking for it. So this might help motivate adding it.

The other idea, generate: open_enum, is less likely to make it through, as it'd invalidate all our tests that assume enums are closed, so while I understand the appeal, I think it's much more likely that we go the path of #383 instead, and let folks customize in their own apps, without complicating the generator itself.