Skip to content

Commit

Permalink
Fix for #1564: Return empty array instead of single element nil array…
Browse files Browse the repository at this point in the history
… in value array processor (#1993)

This changes the return of a nil result through the `RedisValueArrayProcessor` so that it's a `[]` instead of a single element `[ nil ]` in our handling.

This affects the following commands, which are multibulk when a count is provided (even if it's 1), otherwise they are bulkstring. This change affects the non-count case when it's null. Instead of a single element array with a nil value, we'd return an empty array as the Redis surface area intends:
- `LPOP`/`RPOP`
- `SRANDMEMBER`
- `SPOP`

The other usages of `RedisValueArrayProcessor` are _always_ multibulk and are not affected:
- `HMGET`
- `HKEYS`
- `HVALS`
- `LRANGE`
- `MGET`
- `SDIFF`
- `SINTER`
- `SUNION`
- `SMEMBERS`
- `SORT`
- `XCLAIM`
- `Z(REV)RANGE`
- `Z(REV)RANGEBYLEX`
- `Z(REV)RANGEBYSCORE`
  • Loading branch information
NickCraver authored Feb 15, 2022
1 parent 34ba699 commit 4fb787c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 1 deletion.
1 change: 1 addition & 0 deletions docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- More correctly reconnects subscriptions on connection failures, including to other endpoints
- Adds "(vX.X.X)" version suffix to the default client ID so server-side `CLIENT LIST` can more easily see what's connected (#1985 via NickCraver)
- Fix for including (or not including) key names on some message failures (#1990 via NickCraver)
- Fixed return of nil results in `LPOP`, `RPOP`, `SRANDMEMBER`, and `SPOP` (#1993 via NickCraver)

## 2.2.88

Expand Down
5 changes: 4 additions & 1 deletion src/StackExchange.Redis/ResultProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1249,7 +1249,10 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
{
// allow a single item to pass explicitly pretending to be an array; example: SPOP {key} 1
case ResultType.BulkString:
var arr = new[] { result.AsRedisValue() };
// If the result is nil, the result should be an empty array
var arr = result.IsNull
? Array.Empty<RedisValue>()
: new[] { result.AsRedisValue() };
SetResult(message, arr);
return true;
case ResultType.MultiBulk:
Expand Down
18 changes: 18 additions & 0 deletions tests/StackExchange.Redis.Tests/Sets.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public void SetPopMulti_Multi()
Assert.Equal(7, db.SetLength(key));
}
}

[Fact]
public void SetPopMulti_Single()
{
Expand Down Expand Up @@ -225,5 +226,22 @@ public async Task SetAdd_Zero_Async()
Assert.Equal(0, db.SetLength(key));
}
}

[Fact]
public void SetPopMulti_Nil()
{
using (var conn = Create())
{
Skip.IfMissingFeature(conn, nameof(RedisFeatures.SetPopMultiple), r => r.SetPopMultiple);

var db = conn.GetDatabase();
var key = Me();

db.KeyDelete(key, CommandFlags.FireAndForget);

var arr = db.SetPop(key, 1);
Assert.Empty(arr);
}
}
}
}

0 comments on commit 4fb787c

Please sign in to comment.