Skip to content

Commit 10df7c5

Browse files
nanotaboadaCopilotclaude
committed
refactor(api): enforce Problem Details and strengthen test assertions (#418)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6edb907 commit 10df7c5

2 files changed

Lines changed: 61 additions & 90 deletions

File tree

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

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ IValidator<PlayerRequestModel> validator
3131
[Consumes(MediaTypeNames.Application.Json)]
3232
[ProducesResponseType<PlayerResponseModel>(StatusCodes.Status201Created)]
3333
[ProducesResponseType<ProblemDetails>(StatusCodes.Status400BadRequest)]
34-
[ProducesResponseType(StatusCodes.Status409Conflict)]
34+
[ProducesResponseType<ProblemDetails>(StatusCodes.Status409Conflict)]
3535
public async Task<IResult> PostAsync([FromBody] PlayerRequestModel player)
3636
{
3737
var validation = await validator.ValidateAsync(player);
@@ -56,7 +56,16 @@ public async Task<IResult> PostAsync([FromBody] PlayerRequestModel player)
5656
"POST /players failed: Player with Squad Number {SquadNumber} already exists",
5757
player.SquadNumber
5858
);
59-
return TypedResults.Conflict();
59+
return TypedResults.Conflict(
60+
new ProblemDetails
61+
{
62+
Type = "https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409",
63+
Title = "Conflict",
64+
Status = StatusCodes.Status409Conflict,
65+
Detail = $"Player with Squad Number '{player.SquadNumber}' already exists.",
66+
Instance = HttpContext?.Request?.Path.ToString()
67+
}
68+
);
6069
}
6170

6271
var result = await playerService.CreateAsync(player);
@@ -206,16 +215,6 @@ [FromBody] PlayerRequestModel player
206215
instance: HttpContext?.Request?.Path.ToString()
207216
);
208217
}
209-
if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null)
210-
{
211-
logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber);
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-
);
218-
}
219218
if (player.SquadNumber != squadNumber)
220219
{
221220
logger.LogWarning(
@@ -230,6 +229,16 @@ [FromBody] PlayerRequestModel player
230229
instance: HttpContext?.Request?.Path.ToString()
231230
);
232231
}
232+
if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null)
233+
{
234+
logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber);
235+
return TypedResults.Problem(
236+
statusCode: StatusCodes.Status404NotFound,
237+
title: "Not Found",
238+
detail: $"Player with Squad Number '{squadNumber}' was not found.",
239+
instance: HttpContext?.Request?.Path.ToString()
240+
);
241+
}
233242
await playerService.UpdateAsync(player);
234243
// Sanitize user-provided player data before logging to prevent log forging
235244
var sanitizedPlayerString = player

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

Lines changed: 40 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using FluentValidation.Results;
66
using Microsoft.AspNetCore.Http;
77
using Microsoft.AspNetCore.Http.HttpResults;
8+
using Microsoft.AspNetCore.Mvc;
89
using Moq;
910

1011
namespace Dotnet.Samples.AspNetCore.WebApi.Tests.Unit;
@@ -56,11 +57,8 @@ public async Task Post_Players_ValidationError_Returns400BadRequest()
5657
),
5758
Times.Once
5859
);
59-
if (result is ValidationProblem httpResult)
60-
{
61-
httpResult.Should().NotBeNull().And.BeOfType<ValidationProblem>();
62-
httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest);
63-
}
60+
var httpResult = result.Should().BeOfType<ValidationProblem>().Subject;
61+
httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest);
6462
}
6563

6664
[Fact]
@@ -99,11 +97,8 @@ public async Task Post_Players_Existing_Returns409Conflict()
9997
),
10098
Times.Once
10199
);
102-
if (result is Conflict httpResult)
103-
{
104-
httpResult.Should().NotBeNull().And.BeOfType<Conflict>();
105-
httpResult.StatusCode.Should().Be(StatusCodes.Status409Conflict);
106-
}
100+
var httpResult = result.Should().BeOfType<Conflict<ProblemDetails>>().Subject;
101+
httpResult.StatusCode.Should().Be(StatusCodes.Status409Conflict);
107102
}
108103

109104
[Fact]
@@ -174,13 +169,10 @@ public async Task Get_Players_Existing_ReturnsPlayers()
174169

