From c1aaf4f990544b17a7cdcf773bbef1309c88d73c Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Tue, 26 Apr 2022 10:26:12 -0400 Subject: [PATCH] Fix for #2071: StringSet compatibility (#2098) 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. --- docs/ReleaseNotes.md | 1 + .../Interfaces/IDatabase.cs | 17 +++--- .../Interfaces/IDatabaseAsync.cs | 17 +++--- .../KeyspaceIsolation/DatabaseWrapper.cs | 3 +- .../KeyspaceIsolation/WrapperBase.cs | 3 +- src/StackExchange.Redis/PublicAPI.Shipped.txt | 2 + src/StackExchange.Redis/RedisDatabase.cs | 6 ++ .../DatabaseWrapperTests.cs | 8 +++ tests/StackExchange.Redis.Tests/Naming.cs | 10 +++- .../OverloadCompat.cs | 56 +++++++++++++++++++ .../WrapperBaseTests.cs | 8 +++ 11 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 tests/StackExchange.Redis.Tests/OverloadCompat.cs diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index e4197240a..a3bf5e25f 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -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)) diff --git a/src/StackExchange.Redis/Interfaces/IDatabase.cs b/src/StackExchange.Redis/Interfaces/IDatabase.cs index 2485ad27f..d80603dee 100644 --- a/src/StackExchange.Redis/Interfaces/IDatabase.cs +++ b/src/StackExchange.Redis/Interfaces/IDatabase.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Net; namespace StackExchange.Redis @@ -2758,16 +2759,12 @@ IEnumerable SortedSetScan(RedisKey key, /// long StringLength(RedisKey key, CommandFlags flags = CommandFlags.None); - /// - /// Set key to hold the string value. If key already holds a value, it is overwritten, regardless of its type. - /// - /// The key of the string. - /// The value to set. - /// The expiry to set. - /// Which condition to set the value under (defaults to always). - /// The flags to use for this operation. - /// if the string was set, otherwise. - /// https://redis.io/commands/set + /// + [Browsable(false), EditorBrowsable(EditorBrowsableState.Never)] + bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when); + + /// + [Browsable(false), EditorBrowsable(EditorBrowsableState.Never)] bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags); /// diff --git a/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs b/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs index 50d129eea..d0fcaa355 100644 --- a/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs +++ b/src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Net; using System.Threading.Tasks; @@ -2710,16 +2711,12 @@ IAsyncEnumerable SortedSetScanAsync(RedisKey key, /// Task StringLengthAsync(RedisKey key, CommandFlags flags = CommandFlags.None); - /// - /// Set key to hold the string value. If key already holds a value, it is overwritten, regardless of its type. - /// - /// The key of the string. - /// The value to set. - /// The expiry to set. - /// Which condition to set the value under (defaults to always). - /// The flags to use for this operation. - /// if the string was set, otherwise. - /// https://redis.io/commands/set + /// + [Browsable(false), EditorBrowsable(EditorBrowsableState.Never)] + Task StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when); + + /// + [Browsable(false), EditorBrowsable(EditorBrowsableState.Never)] Task StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags); /// diff --git a/src/StackExchange.Redis/KeyspaceIsolation/DatabaseWrapper.cs b/src/StackExchange.Redis/KeyspaceIsolation/DatabaseWrapper.cs index 62a9ae6fb..c2a9fff8a 100644 --- a/src/StackExchange.Redis/KeyspaceIsolation/DatabaseWrapper.cs +++ b/src/StackExchange.Redis/KeyspaceIsolation/DatabaseWrapper.cs @@ -643,9 +643,10 @@ public long StringLength(RedisKey key, CommandFlags flags = CommandFlags.None) = public bool StringSet(KeyValuePair[] 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); diff --git a/src/StackExchange.Redis/KeyspaceIsolation/WrapperBase.cs b/src/StackExchange.Redis/KeyspaceIsolation/WrapperBase.cs index 200cd67b5..c7b0d929f 100644 --- a/src/StackExchange.Redis/KeyspaceIsolation/WrapperBase.cs +++ b/src/StackExchange.Redis/KeyspaceIsolation/WrapperBase.cs @@ -660,9 +660,10 @@ public Task StringLengthAsync(RedisKey key, CommandFlags flags = CommandFl public Task StringSetAsync(KeyValuePair[] values, When when = When.Always, CommandFlags flags = CommandFlags.None) => Inner.StringSetAsync(ToInner(values), when, flags); + public Task StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when) => + Inner.StringSetAsync(ToInner(key), value, expiry, when); public Task StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) => Inner.StringSetAsync(ToInner(key), value, expiry, when, flags); - public Task 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); diff --git a/src/StackExchange.Redis/PublicAPI.Shipped.txt b/src/StackExchange.Redis/PublicAPI.Shipped.txt index 517c48d3a..290c1a2ca 100644 --- a/src/StackExchange.Redis/PublicAPI.Shipped.txt +++ b/src/StackExchange.Redis/PublicAPI.Shipped.txt @@ -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[]! 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 @@ -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.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.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! +StackExchange.Redis.IDatabaseAsync.StringSetAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when) -> System.Threading.Tasks.Task! 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! StackExchange.Redis.IDatabaseAsync.StringSetAsync(System.Collections.Generic.KeyValuePair[]! values, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task! StackExchange.Redis.IDatabaseAsync.StringSetBitAsync(StackExchange.Redis.RedisKey key, long offset, bool bit, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task! diff --git a/src/StackExchange.Redis/RedisDatabase.cs b/src/StackExchange.Redis/RedisDatabase.cs index c3c189909..d12bd4a0f 100644 --- a/src/StackExchange.Redis/RedisDatabase.cs +++ b/src/StackExchange.Redis/RedisDatabase.cs @@ -3018,6 +3018,9 @@ public Task 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); @@ -3033,6 +3036,9 @@ public bool StringSet(KeyValuePair[] values, When when = W return ExecuteSync(msg, ResultProcessor.Boolean); } + // Backwards compatibility overloads: + public Task StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when) => + StringSetAsync(key, value, expiry, false, when); public Task StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) => StringSetAsync(key, value, expiry, false, when, flags); diff --git a/tests/StackExchange.Redis.Tests/DatabaseWrapperTests.cs b/tests/StackExchange.Redis.Tests/DatabaseWrapperTests.cs index 1f47d5d9f..87b12949e 100644 --- a/tests/StackExchange.Redis.Tests/DatabaseWrapperTests.cs +++ b/tests/StackExchange.Redis.Tests/DatabaseWrapperTests.cs @@ -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() { diff --git a/tests/StackExchange.Redis.Tests/Naming.cs b/tests/StackExchange.Redis.Tests/Naming.cs index 55997848c..7540eb8a6 100644 --- a/tests/StackExchange.Redis.Tests/Naming.cs +++ b/tests/StackExchange.Redis.Tests/Naming.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Linq; using System.Reflection; using System.Threading.Tasks; @@ -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() 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(); diff --git a/tests/StackExchange.Redis.Tests/OverloadCompat.cs b/tests/StackExchange.Redis.Tests/OverloadCompat.cs new file mode 100644 index 000000000..ffe8958c2 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/OverloadCompat.cs @@ -0,0 +1,56 @@ +using System; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace StackExchange.Redis.Tests; + +/// +/// This test set is for when we add an overload, to making sure all +/// past versions work correctly and aren't source breaking. +/// +[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); + } +} diff --git a/tests/StackExchange.Redis.Tests/WrapperBaseTests.cs b/tests/StackExchange.Redis.Tests/WrapperBaseTests.cs index 6acbb41f9..122185c37 100644 --- a/tests/StackExchange.Redis.Tests/WrapperBaseTests.cs +++ b/tests/StackExchange.Redis.Tests/WrapperBaseTests.cs @@ -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() {