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

Serialization of strongly typed id should just store the value directly #110

Closed
ShaharPrishMSFT opened this issue Aug 10, 2023 · 11 comments · Fixed by #117
Closed

Serialization of strongly typed id should just store the value directly #110

ShaharPrishMSFT opened this issue Aug 10, 2023 · 11 comments · Fixed by #117

Comments

@ShaharPrishMSFT
Copy link

Today, when you store a stronglytypedid, the result stores it as the object:

        {
          "OrgGroupId": {
            "Value": "MyId"
          },
}

Instead (at least optionally), it should store the value as the wrapped value:

{ "OrgGroupId": "MyId" }
@andrewlock
Copy link
Owner

The details are important here. How are you serialising it? There are a bunch of converters built-in so that it works as you describe, but you have to enable some of them (as some require external assembly references)

@Varveyn
Copy link

Varveyn commented Aug 11, 2023

I'm facing the same issue with System.Text.Json. I've came across this article that describes a workaround, but maybe there's a built-in solution?

@ShaharPrishMSFT
Copy link
Author

Hey Andrewlock

I use System.Json.Text to serialize, usign the default behavior. I am asking about having the default behavior serialize it like a string.

@andrewlock
Copy link
Owner

This is already supported, you can read how to add System.Text.Json converters in the readme😊

@ShaharPrishMSFT
Copy link
Author

We have a part in our system still using newtonsoft. So that was that problem. I am still unable to get this to work as serialized Appsettings in asp.net.

@andrewlock
Copy link
Owner

This is already supported, the Newtonsoft.Json converter is added by default 😊

@Ralf1108
Copy link

Ralf1108 commented Sep 21, 2023

As we had the same issue here is an implementation of a StronglyTypedIdConverterFactory to handle the correct serialization/deserialization for System.Text.Json.
Unitest is included at the end.

Maybe there is a chance to annotate StronglyTypedIds by default with such a StronglyTypedIdConverterFactory to support proper System.Text.Json serialization ?

Usage is:

using System.Text.Json.Serialization;

[StronglyTypedId]
[JsonConverter(typeof(StronglyTypedIdConverterFactory))]
public partial struct TestTypedId { }

implementation is

/// <summary>
/// Creates <see cref="StronglyTypedIdJsonConverter"/> via reflection for all possible StronglyTypedIds />
/// 
/// JsonConverterFactories: https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/converters-how-to?pivots=dotnet-7-0#sample-factory-pattern-converter
/// Expression trees: https://learn.microsoft.com/en-us/dotnet/csharp/advanced-topics/expression-trees/
/// </summary>
public class StronglyTypedIdConverterFactory : JsonConverterFactory
{
    public override bool CanConvert(Type typeToConvert)
    {
        return StronglyTypedIdHelper.IsStronglyTypedId(typeToConvert);
    }

    public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
    {
        var checkedType = typeToConvert.NotNull();

        var underlyingType = StronglyTypedIdHelper.GetUnderlyingType(checkedType);
        if (underlyingType == null)
            throw new InvalidOperationException(
                                                $"Can't find underlying value of StronglyTypedId {checkedType.Name}");

        return CreateConverterInternal(checkedType, underlyingType);
    }

    private static JsonConverter CreateConverterInternal(Type stronglyTypeId, Type underlyingType)
    {
        var createTypedId = CreateCreateTypedId(stronglyTypeId, underlyingType);
        var getUnderlyingValue = CreateGetUnderlyingValue(stronglyTypeId);

        var converterType = typeof(StronglyTypedIdJsonConverter<,>).MakeGenericType(stronglyTypeId, underlyingType);
        var converter = (JsonConverter?)Activator.CreateInstance(converterType, createTypedId, getUnderlyingValue);
        if (converter == null)
        {
            var message = $"Can't create JsonConverter for StronglyTypedId {stronglyTypeId.Name}";
            throw new InvalidOperationException(message);
        }

        return converter;
    }

