adding proper translation for DateTimeOffsets (#406)

redis · Aug 16, 2023 · 8f0e204 · 8f0e204
1 parent b0f2b9e
commit 8f0e204
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 9 deletions.
diff --git a/src/Redis.OM/Aggregation/AggregationPredicates/QueryPredicate.cs b/src/Redis.OM/Aggregation/AggregationPredicates/QueryPredicate.cs
@@ -40,9 +40,21 @@ public IEnumerable<string> Serialize()
         /// <inheritdoc/>
         protected override void ValidateAndPushOperand(Expression expression, Stack<string> stack)
         {
-            if (expression is BinaryExpression binaryExpression
-                && binaryExpression.Left is MemberExpression memberExpression)
+            if (expression is BinaryExpression binaryExpression)
             {
+                var memberExpression = binaryExpression.Left as MemberExpression;
+                if (memberExpression is null)
+                {
+                    if (binaryExpression.Left is UnaryExpression { NodeType: ExpressionType.Convert, Operand: MemberExpression } leftUnary)
+                    {
+                        memberExpression = (MemberExpression)leftUnary.Operand;
+                    }
+                    else
+                    {
+                        throw new InvalidOperationException("Invalid Expression Type");
+                    }
+                }
+
                 if (binaryExpression.Right is ConstantExpression constantExpression)
                 {
                     stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, constantExpression));
@@ -54,17 +66,17 @@ protected override void ValidateAndPushOperand(Expression expression, Stack<stri
                         case ConstantExpression c:
                             stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, c));
                             break;
-                        case MemberExpression mem when mem.Expression is ConstantExpression frame:
+                        case MemberExpression mem:
                         {
-                            var val = ExpressionParserUtilities.GetValue(mem.Member, frame.Value);
+                            var val = ExpressionParserUtilities.GetOperandString(mem);
                             stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, System.Linq.Expressions.Expression.Constant(val)));
                             break;
                         }
                     }
                 }
-                else if (binaryExpression.Right is MemberExpression mem && mem.Expression is ConstantExpression frame)
+                else if (binaryExpression.Right is MemberExpression mem)
                 {
-                    var val = ExpressionParserUtilities.GetValue(mem.Member, frame.Value);
+                    var val = ExpressionParserUtilities.GetOperandString(mem);
                     stack.Push(BuildQueryPredicate(binaryExpression.NodeType, memberExpression, System.Linq.Expressions.Expression.Constant(val)));
                 }
                 else

diff --git a/src/Redis.OM/Common/ExpressionParserUtilities.cs b/src/Redis.OM/Common/ExpressionParserUtilities.cs
@@ -903,6 +903,11 @@ private static string ValueToString(object value)
                 return ((double)value).ToString(CultureInfo.InvariantCulture);
             }
 
+            if (value is DateTimeOffset dto)
+            {
+                return dto.ToUnixTimeMilliseconds().ToString(CultureInfo.InvariantCulture);
+            }
+
             if (value is DateTime dt)
             {
                 return new DateTimeOffset(dt).ToUnixTimeMilliseconds().ToString(CultureInfo.InvariantCulture);

diff --git a/test/Redis.OM.Unit.Tests/ConnectionTests.cs b/test/Redis.OM.Unit.Tests/ConnectionTests.cs
@@ -23,7 +23,7 @@ public void TestConnectStandalone()
             Assert.Equal("Bar",res);
         }
 
-        [Fact]
+        [SkipIfMissingEnvVar("SENTINLE_HOST_PORT")]
         public void TestSentinel()
         {
             var hostInfo = System.Environment.GetEnvironmentVariable("SENTINLE_HOST_PORT") ?? "localhost:26379";
@@ -63,7 +63,7 @@ public void GivenMultiplexerConnection_WhenTestingSetCommand_ThenShouldExecuteSe
             Assert.Equal("Bar", res);
         }
 
-        [Fact]
+        [SkipIfMissingEnvVar("PRIVATE_HOST", "PRIVATE_PORT", "PRIVATE_PASSWORD")]
         public void TestPrivateConnection()
         {
             var host = Environment.GetEnvironmentVariable("PRIVATE_HOST") ?? "redis-private";

diff --git a/test/Redis.OM.Unit.Tests/CoreTests.cs b/test/Redis.OM.Unit.Tests/CoreTests.cs
@@ -454,7 +454,7 @@ public void CommandTest()
             stream.Flush();
         }
 
