CarterCommunity / Carter

Carter is framework that is a thin layer of extension methods and functionality over ASP.NET Core allowing code to be more explicit and most importantly more enjoyable.
MIT License
2.05k stars 172 forks source link

Solution to support Enum's in OpenAPI for Issue #262 #264

Closed Dreamwalker666 closed 3 years ago

Dreamwalker666 commented 3 years ago

Created a possible solution for issue #262

JoeStead commented 3 years ago

I wonder if there should be some config option for this. Enums in C# are a bit weird imo, it makes sense (mostly) for them to be serialised as strings, but I have come across solutions using integers over the wire (this is a bad idea, but we should cater for it)

Dreamwalker666 commented 3 years ago

I wonder if there should be some config option for this. Enums in C# are a bit weird imo, it makes sense (mostly) for them to be serialised as strings, but I have come across solutions using integers over the wire (this is a bad idea, but we should cater for it)

I honestly have the same thoughts but decided to first implement in the way OpenAPI expects. I really think its a bad idea to use numbers! (pesky magic numbers)

Maybe use a custom attribute to override the behavior? I think strings should be the default (we should encourage good API design) but allow it to expose the numeric values. I guess as well turn off (but currently it gives out a blank object which isn't useful at all)

JoeStead commented 3 years ago

Ooof, would be wary of magic attributes. I think string defaults make sense, perhaps an extension to the CarterOptions to allow an override? I thinking turning it off makes no sense, so either string (default) or number (override)

Dreamwalker666 commented 3 years ago

Ooof, would be wary of magic attributes. I think string defaults make sense, perhaps an extension to the CarterOptions to allow an override? I thinking turning it off makes no sense, so either string (default) or number (override)

I only said attribute as you may have some enums that require number but not all. Also the number they will still have to be a string in the schema as enum only supports string from what I've read.

jchannon commented 3 years ago

Can you do a rebase please before I merge!?

Dreamwalker666 commented 3 years ago

@jchannon ok sorted it was expecting that issue due to the other branch :)

jchannon commented 3 years ago

Nice! Thanks 👍