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

Be able to configure Json serialization options #165

Closed
CheloXL opened this issue Jun 6, 2022 · 9 comments
Closed

Be able to configure Json serialization options #165

CheloXL opened this issue Jun 6, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@CheloXL
Copy link

CheloXL commented Jun 6, 2022

Describe the feature

I'm trying Vogen on an existing project. Everything went fine until I needed to serialize my Ids. Since my Ids are of type long, and javascript numbers are restricted to int, I created a custom serializer for longs that convert them to strings.

The problem is that Vogen is writting the id as "number" and from the js side, that number can't be read, nor can be send back to the server, as it's trimmed down to the nearest integer.

I would like a way to customize the serialization so the type is written as string, and parsed back from string (like long.Parse(s, NumberStyles.Integer, CultureInfo.InvariantCulture);).

@CheloXL CheloXL added the enhancement New feature or request label Jun 6, 2022
@SteveDunn
Copy link
Owner

Thanks for the report @CheloXL . What Json library are you using for [de]serialization? By default, Vogen uses System.Text.Json (STJ). And by default, the values are serialized just as the primitive would be. This is a unit test in the Vogen repo:

        [Fact]
        public void CanSerializeToLong_WithSystemTextJsonProvider()
        {
            var vo = SystemTextJsonLongVo.From(123L);

            string serializedVo = SystemTextJsonSerializer.Serialize(vo);
            string serializedLong = SystemTextJsonSerializer.Serialize(vo.Value);

            serializedVo.Equals(serializedLong).Should().BeTrue();
        }

The serialized string from STJ looks, for both the Value Object and primitive, is just "123":

image

The Value Object type looks like this:

    [ValueObject(conversions: Conversions.SystemTextJson, underlyingType: typeof(long))]
    public partial class SystemTextJsonLongVo { }

@CheloXL
Copy link
Author

CheloXL commented Jun 6, 2022

I'm using STJ.
This is the code Vogen is creating for my VO:

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

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

And that's where my problem is. A valid holder id of 720742592373919744 is sent to js, but js reads it as 720742592373919700. That's a js limitation, I'm totally aware, and that's why in my api I have a custom converter for longs, so the id is sent as "720742592373919744" (a string instead of a number).

What I would like is a way to do the same in Vogen. That could be done in two different ways:

  • Configure Vogen so it converts the primitive from/to string (using InvariantCulture).
  • Configure Vogen so I'm able to pass a custom json options, and instead of you writting the primitives as-is, write them as objects.

@SteveDunn
Copy link
Owner

SteveDunn commented Jun 6, 2022

Ah, I see. I think your suggestions are great. It would be handy, in this case, to be able to specify a type that handles this customization, e.g.

[ValueObject(typeof(double), hook: typeof(DoubleToStringSerializationHook)]
public partial struct HolderId {}

And DoubleToStringSerializationHook would have something like:

public class DoubleToStringSerializationHook
{
    public double ToDouble(string input) => ...
    public string ToString(double input) => ...
}

As you might be aware, due to the constraints of not being able to create Value Objects other than the From method, it's difficult to provide your own converter/serializer.

I can take a look at implementing this at the weekend or early next week.

Thanks again for the support and for reporting the issue. I hope the issue isn't a game-changer for you!

@CheloXL
Copy link
Author

CheloXL commented Jun 6, 2022

That would be great!. Maybe the type could be a static class with two static methods named by convention ConvertFrom/ConvertTo? No need to create instances of the converter (no state should be involved in the conversion anyways).

Right now I have this on a specific branch (no need to toy with releases), but so far I like the concept a lot. Thank you for your great work!.

@SteveDunn
Copy link
Owner

Excellent - thank you for saying so. It's great to hear that it's being used!

@SteveDunn
Copy link
Owner

I've implemented this. I haven't yet tested with a javascript client, but I did test it with Swagger.

The follow definition in C#

[ValueObject(underlyingType: typeof(double), customizations: Customizations.TreatDoublesAsStringsInSystemTextJson)]
public partial class HolderId1
{
}

[ValueObject(underlyingType: typeof(double))]
public partial class HolderId2
{
}
    
public class WeatherForecast
{
    public HolderId1 Holder1 { get; set; } = null!;
    public HolderId2 Holder2 { get; set; } = null!;
}

        [HttpGet(Name = "holders")]
        public WeatherForecast Get()
        {
            return new WeatherForecast
            {
                Holder1 = HolderId1.From(720742592373919744),
                Holder2 = HolderId2.From(720742592373919744),
            };
        }

Returns the following json:

{
  "holder1": "7.207425923739197E+17",
  "holder2": 720742592373919700
}

However, posting it back shows:

f.Holder1.Value == 720742592373919744d
true
f.Holder2.Value == 720742592373919744d
true

In fact, any value in the range of 7207425923739197XX yields true.

This is probably due to the accuracy of double:

720742592373919744d == 720742592373919744d  // true
720742592373919744d== 720742592373919700d // true
720742592373919744d== 720742592373919799d  // true

So, it looks like the trip to and from javascript is now fine. Is this what you're expecting? If so, I'll merge it in and do a release.

Hope this helps.

@CheloXL
Copy link
Author

CheloXL commented Jun 17, 2022

Looks fantastic. Do you also have a Customizations.TreatLongsAsStringsInSystemTextJson?
Or maybe it's better to have a Customizations.TreatTypeAsStringsInSystemTextJson and use the right conversion for numbers (guids are always converted to strings and I can't think of any other type you can use as ID).

@SteveDunn
Copy link
Owner

@CheloXL this has now been implemented and release in 1.0.22. The parameter on the attribute is now:

[ValueObject(underlyingType: typeof(double), customizations: Customizations.TreatNumberAsStringInSystemTextJson)]
public partial class HolderId { }

All underlying types that used WriteNumber can now use this customization to instead write a string.

Thanks for the feedback! Let me know if there's any issues.

@CheloXL
Copy link
Author

CheloXL commented Jun 18, 2022

I'm testing this right now and so far, everything is going well. Thank you very much!

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

No branches or pull requests

2 participants