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

The using of StringEnumConverter end up generating unserializable data when different NamingPolicy is needed #178

Open
attilah opened this issue Oct 5, 2023 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@attilah
Copy link

attilah commented Oct 5, 2023

Describe the bug
This problem is with the same API as in #175, they use snake_lower_case for enum values and because JsonConverter will instantiate the JsonStringEnumConverter with default options the deserialization of any type with enum members will fail.

Registering a JsonStringEnumConverter in json serialization settings does not help, because as JsonConverter says: "When placed on a property or field, the specified converter will always be used.".

When JsonConverter is placed on the type, if you've a compatible serializer in settings that will be used.

I see multiple solutions here, but one of them is simple:

  • Have an option to NOT to emit [JsonConverter(typeof(JsonStringEnumConverter))] for enum properties. With this settings will have to be configured but gives great flexibility. Cons: if no settings configured Json serialization will fail because there is nothing registered for enums.
  • Move the JsonConverter from properties to the enum types.

Now:

public enum Foo
{
    FooOne = 0,
    FooTwo = 1
}

public class FooClass
{
    [JsonConverter(typeof(JsonStringEnumConverter))]
    public Foo enumValue { get; set; }
}

With fix:

[JsonConverter(typeof(JsonStringEnumConverter))]
public enum Foo
{
    FooOne = 0,
    FooTwo = 1
}

public class FooClass
{
    public Foo enumValue { get; set; }
}

This way there is no requirement to have settings configured for enums, but gives the flexibility to have the behavior changed or register even typed enum converters there.

Support Key: [my-support-key]
N/A

OpenAPI Specifications
Same as in #175

Additional context
[EnumMember] is not used with System.Text.Json so their declaration can be omitted.

@attilah attilah added the bug Something isn't working label Oct 5, 2023
@christianhelle
Copy link
Owner

@attilah thanks for taking the time to report this

I looked a bit into this and there is very little I can do regarding changing how the contracts/dto types are generated. I piggy back entirely on a tool called NSwag for parsing the OpenAPI spec and for generating contracts.

Implementing customizations to the generated contracts/dto types would require building them by hand, and that's a bit of a big task, which I'm not ready to start on yet. I've done it previously in another project I'm involved in called the ATC REST Generator and it was rather hard to do and we kept seeing bugs.

NSwag seems to have nailed the contract/dto type generation to the point where everyone can almost be happy, but the feature set for generating contract/dto types hasn't really changed much over the years

I'll park this for now and revisit it in the future if I decide that Refitter should also handle contract/dto type generation.

May I suggest that you try other code generators and see if any of the others (e.g. OpenAPI Generator, NSwag, Kiota) can solve your problem? I have a Visual Studio extension that can help with trying out different code generators

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants