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

Make JsonStringEnumConverter support nullable enums #30947

Closed
ffMathy opened this issue Sep 25, 2019 · 10 comments
Closed

Make JsonStringEnumConverter support nullable enums #30947

ffMathy opened this issue Sep 25, 2019 · 10 comments
Assignees
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@ffMathy
Copy link
Contributor

ffMathy commented Sep 25, 2019

Right now, the JsonStringEnumConverter converter does not support nullable enums.

I think it should.

@ahsonkhan
Copy link
Member

cc @JeremyKuhne

@ffMathy
Copy link
Contributor Author

ffMathy commented Sep 26, 2019

5.0?

Is there any chance we can make it earlier, if I contribute with a PR? 👼

@ahsonkhan
Copy link
Member

ahsonkhan commented Sep 26, 2019

Is there any chance we can make it earlier, if I contribute with a PR?

It would be great if you'd like to submit a fix for this against master and we can evaluate whether it makes sense to port it to 3.1 based on risk/complexity and consumer need. Let me know if you need any help with that and definitely ping me on your PR for review whenever you get the chance to work on the fix :)

Can you explain your scenario/use case which is being blocked by the lack of this feature? I also want to try and see if there is a feasible workaround for nullable enums (either via a custom converters or creating your own JsonStringEnumConverter wrapper).

@ffMathy
Copy link
Contributor Author

ffMathy commented Sep 26, 2019

Thanks for the offer - in that case, perhaps you can help with dotnet/corefx#41344?

@Ibro
Copy link

Ibro commented Oct 16, 2019

We are also blocked from using System.Text.JSON in our API. We have issues with Swagger since JsonStringEnumConverter doesn't support nullable enums.

@Davilink
Copy link

Davilink commented Dec 4, 2019

I have the same problem, had to use Newtonsoft because of that.

@Svek
Copy link

Svek commented Dec 7, 2019

While this isn't the most elegant solution, it does serve as a workaround until this gets sorted.

public class StringNullableEnumConverter<T> : JsonConverter<T>
{
	private readonly Type _underlyingType;

	public StringNullableEnumConverter()
	{
		_underlyingType = Nullable.GetUnderlyingType(typeof(T));
	}

	public override bool CanConvert(Type typeToConvert)
	{
		return typeof(T).IsAssignableFrom(typeToConvert);
		//return true;
	}

	public override T Read(ref Utf8JsonReader reader,
		Type typeToConvert, 
		JsonSerializerOptions options)
	{
		string value = reader.GetString();
		if (String.IsNullOrEmpty(value)) return default;

		try
		{
			return (T)Enum.Parse(_underlyingType, value);
		}
		catch (ArgumentException ex)
		{
			throw new JsonException("Invalid value.", ex);
		}
	}

	public override void Write(Utf8JsonWriter writer, 
		T value, 
		JsonSerializerOptions options)
	{
		writer.WriteStringValue(value?.ToString());
	}
}

and you can attach it to your property by decorating it with this

[JsonConverter(typeof(StringNullableEnumConverter<Nullable<MyEnum>>))]
public MyEnum? MyNullableEnum { get; set; }

@Svek
Copy link

Svek commented Dec 9, 2019

To slightly improve the performance of the method above, we can remove the try/catch block with

// for performance, parse with ignoreCase:false first.
if (!Enum.TryParse(_underlyingType, value, ignoreCase: false, out object result) &&
    !Enum.TryParse(_underlyingType, value, ignoreCase: true, out result))
{
    throw new JsonException($"Unable to convert \"{value}\" to Enum \"{_underlyingType}\".");
}
return (T)result;

the try and catch is a bit expensive for something that can be solved with TryParse

@CodeBlanch
Copy link
Contributor

For anyone blocked by this, here's a NuGet package with a converter (JsonStringEnumMemberConverter) we can use ahead of the 5.0 drop: Macross.Json.Extensions

Supports nullable enum types and adds EnumMemberAttribute support.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@layomia layomia self-assigned this Apr 7, 2020
@layomia layomia added this to Backlog in System.Text.Json - 6.0 via automation Apr 7, 2020
@layomia
Copy link
Contributor

layomia commented Apr 9, 2020

This is fixed for 5.0 following #32006.

@layomia layomia closed this as completed Apr 9, 2020
System.Text.Json - 6.0 automation moved this from Backlog to Done Apr 9, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
No open projects
Development

No branches or pull requests

8 participants