JSON serialization/deserialization

bretthysuik commented 9 years ago

The Problem

Currently, every type in this library will be deserialized to 0.0 which is default(double) since double is used as the backing type for every unit.

Since every type in this library is a custom value type (struct) Json.NET (the de facto JSON library for .NET) needs custom converters in the deserialization process.

The Cause

I'm pretty sure it's related to all the unit types having read-only properties calculated from a base unit with a single constructor for that base unit. Json.Net probably doesn't know how to reconstruct the type using the base unit.

To Reproduce

var jsonSerializerSettings = new JsonSerializerSettings()
    {
        TypeNameHandling = TypeNameHandling.Objects,
        Formatting = Formatting.Indented,
        ObjectCreationHandling = ObjectCreationHandling.Replace
    };

var freq = Frequency.FromMegahertz(55);
string serializedFreq = JsonConvert.SerializeObject(freq, jsonSerializerSettings);
Frequency deserializedFreq = JsonConvert.DeserializeObject<Frequency>(serializedFreq, jsonSerializerSettings);

Console.WriteLine(deserializedFreq.Megahertz); // 0.0

Possible Solution

I think the only solution is to write custom JsonConverters for each unit type. Hopefully this can be auto-generated using the code generator scripts (I haven't looked into this).

I think it's probably unecessary to serialize every unit for each type: only the base unit should be serialized. For example, this is what a serialized Mass value looks like currently and it's a bit of a mess:

{
  "$type": "UnitsNet.Mass, UnitsNet",
  "StonePounds": {
    "$type": "UnitsNet.StonePounds, UnitsNet",
    "Stone": 0.0,
    "Pounds": 3.3069339327731635
  },
  "Centigrams": 150000.0,
  "Decagrams": 150.0,
  "Decigrams": 15000.0,
  "Grams": 1500.0,
  "Hectograms": 15.0,
  "Kilograms": 1.5,
  "Kilotonnes": 1.5E-06,
  "LongTons": 0.001476309791416591,
  "Megatonnes": 1.5E-09,
  "Micrograms": 1500000000.0,
  "Milligrams": 1500000.0,
  "Nanograms": 1500000000000.0,
  "Ounces": 52.910942850000005,
  "Pounds": 3.3069339327731635,
  "ShortTons": 0.0016534669663865818,
  "Stone": 0.23620975930540469,
  "Tonnes": 0.0015
}

bretthysuik commented 9 years ago

Relevant topic on SO: http://stackoverflow.com/questions/24726273/why-can-i-not-deserialize-this-custom-struct-using-json-net

bretthysuik commented 9 years ago

@anjdreas if you agree, I think we'll need a UnitsNet.Serialization.JsonNet project added (naming scheme lifted from the NodaTime project). It would be distributed as a separate NuGet package to keep the Json.Net dependency out of the main package.

jcoder76 commented 9 years ago

I think this makes good sense to me.

bretthysuik commented 9 years ago

I guess I should have read the docs first: https://github.com/anjdreas/UnitsNet#serialization

I still think it would be nice to create a package which provides all the Json.Net converters instead of forcing consumers to implement their own.

angularsen commented 9 years ago

Please see #92 and #95. They were recently created. I propose we move the discussion to #92.

angularsen commented 9 years ago

Oh wait, #92 was closed. Let's stick to this one then, the starting post has a nice summary of the problem too.

angularsen commented 9 years ago

A couple of things discussed in #92:

We could implement DataContract for XML, but does not help us with JSON
A custom JsonConverter can be written per type, possibly auto-generated with scripts (see examples), but this needs to be in a separate nuget to avoid dependency on JSON.net in core lib.
We might be able to do with just one custom JsonConverter using reflection to construct unit values and verify unit type by type/namespace checking. Possibly adding an empty interface UnitsNet.IUnit for all units as a marker (also mentioned in #82), if that helps.

Copied from #92:

From the start I did not want to take on serialization. There are so many pitfalls and easy to make breaking changes, so I felt safer leaving this up to client code. I'm open to discuss a solution here though, as I'm sure many will stumble across the same problem.

The most robust method would be to serialize both value and unit, so that in case the default/base unit changes at any point, we have a better chance of still parsing it. Unit abbreviations like "m" and "kg" are pretty fixed, but can be ambiguous. For instance, several Ratio units have no abbreviation. Unit enum names like LengthUnit.Meter are unique, but it is hard to guarantee they will never be renamed. So I'm not sure what would be a good choice for serializing the unit.

Less robust would be to just serialize the base unit value, but also simpler. I am currently leaning towards this option.

bretthysuik commented 9 years ago

I think we should agree on what the JSON should look like and then work backwards from there. Here's a couple of options using the following test object as the input to the serializer:

public class CustomObject
{
    public Frequency Frequency { get; set; }

    public CustomObject() { Frequency = Frequency.FromMegahertz(55); }
}

Option 1 (current solution, no unit)

{
  "Frequency": 55.0
}

Option 2 (unit abbreviation string included)

{
  "Frequency": "55000000 Hz"
}

Option 3 (custom JSON object, unit enum included)

{
  "Frequency": {
    "Value": 55000000.0,
    "Unit": "Hertz"
  }
}

I don't think option 1 carries enough information and option 2 introduces culture parsing issues (due to the inclusion of the abbreviation).

My vote is option 3 because it's the most human-readable (JSON is supposed to be a human-readable format) and contains the most useful, basic information (base unit value and base unit name). It also works for units with no abbreviation like Ratio:

{
  "Ratio": {
    "Value": 22.0,
    "Unit": "DecimalFraction"
  }
}

Unit enum names like LengthUnit.Meter are unique, but it is hard to guarantee they will never be renamed. So I'm not sure what would be a good choice for serializing the unit.

I don't think this is a problem because the likelihood of the enum values changing is small (if this happens it means the unit was designed incorrectly which shouldn't happen often, if at all). However, if the enum value DOES change, then it would be a breaking API change in the main project and a new version of the NuGet package would have to be released with a major version bump (according to semver rules). It's then up to consumers to decide whether they want to do a major version upgrade.

The downside to option 3 is that there's no way to use a single JsonConverter for all units (reflection won't work) unless we add an interface like below in order to pull the base unit value and enum for any type in the library:

