[Question] ModelBinding [Required] is MVC5 BackCompat needed?

[Bartmax]

Back in february, there were an issue (#1971) about binding value types with [Required] attribute that doesn't creates a ModelState error.
This was 'fixed' and now we can use

[Required]
public int Age { get; set;}

and the ModelBinder will trigger a ModelState error if the parameter is not in the request. Which (to me) looks like the correct behavior.

Unfortunatelly, this isn't backcompat with MVC5 (also pointed out by myself on #1971).

Right now, I think there's work to bring back said compatibility (fa56df9) and today I came across a issue about MVC5 Back Compat in a somehow related scenario when using [FromBody] here: #2423 (comment)

I saw a [BindRequired] attribute mentioned here: #2526 and here: #2493, while I'm not sure if this is designed because of bringing back MVC5 compat with the [Required] attribute, looks like it.

Is there any real reason to keep that Back Compatibility or is just because of back-compat ? While I don't think so, I'm asking because I don't see any value of having a [Required] attribute that doesn't create ModelState errors on value types, actually I think it's a bug (not intended behavior) in MVC5.
What's the purpose of a [Required] attribute if it will always have a value and will never trigger the Required expectation ? What am I missing ?

[dougbu]

@Bartmax the [Required] attribute is not part of MVC. What we are doing in MVC 6 is matching the BCL semantics and avoiding additional magic beyond that. See https://msdn.microsoft.com/en-us/library/system.componentmodel.dataannotations.requiredattribute.aspx for more information.

If you prefer to ensure a particular value appears on the wire, please use [BindRequired].

[Bartmax]

Then I propose to change the BCL semantics, make [Required] so it's not null, is on payload and is not empty or whitespace (i would call this smart not magic) and if you still need [NotNull] attribute then can be one but I don't see a use for it.

[danroth27]

We can't change the underlying BCL semantics - those semantics are not specific to MVC. You have to use the features of the underlying formatter if you want these semantics.

[Bartmax]

ok, I hope we can use in mvc [BindRequired] for everything (like inheriting from Required) and not have to deal with some attributes with required, others with bindrequired or worst with both.

I still think it's is a bad design (from a framework api perspective).

[rynowak]

The other thing you can if you want this behavior is just customize ModelMetadata. That's a much easier task now than it was in previous versions.

Sample:

public class MyBindingDetailsProvider : IBindingMetadataProvider
{
    public void GetBindingMetadata(BindingMetadataProviderContext context)
    {
        if (context.PropertyAttributes.OfType<RequiredAttribute>().Any())
        {
            context.BindingMetadata.IsBindingRequired = true;
        }
    }
}

// In startup.cs
options.ModelMetadataDetailsProviders.Add(new MyBindingDetailsProvider());

Now [Required] implies [BindRequired]

[Bartmax]

@rynowak beautiful! that's something I'm going to use 100% of the time for sure. thanks!

[Bartmax]

@rynowak just for curiosity, why that cannot be the default binding configuration mvc uses?

[dougbu]

why that cannot be the default binding configuration mvc uses?

[Required] is a .NET data annotation with a defined meaning and MVC interpreting it differently would be confusing. From https://msdn.microsoft.com/en-us/library/system.componentmodel.dataannotations.requiredattribute(v=vs.110).aspx

A validation exception is raised if the property is null, contains an empty string (""), or contains only white-space characters.

But as @rynowak showed, your application can easily change how [Required] is interpreted.

[Bartmax]

I see your point, and from a technical perspective that's fine but I still found very confusing (specially for new comers) that in the following code

[Required]
public int Age {get;set;}

Required does nothing, and in a web api endpoint one should use BOTH, Required and BindRequired. This clearly doesn't work as anyone would expect.

I don't see why we cannot have a extra documentation that says that for MVC ModelBinders when a required attribute is used and the property is not included in the request payload it will trigger a modelstate error.