    private static Delegate CreateCreateTypedId(Type stronglyTypeId, Type underlyingType)
    {
#pragma warning disable S125

        // Func<Guid, TestTypedId> _createTypedId = x => new TestTypedId(x);
        var ctorInfo = StronglyTypedIdHelper.GetConstructor(stronglyTypeId);
        if (ctorInfo == null)
            throw new InvalidOperationException($"Can't find Ctor in {stronglyTypeId.Name}");

        var value = Expression.Parameter(underlyingType, "x"); // (Guid x);
        var ctor = Expression.New(ctorInfo, value); // new TestTypedId(x);
        var expression = Expression.Lambda(ctor, value); // x => new TestTypedId(x);
        var ctorCall = expression.Compile();
        return ctorCall;
#pragma warning restore S125
    }

    private static Delegate CreateGetUnderlyingValue(Type stronglyTypeId)
    {
#pragma warning disable S125

        //Func<TestTypedId, Guid> _getValue = x => x.Value;
        var propertyInfo = stronglyTypeId.GetProperty("Value");
        if (propertyInfo == null)
            throw new InvalidOperationException($"Can't find Value property in {stronglyTypeId.Name}");

        var arg = Expression.Parameter(stronglyTypeId, "x"); // (Guid x);
        var propertyGetter = Expression.Property(arg, propertyInfo); // x.Value;
        var expression = Expression.Lambda(propertyGetter, arg); // x => x.Value;
        var getValue = expression.Compile();
        return getValue;
#pragma warning restore S125
    }
}

/// <summary>
/// Generic JsonConverter for all StronglyTypedIds
/// </summary>
/// <typeparam name="TStronglyTypedId"></typeparam>
/// <typeparam name="TUnderlyingType"></typeparam>
public class StronglyTypedIdJsonConverter<TStronglyTypedId, TUnderlyingType> : JsonConverter<TStronglyTypedId>
    where TStronglyTypedId : struct
{
    private readonly Func<TUnderlyingType, TStronglyTypedId> _createTypedId;
    private readonly Func<TStronglyTypedId, TUnderlyingType?> _getUnderlyingValue;

    public StronglyTypedIdJsonConverter(
        Func<TUnderlyingType, TStronglyTypedId> createTypedId,
        Func<TStronglyTypedId, TUnderlyingType?> getUnderlyingValue)
    {
        var type = typeof(TStronglyTypedId);
        if (!StronglyTypedIdHelper.IsStronglyTypedId(type))
            throw new InvalidOperationException($"Type {type.Name} is no valid StronglyTypedId");

        _createTypedId = createTypedId;
        _getUnderlyingValue = getUnderlyingValue;
    }

    public override TStronglyTypedId Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        if (reader.TokenType is JsonTokenType.Null)
            return default;

        var value = JsonSerializer.Deserialize<TUnderlyingType>(ref reader, options);
        if (value == null)
        {
            var message =
                $"Expected deserialized non null {typeof(TUnderlyingType).Name} for StronglyTypedId {typeToConvert.NotNull().Name}";
            throw new JsonException(message);
        }

        return _createTypedId(value);
    }

    public override void Write(Utf8JsonWriter writer, TStronglyTypedId value, JsonSerializerOptions options)
    {
        JsonSerializer.Serialize(writer, _getUnderlyingValue(value), options);
    }
}

public static class StronglyTypedIdHelper
{
    /// <summary>
    /// This method tries to get the underlying type of a StronglyTypedId by checking common indicators of generated struct
    /// </summary>
    /// <param name="type"></param>
    /// <returns></returns>
    public static Type? GetUnderlyingType(Type type)
    {
        var unwrappedType = type.UnwrapNullable();
        if (!unwrappedType.IsStruct())
            return null;

        var valuePropertyInfo = unwrappedType.GetProperty("Value");
        if (valuePropertyInfo == null)
            return null;

        var emptyProperty = unwrappedType.GetField("Empty",
                                                   BindingFlags.Public | BindingFlags.Static);
        if (emptyProperty == null)
            return null;
        if (!emptyProperty.IsInitOnly)
            return null;

        return valuePropertyInfo.PropertyType;
    }

    public static bool IsStronglyTypedId(Type type)
    {
        return GetUnderlyingType(type) != null;
    }

    public static bool HasUnderlyingType(Type type, Type expectedUnderlyingType)
    {
        var underlyingType = GetUnderlyingType(type);
        return underlyingType == expectedUnderlyingType;
    }