-        [Fact]
+        [SkipIfMissingEnvVar("AGGRESSIVELY_SHORT_TIMEOUT_REDIS")]
         public async Task SearchTimeoutTest()
         {
             var hostInfo = Environment.GetEnvironmentVariable("AGGRESSIVELY_SHORT_TIMEOUT_REDIS") ?? string.Empty;

diff --git a/test/Redis.OM.Unit.Tests/RediSearchTests/AggregationSetTests.cs b/test/Redis.OM.Unit.Tests/RediSearchTests/AggregationSetTests.cs
@@ -558,6 +558,34 @@ public void CustomPropertyNamesInQuery()
                 "COUNT",
                 "10");
         }
+
+        [Fact]
+        public void DateTimeQuery()
+        {
+            var dt = DateTime.Now;
+            var dtMs = new DateTimeOffset(dt).ToUnixTimeMilliseconds();
+
+            var dto = DateTimeOffset.Now.Subtract(TimeSpan.FromHours(3));
+            var dtoMs = dto.ToUnixTimeMilliseconds();
+            var collection = new RedisAggregationSet<ObjectWithDateTime>(_substitute, true, chunkSize: 10000);
+            _substitute.Execute("FT.AGGREGATE", Arg.Any<string[]>()).Returns(MockedResult);
+            _substitute.Execute("FT.CURSOR", Arg.Any<string[]>()).Returns(MockedResultCursorEnd);
+
+            Expression<Func<AggregationResult<ObjectWithDateTime>, bool>> query = a => a.RecordShell.Timestamp > dt;
+            _ = collection.Where(query).ToList();
+            _substitute.Received().Execute("FT.AGGREGATE", "objectwithdatetime-idx", $"@Timestamp:[({dtMs} inf]", "WITHCURSOR", "COUNT", "10000");
+
+            query = a => a.RecordShell.Timestamp > dto;
+            _ = collection.Where(query).ToList();
+            _substitute.Received().Execute("FT.AGGREGATE", "objectwithdatetime-idx", $"@Timestamp:[({dtoMs} inf]", "WITHCURSOR", "COUNT", "10000");
 
+            query = a => a.RecordShell.TimestampOffset > dto;
+            _ = collection.Where(query).ToList();
+            _substitute.Received().Execute("FT.AGGREGATE", "objectwithdatetime-idx", $"@TimestampOffset:[({dtoMs} inf]", "WITHCURSOR", "COUNT", "10000");
+
+            query = a => a.RecordShell.TimestampOffset > dt;
+            _ = collection.Where(query).ToList();
+            _substitute.Received().Execute("FT.AGGREGATE", "objectwithdatetime-idx", $"@TimestampOffset:[({dtMs} inf]", "WITHCURSOR", "COUNT", "10000");
+        }
     }
 }
diff --git a/test/Redis.OM.Unit.Tests/RediSearchTests/ObjectWithDateTime.cs b/test/Redis.OM.Unit.Tests/RediSearchTests/ObjectWithDateTime.cs
@@ -10,6 +10,8 @@ public class ObjectWithDateTime
     public string Id { get; set; }
     [Indexed(Sortable = true)]
     public DateTime Timestamp { get; set; }
+    [Indexed(Sortable = true)]
+    public DateTimeOffset TimestampOffset { get; set; }
     [Indexed]
     public DateTime? NullableTimestamp { get; set; }
 }

diff --git a/test/Redis.OM.Unit.Tests/RediSearchTests/SearchTests.cs b/test/Redis.OM.Unit.Tests/RediSearchTests/SearchTests.cs
@@ -2387,6 +2387,84 @@ public void Issue201()
             );
         }
 
