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

Source generated extension method for SystemTextJson converters #644

Closed
Hona opened this issue Jul 20, 2024 · 8 comments
Closed

Source generated extension method for SystemTextJson converters #644

Hona opened this issue Jul 20, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@Hona
Copy link

Hona commented Jul 20, 2024

Describe the feature

Currently, we have to wire up all the JSON converters in all STJ options:

builder.Services.ConfigureHttpJsonOptions(options =>
{
    options.SerializerOptions.Converters.Add(new JsonStringEnumConverter());
    options.SerializerOptions.Converters.Add(new GameId.GameIdSystemTextJsonConverter());
    options.SerializerOptions.Converters.Add(new GameId.PlayerIdSystemTextJsonConverter());
    ...
});

It would be great for some extension method, example syntax being:

builder.Services.ConfigureHttpJsonOptions(options =>
{
    options.SerializerOptions.Converters.Add(new JsonStringEnumConverter());
    options.SerializerOptions.Converters.AddVogenJsonConverteres();
});
@Hona Hona added the enhancement New feature or request label Jul 20, 2024
@SteveDunn
Copy link
Owner

Thanks for the feedback. This would be useful. Something similar is generated when using System.Text.Json source generated (if it's not being emitted, it can be controlled by the systemTextJsonConverterFactoryGeneration parameter in global config).

When specified, it generates a type named VogenTypesFactory. It looks similar to this:

public class VogenTypesFactory : System.Text.Json.Serialization.JsonConverterFactory
{
    private static readonly Dictionary<Type, Lazy<System.Text.Json.Serialization.JsonConverter>> _lookup =
        new()
        {
            { typeof(Age), new Lazy<System.Text.Json.Serialization.JsonConverter>(() => new Age.AgeSystemTextJsonConverter()) },
            { typeof(Name), new Lazy<System.Text.Json.Serialization.JsonConverter>(() => new Name.NameSystemTextJsonConverter()) },
            { typeof(Address), new Lazy<System.Text.Json.Serialization.JsonConverter>(() => new Address.AddressSystemTextJsonConverter()) }
        };
    
    public override bool CanConvert(Type typeToConvert) => _lookup.ContainsKey(typeToConvert);
    
    public override System.Text.Json.Serialization.JsonConverter CreateConverter(Type typeToConvert, System.Text.Json.JsonSerializerOptions options) =>
        _lookup[typeToConvert].Value;
}

It's used like this:

var options = new JsonSerializerOptions
{
    WriteIndented = true,
    Converters =
    {
        new VogenTypesFactory()
    }
};

var ctx = new JsonSourceGenerationContext(options);

// use it
var json = JsonSerializer.Serialize(person, ctx.Person);

The source of this can be found in the samples.

@SteveDunn
Copy link
Owner

@Hona - I'm trying to find a scenario where you would need to manually add these converters that are generated by Vogen.
If the value object is decorated with the JsonConverter, the serialization will use that.
Have you got a small repo that reproduces this issue?

@Hona
Copy link
Author

Hona commented Jul 22, 2024

Heya, I wasn't aware of the VogenTypesFactory

Instead of

builder.Services.ConfigureHttpJsonOptions(options =>
{
    options.SerializerOptions.Converters.Add(new JsonStringEnumConverter());
    options.SerializerOptions.Converters.Add(new GameId.GameIdSystemTextJsonConverter());
    options.SerializerOptions.Converters.Add(new GameId.PlayerIdSystemTextJsonConverter());
    ...
});

I can do

builder.Services.ConfigureHttpJsonOptions(options =>
{
    options.SerializerOptions.Converters.Add(new JsonStringEnumConverter());
    options.SerializerOptions.Converters.Add(new VogenTypesFactory());
    ...
});

Thanks :)

I wonder if the doco could be clearer on all the wireups/integration? I can take a look once I finalise adding vogen etc to my v2 VSA template

@Hona
Copy link
Author

Hona commented Jul 22, 2024

The example (very WIP repo/branch) is here: https://github.com/Hona/VerticalSliceArchitecture/blob/v2-alpha/template/src/VerticalSliceArchitectureTemplate/Features/Games/PlayTurnCommand.cs

When you're binding the VO itself as a [FromBody] or other input straight from the API, we obviously have to configure ASP.NET Core to use the JSON converter - hence the snippets above

@CheloXL
Copy link

CheloXL commented Jul 22, 2024

@Hona I'm using VoGen in several projects, binding directly both in query params/body params and never had to register anything in ConfigureHttpJsonOptions, as the VOs are all already decorated with JsonConverter/TypeConverter attributes.
Also, since now they also extend IParsable, I'm using them in some minimal api projects, again, without any issues.
Maybe the problem is within the Hona.Endpoints package, that seems you are using to handle requests, as with standard api controller/actions and/or minimal api, VoGen works without any issues.

@Hona
Copy link
Author

Hona commented Jul 22, 2024

Hey @CheloXL thanks for your context!

There's a lot of work to go on that package so I'll check it out... there's a few ways to make inputs work with minimal apis so it's probably that

@SteveDunn
Copy link
Owner

Hi @Hona - did you manage to get this working?

@Hona
Copy link
Author

Hona commented Jul 29, 2024

Hey - yep its working great, no issue with vogen :)

@Hona Hona closed this as completed Jul 29, 2024
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

3 participants