Handle null cache client implementations in DarcLib

[adamzip]

#4737

[Copilot]

Pull Request Overview

This PR refactors the cache client abstraction in DarcLib by replacing the Redis-specific interface with a generic distributed cache interface and adds null safety validation. The purpose is to make the caching layer more generic and prevent null reference exceptions when cache clients are not properly configured.

• Replaces IRedisCacheClient with IDistributedCacheClient interface for better abstraction
• Renames NoOpRedisClient to NoOpCacheClient to reflect the generic nature
• Adds null validation in the Remote constructor with descriptive error messaging

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/Microsoft.DotNet.Darc/DarcLib/Remote.cs	Updates constructor to use generic cache interface and adds null validation with descriptive error message
src/Microsoft.DotNet.Darc/DarcLib/IRedisCacheClient.cs	Removes Redis-specific interface and no-op implementation (file deleted)
src/ProductConstructionService/ProductConstructionService.Common/RedisCacheClient.cs	Updates to implement generic interface, adds default expiration, and improves documentation
src/ProductConstructionService/ProductConstructionService.Common/ProductConstructionServiceExtension.cs	Updates DI registration to use generic cache interface
src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs	Updates service registration to use renamed no-op cache client
test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs	Updates test to use renamed no-op cache client class
test/Microsoft.DotNet.DarcLib.Tests/RemoteTests.cs	Updates test to use renamed no-op cache client class

[premun]

I think the cache key should be something more than a SHA. Maybe SourceManifest_[SHA]?
(because maybe we will cache other files from that SHA in the future?)

And also so that we can tell what it is when looking at redis.

[adamzip]

I think the cache key should be something more than a SHA. Maybe SourceManifest_[SHA]? (because maybe we will cache other files from that SHA in the future?)

And also so that we can tell what it is when looking at redis.

The key in redis will be SourceManifestWrapper_SHA, because the implementation uses our existing RedisCacheFactory, which does include the type name in the key.

[premun]

@adamzip I was asking in Teams but probably you missed that one - why does it need to be Distributed? It can very well be just memory cache, no? Isn't it misleading? It just needs to be a cache?

[adamzip]

It should be persistent if possible but I suppose technically it can be a memory cache, yes. So do you propose just ICacheClient?