csharpvitamins / CSharpVitamins.ShortGuid

A convenience wrapper for dealing with base64 encoded Guids
MIT License
103 stars 20 forks source link

Model Binding #5

Closed imukai closed 3 years ago

imukai commented 3 years ago

I've struggled with this over the years. I can't seem to get a controller to model bind a ShortGuid type. The form submits it with the correct property name, but it is a string.

For whatever reason the ShortGuid is always the default.

Any ideas?

davetransom commented 3 years ago

Depending on how far you want to go and how often it's used @imukai, it is okay to use just use it a string parameter and parse it as needed (I do this for one-offs) when it's simply an input.

However, if you want it setup as a model binder to handle things more automatically (peace of mind to just define a param/property as a ShortGuid) you can have a look at this gist. There's a few notes in there, so you can pick/choose your implementation, and has examples for MVC and WebApi.

Hope that helps. Let me know how you get on.

imukai commented 3 years ago

@davetransom so this is remediated by a custom model binder? I was hoping something could be done with the ShortGuid itself to behave better with the standard binder. Up until now (years) I have been pulling the string value manually from the form and ignoring the model .IsValid property coming into the action.

Thank you for sharing the gist, I will take a look at implementing one of the solutions unless I stumble across a permanent fix somewhere.

imukai commented 3 years ago

Your binders had some bits that my solution didn't like (Core 3.1) so I consulted MS and used one of their examples to craft this:

    public class ShortGuidBinder : IModelBinder
    {
        public Task BindModelAsync(ModelBindingContext bindingContext)
        {
            if (bindingContext == null)
            {
                throw new ArgumentNullException(nameof(bindingContext));
            }

            var modelName = bindingContext.ModelName;

            // Try to fetch the value of the argument by name
            var valueProviderResult = bindingContext.ValueProvider.GetValue(modelName);

            if (valueProviderResult == ValueProviderResult.None)
            {
                return Task.CompletedTask;
            }

            bindingContext.ModelState.SetModelValue(modelName, valueProviderResult);

            var value = valueProviderResult.FirstValue;

            // Check if the argument value is null or empty
            if (string.IsNullOrEmpty(value))
            {
                return Task.CompletedTask;
            }
            var model = new ShortGuid(value);
            bindingContext.Result = ModelBindingResult.Success(model);
            return Task.CompletedTask;
        }
    }

and simply decorated the ShortGuid struct with it:

    [ModelBinder(typeof(ShortGuidBinder))]
    public struct ShortGuid    {`

And all works a treat. The incoming models have their ShortGuid instead of screaming (AAAAAAAAAAA....)

davetransom commented 3 years ago

Glad you found a solution @imukai! (and yes, those examples are pre .NET Core - I'll add yours to the gist).

The intention of making the this class was to make it super easy to use, and be able to signify the type so they're easily identifiable when reading/debugging it, so I think you have a valid point.

Using the model binder syntax, while it works here, means you have to have a copy of the ShortGuid struct locally with the attribute on it, as it doesn't make sense to take a dependency like AspNetCore in this library. I'd still suggest adding the model binder to your app (example under ConfigureServices), if you still want to use package as a dependency.

However, maybe adding a simple TypeConverter to the struct would get this to the same point without adding any dependencies or requiring the configuration of model binders. I can't recall if support for this was patchy though, and still require a model binder.