Conversation
## Summary Adds `squad-mark-released.yml` — a GitHub Actions workflow that automatically moves all **Done** items on the MyBlog project board to **Released** whenever a real GitHub Release is published. Closes #107 ## Changes - New workflow: `.github/workflows/squad-mark-released.yml` - Triggers on `release: published` and `release: released` - Skips pre-releases (insider builds) and drafts - Paginates through all project items (handles boards > 100 items) - Matches Status field by ID (robust against field renames) - Uses `fieldValues(first: 50)` to avoid silent misses ## How it fits the release pipeline ``` squad-milestone-release.yml → creates GitHub Release → release: published → squad-mark-released.yml (this) → moves Done → Released → release-blog.yml → triggers Bilbo blog post ``` ## Testing Will fire on the next `gh release create` from `squad-milestone-release.yml`. Pre-releases from `squad-insider-release.yml` are skipped. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Boromir <boromir@squad.dev>
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
There was a problem hiding this comment.
Pull request overview
Promotes dev → main for the Sprint 5 (v1.2.0) release, introducing a centralized two-tier caching abstraction (L1 IMemoryCache + L2 Redis) and refactoring BlogPost handlers/tests to use it.
Changes:
- Added
IBlogPostCacheService+BlogPostCacheService(L1/L2 caching) with centralizedBlogPostCacheKeys, and wired into DI viaAddBlogPostCaching(). - Refactored BlogPost MediatR handlers to depend only on
IBlogPostCacheService(removing directIMemoryCache/IDistributedCacheusage). - Added/updated unit, integration (Redis via Testcontainers), and architecture tests; adjusted CI workflow(s) related to release automation and dev validation.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Web.Tests/Handlers/GetBlogPostsHandlerTests.cs | Updates handler unit tests to mock IBlogPostCacheService instead of raw caches. |
| tests/Web.Tests/Handlers/EditBlogPostHandlerTests.cs | Updates edit/get-by-id handler tests to use IBlogPostCacheService invalidation + fetch methods. |
| tests/Web.Tests/Handlers/DeleteBlogPostHandlerTests.cs | Updates delete handler tests to assert cache invalidation via abstraction. |
| tests/Web.Tests/Handlers/CreateBlogPostHandlerTests.cs | Updates create handler tests to assert InvalidateAllAsync via abstraction. |
| tests/Web.Tests/GlobalUsings.cs | Adds global using for caching abstraction in Web unit tests. |
| tests/Web.Tests.Integration/Web.Tests.Integration.csproj | Adds Redis Testcontainers dependency for integration tests. |
| tests/Web.Tests.Integration/Infrastructure/RedisFixture.cs | Introduces Redis container fixture and helper to build cache service instances. |
| tests/Web.Tests.Integration/Infrastructure/RedisCachingCollection.cs | Adds xUnit collection definition for Redis integration tests. |
| tests/Web.Tests.Integration/GlobalUsings.cs | Adds global usings needed for integration caching tests. |
| tests/Web.Tests.Integration/Caching/BlogPostCacheServiceTests.cs | Adds Redis-backed integration tests validating L2 behavior and invalidation. |
| tests/Web.Tests.Bunit/Components/Theme/ThemeSelectorTests.cs | Adjusts JSInterop void setup calls in bUnit theme selector tests. |
| tests/Unit.Tests/Unit.Tests.csproj | Adds new Unit test project to solution structure. |
| tests/Unit.Tests/Handlers/GetBlogPostsHandlerTests.cs | Adds unit tests asserting handler→cache delegation for list endpoint. |
| tests/Unit.Tests/Handlers/GetBlogPostByIdHandlerTests.cs | Adds unit tests asserting handler→cache delegation for by-id query path. |
| tests/Unit.Tests/Handlers/EditBlogPostHandlerTests.cs | Adds unit tests for edit path invalidation behavior via cache abstraction. |
| tests/Unit.Tests/Handlers/DeleteBlogPostHandlerTests.cs | Adds unit tests for delete path invalidation behavior via cache abstraction. |
| tests/Unit.Tests/Handlers/CreateBlogPostHandlerTests.cs | Adds unit tests for create path invalidation behavior via cache abstraction. |
| tests/Unit.Tests/GlobalUsings.cs | Adds global usings for the new Unit test project. |
| tests/Domain.Tests/GlobalUsings.cs | Adds global usings for the new Domain test project. |
| tests/Domain.Tests/Domain.Tests.csproj | Adds new Domain test project. |
| tests/Architecture.Tests/CachingLayerTests.cs | Adds architecture tests preventing direct cache dependency usage in MyBlog.Web.Features. |
| src/Web/Program.cs | Registers caching abstraction with DI (AddBlogPostCaching()). |
| src/Web/Infrastructure/Caching/IBlogPostCacheService.cs | Defines caching contract for list/by-id fetch + invalidation operations. |
| src/Web/Infrastructure/Caching/CachingServiceExtensions.cs | Adds DI registration extension AddBlogPostCaching(). |
| src/Web/Infrastructure/Caching/BlogPostCacheService.cs | Implements L1/L2 cache behavior (TTL, serialization, corruption handling). |
| src/Web/Infrastructure/Caching/BlogPostCacheKeys.cs | Centralizes cache key generation/constants. |
| src/Web/GlobalUsings.cs | Adds global using for caching namespace in Web project. |
| src/Web/Features/BlogPosts/List/GetBlogPostsHandler.cs | Refactors handler to call cache service with repo-backed fetch delegate. |
| src/Web/Features/BlogPosts/Edit/EditBlogPostHandler.cs | Refactors edit + get-by-id behavior to use cache service abstraction. |
| src/Web/Features/BlogPosts/Delete/DeleteBlogPostHandler.cs | Refactors delete handler to invalidate cache via abstraction. |
| src/Web/Features/BlogPosts/Create/CreateBlogPostHandler.cs | Refactors create handler to invalidate cache via abstraction. |
| docs/sprint5-xml-doc-stubs.md | Adds documentation stubs intended for caching abstraction types. |
| docs/adr/sprint5-caching-abstraction.md | Adds ADR documenting rationale and consequences of caching abstraction. |
| MyBlog.slnx | Adds new Domain.Tests and Unit.Tests projects to the solution. |
| Directory.Packages.props | Adds version pin for Testcontainers.Redis. |
| .squad/agents/aragorn/history.md | Minor update to agent history file. |
| .github/workflows/squad-preview.yml | Changes dev-branch validation to run a subset of test projects. |
| .github/workflows/squad-mark-released.yml | Adds workflow to auto-move “Done → Released” on GitHub Release publish. |
| .Returns<ValueTask<BlogPostDto?>>(ci => | ||
| { | ||
| var fetch = ci.Arg<Func<Task<BlogPostDto?>>>(); | ||
| return new ValueTask<BlogPostDto?>(fetch().GetAwaiter().GetResult()); | ||
| }); |
There was a problem hiding this comment.
Same issue here: the setup blocks on an async delegate with GetAwaiter().GetResult(). Return the delegate Task/ValueTask instead so the test remains purely async and doesn’t risk deadlocks.
| var services = new ServiceCollection(); | ||
| services.AddMemoryCache(); | ||
| services.AddStackExchangeRedisCache(opt => opt.Configuration = ConnectionString); | ||
| services.AddBlogPostCaching(); | ||
| return services.BuildServiceProvider().GetRequiredService<IBlogPostCacheService>(); |
There was a problem hiding this comment.
CreateCacheService() builds a new ServiceProvider each call but never disposes it. That can leak Redis connections/timers across tests. Consider returning an owned ServiceProvider (so callers can dispose), or have the fixture track and dispose created providers in DisposeAsync.
| public async Task InvalidateAllAsync_removes_all_entries_from_Redis() | ||
| { | ||
| // Arrange — ensure clean state then populate Redis via service #1 | ||
| await fixture.CreateCacheService().InvalidateAllAsync(); | ||
|
|
There was a problem hiding this comment.
Test name and comments say InvalidateAllAsync “removes all entries from Redis”, but the contract/implementation only removes the "blog:all" key (not per-post keys). Rename this test to reflect the actual behavior to avoid misleading future readers.
| /// <returns> | ||
| /// A <see cref="Task{TResult}"/> that resolves to a read-only list of <see cref="BlogPostDto"/> | ||
| /// instances, or <see langword="null"/> if the entry is not present in either cache tier. | ||
| /// </returns> | ||
| Task<IReadOnlyList<BlogPostDto>?> GetAllAsync(CancellationToken ct); |
There was a problem hiding this comment.
The IBlogPostCacheService stub here documents a different API surface (GetAllAsync/SetAllAsync/GetByIdAsync/SetByIdAsync) than the actual IBlogPostCacheService introduced in this PR (GetOrFetchAllAsync/GetOrFetchByIdAsync with fetch delegates). Please update these stubs to match the current interface so the doc isn’t misleading.
|
|
||
| ### Registration | ||
|
|
||
| `BlogPostCacheService` is registered in `Program.cs` as a scoped or singleton service alongside `IMemoryCache` and the Redis `IDistributedCache`. |
There was a problem hiding this comment.
The ADR says BlogPostCacheService is registered “as a scoped or singleton service”, but the implementation here registers it as a singleton via AddBlogPostCaching(). Update the ADR wording to reflect the actual lifetime decision.
| `BlogPostCacheService` is registered in `Program.cs` as a scoped or singleton service alongside `IMemoryCache` and the Redis `IDistributedCache`. | |
| `BlogPostCacheService` is registered in `Program.cs` as a singleton service alongside `IMemoryCache` and the Redis `IDistributedCache`. |
| dotnet test tests/Architecture.Tests --configuration Release --no-build --verbosity normal | ||
| dotnet test tests/Web.Tests --configuration Release --no-build --verbosity normal | ||
| dotnet test tests/Web.Tests.Integration --configuration Release --no-build --verbosity normal |
There was a problem hiding this comment.
This workflow now runs a subset of test projects and no longer executes the newly added tests/Unit.Tests and tests/Domain.Tests (and also skips Web.Tests.Bunit/AppHost.Tests). If the goal is to keep dev green, consider adding those projects here or explicitly documenting why they’re excluded so regressions in those suites don’t slip through on dev pushes.
| dotnet test tests/Architecture.Tests --configuration Release --no-build --verbosity normal | |
| dotnet test tests/Web.Tests --configuration Release --no-build --verbosity normal | |
| dotnet test tests/Web.Tests.Integration --configuration Release --no-build --verbosity normal | |
| dotnet test tests/Architecture.Tests --configuration Release --no-build --verbosity normal | |
| dotnet test tests/Unit.Tests --configuration Release --no-build --verbosity normal | |
| dotnet test tests/Domain.Tests --configuration Release --no-build --verbosity normal | |
| dotnet test tests/Web.Tests --configuration Release --no-build --verbosity normal | |
| dotnet test tests/Web.Tests.Bunit --configuration Release --no-build --verbosity normal | |
| dotnet test tests/Web.Tests.Integration --configuration Release --no-build --verbosity normal | |
| dotnet test tests/AppHost.Tests --configuration Release --no-build --verbosity normal |
| .Returns<ValueTask<IReadOnlyList<BlogPostDto>>>(ci => | ||
| { | ||
| var fetch = ci.Arg<Func<Task<IReadOnlyList<BlogPostDto>>>>(); | ||
| return new ValueTask<IReadOnlyList<BlogPostDto>>(fetch().GetAwaiter().GetResult()); | ||
| }); |
There was a problem hiding this comment.
The cache-miss setup invokes the async fetch delegate via GetAwaiter().GetResult(), which blocks and can hide deadlocks or change exception behavior. Prefer returning the Task/ValueTask from the delegate directly (similar to the Unit.Tests pattern) so the test remains fully async.
| .Returns<ValueTask<BlogPostDto?>>(ci => | ||
| { | ||
| var fetch = ci.Arg<Func<Task<BlogPostDto?>>>(); | ||
| return new ValueTask<BlogPostDto?>(fetch().GetAwaiter().GetResult()); | ||
| }); |
There was a problem hiding this comment.
This NSubstitute setup calls the async fetch delegate synchronously with GetAwaiter().GetResult(). It’s safer to keep the setup async by returning the Task/ValueTask from the delegate instead of blocking, to avoid deadlock-prone patterns and preserve async exception flow.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #122 +/- ##
==========================================
- Coverage 78.19% 76.93% -1.26%
==========================================
Files 40 43 +3
Lines 642 672 +30
Branches 107 111 +4
==========================================
+ Hits 502 517 +15
- Misses 95 105 +10
- Partials 45 50 +5
🚀 New features to boost your workflow:
|
Sprint 5 Release — Redis & Caching (v1.2.0)
Promotes
dev→mainfor the Sprint 5 release.What's in this release
IBlogPostCacheService— caching contract withGetOrFetchAllAsync,GetOrFetchByIdAsync,InvalidateAllAsync,InvalidateByIdAsyncBlogPostCacheService— two-tier L1 (IMemoryCache, 1-min TTL) + L2 (Redis, 5-min TTL), registered as Singleton viaAddBlogPostCaching()IBlogPostCacheServiceonlydocs/adr/sprint5-caching-abstraction.mdClosed issues
Closes #109, #110, #111, #112, #113, #114
CI status
All checks green on
dev✅