Skip to content

Feat/49/implement specification pattern - #68

Open
Loki22978964 wants to merge 7 commits into
devfrom
feat/49/implement-specification-pattern
Open

Feat/49/implement specification pattern#68
Loki22978964 wants to merge 7 commits into
devfrom
feat/49/implement-specification-pattern

Conversation

@Loki22978964

Copy link
Copy Markdown
Contributor

Summary of issue

Repository layer used ad-hoc predicate/include lambda 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. multiple Include chains combined with Where conditions).

Summary of change

  • Added Ardalis.Specification and Ardalis.Specification.EntityFrameworkCore packages to Streetcode.DAL.
  • Extended IRepositoryBase<T> / RepositoryBase<T> with four specification-based methods: ListAsync, GetBySpecAsync, CountAsync, AnyAsync, backed by a single private ApplySpecification helper using SpecificationEvaluator.Default. All existing repository methods (GetAllAsync, GetFirstOrDefaultAsync, write operations, etc.) are unchanged.
  • Created Streetcode.DAL/Specifications/Team and Streetcode.DAL/Specifications/Partners with one specification class per query scenario:
    • Team: GetAllTeamSpecification, GetAllMainTeamSpecification, GetByIdTeamSpecification.
    • Partners: GetAllPartnersSpecification, GetAllPartnerShortSpecification, GetPartnerByIdSpecification, GetPartnersByStreetcodeIdSpecification.
  • Migrated the corresponding MediatR handlers (GetAllTeamHandler, GetAllMainTeamHandler, GetByIdTeamHandler, GetAllPartnersHandler, GetAllPartnerShortHandler, GetPartnerByIdHandler, GetPartnersByStreetcodeIdHandler) to build a specification and call the matching repository method instead of passing predicate/include lambdas directly.
  • The rest of the project keeps using the pre-existing repository methods unchanged — no wider migration was in scope for this task.

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

  • СI passed
  • Сode coverage >=95%
  • PR is reviewed manually again (to make sure you have 100% ready code)
  • All reviewers agreed to merge the PR
  • I've checked new feature as logged in and logged out user if needed
  • PR meets all conventions

- 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)

@DrFaust555 DrFaust555 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes on one item.

  1. 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:

  1. 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.
  2. CountAsync and AnyAsync are added to IRepositoryBase and used nowhere. Either use them or drop them until needed.
  3. GetPartnersByStreetcodeIdSpecification.cs: the .Where and .Include lines are indented to column 8 instead of lining up under Query.
  4. GetByIdTeamHandler: the call is split as GetBySpecAsync( newline specification, cancellationToken). Put it on one line.
  5. 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.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants