Skip to content

Commit 6edb907

Browse files
nanotaboadaCopilotclaude
committed
refactor(api): use RFC 7807 Problem Details for error responses (#418)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 26d0de6 commit 6edb907

2 files changed

Lines changed: 107 additions & 37 deletions

File tree

‎src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs‎

Lines changed: 56 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ IValidator<PlayerRequestModel> validator
3030
[HttpPost(Name = "Create")]
3131
[Consumes(MediaTypeNames.Application.Json)]
3232
[ProducesResponseType<PlayerResponseModel>(StatusCodes.Status201Created)]
33-
[ProducesResponseType(StatusCodes.Status400BadRequest)]
33+
[ProducesResponseType<ProblemDetails>(StatusCodes.Status400BadRequest)]
3434
[ProducesResponseType(StatusCodes.Status409Conflict)]
3535
public async Task<IResult> PostAsync([FromBody] PlayerRequestModel player)
3636
{
@@ -39,11 +39,15 @@ public async Task<IResult> PostAsync([FromBody] PlayerRequestModel player)
3939
if (!validation.IsValid)
4040
{
4141
var errors = validation
42-
.Errors.Select(error => new { error.PropertyName, error.ErrorMessage })
43-
.ToArray();
42+
.Errors.GroupBy(e => e.PropertyName)
43+
.ToDictionary(g => g.Key, g => g.Select(e => e.ErrorMessage).ToArray());
4444

4545
logger.LogWarning("POST /players validation failed: {@Errors}", errors);
46-
return TypedResults.BadRequest(errors);
46+
return TypedResults.ValidationProblem(
47+
errors,
48+
detail: "See the errors field for details.",
49+
instance: HttpContext?.Request?.Path.ToString()
50+
);
4751
}
4852

4953
if (await playerService.RetrieveBySquadNumberAsync(player.SquadNumber) != null)
@@ -76,7 +80,7 @@ public async Task<IResult> PostAsync([FromBody] PlayerRequestModel player)
7680
/// <response code="404">Not Found</response>
7781
[HttpGet(Name = "Retrieve")]
7882
[ProducesResponseType<PlayerResponseModel>(StatusCodes.Status200OK)]
79-
[ProducesResponseType(StatusCodes.Status404NotFound)]
83+
[ProducesResponseType<ProblemDetails>(StatusCodes.Status404NotFound)]
8084
public async Task<IResult> GetAsync()
8185
{
8286
var players = await playerService.RetrieveAsync();
@@ -89,7 +93,12 @@ public async Task<IResult> GetAsync()
8993
else
9094
{
9195
logger.LogWarning("GET /players not found");
92-
return TypedResults.NotFound();
96+
return TypedResults.Problem(
97+
statusCode: StatusCodes.Status404NotFound,
98+
title: "Not Found",
99+
detail: "No players were found.",
100+
instance: HttpContext?.Request?.Path.ToString()
101+
);
93102
}
94103
}
95104

@@ -102,7 +111,7 @@ public async Task<IResult> GetAsync()
102111
[Authorize]
103112
[HttpGet("{id:Guid}", Name = "RetrieveById")]
104113
[ProducesResponseType<PlayerResponseModel>(StatusCodes.Status200OK)]
105-
[ProducesResponseType(StatusCodes.Status404NotFound)]
114+
[ProducesResponseType<ProblemDetails>(StatusCodes.Status404NotFound)]
106115
public async Task<IResult> GetByIdAsync([FromRoute] Guid id)
107116
{
108117
var player = await playerService.RetrieveByIdAsync(id);
@@ -114,7 +123,12 @@ public async Task<IResult> GetByIdAsync([FromRoute] Guid id)
114123
else
115124
{
116125
logger.LogWarning("GET /players/{Id} not found", id);
117-
return TypedResults.NotFound();
126+
return TypedResults.Problem(
127+
statusCode: StatusCodes.Status404NotFound,
128+
title: "Not Found",
129+
detail: $"Player with Id '{id}' was not found.",
130+
instance: HttpContext?.Request?.Path.ToString()
131+
);
118132
}
119133
}
120134

@@ -126,7 +140,7 @@ public async Task<IResult> GetByIdAsync([FromRoute] Guid id)
126140
/// <response code="404">Not Found</response>
127141
[HttpGet("squadNumber/{squadNumber:int}", Name = "RetrieveBySquadNumber")]
128142
[ProducesResponseType<PlayerResponseModel>(StatusCodes.Status200OK)]
129-
[ProducesResponseType(StatusCodes.Status404NotFound)]
143+
[ProducesResponseType<ProblemDetails>(StatusCodes.Status404NotFound)]
130144
public async Task<IResult> GetBySquadNumberAsync([FromRoute] int squadNumber)
131145
{
132146
var player = await playerService.RetrieveBySquadNumberAsync(squadNumber);
@@ -142,7 +156,12 @@ public async Task<IResult> GetBySquadNumberAsync([FromRoute] int squadNumber)
142156
else
143157
{
144158
logger.LogWarning("GET /players/squadNumber/{SquadNumber} not found", squadNumber);
145-
return TypedResults.NotFound();
159+
return TypedResults.Problem(
160+
statusCode: StatusCodes.Status404NotFound,
161+
title: "Not Found",
162+
detail: $"Player with Squad Number '{squadNumber}' was not found.",
163+
instance: HttpContext?.Request?.Path.ToString()
164+
);
146165
}
147166
}
148167

@@ -162,8 +181,8 @@ public async Task<IResult> GetBySquadNumberAsync([FromRoute] int squadNumber)
162181
[HttpPut("squadNumber/{squadNumber:int}", Name = "Update")]
163182
[Consumes(MediaTypeNames.Application.Json)]
164183
[ProducesResponseType(StatusCodes.Status204NoContent)]
165-
[ProducesResponseType(StatusCodes.Status400BadRequest)]
166-
[ProducesResponseType(StatusCodes.Status404NotFound)]
184+
[ProducesResponseType<ProblemDetails>(StatusCodes.Status400BadRequest)]
185+
[ProducesResponseType<ProblemDetails>(StatusCodes.Status404NotFound)]
167186
public async Task<IResult> PutAsync(
168187
[FromRoute] int squadNumber,
169188
[FromBody] PlayerRequestModel player
@@ -173,20 +192,29 @@ [FromBody] PlayerRequestModel player
173192
if (!validation.IsValid)
174193
{
175194
var errors = validation
176-
.Errors.Select(error => new { error.PropertyName, error.ErrorMessage })
177-
.ToArray();
195+
.Errors.GroupBy(e => e.PropertyName)
196+
.ToDictionary(g => g.Key, g => g.Select(e => e.ErrorMessage).ToArray());
178197

179198
logger.LogWarning(
180199
"PUT /players/squadNumber/{SquadNumber} validation failed: {@Errors}",
181200
squadNumber,
182201
errors
183202
);
184-
return TypedResults.BadRequest(errors);
203+
return TypedResults.ValidationProblem(
204+
errors,
205+
detail: "See the errors field for details.",
206+
instance: HttpContext?.Request?.Path.ToString()
207+
);
185208
}
186209
if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null)
187210
{
188211
logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber);
189-
return TypedResults.NotFound();
212+
return TypedResults.Problem(
213+
statusCode: StatusCodes.Status404NotFound,
214+
title: "Not Found",
215+
detail: $"Player with Squad Number '{squadNumber}' was not found.",
216+
instance: HttpContext?.Request?.Path.ToString()
217+
);
190218
}
191219
if (player.SquadNumber != squadNumber)
192220
{
@@ -195,11 +223,11 @@ [FromBody] PlayerRequestModel player
195223
squadNumber,
196224
player.SquadNumber
197225
);
198-
return TypedResults.BadRequest(
199-
new
200-
{
201-
Error = "Squad number in the route does not match squad number in the request body."
202-
}
226+
return TypedResults.Problem(
227+
statusCode: StatusCodes.Status400BadRequest,
228+
title: "Bad Request",
229+
detail: "Squad number in the route does not match squad number in the request body.",
230+
instance: HttpContext?.Request?.Path.ToString()
203231
);
204232
}
205233
await playerService.UpdateAsync(player);
@@ -229,13 +257,18 @@ [FromBody] PlayerRequestModel player
229257
/// <response code="404">Not Found</response>
230258
[HttpDelete("squadNumber/{squadNumber:int}", Name = "Delete")]
231259
[ProducesResponseType(StatusCodes.Status204NoContent)]
232-
[ProducesResponseType(StatusCodes.Status404NotFound)]
260+
[ProducesResponseType<ProblemDetails>(StatusCodes.Status404NotFound)]
233261
public async Task<IResult> DeleteAsync([FromRoute] int squadNumber)
234262
{
235263
if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null)
236264
{
237265
logger.LogWarning("DELETE /players/squadNumber/{SquadNumber} not found", squadNumber);
238-
return TypedResults.NotFound();
266+
return TypedResults.Problem(
267+
statusCode: StatusCodes.Status404NotFound,
268+
title: "Not Found",
269+
detail: $"Player with Squad Number '{squadNumber}' was not found.",
270+
instance: HttpContext?.Request?.Path.ToString()
271+
);
239272
}
240273
else
241274
{

‎test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs‎

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ public async Task Post_Players_ValidationError_Returns400BadRequest()
5656
),
5757
Times.Once
5858
);
59-
if (result is BadRequest httpResult)
59+
if (result is ValidationProblem httpResult)
6060
{
61-
httpResult.Should().NotBeNull().And.BeOfType<BadRequest>();
61+
httpResult.Should().NotBeNull().And.BeOfType<ValidationProblem>();
6262
httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest);
6363
}
6464
}
@@ -198,9 +198,9 @@ public async Task Get_Players_NonExisting_Returns404NotFound()
198198

