UnitsNetIQuantityJsonConverter does not validate the existence of the unit

[lipchev]

As I was working on re-implementing the existing JsonNet converters for System.Text.Json I realized that there is no validation happening anywhere inside the UnitsNetIQuantityJsonConverter regarding the existence of the Unit. Enum.Parse(..) is perfectly fine with accepting a number, representing a potentially invalid value (w.r.t. to what we currently support in the switch statements).

        [Fact]
        public void UnitsNetIQuantityJsonConverter_AcceptsNumericInputs_WithoutValidation()
        {
            var json = "{ \"unit\": \"PowerUnit.-1\", \"Value\": 10.3654}";

            IQuantity result;

            using(var stringReader = new StringReader(json))
            using (var jsonReader = new JsonTextReader(stringReader))
            {
                result = _sut.ReadJson(jsonReader, typeof(IQuantity), null, false, JsonSerializer.CreateDefault());
            }

            Assert.NotNull(result);
            Assert.IsType<Power>(result);
            Assert.Equal(10.3654, ((Power)result).Value);
            Assert.Equal((PowerUnit)(-1), ((Power)result).Unit);
        }

So there are several questions here (we don't have to do these for JsonNet, but we have to make our mind about it for System.Text) :

1. Do we want to validate the existence of the unit?
2. Do we want to accept numeric inputs after the . and do we want to include an option to toggle this (there is one for the StringEnumConverter)?
3. If the answer to 2) is "No", then what do you think about replacing the current format {QuantityInfo.UnitType.Name}.{Quantity.Unit} with {QuantityInfo.UnitType.Name}.{UnitInfo.Name}. These two currently represent the same thing, only the second one would be much faster to serialize (as we don't re-construct the enum-string every time), however this would be a breaking change for anyone who may be using custom quantities where the UnitInfo.Name is not the same as the UnitInfo.Value.Name (my guess is that's a very small number of people, if any..)
4. If the answer to 3_) is Ok or Ok, let's make two versions then we can tag another option to consider: do we want to support case-insensitive matching (this could work with both the unit type and name: "powerunit.watt").

I currently have three types extending JsonConverter<TUnit>, tell me if you'd rather have another configuration:

1. UnitTypeAndEnumNameConverter<TUnit> : this one I've currently annotated as [Obsolete("Replaced by the UnitTypeAndNameConverter")]
2. UnitTypeAndNameConverter<TUnit> : this is the proposed option 3)
3. AbbreviatedUnitConverter<TUnit> : nothing much to talk about here..

Also, what do you think about the project structure for these: so far I just have one project named UnitsNet.Serialization.SystemTextJson but if you want, we could have each schema in a separate project, but that would like require an additional common project for the shared bits.

[angularsen]

Do we want to validate the existence of the unit?

Yes I think we should throw an explanatory exception for this case.

Do we want to accept numeric inputs after the .

No, that seems weird to me. Let's only accept string.

If the answer to 2) is "No", then what do you think about replacing the current format

I don't see why anyone would make the name different, so this optimization is probably OK. We could document that the serialization based on UnitInfo.Name in either wiki or some relevant piece of code, or both.

case-insensitive matching

Yes, I don't see why not.

so far I just have one project named UnitsNet.Serialization.SystemTextJson but if you want, we could have each schema in a separate project

One project is plenty I think.