-
Couldn't load subscription status.
- Fork 254
Redefine WithProjectionOf as an extension to Specification instead of ISpecification #517
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 projection extension and internal state copying by removing the old CopyTo hook on ISpecification<T>, introducing a Clone API on Specification<T>, and updating WithProjectionOf to leverage Clone<TResult>. It also cleans up unused explicit interface implementations in custom spec test classes and adds tests for the new Clone methods.
- Removed
ISpecification<T>.CopyToand related explicit implementations in test fixtures - Added
Clone()andClone<TResult>()(with a sharedCopyStatehelper) inSpecification<T> - Updated
SpecificationExtensions.WithProjectionOfto acceptSpecification<T>and useClone<TResult> - Introduced unit tests verifying
Clonebehavior
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Ardalis.Specification.Tests/Validators/SearchValidatorCustomSpecTests.cs | Removed unused ISpecification<T>.CopyTo impl |
| tests/Ardalis.Specification.Tests/Evaluators/SearchMemoryEvaluatorCustomSpecTests.cs | Removed unused ISpecification<T>.CopyTo impl |
| tests/Ardalis.Specification.EntityFrameworkCore.Tests/Evaluators/SearchEvaluatorCustomSpecTests.cs | Removed unused ISpecification<T>.CopyTo impl |
| tests/Ardalis.Specification.Tests/SpecificationTests.cs | Added Clone and Clone<TResult> unit tests |
| src/Ardalis.Specification/SpecificationExtensions.cs | Changed WithProjectionOf to accept Specification<T> and use Clone<TResult> |
| src/Ardalis.Specification/Specification.cs | Introduced Clone(), Clone<TResult>(), and CopyState helper |
| src/Ardalis.Specification/ISpecification.cs | Removed internal void CopyTo member |
Comments suppressed due to low confidence (2)
src/Ardalis.Specification/SpecificationExtensions.cs:20
- Changing
WithProjectionOfto only acceptSpecification<T>is a breaking API change. Consider documenting this in the changelog and updating XML docs to guide consumers through the new usage.
public static Specification<T, TResult> WithProjectionOf<T, TResult>(this Specification<T> source, Specification<T, TResult> projectionSpec)
tests/Ardalis.Specification.Tests/SpecificationTests.cs:21
- [nitpick] There are no unit tests covering the new
WithProjectionOfextension. Adding a test to verify that projection selectors and post-processing actions are correctly copied would increase confidence in this change.
[Fact]
Fixes #516