199199
// Assert
200200
service.Verify(service => service.RetrieveAsync(), Times.Once);
201-
if (result is NotFound httpResult)
201+
if (result is ProblemHttpResult httpResult)
202202
{
203-
httpResult.Should().NotBeNull().And.BeOfType<NotFound>();
203+
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
204204
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
205205
}
206206
}
@@ -223,9 +223,9 @@ public async Task Get_PlayerById_NonExisting_Returns404NotFound()
223223

224224
// Assert
225225
service.Verify(service => service.RetrieveByIdAsync(It.IsAny<Guid>()), Times.Once);
226-
if (result is NotFound httpResult)
226+
if (result is ProblemHttpResult httpResult)
227227
{
228-
httpResult.Should().NotBeNull().And.BeOfType<NotFound>();
228+
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
229229
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
230230
}
231231
}
@@ -274,9 +274,9 @@ public async Task Get_PlayerBySquadNumber_NonExisting_Returns404NotFound()
274274

275275
// Assert
276276
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
277-
if (result is NotFound httpResult)
277+
if (result is ProblemHttpResult httpResult)
278278
{
279-
httpResult.Should().NotBeNull().And.BeOfType<NotFound>();
279+
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
280280
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
281281
}
282282
}
@@ -350,9 +350,9 @@ public async Task Put_PlayerBySquadNumber_ValidationError_Returns400BadRequest()
350350
),
351351
Times.Once
352352
);
353-
if (result is BadRequest httpResult)
353+
if (result is ValidationProblem httpResult)
354354
{
355-
httpResult.Should().NotBeNull().And.BeOfType<BadRequest>();
355+
httpResult.Should().NotBeNull().And.BeOfType<ValidationProblem>();
356356
httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest);
357357
}
358358
}
@@ -392,13 +392,50 @@ public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound()
392392
),
393393
Times.Once
394394
);
395-
if (result is NotFound httpResult)
395+
if (result is ProblemHttpResult httpResult)
396396
{
397-
httpResult.Should().NotBeNull().And.BeOfType<NotFound>();
397+
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
398398
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
399399
}
400400
}
401401

