add redisvalue equivalency tests ... and fix the gaps :)

StackExchange.Redis.Tests/KeysAndValues.cs

@@ -62,19 +62,19 @@ public void TestValues()
             CheckNotSame(bool1, bool2);
         }
 
-        private void CheckSame(RedisValue x, RedisValue y)
+        internal static void CheckSame(RedisValue x, RedisValue y)
         {
-            Assert.True(Equals(x, y));
-            Assert.True(Equals(y, x));
-            Assert.True(EqualityComparer<RedisValue>.Default.Equals(x, y));
-            Assert.True(EqualityComparer<RedisValue>.Default.Equals(y, x));
-            Assert.True(x == y);
-            Assert.True(y == x);
-            Assert.False(x != y);
-            Assert.False(y != x);
-            Assert.True(x.Equals(y));
-            Assert.True(y.Equals(x));
-            Assert.True(x.GetHashCode() == y.GetHashCode());
+            Assert.True(Equals(x, y), "Equals(x, y)");
+            Assert.True(Equals(y, x), "Equals(y, x)");
+            Assert.True(EqualityComparer<RedisValue>.Default.Equals(x, y), "EQ(x,y)");
+            Assert.True(EqualityComparer<RedisValue>.Default.Equals(y, x), "EQ(y,x)");
+            Assert.True(x == y, "x==y");
+            Assert.True(y == x, "y==x");
+            Assert.False(x != y, "x!=y");
+            Assert.False(y != x, "y!=x");
+            Assert.True(x.Equals(y),"x.EQ(y)");
+            Assert.True(y.Equals(x), "y.EQ(x)");
+            Assert.True(x.GetHashCode() == y.GetHashCode(), "GetHashCode");
         }
 
         private void CheckNotSame(RedisValue x, RedisValue y)
@@ -108,7 +108,7 @@ private void CheckNotNull(RedisValue value)
             CheckNotSame(value, (byte[])null);
         }
 
-        private void CheckNull(RedisValue value)
+        internal static void CheckNull(RedisValue value)
         {
             Assert.True(value.IsNull);
             Assert.True(value.IsNullOrEmpty);
@@ -172,42 +172,5 @@ public void CanBeDynamic()
             Assert.Equal((byte)'b', blob[1]);
             Assert.Equal((byte)'c', blob[2]);
         }
-
-        [Fact]
-        public void TryParse()
-        {
-            {
-                RedisValue val = "1";
-                Assert.True(val.TryParse(out int i));
-                Assert.Equal(1, i);
-                Assert.True(val.TryParse(out long l));
-                Assert.Equal(1L, l);
-                Assert.True(val.TryParse(out double d));
-                Assert.Equal(1.0, l);
-            }
-
-            {
-                RedisValue val = "8675309";
-                Assert.True(val.TryParse(out int i));
-                Assert.Equal(8675309, i);
-                Assert.True(val.TryParse(out long l));
-                Assert.Equal(8675309L, l);
-                Assert.True(val.TryParse(out double d));
-                Assert.Equal(8675309.0, l);
-            }
-
-            {
-                RedisValue val = "3.14159";
-                Assert.True(val.TryParse(out double d));
-                Assert.Equal(3.14159, d);
-            }
-
-            {
-                RedisValue val = "not a real number";
-                Assert.False(val.TryParse(out int i));
-                Assert.False(val.TryParse(out long l));
-                Assert.False(val.TryParse(out double d));
-            }
-        }
     }
 }

StackExchange.Redis.Tests/RedisValueEquivalency.cs

