Merge pull request #30 from ZEXSM/bugfix/support-bracketing

bugfix/support-bracketing
ZEXSM · Aug 29, 2020 · 85044f1 · 85044f1
2 parents 1eabf4c + 43ce036
commit 85044f1
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 18 deletions.
diff --git a/README.md b/README.md
@@ -159,6 +159,20 @@ var uri = new ODataQueryBuilder<ODataInfoContainer>("http://mock/odata")
 .Filter(s => s.ODataKind.Color == ColorEnum.Blue)
 ```
 > $filter=ODataKind/Color eq 2
+
+:information_source: *Use parenthesis in filter*
+```csharp
+var constStrIds = new[] { "123", "512" };
+var constValue = 3;
+...
+.Filter((s, f, o) => s.IdRule == constValue
+    && s.IsActive
+    && (f.Date(s.EndDate.Value) == default(DateTimeOffset?) || s.EndDate > DateTime.Today)
+    && (f.Date((DateTimeOffset)s.BeginDate) != default(DateTime?) || f.Date((DateTime)s.BeginDate) <= DateTime.Now)
+    && o.In(s.ODataKind.ODataCode.Code, constStrIds), useParenthesis: true)
+```
+
+> $filter=(((IdRule eq 3 and IsActive) and (date(EndDate) eq null or EndDate gt 2020-08-29T00:00:00Z)) and (date(BeginDate) ne null or date(BeginDate) le 2020-08-29T18:09:15Z)) and ODataKind/ODataCode/Code in ('123','512')
 #### <a name="orderby"/> orderby
 ```csharp
 .OrderBy(s => s.IdType)

