ActiveLogin / ActiveLogin.Identity

Parsing and validation of Swedish identities such as Personal Identity Number (svenskt personnummer) in .NET.
https://activelogin.net
MIT License
56 stars 9 forks source link

Investigate serialization support #131

Open viktorvan opened 4 years ago

viktorvan commented 4 years ago

Is your feature request related to a problem? Please describe. There could potentially be an issue with serializing/deserializing the number types in this library.

Describe the solution you'd like We should investigate this and add unit tests for some common scenarios. Look at https://github.com/dotnet/runtime/blob/master/src/libraries/System.Private.CoreLib/src/System/DateTime.cs#L49 Maybe we should implement ISerializable.

Describe alternatives you've considered Using the 12-string representation for persistence will always work.

PeterOrneholm commented 4 years ago

Would be nice to see if this could be added to 3.0.0, in case it would require a breaking change.

viktorvan commented 4 years ago

Ok, this presents some problems.

First off we can implement ISerializable, which works fine for NewtonSoft.Json but is not used by System.Text.Json, and I cannot find any info if they ever plan to support it.

The other route is to write custom JsonConverters. We would have to write two of them, one for NewtonSoft.Json and one for System.Text.Json (and any other serialization library we would want to support).

The problem is that we would have to take dependencies on Newtonsoft.Json and System.Text.Json in that case, which I think we should avoid for ActiveLogin.Identity.Swedish.

So right now I see 3 options:

  1. Implement ISerializable and only support Newtonsoft.Json
  2. Add a new nuget package ActiveLogin.Identity.Swedish.Serialization (or even separate packages for NewtonSoft and System.Text.Json) and only depend on the third party libraries from there. But this would require the user to know that this library exists, and remember to reference it and setup their JsonSerializer to use our custom converters.
  3. We state (even more) clearly in the documentation that if the types should be serialized/persisted you should use the 12DigitString.

Right now it is not possible to deserialize our types, you would get an error like this:

System.NotSupportedException: Deserialization of reference types without parameterless constructor is not supported. Type 'ActiveLogin.Identity.Swedish.PersonalIdentityNumber'

So I think 3. (maybe together with 1. ?) is the best option to go with, thoughts @PeterOrneholm (and @nikolaykrondev, I know you have looked at System.Text.Json for ActiveLogin.Authentication)

P.S. If we look at DateTime as an example it implements ISerializable.

viktorvan commented 4 years ago

We will add ISerializable which will support Newtonsoft.Json. And we will also compile for netstandard2.1 and conditionally add a JsonConverter for System.Text.Json support since System.Text.Json is included in the framework from netstandard2.1. and no additional dependency is needed then.

For users of netstandard2.0 and System.Text.Json we update the readme with information how they can add the custom JsonConverter themselves.