From 416a0040370fe173d6187c10ff0d73853a7aee41 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Tue, 17 Mar 2026 22:14:13 -0300 Subject: [PATCH 1/6] refactor(api)!: add /squadNumber/ path segment to routes (#418) BREAKING CHANGE: old /players/{int} routes return 404; use /players/squadNumber/{int} instead. Co-authored-by: Claude --- README.md | 6 ++--- .../Controllers/PlayerController.cs | 24 +++++++++++-------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index ae5c85e..27c2ba0 100644 --- a/README.md +++ b/README.md @@ -218,10 +218,10 @@ Interactive API documentation is available via Swagger UI at `https://localhost: - `GET /players` - List all players - `GET /players/{id:Guid}` - Get player by ID (requires authentication) -- `GET /players/{squadNumber:int}` - Get player by squad number +- `GET /players/squadNumber/{squadNumber:int}` - Get player by squad number - `POST /players` - Create new player -- `PUT /players/{squadNumber}` - Update player -- `DELETE /players/{squadNumber}` - Remove player +- `PUT /players/squadNumber/{squadNumber}` - Update player +- `DELETE /players/squadNumber/{squadNumber}` - Remove player - `GET /health` - Health check For complete endpoint documentation with request/response schemas, explore the [interactive Swagger UI](https://localhost:9000/swagger/index.html). diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index 3a566c9..3aaf545 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -124,7 +124,7 @@ public async Task GetByIdAsync([FromRoute] Guid id) /// The Squad Number of the Player /// OK /// Not Found - [HttpGet("{squadNumber:int}", Name = "RetrieveBySquadNumber")] + [HttpGet("squadNumber/{squadNumber:int}", Name = "RetrieveBySquadNumber")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) @@ -133,7 +133,7 @@ public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) if (player != null) { logger.LogInformation( - "GET /players/{SquadNumber} retrieved: {@Player}", + "GET /players/squadNumber/{SquadNumber} retrieved: {@Player}", squadNumber, player ); @@ -141,7 +141,7 @@ public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) } else { - logger.LogWarning("GET /players/{SquadNumber} not found", squadNumber); + logger.LogWarning("GET /players/squadNumber/{SquadNumber} not found", squadNumber); return TypedResults.NotFound(); } } @@ -159,7 +159,7 @@ public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) /// No Content /// Bad Request /// Not Found - [HttpPut("{squadNumber:int}", Name = "Update")] + [HttpPut("squadNumber/{squadNumber:int}", Name = "Update")] [Consumes(MediaTypeNames.Application.Json)] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status400BadRequest)] @@ -177,7 +177,7 @@ [FromBody] PlayerRequestModel player .ToArray(); logger.LogWarning( - "PUT /players/{SquadNumber} validation failed: {@Errors}", + "PUT /players/squadNumber/{SquadNumber} validation failed: {@Errors}", squadNumber, errors ); @@ -185,12 +185,16 @@ [FromBody] PlayerRequestModel player } if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null) { - logger.LogWarning("PUT /players/{SquadNumber} not found", squadNumber); + logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber); return TypedResults.NotFound(); } await playerService.UpdateAsync(player); // codeql[cs/log-forging] Serilog structured logging with @ destructuring automatically escapes control characters - logger.LogInformation("PUT /players/{SquadNumber} updated: {@Player}", squadNumber, player); + logger.LogInformation( + "PUT /players/squadNumber/{SquadNumber} updated: {@Player}", + squadNumber, + player + ); return TypedResults.NoContent(); } @@ -204,20 +208,20 @@ [FromBody] PlayerRequestModel player /// The Squad Number of the Player /// No Content /// Not Found - [HttpDelete("{squadNumber:int}", Name = "Delete")] + [HttpDelete("squadNumber/{squadNumber:int}", Name = "Delete")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task DeleteAsync([FromRoute] int squadNumber) { if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null) { - logger.LogWarning("DELETE /players/{SquadNumber} not found", squadNumber); + logger.LogWarning("DELETE /players/squadNumber/{SquadNumber} not found", squadNumber); return TypedResults.NotFound(); } else { await playerService.DeleteAsync(squadNumber); - logger.LogInformation("DELETE /players/{SquadNumber} deleted", squadNumber); + logger.LogInformation("DELETE /players/squadNumber/{SquadNumber} deleted", squadNumber); return TypedResults.NoContent(); } } From 08455c9a2c20e206c79e725da90e7c5e353eb223 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Tue, 17 Mar 2026 22:46:39 -0300 Subject: [PATCH 2/6] fix(security): sanitize player data before logging to prevent log forging - Replace Serilog @-destructuring with explicit newline stripping on the player's string representation to address CodeQL alert #384 (CWE-117). Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../Controllers/PlayerController.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index 3aaf545..2cefcf1 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -189,11 +189,16 @@ [FromBody] PlayerRequestModel player return TypedResults.NotFound(); } await playerService.UpdateAsync(player); - // codeql[cs/log-forging] Serilog structured logging with @ destructuring automatically escapes control characters + // Sanitize user-provided player data before logging to prevent log forging + var sanitizedPlayerString = player? + .ToString()? + .Replace(Environment.NewLine, string.Empty) + .Replace("\r", string.Empty) + .Replace("\n", string.Empty); logger.LogInformation( - "PUT /players/squadNumber/{SquadNumber} updated: {@Player}", + "PUT /players/squadNumber/{SquadNumber} updated: {Player}", squadNumber, - player + sanitizedPlayerString ); return TypedResults.NoContent(); } From 26d0de6d0d3a708ad36545aa6c513888f7069409 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Tue, 17 Mar 2026 23:01:38 -0300 Subject: [PATCH 3/6] fix(api): add squad number mismatch guard and update README (#418) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude --- README.md | 4 ++-- .../Controllers/PlayerController.cs | 20 ++++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 27c2ba0..3af7aa5 100644 --- a/README.md +++ b/README.md @@ -220,8 +220,8 @@ Interactive API documentation is available via Swagger UI at `https://localhost: - `GET /players/{id:Guid}` - Get player by ID (requires authentication) - `GET /players/squadNumber/{squadNumber:int}` - Get player by squad number - `POST /players` - Create new player -- `PUT /players/squadNumber/{squadNumber}` - Update player -- `DELETE /players/squadNumber/{squadNumber}` - Remove player +- `PUT /players/squadNumber/{squadNumber:int}` - Update player +- `DELETE /players/squadNumber/{squadNumber:int}` - Remove player - `GET /health` - Health check For complete endpoint documentation with request/response schemas, explore the [interactive Swagger UI](https://localhost:9000/swagger/index.html). diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index 2cefcf1..67c8317 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -188,11 +188,25 @@ [FromBody] PlayerRequestModel player logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber); return TypedResults.NotFound(); } + if (player.SquadNumber != squadNumber) + { + logger.LogWarning( + "PutAsync squad number mismatch: route {SquadNumber} != body {PlayerSquadNumber}", + squadNumber, + player.SquadNumber + ); + return TypedResults.BadRequest( + new + { + Error = "Squad number in the route does not match squad number in the request body." + } + ); + } await playerService.UpdateAsync(player); // Sanitize user-provided player data before logging to prevent log forging - var sanitizedPlayerString = player? - .ToString()? - .Replace(Environment.NewLine, string.Empty) + var sanitizedPlayerString = player + ?.ToString() + ?.Replace(Environment.NewLine, string.Empty) .Replace("\r", string.Empty) .Replace("\n", string.Empty); logger.LogInformation( From 6edb90713c19eb0835a1436aa17b7c12a6d6812b Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Wed, 18 Mar 2026 00:57:51 -0300 Subject: [PATCH 4/6] 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 --- .../Controllers/PlayerController.cs | 79 +++++++++++++------ .../Unit/PlayerControllerTests.cs | 65 +++++++++++---- 2 files changed, 107 insertions(+), 37 deletions(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index 67c8317..fe35be7 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -30,7 +30,7 @@ IValidator validator [HttpPost(Name = "Create")] [Consumes(MediaTypeNames.Application.Json)] [ProducesResponseType(StatusCodes.Status201Created)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status409Conflict)] public async Task PostAsync([FromBody] PlayerRequestModel player) { @@ -39,11 +39,15 @@ public async Task PostAsync([FromBody] PlayerRequestModel player) if (!validation.IsValid) { var errors = validation - .Errors.Select(error => new { error.PropertyName, error.ErrorMessage }) - .ToArray(); + .Errors.GroupBy(e => e.PropertyName) + .ToDictionary(g => g.Key, g => g.Select(e => e.ErrorMessage).ToArray()); logger.LogWarning("POST /players validation failed: {@Errors}", errors); - return TypedResults.BadRequest(errors); + return TypedResults.ValidationProblem( + errors, + detail: "See the errors field for details.", + instance: HttpContext?.Request?.Path.ToString() + ); } if (await playerService.RetrieveBySquadNumberAsync(player.SquadNumber) != null) @@ -76,7 +80,7 @@ public async Task PostAsync([FromBody] PlayerRequestModel player) /// Not Found [HttpGet(Name = "Retrieve")] [ProducesResponseType(StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetAsync() { var players = await playerService.RetrieveAsync(); @@ -89,7 +93,12 @@ public async Task GetAsync() else { logger.LogWarning("GET /players not found"); - return TypedResults.NotFound(); + return TypedResults.Problem( + statusCode: StatusCodes.Status404NotFound, + title: "Not Found", + detail: "No players were found.", + instance: HttpContext?.Request?.Path.ToString() + ); } } @@ -102,7 +111,7 @@ public async Task GetAsync() [Authorize] [HttpGet("{id:Guid}", Name = "RetrieveById")] [ProducesResponseType(StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetByIdAsync([FromRoute] Guid id) { var player = await playerService.RetrieveByIdAsync(id); @@ -114,7 +123,12 @@ public async Task GetByIdAsync([FromRoute] Guid id) else { logger.LogWarning("GET /players/{Id} not found", id); - return TypedResults.NotFound(); + return TypedResults.Problem( + statusCode: StatusCodes.Status404NotFound, + title: "Not Found", + detail: $"Player with Id '{id}' was not found.", + instance: HttpContext?.Request?.Path.ToString() + ); } } @@ -126,7 +140,7 @@ public async Task GetByIdAsync([FromRoute] Guid id) /// Not Found [HttpGet("squadNumber/{squadNumber:int}", Name = "RetrieveBySquadNumber")] [ProducesResponseType(StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) { var player = await playerService.RetrieveBySquadNumberAsync(squadNumber); @@ -142,7 +156,12 @@ public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) else { logger.LogWarning("GET /players/squadNumber/{SquadNumber} not found", squadNumber); - return TypedResults.NotFound(); + return TypedResults.Problem( + statusCode: StatusCodes.Status404NotFound, + title: "Not Found", + detail: $"Player with Squad Number '{squadNumber}' was not found.", + instance: HttpContext?.Request?.Path.ToString() + ); } } @@ -162,8 +181,8 @@ public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) [HttpPut("squadNumber/{squadNumber:int}", Name = "Update")] [Consumes(MediaTypeNames.Application.Json)] [ProducesResponseType(StatusCodes.Status204NoContent)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task PutAsync( [FromRoute] int squadNumber, [FromBody] PlayerRequestModel player @@ -173,20 +192,29 @@ [FromBody] PlayerRequestModel player if (!validation.IsValid) { var errors = validation - .Errors.Select(error => new { error.PropertyName, error.ErrorMessage }) - .ToArray(); + .Errors.GroupBy(e => e.PropertyName) + .ToDictionary(g => g.Key, g => g.Select(e => e.ErrorMessage).ToArray()); logger.LogWarning( "PUT /players/squadNumber/{SquadNumber} validation failed: {@Errors}", squadNumber, errors ); - return TypedResults.BadRequest(errors); + return TypedResults.ValidationProblem( + errors, + detail: "See the errors field for details.", + instance: HttpContext?.Request?.Path.ToString() + ); } if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null) { logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber); - return TypedResults.NotFound(); + return TypedResults.Problem( + statusCode: StatusCodes.Status404NotFound, + title: "Not Found", + detail: $"Player with Squad Number '{squadNumber}' was not found.", + instance: HttpContext?.Request?.Path.ToString() + ); } if (player.SquadNumber != squadNumber) { @@ -195,11 +223,11 @@ [FromBody] PlayerRequestModel player squadNumber, player.SquadNumber ); - return TypedResults.BadRequest( - new - { - Error = "Squad number in the route does not match squad number in the request body." - } + return TypedResults.Problem( + statusCode: StatusCodes.Status400BadRequest, + title: "Bad Request", + detail: "Squad number in the route does not match squad number in the request body.", + instance: HttpContext?.Request?.Path.ToString() ); } await playerService.UpdateAsync(player); @@ -229,13 +257,18 @@ [FromBody] PlayerRequestModel player /// Not Found [HttpDelete("squadNumber/{squadNumber:int}", Name = "Delete")] [ProducesResponseType(StatusCodes.Status204NoContent)] - [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task DeleteAsync([FromRoute] int squadNumber) { if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null) { logger.LogWarning("DELETE /players/squadNumber/{SquadNumber} not found", squadNumber); - return TypedResults.NotFound(); + return TypedResults.Problem( + statusCode: StatusCodes.Status404NotFound, + title: "Not Found", + detail: $"Player with Squad Number '{squadNumber}' was not found.", + instance: HttpContext?.Request?.Path.ToString() + ); } else { diff --git a/test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs b/test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs index 2532861..8fc92db 100644 --- a/test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs +++ b/test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs @@ -56,9 +56,9 @@ public async Task Post_Players_ValidationError_Returns400BadRequest() ), Times.Once ); - if (result is BadRequest httpResult) + if (result is ValidationProblem httpResult) { - httpResult.Should().NotBeNull().And.BeOfType(); + httpResult.Should().NotBeNull().And.BeOfType(); httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); } } @@ -198,9 +198,9 @@ public async Task Get_Players_NonExisting_Returns404NotFound() // Assert service.Verify(service => service.RetrieveAsync(), Times.Once); - if (result is NotFound httpResult) + if (result is ProblemHttpResult httpResult) { - httpResult.Should().NotBeNull().And.BeOfType(); + httpResult.Should().NotBeNull().And.BeOfType(); httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } } @@ -223,9 +223,9 @@ public async Task Get_PlayerById_NonExisting_Returns404NotFound() // Assert service.Verify(service => service.RetrieveByIdAsync(It.IsAny()), Times.Once); - if (result is NotFound httpResult) + if (result is ProblemHttpResult httpResult) { - httpResult.Should().NotBeNull().And.BeOfType(); + httpResult.Should().NotBeNull().And.BeOfType(); httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } } @@ -274,9 +274,9 @@ public async Task Get_PlayerBySquadNumber_NonExisting_Returns404NotFound() // Assert service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); - if (result is NotFound httpResult) + if (result is ProblemHttpResult httpResult) { - httpResult.Should().NotBeNull().And.BeOfType(); + httpResult.Should().NotBeNull().And.BeOfType(); httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } } @@ -350,9 +350,9 @@ public async Task Put_PlayerBySquadNumber_ValidationError_Returns400BadRequest() ), Times.Once ); - if (result is BadRequest httpResult) + if (result is ValidationProblem httpResult) { - httpResult.Should().NotBeNull().And.BeOfType(); + httpResult.Should().NotBeNull().And.BeOfType(); httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); } } @@ -392,13 +392,50 @@ public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound() ), Times.Once ); - if (result is NotFound httpResult) + if (result is ProblemHttpResult httpResult) { - httpResult.Should().NotBeNull().And.BeOfType(); + httpResult.Should().NotBeNull().And.BeOfType(); httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } } + [Fact] + [Trait("Category", "Unit")] + public async Task Put_PlayerBySquadNumber_SquadNumberMismatch_Returns400BadRequest() + { + // Arrange + var squadNumber = 23; + var request = PlayerFakes.MakeRequestModelForUpdate(squadNumber); + request.SquadNumber = 99; // Mismatched squad number + var response = PlayerFakes.MakeResponseModelForUpdate(squadNumber); + var (service, logger, validator) = PlayerMocks.InitControllerMocks(); + service + .Setup(service => service.RetrieveBySquadNumberAsync(squadNumber)) + .ReturnsAsync(response); + validator + .Setup(validator => + validator.ValidateAsync( + It.IsAny(), + It.IsAny() + ) + ) + .ReturnsAsync(new ValidationResult(new List { })); + + var controller = new PlayerController(service.Object, logger.Object, validator.Object); + + // Act + var result = await controller.PutAsync(squadNumber, request); + + // Assert + service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); + service.Verify(service => service.UpdateAsync(It.IsAny()), Times.Never); + if (result is ProblemHttpResult httpResult) + { + httpResult.Should().NotBeNull().And.BeOfType(); + httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); + } + } + [Fact] [Trait("Category", "Unit")] public async Task Put_PlayerBySquadNumber_Existing_Returns204NoContent() @@ -461,9 +498,9 @@ public async Task Delete_PlayerBySquadNumber_NonExisting_Returns404NotFound() // Assert service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); service.Verify(service => service.DeleteAsync(It.IsAny()), Times.Never); - if (result is NotFound httpResult) + if (result is ProblemHttpResult httpResult) { - httpResult.Should().NotBeNull().And.BeOfType(); + httpResult.Should().NotBeNull().And.BeOfType(); httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } } From 10df7c5a702791c812f411204e04f86329f4e099 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Wed, 18 Mar 2026 01:15:48 -0300 Subject: [PATCH 5/6] refactor(api): enforce Problem Details and strengthen test assertions (#418) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude --- .../Controllers/PlayerController.cs | 33 +++-- .../Unit/PlayerControllerTests.cs | 118 ++++++------------ 2 files changed, 61 insertions(+), 90 deletions(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index fe35be7..84066ea 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -31,7 +31,7 @@ IValidator validator [Consumes(MediaTypeNames.Application.Json)] [ProducesResponseType(StatusCodes.Status201Created)] [ProducesResponseType(StatusCodes.Status400BadRequest)] - [ProducesResponseType(StatusCodes.Status409Conflict)] + [ProducesResponseType(StatusCodes.Status409Conflict)] public async Task PostAsync([FromBody] PlayerRequestModel player) { var validation = await validator.ValidateAsync(player); @@ -56,7 +56,16 @@ public async Task PostAsync([FromBody] PlayerRequestModel player) "POST /players failed: Player with Squad Number {SquadNumber} already exists", player.SquadNumber ); - return TypedResults.Conflict(); + return TypedResults.Conflict( + new ProblemDetails + { + Type = "https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409", + Title = "Conflict", + Status = StatusCodes.Status409Conflict, + Detail = $"Player with Squad Number '{player.SquadNumber}' already exists.", + Instance = HttpContext?.Request?.Path.ToString() + } + ); } var result = await playerService.CreateAsync(player); @@ -206,16 +215,6 @@ [FromBody] PlayerRequestModel player instance: HttpContext?.Request?.Path.ToString() ); } - if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null) - { - logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber); - return TypedResults.Problem( - statusCode: StatusCodes.Status404NotFound, - title: "Not Found", - detail: $"Player with Squad Number '{squadNumber}' was not found.", - instance: HttpContext?.Request?.Path.ToString() - ); - } if (player.SquadNumber != squadNumber) { logger.LogWarning( @@ -230,6 +229,16 @@ [FromBody] PlayerRequestModel player instance: HttpContext?.Request?.Path.ToString() ); } + if (await playerService.RetrieveBySquadNumberAsync(squadNumber) == null) + { + logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber); + return TypedResults.Problem( + statusCode: StatusCodes.Status404NotFound, + title: "Not Found", + detail: $"Player with Squad Number '{squadNumber}' was not found.", + instance: HttpContext?.Request?.Path.ToString() + ); + } await playerService.UpdateAsync(player); // Sanitize user-provided player data before logging to prevent log forging var sanitizedPlayerString = player diff --git a/test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs b/test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs index 8fc92db..38af79b 100644 --- a/test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs +++ b/test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs @@ -5,6 +5,7 @@ using FluentValidation.Results; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.HttpResults; +using Microsoft.AspNetCore.Mvc; using Moq; namespace Dotnet.Samples.AspNetCore.WebApi.Tests.Unit; @@ -56,11 +57,8 @@ public async Task Post_Players_ValidationError_Returns400BadRequest() ), Times.Once ); - if (result is ValidationProblem httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); } [Fact] @@ -99,11 +97,8 @@ public async Task Post_Players_Existing_Returns409Conflict() ), Times.Once ); - if (result is Conflict httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status409Conflict); - } + var httpResult = result.Should().BeOfType>().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status409Conflict); } [Fact] @@ -174,13 +169,10 @@ public async Task Get_Players_Existing_ReturnsPlayers() // Assert service.Verify(service => service.RetrieveAsync(), Times.Once); - if (result is Ok> httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType>>(); - httpResult.StatusCode.Should().Be(StatusCodes.Status200OK); - httpResult.Value.Should().NotBeNull().And.BeOfType>(); - httpResult.Value.Should().BeEquivalentTo(response); - } + var httpResult = result.Should().BeOfType>>().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status200OK); + httpResult.Value.Should().NotBeNull().And.BeOfType>(); + httpResult.Value.Should().BeEquivalentTo(response); } [Fact] @@ -198,11 +190,8 @@ public async Task Get_Players_NonExisting_Returns404NotFound() // Assert service.Verify(service => service.RetrieveAsync(), Times.Once); - if (result is ProblemHttpResult httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } [Fact] @@ -223,11 +212,8 @@ public async Task Get_PlayerById_NonExisting_Returns404NotFound() // Assert service.Verify(service => service.RetrieveByIdAsync(It.IsAny()), Times.Once); - if (result is ProblemHttpResult httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } [Fact] @@ -247,13 +233,10 @@ public async Task Get_PlayerById_Existing_Returns200OK() // Assert service.Verify(service => service.RetrieveByIdAsync(It.IsAny()), Times.Once); - if (result is Ok httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType>(); - httpResult.StatusCode.Should().Be(StatusCodes.Status200OK); - httpResult.Value.Should().NotBeNull().And.BeOfType(); - httpResult.Value.Should().BeEquivalentTo(response); - } + var httpResult = result.Should().BeOfType>().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status200OK); + httpResult.Value.Should().NotBeNull().And.BeOfType(); + httpResult.Value.Should().BeEquivalentTo(response); } [Fact] @@ -274,11 +257,8 @@ public async Task Get_PlayerBySquadNumber_NonExisting_Returns404NotFound() // Assert service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); - if (result is ProblemHttpResult httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } [Fact] @@ -300,13 +280,10 @@ public async Task Get_PlayerBySquadNumber_Existing_Returns200OK() // Assert service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); - if (result is Ok httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType>(); - httpResult.StatusCode.Should().Be(StatusCodes.Status200OK); - httpResult.Value.Should().NotBeNull().And.BeOfType(); - httpResult.Value.Should().BeEquivalentTo(response); - } + var httpResult = result.Should().BeOfType>().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status200OK); + httpResult.Value.Should().NotBeNull().And.BeOfType(); + httpResult.Value.Should().BeEquivalentTo(response); } /* ------------------------------------------------------------------------- @@ -350,11 +327,8 @@ public async Task Put_PlayerBySquadNumber_ValidationError_Returns400BadRequest() ), Times.Once ); - if (result is ValidationProblem httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); } [Fact] @@ -379,7 +353,10 @@ public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound() var controller = new PlayerController(service.Object, logger.Object, validator.Object); // Act - var result = await controller.PutAsync(squadNumber, It.IsAny()); + var result = await controller.PutAsync( + squadNumber, + new PlayerRequestModel { SquadNumber = squadNumber } + ); // Assert service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); @@ -392,11 +369,8 @@ public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound() ), Times.Once ); - if (result is ProblemHttpResult httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } [Fact] @@ -427,13 +401,10 @@ public async Task Put_PlayerBySquadNumber_SquadNumberMismatch_Returns400BadReque var result = await controller.PutAsync(squadNumber, request); // Assert - service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); + service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Never); service.Verify(service => service.UpdateAsync(It.IsAny()), Times.Never); - if (result is ProblemHttpResult httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status400BadRequest); } [Fact] @@ -468,11 +439,8 @@ public async Task Put_PlayerBySquadNumber_Existing_Returns204NoContent() // Assert service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); service.Verify(service => service.UpdateAsync(It.IsAny()), Times.Once); - if (result is NoContent httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status204NoContent); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status204NoContent); } /* ------------------------------------------------------------------------- @@ -498,11 +466,8 @@ public async Task Delete_PlayerBySquadNumber_NonExisting_Returns404NotFound() // Assert service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); service.Verify(service => service.DeleteAsync(It.IsAny()), Times.Never); - if (result is ProblemHttpResult httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status404NotFound); } [Fact] @@ -526,11 +491,8 @@ public async Task Delete_PlayerBySquadNumber_Existing_Returns204NoContent() // Assert service.Verify(service => service.RetrieveBySquadNumberAsync(It.IsAny()), Times.Once); service.Verify(service => service.DeleteAsync(It.IsAny()), Times.Once); - if (result is NoContent httpResult) - { - httpResult.Should().NotBeNull().And.BeOfType(); - httpResult.StatusCode.Should().Be(StatusCodes.Status204NoContent); - } + var httpResult = result.Should().BeOfType().Subject; + httpResult.StatusCode.Should().Be(StatusCodes.Status204NoContent); } protected virtual void Dispose(bool disposing) From a18913d0b5f040828858700eca151ac7bacb25a8 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Wed, 18 Mar 2026 01:23:34 -0300 Subject: [PATCH 6/6] refactor(api): extract NotFoundTitle constant and remove redundant ?. (#418) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude --- .../Controllers/PlayerController.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index 84066ea..ada6515 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -16,6 +16,8 @@ public class PlayerController( IValidator validator ) : ControllerBase { + private const string NotFoundTitle = "Not Found"; + /* ------------------------------------------------------------------------- * HTTP POST * ---------------------------------------------------------------------- */ @@ -104,7 +106,7 @@ public async Task GetAsync() logger.LogWarning("GET /players not found"); return TypedResults.Problem( statusCode: StatusCodes.Status404NotFound, - title: "Not Found", + title: NotFoundTitle, detail: "No players were found.", instance: HttpContext?.Request?.Path.ToString() ); @@ -134,7 +136,7 @@ public async Task GetByIdAsync([FromRoute] Guid id) logger.LogWarning("GET /players/{Id} not found", id); return TypedResults.Problem( statusCode: StatusCodes.Status404NotFound, - title: "Not Found", + title: NotFoundTitle, detail: $"Player with Id '{id}' was not found.", instance: HttpContext?.Request?.Path.ToString() ); @@ -167,7 +169,7 @@ public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) logger.LogWarning("GET /players/squadNumber/{SquadNumber} not found", squadNumber); return TypedResults.Problem( statusCode: StatusCodes.Status404NotFound, - title: "Not Found", + title: NotFoundTitle, detail: $"Player with Squad Number '{squadNumber}' was not found.", instance: HttpContext?.Request?.Path.ToString() ); @@ -234,7 +236,7 @@ [FromBody] PlayerRequestModel player logger.LogWarning("PUT /players/squadNumber/{SquadNumber} not found", squadNumber); return TypedResults.Problem( statusCode: StatusCodes.Status404NotFound, - title: "Not Found", + title: NotFoundTitle, detail: $"Player with Squad Number '{squadNumber}' was not found.", instance: HttpContext?.Request?.Path.ToString() ); @@ -242,7 +244,7 @@ [FromBody] PlayerRequestModel player await playerService.UpdateAsync(player); // Sanitize user-provided player data before logging to prevent log forging var sanitizedPlayerString = player - ?.ToString() + .ToString() ?.Replace(Environment.NewLine, string.Empty) .Replace("\r", string.Empty) .Replace("\n", string.Empty);