Feat/49/implement specification pattern - #68
Open
Loki22978964 wants to merge 7 commits into
Open
Conversation
- Add GetAllTeamSpecification and GetAllMainTeamSpecification - Update GetAllTeamHandler and GetAllMainTeamHandler to use ListAsync
…ification-pattern sh: line 1: q: command not found
- GetPartnerByIdHandlerTests: align mocks with updated repository calls - GetPartnersByStreetcodeIdHandlerTests: replace GetAllAsync mocks with ListAsync(GetPartnersByStreetcodeIdSpecification, CancellationToken) - Fix Times.Never verification to check ListAsync instead of removed GetAllAsync - Replace Errors.First() with Errors[0] indexing (csharpsquid:S6608) All tests passing (6/6)
Loki22978964
requested review from
DrFaust555,
Exlizardium,
Laminate32,
M1R4MI,
Mult1pers,
TakerTrip,
VitaliyKorostil,
emil720a1 and
skorpionreser
August 16, 2026 16:31
DrFaust555
requested changes
Aug 17, 2026
DrFaust555
left a comment
Contributor
There was a problem hiding this comment.
Requesting changes on one item.
- ApplySpecification drops AsNoTracking. GetQueryable ends with query.AsNoTracking(), so every read through the old methods was untracked. SpecificationEvaluator.Default.GetQuery(_dbContext.Set().AsQueryable(), specification) is not. All seven migrated handlers are read-only and now attach their entities to the change tracker. Add .AsNoTracking() in ApplySpecification, or call Query.AsNoTracking() in each specification.
Also fix:
- No test covers a specification. The handler tests assert It.IsAny(), which only proves a spec of that type was passed. The filtering and include logic moved into these classes and nothing verifies it. Test them directly with specification.Evaluate(list) against an in-memory collection.
- CountAsync and AnyAsync are added to IRepositoryBase and used nowhere. Either use them or drop them until needed.
- GetPartnersByStreetcodeIdSpecification.cs: the .Where and .Include lines are indented to column 8 instead of lining up under Query.
- GetByIdTeamHandler: the call is split as GetBySpecAsync( newline specification, cancellationToken). Put it on one line.
- RepositoryBase now declares Interfaces.Base.IRepositoryBase fully qualified because Ardalis.Specification exports its own IRepositoryBase. Add a using alias instead so the collision is stated once, not worked around inline.
…sed methods, formatting, using alias)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary of issue
Repository layer used ad-hoc
predicate/includelambda parameters (GetAllAsync,GetSingleOrDefaultAsync, etc.) for every query. This scattered filtering/include logic across handlers, made queries hard to reuse or unit test in isolation, and had no single place to compose complex predicates (e.g. multipleIncludechains combined withWhereconditions).Summary of change
Ardalis.SpecificationandArdalis.Specification.EntityFrameworkCorepackages toStreetcode.DAL.IRepositoryBase<T>/RepositoryBase<T>with four specification-based methods:ListAsync,GetBySpecAsync,CountAsync,AnyAsync, backed by a single privateApplySpecificationhelper usingSpecificationEvaluator.Default. All existing repository methods (GetAllAsync,GetFirstOrDefaultAsync, write operations, etc.) are unchanged.Streetcode.DAL/Specifications/TeamandStreetcode.DAL/Specifications/Partnerswith one specification class per query scenario:GetAllTeamSpecification,GetAllMainTeamSpecification,GetByIdTeamSpecification.GetAllPartnersSpecification,GetAllPartnerShortSpecification,GetPartnerByIdSpecification,GetPartnersByStreetcodeIdSpecification.GetAllTeamHandler,GetAllMainTeamHandler,GetByIdTeamHandler,GetAllPartnersHandler,GetAllPartnerShortHandler,GetPartnerByIdHandler,GetPartnersByStreetcodeIdHandler) to build a specification and call the matching repository method instead of passingpredicate/includelambdas directly.Testing approach
Updated existing unit tests for all migrated handlers so mocked repository calls assert on the concrete specification type used by the handler (e.g.
ListAsync(It.IsAny<GetAllTeamSpecification>(), It.IsAny<CancellationToken>())), instead of matching on the old lambda-based signature. This makes the tests verify that the handler actually builds and passes the intended specification, not just that "some query" was made. Each handler has coverage for: success with data, success with an empty result, and failure/null scenario (with logger verification where applicable). All 89 tests pass locally.CHECK LIST