There is no condition here at all - the transaction exists only to make two commands atomic, and a single command already does both.
// flagged
var tran = db.CreateTransaction();
var value = tran.StringGetAsync(key);
_ = tran.KeyDeleteAsync(key);
await tran.ExecuteAsync();
// suggested
RedisValue value = await db.StringGetDeleteAsync(key);
| Queued pair | Single command | Server |
|---|---|---|
StringGet + KeyDelete |
StringGetDelete (GETDEL) |
6.2 |
StringGet + KeyExpire |
StringGetSetExpiry (GETEX) |
6.2 |
StringGet + KeyPersist |
StringGetSetExpiry(key, null) (GETEX PERSIST) |
6.2 |
StringGet + StringSet |
StringSetAndGet (SET … GET) |
6.2 |
HashGet + HashDelete |
HashFieldGetAndDelete (HGETDEL) |
8.0 |
StringSet + KeyExpire |
StringSet(key, value, expiry) (SET … EX) |
any |
SetRemove + SetAdd |
SetMove (SMOVE) |
any |
The requirement varies across this family, from “any server” for SMOVE up to 8.0 for HGETDEL, so each message names its own - see declaring your server version to be shown only what your server supports.
One caveat on StringSet + KeyExpire: an absolute expiry works too, because Expiration converts implicitly
from DateTime as well as TimeSpan - but the SET ... EXAT that produces does want a 6.2 server, where the
relative form has worked since 2.6.12. The version column above is the relative case, which is the common one.
Order matters
Which way round the pair is queued is part of the meaning, for two different reasons.
The reads return a value: SET ... GET hands back the value from before the write, so it matches a queued get
followed by a set - and not a set followed by a get, which asks for the value afterwards. That pairing is
left alone.
The writes overwrite each other: SET clears any TTL on the key, so StringSet + KeyExpire is one command
with a lifetime, while KeyExpire + StringSet ends with no expiry at all. Only the first order is flagged.
For the same reason a StringSet that already carries an expiry, followed by a KeyExpire that overrides it,
is left alone: which of the two lifetimes the single command should carry is a guess.
SetRemove + SetAdd is the exception: within a transaction both effects happen regardless of order, so either
spelling is flagged.
What changes when you apply it
tran.Execute()returns whether the transaction ran; the compound command returns its own result - usually the value you were reading anyway.- The queued
Task<T>s collapse into the single command’s result. CommandFlagsmust be carried over verbatim.
Cases that are deliberately not flagged
ListRightPop+ListLeftPush. This looks likeLMOVE, and it is not.LMOVEmoves the element it popped; inside a transaction the pop’s result is an unresolvedTask, so the caller cannot pass it to the push - whatever value is being pushed is a different one, andLMOVEwould not reproduce it. The same reasoning rules out every read-modify-write pairing.- Different keys (or different members, for
SetMove) - those are genuinely two operations. - Anything with a condition, which is SER300-SER302 territory.
- A key local reassigned between the two calls - the keys are compared as source text, so a reassignment means identical text can be two different keys, and the rule stays quiet.
- Three or more queued commands, and anything queued in a loop. Note that the same command repeated - which can be three or more - is SER304 rather than this rule.
Plus everything under when these rules stay quiet, which is where the family-wide cases live - a third queued command, commands in different branches, and arguments the compound command cannot carry.
See also Transactions.
Guidance, not a verdict
This rule is a heuristic. It reads your source text - it cannot see your keys, your server, or what you know about the code - so it is deliberately conservative and stays quiet wherever it is unsure. Everything it flags still works, and will keep working: this is a suggestion, not a defect report.
That conservatism is meant to make a false positive rare, not impossible. If you think the rule has flagged something it should not have, please report it, including the transaction as written. A rule that fires on correct code is a bug in the rule - and one that reaches every consumer of the package - so it is worth fixing rather than quietly suppressing.
Suppressing
Reported as a warning, so TreatWarningsAsErrors builds fail until you act on it or turn it down.
<NoWarn>$(NoWarn);SER303</NoWarn>
or locally:
#pragma warning disable SER303