    public static ConstructorInfo? GetConstructor(Type type, bool throwIfNotFound = true)
    {
        var checkedType = type.NotNull();
        var underlyingType = GetUnderlyingType(checkedType);
        if (underlyingType == null)
            throw new InvalidOperationException("Invalid strongly type");

        var constructorInfo = checkedType.GetConstructor(
                                                         BindingFlags.Public | BindingFlags.Instance,
                                                         new[] { underlyingType });
        if (constructorInfo == null || !IsValidConstructor(constructorInfo, underlyingType))
        {
            if (!throwIfNotFound)
                return null;

            var message = $"Can't find matching ctor with 1 parameter in {checkedType.Name}";
            throw new InvalidOperationException(message);
        }

        return constructorInfo;
    }

    private static bool IsValidConstructor(ConstructorInfo constructorInfo, Type underlyingType)
    {
        var parameters = constructorInfo.NotNull().GetParameters();
        return parameters.Length == 1
               && parameters.Single().ParameterType == underlyingType;
    }
}

public static class TypeExtensions
{
    public static bool IsStruct(this Type type)
    {
        return type is { IsValueType: true, IsEnum: false };
    }

    public static Type UnwrapNullable(this Type type)
    {
        if (IsNullable(type))
            return Nullable.GetUnderlyingType(type) ?? type;

        return type;
    }

    public static bool IsNullable(this Type type)
    {
        return type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>);
    }
}

public static class GuardExtensions
{
    public static T NotNull<T>([NotNull] this T? value, [CallerArgumentExpression("value")] string name = "") where T : class
    {
        return value ?? throw new ArgumentNullException(name);
    }
}

unitest is

public class StronglyTypedIdSerializationTests
{
    [Fact]
    public void GivenTypedId_WhenSerializedWithGenericJsonConverter_ThenSerializationShouldBeCorrect()
    {
        Func<Guid, TestTypedId> createTypedId = x => new TestTypedId(x);
        Func<TestTypedId, Guid> getValue = x => x.Value;
        var converter = new StronglyTypedIdJsonConverter<TestTypedId, Guid>(createTypedId, getValue);
        RunTest(converter);
    }

    [Fact]
    public void GivenTypedId_WhenSerializedWithReflectionJsonConverter_ThenSerializationShouldBeCorrect()
    {
        // Arrange
        var converterType = typeof(StronglyTypedIdJsonConverter<,>).MakeGenericType(typeof(TestTypedId), typeof(Guid));
        Func<Guid, TestTypedId> createTypedId = x => new TestTypedId(x);
        Func<TestTypedId, Guid> getValue = x => x.Value;
        var converter = (JsonConverter?)Activator.CreateInstance(converterType, createTypedId, getValue);

        // Act + Assert
        RunTest(converter);
    }

    [Fact]
    public void GivenTypedId_WhenSerializedWithJsonConverterFactory_ThenSerializationShouldBeCorrect()
    {
        // Arrange
        var factory = new StronglyTypedIdConverterFactory();

        // Act + Assert
        RunTest(factory);
    }

    private static void RunTest(JsonConverter? jsonConverter)
    {
        if (jsonConverter == null)
            throw new InvalidOperationException("JsonConverter has to be not null");

        // Arrange
        var guid = Guid.Parse("da7850db-02e0-4699-a92f-676d7ae5d5e7");
        var testObject = new TestClass { TypedId = new TestTypedId(guid) };

        // Act
        var options = new JsonSerializerOptions();
        options.Converters.Add(jsonConverter);
        var json = JsonSerializer.Serialize(testObject, options);

        // Assert
        // instead of: {"TypedId":{"Value":"f039d86f-e6f6-46f1-a9e8-a0f16c43fbc8"}}
        json.Should().Be("{\"TypedId\":\"da7850db-02e0-4699-a92f-676d7ae5d5e7\"}");

        // Act
        var deserialized = JsonSerializer.Deserialize<TestClass>(json, options);

        // Assert
        deserialized.Should().BeEquivalentTo(testObject);
    }

    class TestClass
    {
        public TestTypedId TypedId { get; set; }
    }
}

