Skip to content

Commit

Permalink
Fix for #2071: StringSet compatibility (#2098)
Browse files Browse the repository at this point in the history
I missed an overload case being an idiot - adding the missing source break for the case in #2071.

...also fixing KeyTouch ordering while in here.

Note that adding `CommandFlags` back optional seems like a quick fix and I did try that route, but in a full test suite here it became apparent that created other ambiguous overload cases, so went this route.
  • Loading branch information
NickCraver authored Apr 26, 2022
1 parent 77a159c commit c1aaf4f
Show file tree
Hide file tree
Showing 11 changed files with 108 additions and 23 deletions.
1 change: 1 addition & 0 deletions docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- Adds: Support for `LMPOP` with `.ListLeftPop()`/`.ListLeftPopAsync()` and `.ListRightPop()`/`.ListRightPopAsync()` ([#2094 by slorello89](https://github.com/StackExchange/StackExchange.Redis/pull/2094))
- Adds: Support for `ZMPOP` with `.SortedSetPop()`/`.SortedSetPopAsync()` ([#2094 by slorello89](https://github.com/StackExchange/StackExchange.Redis/pull/2094))
- Adds: Support for `XAUTOCLAIM` with `.StreamAutoClaim()`/.`StreamAutoClaimAsync()` and `.StreamAutoClaimIdsOnly()`/.`StreamAutoClaimIdsOnlyAsync()` ([#2095 by ttingen](https://github.com/StackExchange/StackExchange.Redis/pull/2095))
- Fix [#2071](https://github.com/StackExchange/StackExchange.Redis/issues/2071): Add `.StringSet()`/`.StringSetAsync()` overloads for source compat broken for 1 case in 2.5.61 ([#2098 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2098))
- Adds: Support for `OBJECT FREQ` with `.KeyFrequency()`/`.KeyFrequencyAsync()` ([#2105 by Avital-Fine](https://github.com/StackExchange/StackExchange.Redis/pull/2105))
- Performance: Avoids allocations when computing cluster hash slots or testing key equality ([#2110 by Marc Gravell](https://github.com/StackExchange/StackExchange.Redis/pull/2110))
- Adds: Support for `SORT_RO` with `.Sort()`/`.SortAsync()` ([#2111 by slorello89](https://github.com/StackExchange/StackExchange.Redis/pull/2111))
Expand Down
17 changes: 7 additions & 10 deletions src/StackExchange.Redis/Interfaces/IDatabase.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Net;

namespace StackExchange.Redis
Expand Down Expand Up @@ -2758,16 +2759,12 @@ IEnumerable<SortedSetEntry> SortedSetScan(RedisKey key,
/// <remarks><seealso href="https://redis.io/commands/strlen"/></remarks>
long StringLength(RedisKey key, CommandFlags flags = CommandFlags.None);

/// <summary>
/// Set key to hold the string value. If key already holds a value, it is overwritten, regardless of its type.
/// </summary>
/// <param name="key">The key of the string.</param>
/// <param name="value">The value to set.</param>
/// <param name="expiry">The expiry to set.</param>
/// <param name="when">Which condition to set the value under (defaults to always).</param>
/// <param name="flags">The flags to use for this operation.</param>
/// <returns><see langword="true"/> if the string was set, <see langword="false"/> otherwise.</returns>
/// <remarks>https://redis.io/commands/set</remarks>
/// <inheritdoc cref="StringSet(RedisKey, RedisValue, TimeSpan?, bool, When, CommandFlags)" />
[Browsable(false), EditorBrowsable(EditorBrowsableState.Never)]
bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when);

/// <inheritdoc cref="StringSet(RedisKey, RedisValue, TimeSpan?, bool, When, CommandFlags)" />
[Browsable(false), EditorBrowsable(EditorBrowsableState.Never)]
bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags);

/// <summary>
Expand Down
17 changes: 7 additions & 10 deletions src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Net;
using System.Threading.Tasks;

Expand Down Expand Up @@ -2710,16 +2711,12 @@ IAsyncEnumerable<SortedSetEntry> SortedSetScanAsync(RedisKey key,
/// <remarks><seealso href="https://redis.io/commands/strlen"/></remarks>
Task<long> StringLengthAsync(RedisKey key, CommandFlags flags = CommandFlags.None);

/// <summary>
/// Set key to hold the string value. If key already holds a value, it is overwritten, regardless of its type.
/// </summary>
/// <param name="key">The key of the string.</param>
/// <param name="value">The value to set.</param>
/// <param name="expiry">The expiry to set.</param>
/// <param name="when">Which condition to set the value under (defaults to always).</param>
/// <param name="flags">The flags to use for this operation.</param>
/// <returns><see langword="true"/> if the string was set, <see langword="false"/> otherwise.</returns>
/// <remarks>https://redis.io/commands/set</remarks>
/// <inheritdoc cref="StringSetAsync(RedisKey, RedisValue, TimeSpan?, bool, When, CommandFlags)" />
[Browsable(false), EditorBrowsable(EditorBrowsableState.Never)]
Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when);

/// <inheritdoc cref="StringSetAsync(RedisKey, RedisValue, TimeSpan?, bool, When, CommandFlags)" />
[Browsable(false), EditorBrowsable(EditorBrowsableState.Never)]
Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags);

/// <summary>
Expand Down
3 changes: 2 additions & 1 deletion src/StackExchange.Redis/KeyspaceIsolation/DatabaseWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,10 @@ public long StringLength(RedisKey key, CommandFlags flags = CommandFlags.None) =
public bool StringSet(KeyValuePair<RedisKey, RedisValue>[] values, When when = When.Always, CommandFlags flags = CommandFlags.None) =>
Inner.StringSet(ToInner(values), when, flags);

public bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when) =>
Inner.StringSet(ToInner(key), value, expiry, when);
public bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) =>
Inner.StringSet(ToInner(key), value, expiry, when, flags);

public bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry = null, bool keepTtl = false, When when = When.Always, CommandFlags flags = CommandFlags.None) =>
Inner.StringSet(ToInner(key), value, expiry, keepTtl, when, flags);

Expand Down
3 changes: 2 additions & 1 deletion src/StackExchange.Redis/KeyspaceIsolation/WrapperBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -660,9 +660,10 @@ public Task<long> StringLengthAsync(RedisKey key, CommandFlags flags = CommandFl
public Task<bool> StringSetAsync(KeyValuePair<RedisKey, RedisValue>[] values, When when = When.Always, CommandFlags flags = CommandFlags.None) =>
Inner.StringSetAsync(ToInner(values), when, flags);

public Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when) =>
Inner.StringSetAsync(ToInner(key), value, expiry, when);
public Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) =>
Inner.StringSetAsync(ToInner(key), value, expiry, when, flags);

public Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry = null, bool keepTtl = false, When when = When.Always, CommandFlags flags = CommandFlags.None) =>
Inner.StringSetAsync(ToInner(key), value, expiry, keepTtl, when, flags);

Expand Down
2 changes: 2 additions & 0 deletions src/StackExchange.Redis/PublicAPI.Shipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,7 @@ StackExchange.Redis.IDatabase.StringIncrement(StackExchange.Redis.RedisKey key,
StackExchange.Redis.IDatabase.StringIncrement(StackExchange.Redis.RedisKey key, long value = 1, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> long
StackExchange.Redis.IDatabase.StringLength(StackExchange.Redis.RedisKey key, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> long
StackExchange.Redis.IDatabase.StringSet(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry = null, bool keepTtl = false, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> bool
StackExchange.Redis.IDatabase.StringSet(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when) -> bool
StackExchange.Redis.IDatabase.StringSet(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when, StackExchange.Redis.CommandFlags flags) -> bool
StackExchange.Redis.IDatabase.StringSet(System.Collections.Generic.KeyValuePair<StackExchange.Redis.RedisKey, StackExchange.Redis.RedisValue>[]! values, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> bool
StackExchange.Redis.IDatabase.StringSetAndGet(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry = null, bool keepTtl = false, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> StackExchange.Redis.RedisValue
Expand Down Expand Up @@ -927,6 +928,7 @@ StackExchange.Redis.IDatabaseAsync.StringLengthAsync(StackExchange.Redis.RedisKe
StackExchange.Redis.IDatabaseAsync.StringSetAndGetAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry = null, bool keepTtl = false, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task<StackExchange.Redis.RedisValue>!
StackExchange.Redis.IDatabaseAsync.StringSetAndGetAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when, StackExchange.Redis.CommandFlags flags) -> System.Threading.Tasks.Task<StackExchange.Redis.RedisValue>!
StackExchange.Redis.IDatabaseAsync.StringSetAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry = null, bool keepTtl = false, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when, StackExchange.Redis.CommandFlags flags) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetAsync(System.Collections.Generic.KeyValuePair<StackExchange.Redis.RedisKey, StackExchange.Redis.RedisValue>[]! values, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetBitAsync(StackExchange.Redis.RedisKey key, long offset, bool bit, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task<bool>!
Expand Down
6 changes: 6 additions & 0 deletions src/StackExchange.Redis/RedisDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3018,6 +3018,9 @@ public Task<long> StringLengthAsync(RedisKey key, CommandFlags flags = CommandFl
return ExecuteAsync(msg, ResultProcessor.Int64);
}

// Backwards compatibility overloads:
public bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when) =>
StringSet(key, value, expiry, false, when, CommandFlags.None);
public bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) =>
StringSet(key, value, expiry, false, when, flags);

Expand All @@ -3033,6 +3036,9 @@ public bool StringSet(KeyValuePair<RedisKey, RedisValue>[] values, When when = W
return ExecuteSync(msg, ResultProcessor.Boolean);
}

// Backwards compatibility overloads:
public Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when) =>
StringSetAsync(key, value, expiry, false, when);
public Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) =>
StringSetAsync(key, value, expiry, false, when, flags);

Expand Down
8 changes: 8 additions & 0 deletions tests/StackExchange.Redis.Tests/DatabaseWrapperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1317,6 +1317,14 @@ public void StringSet_3()
mock.Verify(_ => _.StringSet(It.Is(valid), When.Exists, CommandFlags.None));
}

[Fact]
public void StringSet_Compat()
{
TimeSpan? expiry = null;
wrapper.StringSet("key", "value", expiry, When.Exists);
mock.Verify(_ => _.StringSet("prefix:key", "value", expiry, When.Exists));
}

[Fact]
public void StringSetBit()
{
Expand Down
10 changes: 9 additions & 1 deletion tests/StackExchange.Redis.Tests/Naming.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
Expand Down Expand Up @@ -138,7 +139,14 @@ public void CheckSyncAsyncMethodsMatch(Type from, Type to)
var pFrom = method.GetParameters();
Type[] args = pFrom.Select(x => x.ParameterType).ToArray();
Log("Checking: {0}.{1}", from.Name, method.Name);
Assert.Equal(typeof(CommandFlags), args.Last());
if (method.GetCustomAttribute<EditorBrowsableAttribute>() is EditorBrowsableAttribute attr && attr.State == EditorBrowsableState.Never)
{
// For compatibility overloads, explicitly don't ensure CommandFlags is last
}
else
{
Assert.Equal(typeof(CommandFlags), args.Last());
}
var found = to.GetMethod(huntName, flags, null, method.CallingConvention, args, null);
Assert.NotNull(found); // "Found " + name + ", no " + huntName
var pTo = found.GetParameters();
Expand Down
56 changes: 56 additions & 0 deletions tests/StackExchange.Redis.Tests/OverloadCompat.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
using System;
using System.Threading.Tasks;
using Xunit;
using Xunit.Abstractions;

namespace StackExchange.Redis.Tests;

/// <summary>
/// This test set is for when we add an overload, to making sure all
/// past versions work correctly and aren't source breaking.
/// </summary>
[Collection(SharedConnectionFixture.Key)]
public class OverloadCompat : TestBase
{
public OverloadCompat(ITestOutputHelper output, SharedConnectionFixture fixture) : base (output, fixture) { }

[Fact]
public async Task StringGet()
{
using var conn = Create();
var db = conn.GetDatabase();
RedisKey key = Me();
RedisValue val = "myval";
var expiresIn = TimeSpan.FromSeconds(10);
var when = When.Always;
var flags = CommandFlags.None;

db.StringSet(key, val);
db.StringSet(key, val, expiry: expiresIn);
db.StringSet(key, val, when: when);
db.StringSet(key, val, flags: flags);
db.StringSet(key, val, expiry: expiresIn, when: when);
db.StringSet(key, val, expiry: expiresIn, when: when, flags: flags);
db.StringSet(key, val, expiry: expiresIn, when: when, flags: flags);

db.StringSet(key, val, expiresIn, When.NotExists);
db.StringSet(key, val, expiresIn, When.NotExists, flags);
db.StringSet(key, val, null);
db.StringSet(key, val, null, When.NotExists);
db.StringSet(key, val, null, When.NotExists, flags);

await db.StringSetAsync(key, val);
await db.StringSetAsync(key, val, expiry: expiresIn);
await db.StringSetAsync(key, val, when: when);
await db.StringSetAsync(key, val, flags: flags);
await db.StringSetAsync(key, val, expiry: expiresIn, when: when);
await db.StringSetAsync(key, val, expiry: expiresIn, when: when, flags: flags);
await db.StringSetAsync(key, val, expiry: expiresIn, when: when, flags: flags);

await db.StringSetAsync(key, val, expiresIn, When.NotExists);
await db.StringSetAsync(key, val, expiresIn, When.NotExists, flags);
await db.StringSetAsync(key, val, null);
await db.StringSetAsync(key, val, null, When.NotExists);
await db.StringSetAsync(key, val, null, When.NotExists, flags);
}
}
8 changes: 8 additions & 0 deletions tests/StackExchange.Redis.Tests/WrapperBaseTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,14 @@ public void StringSetAsync_3()
mock.Verify(_ => _.StringSetAsync(It.Is(valid), When.Exists, CommandFlags.None));
}

[Fact]
public void StringSetAsync_Compat()
{
TimeSpan expiry = TimeSpan.FromSeconds(123);
wrapper.StringSetAsync("key", "value", expiry, When.Exists);
mock.Verify(_ => _.StringSetAsync("prefix:key", "value", expiry, When.Exists));
}

[Fact]
public void StringSetBitAsync()
{
Expand Down

0 comments on commit c1aaf4f

Please sign in to comment.