ModelBinder - Model becomes NULL if only One of its properties is invalid

[mdmoura]

Describe the bug

Model becomes NULL if only ONE of its properties is invalid

To Reproduce

Using ASP.NET Core 2.2 I have, on an API Project, the following Controller:

public class BookController : Controller {

  public async Task<IActionResult> Create([FromBody]BookModel model) {
      // Validate and Create book      
  }

}

The BookModel is:

public class BookModel { 
  public String Title { get; set; }
  public DateTime? PublishedAt { get; set; }       
}

I called the action with an invalid PublishedAt date:

{
  "Title": "book title",
  "PublishedAt": "2019-04-32"
}

In this case, on my action, model is NULL even if Title is defined and is valid.

If I call the action with a valid PublishedAt date or NULL then the model is not null.

Expected behavior

Shouldn't model in action be defined even if one of its properties fails to bind?

Additional context

.NET Core SDK (reflecting any global.json):
Version: 2.2.104
Commit: 73f036d4ac

Runtime Environment:
OS Name: Mac OS X
OS Version: 10.14
OS Platform: Darwin
RID: osx.10.14-x64
Base Path: /usr/local/share/dotnet/sdk/2.2.104/

Host (useful for support):
Version: 2.2.2
Commit: a4fd7b2c84

.NET Core SDKs installed:
2.2.104 [/usr/local/share/dotnet/sdk]

[pranavkm]

You likely have a error in ModelState that says that the Json deserilization failed when it encountered an invalid date. You should generally see a constructed model if validation fails, for instance if you have a JsonProperty(Required = Required.Always), but not if deserialization errors.

[mdmoura]

@pranavkm Wouldn't be possible to build the model even if deserialising of some properties fails?

The model properties with deserialisation errors would become null, if nullable, or have default value.

The deserialisation errors would be added to ModelState the same was as they are now ...

With the partial model validation could carry on to next step, Fluent Validation or Annotations ...

The other properties, the ones that not failed binding, could be validated ...

And in the end all errors could be returned in one single Bad Request.

Wouldn't this make sense?

The other option would be to have all Model properties as Strings ... Which seems strange to me.

[mdmoura]

@pranavkm Maybe a flag could be added to each error indicating the origin of the error: binding or value validation. Then all the errors, and what is returned, could be handled in:

services.Configure<ApiBehaviorOptions>(x => {
  x.SuppressModelStateInvalidFilter = false;
  x.InvalidModelStateResponseFactory = y => {
    // Get all errors and define what to return
    return new BadRequestObjectResult(errors);
  };
});

I think this would make sense instead of returning BadRequests in two steps as it is now:

1. Errors from model binder
2. Errors from validation

[mdmoura]

@Eilon @pranavkm Maybe ASP.NET Core 3 could allow process to carry on after a model binding error occurs? I believe you are implementing a new JSON parser for ASP.NET Core?

[pranavkm]

@mdmoura you should see all Json formatting errors that Json.Net reports as part of the bad result. Are you seeing otherwise?

[mdmoura]

@pranavkm Yes, I see the formatting errors that Json.Net reports.

But the ideal would be, after those errors are added to Model State, the model to be build with the fields that were able to deserialize and carry on to "normal" validation ...

Then the fields that were serialize could be validated and their validation errors also added to Model State.

Finally, return all errors, from deserialising / binding and validation in One Single Bad Request.

Is my explanation clear now?

Thank You