Skip to content

Commit 7028e73

Browse files
nanotaboadaclaude
nanotaboada
and
claude
committed
fix(validators): scope BeUniqueSquadNumber to \"Create\" rule set (#424)
Organizes all PlayerRequestModelValidator rules into explicit CRUD-named rule sets: \"Create\" (POST, includes BeUniqueSquadNumber) and \"Update\" (PUT, omits it). The controller passes IncludeRuleSets(\"Create\") or IncludeRuleSets(\"Update\") so only the appropriate rules run per operation. Adds ValidateAsync_SquadNumber_BelongsToPlayerBeingUpdated_ReturnsNoErrors as a regression test for the PUT 400 bug. Updates all controller test mocks to match on IValidationContext (the overload the rule-set extension method routes through). Co-authored-by: Claude <noreply@anthropic.com>
1 parent e7af849 commit 7028e73

4 files changed

Lines changed: 207 additions & 53 deletions

File tree

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ IValidator<PlayerRequestModel> validator
3636
[ProducesResponseType<ProblemDetails>(StatusCodes.Status409Conflict)]
3737
public async Task<IResult> PostAsync([FromBody] PlayerRequestModel player)
3838
{
39-
var validation = await validator.ValidateAsync(player);
39+
// Use the "Create" rule set, which includes BeUniqueSquadNumber.
40+
var validation = await validator.ValidateAsync(
41+
player,
42+
options => options.IncludeRuleSets("Create")
43+
);
4044

4145
if (!validation.IsValid)
4246
{
@@ -199,7 +203,13 @@ public async Task<IResult> PutAsync(
199203
[FromBody] PlayerRequestModel player
200204
)
201205
{
202-
var validation = await validator.ValidateAsync(player);
206+
// Use the "Update" rule set, which omits BeUniqueSquadNumber.
207+
// The player being updated already exists in the database, so a
208+
// uniqueness check on its own squad number would always fail.
209+
var validation = await validator.ValidateAsync(
210+
player,
211+
options => options.IncludeRuleSets("Update")
212+
);
203213
if (!validation.IsValid)
204214
{
205215
var errors = validation

‎src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs‎

Lines changed: 94 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,24 @@ namespace Dotnet.Samples.AspNetCore.WebApi.Validators;
1111
/// PlayerRequestModel.
1212
/// </summary>
1313
/// <remarks>
14-
/// This class is part of the FluentValidation library, which provides a fluent
15-
/// interface for building validation rules.
14+
/// Rules are organized into CRUD-named rule sets to make their intent explicit.
15+
/// This prevents <c>BeUniqueSquadNumber</c> from running on PUT requests, where
16+
/// the player's squad number already exists in the database by definition.
17+
///
18+
/// <list type="bullet">
19+
/// <item><description>
20+
/// <c>"Create"</c> — used by <c>POST /players</c>; includes all rules plus
21+
/// the uniqueness check for <c>SquadNumber</c>.
22+
/// </description></item>
23+
/// <item><description>
24+
/// <c>"Update"</c> — used by <c>PUT /players/squadNumber/{n}</c>; same
25+
/// rules, but <c>BeUniqueSquadNumber</c> is intentionally omitted.
26+
/// </description></item>
27+
/// </list>
28+
///
29+
/// Controllers pass <c>opts.IncludeRuleSets("Create")</c> or
30+
/// <c>opts.IncludeRuleSets("Update")</c> so that only the appropriate rule
31+
/// set runs for each operation.
1632
/// </remarks>
1733
public class PlayerRequestModelValidator : AbstractValidator<PlayerRequestModel>
1834
{
@@ -26,35 +42,88 @@ public PlayerRequestModelValidator(
2642
_playerRepository = playerRepository;
2743
var clock = timeProvider ?? TimeProvider.System;
2844

29-
RuleFor(player => player.FirstName).NotEmpty().WithMessage("FirstName is required.");
45+
// "Create" rule set — POST /players
46+
// Includes BeUniqueSquadNumber to prevent duplicate squad numbers on insert.
47+
RuleSet(
48+
"Create",
49+
() =>
50+
{
51+
RuleFor(player => player.FirstName)
52+
.NotEmpty()
53+
.WithMessage("FirstName is required.");
54+
55+
RuleFor(player => player.LastName).NotEmpty().WithMessage("LastName is required.");
3056

31-
RuleFor(player => player.LastName).NotEmpty().WithMessage("LastName is required.");
57+
RuleFor(player => player.SquadNumber)
58+
.NotEmpty()
59+
.WithMessage("SquadNumber is required.")
60+
.GreaterThan(0)
61+
.WithMessage("SquadNumber must be greater than 0.")
62+
.MustAsync(BeUniqueSquadNumber)
63+
.WithMessage("SquadNumber must be unique.");
3264

33-
RuleFor(player => player.SquadNumber)
34-
.NotEmpty()
35-
.WithMessage("SquadNumber is required.")
36-
.GreaterThan(0)
37-
.WithMessage("SquadNumber must be greater than 0.")
38-
.MustAsync(BeUniqueSquadNumber)
39-
.WithMessage("SquadNumber must be unique.");
65+
RuleFor(player => player.AbbrPosition)
66+
.NotEmpty()
67+
.WithMessage("AbbrPosition is required.")
68+
.Must(Position.IsValidAbbr)
69+
.WithMessage("AbbrPosition is invalid.");
4070

41-
RuleFor(player => player.AbbrPosition)
42-
.NotEmpty()
43-
.WithMessage("AbbrPosition is required.")
44-
.Must(Position.IsValidAbbr)
45-
.WithMessage("AbbrPosition is invalid.");
71+
When(
72+
player => player.DateOfBirth.HasValue,
73+
() =>
74+
{
75+
RuleFor(player => player.DateOfBirth)
76+
.Must(date => date!.Value.Date < clock.GetUtcNow().Date)
77+
.WithMessage("DateOfBirth must be a date in the past.")
78+
.Must(date =>
79+
date!.Value.Date
80+
>= new DateTime(1900, 1, 1, 0, 0, 0, DateTimeKind.Utc)
81+
)
82+
.WithMessage("DateOfBirth must be on or after January 1, 1900.");
83+
}
84+
);
85+
}
86+
);
4687

47-
When(
48-
player => player.DateOfBirth.HasValue,
88+
// "Update" rule set — PUT /players/squadNumber/{n}
89+
// BeUniqueSquadNumber is intentionally omitted: on PUT the player being
90+
// updated already exists in the database, so the check would always fail.
91+
RuleSet(
92+
"Update",
4993
() =>
5094
{
51-
RuleFor(player => player.DateOfBirth)
52-
.Must(date => date!.Value.Date < clock.GetUtcNow().Date)
53-
.WithMessage("DateOfBirth must be a date in the past.")
54-
.Must(date =>
55-
date!.Value.Date >= new DateTime(1900, 1, 1, 0, 0, 0, DateTimeKind.Utc)
56-
)
57-
.WithMessage("DateOfBirth must be on or after January 1, 1900.");
95+
RuleFor(player => player.FirstName)
96+
.NotEmpty()
97+
.WithMessage("FirstName is required.");
98+
99+
RuleFor(player => player.LastName).NotEmpty().WithMessage("LastName is required.");
100+
101+
RuleFor(player => player.SquadNumber)
102+
.NotEmpty()
103+
.WithMessage("SquadNumber is required.")
104+
.GreaterThan(0)
105+
.WithMessage("SquadNumber must be greater than 0.");
106+
107+
RuleFor(player => player.AbbrPosition)
108+
.NotEmpty()
109+
.WithMessage("AbbrPosition is required.")
110+
.Must(Position.IsValidAbbr)
111+
.WithMessage("AbbrPosition is invalid.");
112+
113+
When(
114+
player => player.DateOfBirth.HasValue,
115+
() =>
116+
{
117+
RuleFor(player => player.DateOfBirth)
118+
.Must(date => date!.Value.Date < clock.GetUtcNow().Date)
119+
.WithMessage("DateOfBirth must be a date in the past.")
120+
.Must(date =>
121+
date!.Value.Date
122+
>= new DateTime(1900, 1, 1, 0, 0, 0, DateTimeKind.Utc)
123+
)
124+
.WithMessage("DateOfBirth must be on or after January 1, 1900.");
125+
}
126+
);
58127
}
59128
);
60129
}

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

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Dotnet.Samples.AspNetCore.WebApi.Models;
33
using Dotnet.Samples.AspNetCore.WebApi.Tests.Utilities;
44
using FluentAssertions;
5+
using FluentValidation;
56
using FluentValidation.Results;
67
using Microsoft.AspNetCore.Http;
78
using Microsoft.AspNetCore.Http.HttpResults;
@@ -31,12 +32,17 @@ public async Task Post_Players_ValidationError_Returns400BadRequest()
3132
var request = PlayerFakes.MakeRequestModelForCreate();
3233
var (service, logger, validator) = PlayerMocks.InitControllerMocks();
3334
validator
34-
.Setup(validator => validator.ValidateAsync(request, It.IsAny<CancellationToken>()))
35+
.Setup(validator =>
36+
validator.ValidateAsync(
37+
It.IsAny<IValidationContext>(),
38+
It.IsAny<CancellationToken>()
39+
)
40+
)
3541
.ReturnsAsync(
3642
new ValidationResult(
3743
new List<ValidationFailure>
3844
{
39-
new("SquadNumber", "SquadNumber must be greater than 0.")
45+
new("SquadNumber", "SquadNumber must be greater than 0."),
4046
}
4147
)
4248
);
@@ -52,7 +58,7 @@ public async Task Post_Players_ValidationError_Returns400BadRequest()
5258
validator.Verify(
5359
validator =>
5460
validator.ValidateAsync(
55-
It.IsAny<PlayerRequestModel>(),
61+
It.IsAny<IValidationContext>(),
5662
It.IsAny<CancellationToken>()
5763
),
5864
Times.Once
@@ -75,7 +81,7 @@ public async Task Post_Players_Existing_Returns409Conflict()
7581
validator
7682
.Setup(validator =>
7783
validator.ValidateAsync(
78-
It.IsAny<PlayerRequestModel>(),
84+
It.IsAny<IValidationContext>(),
7985
It.IsAny<CancellationToken>()
8086
)
8187
)
@@ -92,7 +98,7 @@ public async Task Post_Players_Existing_Returns409Conflict()
9298
validator.Verify(
9399
validator =>
94100
validator.ValidateAsync(
95-
It.IsAny<PlayerRequestModel>(),
101+
It.IsAny<IValidationContext>(),
96102
It.IsAny<CancellationToken>()
97103
),
98104
Times.Once
@@ -116,7 +122,7 @@ public async Task Post_Players_NonExisting_Returns201Created()
116122
validator
117123
.Setup(validator =>
118124
validator.ValidateAsync(
119-
It.IsAny<PlayerRequestModel>(),
125+
It.IsAny<IValidationContext>(),
120126
It.IsAny<CancellationToken>()
121127
)
122128
)
@@ -136,7 +142,7 @@ public async Task Post_Players_NonExisting_Returns201Created()
136142
validator.Verify(
137143
validator =>
138144
validator.ValidateAsync(
139-
It.IsAny<PlayerRequestModel>(),
145+
It.IsAny<IValidationContext>(),
140146
It.IsAny<CancellationToken>()
141147
),
142148
Times.Once
@@ -303,12 +309,17 @@ public async Task Put_PlayerBySquadNumber_ValidationError_Returns400BadRequest()
303309
var controller = new PlayerController(service.Object, logger.Object, validator.Object);
304310

305311
validator
306-
.Setup(validator => validator.ValidateAsync(request, It.IsAny<CancellationToken>()))
312+
.Setup(validator =>
313+
validator.ValidateAsync(
314+
It.IsAny<IValidationContext>(),
315+
It.IsAny<CancellationToken>()
316+
)
317+
)
307318
.ReturnsAsync(
308319
new ValidationResult(
309320
new List<ValidationFailure>
310321
{
311-
new("SquadNumber", "SquadNumber must be greater than 0.")
322+
new("SquadNumber", "SquadNumber must be greater than 0."),
312323
}
313324
)
314325
);
@@ -322,7 +333,7 @@ public async Task Put_PlayerBySquadNumber_ValidationError_Returns400BadRequest()
322333
validator.Verify(
323334
validator =>
324335
validator.ValidateAsync(
325-
It.IsAny<PlayerRequestModel>(),
336+
It.IsAny<IValidationContext>(),
326337
It.IsAny<CancellationToken>()
327338
),
328339
Times.Once
@@ -344,7 +355,7 @@ public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound()
344355
validator
345356
.Setup(validator =>
346357
validator.ValidateAsync(
347-
It.IsAny<PlayerRequestModel>(),
358+
It.IsAny<IValidationContext>(),
348359
It.IsAny<CancellationToken>()
349360
)
350361
)
@@ -364,7 +375,7 @@ public async Task Put_PlayerBySquadNumber_NonExisting_Returns404NotFound()
364375
validator.Verify(
365376
validator =>
366377
validator.ValidateAsync(
367-
It.IsAny<PlayerRequestModel>(),
378+
It.IsAny<IValidationContext>(),
368379
It.IsAny<CancellationToken>()
369380
),
370381
Times.Once
@@ -389,7 +400,7 @@ public async Task Put_PlayerBySquadNumber_SquadNumberMismatch_Returns400BadReque
389400
validator
390401
.Setup(validator =>
391402
validator.ValidateAsync(
392-
It.IsAny<PlayerRequestModel>(),
403+
It.IsAny<IValidationContext>(),
393404
It.IsAny<CancellationToken>()
394405
)
395406
)
@@ -425,7 +436,7 @@ public async Task Put_PlayerBySquadNumber_Existing_Returns204NoContent()
425436
validator
426437
.Setup(validator =>
427438
validator.ValidateAsync(
428-
It.IsAny<PlayerRequestModel>(),
439+
It.IsAny<IValidationContext>(),
429440
It.IsAny<CancellationToken>()
430441
)
431442
)

0 commit comments

Comments
 (0)