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

[Feature] Enum types from strings or metadata #146

Open
ghost opened this issue Apr 21, 2020 · 8 comments
Open

[Feature] Enum types from strings or metadata #146

ghost opened this issue Apr 21, 2020 · 8 comments

Comments

@ghost
Copy link

ghost commented Apr 21, 2020

Here is a schema.json that demonstrates the issue.

When "enum" schema components are encountered in json schema, SwaggerProvider seems to strip their info away and just label the type as strings or int (depending on JsonStringEnumConverter).

This makes working with APIs a bit painful. Azure seems to deal with this issue using x-ms-enum which means it would add meta data for codegen (regardless of if its an int or string encoding for enum value).

Code to demonstrate issue, noting that it is not possible to even create a TaskType, one has to work out what the valid values are by looking at the raw JSON schema (and hoping it doesn't change like it did for me):

open SwaggerProvider

type Api = OpenApiClientProvider<"schema.json">
let client = Api.Client()
client.GetApiAdminTest("aosdijoasjid") |> ignore

It would be nice if SwaggerProvider could generate this:
type TaskType = This | That | Other based on x-ms-enum or string

Related information

NET Core SDK & Linux

@ghost
Copy link
Author

ghost commented Apr 22, 2020

@sergey-tihon I made a first pass at this here but I am having trouble in a few areas and need pointers (I only took a guess at how to do it):

  • I needed to remove net461 to get compile working on Linux, so I dont think I can PR yet
  • Where do I need to add in the code that handles serialization? e.g. Enum -> String?
  • Testing feels very light and I feel I am not adding them in the right place (I left it commented out on my fork for now)

@sergey-tihon
Copy link
Member

It would be nice if SwaggerProvider could generate this:
type TaskType = This | That | Other based on x-ms-enum or string

First of all, I am not surer that it will be easy task to generate provided enums
fsprojects/FSharp.TypeProviders.SDK#264 AFAIK, SqlEnumProvider.fs is best starting point but I cannot guarantee that it will work as you expect.

Where do I need to add in the code that handles serialization? e.g. Enum -> String?

Serialization is done here https://github.com/fsprojects/SwaggerProvider/blob/master/src/SwaggerProvider.Runtime/ProvidedApiClientBase.fs#L34-L37 but you even can inherit from ProvidedClient and experiment with serialization without modifying TP code http://fsprojects.github.io/SwaggerProvider/#/Customization#serialization

Testing feels very light and I feel I am not adding them in the right place (I left it commented out on my fork for now)

Is it possible to add new controller here https://github.com/fsprojects/SwaggerProvider/tree/master/tests/Swashbuckle.WebApi.Server/Controllers and integrate AutoRest Extensions for Swagger 2.0 with Swashbuckle to emit x-ms-enum?

@ghost
Copy link
Author

ghost commented Apr 30, 2020

@sergey-tihon I am very close to getting it to work. There are two failings tests, both with the same root cause (very similar to this issue), which looks like a serialization issue.

EDIT: I think this issue may be due to headers and query parameters not going thru the OptionConverter json converter.

EDIT2: It seems that "toQueryParams" falls back to use obj.ToString() if the type is not mapped, which makes sense as ToString inside DefinitionCompiler can use more context to serialize the type. I am however stuck with how to implement ToString on the enum type.

(Note that other enum tests are not failing, which suggests this is an issue with the Option type):

15:47:04 DBG] All/v3/Swashbuckle.UpdateControllers.Tests.Update Enum GET Test starting... <Expecto>
      Request starting HTTP/1.1 GET http://localhost:5000/api/UpdateEnum?x=Some(Absolute)  
  • I updated tests to correctly test enum values (previously test code used int).
  • Added support in for values field of x-ms-enum
  • added a Filter to Swashbuckle.WebApi.Server/ to generate x-ms-enum.
  • Latest code still lives on https://github.com/tick-rick/SwaggerProvider as I cant test with net46 targetframework

x-ms-enum in swagger.json:
Screenshot_2020-04-30 Screenshot

@abelbraaksma
Copy link
Member

Just got hit by this myself, in the sense that I thought my OpenApi was wrong as I expected a DU or something like that to be generated from the enum values in the OpenApi yaml specification.

I appreciate that this isn't trivial to add (just mapping it to .NET enums clearly won't work, though DU's seem a natural fit). Just good to know this isn't supported atm.

@petrkoutnycz
Copy link

petrkoutnycz commented Dec 14, 2022

Just leaving a comment here that handling enums in a strongly-typed fashion would be nice.
For instance, I have a string enum defined in Swagger and it is basically skipped so I need to put a string manually which is waaaay too much cumbersome :-)

image

@abelbraaksma
Copy link
Member

abelbraaksma commented Dec 22, 2022

Just realising this, but isn’t generating DUs from a TP currently simply not supported? I mean, CLI enums seems to the best we can do here, right? Or is that essentially the suggestion here?

@sergey-tihon
Copy link
Member

Just realising this, but isn’t generating DUs from a TP currently simply not supported?

Yes, all F# specific types are not supported (like DUs and records), that is why it is int for now.

Not sure about CLI enums ... Never saw them emitted by TP.

@petrkoutnycz
Copy link

CLI enums would be better than nothing, that is for sure. In my case (see the picture above), I can assign just pure string - not DU and not even CLI enum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants