-
Notifications
You must be signed in to change notification settings - Fork 255
Refactor the state of include expressions as OneOrMany. #510
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
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 refactors the state management for include expressions and include strings to use the OneOrMany generic type, addressing issue #506. Key changes include refactoring Specification.cs to use OneOrMany for include state, updating the IncludeEvaluator and IncludeStringEvaluator to handle the new state, and adding tests for include scenarios.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/Ardalis.Specification.EntityFrameworkCore.Tests/Evaluators/IncludeStringEvaluatorTests.cs | Added a test to verify query behavior when no include string is provided. |
tests/Ardalis.Specification.EntityFrameworkCore.Tests/Evaluators/IncludeEvaluatorTests.cs | Added tests to verify behavior for specs with no or a single include expression. |
src/Ardalis.Specification/Specification.cs | Refactored include state from List to OneOrMany, updating add and copy functionality. |
src/Ardalis.Specification.EntityFrameworkCore/Evaluators/IncludeStringEvaluator.cs | Updated evaluator to check OneOrMany include strings for optimized processing. |
src/Ardalis.Specification.EntityFrameworkCore/Evaluators/IncludeEvaluator.cs | Updated evaluator to check OneOrMany include expressions and use caching for delegates. |
Comments suppressed due to low confidence (2)
src/Ardalis.Specification.EntityFrameworkCore/Evaluators/IncludeStringEvaluator.cs:16
- [nitpick] Ensure that the SingleOrDefault property on OneOrMany returns null when there are multiple elements so that the fallback loop is reliably executed. If not already documented, please clarify this behavior in the OneOrMany API documentation.
if (spec.OneOrManyIncludeStrings.SingleOrDefault is { } includeString)
src/Ardalis.Specification.EntityFrameworkCore/Evaluators/IncludeEvaluator.cs:46
- [nitpick] Confirm that the SingleOrDefault property on OneOrMany returns null when more than one include expression is present so that the subsequent iteration over IncludeExpressions properly handles multiple items. Consider documenting this behavior for future maintainers.
if (spec.OneOrManyIncludeExpressions.SingleOrDefault is { } includeExpression)
It addresses the issue #506