Add [DataContract] and [DataMember] annotations

[lipchev]

• added to all quantities and their respective fields (Unit, Value)
• added a set of serialization tests (common to all serializers)
• added tests for the DataContractSerializer (xml) and DataContractJsonSerializer (default settings)

Related to #907

[angularsen]

Will try to get to this soon.

[angularsen]

Some initial questions

[angularsen]

I keep forgetting, but wasn't there some good reason to use quoted numbers when serializing?

At first I thought json could not represent double.MaxValue or decimal.MaxValue, but it seems just fine representing both of these in a round-trip.

I tried a quick example in linqpad:

void Main()
{
	//decimal smallestDecimal = (decimal) (1 / Math.Pow(10, 28));
	//double smallestDouble = double.Epsilon;
	//var foo = new Foo(smallestDouble, smallestDecimal);

	var foo = new Foo(double.MaxValue, decimal.MaxValue);
	var systemTextJson = System.Text.Json.JsonSerializer.Serialize(foo).Dump("system.text.json");
	var newtonsoftJson = JsonConvert.SerializeObject(foo).Dump("newtonsoft");
	
	var systemTextFoo =  System.Text.Json.JsonSerializer.Deserialize<Foo>(systemTextJson).Dump("systemTextDeser");
	var newtonsoftFoo = JsonConvert.DeserializeObject<Foo>(newtonsoftJson).Dump("newtonsoftDeser");
	systemTextFoo.Equals(foo).Dump("systemTextDeser.Equals(original)");
	newtonsoftFoo.Equals(foo).Dump("newtonsoftDeser.Equals(original)");	
}

public record Foo(double MyDouble, decimal MyDecimal);

Result:

Similarly, if I instead of MaxValue chose the smallest number of each, it still roundtrips just fine for both Newtonsoft and System.Text.Json.

	decimal smallestDecimal = (decimal) (1 / Math.Pow(10, 28));
	double smallestDouble = double.Epsilon;
	var foo = new Foo(smallestDouble, smallestDecimal);

So. I can't really remember why quoted strings were used to begin with? Is it maybe not necessary after all? Do you remember?

This is the one that introduced it, but I don't really see a description of the problem it solved:
#868

Serialize decimal values as string to keep number of decimal places

[lipchev]

