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

Add serialization support for System.Text.Json (Attempt #2) #966

Closed

Conversation

akaegi
Copy link

@akaegi akaegi commented Aug 30, 2021

Second attempt for System.Text.Json support.
Should close #901.

Tried to adress issues mentioned by @chucker in #905 (thanks!).
The problem lies in non-polymorphic deserialization IMO. The original deserialization failed when the constructor had an IQuantity subtype as argument (e.g. Information) but the converter converted to IQuantity. I addressed that by returning specific converters in the factory as described in https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-converters-how-to?pivots=dotnet-5-0#support-round-trip-for-stackt.

Further notes:

  • Carefully tried to extract common serialization/deserialization of JsonNet and System.Text.Json in a common project "UnitsNet.Serialization"
  • UnitsNetIComparableJsonConverter is still missing -> time is running out...

Hope it works and helps :-)

@chucker
Copy link

chucker commented Sep 6, 2021

(Haven't tried this yet, but thanks for giving it a shot!)

@angularsen
Copy link
Owner

FYI, #972 was merged.

Also, there was some discussion that reveals our current JSON schema is maybe not optimal.
ValueType and ValueString properties are likely not necessary in order to preserve decimal precision:
#972 (comment)

@dmytro-gokun
Copy link

@angularsen Is there any ETA on this? Maybe some help is required? We miss this feature a lot, it's the only thing that holds us back from migrating a big project to System.Text.Json :)

@angularsen
Copy link
Owner

angularsen commented Dec 1, 2021

@dmytro-gokun
ETA is whenever someone implements it 😄

If you want to help out, then great! I am not actively working on it myself, but @lipchev has done quite a bit of work on json lately.

Some relevant background:

#972
#985
#990

In particular, we see that maybe the current JSON schema can be simplified a bit, removing fields ValueString and ValueType that were originally added to support decimal.

@lipchev Do you plan to attack system.text.json anytime soon?

@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 2, 2022
@stale stale bot closed this Apr 16, 2022
@maxreb
Copy link

maxreb commented Mar 20, 2023

I can see that its quite hard to find a efficient way to make Units.NET work with STJ, but wouldn't it for now be an option to just use the ToString and Parse methods so we have something which works with STJ? For every IQuantity we would need something like this:

public class LevelConverter : JsonConverter<Length>
{
	public override Length Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) 
		=> Length.Parse(reader.GetString());
	public override void Write(Utf8JsonWriter writer, Length value, JsonSerializerOptions options) 
		=> writer.WriteStringValue(value.ToString());
}

and then above every IQuantity struct this line:

[JsonConverter(typeof(JsonConverter<Length>))]

As is said, I get it that the Parse method is not that efficient and on the long run we may should find another way, but this will at least make Units.NET work with STJ.

I would implement this, if you agree.

@maxreb
Copy link

maxreb commented Mar 20, 2023

Okay I found another solution how this will work with STJ. Install NuGet package ZCS.DataContractResolver and then replace the default TypeInfoResolver with the DataContractResolver. JST is now using the DataContracts:

//LevelSensorData is a class which implements Units.NET quantities
var opt = new JsonSerializerOptions()
{
	TypeInfoResolver = System.Text.Json.Serialization.Metadata.DataContractResolver.Default,
	//This line is optional, will convert the enums to string (so it will show "Millimeter" instead of "26")
	Converters = { new JsonStringEnumConverter() }
};
var str = JsonSerializer.Serialize(originalObject, opt);
var obj = JsonSerializer.Deserialize<LevelSensorData>(str, opt);

@maxreb maxreb mentioned this pull request Mar 21, 2023
13 tasks
@angularsen
Copy link
Owner

angularsen commented Mar 26, 2023

Thanks for sharing @maxreb . Yes, DataContract should be one way to achieve this.

Another option is to implement custom converters, as this PR originally tried to. I believe that gives more control on the serialized format, but maybe data contract approach works well enough already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add serialization support for System.Text.Json
5 participants