diff --git a/src/OData.QueryBuilder/Conventions/Options/IODataOptionList.cs b/src/OData.QueryBuilder/Conventions/Options/IODataOptionList.cs
@@ -8,11 +8,11 @@ namespace OData.QueryBuilder.Conventions.Options
 {
     public interface IODataOptionList<TEntity> : IODataQuery
     {
-        IODataOptionList<TEntity> Filter(Expression<Func<TEntity, bool>> entityFilter);
+        IODataOptionList<TEntity> Filter(Expression<Func<TEntity, bool>> entityFilter, bool useParenthesis = false);
 
-        IODataOptionList<TEntity> Filter(Expression<Func<TEntity, IODataFunction, bool>> entityFilter);
+        IODataOptionList<TEntity> Filter(Expression<Func<TEntity, IODataFunction, bool>> entityFilter, bool useParenthesis = false);
 
-        IODataOptionList<TEntity> Filter(Expression<Func<TEntity, IODataFunction, IODataOperator, bool>> entityFilter);
+        IODataOptionList<TEntity> Filter(Expression<Func<TEntity, IODataFunction, IODataOperator, bool>> entityFilter, bool useParenthesis = false);
 
         IODataOptionList<TEntity> Expand(Expression<Func<TEntity, object>> entityExpand);
 

diff --git a/src/OData.QueryBuilder/Conventions/Options/Nested/IODataOptionNested.cs b/src/OData.QueryBuilder/Conventions/Options/Nested/IODataOptionNested.cs
@@ -10,7 +10,7 @@ public interface IODataOptionNested<TEntity>
 
         IODataOptionNested<TEntity> Expand(Expression<Func<TEntity, object>> entityNestedExpand);
 
-        IODataOptionNested<TEntity> Filter(Expression<Func<TEntity, bool>> entityNestedFilter);
+        IODataOptionNested<TEntity> Filter(Expression<Func<TEntity, bool>> entityNestedFilter, bool useParenthesis = false);
 
         IODataOptionNested<TEntity> Select(Expression<Func<TEntity, object>> entityNestedSelect);
 

diff --git a/src/OData.QueryBuilder/Conventions/Options/Nested/ODataOptionNested.cs b/src/OData.QueryBuilder/Conventions/Options/Nested/ODataOptionNested.cs
@@ -38,9 +38,9 @@ public IODataOptionNested<TEntity> Expand(Action<IODataQueryExpandNestedResource
             return this;
         }
 
-        public IODataOptionNested<TEntity> Filter(Expression<Func<TEntity, bool>> entityNestedFilter)
+        public IODataOptionNested<TEntity> Filter(Expression<Func<TEntity, bool>> entityNestedFilter, bool useParenthesis = false)
         {
-            var query = _visitorExpression.ToString(entityNestedFilter.Body);
+            var query = _visitorExpression.ToString(entityNestedFilter.Body, useParenthesis);
 
             _stringBuilder.Append($"{ODataOptionNames.Filter}{QuerySeparators.EqualSignString}{query}{QuerySeparators.NestedString}");
 

diff --git a/src/OData.QueryBuilder/Conventions/Options/ODataOptionList.cs b/src/OData.QueryBuilder/Conventions/Options/ODataOptionList.cs
@@ -18,27 +18,27 @@ public ODataOptionList(StringBuilder stringBuilder, ODataQueryBuilderOptions oda
             : base(stringBuilder, odataQueryBuilderOptions) =>
             _visitorExpression = new VisitorExpression(odataQueryBuilderOptions);
 
-        public IODataOptionList<TEntity> Filter(Expression<Func<TEntity, bool>> entityFilter)
+        public IODataOptionList<TEntity> Filter(Expression<Func<TEntity, bool>> entityFilter, bool useParenthesis = false)
         {
-            var query = _visitorExpression.ToString(entityFilter.Body);
+            var query = _visitorExpression.ToString(entityFilter.Body, useParenthesis);
 
             _stringBuilder.Append($"{ODataOptionNames.Filter}{QuerySeparators.EqualSignString}{query}{QuerySeparators.MainString}");
 
             return this;
         }
 
-        public IODataOptionList<TEntity> Filter(Expression<Func<TEntity, IODataFunction, bool>> entityFilter)
+        public IODataOptionList<TEntity> Filter(Expression<Func<TEntity, IODataFunction, bool>> entityFilter, bool useParenthesis = false)
         {
-            var query = _visitorExpression.ToString(entityFilter.Body);
+            var query = _visitorExpression.ToString(entityFilter.Body, useParenthesis);
 
             _stringBuilder.Append($"{ODataOptionNames.Filter}{QuerySeparators.EqualSignString}{query}{QuerySeparators.MainString}");
 
             return this;
         }
 
-        public IODataOptionList<TEntity> Filter(Expression<Func<TEntity, IODataFunction, IODataOperator, bool>> entityFilter)
+        public IODataOptionList<TEntity> Filter(Expression<Func<TEntity, IODataFunction, IODataOperator, bool>> entityFilter, bool useParenthesis = false)
         {
-            var query = _visitorExpression.ToString(entityFilter.Body);
+            var query = _visitorExpression.ToString(entityFilter.Body, useParenthesis);
 
             _stringBuilder.Append($"{ODataOptionNames.Filter}{QuerySeparators.EqualSignString}{query}{QuerySeparators.MainString}");
 

diff --git a/src/OData.QueryBuilder/Options/ODataQueryBuilderOptions.cs b/src/OData.QueryBuilder/Options/ODataQueryBuilderOptions.cs
@@ -3,6 +3,7 @@
     public class ODataQueryBuilderOptions
     {
         public bool SuppressExceptionOfNullOrEmptyFunctionArgs { get; set; } = false;
+
         public bool SuppressExceptionOfNullOrEmptyOperatorArgs { get; set; } = false;
     }
 }
diff --git a/src/OData.QueryBuilder/Visitors/VisitorExpression.cs b/src/OData.QueryBuilder/Visitors/VisitorExpression.cs
@@ -13,6 +13,9 @@ internal class VisitorExpression
     {
         protected readonly ODataQueryBuilderOptions _odataQueryBuilderOptions;
 
+        private bool _useParenthesis;
+        private ExpressionType? _expressionType;
+
         public VisitorExpression(ODataQueryBuilderOptions odataQueryBuilderOptions) =>
             _odataQueryBuilderOptions = odataQueryBuilderOptions;
 
@@ -31,6 +34,8 @@ public VisitorExpression(ODataQueryBuilderOptions odataQueryBuilderOptions) =>
 
         protected virtual string VisitBinaryExpression(BinaryExpression binaryExpression)
         {
+            var hasParenthesis = _useParenthesis && HasParenthesis(binaryExpression.NodeType);
+
             var left = VisitExpression(binaryExpression.Left);
             var right = VisitExpression(binaryExpression.Right);
 
@@ -44,7 +49,10 @@ protected virtual string VisitBinaryExpression(BinaryExpression binaryExpression
                 return left;
             }
 
-            return $"{left} {binaryExpression.NodeType.ToODataQueryOperator()} {right}";
+            return hasParenthesis ?
+                $"({left} {binaryExpression.NodeType.ToODataQueryOperator()} {right})"
+                :
+                $"{left} {binaryExpression.NodeType.ToODataQueryOperator()} {right}";
         }
 
         protected virtual string VisitMemberExpression(MemberExpression memberExpression) =>
@@ -292,6 +300,27 @@ protected string CreateResourcePath(MemberExpression memberExpression)
                 name : $"{name}/{memberExpression.Member.Name}";
         }
 
-        public string ToString(Expression expression) => VisitExpression(expression);
+        private bool HasParenthesis(ExpressionType expressionType)
+        {
+            var hasParenthesis = _expressionType.HasValue && expressionType switch
+            {
+                ExpressionType.And => true,
+                ExpressionType.AndAlso => true,
+                ExpressionType.Or => true,
+                ExpressionType.OrElse => true,
+                _ => false,
+            };
+
+            _expressionType = expressionType;
+
+            return hasParenthesis;
+        }
+
+        public string ToString(Expression expression, bool useParenthesis = false)
+        {
+            _useParenthesis = useParenthesis;
+
+            return VisitExpression(expression);
+        }
     }
 }
diff --git a/test/OData.QueryBuilder.Test/ODataQueryOptionListTest.cs b/test/OData.QueryBuilder.Test/ODataQueryOptionListTest.cs
@@ -842,8 +842,8 @@ public void ODataQueryBuilderList_Filter_Boolean_Values_Success()
             uri.OriginalString.Should().Be("http://mock/odata/ODataType?$filter=IsActive and IsOpen eq false and IsOpen eq true and ODataKind/ODataCode/IdActive eq false");
         }
 
-        [Fact(DisplayName = "Filter brackets => Success")]
-        public void ODataQueryBuilderList_Filter_Brackets_Success()
+        [Fact(DisplayName = "Filter support parentheses => Success")]
+        public void ODataQueryBuilderList_Filter_support_parentheses_Success()
         {
             var constStrIds = new[] { "123", "512" };
             var constValue = 3;
@@ -855,10 +855,14 @@ public void ODataQueryBuilderList_Filter_Brackets_Success()
                     && s.IsActive
                     && (f.Date(s.EndDate.Value) == default(DateTimeOffset?) || s.EndDate > DateTime.Today)
                     && (f.Date((DateTimeOffset)s.BeginDate) != default(DateTime?) || f.Date((DateTime)s.BeginDate) <= DateTime.Now)
-                    && o.In(s.ODataKind.ODataCode.Code, constStrIds))
+                    && o.In(s.ODataKind.ODataCode.Code, constStrIds), useParenthesis: true)
                 .ToUri();
 
-            uri.OriginalString.Should().Be($"http://mock/odata/ODataType?$filter=IdRule eq 3 and IsActive and date(EndDate) eq null or EndDate gt {DateTime.Today:s}Z and date(BeginDate) ne null or date(BeginDate) le {DateTime.Now:s}Z and ODataKind/ODataCode/Code in ('123','512')");
+            uri.OriginalString.Should().Be($"http://mock/odata/ODataType?$filter=(((IdRule eq 3" +
+                $" and IsActive)" +
+                $" and (date(EndDate) eq null or EndDate gt {DateTime.Today:s}Z))" +
+                $" and (date(BeginDate) ne null or date(BeginDate) le {DateTime.Now:s}Z))" +
+                $" and ODataKind/ODataCode/Code in ('123','512')");
         }
 
         [Theory(DisplayName = "Count value => Success")]