[DefaultCodegen] generate unknown default case #11078

4brunu · 2021-12-09T10:53:18Z

This PR makes #11013 available to all generators, and fixes #1529 to all generators

Currently if the server adds new enum cases, that are unknown by old spec/client, the client will fail to parse the network response.
With the enumUnknownDefaultCase option enabled, each enum will have a new case, ´unknown_default_open_api´, so that when the server send an enum case that is not known by the client/spec, they can safely fallback to this case.

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
```
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master (5.3.0), 6.0.x
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Core Team
@wing328 (2015/07) ❤️
@jimschubert (2016/05) ❤️
@cbornet (2016/05)
@ackintosh (2018/02) ❤️
@jmini (2018/04) ❤️
@etherealjoy (2019/06)
@spacether (2020/05)

…Codegen implementation

jarrodparkes · 2021-12-09T15:13:50Z

docs/generators/ada-server.md

@@ -10,6 +10,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
 |allowUnicodeIdentifiers|boolean, toggles whether unicode identifiers are allowed in names or not, default is false| |false|
 |disallowAdditionalPropertiesIfNotPresent|If false, the 'additionalProperties' implementation (set to true by default) is compliant with the OAS and JSON schema specifications. If true (default), keep the old (incorrect) behaviour that 'additionalProperties' is set to false by default.|<dl><dt>**false**</dt><dd>The 'additionalProperties' implementation is compliant with the OAS and JSON schema specifications.</dd><dt>**true**</dt><dd>Keep the old (incorrect) behaviour that 'additionalProperties' is set to false by default.</dd></dl>|true|
 |ensureUniqueParams|Whether to ensure parameter names are unique in an operation (rename parameters that are not).| |true|
+|enumUnknownDefaultCase|If the server adds new enum cases, that are unknown by old spec/client, the client will fail to parse the network response.With the enumUnknownDefaultCase option enabled, each enum will have a new case, &acute;unknown_default_open_api&acute;, so that when the server send an enum case that is not known by the client/spec, they can safely fallback to this case.|<dl><dt>**false**</dt><dd>No changes to the enum's are made, this is the default option.</dd><dt>**true**</dt><dd>With this option enabled, each enum will have a new cases, &acute;unknown_default_open_api&acute;, so that when the enum case sent by the server is not known by the client/spec, can safely be decoded to this case.</dd></dl>|false|


I addressed a few typos in the description that can be applied across each generator's doc:

|enumUnknownDefaultCase|If the server adds new enum cases, that are unknown by an old spec/client, the client will fail to parse the network response. With this option enabled, each enum will have a new case, ´unknown_default_open_api´, so that when the server sends an enum case that is not known by the client/spec, they can safely fallback to this case.|<dl><dt>**false**</dt><dd>No changes to the enum's are made, this is the default option.</dd><dt>**true**</dt><dd>With this option enabled, each enum will have a new case, ´unknown_default_open_api´, so that when the enum case sent by the server is not known by the client/spec, can safely be decoded to this case.</dd></dl>|false|

jarrodparkes · 2021-12-09T15:18:20Z

modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java

+            } else {
+                // This is a dummy value that attempts to avoid collisions with previously specified cases.
+                // Int.min / 192
+                enumValue = String.valueOf(-11184809);


For context, does the use of "192" need to be explained here (the number of the Swift evolution issue describing frozen/non-frozen enums)?

That's a good question. In Swift it made sense, here I'm not sure. Or maybe we can explain that this feature was initial introduced in Swift, therefore the number.
What do you think?

yeah, maybe a short comment snippet could be helpful. you could keep the comment short and provide the links to the PR/Apple if people want to dig for more context

jarrodparkes · 2021-12-09T15:21:42Z

@4brunu it's awesome that other generators are finding this update useful. it is also pretty cool how you've implemented this — im getting to see new parts of openapi-generator that I haven't seen before. as long as the behavior is equivalent on Swift (I haven't tried to fully test this yet), then that's a pretty nifty way to make this available to all generators 😎

4brunu · 2021-12-09T15:24:31Z

...les/client/petstore/swift5/frozenEnums/PetstoreClient/Classes/OpenAPIs/Models/EnumTest.swift

-        // [SE-0192](https://github.com/apple/swift-evolution/blob/master/proposals/0192-non-exhaustive-enums.md)
-        //
-        case unknownDefaultOpenAPI = -11184809 // Int.min / 192
+        case numberUnknownDefaultOpenApi = -11184809


@jarrodparkes Here we have a side effect that it add's the prefix number to the enum case name... I'm not sure if it's worth the effort to fix this... What do you think?

is keeping the case name unknownDefaultOpenAPI across all enums possible? if so, id say it is desirable. im not sure of the amount of work required to keep it consistent

I don't know either, but I will investigate 👍

# Conflicts: # docs/generators/java.md

# Conflicts: # samples/client/petstore/kotlin-gson/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-jackson/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-jvm-okhttp4-coroutines/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-moshi-codegen/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-nonpublic/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-nullable/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-okhttp3/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-string/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-threetenbp/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin/src/main/kotlin/org/openapitools/client/apis/PetApi.kt

# Conflicts: # modules/openapi-generator/src/main/resources/kotlin-client/libraries/jvm-okhttp/api.mustache # samples/client/petstore/kotlin-gson/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-jackson/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-jvm-okhttp4-coroutines/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-moshi-codegen/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-nonpublic/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-nullable/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-okhttp3/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-string/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin-threetenbp/src/main/kotlin/org/openapitools/client/apis/PetApi.kt # samples/client/petstore/kotlin/src/main/kotlin/org/openapitools/client/apis/PetApi.kt

wing328 · 2021-12-17T04:17:39Z

Can you please resolve the merge conflicts when you've time ?

# Conflicts: # modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/Swift5ClientCodegen.java

4brunu · 2021-12-17T11:57:47Z

Done 🙂

4brunu · 2021-12-20T10:19:53Z

CI is failing, but it seems not to be related to this PR.

wing328 · 2021-12-21T07:39:27Z

Restarted the bitrise CI job and all tests passed.

wing328 · 2021-12-21T07:40:09Z

Thanks for the PR. LGTM. Let's include this in the v5.3.1 release.

vandadnp · 2022-01-18T10:35:06Z

@4brunu hello and thank you so much for your contribution. Could you please provide an example of actually using this flag? I mean we have placed it in our config files, our enums are getting this new case, but nobody is really creating this case when parsing goes wrong. So how is this supposed to be used? Thank you again

4brunu · 2022-01-18T11:19:10Z

Hi @vandadnp, you don't need to do anything to special to use this.
When the parse of the data sent by the server fails, the UnknownDefaultCase will be the default enum automatically.

On Kotlin, this does the trick

openapi-generator/samples/client/petstore/kotlin-okhttp3/src/main/kotlin/org/openapitools/client/infrastructure/SerializerHelper.kt

Lines 7 to 13 in 90972c6

    
           fun addEnumUnknownDefaultCase(moshiBuilder: Moshi.Builder): Moshi.Builder { 
        
               return moshiBuilder 
        
                   .add(org.openapitools.client.models.Order.Status::class.java, EnumJsonAdapter.create(org.openapitools.client.models.Order.Status::class.java) 
        
                       .withUnknownFallback(org.openapitools.client.models.Order.Status.unknownDefaultOpenApi)) 
        
                   .add(org.openapitools.client.models.Pet.Status::class.java, EnumJsonAdapter.create(org.openapitools.client.models.Pet.Status::class.java) 
        
                       .withUnknownFallback(org.openapitools.client.models.Pet.Status.unknownDefaultOpenApi)) 
        
           }

On Swift, this does the trick

openapi-generator/samples/client/petstore/swift5/alamofireLibrary/PetstoreClient/Classes/OpenAPIs/Models.swift

Lines 18 to 38 in 90972c6

    
           /// An enum where the last case value can be used as a default catch-all. 
        
           protocol CaseIterableDefaultsLast: Decodable & CaseIterable & RawRepresentable 
        
           where RawValue: Decodable, AllCases: BidirectionalCollection {} 
        
           extension CaseIterableDefaultsLast { 
        
               /// Initializes an enum such that if a known raw value is found, then it is decoded. 
        
               /// Otherwise the last case is used. 
        
               /// - Parameter decoder: A decoder. 
        
               public init(from decoder: Decoder) throws { 
        
                   if let value = try Self(rawValue: decoder.singleValueContainer().decode(RawValue.self)) { 
        
                       self = value 
        
                   } else if let lastValue = Self.allCases.last { 
        
                       self = lastValue 
        
                   } else { 
        
                       throw DecodingError.valueNotFound( 
        
                           Self.Type.self, 
        
                           .init(codingPath: decoder.codingPath, debugDescription: "CaseIterableDefaultsLast") 
        
                       ) 
        
                   } 
        
               } 
        
           }

So, as a user of openapi, you just need to enable the flag enumUnknownDefaultCase, everything else is automatic.
You can test this locally trying to parse a json string that contains an unknown enum case.

Hope that I answer your question.

vandadnp · 2022-01-18T11:44:00Z

@4brunu thank you so much for your reply.

What we can see is that our enums are getting the extra case, however, nowhere in the generated code is that case actually being created. So could it be that we have to update our Mustache files?

Thank you
/v

4brunu · 2022-01-18T11:44:54Z

Are you using Swift or Kotlin? Do you have custom Mustache files?

vandadnp · 2022-01-18T11:54:16Z

@4brunu we are using Swift and we just downloaded the latest Mustache files from the repo openapi-generator repo and still facing the same issue. All enums now have the unknownDefaultOpenApi case but searching through the entire code base we don't actually see any code that returns this value at all. They are just there, but nobody returns those values.

vandadnp · 2022-01-18T11:57:48Z

Okay perhaps now I can see what's going on as you showed in your example. After updating to the latest Mustache files I can see this code:

So I believe this will fix the issue, correct? Much appreciated

4brunu · 2022-01-18T11:58:37Z

Those enums should conform to the protocol CaseIterableDefaultsLast, that were the logic is.

openapi-generator/samples/client/petstore/swift5/alamofireLibrary/PetstoreClient/Classes/OpenAPIs/Models.swift

Lines 18 to 38 in 90972c6

    
           /// An enum where the last case value can be used as a default catch-all. 
        
           protocol CaseIterableDefaultsLast: Decodable & CaseIterable & RawRepresentable 
        
           where RawValue: Decodable, AllCases: BidirectionalCollection {} 
        
           extension CaseIterableDefaultsLast { 
        
               /// Initializes an enum such that if a known raw value is found, then it is decoded. 
        
               /// Otherwise the last case is used. 
        
               /// - Parameter decoder: A decoder. 
        
               public init(from decoder: Decoder) throws { 
        
                   if let value = try Self(rawValue: decoder.singleValueContainer().decode(RawValue.self)) { 
        
                       self = value 
        
                   } else if let lastValue = Self.allCases.last { 
        
                       self = lastValue 
        
                   } else { 
        
                       throw DecodingError.valueNotFound( 
        
                           Self.Type.self, 
        
                           .init(codingPath: decoder.codingPath, debugDescription: "CaseIterableDefaultsLast") 
        
                       ) 
        
                   } 
        
               } 
        
           }

if the decode of json string to enum fails, it uses by default the last enum case.

else if let lastValue = Self.allCases.last { 
     self = lastValue 
}

So you don't need to do anything, this will just work.

ma-pe · 2022-03-02T15:02:03Z

@4brunu we are using Swift and we just downloaded the latest Mustache files from the repo openapi-generator repo and still facing the same issue. All enums now have the unknownDefaultOpenApi case but searching through the entire code base we don't actually see any code that returns this value at all. They are just there, but nobody returns those values.

I am getting exactly this behaviour when using dart-dio-next as generator. The enum value unknownDefaultOpenApi is being added to every enum. Yet when deserialising an unknown value the deserialisation fails.

I am a little bit lost where to start digging.

4brunu · 2022-03-02T15:10:56Z

@ma-pe the problem is that this two step feature.
The first step is to generate the enum value unknownDefaultOpenApi, and this was implemented globally for all the generators.
The second step is configure the library that decode/parses the json response into the models.
This second step needs to be implemented generator by generator, library by library.
Right now the second step, as far as I know, is only implemented in the kotlin client generator when using Moshi, and in the Swift 5 generator using Codable.
All the other generators (java, ruby, dart, etc) and libraries (gson, jackson, kotlin serializable, etc) need to implement this second step.

ma-pe · 2022-03-02T15:17:19Z

@4brunu Ah, I see. Thanks a lot for your (super fast) response and explanation.

janisz · 2022-06-13T10:22:46Z

Is it working for Java? For me it indeed creates a new enum value but not using it and still throws exception

  UNKNOWN_DEFAULT_OPEN_API("unknown_default_open_api");

  public static StorageImageScanNote fromValue(String value) {
    for (StorageImageScanNote b : StorageImageScanNote.values()) {
      if (b.value.equals(value)) {
        return b;
      }
    }
    throw new IllegalArgumentException("Unexpected value '" + value + "'");
  }

4brunu · 2022-06-13T10:28:35Z

@janisz As far as I can tell it's not, and #11078 (comment) is still updated.

4brunu added 8 commits December 8, 2021 17:30

[DefaultCodegen] generate unknown default case

e4d39c6

[DefaultCodegen] replace Swift custom implementation with the Default…

d2de1b1

…Codegen implementation

[DefaultCodegen] generate unknown default case

a2a0738

[DefaultCodegen] generate unknown default case

013695c

[DefaultCodegen] generate unknown default case

c657c92

[DefaultCodegen] generate unknown default case

732a345

[DefaultCodegen] generate unknown default case

f935cae

[DefaultCodegen] generate unknown default case

938497c

4brunu mentioned this pull request Dec 9, 2021

Add swift5 option for generating frozen enums #11013

Merged

5 tasks

4brunu added 2 commits December 9, 2021 11:27

[DefaultCodegen] generate unknown default case

f01ba3a

[DefaultCodegen] generate unknown default case

1fe52fb

jarrodparkes reviewed Dec 9, 2021

View reviewed changes

4brunu commented Dec 9, 2021

View reviewed changes

4brunu added 8 commits December 9, 2021 15:42

[DefaultCodegen] update docs

fc31f04

[DefaultCodegen] fix Swift enum case name

9fa7227

Merge branch 'master' into feature/kotlin-unknown-enum-case

3c59db7

# Conflicts: # docs/generators/java.md

[DefaultCodegen] generate unknown default case

bb49ccb

[DefaultCodegen] generate unknown default case

ec6533d

Merge branch 'master' into feature/kotlin-unknown-enum-case

f15edb8

wing328 added Enhancement: Feature Feature: Enum labels Dec 17, 2021

wing328 added this to the 5.3.1 milestone Dec 17, 2021

Merge branch 'master' into feature/kotlin-unknown-enum-case

88f56ce

# Conflicts: # modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/Swift5ClientCodegen.java

Merge branch 'master' into feature/kotlin-unknown-enum-case

5b4b7ba

wing328 merged commit c305c71 into OpenAPITools:master Dec 21, 2021

4brunu deleted the feature/kotlin-unknown-enum-case branch January 18, 2022 11:09

dhaus67 mentioned this pull request Jun 13, 2022

ROX-11356: Update API spec to latest version (3.70.0). stackrox/jenkins-plugin#152

Merged

4brunu mentioned this pull request Jul 20, 2022

[kotlin][client] add support for unknown default case with Kotlinx Serialization #12930

Merged

5 tasks

4brunu mentioned this pull request Jul 18, 2024

add oneOf-default-case option for swift5 #19094

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DefaultCodegen] generate unknown default case #11078

[DefaultCodegen] generate unknown default case #11078

4brunu commented Dec 9, 2021

jarrodparkes Dec 9, 2021

jarrodparkes Dec 9, 2021

4brunu Dec 9, 2021

jarrodparkes Dec 9, 2021

jarrodparkes commented Dec 9, 2021

4brunu Dec 9, 2021

jarrodparkes Dec 9, 2021

4brunu Dec 9, 2021

4brunu Dec 9, 2021

wing328 commented Dec 17, 2021

4brunu commented Dec 17, 2021

4brunu commented Dec 20, 2021

wing328 commented Dec 21, 2021

wing328 commented Dec 21, 2021

vandadnp commented Jan 18, 2022

4brunu commented Jan 18, 2022 •

edited

Loading

vandadnp commented Jan 18, 2022

4brunu commented Jan 18, 2022 •

edited

Loading

vandadnp commented Jan 18, 2022

vandadnp commented Jan 18, 2022

4brunu commented Jan 18, 2022

ma-pe commented Mar 2, 2022

4brunu commented Mar 2, 2022

ma-pe commented Mar 2, 2022

janisz commented Jun 13, 2022

4brunu commented Jun 13, 2022

[DefaultCodegen] generate unknown default case #11078

[DefaultCodegen] generate unknown default case #11078

Conversation

4brunu commented Dec 9, 2021

PR checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarrodparkes commented Dec 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wing328 commented Dec 17, 2021

4brunu commented Dec 17, 2021

4brunu commented Dec 20, 2021

wing328 commented Dec 21, 2021

wing328 commented Dec 21, 2021

vandadnp commented Jan 18, 2022

4brunu commented Jan 18, 2022 • edited Loading

vandadnp commented Jan 18, 2022

4brunu commented Jan 18, 2022 • edited Loading

vandadnp commented Jan 18, 2022

vandadnp commented Jan 18, 2022

4brunu commented Jan 18, 2022

ma-pe commented Mar 2, 2022

4brunu commented Mar 2, 2022

ma-pe commented Mar 2, 2022

janisz commented Jun 13, 2022

4brunu commented Jun 13, 2022

4brunu commented Jan 18, 2022 •

edited

Loading

4brunu commented Jan 18, 2022 •

edited

Loading