public interface IUnitOfMeasure
{
    double BaseUnitValue { get; }
    Enum BaseUnit { get; }
}

I don't think the API in the interface is particularly useful in most situations so it could get explicitly implemented to reduce API noise.

The alternative is a code-generated converter for each unit.

angularsen commented 9 years ago

Option1: Base unit value should be used here, but we still don't know we're dealing with frequency other than the property name, so too little info

Option2: Agree, also as pointed out earlier, some units don't have abbreviations

Option3: Agree, but I think we need to do it like this?

{
  "Frequency": {
    "Value": 55000000.0,
    "Unit": "FrequencyUnit.Hertz"
  }
}

There is no guarantee that two unit classes don't overlap in unit names, but we can guarantee that they won't overlap on enum type.

This way I assume(?) we can use reflection to construct FrequencyUnit.Hertz enum value from the string "FrequencyUnit.Hertz", and use reflection to construct Frequency.From(value, Frequency.Hertz). I haven't tried something like this before, but it seems feasible since all units follow the same pattern of method names. One concern is that most units use double as backing type, but some use long or decimal. Just something to be aware of.

Update: Fixed code example for .From().

bretthysuik commented 9 years ago

Ah, I see what you mean using reflection now. That would definitely be a simpler solution and I'll look into this.

bretthysuik commented 9 years ago

Reading the JSON using reflection is possible (see here) but it's a bit ugly. I'm pretty sure the JSON can't be generated without some sort of interface such as IUnitOfMeasure mentioned previously.

It would be easier to just use the converter base class from #92, tweak the generated JSON to include the unit, and use scripts to auto-generate all the converter classes.

bretthysuik commented 9 years ago

See #100 for a work-in-progress implementation

angularsen commented 9 years ago

Solved by #100 and #101. Nugets are now out: UnitsNet.3.17.0 and UnitsNet.Serialization.JsonNet.1.0.0-alpha2.

angularsen / UnitsNet