dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.48k stars 10.04k forks source link

Form data naming strategy #47187

Closed lus closed 1 year ago

lus commented 1 year ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe the problem.

I am currently trying to support both, form data and JSON, for an API endpoint. When registering a custom JSON naming strategy, this obviously won't get applied to the form data deserializer. Is there any possibility of doing this? I want form data requests to follow the same naming strategy (snake case in this case).

Describe the solution you'd like

An API just like the JsonSerializerSettings would be awesome. Maybe exactly this already exists, but I could not find anything.

Additional context

No response

captainsafia commented 1 year ago

@lus Unfortunately, this is not supported in minimal APIs at the moment.

You can roll out a BindAsync implementation on the form parameter type that you are you using and write a processor for the form data that uses the JSON serializer options.

lus commented 1 year ago

@captainsafia I don't use minimal APIs, but controllers. I am sorry I did not make that clear initially. Does that change something or should I use the same workaround?

lus commented 1 year ago

I just wrote a custom FormValueProvider like this:

public class JsonNamingApplyingFormValueProvider : FormValueProvider
{

    private readonly JsonNamingPolicy _namingPolicy;

    public JsonNamingApplyingFormValueProvider(
        BindingSource bindingSource,
        IFormCollection values,
        CultureInfo? culture,
        JsonNamingPolicy namingPolicy
    ) : base(bindingSource, values, culture)
    {
        _namingPolicy = namingPolicy;
    }

    public override IDictionary<string, string> GetKeysFromPrefix(string prefix)
    {
        return base.GetKeysFromPrefix(_namingPolicy.ConvertName(prefix));
    }

    public override bool ContainsPrefix(string prefix)
    {
        return base.ContainsPrefix(_namingPolicy.ConvertName(prefix));
    }

    public override ValueProviderResult GetValue(string key)
    {
        return base.GetValue(_namingPolicy.ConvertName(key));
    }

}

This works perfectly for keys, but not for enum members as values. So I created this simple type converter:

public class SnakeCaseEnumConverter : EnumConverter
{

    public SnakeCaseEnumConverter(Type type) : base(type)
    {
    }

    public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value)
    {
        if (value is string raw)
        {
            string? candidate = Enum.GetNames(EnumType)
                .FirstOrDefault(name => raw.ToLower().Trim() == NamingPolicies.SnakeCase.ConvertName(name));
            return string.IsNullOrEmpty(candidate) ? null : Enum.Parse(EnumType, candidate, true);
        }

        return base.ConvertFrom(context, culture, value);
    }

}

(NamingPolicies.SnakeCase is a static instance of JsonNamingPolicy)

captainsafia commented 1 year ago

@captainsafia I don't use minimal APIs, but controllers. I am sorry I did not make that clear initially. Does that change something or should I use the same workaround?

Yes, the alternative strategy that you ended up using is sufficient when you're using MVC modelbinding. Thanks for sharing it!

In the long-term, we don't support form-binding in minimal APIs at the moment. We've had design conversations that hypothesized an implementation that routes form data through JSON to aid with the processing, which would solve the problem that you have here.

For MVC, there's a high level of customizability for binding behavior so I don't think we'd introduce a new built-in API for this particular request.

lus commented 1 year ago

@captainsafia thank you for your reply. However, I think a simpler way of ensuring consistent deserialization strategies would be very useful anyways. Maybe a built-in adapter that applies all applicable JSON options to form data deserialization could be the way to go here? I am not sure, but maybe I am not the only one who would appreciate something like this.

captainsafia commented 1 year ago

I am not sure, but maybe I am not the only one who would appreciate something like this.

Yep! As is usually the case, if we get more issues filed with the same request, it'll be an indicator that it's a worthwhile API to include by default.

Out of curiosity, are you doing something on the client-side to map your form data keys to snake-case?

lus commented 1 year ago

The concrete case here was that I have multiple JSON endpoints using snake case. One endpoint should support form data as well, as it has to accept files and base64 strings in JSON cannot be streamed that easily, so I chose form data. As it accepts the same data as its JSON equivalent, I wanted the keys to be consistent. Additionally, The Swashbuckle Swagger generator generates keys using the JSON serializer options, regardless of the content type of the request, so it generated snake case key names. I hope I understood your question correctly.

captainsafia commented 1 year ago

Additionally, The Swashbuckle Swagger generator generates keys using the JSON serializer options, regardless of the content type of the request, so it generated snake case key names.

Now this is an interesting observation. Do you mind filing an issue for the Swagger-specific issue with an example of what the OpenAPI JSON and the Swagger UI look like?

I assume based on your description here that the default behavior (form data not using JSON serializer options) doesn't work properly in Swagger UI.

lus commented 1 year ago

Well, I'll have to look into it again, as I also use custom filters. Maybe one of them generates the request example. They still haven't fixed their issue of multiple endpoints using the same route data but the consumed content type resulting in a conflict error (it's been two years since the issue was opened), so some hacky workaround was needed. Maybe that one induced the wrongly generated keys and the issue is my fault.

ghost commented 1 year ago

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.