+using System.Text;
+using Xunit;
+
+namespace StackExchange.Redis.Tests
+{
+    public class RedisValueEquivalency
+    {
+        // internal storage types: null, integer, double, string, raw
+        // public perceived types: int, long, double, bool, memory / byte[]
+
+        [Fact]
+        public void Int32_Matrix()
+        {
+            void Check(RedisValue known, RedisValue test)
+            {
+                KeysAndValues.CheckSame(known, test);
+                if (known.IsNull)
+                {
+                    Assert.True(test.IsNull);
+                    Assert.False(((int?)test).HasValue);
+                }
+                else
+                {
+                    Assert.False(test.IsNull);
+                    Assert.Equal((int)known, ((int?)test).Value);
+                    Assert.Equal((int)known, (int)test);
+                }
+                Assert.Equal((int)known, (int)test);
+            }
+            Check(42, 42);
+            Check(42, 42.0);
+            Check(42, "42");
+            Check(42, "42.0");
+            Check(42, Bytes("42"));
+            Check(42, Bytes("42.0"));
+            CheckString(42, "42");
+
+            Check(-42, -42);
+            Check(-42, -42.0);
+            Check(-42, "-42");
+            Check(-42, "-42.0");
+            Check(-42, Bytes("-42"));
+            Check(-42, Bytes("-42.0"));
+            CheckString(-42, "-42");
+
+            Check(1, true);
+            Check(0, false);
+        }
+
+        [Fact]
+        public void Int64_Matrix()
+        {
+            void Check(RedisValue known, RedisValue test)
+            {
+                KeysAndValues.CheckSame(known, test);
+                if (known.IsNull)
+                {
+                    Assert.True(test.IsNull);
+                    Assert.False(((long?)test).HasValue);
+                }
+                else
+                {
+                    Assert.False(test.IsNull);
+                    Assert.Equal((long)known, ((long?)test).Value);
+                    Assert.Equal((long)known, (long)test);
+                }
+                Assert.Equal((long)known, (long)test);
+            }
+            Check(1099511627848, 1099511627848);
+            Check(1099511627848, 1099511627848.0);
+            Check(1099511627848, "1099511627848");
+            Check(1099511627848, "1099511627848.0");
+            Check(1099511627848, Bytes("1099511627848"));
+            Check(1099511627848, Bytes("1099511627848.0"));
+            CheckString(1099511627848, "1099511627848");
+
+            Check(-1099511627848, -1099511627848);
+            Check(-1099511627848, -1099511627848);
+            Check(-1099511627848, "-1099511627848");
+            Check(-1099511627848, "-1099511627848.0");
+            Check(-1099511627848, Bytes("-1099511627848"));
+            Check(-1099511627848, Bytes("-1099511627848.0"));
+            CheckString(-1099511627848, "-1099511627848");
+
+            Check(1L, true);
+            Check(0L, false);
+        }
+
+        [Fact]
+        public void Double_Matrix()
+        {
+            void Check(RedisValue known, RedisValue test)
+            {
+                KeysAndValues.CheckSame(known, test);
+                if (known.IsNull)
+                {
+                    Assert.True(test.IsNull);
+                    Assert.False(((double?)test).HasValue);
+                }
+                else
+                {
+                    Assert.False(test.IsNull);
+                    Assert.Equal((double)known, ((double?)test).Value);
+                    Assert.Equal((double)known, (double)test);
+                }
+                Assert.Equal((double)known, (double)test);
+            }
+            Check(1099511627848.0, 1099511627848);
+            Check(1099511627848.0, 1099511627848.0);
+            Check(1099511627848.0, "1099511627848");
+            Check(1099511627848.0, "1099511627848.0");
+            Check(1099511627848.0, Bytes("1099511627848"));
+            Check(1099511627848.0, Bytes("1099511627848.0"));
+            CheckString(1099511627848.0, "1099511627848");
+
+            Check(-1099511627848.0, -1099511627848);
+            Check(-1099511627848.0, -1099511627848);
+            Check(-1099511627848.0, "-1099511627848");
+            Check(-1099511627848.0, "-1099511627848.0");
+            Check(-1099511627848.0, Bytes("-1099511627848"));
+            Check(-1099511627848.0, Bytes("-1099511627848.0"));
+            CheckString(-1099511627848.0, "-1099511627848");
+
+            Check(1.0, true);
+            Check(0.0, false);
+
+            Check(1099511627848.6001, 1099511627848.6001);
+            Check(1099511627848.6001, "1099511627848.6001");
+            Check(1099511627848.6001, Bytes("1099511627848.6001"));
+            CheckString(1099511627848.6001, "1099511627848.6001");
+
+            Check(-1099511627848.6001, -1099511627848.6001);
+            Check(-1099511627848.6001, "-1099511627848.6001");
+            Check(-1099511627848.6001, Bytes("-1099511627848.6001"));
+            CheckString(-1099511627848.6001, "-1099511627848.6001");
+
+            Check(double.NegativeInfinity, double.NegativeInfinity);
+            Check(double.NegativeInfinity, "-inf");
+            CheckString(double.NegativeInfinity, "-inf");
+
+            Check(double.PositiveInfinity, double.PositiveInfinity);
+            Check(double.PositiveInfinity, "+inf");
+            CheckString(double.PositiveInfinity, "+inf");
+        }
+
+        static void CheckString(RedisValue value, string expected)
+        {
+            var s = value.ToString();
+            Assert.True(s == expected, $"'{s}' vs '{expected}'");
+        }
+
+        static byte[] Bytes(string s) => s == null ? null : Encoding.UTF8.GetBytes(s);
+    }
+}

