Description
Describe the bug
This looks like a duplicate of this issue but I was lead there by my attempt to remove the IConvertible
interface...
In essence, none of the UnitsNetBaseJsonConverter
we have are ever getting called for a property such as public HowMuch? NullableQuantity { get; set; }
... because it doesn't implement the IConvertible
interface 😆 ..
To Reproduce
I took inspiration from this test, and tried to replicate it in the HowMuchTests
Here's the new HowMuchTets.cs
:
public class HowMuchTests
{
[Fact]
public static void SerializeAndDeserializeCreatesSameObjectForIQuantity()
{
var jsonSerializerSettings = new JsonSerializerSettings { Formatting = Formatting.Indented };
var quantityConverter = new UnitsNetIQuantityJsonConverter();
quantityConverter.RegisterCustomType(typeof(HowMuch), typeof(HowMuchUnit));
jsonSerializerSettings.Converters.Add(quantityConverter);
var quantity = new HowMuch(12.34, HowMuchUnit.ATon);
var serializedQuantity = JsonConvert.SerializeObject(quantity, jsonSerializerSettings);
var deserializedQuantity = JsonConvert.DeserializeObject<HowMuch>(serializedQuantity, jsonSerializerSettings);
Assert.Equal(quantity, deserializedQuantity);
}
[Fact]
public static void SerializeObjectWithNullQuantity()
{
var jsonSerializerSettings = new JsonSerializerSettings { Formatting = Formatting.Indented };
var quantityConverter = new UnitsNetIQuantityJsonConverter();
quantityConverter.RegisterCustomType(typeof(HowMuch), typeof(HowMuchUnit));
jsonSerializerSettings.Converters.Add(quantityConverter);
var quantity = new HowMuch(12.34, HowMuchUnit.ATon);
var objectToSerialize = new TestObject { NonNullableQuantity = quantity};
var serializedQuantity = JsonConvert.SerializeObject(objectToSerialize, jsonSerializerSettings);
TestObject deserializedQuantity = JsonConvert.DeserializeObject<TestObject>(serializedQuantity, jsonSerializerSettings);
Assert.Equal(quantity, deserializedQuantity.NonNullableQuantity);
Assert.Null(deserializedQuantity.NullableQuantity);
}
[Fact]
public static void SerializeObjectWithNullableQuantity()
{
var jsonSerializerSettings = new JsonSerializerSettings { Formatting = Formatting.Indented};
var quantityConverter = new UnitsNetIQuantityJsonConverter();
quantityConverter.RegisterCustomType(typeof(HowMuch), typeof(HowMuchUnit));
jsonSerializerSettings.Converters.Add(quantityConverter);
var quantity = new HowMuch(12.34, HowMuchUnit.ATon);
var objectToSerialize = new TestObject { NonNullableQuantity = quantity, NullableQuantity = quantity };
var serializedQuantity = JsonConvert.SerializeObject(objectToSerialize, jsonSerializerSettings);
TestObject deserializedQuantity = JsonConvert.DeserializeObject<TestObject>(serializedQuantity, jsonSerializerSettings); // exception here
Assert.Equal(quantity, deserializedQuantity.NonNullableQuantity);
Assert.Equal(quantity, deserializedQuantity.NullableQuantity);
}
}
public sealed class TestObject
{
public HowMuch? NullableQuantity { get; set; }
public HowMuch NonNullableQuantity { get; set; }
}
SerializeObjectWithNullableQuantity
fails. with
Newtonsoft.Json.JsonSerializationException
Unable to find a constructor to use for type UnitsNet.UnitInfo. A class should either have a default constructor, one constructor with arguments or a constructor marked with the JsonConstructor attribute. Path 'NullableQuantity.QuantityInfo.UnitInfos[0].Value', line 18, position 18.
This is because the nullable property is not being matched against the converter, so the serializedQuantity
string contains all public properties and their descendants (not crashing yet), and on the way back- it once again ignores the converter, and attempts to reconstruct everything, and fails (because of the constructors).
And what about the IConvertible
- the exact same test passes when we use the internal quantities:
public sealed class TestObject
{
public Frequency? NullableFrequency { get; set; }
public Frequency NonNullableFrequency { get; set; }
}
... frankly I'm not sure, but what I know is that as soon as as I removed the IConvertible
from the Frequence
(and all other quantities), the nullable-struct-property tests started failing in the same fashion. Meanwhile, I know that none of the IConvertible
interface members are being called in the original tests- yet there must be something about the interface that is obviously being checked for somewhere..
Puzzled by it, I looked around and found a few open issues that seem to be related...
The suggested workarounds mentioned using the JsonConverter
instead of the JsonConverter<T>
and overriding the CanConvert
such that it handles the Nullable<T>
case as well..
So I did (comments omitted for brevity)..
public abstract class NullableQuantityConverter<T> : JsonConverter
{
public override bool CanConvert(Type objectType)
{
// Check if the type directly implements TQuantity
if (typeof(T).IsAssignableFrom(objectType))
{
return true;
}
// Check if the type is a Nullable<T> where T implements IQuantity
Type? underlyingType = Nullable.GetUnderlyingType(objectType);
return underlyingType != null && typeof(T).IsAssignableFrom(underlyingType);
}
public sealed override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
{
switch (value)
{
case T quantity:
WriteJson(writer, quantity, serializer);
break;
case null:
WriteJson(writer, default, serializer);
break;
default:
throw new JsonSerializationException("Converter cannot write specified value to JSON. An IQuantity is required.");
}
}
public abstract void WriteJson(JsonWriter writer, T? quantity, JsonSerializer serializer);
public sealed override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer)
{
return existingValue switch
{
T existingQuantity => ReadJson(reader, objectType, existingQuantity, true, serializer),
null => ReadJson(reader, objectType, default, false, serializer),
_ => throw new JsonSerializationException("Converter cannot read JSON with the specified existing value. An IQuantity is required.")
};
}
public abstract T? ReadJson(JsonReader reader, Type objectType, T? existingValue, bool hasExistingValue, JsonSerializer serializer);
}
After which I simply switched the UnitsNetBaseJsonConverter
s base class from JsonConverter<T>
to the NullableQuantityConverter<T>
and all tests are now green (including in the version where Frequence
is not IConvertible
)...
As it stands, the current issue only affects the nullable external quantities, however if we decide to move away from the IConvertible
interface, this issue will really become relevant.