imperugo / imperugo/StackExchange.Redis.Extensions

Add ValueTuple overloads to AddAllAsync and deprecate the Tuple ones

Open
#662 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
C#
Stars
628
Forks
177
PR merge metrics
No merged PRs in 30d

Description

Suggested by @LeaFrock.

Removal of the deprecated overloads is tracked separately in #663 (v14.0).

Proposal

Add (string Key, T Value)[] overloads to the three AddAllAsync bulk methods on IRedisDatabase, and mark the existing Tuple<string, T>[] ones [Obsolete]:

// new — the path forward
Task<bool> AddAllAsync<T>((string Key, T Value)[] items, When when = When.Always, CommandFlags flag = CommandFlags.None);
Task<bool> AddAllAsync<T>((string Key, T Value)[] items, DateTimeOffset expiresAt, When when = When.Always, CommandFlags flag = CommandFlags.None);
Task<bool> AddAllAsync<T>((string Key, T Value)[] items, TimeSpan expiresAt, When when = When.Always, CommandFlags flag = CommandFlags.None);

// existing — deprecated, removed in v14 (#663)
[Obsolete("Use the (string Key, T Value)[] overload. This overload will be removed in v14.", DiagnosticId = "SRE0001")]
Task<bool> AddAllAsync<T>(Tuple<string, T>[] items, When when = When.Always, CommandFlags flag = CommandFlags.None);
// ...same for the two expiry overloads

This ships in v13.5 as a fully backward-compatible minor: existing code keeps compiling and binding to the Tuple overloads, with a warning pointing at the replacement.

Rationale

Although both are arrays, the memory layout is fundamentally different. Tuple<string, T>[] is an array of references pointing at separately allocated heap objects: for N items that is N individual allocations (24+ bytes of object header and fields each on 64-bit) plus the reference array, scattered across the heap and each requiring a pointer dereference to read. (string, T)[] is a single contiguous block of values — one allocation total, sequential in memory, far friendlier to both the GC and the CPU cache.

This matters specifically here because AddAllAsync is the bulk-write API: it is the method most likely to be called with large batches, which is exactly where per-element allocation overhead compounds.

Named elements (Key, Value) also read better at the call-site than Item1/Item2.

Implementation notes

The two overload sets do not collide. Tuple<string,T>[] and (string,T)[] are distinct types with no implicit conversion between them, so existing call-sites keep resolving to the deprecated overloads with no ambiguity error.

TreatWarningsAsErrors is true in this repo, so [Obsolete] breaks our own build. There are 23 internal call-sites — 5 in src/, 18 in the test project. The ones in src/ should be migrated to the new overloads. The tests need to keep exercising the deprecated path for as long as we still ship it, so those call-sites need a localised #pragma warning disable CS0618 with a comment explaining that it is intentional until #663.

Mark both the interface and the implementation. An [Obsolete] on IRedisDatabase alone is invisible to anyone typing their variable as the concrete RedisDatabase.

Use DiagnosticId. A dedicated id (e.g. SRE0001) lets consumers suppress this specific deprecation instead of blanket-disabling CS0618 across their project, which would also hide unrelated deprecations from other libraries.

Do not make the new path pay for the old one. The internal helper ValueLengthExtensions.ToRedisEntries<T> currently takes Tuple<string, T>[]. The tempting shortcut is to have the deprecated overloads convert their array to ValueTuple and delegate, but that adds an allocation on exactly the bulk API we are trying to optimise. It is a ten-line internal helper — duplicate it for the two input types so each overload does a single pass.

Scope

  • IRedisDatabase.cs — 3 new overload declarations, 3 existing ones marked [Obsolete]
  • RedisDatabase.cs — 3 new implementations, 3 existing ones marked [Obsolete]
  • ValueLengthExtensions.csToRedisEntries<T> overload for (string, T)[]
  • Tests — cover the new overloads; keep the existing cases with scoped CS0618 suppressions
  • Documentation — doc/, llms.txt, llms-full.txt, plus a deprecations section in a new doc/migration-v13-to-v14.md so 13.5 users already know what is going away

Acceptance criteria

  • ValueTuple overloads added and covered by tests
  • Tuple overloads marked [Obsolete] with DiagnosticId, on both interface and implementation
  • Internal call-sites in src/ migrated to the new overloads
  • Test call-sites for the deprecated path suppressed locally with an explanatory comment
  • Build clean with TreatWarningsAsErrors on all TFMs
  • Deprecation documented for consumers upgrading to 13.5

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with IRedisDatabase.cs and RedisDatabase.cs to compare the three existing AddAllAsync overloads, then inspect ValueLengthExtensions.cs and the current tests. Add and test the ValueTuple overloads, deprecate both Tuple paths on the interface and implementation, migrate internal call-sites, and update the listed documentation. Verify the build remains clean with TreatWarningsAsErrors across all TFMs.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, redis
Domain
backend, databases, documentation, testing
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.