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

[BUG] [c-sharp] Inconsistency in default values discriminator #6225

Closed
5 of 6 tasks
tomelfring opened this issue May 8, 2020 · 2 comments
Closed
5 of 6 tasks

[BUG] [c-sharp] Inconsistency in default values discriminator #6225

tomelfring opened this issue May 8, 2020 · 2 comments

Comments

@tomelfring
Copy link
Contributor

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • What's the version of OpenAPI Generator used?
  • Have you search for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Bounty to sponsor the fix (example)
Description

Using the C# generator I found some inconsistencies in the default values for classes with a discriminator with a mapping.

When the discriminator propertyName is in camelCase, I notice the following:

  • JsonConverter uses a camelCase value
  • The property of the discriminator has a DataMemberAttribute with a camelCase name
  • In subclasses, the default value of the discriminator is default(string)

When the discriminator propertyname is in PascalCase, I notice the following:

  • JsonConverter uses a camelCase value
  • The property of the discriminator has a DataMemberAttribute with a PascalCase name
  • In subclasses, the default value of the discriminator is matching the spec

Expected:

  • The JsonConverter converterParameters and Datamember name to match
  • The subclasses having meaningful default values for discriminator parameters when mapping is provided in the spec
openapi-generator version

openapi-generator-cli-4.3.1.jar

OpenAPI declaration file content or url

Two files, one with camelCase, one with PascalCase:
https://gist.github.com/thommy101/39b65b3a365a09d3738f26edc3e1682b

Command line used for generation

java -jar openapi-generator-cli-4.3.1.jar generate -i camelCase.yaml -g csharp -o camelCase
java -jar openapi-generator-cli-4.3.1.jar generate -i pascalCase.yaml -g csharp -o pascalCase

Steps to reproduce
  • Execute above two command lines
  • Compare the camelCase and pascalCase variants of src/Org.OpenAPITools/Models/Animal.cs and src/Org.OpenAPITools/Models/Cat.cs
  • See the CamelCase variant of Animal having consistent JsonConverter-attribute on the class and DataMember-attribute on the property ClassName, but not having a default value for Cat's constructor parameter className (default(string))
  • See the PascalCase variant of Animal having inconsistent JsonConverter-attribute on the class and Datamember-attribute on the property ClassName, but having a correct default value for Cat's constructor parameter className ("Cat")
Related issues/PRs

#3308

#5680: Related because forcing the JsonConverter attribute to camelCase and because of the comment:

Right now if this discriminator isn't camelCased it causes things to not deserialize properly when receiving them from an API (since the json is expected to come back camelcased)

Suggest a fix
@auto-labeler
Copy link

auto-labeler bot commented May 8, 2020

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

tomelfring added a commit to tomelfring/openapi-generator that referenced this issue May 9, 2020
@tomelfring
Copy link
Contributor Author

Did some debugging on the default value, found a bug in the generator. I've opened PR #6232 for that.

The inconsistency of the attributes is caused by #5680 which forces the JsonConverter converterParameters to be camelCase. I don't know how to fix it, as that PR comments that PascalCase is not in line with the JSON standard.

tomelfring added a commit to tomelfring/openapi-generator that referenced this issue May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant