-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix validation generator to ignore properties with [FromServices] or [FromKeyedServices] attributes #62669
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
Fix validation generator to ignore properties with [FromServices] or [FromKeyedServices] attributes #62669
Conversation
Co-authored-by: captainsafia <[email protected]>
Co-authored-by: captainsafia <[email protected]>
@copilot Update the tests in https://github.com/dotnet/aspnetcore/blob/main/src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.ComplexType.cs to verify that properties with [FromService] or [FromKeyedService] attributes are not validated in class types. Update tests in https://github.com/dotnet/aspnetcore/blob/main/src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGenerator.RecordType.cs to validate that properties with [FromService] or [FromKeyedService] attributes are not validated in record types. |
Co-authored-by: captainsafia <[email protected]>
I've added the requested tests for both class and record types with [FromServices] and [FromKeyedServices] attributes. The tests verify the filtering behavior: Class test (
Record test (
The implementation is mostly working but there's still an issue with |
@copilot Instead of creating a new test method for the scenario, update the types used in the existing test methods for class and record types with examples that include properties with FromService and FromKeyedService attributes. |
Co-authored-by: captainsafia <[email protected]>
I've updated the tests to use existing types with FromServices attributes instead of creating new test methods. The implementation is mostly working correctly: ✅ Class test: Properties with The filtering logic is implemented correctly for both scenarios, but there appears to be an issue with the record parameter attribute detection that requires further investigation. Commit hash: d7ad03c |
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.
Pull Request Overview
This PR fixes a validation generator issue where properties with [FromServices]
or [FromKeyedServices]
attributes were incorrectly being validated instead of skipped. The solution adds proper filtering for service-injected properties in both regular classes and record types, matching the existing behavior for parameters.
Key changes:
- Added
IsServiceProperty
extension method to identify service-injected properties - Updated validation generator to skip properties with service injection attributes
- Enhanced test coverage to verify service properties are properly ignored
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
ITypeSymbolExtensions.cs |
Added IsServiceProperty method to detect service injection attributes on properties |
ValidationsGenerator.TypesParser.cs |
Updated validation logic to skip service-injected properties and constructor parameters |
ValidationsGenerator.ComplexType.cs |
Added test case with [FromServices] property to verify filtering works for classes |
ValidationsGenerator.RecordType.cs |
Added test cases with both [FromServices] and [FromKeyedServices] to verify filtering works for records |
Co-authored-by: Copilot <[email protected]>
Fixes #62173
Problem
When using
AsParameters
with properties that have[FromServices]
or[FromKeyedServices]
attributes, the validation generator incorrectly tries to validate those properties instead of skipping them. This causes a500 Internal Server Error
with anInvalidOperationException
when the validation logic attempts to validate service-injected properties.Solution
Updated the validation generator's
ExtractValidatableMembers
method to skip properties with service injection attributes, matching the existing behavior for parameters:IsServiceProperty
extension method: Similar to the existingIsServiceParameter
method, checks if a property has[FromServices]
or[FromKeyedServices]
attributesChanges
ITypeSymbolExtensions.cs
: AddedIsServiceProperty
method to check for service injection attributes on propertiesValidationsGenerator.TypesParser.cs
: UpdatedExtractValidatableMembers
to skip properties and constructor parameters with service attributesVerification
The generated validation code now correctly includes only
ValidatableProperty
and excludes the service-injected properties, preventing the validation exceptions.Impact
This fix resolves the validation exceptions when using
AsParameters
with service-injected properties, enabling proper use of dependency injection in Minimal API validation scenarios.💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.