175170
// Assert
176171
service.Verify(service => service.RetrieveAsync(), Times.Once);
177-
if (result is Ok<List<PlayerResponseModel>> httpResult)
178-
{
179-
httpResult.Should().NotBeNull().And.BeOfType<Ok<List<PlayerResponseModel>>>();
180-
httpResult.StatusCode.Should().Be(StatusCodes.Status200OK);
181-
httpResult.Value.Should().NotBeNull().And.BeOfType<List<PlayerResponseModel>>();
182-
httpResult.Value.Should().BeEquivalentTo(response);
183-
}
172+
var httpResult = result.Should().BeOfType<Ok<List<PlayerResponseModel>>>().Subject;
173+
httpResult.StatusCode.Should().Be(StatusCodes.Status200OK);
174+
httpResult.Value.Should().NotBeNull().And.BeOfType<List<PlayerResponseModel>>();
175+
httpResult.Value.Should().BeEquivalentTo(response);
184176
}
185177

186178
[Fact]
@@ -198,11 +190,8 @@ public async Task Get_Players_NonExisting_Returns404NotFound()
198190

199191
// Assert
200192
service.Verify(service => service.RetrieveAsync(), Times.Once);
201-
if (result is ProblemHttpResult httpResult)
202-
{
203-
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
204-
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
205-
}
193+
var httpResult = result.Should().BeOfType<ProblemHttpResult>().Subject;
194+
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
206195
}
207196

208197
[Fact]
@@ -223,11 +212,8 @@ public async Task Get_PlayerById_NonExisting_Returns404NotFound()
223212

224213
// Assert
225214
service.Verify(service => service.RetrieveByIdAsync(It.IsAny<Guid>()), Times.Once);
226-
if (result is ProblemHttpResult httpResult)
227-
{
228-
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
229-
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
230-
}
215+
var httpResult = result.Should().BeOfType<ProblemHttpResult>().Subject;
216+
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
231217
}
232218

233219
[Fact]
@@ -247,13 +233,10 @@ public async Task Get_PlayerById_Existing_Returns200OK()
247233

248234
// Assert
249235
service.Verify(service => service.RetrieveByIdAsync(It.IsAny<Guid>()), Times.Once);
250-
if (result is Ok<PlayerResponseModel> httpResult)
251-
{
252-
httpResult.Should().NotBeNull().And.BeOfType<Ok<PlayerResponseModel>>();
253-
httpResult.StatusCode.Should().Be(StatusCodes.Status200OK);
254-
httpResult.Value.Should().NotBeNull().And.BeOfType<PlayerResponseModel>();
255-
httpResult.Value.Should().BeEquivalentTo(response);
256-
}
236+
var httpResult = result.Should().BeOfType<Ok<PlayerResponseModel>>().Subject;
237+
httpResult.StatusCode.Should().Be(StatusCodes.Status200OK);
238+
httpResult.Value.Should().NotBeNull().And.BeOfType<PlayerResponseModel>();
239+
httpResult.Value.Should().BeEquivalentTo(response);
257240
}
258241

259242
[Fact]
@@ -274,11 +257,8 @@ public async Task Get_PlayerBySquadNumber_NonExisting_Returns404NotFound()
274257

275258
// Assert
276259
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
277-
if (result is ProblemHttpResult httpResult)
278-
{
279-
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
280-
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
281-
}
260+
var httpResult = result.Should().BeOfType<ProblemHttpResult>().Subject;
261+
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
282262
}
283263

284264
[Fact]
@@ -300,13 +280,10 @@ public async Task Get_PlayerBySquadNumber_Existing_Returns200OK()
300280

301281
// Assert
302282
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
303-
if (result is Ok<PlayerResponseModel> httpResult)
304-
{
305-
httpResult.Should().NotBeNull().And.BeOfType<Ok<PlayerResponseModel>>();
306-
httpResult.StatusCode.Should().Be(StatusCodes.Status200OK);
307-
httpResult.Value.Should().NotBeNull().And.BeOfType<PlayerResponseModel>();
308-
httpResult.Value.Should().BeEquivalentTo(response);
309-
}
283+
var httpResult = result.Should().BeOfType<Ok<PlayerResponseModel>>().Subject;
284+
httpResult.StatusCode.Should().Be(StatusCodes.Status200OK);
285+
httpResult.Value.Should().NotBeNull().And.BeOfType<PlayerResponseModel>();
286+
httpResult.Value.Should().BeEquivalentTo(response);
310287
}
311288