@andrewlock
Copy link
Owner

I'm not sure why the [StronglyTypedIdConverterFactory] is necessary... isn't this what you get already with this:

using StronglyTypedIds;

[StronglyTypedId(converters: StronglyTypedIdConverter.SystemTextJson)] 
public partial struct TestTypedId { }

Maybe I'm missing something here? 🤔

@klinki
Copy link

klinki commented Oct 9, 2023

@andrewlock I just encountered the same issue. I believe generated SystemTextJsonConverter class should be public, but currently it is not. So maybe that is the reason why it is not used?

For reference, I use following code:

[assembly:StronglyTypedIdDefaults(
    backingType: StronglyTypedIdBackingType.Int,
    converters: StronglyTypedIdConverter.SystemTextJson | StronglyTypedIdConverter.EfCoreValueConverter
)]

[StronglyTypedId]
public partial struct UserId { }

and it generates this:

Generated code
[System.Text.Json.Serialization.JsonConverter(typeof(UserIdSystemTextJsonConverter))]
    readonly partial struct UserId : System.IComparable<UserId>, System.IEquatable<UserId>
    {
        public int Value { get; }

        public UserId(int value)
        {
            Value = value;
        }

        public static readonly UserId Empty = new UserId(0);

        public bool Equals(UserId other) => this.Value.Equals(other.Value);
        public override bool Equals(object obj)
        {
            if (ReferenceEquals(null, obj)) return false;
            return obj is UserId other && Equals(other);
        }

        public override int GetHashCode() => Value.GetHashCode();

        public override string ToString() => Value.ToString();
        public static bool operator ==(UserId a, UserId b) => a.Equals(b);
        public static bool operator !=(UserId a, UserId b) => !(a == b);
        public int CompareTo(UserId other) => Value.CompareTo(other.Value);

        public class EfCoreValueConverter : Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter<UserId, int>
        {
            public EfCoreValueConverter() : this(null) { }
            public EfCoreValueConverter(Microsoft.EntityFrameworkCore.Storage.ValueConversion.ConverterMappingHints mappingHints = null)
                : base(
                    id => id.Value,
                    value => new UserId(value),
                    mappingHints
                ) { }
        }

        class UserIdSystemTextJsonConverter : System.Text.Json.Serialization.JsonConverter<UserId>
        {
            public override UserId Read(ref System.Text.Json.Utf8JsonReader reader, System.Type typeToConvert, System.Text.Json.JsonSerializerOptions options)
            {
                return new UserId(reader.GetInt32());
            }

            public override void Write(System.Text.Json.Utf8JsonWriter writer, UserId value, System.Text.Json.JsonSerializerOptions options)
            {
                writer.WriteNumberValue(value.Value);
            }
        }
    }

@Ralf1108
Copy link

I'm not sure why the [StronglyTypedIdConverterFactory] is necessary... isn't this what you get already with this:

using StronglyTypedIds;

[StronglyTypedId(converters: StronglyTypedIdConverter.SystemTextJson)] 
public partial struct TestTypedId { }

Maybe I'm missing something here? 🤔

Ah yes, didn't saw it... and the required nuget package is in beta.
May I ask why? Or when will be a stable version released?

@andrewlock
Copy link
Owner

Hi @klinki,

I just encountered the same issue. I believe generated SystemTextJsonConverter class should be public, but currently it is not. So maybe that is the reason why it is not used?

Hmmm.... 🤔 As far as I know, that shouldn't make a difference... I've run integration tests that confirm this too. Even if the IDs is defined in a different project, the System.Text.Json converter still detects the converter attribute and can use it. I think the one case it would be an issue is if you're using the System.Text.Json source generator, because in that case you have to "manually" add the converter to the serialization context.

So in short, I think it makes sense to make them public anyway, and I've done that in #117.

Ah yes, didn't saw it... and the required nuget package is in beta.
May I ask why? Or when will be a stable version released?

I'm currently working on a big redesign of the library in this PR:

The main idea is to make the library much more maintainable while also giving people a mechanism to customise the generated IDs as much as they like. At some point after that I'll probably move it out of beta

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

Successfully merging a pull request may close this issue.

5 participants