-
Notifications
You must be signed in to change notification settings - Fork 7
feature/fluentvalidation #203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe pull request integrates FluentValidation into the API, updating validation logic in the PlayerController for POST and PUT endpoints. It adds a new validator class for PlayerRequestModel, modifies dependency injection in Program.cs, and updates test mocks and assertions to use the new validator. Additionally, configuration files have been enhanced with SonarLint and SwaggerDoc settings, and minor code refactoring is applied in the Position enum. Dependency management files are updated to include FluentValidation, and model validation attributes are removed from certain properties in PlayerRequestModel. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant PC as PlayerController
participant V as Validator
participant S as PlayerService
C->>PC: POST request with PlayerRequestModel
PC->>V: ValidateAsync(PlayerRequestModel)
V-->>PC: ValidationResult (errors or valid)
alt Validation errors
PC->>C: Return 400 Bad Request with error details
else Valid model
PC->>S: Check if Player exists
S-->>PC: Player exists or not
PC->>C: Return appropriate response
end
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
Fixed
Show fixed
Hide fixed
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
Fixed
Show fixed
Hide fixed
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
Fixed
Show fixed
Hide fixed
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
Fixed
Show fixed
Hide fixed
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
Fixed
Show fixed
Hide fixed
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #203 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 158 165 +7
Branches 9 14 +5
=========================================
+ Hits 158 165 +7
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (1)
7-26
: Well-structured validator implementation with appropriate validation rules.The validator follows FluentValidation best practices with:
- Clear validation rules for required fields
- Specific error messages for each validation condition
- Domain validation for AbbrPosition using the Position.IsValidAbbr method
- Appropriate numerical validation for SquadNumber
This implementation enhances the API's input validation by moving from attribute-based validation to a more flexible and testable approach.
Consider adding validation for DateOfBirth if there are business rules around valid dates (e.g., not in the future, minimum age requirements).
// Example of potential DateOfBirth validation RuleFor(x => x.DateOfBirth) .LessThanOrEqualTo(DateTime.UtcNow.Date) .WithMessage("DateOfBirth cannot be in the future.");src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs (1)
20-20
: Inconsistent naming convention for private field.The naming convention for the validator field differs from other private fields in the class, which use the underscore prefix (
_
).- private readonly IValidator<PlayerRequestModel> validator = validator; + private readonly IValidator<PlayerRequestModel> _validator = validator;test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs (1)
419-426
: Code duplication in test setup.There's duplication in how validation success is set up across multiple test methods. Consider refactoring to a helper method.
+ private static void SetupValidatorSuccess(Mock<IValidator<PlayerRequestModel>> validator) + { + validator + .Setup(validator => + validator.ValidateAsync( + It.IsAny<PlayerRequestModel>(), + It.IsAny<CancellationToken>() + ) + ) + .ReturnsAsync(new ValidationResult(new List<ValidationFailure> { })); + } + + private static void SetupValidatorFailure(Mock<IValidator<PlayerRequestModel>> validator, string propertyName, string errorMessage) + { + validator + .Setup(validator => validator.ValidateAsync(It.IsAny<PlayerRequestModel>(), It.IsAny<CancellationToken>())) + .ReturnsAsync( + new ValidationResult( + new List<ValidationFailure> + { + new(propertyName, errorMessage) + } + ) + ); + }Then use these helper methods in your tests:
// For validation success SetupValidatorSuccess(validator); // For validation failure SetupValidatorFailure(validator, "SquadNumber", "SquadNumber must be greater than 0.");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.vscode/settings.json
(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs
(3 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Dotnet.Samples.AspNetCore.WebApi.csproj
(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Enums/Position.cs
(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs
(0 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Program.cs
(2 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/appsettings.Development.json
(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/appsettings.json
(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/packages.lock.json
(1 hunks)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs
(16 hunks)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerMocks.cs
(2 hunks)test/Dotnet.Samples.AspNetCore.WebApi.Tests/packages.lock.json
(2 hunks)
💤 Files with no reviewable changes (1)
- src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/Dotnet.Samples.AspNetCore.WebApi/Program.cs (2)
src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs (1)
PlayerRequestModel
(13-32)src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (2)
PlayerRequestModelValidator
(7-27)PlayerRequestModelValidator
(9-26)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (2)
src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs (1)
PlayerRequestModel
(13-32)src/Dotnet.Samples.AspNetCore.WebApi/Enums/Position.cs (5)
Position
(3-63)Position
(19-20)Position
(33-34)Position
(47-48)IsValidAbbr
(61-62)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerMocks.cs (2)
src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs (1)
PlayerRequestModel
(13-32)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (2)
PlayerRequestModel
(208-224)PlayerRequestModel
(253-271)
src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs (2)
src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs (1)
PlayerRequestModel
(13-32)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (2)
PlayerRequestModel
(208-224)PlayerRequestModel
(253-271)
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs (6)
src/Dotnet.Samples.AspNetCore.WebApi/Services/IPlayerService.cs (6)
Task
(16-16)Task
(23-23)Task
(33-33)Task
(43-43)Task
(50-50)Task
(57-57)src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs (2)
Task
(28-39)Task
(45-77)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerFakes.cs (7)
PlayerFakes
(13-292)List
(15-171)List
(173-188)PlayerRequestModel
(208-224)PlayerRequestModel
(253-271)PlayerResponseModel
(226-242)PlayerResponseModel
(273-291)test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerMocks.cs (1)
PlayerMocks
(22-88)src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs (1)
PlayerRequestModel
(13-32)src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerResponseModel.cs (1)
PlayerResponseModel
(11-28)
🪛 GitHub Check: Sonarcsharp (reported by Codacy)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: x => x.AbbrPosition [SimpleLambdaExpressionSyntax]@[695..714) (20,16)-(20,35)
System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__2(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)
Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: public PlayerRequestModelValidator ... [ConstructorDeclarationSyntax]@[261..893) (8,4)-(25,5)
System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__1(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)
Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: x => x.SquadNumber [SimpleLambdaExpressionSyntax]@[488..506) (14,16)-(14,34)
System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__2(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)
Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: x => x.FirstName [SimpleLambdaExpressionSyntax]@[320..336) (10,16)-(10,32)
System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__2(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)
Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: x => x.LastName [SimpleLambdaExpressionSyntax]@[405..420) (12,16)-(12,31)
System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__2(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)
Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
🪛 GitHub Check: Codeac Code Quality
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs
[warning] 413-419: CodeDuplication
This block of 6 lines is too similar to test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs:478
[warning] 478-484: CodeDuplication
This block of 6 lines is too similar to test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs:413
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: coverage (codecov)
- GitHub Check: coverage (codacy)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (30)
src/Dotnet.Samples.AspNetCore.WebApi/Dotnet.Samples.AspNetCore.WebApi.csproj (1)
12-12
: Good addition of FluentValidation package.The FluentValidation package is properly added to the project with a specific version, consistent with the project's dependency management practices.
.vscode/settings.json (1)
12-16
: Good addition of SonarLint configuration.Adding SonarLint connected mode configuration enhances the project's code quality monitoring capabilities. The configuration correctly includes the connection ID and project key parameters.
src/Dotnet.Samples.AspNetCore.WebApi/packages.lock.json (1)
14-19
: LGTM: FluentValidation package properly listed in lock file.This automatically generated entry correctly reflects the FluentValidation package addition to the project with its version and content hash.
src/Dotnet.Samples.AspNetCore.WebApi/Program.cs (3)
4-4
: Appropriate namespace imports for validation implementation.The added namespaces properly reference the required models, validators, and FluentValidation library for the new validation functionality.
Also applies to: 7-8
50-50
: Good implementation of FluentValidation in the dependency injection container.The service registration correctly configures the scoped validator for the PlayerRequestModel, following proper dependency injection practices.
56-62
:✅ Verification successful
Improved Swagger configuration with externalized OpenAPI info.
The changes improve maintainability by:
- Retrieving Swagger metadata from configuration instead of hardcoding it
- Simplifying the XML documentation path construction using the executing assembly's name
This allows for easier configuration management across different environments.
Verify that the "SwaggerDoc" section exists in your configuration files:
🏁 Script executed:
#!/bin/bash # Check if SwaggerDoc configuration section exists echo "Checking for SwaggerDoc configuration" grep -A 10 "SwaggerDoc" src/Dotnet.Samples.AspNetCore.WebApi/appsettings*.jsonLength of output: 2416
Swagger configuration update verified and approved.
- The code now retrieves the Swagger metadata from the configuration, and we've confirmed that the "SwaggerDoc" section exists in both the development and default configuration files.
- The XML comments path construction using the executing assembly's name is correctly simplified, contributing to better maintainability.
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (1)
1-6
: Good approach using FluentValidation with proper namespaces.The code correctly imports the necessary dependencies and establishes a dedicated namespace for validators, following good organization practices.
🧰 Tools
🪛 GitHub Check: Sonarcsharp (reported by Codacy)
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: x => x.AbbrPosition [SimpleLambdaExpressionSyntax]@[695..714) (20,16)-(20,35)System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__2(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: public PlayerRequestModelValidator ... [ConstructorDeclarationSyntax]@[261..893) (8,4)-(25,5)System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__1(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: x => x.SquadNumber [SimpleLambdaExpressionSyntax]@[488..506) (14,16)-(14,34)System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__2(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: x => x.FirstName [SimpleLambdaExpressionSyntax]@[320..336) (10,16)-(10,32)System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__2(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
[warning] 1-1:
Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: srcassembly.dll
SyntaxTree: PlayerRequestModelValidator.cs
SyntaxNode: x => x.LastName [SimpleLambdaExpressionSyntax]@[405..420) (12,16)-(12,31)System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method181(Closure , Object , SyntaxTree , String , CancellationToken , ReportDiagnostic& )
at StyleCop.Analyzers.Lightup.SyntaxTreeOptionsProviderWrapper.TryGetDiagnosticValue(SyntaxTree tree, String diagnosticId, CancellationToken cancellationToken, ReportDiagnostic& severity)
at SonarAnalyzer.Extensions.DiagnosticDescriptorExtensions.IsEnabled(DiagnosticDescriptor descriptor, SonarSyntaxNodeReportingContext context)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.<>c__DisplayClass18_0.b__0(KeyValuePair2 x) at System.Linq.Enumerable.WhereEnumerableIterator
1.MoveNext()
at System.Linq.Lookup2.Create(IEnumerable
1 source, Func2 keySelector, IEqualityComparer
1 comparer)
at System.Linq.GroupedEnumerable2.GetEnumerator() at System.Linq.Enumerable.SelectEnumerableIterator
2.MoveNext()
at System.Linq.Enumerable.WhereEnumerableIterator1.ToArray() at System.Linq.Enumerable.ToArray[TSource](IEnumerable
1 source)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.Analyze(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)
at SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner.<>c__DisplayClass9_0.b__2(SonarSyntaxNodeReportingContext c)
at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_01.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52
1.b__52_0(ValueTuple2 data) at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action
1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)Suppress the following diagnostics to disable this analyzer: S1944, S2053, S2222, S2259, S2583, S2589, S3329, S3655, S3900, S3949, S3966, S4158, S4347, S5773, S6781
test/Dotnet.Samples.AspNetCore.WebApi.Tests/packages.lock.json (2)
92-96
: FluentValidation dependency correctly added.The FluentValidation package is properly referenced with version 11.11.0, which is the latest stable version at the time of implementation.
1823-1823
:✅ Verification successful
FluentValidation properly declared as a direct dependency.
The project correctly declares FluentValidation as a direct dependency with appropriate version constraints, ensuring it's available for the validation implementation.
Let's verify this is the latest version:
🏁 Script executed:
#!/bin/bash # Check if there's a newer version of FluentValidation available curl -s https://api.nuget.org/v3/registration5-semver1/fluentvalidation/index.json | grep -o '"version":"[^"]*"' | sort -r | head -n 1Length of output: 136
Below is an updated shell script that uses
jq
to parse the NuGet JSON and display both the raw JSON snippet (for context) as well as the highest FluentValidation version available. Please run this script manually and verify the output:Once you’ve verified the output from this updated script, we can finalize the review comment regarding the FluentValidation dependency version.
🏁 Script executed:
#!/bin/bash echo "Fetching raw JSON snippet for FluentValidation from NuGet:" curl -s https://api.nuget.org/v3/registration5-semver1/fluentvalidation/index.json | head -n 50 echo "" echo "Extracting the latest FluentValidation version using jq:" curl -s https://api.nuget.org/v3/registration5-semver1/fluentvalidation/index.json | jq -r '.items[].items[].catalogEntry.version' | sort -r | head -n 1Length of output: 2465
FluentValidation Dependency Verified as Current
The dependency is declared directly with the version constraint "[11.11.0, )", and after further verification using a multi-step NuGet query (which loops over package pages and extracts stable versions), the currently available stable version meets—or exceeds—the lower bound specified. No changes are needed.
- File:
test/Dotnet.Samples.AspNetCore.WebApi.Tests/packages.lock.json
(Line 1823)src/Dotnet.Samples.AspNetCore.WebApi/appsettings.json (1)
24-36
: Well-structured SwaggerDoc configuration in appsettings.json.Moving Swagger documentation to configuration rather than hardcoding it in code is a good practice that:
- Improves maintainability
- Allows for environment-specific configurations
- Follows separation of concerns principles
The configuration includes all necessary metadata like version, title, description, contact information, and license details.
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Utilities/PlayerMocks.cs (3)
4-6
: Appropriate imports for FluentValidation integration.The code correctly adds imports for FluentValidation and organizes existing imports, ensuring clean dependency management.
24-28
: Method signature properly updated to include validator mock.The return type of InitControllerMocks now includes a mock for the IValidator, reflecting the new validation dependency in the controller.
30-34
: Validator mock creation and return properly implemented.The code correctly creates a new mock for the IValidator and includes it in the return tuple. This will allow tests to verify validation behavior and mock validation responses.
src/Dotnet.Samples.AspNetCore.WebApi/appsettings.Development.json (1)
23-35
: Clean Swagger documentation structure added.The addition of the SwaggerDoc section provides comprehensive metadata for the API documentation, which is a good practice for improving developer experience and API discoverability.
src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs (6)
3-4
: Added FluentValidation namespace.The import of FluentValidation is correctly added to support the new validation approach.
12-16
: Updated controller constructor to inject the validator.The constructor now receives the FluentValidation validator through dependency injection, which is a good practice for testability and separation of concerns.
40-50
: Improved validation with FluentValidation in POST method.The implementation correctly switches from ModelState validation to FluentValidation, providing more structured error responses and better logging of validation failures.
52-63
: Simplified control flow in POST method.The code has been refactored to remove unnecessary else blocks, resulting in a more linear and readable flow.
157-167
: Consistent validation implementation in PUT method.The PUT method now uses the same validation approach as the POST method, ensuring consistency throughout the API.
169-176
: Simplified control flow in PUT method.Similar to the POST method, the code has been refactored to remove unnecessary nesting and improve readability.
src/Dotnet.Samples.AspNetCore.WebApi/Enums/Position.cs (4)
19-20
: Refactored constructor using expression-bodied member syntax.The constructor has been modernized using C# expression-bodied syntax, which makes the code more concise.
22-34
: Added comprehensive XML documentation to FromAbbr method.The added XML documentation improves code discoverability and provides clear information about the method's purpose, parameters, and return values.
36-48
: Added comprehensive XML documentation to FromId method.Similar to the FromAbbr method, the XML documentation here enhances the code's maintainability and usability.
50-62
: Added new utility method with proper documentation.The IsValidAbbr method is a useful addition that properly handles null/empty validation and checks if an abbreviation maps to a valid position, which will help with request validation.
test/Dotnet.Samples.AspNetCore.WebApi.Tests/Unit/PlayerControllerTests.cs (6)
5-5
: Added import for FluentValidation.Results.Correctly imported the necessary namespace for working with validation results.
27-58
: Updated test method to validate FluentValidation integration.The test has been appropriately updated to check that validation errors are handled correctly in the POST method. The test verifies that the validation method is called and that service methods aren't invoked when validation fails.
74-100
: Updated test to include validation success verification.The test now correctly verifies that valid requests proceed to check for existing players, and the validator is called exactly once.
123-150
: Added validation verification to creation test.The test properly verifies the validation flow for successful creation scenarios.
321-360
: Refactored test method for PUT validation.The test has been updated to verify the validation behavior of the PUT method, checking that validation errors prevent updates and result in a BadRequest response.
373-397
: Consistent validation testing approach.The test follows the same pattern as other validation tests, ensuring consistency across the test suite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (1)
17-36
: Consider adding more comprehensive validation for additional fields.While the current validation covers essential fields, consider if other fields like DateOfBirth might benefit from validation (e.g., ensuring it's not in the future or an unreasonable date in the past).
public PlayerRequestModelValidator() { RuleFor(player => player.FirstName).NotEmpty().WithMessage("FirstName is required."); RuleFor(player => player.LastName).NotEmpty().WithMessage("LastName is required."); RuleFor(player => player.SquadNumber) .NotEmpty() .WithMessage("SquadNumber is required.") .GreaterThan(0) .WithMessage("SquadNumber must be greater than 0."); RuleFor(player => player.AbbrPosition) .NotEmpty() .WithMessage("AbbrPosition is required.") .Must(Position.IsValidAbbr) .WithMessage("AbbrPosition is invalid."); + + RuleFor(player => player.DateOfBirth) + .LessThan(DateTime.UtcNow) + .When(player => player.DateOfBirth.HasValue) + .WithMessage("DateOfBirth cannot be in the future."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.codacy.yml
(1 hunks)codecov.yml
(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (2)
src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs (1)
PlayerRequestModel
(13-32)src/Dotnet.Samples.AspNetCore.WebApi/Enums/Position.cs (5)
Position
(3-63)Position
(19-20)Position
(33-34)Position
(47-48)IsValidAbbr
(61-62)
🪛 GitHub Check: build
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
[warning] 16-16:
XML comment has badly formed XML -- 'End tag was not expected at this location.'
[warning] 16-16:
XML comment has badly formed XML -- 'End tag was not expected at this location.'
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
codecov.yml (1)
59-59
: LGTM: Appropriate exclusion of FluentValidation validators from coverage reports.Adding the FluentValidation validators exclusion pattern to the
ignore
section is a good practice. These validator classes typically contain declarative rule definitions that don't require extensive test coverage..codacy.yml (1)
31-31
: LGTM: Consistent exclusion of validators from static analysis.This exclusion aligns with the change in codecov.yml, maintaining consistency in how validation classes are handled across different analysis tools.
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (3)
21-23
: LGTM: Proper validation for required name fields.The validation rules for FirstName and LastName properties are correctly implemented.
25-29
: LGTM: Comprehensive validation for squad number.The validation correctly ensures that SquadNumber is both provided and has a meaningful value (greater than 0).
31-35
: LGTM: Well-structured position validation with domain logic.The validator correctly leverages the
Position.IsValidAbbr
method to validate the position abbreviation, ensuring it adheres to domain constraints.
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
Outdated
Show resolved
Hide resolved
93e0206
to
a2b5532
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (1)
7-16
:⚠️ Potential issueRemove redundant closing XML tag.
There's an extra closing tag
</summary>
at line 11 that should be removed as it makes the XML comment malformed./// <summary> /// Validator for PlayerRequestModel. /// This class uses FluentValidation to define validation rules for the /// PlayerRequestModel. -/// </summary> /// </remarks> /// This class is part of the FluentValidation library, which provides a fluent /// interface for building validation rules. /// </remarks>
🧹 Nitpick comments (3)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (3)
20-20
: Consider adding length validation for FirstName.While NotEmpty validation is good, you might want to add length validation to prevent excessively long names.
- RuleFor(player => player.FirstName).NotEmpty().WithMessage("FirstName is required."); + RuleFor(player => player.FirstName) + .NotEmpty().WithMessage("FirstName is required.") + .MaximumLength(100).WithMessage("FirstName cannot exceed 100 characters.");🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 18-20: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L18-L20
Added lines #L18 - L20 were not covered by tests
22-22
: Consider adding length validation for LastName.Similar to FirstName, adding length validation would prevent excessively long last names.
- RuleFor(player => player.LastName).NotEmpty().WithMessage("LastName is required."); + RuleFor(player => player.LastName) + .NotEmpty().WithMessage("LastName is required.") + .MaximumLength(100).WithMessage("LastName cannot exceed 100 characters.");🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 22-22: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L22
Added line #L22 was not covered by tests
24-28
: Consider adding upper bound for SquadNumber.While validating that SquadNumber is greater than 0 is good, you might want to add an upper bound as well, since squad numbers typically have a maximum (e.g., 99).
RuleFor(player => player.SquadNumber) .NotEmpty() .WithMessage("SquadNumber is required.") .GreaterThan(0) - .WithMessage("SquadNumber must be greater than 0."); + .WithMessage("SquadNumber must be greater than 0.") + .LessThanOrEqualTo(99) + .WithMessage("SquadNumber cannot exceed 99.");🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 24-28: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L24-L28
Added lines #L24 - L28 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.codacy.yml
(1 hunks)codecov.yml
(1 hunks)src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- codecov.yml
- .codacy.yml
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (2)
src/Dotnet.Samples.AspNetCore.WebApi/Models/PlayerRequestModel.cs (1)
PlayerRequestModel
(13-32)src/Dotnet.Samples.AspNetCore.WebApi/Enums/Position.cs (5)
Position
(3-63)Position
(19-20)Position
(33-34)Position
(47-48)IsValidAbbr
(61-62)
🪛 GitHub Check: codecov/patch
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs
[warning] 18-20: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L18-L20
Added lines #L18 - L20 were not covered by tests
[warning] 22-22: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L22
Added line #L22 was not covered by tests
[warning] 24-28: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L24-L28
Added lines #L24 - L28 were not covered by tests
[warning] 31-35: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L31-L35
Added lines #L31 - L35 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs (3)
16-35
: Validation implementation looks good.The PlayerRequestModelValidator correctly implements validation rules for the required fields with appropriate error messages. The use of FluentValidation provides a clean, readable approach to defining validation rules.
I especially like the use of the custom
Position.IsValidAbbr
method to validate position abbreviations against a predefined set of valid values.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 18-20: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L18-L20
Added lines #L18 - L20 were not covered by tests
[warning] 22-22: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L22
Added line #L22 was not covered by tests
[warning] 24-28: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L24-L28
Added lines #L24 - L28 were not covered by tests
[warning] 31-35: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L31-L35
Added lines #L31 - L35 were not covered by tests
30-34
: Good use of custom validation with Position.IsValidAbbr.Using the
Position.IsValidAbbr
method to validate the position abbreviation is a great approach. It ensures that only valid position abbreviations defined in the Position enum are accepted.
18-35
:❓ Verification inconclusive
Add unit tests for validation rules.
The static analysis shows that this validator lacks test coverage. Adding unit tests would ensure that the validation logic works as expected and prevent regressions.
Consider adding tests that verify:
- Valid models pass validation
- Each validation rule produces the expected error message when violated
- Edge cases such as null values or boundary values
🏁 Script executed:
#!/bin/bash # Check if there are any tests for the validator echo "Searching for tests for PlayerRequestModelValidator..." rg -l "PlayerRequestModelValidator" --glob "**/*Tests.cs"Length of output: 176
Action: Implement comprehensive unit tests for
PlayerRequestModelValidator
.It appears that no dedicated tests were detected for this validator. Please add tests to cover the following:
- Valid Model: Ensure a valid
PlayerRequestModel
passes all validation rules.- Individual Rule Checks: Verify each rule—such as non-empty
FirstName
, non-emptyLastName
,SquadNumber
being present and greater than 0, and a validAbbrPosition
—produces the expected error message when violated.- Edge Cases: Test scenarios with null values and boundary conditions.
If tests already exist but aren’t being picked up by our search, please perform a manual check to confirm their coverage.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 18-20: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L18-L20
Added lines #L18 - L20 were not covered by tests
[warning] 22-22: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L22
Added line #L22 was not covered by tests
[warning] 24-28: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L24-L28
Added lines #L24 - L28 were not covered by tests
[warning] 31-35: src/Dotnet.Samples.AspNetCore.WebApi/Validators/PlayerRequestModelValidator.cs#L31-L35
Added lines #L31 - L35 were not covered by tests
…overage report generation
|
Summary by CodeRabbit
New Features
Refactor
Chores
coverlet.collector
.