402+
[Fact]
403+
[Trait("Category", "Unit")]
404+
public async Task Put_PlayerBySquadNumber_SquadNumberMismatch_Returns400BadRequest()
405+
{
406+
// Arrange
407+
var squadNumber = 23;
408+
var request = PlayerFakes.MakeRequestModelForUpdate(squadNumber);
409+
request.SquadNumber = 99; // Mismatched squad number
410+
var response = PlayerFakes.MakeResponseModelForUpdate(squadNumber);
411+
var (service, logger, validator) = PlayerMocks.InitControllerMocks();
412+
service
413+
.Setup(service => service.RetrieveBySquadNumberAsync(squadNumber))
414+
.ReturnsAsync(response);
415+
validator
416+
.Setup(validator =>
417+
validator.ValidateAsync(
418+
It.IsAny<PlayerRequestModel>(),
419+
It.IsAny<CancellationToken>()
420+
)
421+
)
422+
.ReturnsAsync(new ValidationResult(new List<ValidationFailure> { }));
423+
424+
var controller = new PlayerController(service.Object, logger.Object, validator.Object);
425+
426+
// Act
427+
var result = await controller.PutAsync(squadNumber, request);
428+
429+
// Assert
430+
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
431+
service.Verify(service => service.UpdateAsync(It.IsAny<PlayerRequestModel>()), Times.Never);
432+
if (result is ProblemHttpResult httpResult)
433+
{
434+
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
435+
httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest);
436+
}
437+
}
438+
402439
[Fact]
403440
[Trait("Category", "Unit")]
404441
public async Task Put_PlayerBySquadNumber_Existing_Returns204NoContent()
@@ -461,9 +498,9 @@ public async Task Delete_PlayerBySquadNumber_NonExisting_Returns404NotFound()
461498
// Assert
462499
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
463500
service.Verify(service => service.DeleteAsync(It.IsAny<int>()), Times.Never);
464-
if (result is NotFound httpResult)
501+
if (result is ProblemHttpResult httpResult)
465502
{
466-
httpResult.Should().NotBeNull().And.BeOfType<NotFound>();
503+
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
467504
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
468505
}
469506
}

0 commit comments

Comments
 (0)