312289
/* -------------------------------------------------------------------------
@@ -350,11 +327,8 @@ public async Task Put_PlayerBySquadNumber_ValidationError_Returns400BadRequest()
350327
),
351328
Times.Once
352329
);
353-
if (result is ValidationProblem httpResult)
354-
{
355-
httpResult.Should().NotBeNull().And.BeOfType<ValidationProblem>();
356-
httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest);
357-
}
330+
var httpResult = result.Should().BeOfType<ValidationProblem>().Subject;
331+
httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest);
358332
}
359333

360334
[Fact]
@@ -379,7 +353,10 @@ public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound()
379353
var controller = new PlayerController(service.Object, logger.Object, validator.Object);
380354

381355
// Act
382-
var result = await controller.PutAsync(squadNumber, It.IsAny<PlayerRequestModel>());
356+
var result = await controller.PutAsync(
357+
squadNumber,
358+
new PlayerRequestModel { SquadNumber = squadNumber }
359+
);
383360

384361
// Assert
385362
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
@@ -392,11 +369,8 @@ public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound()
392369
),
393370
Times.Once
394371
);
395-
if (result is ProblemHttpResult httpResult)
396-
{
397-
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
398-
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
399-
}
372+
var httpResult = result.Should().BeOfType<ProblemHttpResult>().Subject;
373+
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
400374
}
401375

402376
[Fact]
@@ -427,13 +401,10 @@ public async Task Put_PlayerBySquadNumber_SquadNumberMismatch_Returns400BadReque
427401
var result = await controller.PutAsync(squadNumber, request);
428402

429403
// Assert
430-
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
404+
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Never);
431405
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-
}
406+
var httpResult = result.Should().BeOfType<ProblemHttpResult>().Subject;
407+
httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest);
437408
}
438409

439410
[Fact]
@@ -468,11 +439,8 @@ public async Task Put_PlayerBySquadNumber_Existing_Returns204NoContent()
468439
// Assert
469440
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
470441
service.Verify(service => service.UpdateAsync(It.IsAny<PlayerRequestModel>()), Times.Once);
471-
if (result is NoContent httpResult)
472-
{
473-
httpResult.Should().NotBeNull().And.BeOfType<NoContent>();
474-
httpResult.StatusCode.Should().Be(StatusCodes.Status204NoContent);
475-
}
442+
var httpResult = result.Should().BeOfType<NoContent>().Subject;
443+
httpResult.StatusCode.Should().Be(StatusCodes.Status204NoContent);
476444
}
477445

478446
/* -------------------------------------------------------------------------
@@ -498,11 +466,8 @@ public async Task Delete_PlayerBySquadNumber_NonExisting_Returns404NotFound()
498466
// Assert
499467
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
500468
service.Verify(service => service.DeleteAsync(It.IsAny<int>()), Times.Never);
501-
if (result is ProblemHttpResult httpResult)
502-
{
503-
httpResult.Should().NotBeNull().And.BeOfType<ProblemHttpResult>();
504-
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
505-
}
469+
var httpResult = result.Should().BeOfType<ProblemHttpResult>().Subject;
470+
httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound);
506471
}
507472

508473
[Fact]
@@ -526,11 +491,8 @@ public async Task Delete_PlayerBySquadNumber_Existing_Returns204NoContent()
526491
// Assert
527492
service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny<int>()), Times.Once);
528493
service.Verify(service => service.DeleteAsync(It.IsAny<int>()), Times.Once);
529-
if (result is NoContent httpResult)
530-
{
531-
httpResult.Should().NotBeNull().And.BeOfType<NoContent>();
532-
httpResult.StatusCode.Should().Be(StatusCodes.Status204NoContent);
533-
}
494+
var httpResult = result.Should().BeOfType<NoContent>().Subject;
495+
httpResult.StatusCode.Should().Be(StatusCodes.Status204NoContent);
534496
}
535497

536498
protected virtual void Dispose(bool disposing)

0 commit comments

Comments
 (0)