StackExchange.Redis/StackExchange/Redis/Format.cs

@@ -62,7 +62,16 @@ internal static string ToString(double value)
             return value.ToString("G17", NumberFormatInfo.InvariantInfo);
         }
 
-        internal static string ToString(object value) => Convert.ToString(value, CultureInfo.InvariantCulture);
+        internal static string ToString(object value)
+        {
+            if (value == null) return "";
+            if (value is long l) return ToString(l);
+            if (value is int i) return ToString(i);
+            if (value is float f) return ToString(f);
+            if (value is double d) return ToString(d);
+            if (value is EndPoint e) return ToString(e);
+            return Convert.ToString(value, CultureInfo.InvariantCulture);
+        }
 
         internal static string ToString(EndPoint endpoint)
         {

StackExchange.Redis/StackExchange/Redis/RedisValue.cs

@@ -82,6 +82,8 @@ public bool IsNullOrEmpty
         /// <param name="y">The second <see cref="RedisValue"/> to compare.</param>
         public static bool operator ==(RedisValue x, RedisValue y)
         {
+            x = x.Simplify();
+            y = y.Simplify();
             StorageType xType = x.Type, yType = y.Type;
 
             if (xType == StorageType.Null) return yType == StorageType.Null;
@@ -128,21 +130,23 @@ public override bool Equals(object obj)
         /// <summary>
         /// See Object.GetHashCode()
         /// </summary>
-        public override int GetHashCode()
+        public override int GetHashCode() => GetHashCode(this);
+        static int GetHashCode(RedisValue x)
         {
-            switch (Type)
+            x = x.Simplify();
+            switch (x.Type)
             {
                 case StorageType.Null:
                     return -1;
                 case StorageType.Double:
-                    return OverlappedValueDouble.GetHashCode();
+                    return x.OverlappedValueDouble.GetHashCode();
                 case StorageType.Int64:
-                    return _overlappedValue64.GetHashCode();
+                    return x._overlappedValue64.GetHashCode();
                 case StorageType.Raw:
-                    return GetHashCode(_memory);
+                    return ((string)x).GetHashCode(); // to match equality
                 case StorageType.String:
                 default:
-                    return _objectOrSentinel.GetHashCode();
+                    return x._objectOrSentinel.GetHashCode();
             }
         }
 
@@ -464,12 +468,7 @@ internal static RedisValue TryParse(object obj)
         /// </summary>
         /// <param name="value">The <see cref="RedisValue"/> to convert.</param>
         public static explicit operator int(RedisValue value)
-        {
-            checked
-            {
-                return (int)(long)value;
-            }
-        }
+            => checked((int)(long)value);
 
         /// <summary>
         /// Converts a <see cref="RedisValue"/> to a <see cref="long"/>.
@@ -477,25 +476,15 @@ internal static RedisValue TryParse(object obj)
         /// <param name="value">The <see cref="RedisValue"/> to convert.</param>
         public static explicit operator long(RedisValue value)
         {
+            value = value.Simplify();
             switch (value.Type)
             {
                 case StorageType.Null:
                     return 0; // in redis, an arithmetic zero is kinda the same thing as not-exists (think "incr")
                 case StorageType.Int64:
                     return value._overlappedValue64;
-                case StorageType.Double:
-                    var f64 = value.OverlappedValueDouble;
-                    var i64 = (long)f64;
-                    if (f64 == i64) return i64;
-                    break;
-                case StorageType.String:
-                    if (TryParseInt64((string)value._objectOrSentinel, out i64)) return i64;
-                    break;
-                case StorageType.Raw:
-                    if (TryParseInt64(value._memory.Span, out i64)) return i64;
-                    break;
             }
-            throw new InvalidCastException($"Unable to cast from {value.Type} to long");
+            throw new InvalidCastException($"Unable to cast from {value.Type} to long: '{value}'");
         }
 
         /// <summary>
@@ -504,6 +493,7 @@ internal static RedisValue TryParse(object obj)
         /// <param name="value">The <see cref="RedisValue"/> to convert.</param>
         public static explicit operator double(RedisValue value)
         {
+            value = value.Simplify();
             switch (value.Type)
             {
                 case StorageType.Null:
@@ -512,14 +502,8 @@ internal static RedisValue TryParse(object obj)
                     return value._overlappedValue64;
                 case StorageType.Double:
                     return value.OverlappedValueDouble;
-                case StorageType.String:
-                    if (Format.TryParseDouble((string)value._objectOrSentinel, out var f64)) return f64;
-                    break;
-                case StorageType.Raw:
-                    if (TryParseDouble(value._memory.Span, out f64)) return f64;
-                    break;
             }
-            throw new InvalidCastException($"Unable to cast from {value.Type} to double");
+            throw new InvalidCastException($"Unable to cast from {value.Type} to double: '{value}'");
         }
 
         private static bool TryParseDouble(ReadOnlySpan<byte> blob, out double value)
@@ -539,40 +523,28 @@ private static bool TryParseDouble(ReadOnlySpan<byte> blob, out double value)
         /// </summary>
         /// <param name="value">The <see cref="RedisValue"/> to convert.</param>
         public static explicit operator double? (RedisValue value)
-        {
-            if (value.IsNull) return null;
-            return (double)value;
-        }
+            => value.IsNull ? (double?)null : (double)value;
 
         /// <summary>
         /// Converts the <see cref="RedisValue"/> to a <see cref="T:Nullable{long}"/>.
         /// </summary>
         /// <param name="value">The <see cref="RedisValue"/> to convert.</param>
         public static explicit operator long? (RedisValue value)
-        {
-            if (value.IsNull) return null;
-            return (long)value;
-        }
+            => value.IsNull ? (long?)null : (long)value;
 
         /// <summary>
         /// Converts the <see cref="RedisValue"/> to a <see cref="T:Nullable{int}"/>.
         /// </summary>
         /// <param name="value">The <see cref="RedisValue"/> to convert.</param>
         public static explicit operator int? (RedisValue value)
-        {
-            if (value.IsNull) return null;
-            return (int)value;
-        }
+            => value.IsNull ? (int?)null : (int)value;
 
         /// <summary>
         /// Converts the <see cref="RedisValue"/> to a <see cref="T:Nullable{bool}"/>.
         /// </summary>
         /// <param name="value">The <see cref="RedisValue"/> to convert.</param>
         public static explicit operator bool? (RedisValue value)
-        {
-            if (value.IsNull) return null;
-            return (bool)value;
-        }
+            => value.IsNull ? (bool?)null : (bool)value;
         
         /// <summary>
         /// Converts a <see cref="RedisValue"/> to a <see cref="string"/>.
@@ -583,7 +555,7 @@ private static bool TryParseDouble(ReadOnlySpan<byte> blob, out double value)
             switch (value.Type)
             {
                 case StorageType.Null: return null;
-                case StorageType.Double: return Format.ToString(value.OverlappedValueDouble.ToString());
+                case StorageType.Double: return Format.ToString(value.OverlappedValueDouble);
                 case StorageType.Int64: return Format.ToString(value._overlappedValue64);
                 case StorageType.String: return (string)value._objectOrSentinel;
                 case StorageType.Raw:
@@ -712,68 +684,36 @@ object IConvertible.ToType(Type conversionType, IFormatProvider provider)
         ulong IConvertible.ToUInt64(IFormatProvider provider) => (ulong)this;
 
         /// <summary>
-        /// Convert to a long if possible, returning true.
-        ///
-        /// Returns false otherwise.
+        /// Attempt to reduce to canonical terms ahead of time; parses integers, floats, etc
+        /// Note: we don't use this aggressively ahead of time, a: because of extra CPU,
+        /// but more importantly b: because it can change values - for example, if they start
+        /// with "123.000", it should **stay** as "123.000", not become 123L; this could be
+        /// a hash key or similar - we don't want to break it; RedisConnection uses
+        /// the storage type, not the "does it look like a long?" - for this reason
         /// </summary>
-        /// <param name="val">The <see cref="long"/> value, if conversion was possible.</param>
-        public bool TryParse(out long val)
+        private RedisValue Simplify()
         {
-            switch (Type)
+            switch(Type)
             {
-                case StorageType.Null: val = 0; return true; // in redis-land 0 approx. equal null; so roll with it
-                case StorageType.Int64: val = _overlappedValue64; return true;
-                case StorageType.String: return TryParseInt64((string)_objectOrSentinel, out val);
-                case StorageType.Raw: return TryParseInt64(_memory.Span, out val);
+                case StorageType.String:
+                    string s = (string)_objectOrSentinel;
+                    if (TryParseInt64(s, out var i64)) return i64;
+                    if (Format.TryParseDouble(s, out var f64)) return f64;
+                    break;
+                case StorageType.Raw:
+                    var b = _memory.Span;
+                    if (TryParseInt64(b, out i64)) return i64;
+                    if (TryParseDouble(b, out f64)) return f64;
+                    break;
                 case StorageType.Double:
-                    var f64 = OverlappedValueDouble;
-                    if (f64 >= long.MinValue && f64 <= long.MaxValue)
-                    {
-                        val = (long)f64;
-                        return true;
-                    }
+                    // is the double actually an integer?
+                    f64 = OverlappedValueDouble;                    
+                    if (f64 >= long.MinValue && f64 <= long.MaxValue
+                        && (i64 = (long)f64) == f64) return i64;
                     break;
-            }
-            val = default;
-            return false;
-        }
-
-        /// <summary>
-        /// Convert to a int if possible, returning true.
-        ///
-        /// Returns false otherwise.
-        /// </summary>
-        /// <param name="val">The <see cref="int"/> value, if conversion was possible.</param>
-        public bool TryParse(out int val)
-        {
-            if (TryParse(out long l) && l >= int.MinValue && l <= int.MaxValue)
-            {
-                val = (int)l;
-                return true;
-            }
-            val = default;
-            return false;
-
-        }
 
-        /// <summary>
-        /// Convert to a double if possible, returning true.
-        ///
-        /// Returns false otherwise.
-        /// </summary>
-        /// <param name="val">The <see cref="double"/> value, if conversion was possible.</param>
-        public bool TryParse(out double val)
-        {
-            switch (Type)
-            {
-                case StorageType.Null: val = 0; return true;
-                case StorageType.Int64: val = _overlappedValue64; return true;
-                case StorageType.Double: val = OverlappedValueDouble; return true;
-                case StorageType.String: return Format.TryParseDouble((string)_objectOrSentinel, out val);
-                case StorageType.Raw: return TryParseDouble(_memory.Span, out val);
             }
-            val = default;
-            return false;
+            return this;
         }
     }
 }