Add basic tests for keyspace isolation; use polymorphism for Condition...

Add basic tests for keyspace isolation; use polymorphism for Condition re-writing; slight tweaks to wrapper construction (no double-wrapping, for example)

StackExchange.Redis.Tests/StackExchange.Redis.Tests.csproj

@@ -107,6 +107,7 @@
     <Compile Include="TestInfoReplicationChecks.cs" />
     <Compile Include="Transactions.cs" />
     <Compile Include="VPNTest.cs" />
+    <Compile Include="WithKeyPrefixTests.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />

StackExchange.Redis.Tests/WithKeyPrefixTests.cs

+using NUnit.Framework;
+using StackExchange.Redis.StackExchange.Redis.KeyspaceIsolation;
+using System;
+
+namespace StackExchange.Redis.Tests
+{
+
+    [TestFixture]
+    public class WithKeyPrefixTests : TestBase
+    {
+        [Test]
+        public void BasicSmokeTest()
+        {
+            using(var conn = Create())
+            {
+                var raw = conn.GetDatabase(1);
+
+                var vanilla = raw.WithKeyPrefix("");
+                Assert.AreSame(raw, vanilla);
+                vanilla = raw.WithKeyPrefix((byte[])null);
+                Assert.AreSame(raw, vanilla);
+                vanilla = raw.WithKeyPrefix((string)null);
+                Assert.AreSame(raw, vanilla);
+
+                var foo = raw.WithKeyPrefix("foo");
+                var foobar = foo.WithKeyPrefix("bar");
+
+                string key = Me();
+
+                string s = Guid.NewGuid().ToString(), t = Guid.NewGuid().ToString();
+
+                foo.StringSet(key, s);
+                var val = (string)foo.StringGet(key);
+                Assert.AreEqual(s, val); // fooBasicSmokeTest
+
+                foobar.StringSet(key, t);
+                val = (string)foobar.StringGet(key);
+                Assert.AreEqual(t, val); // foobarBasicSmokeTest
+
+                val = (string)foo.StringGet("bar" + key);
+                Assert.AreEqual(t, val); // foobarBasicSmokeTest
+
+                val = (string)raw.StringGet("foo" + key);
+                Assert.AreEqual(s, val); // fooBasicSmokeTest
+
+                val = (string)raw.StringGet("foobar" + key);
+                Assert.AreEqual(t, val); // foobarBasicSmokeTest
+            }
+        }
+        [Test]
+        public void ConditionTest()
+        {
+            using(var conn = Create())
+            {
+                var raw = conn.GetDatabase(2);
+
+                var foo = raw.WithKeyPrefix("tran:");
+
+                raw.KeyDelete("tran:abc");
+                raw.KeyDelete("tran:i");
+
+                // execute while key exists
+                raw.StringSet("tran:abc", "def");
+                var tran = foo.CreateTransaction();
+                tran.AddCondition(Condition.KeyExists("abc"));
+                tran.StringIncrementAsync("i");
+                tran.Execute();
+
+                int i = (int)raw.StringGet("tran:i");
+                Assert.AreEqual(1, i);
+
+                // repeat without key
+                raw.KeyDelete("tran:abc");
+                tran = foo.CreateTransaction();
+                tran.AddCondition(Condition.KeyExists("abc"));
+                tran.StringIncrementAsync("i");
+                tran.Execute();
+
+                i = (int)raw.StringGet("tran:i");
+                Assert.AreEqual(1, i);
+            }
+        }
+    }
+}

StackExchange.Redis/StackExchange/Redis/Condition.cs

