Aspnetcore: Model Binder more misleading that before when using Required non-nullable strings.

[Bartmax]

I understand why this is the way it is, what I'm not sure if there are any intention to "fix"/change this behavior. My concern being that now is significant more misleading than it was before.
aka, not predictable, easy to miss and have bugs.

Using C#8 <Nullable>Enable</Nullable> property.

Posting a model with same "apparent" requirements on model binder (required, non null with a default value) will return error for string but not for other non-nullable types.
Behaves exactly as before, as if string is a nullable type.

        [Required]
        public string StringAsEmpty { get; set; } = string.Empty;

        [Required]
        public string StringAsDefault { get; set; } = default!;

        [Required]
        public int IntAsEmpty { get; set; } = default; // same as 0, or not assigned.

As "expected" posting to a controller that accepts that model without any form field/value:

<form asp-action="Create">
    <div asp-validation-summary="All"></div>
    <button type="submit">Submit</button>
</form> 

It will generate ModelState errors for string properties but not for int. (working as if string is a nullable type)

[javiercn]

@Bartmax Thanks for contacting us.

As far as I know, modelbinding is not affected in any way by the use of Nullable types in C#. Nullable types help the compiler specify the behavior, but don't offer any runtime guarantee about it nor extend the range of values a type can take.

C# 8 nullable types in essence are syntactic sugar to indicate that there should not be a null value in there, but the types it apply to are those that can have null as a value (references), while that is not the case for value types like int, where null is not a valid value for the type and the default value is not special in any way.

What I'm trying to say with this is that modelbinding should not be affected by C# 8 nullable types (I don't expect the behavior to have changed) and that the behavior you are seeing is modelbinding's behavior with the feature on or off.

Maybe a solution is to model your DTOs differently (use int?). The reason modelbinding doesn't give you an error on int fields is because it acts on the model, not on the payload of the request and value types always have a valid value, so they are always present.

@rynowak can correct me if I'm wrong :)

Hope that helps.

[rynowak]

@rynowak can correct me if I'm wrong :)

Ok, here I am. You're wrong 😆

@Bartmax - you might be running into a common point of confusion/contention with [Required]. [Required] is defined-as not-null. An integer is never null, so it will never trigger a validation error with [Required]. All of the types in System.Collections.DataAnnotations refer to the state of a model object once binding is complete - they don't say anything about whether or not there was data on the wire. If you want to validate that data was included, use [BindRequired].

---

Can you be really specific about what you mean by:

Posting a model with same "apparent" requirements on model binder (required, non null with a default value) will return error for string but not for other non-nullable types.

I re-read your post a few times, and I'm not sure what I'm missing but I don't completely understand what you're saying the bug is. What are the behaviour differences from 2.2?

[javiercn]

[Required] is defined-as not-null.

I wasn't, this is what I meant. But maybe my English is broken :)

[Bartmax]

Ok, I think both are 👍 and also missing the point here, sorry for not making it clear enough.
So let me address what's the issue or my point in here.

As @rynowak said,

[Required] is defined-as not-null. An integer is never null, so it will never trigger a validation error with [Required].

From a " concept " point of view, we now have, with c#8, string as not-null, following the same logic, a non-null value never trigger a validation error with [Required] before, but as of right now, a non-null string will trigger validation error.

TLDR, what changed?:
Before:
Non-null value will never trigger validation error.
Now:
Non-null value except non-null string will never trigger validation error.

It's ok from code point of view, it's misleading or confusing from api consumer point of view.

[rynowak]

From a " concept " point of view, we now have, with c#8, string as not-null, following the same logic, a non-null value never trigger a validation error with [Required] before, but as of right now, a non-null string will trigger validation error.

Ah! Ok. This makes sense now. Let me elaborate.

TLDR: MVC will give you a validation error when it violates your nullability contract

C#8 not-null is not a runtime contract. You can easily produces instance of objects that contain null where you said you don't expect to find null. You can prove this by newing up the class from your example.

MVC's error handling contract for model binding is that we new up your class, and hand you an instance in your action method - even if it's invalid. This presents us (MVC team) with a decision to make - we're going to give you an instance of your object sometimes that has null where you don't expect to find null.

The only real pivot we have is to make this a validation error or not, so we chose to make it a validation error. I hope that makes sense.

To contrast this with what you said about integers, there's no way MVC can stick a null in your integer property, the value will always be non-null. That's not true for non-nullable reference types, the default value will often be null.

[Bartmax]

Thanks, @rynowak. I think the key point is that C#8 no-null is not a runtime contract, hence i don't think there's anything actionable here, also I agree making it a validation error was the correct call.

[brunosantosrhsenso]

This is really confusing. How do I make a string property not required?

This property:

public string? Details { get; set; }

Is being considered as required:

{"errors":{"Details":["The Details field is required."]}

Are we saying it's not possible for modelbinding to allow null on a string?

[Bartmax]

@brunosantosrhsenso string? should not be marked as required. Try to do [Required(false)] and see if changes. I can only assume that you are marking it as required somewhere else or using old bins, something along these lines or a bug somewhere.

You can totally bind nullable strings as non-required.

[colotiline]

@Bartmax [Required(false)] doesn't exist.

[colotiline]

@Bartmax [Required(AllowEmptyStrings = true)] doesn't allow nulls.

[colotiline]

@Bartmax I've got same situation as @brunosantosrhsenso .

Code

public class QueryFilter
{
    private string? query;
    [StringLength(255, MinimumLength = 2, ErrorMessage = "Min length is 2.")]
    public string? Query 
    { 
        get 
        { 
            if (string.IsNullOrEmpty(this.query))
            {
                return this.query;
            }

            return query.ToUpper(); 
        }

        set { this.query = value; }
    }
}

public class QuerySortingFilter : QueryFilter
{
    public QuerySortingFilter()
    {
        Sorting = new SortingFilter();
    }
    public SortingFilter Sorting { get; set; }
}

public class GetByStatusesFilter : QuerySortingFilter
{
    public DocStatus[]? Statuses { get; set; }
}

[HttpGet("docs/{userId}")]
public async Task<IActionResult> GetDocsAsync
(
    Guid userId,
    [FromQuery] ListFilter<GetByStatusesFilter> listFilter
)
{
    // Something.
}

Error

Details
My controller has attribute [ApiController].

[Bartmax]

It shouldn't.
Submit a new issue as a bug.
Not setting the required attribute should not trigger a validation error