-
Notifications
You must be signed in to change notification settings - Fork 253
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>.CopyTo
and related explicit implementations in test fixtures - Added
Clone()
andClone<TResult>()
(with a sharedCopyState
helper) inSpecification<T>
- Updated
SpecificationExtensions.WithProjectionOf
to acceptSpecification<T>
and useClone<TResult>
- Introduced unit tests verifying
Clone
behavior
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
WithProjectionOf
to 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
WithProjectionOf
extension. Adding a test to verify that projection selectors and post-processing actions are correctly copied would increase confidence in this change.
[Fact]
Fixes #516