+        [Fact]
+        public void RangeOnDateTimeWithMultiplePredicates()
+        {
+            _substitute.ClearSubstitute();
+            _substitute.Execute(Arg.Any<string>(), Arg.Any<string[]>()).Returns(_mockReply);
+
+            var fromDto = DateTimeOffset.UtcNow.Subtract(TimeSpan.FromHours(4));
+            var toDto = DateTimeOffset.UtcNow;
+
+            var fromDt = fromDto.DateTime;
+            var toDt = toDto.DateTime;
+
+            var msFrom = fromDto.ToUnixTimeMilliseconds();
+            var msTo = toDto.ToUnixTimeMilliseconds();
+
+            var collection = new RedisCollection<ObjectWithDateTime>(_substitute, 1000);
+            _ = collection.Where(x => x.TimestampOffset <= fromDto && x.TimestampOffset >= toDto).ToList();
+            _ = collection.Where(x => x.TimestampOffset <= fromDt && x.TimestampOffset >= toDt).ToList();
+            _ = collection.Where(x => x.Timestamp <= fromDto && x.Timestamp >= toDto).ToList();
+            _ = collection.Where(x => x.Timestamp <= fromDt && x.Timestamp >= toDt).ToList();
+            _ = collection.Where(x => x.NullableTimestamp <= fromDto && x.NullableTimestamp >= toDto).ToList();
+            _ = collection.Where(x => x.NullableTimestamp <= fromDt && x.NullableTimestamp >= toDt).ToList();
+
+            _substitute.Received().Execute(
+                "FT.SEARCH",
+                "objectwithdatetime-idx",
+                $"((@TimestampOffset:[-inf {msFrom}]) (@TimestampOffset:[{msTo} inf]))",
+                "LIMIT",
+                "0",
+                "1000"
+            );
+
+            _substitute.Received().Execute(
+                "FT.SEARCH",
+                "objectwithdatetime-idx",
+                $"((@TimestampOffset:[-inf {new DateTimeOffset(fromDt).ToUnixTimeMilliseconds()}]) (@TimestampOffset:[{new DateTimeOffset(toDt).ToUnixTimeMilliseconds()} inf]))",
+                "LIMIT",
+                "0",
+                "1000"
+            );
+
+            _substitute.Received().Execute(
+                "FT.SEARCH",
+                "objectwithdatetime-idx",
+                $"((@Timestamp:[-inf {msFrom}]) (@Timestamp:[{msTo} inf]))",
+                "LIMIT",
+                "0",
+                "1000"
+            );
+
+            _substitute.Received().Execute(
+                "FT.SEARCH",
+                "objectwithdatetime-idx",
+                $"((@Timestamp:[-inf {new DateTimeOffset(fromDt).ToUnixTimeMilliseconds()}]) (@Timestamp:[{new DateTimeOffset(toDt).ToUnixTimeMilliseconds()} inf]))",
+                "LIMIT",
+                "0",
+                "1000"
+            );
+
+            _substitute.Received().Execute(
+                "FT.SEARCH",
+                "objectwithdatetime-idx",
+                $"((@NullableTimestamp:[-inf {msFrom}]) (@NullableTimestamp:[{msTo} inf]))",
+                "LIMIT",
+                "0",
+                "1000"
+            );
+
+            _substitute.Received().Execute(
+                "FT.SEARCH",
+                "objectwithdatetime-idx",
+                $"((@NullableTimestamp:[-inf {new DateTimeOffset(fromDt).ToUnixTimeMilliseconds()}]) (@NullableTimestamp:[{new DateTimeOffset(toDt).ToUnixTimeMilliseconds()} inf]))",
+                "LIMIT",
+                "0",
+                "1000"
+            );
+        }
+
         [Fact]
         public void RangeOnDatetime()
         {

diff --git a/test/Redis.OM.Unit.Tests/SkipIfMissingEnvVarAttribute.cs b/test/Redis.OM.Unit.Tests/SkipIfMissingEnvVarAttribute.cs
@@ -0,0 +1,30 @@
+using System;
+using System.Linq;
+using Xunit;
+
+namespace Redis.OM.Unit.Tests;
+
+[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
+public class SkipIfMissingEnvVarAttribute : FactAttribute
+{
+    private readonly string[] _envVars;
+
+    public SkipIfMissingEnvVarAttribute(params string[] envVars)
+    {
+        _envVars = envVars;
+    }
+
+    public override string Skip
+    {
+        get
+        {
+            var missingEnvVars = _envVars.Where(x => Environment.GetEnvironmentVariable(x) == null).ToArray();
+            if (missingEnvVars.Any())
+            {
+                return $"Skipping because the following environment variables were missing: {string.Join(",", missingEnvVars)}";
+            }
+
+            return null;
+        }
+    }
+}