[WIP] Add an extension for Redis Cache in AppHost#90
Conversation
Co-authored-by: mpaulosky <60372079+mpaulosky@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Redis cache support to the Aspire AppHost extension layer, including dashboard commands to clear Redis data, aligning with the existing MongoDB management-command pattern.
Changes:
- Extend
AddRedisCacheto support a Redis database index and register two dashboard commands (clear selected DB, clear all DBs). - Add
StackExchange.Redisdependency and global using to support Redis-specific exceptions/types. - Update/add AppHost unit tests for the new Redis extension behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/AppHost.Tests/GlobalUsings.cs | Adds global using for AppHost helper types used by tests. |
| tests/AppHost.Tests/Extensions/RedisExtensionsTests.cs | Updates Redis extension tests and adds new cases for the database parameter and command/annotation presence. |
| src/AppHost/GlobalUsings.cs | Adds global using for StackExchange.Redis. |
| src/AppHost/Extensions/RedisExtensions.cs | Adds database parameter + dashboard commands for clearing Redis DB(s). |
| src/AppHost/AppHost.csproj | Adds StackExchange.Redis package reference to AppHost. |
| /// <summary> | ||
| /// Test that AddRedisCache with custom database number returns a valid resource builder. | ||
| /// </summary> | ||
| [Fact] | ||
| public void AddRedisCache_WithCustomDatabase_ReturnsResourceBuilder() | ||
| { | ||
| // Arrange | ||
| var builder = DistributedApplication.CreateBuilder(); | ||
| const int database = 1; | ||
|
|
||
| // Act | ||
| var result = builder.AddRedisCache(database: database); | ||
|
|
||
| // Assert | ||
| result.Should().NotBeNull(); | ||
| } |
There was a problem hiding this comment.
These tests don't currently assert any behavior related to the database parameter (they only check the result is non-null). This makes it easy for a regression where the database argument is ignored to slip through. Consider asserting on the created command annotations/options (e.g., that the "Clear Cache" command description references the selected database, or that the command metadata carries the expected database value).
| /// <summary> | ||
| /// Test that AddRedisCache uses default database 0 when not provided. | ||
| /// </summary> | ||
| [Fact] | ||
| public void AddRedisCache_WithoutDatabase_UsesDefaultDatabase() | ||
| { | ||
| // Arrange | ||
| var builder = DistributedApplication.CreateBuilder(); | ||
|
|
||
| // Act | ||
| var result = builder.AddRedisCache(); | ||
|
|
||
| // Assert | ||
| result.Should().NotBeNull(); | ||
| } |
There was a problem hiding this comment.
AddRedisCache_WithoutDatabase_UsesDefaultDatabase doesn't verify the default database behavior (it only asserts non-null). If the intent is to guarantee database 0 is used when omitted, add an assertion against the generated command metadata (e.g., the "Clear Cache" command description mentions database 0, or there are command annotations configured for db 0).
| // Assert | ||
| redis.Resource.Annotations.Should().NotBeEmpty("Resource should have annotations"); | ||
| redis.Resource.Annotations.Should().Contain(a => a.GetType().Name.Contains("Command"), | ||
| "Resource should have command annotations"); | ||
| } |
There was a problem hiding this comment.
This test only checks that some annotation type name contains "Command". Since AddRedisCache registers two specific dashboard commands, it would be more robust to assert that both expected commands are present (e.g., by counting command annotations or matching their names/display names).
| <PackageReference Include="Aspire.Hosting.MongoDB" /> | ||
| <PackageReference Include="Aspire.Hosting.Redis" /> | ||
| <PackageReference Include="MongoDB.Driver" /> | ||
| <PackageReference Include="StackExchange.Redis" /> |
There was a problem hiding this comment.
PR title still includes "[WIP]" while the PR description indicates the feature is complete. Consider removing the WIP marker (or updating the description) so merge readiness is unambiguous.
| await CacheAdminHelper.ClearRedisDatabaseAsync(connectionString, database); | ||
| return CommandResults.Success(); | ||
| } | ||
| catch (RedisException) | ||
| { | ||
| // Redis-specific errors (connection, authentication, etc.) | ||
| return CommandResults.Failure(); | ||
| } |
There was a problem hiding this comment.
The command handler is catching RedisException, but CacheAdminHelper.ClearRedisDatabaseAsync wraps all failures in InvalidOperationException (see src/AppHost/Helpers/CacheAdminHelper.cs). As a result, Redis connection/auth failures will bypass this catch block and can surface as unhandled exceptions in the dashboard command. Catch InvalidOperationException (or a broader Exception) here, or adjust the helper to let RedisException propagate so the command returns CommandResults.Failure() as intended.
| { | ||
| // Redis-specific errors (connection, authentication, etc.) | ||
| return CommandResults.Failure(); | ||
| } |
There was a problem hiding this comment.
Same issue in the "Clear All Databases" command: CacheAdminHelper.ClearAllRedisDatabasesAsync throws InvalidOperationException on failures, so catching only RedisException will miss the actual error path and can lead to an unhandled exception instead of a clean CommandResults.Failure() response.
| } | |
| } | |
| catch (InvalidOperationException) | |
| { | |
| // Helper reported an invalid operation while clearing all databases | |
| return CommandResults.Failure(); | |
| } |
| @@ -50,9 +55,97 @@ public static IResourceBuilder<RedisResource> AddRedisCache( | |||
| throw new ArgumentException("Name cannot be null or empty.", nameof(name)); | |||
| } | |||
|
|
|||
| return builder | |||
| if (database < 0) | |||
| { | |||
| throw new ArgumentException("Database number cannot be negative.", nameof(database)); | |||
| } | |||
There was a problem hiding this comment.
Input validation here is implemented with manual if/throw blocks, but the other AppHost extensions use ArgumentNullException.ThrowIfNull / ArgumentException.ThrowIfNullOrWhiteSpace (e.g., MongoDBHostingExtensions). Consider switching to the same guard-clause pattern for consistency and to avoid diverging exception behavior/messages across extensions.
Redis Cache Extension with Dashboard Commands - COMPLETE ✅
Summary
Successfully implemented Redis Cache extension for AppHost with dashboard commands to manage Redis cache data.
Features Added
Changes Made
Technical Details
Testing
✅ All 31 unit tests pass
✅ Full solution builds successfully
✅ No breaking changes to existing functionality
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.