I think this relates to the scale being lost while de-serializing using Newtonsoft - you could see it in the debugger (or in you example of ****0335.0 - that's a double right there). At the same time - this doesn't happen during serialization - the exact scale is preserved.

After doing the initial round of tests- I actually stumbled upon an option that might be able to resolve this issue- but haven't had time to actually figure out what it does: FloatParseHandling.

[lipchev]

There is also an option for the special floating point numbers, e.g. NaN, PositiveInfinity and NegativeInfinity - however given that none of those are really supported by UnitsNet (the Guard aggressively protecting against it) I don't think this would have been a reason to use the string representation for all numbers.

[angularsen]

I'm just trying to understand better, so here is another example, now using our json.net serializer and schema:

void Main()
{
	decimal smallestDecimal = (decimal) (1 / Math.Pow(10, 28));
	
	var foo = new Foo(
		LargestDecimal:  new BitRate(decimal.MaxValue, BitRateUnit.BitPerSecond),
		SmallestDecimal: new BitRate(smallestDecimal, BitRateUnit.BitPerSecond),		
		LargestDouble:   new Length(double.MaxValue, LengthUnit.Meter),
		SmallestDouble:  new Length(double.Epsilon, LengthUnit.Meter)
	).Dump("foo");

	var newtonJson = JsonConvert.SerializeObject(foo, Newtonsoft.Json.Formatting.Indented, new UnitsNetIQuantityJsonConverter()).Dump("newtonJson");
	//System.Text.Json.JsonSerializer.Serialize(foo).Dump("systemJson"); // throws cyclical exception
	
	var newtonFoo = JsonConvert.DeserializeObject<Foo>(newtonJson, new UnitsNetIQuantityJsonConverter()).Dump("newtonFoo");
	newtonFoo.Equals(foo).Dump("newtonFoo.Equals(foo)");
}

public record Foo(IQuantity LargestDecimal, IQuantity SmallestDecimal, IQuantity LargestDouble, IQuantity SmallestDouble);

{
  "LargestDecimal": {
    "Unit": "BitRateUnit.BitPerSecond",
    "Value": 7.922816251426434E+28,
    "ValueString": "79228162514264337593543950335",
    "ValueType": "decimal"
  },
  "SmallestDecimal": {
    "Unit": "BitRateUnit.BitPerSecond",
    "Value": 1.0000000000000001E-28,
    "ValueString": "0.0000000000000000000000000001",
    "ValueType": "decimal"
  },
  "LargestDouble": {
    "Unit": "LengthUnit.Meter",
    "Value": 1.7976931348623157E+308
  },
  "SmallestDouble": {
    "Unit": "LengthUnit.Meter",
    "Value": 5E-324
  }
}

You wrote:

I think this relates to the scale being lost while de-serializing using Newtonsoft [...] 0335.0 - that's a double right there

I think you are right, but only if we were actually trying to deserialize back into the wrong number type:

• double.MaxValue serialized to JSON and deserialized to a decimal property would probably result in OverflowException
• decimal.MaxValue serialized to JSON and deserialized to a double property, would lose precision.

However, this shouldn't be a problem when serializing decimal and double quantities if our converters do their job.

What I am beginning to realize is that the problem is not JSON itself, nor is it SystemTextJson or NewtonsoftJson. It is simply that when deserializing, we need to know whether to deserialize to double or to decimal. And we do have this information encoded in our JSON schema. So that leads me to think that the ValueString property in our schema is really redundant. ValueType should be sufficient, shouldn't it?

If I am not entirely missing something, then bringing this line of thought back to this PR; should the data contract have a ValueType property to support deserializing decimal values without losing precision?
And, should we deprecate the ValueString property in our json.net schema and bump the major version with a breaking change to remove that redundant property?

[lipchev]

In my opinion- the ValueType is redundant - it can easily be determined from the Unit (I'm checking the corresponding QuantityInfo.Zero).

Unfortunately, in the case of Newtonsoft- knowing the target doesn't help. Here's a fiddle for you to consider:
JToken.Load(new JsonTextReader(new StringReader("{\"DecimalValue\":1.20}"))).Value<decimal>("DecimalValue").Dump()

Dumping object(Decimal)
1.2

[lipchev]

All our converters use JToken.Load, but that might be because everybody was copying over from the previous implementation..
The documentation isn't much help either- the first example uses JToken for the WriteJson (without implementing a corresponding ReadJson !?).
The second one is just as useless (I'm not even sure what it is supposed to demonstrate) .

After browsing through the source code - it seems like the implementation of DeserializeObject avoids calling JToken/JObject.Load: it's going through a while(reader.Read()) loop, parsing each token it encounters (e.g. TokenType.StartObject -> TokenType.PropertyName ... -> TokenType.EndObject).

Following that logic, I managed to deserialize my DecimalValue- it isn't pretty, but I think we can manage..

[angularsen]

Ok, so to recap.

• It is possible do deprecate ValueType and ValueString in our existing json.net schema, the converters should be able to do without since we can look up the number type from QuantityInfo.
• This PR's data contract is already good, but, more changes are needed to support full precision of decimal?

[lipchev]

This PR doesn't affect the existing code in any way... It's just about the DataContract and DataMember annotations- and the tests for the corresponding DataContract serializers (xml and json - but without any of the surrogate definitions)..
I sensed you were still in the spirit of our discussion in #907 but the point of starting over was to take it one step at a time. I was of course, going to propose an implementation for a JsonConverter but in another PR.
Other than that - yes you are correct, I think we can manage without the extra properties - and yes, my current implementation (not part of this PR) only supported decimals using the "quoted" numbers schema- and needs to be re-written (once again).

[lipchev]

You should however, take a close look at the tests- as they would probably be shared between all serializers (DC, Newtonsoft, System.Text and whatever else comes after..).

[angularsen]

Sorry if I am being slow to grasp all the things, I keep forgetting details as time passes between each time I visit this topic. I think I'm once again getting the mental map decent and this PR looks ready to merge to me.

Just for my own summary:

• This PR adds the contracts to all quantities
• Contracts are supported by built-in XML/JSON serializers, newtonsoft, protobuf and a bunch of others - but currently not system.text.json
• This PR adds tests for built-in XML/JSON serializers, the same tests can be reused as you mention
• The built-in XML/JSON serializers preserve decimal scale out of the box
• Surrogates can be built to enhance/change the built-in XML/JSON serializers
• System.Text.Json custom converter can be provided separately WIP: Add serialization support for System.Text.Json #905 Add serialization support for System.Text.Json (Attempt #2) #966
• Newtonsoft JSON converter works, but its schema has redundant properties

The tests are excellent, really thorough 👏

[angularsen]

Release UnitsNet/4.103.0 · angularsen/UnitsNet