chore(cache): deprecate ShardKeyEnabled, which changes which keys co-locate and nothing else

[cosmin-staicu]

Follow-up to #177. Docs and an [Obsolete] only — no behaviour change.

Why

CacheOptions.ShardKeyEnabled wraps an untagged cache key in a {...} hash tag so the slot follows the key rather than AppShortName and the keyspace. It promises cluster sharding and delivers almost none of it.

It does not improve distribution. The tag it adds is the whole key, so it is unique per key and CRC16 spreads it exactly as the untagged key would.

It cannot make a multi-key batch single-slot. Same reason — every key gets its own tag, so GetAsync(CacheKey[]) is cross-slot with the flag on just as without it.

It makes no difference even to a caller who tags their own keys. Take {org1}:a and {org1}:b:

	rendered key	Redis tag
ShardKeyEnabled=true	app:s:{org1}:a — valid tag, left alone	org1
ShardKeyEnabled=false	app:s:{org1}:a — prefix joined	org1

Identical. So the flag is a no-op for batching in both directions.

What it actually does is co-locate the same key across the s and h keyspaces and across apps with a different AppShortName. Neither is something anyone asked for, and the second is what prompted the question that led here.

Meanwhile everything that matters on a cluster is already unconditional:

Concern	Handled by
A caller's own hash tag survives	#166
A cross-slot batch is refused, naming the two keys that disagree	#166
Keys the library itself must touch together co-locate	#177

None of those consult ShardKeyEnabled.

What changed

• [Obsolete] on the property, with a message saying what it really does and that flipping it relocates every entry.
• DefaultRedisKeyStrategyFactory suppresses its own CS0618. The flag is still honored — a deployment that set it would relocate every key if it stopped being read.
• docs/reference/settings.md no longer tells readers to "enable for Redis Cluster deployments that span multiple shards", and points them at giving their keys a shared hash tag instead.
• docs/how-to/telemetry-and-strategies.md no longer describes it as handling "shard-key routing automatically".
• A ### Deprecated entry in the changelog.

ShardPrefixRedisKeyStrategy stays as it is: it is public API, it is what the still-honored flag selects, and a custom IRedisKeyStrategyFactory can use it deliberately. It would go with the flag in a future major, not here.

Tests

Unchanged and green — net10.0 1704 pass, net8.0 1683 pass, 0 failures (Redis integration tests skipped, no Docker). Build clean at 0 warnings with the suppression scoped to the one line that reads the flag.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NsPw6MZHPGmpbo6WDuLzF1

[Copilot]

🟡 Changes recommended

Sample guidance remains contradictory, and one changelog guarantee depends on unmerged PR #177.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Deprecates ShardKeyEnabled while preserving existing key behavior and correcting Redis Cluster guidance.

Changes:

• Marks ShardKeyEnabled obsolete while retaining factory behavior.
• Updates documentation and changelog with migration guidance.
• Clarifies caller-defined hash tags for batching.

File summaries

File	Description
src/UiPath.Caching/Redis/DefaultRedisKeyStrategyFactory.cs	Suppresses the intentional obsolete-property access.
src/UiPath.Caching.Abstractions/CacheOptions.cs	Marks ShardKeyEnabled obsolete.
docs/reference/settings.md	Documents deprecation and alternatives.
docs/how-to/telemetry-and-strategies.md	Corrects strategy guidance.
CHANGELOG.md	Records the deprecation.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Documentation remains inconsistent, and it claims behavior supplied only by the still-unmerged PR #177.

Review details

Suppressed comments (2)

docs/reference/settings.md:23

• This reference explicitly says it mirrors samples/UiPath.Caching.Sample/appsettings.all.json, but that file still documents ShardKeyEnabled as “enable for Redis Cluster with multiple shards” (line 12). Readers following the linked binding reference will therefore still receive the recommendation this deprecation is meant to remove. Update the mirrored JSON comment (and the sample's cluster-mode guidance if it is no longer intentional) in this PR.

| `ShardKeyEnabled` | `bool` | `false` | App-wide | **Deprecated.** Wraps the cache key in a `{...}` hash tag so only the key, not `AppShortName` or the keyspace, picks the slot. That changes *which* keys share a slot, not how evenly they spread: the tag is the whole key, so it is unique per key and distributes exactly as the untagged key would, and it cannot make a multi-key batch single-slot. A caller that placed its own `{tag}` keeps the slot it chose with or without it. Its only effect is co-locating the same key across the `s` and `h` keyspaces and across apps with a different `AppShortName`. Flipping it rewrites every key, so leave it as your deployment has it. To batch across keys on a cluster, give them a shared hash tag yourself. |

CHANGELOG.md:382

• This guarantee is not present on the current base: RedisStreamHealthMaintainer still concatenates a separately rendered quarantine prefix and then issues a two-key delete, so those library-owned keys can remain cross-slot. GitHub PR #177 contains that fix but is still open and unmerged. Make #177 a merge prerequisite/rebase this branch onto it, or avoid publishing this behavior as already unconditional.

  of which anyone asked for. Everything that matters on a cluster is now unconditional: a caller's own tag survives,
  a cross-slot batch is refused with a message naming the two keys, and keys the library itself must touch together
  co-locate. The property is still read, because a deployment that set it would relocate every entry if it stopped

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Several statements assume #177’s malformed-brace handling and incorrectly claim that every key relocates when the setting changes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 7
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The documentation incorrectly limits keyspace co-location to s and h, omitting set-cache and custom differentiators.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The suppression justification still incorrectly claims that every key would relocate.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Several descriptions incorrectly state that every brace in a rendered prefix supplies an earlier Redis hash tag.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

Behavior remains compatible, with only a minor non-blocking terminology correction identified.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The migration documentation remains incomplete and internally inconsistent for supported brace-containing prefixes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

CHANGELOG.md:398

• This list also misses an unmatched opening { in the rendered prefix. Because that value is accepted, it can pair with a later } from the key or wrapper and become the first unintended Redis tag; add that fourth outcome so the migration note covers all supported brace shapes.

  picks the tag — a valid pair there comes first and overrides the key's own, while an empty first pair disables
  tagging altogether so the whole rendered key is hashed, and a stray `}` ahead of any `{` is ignored.

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The deprecation preserves runtime behavior while providing consistent migration guidance across code, tests, samples, and documentation.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

🟡 Changes recommended

Several deprecation messages omit or contradict supported prefix and malformed-key behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

CHANGELOG.md:395

• This brace-free assumption omits RedisCacheOptions.KeyPrefix, even though it is prepended before AppShortName and can therefore supply or alter the first Redis hash tag. Include it so the preceding slot-selection claims cover the complete physical prefix.

  across keys on a cluster, give the keys a shared hash tag yourself. All of this assumes `AppShortName`,
  `Separator` and the differentiator are brace-free, since braces there change which span Redis hashes.

• Files reviewed: 8/8 changed files
• Comments generated: 5
• Review effort level: Balanced