fix(cache): let packages reserve the Redis keyspaces the distributed cache must avoid - #159
Conversation
github-actions
Bot
added
the
needs-cla-review
|
🔎 Maintainer heads-up: automated triage flagged this PR as potentially material, so it may need a signed CLA in addition to the DCO sign-off. Strong signals
This is advisory only — the bot does not decide. Please judge against the CLA criteria (material, product-critical, patent-sensitive, corporate contributor, broad commercial use). Note that thresholds can be gamed by splitting PRs, so use your judgement.
|
There was a problem hiding this comment.
🟡 Changes recommended
Registration-order independence fails when packages use separate AddCaching calls, and some documented behavior lacks coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds extensible Redis keyspace reservations so layered packages can prevent distributed-cache collisions.
Changes:
- Introduces reservation APIs and validation.
- Registers core and Queue keyspaces.
- Adds tests, API declarations, and documentation.
File summaries
| File | Description |
|---|---|
tests/.../DistributedCacheRegistrationTests.cs |
Tests reservations and collisions. |
src/UiPath.Caching/Redis/ReservedRedisKeyspace.cs |
Defines reservation API. |
src/UiPath.Caching/PublicAPI.Unshipped.txt |
Declares public API additions. |
src/UiPath.Caching/Config/ServiceCollectionExtensions.cs |
Reserves core prefixes. |
src/UiPath.Caching/Config/DistributedCacheCollectionExtensions.cs |
Validates registered keyspaces. |
src/UiPath.Caching.Queue/RedisSetCache.cs |
Exposes Queue’s prefix internally. |
src/UiPath.Caching.Queue/Config/QueueCacheCollectionExtensions.cs |
Reserves Queue’s keyspace. |
docs/reference/settings.md |
Documents option validation. |
docs/how-to/extending.md |
Documents package reservations. |
CHANGELOG.md |
Records the fix. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
61f4bf3 to
166b3ce
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Public and reference documentation still describes the previous validation scope and lifecycle.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Balanced
166b3ce to
191635c
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The implementation addresses both registration orders, preserves disabled-registration behavior, and includes focused regression coverage.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
- Review effort level: Balanced
191635c to
0bb1060
Compare
0bb1060 to
92573d9
Compare
92573d9 to
ac1be4a
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The implementation addresses registration ordering and resolution-time custom-strategy collisions with comprehensive tests and consistent documentation.
Review details
- Files reviewed: 24/24 changed files
- Comments generated: 0 new
- Review effort level: Balanced
354c18a to
853fdf3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The glossary misattributes broadcast key rendering, and the probe’s rendering-failure behavior lacks regression coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 25/25 changed files
- Comments generated: 2
- Review effort level: Balanced
853fdf3 to
1d9f29d
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Exact-string reservation matching still permits separator-delimited keyspaces whose generated Redis keys overlap.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Balanced
1d9f29d to
e89b463
Compare
e89b463 to
10a45e4
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Alphanumeric separators can bypass the new single-segment validation and still produce colliding Redis keys.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Balanced
10a45e4 to
4ac1b16
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Nested package keyspaces are not validated unless a Redis-backed distributed cache is resolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 25/25 changed files
- Comments generated: 2
- Review effort level: Balanced
4ac1b16 to
0f9efd4
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Separator-dependent validation is skipped when caching is disabled, and the changelog overstates the segment guarantee.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 26/26 changed files
- Comments generated: 2
- Review effort level: Balanced
…cache must avoid AddDistributedCache compared RedisKeyDifferentiator against a hardcoded list of the core's RedisTypePrefixes, so a package layered on top, such as the Queue set cache on "se", was invisible to it. Packages now declare the prefixes they occupy through IServiceCollection.ReserveRedisKeyspace; the core reserves its four and AddQueueRedis reserves "se". The check runs when AddCaching completes, so it sees every registration whichever order they were added in while still failing at registration, and the resolution-time probe enumerates the same reservations. Closes #130 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V9jRarnMLeZRZ5rYySRhkh Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>
0f9efd4 to
eb98b3c
Compare
|
Closes #130.
The problem
AddDistributedCachecomparedRedisKeyDifferentiatoragainst a hardcoded list of the core's own Redis keyspaces. A package layered on top was invisible to it, so a differentiator of"se"was accepted even thoughUiPath.Caching.Queue's set cache already writes there, and the two would have shared one Redis keyspace.One registry, one rule
Packages declare the keyspaces they occupy through
IServiceCollection.ReserveRedisKeyspace(keyspace, owner). The core reserves its four, andAddQueueRedis/AddQueueInMemoryRedisreserve"se"whether or not that backing is enabled.AddDistributedCachereserves its own differentiator the same way, so a single rule answers every case: the second party to claim a keyspace is rejected, whoever it is.That holds in either registration order, across separate
AddCachingcalls, and whenCacheOptions.Enabledis false, because a keyspace collision is a configuration error rather than a runtime condition. Two packages claiming one keyspace is now rejected as well, where the later reservation used to be dropped silently; a repeat by the same owner is still ignored, sinceAddQueueRedisandAddQueueInMemoryRedisboth reserve.The resolution-time probe that guards a custom
IRedisKeyStrategyFactoryenumerates the same reservations. It skips any keyspace the factory refuses to build a key for, since a factory that rejects a keyspace cannot land on it, and it no longer emits duplicate probes when the type-based and keyspace-based lookups produce the same key.BREAKING:
RedisTypePrefixesis nowRedisKeyspacesThe library called one thing two names: a type prefix in the core, a keyspace in the reservation API. It is a keyspace everywhere now, which renames a shipped public type and the
IReservedRedisKeyspace.Keyspacemember. The constant values are unchanged ("s","h","ps","st"), so no Redis key changes shape and no data moves; already-compiled assemblies keep working and a recompile needsRedisTypePrefixesreplaced withRedisKeyspaces. No obsolete forwarder, deliberately: keeping the old name alive would keep the second vocabulary alive. "Prefix" now means only an actual key prefix.ReservedRedisKeyspace, the concrete class, is internal.ReserveRedisKeyspaceis the supported way in, and a reservation registered any other way is now reported rather than being invisible to the registration-time check while still counting at resolution.Verification
Solution builds with zero warnings. Full suite passes on both frameworks, 1655 tests on .NET 10 and 1634 on .NET 8. Coverage of the new paths includes both registration orders, separate
AddCachingcalls, the disabled-caching path, same-owner versus different-owner reservation, a factory-registered reservation, and a key strategy that rejects another package's keyspace. The queue package's own reservations are pinned in its own test file rather than in the core suite.🤖 Generated with Claude Code
https://claude.ai/code/session_017fwLrS3Sbcen8v6iRkUaFB