@@ -9,6 +9,7 @@ namespace StackExchange.Redis
     /// </summary>
     public abstract class Condition
     {
+        internal abstract Condition MapKeys(Func<RedisKey,RedisKey> map);
 
         private Condition() { }
 
@@ -144,12 +145,14 @@ internal override void WriteImpl(PhysicalConnection physical)
 
         internal class ExistsCondition : Condition
         {
-            internal readonly bool expectedResult;
-
-            internal readonly RedisValue hashField;
-
-            internal readonly RedisKey key;
+            private readonly bool expectedResult;
+            private readonly RedisValue hashField;
+            private readonly RedisKey key;
 
+            internal override Condition MapKeys(Func<RedisKey,RedisKey> map)
+            {
+                return new ExistsCondition(map(key), hashField, expectedResult);
+            }
             public ExistsCondition(RedisKey key, RedisValue hashField, bool expectedResult)
             {
                 if (key.IsNull) throw new ArgumentException("key");
@@ -199,9 +202,14 @@ internal override bool TryValidate(RawResult result, out bool value)
 
         internal class EqualsCondition : Condition
         {
-            internal readonly bool expectedEqual;
-            internal readonly RedisValue hashField, expectedValue;
-            internal readonly RedisKey key;
+
+            internal override Condition MapKeys(Func<RedisKey,RedisKey> map)
+            {
+                return new EqualsCondition(map(key), hashField, expectedEqual, expectedValue);
+            }
+            private readonly bool expectedEqual;
+            private readonly RedisValue hashField, expectedValue;
+            private readonly RedisKey key;
             public EqualsCondition(RedisKey key, RedisValue hashField, bool expectedEqual, RedisValue expectedValue)
             {
                 if (key.IsNull) throw new ArgumentException("key");

StackExchange.Redis/StackExchange/Redis/KeyspaceIsolation/DatabaseExtension.cs

@@ -5,7 +5,7 @@ namespace StackExchange.Redis.StackExchange.Redis.KeyspaceIsolation
     /// <summary>
     ///     Provides the <see cref="WithKeyPrefix"/> extension method to <see cref="IDatabase"/>.
     /// </summary>
-    public static class DatabaseExtension
+    public static class DatabaseExtensions
     {
         /// <summary>
         ///     Creates a new <see cref="IDatabase"/> instance that provides an isolated key space
@@ -46,7 +46,15 @@ public static IDatabase WithKeyPrefix(this IDatabase database, RedisKey keyPrefi
 
             if (keyPrefix.IsNull || keyPrefix.Value.Length == 0)
             {
-                throw new ArgumentException("The specified prefix cannot be null or empty", "keyPrefix");
+                return database; // fine - you can keep using the original, then
+            }
+
+            if(database is DatabaseWrapper)
+            {
+                // combine the key in advance to minimize indirection
+                var wrapper = (DatabaseWrapper)database;
+                keyPrefix = wrapper.ToInner(keyPrefix);
+                database = wrapper.Inner;
             }
 
             return new DatabaseWrapper(database, keyPrefix);

StackExchange.Redis/StackExchange/Redis/KeyspaceIsolation/TransactionWrapper.cs

@@ -12,7 +12,7 @@ public TransactionWrapper(ITransaction inner, RedisKey prefix)
 
         public ConditionResult AddCondition(Condition condition)
         {
-            return this.Inner.AddCondition(this.ToInner(condition));
+            return this.Inner.AddCondition(condition == null ? null : condition.MapKeys(GetMapFunction()));
         }
 
         public bool Execute(CommandFlags flags = CommandFlags.None)

StackExchange.Redis/StackExchange/Redis/KeyspaceIsolation/WrapperBase.cs

@@ -644,7 +644,7 @@ public Task<string> ClientGetNameAsync(CommandFlags flags = CommandFlags.None)
             return this.Inner.ClientGetNameAsync(flags);
         }
 
-        protected RedisKey ToInner(RedisKey outer)
+        protected internal RedisKey ToInner(RedisKey outer)
         {
             return this.Prefix + outer;
         }
@@ -757,41 +757,11 @@ protected RedisChannel ToInner(RedisChannel outer)
             return this.Prefix + outer;
         }
 
-        protected Condition ToInner(Condition outer)
+        private Func<RedisKey, RedisKey> mapFunction;
+        protected Func<RedisKey, RedisKey> GetMapFunction()
         {
-            Condition.ExistsCondition asExists = outer as Condition.ExistsCondition;
-
-            if (asExists != null)
-            {
-                return this.ToInner(asExists);
-            }
-
-            Condition.EqualsCondition asEquals = outer as Condition.EqualsCondition;
-
-            if (asEquals != null)
-            {
-                return this.ToInner(asEquals);
-            }
-            
-            throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture,
-                "Unsupported condition: {0}", outer));
-        }
-
-        private Condition.ExistsCondition ToInner(Condition.ExistsCondition outer)
-        {
-            return new Condition.ExistsCondition(
-                this.ToInner(outer.key),
-                outer.hashField,
-                outer.expectedResult);
-        }
-
-        private Condition.EqualsCondition ToInner(Condition.EqualsCondition outer)
-        {
-            return new Condition.EqualsCondition(
-                this.ToInner(outer.key),
-                outer.hashField,
-                outer.expectedEqual,
-                outer.expectedValue);
+            // create as a delegate when first required, then re-use
+            return mapFunction ?? (mapFunction = new Func<RedisKey, RedisKey>(this.ToInner)); 
         }
     }
 }