dotnet / dotnet/aspnetcore

JsonPatchDocument.ApplyTo<T> should validate target model

Open
#7,158 6 comments 8 reactions 0 assignees View on GitHub
affected-medium area-mvc area-networking breaking-change bug feature-json-patch severity-minor
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

When ```ModelState.IsValid``` is called on model which is provided as a input parameter and the model isn't valid, ```ProblemDetails``` is generated and provided as a body output by all operations with status code >= 400.

```csharp
[HttpPost]
public async Task> Create(ToDoInsertModel toDoToInsert)
{
if (!ModelState.IsValid || toDoToInsert == null) { return BadRequest(); }

var toDo = _mapper.Map(toDoToInsert);
await _toDoAppDbContext.ToDos.AddAsync(toDo);
await _toDoAppDbContext.SaveChangesAsync();

return CreatedAtAction(nameof(Get), new {id = toDo.Id}, toDo);
}
```
In case above when ```ToDoInsertModel``` validation fails because ```Description``` is _null_, ```BadRequest``` returns following response:

```
HTTP/1.1 400 Bad Request
Transfer-Encoding: chunked
Content-Type: application/json; charset=utf-8
Server: Microsoft-IIS/10.0
X-SourceFiles: =?UTF-8?B?QzpcUHJvamVjdHNcYW50b25rYWxjaWstYXZhbmFkZS52aXN1YWxzdHVkaW8uY29tXFRvRG9BcHBcc3JjXFRvRG9BcHBTb2x1dGlvblxUb0RvQXBwLkFwaVx0b2Rvc1w0MA==?=
X-Powered-By: ASP.NET
Date: Thu, 31 Jan 2019 15:30:12 GMT

{
"errors": {
"Description": [
"The Description field is required."
]
},
"title": "One or more validation errors occurred.",
"status": 400,
"traceId": "800000b1-0002-f100-b63f-84710c7967bb"
}
```
Now implementation of ```HttpPatch``` Method with ```JsonPatchDocument.ApplyTo```.

```csharp
[Route("{id:int}")]
[HttpPatch]
public async Task> Modify(int id, JsonPatchDocument patch)
{
ToDoModel toDoDatabaseEntry = await _toDoAppDbContext.ToDos.SingleOrDefaultAsync(toDo => toDo.Id == id);

if (toDoDatabaseEntry == null) { return NotFound(); }

var toDoFromDatabase = _mapper.Map(toDoDatabaseEntry);
patch.ApplyTo(toDoFromDatabase, ModelState);
TryValidateModel(toDoFromDatabase);

if (!ModelState.IsValid) { return BadRequest(ModelState); }

_toDoAppDbContext.Entry(toDoDatabaseEntry).CurrentValues.SetValues(toDoFromDatabase);
await _toDoAppDbContext.SaveChangesAsync();

return toDoDatabaseEntry;
}
```
The output is different.

```
HTTP/1.1 400 Bad Request
Transfer-Encoding: chunked
Content-Type: application/json; charset=utf-8
Server: Microsoft-IIS/10.0
X-SourceFiles: =?UTF-8?B?QzpcUHJvamVjdHNcYW50b25rYWxjaWstYXZhbmFkZS52aXN1YWxzdHVkaW8uY29tXFRvRG9BcHBcc3JjXFRvRG9BcHBTb2x1dGlvblxUb0RvQXBwLkFwaVx0b2Rvc1w2?=
X-Powered-By: ASP.NET
Date: Thu, 31 Jan 2019 15:33:15 GMT

{
"Description": [
"The Description field is required."
]
}
```

The reason for this is that ```patch.ApplyTo(toDoFromDatabase, ModelState);``` doesn't invoke validation on the target model, which should be expected behavior. Because of this ``` TryValidateModel(toDoFromDatabase);``` need to be called explicitly, and ```ModeState``` object must be provided as an input for ```BadRequest``` object, which leads to inconsistent output. Need to notice when you don't provide ```ModeState``` you don't get any validation details errors.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.