Merge pull request #95 from kevindost/fix/accessing-null-members

Fix issue where sorting or filtering a collection fails on accesssing null members.

Sieve/Extensions/OrderByDynamic.cs

@@ -1,36 +1,62 @@
 using System;
 using System.Linq;
 using System.Linq.Expressions;
-using System.Reflection;
 
 namespace Sieve.Extensions
 {
-    public static partial class LinqExtentions
+    public static partial class LinqExtensions
     {
-        public static IQueryable<TEntity> OrderByDynamic<TEntity>(this IQueryable<TEntity> source, string fullPropertyName, PropertyInfo propertyInfo,
+        public static IQueryable<TEntity> OrderByDynamic<TEntity>(
-                          bool desc, bool useThenBy)
+            this IQueryable<TEntity> source,
+            string fullPropertyName,
+            bool desc,
+            bool useThenBy)
         {
-            string command = desc ?
+            var lambda = GenerateLambdaWithSafeMemberAccess<TEntity>(fullPropertyName);
-                (useThenBy ? "ThenByDescending" : "OrderByDescending") :
-                (useThenBy ? "ThenBy" : "OrderBy");
-            var type = typeof(TEntity);
-            var parameter = Expression.Parameter(type, "p");
 
-            dynamic propertyValue = parameter;
+            var command = desc
-            if (fullPropertyName.Contains("."))
+                ? (useThenBy ? "ThenByDescending" : "OrderByDescending")
+                : (useThenBy ? "ThenBy" : "OrderBy");
+
+            var resultExpression = Expression.Call(
+                typeof(Queryable),
+                command,
+                new Type[] { typeof(TEntity), lambda.ReturnType },
+                source.Expression,
+                Expression.Quote(lambda));
+
+            return source.Provider.CreateQuery<TEntity>(resultExpression);
+        }
+
+        private static Expression<Func<TEntity, object>> GenerateLambdaWithSafeMemberAccess<TEntity>(string fullPropertyName)
+        {
+            var parameter = Expression.Parameter(typeof(TEntity), "e");
+            Expression propertyValue = parameter;
+            Expression nullCheck = null;
+
+            foreach (var name in fullPropertyName.Split('.'))
             {
-                var parts = fullPropertyName.Split('.');
+                propertyValue = Expression.PropertyOrField(propertyValue, name);
-                for (var i = 0; i < parts.Length - 1; i++)
+
+                if (propertyValue.Type.IsNullable())
                 {
-                    propertyValue = Expression.PropertyOrField(propertyValue, parts[i]);
+                    nullCheck = GenerateOrderNullCheckExpression(propertyValue, nullCheck);
                 }
             }
 
-            var propertyAccess = Expression.MakeMemberAccess(propertyValue, propertyInfo);
+            var expression = nullCheck == null
-            var orderByExpression = Expression.Lambda(propertyAccess, parameter);
+                ? propertyValue
-            var resultExpression = Expression.Call(typeof(Queryable), command, new Type[] { type, propertyInfo.PropertyType },
+                : Expression.Condition(nullCheck, Expression.Default(propertyValue.Type), propertyValue);
-                                          source.Expression, Expression.Quote(orderByExpression));
+
-            return source.Provider.CreateQuery<TEntity>(resultExpression);
+            var converted = Expression.Convert(expression, typeof(object));
+            return Expression.Lambda<Func<TEntity, object>>(converted, parameter);
+        }
+
+        private static Expression GenerateOrderNullCheckExpression(Expression propertyValue, Expression nullCheckExpression)
+        {
+            return nullCheckExpression == null
+                ? Expression.Equal(propertyValue, Expression.Default(propertyValue.Type))
+                : Expression.OrElse(nullCheckExpression, Expression.Equal(propertyValue, Expression.Default(propertyValue.Type)));
         }
     }
 }

Sieve/Extensions/TypeExtensions.cs

@@ -0,0 +1,12 @@
+using System;
+
+namespace Sieve.Extensions
+{
+    public static partial class TypeExtensions
+    {
+        public static bool IsNullable(this Type type)
+        {
+            return !type.IsValueType || Nullable.GetUnderlyingType(type) != null;
+        }
+    }
+}

Sieve/Services/SieveProcessor.cs

@@ -170,21 +170,27 @@ namespace Sieve.Services
             }
 
             Expression outerExpression = null;
-            var parameterExpression = Expression.Parameter(typeof(TEntity), "e");
+            var parameter = Expression.Parameter(typeof(TEntity), "e");
             foreach (var filterTerm in model.GetFiltersParsed())
             {
                 Expression innerExpression = null;
                 foreach (var filterTermName in filterTerm.Names)
                 {
-                    var (fullName, property) = GetSieveProperty<TEntity>(false, true, filterTermName);
+                    var (fullPropertyName, property) = GetSieveProperty<TEntity>(false, true, filterTermName);
                     if (property != null)
                     {
                         var converter = TypeDescriptor.GetConverter(property.PropertyType);
+                        Expression propertyValue = parameter;
+                        Expression nullCheck = null;
 
-                        dynamic propertyValue = parameterExpression;
+                        foreach (var name in fullPropertyName.Split('.'))
-                        foreach (var part in fullName.Split('.'))
                         {
-                            propertyValue = Expression.PropertyOrField(propertyValue, part);
+                            propertyValue = Expression.PropertyOrField(propertyValue, name);
+
+                            if (propertyValue.Type.IsNullable())
+                            {
+                                nullCheck = GenerateFilterNullCheckExpression(propertyValue, nullCheck);
+                            }
                         }
 
                         if (filterTerm.Values == null) continue;
@@ -217,6 +223,11 @@ namespace Sieve.Services
                                 expression = Expression.Not(expression);
                             }
 
+                            if (nullCheck != null)
+                            {
+                                expression = Expression.AndAlso(nullCheck, expression);
+                            }
+
                             if (innerExpression == null)
                             {
                                 innerExpression = expression;
@@ -251,7 +262,14 @@ namespace Sieve.Services
             }
             return outerExpression == null
                 ? result
-                : result.Where(Expression.Lambda<Func<TEntity, bool>>(outerExpression, parameterExpression));
+                : result.Where(Expression.Lambda<Func<TEntity, bool>>(outerExpression, parameter));
+        }
+
+        private static Expression GenerateFilterNullCheckExpression(Expression propertyValue, Expression nullCheckExpression)
+        {
+            return nullCheckExpression == null
+                ? Expression.NotEqual(propertyValue, Expression.Default(propertyValue.Type))
+                : Expression.AndAlso(nullCheckExpression, Expression.NotEqual(propertyValue, Expression.Default(propertyValue.Type)));
         }
 
         private static Expression GetExpression(TFilterTerm filterTerm, dynamic filterValue, dynamic propertyValue)
@@ -311,7 +329,7 @@ namespace Sieve.Services
 
                 if (property != null)
                 {
-                    result = result.OrderByDynamic(fullName, property, sortTerm.Descending, useThenBy);
+                    result = result.OrderByDynamic(fullName, sortTerm.Descending, useThenBy);
                 }
                 else
                 {
@@ -373,12 +391,12 @@ namespace Sieve.Services
             bool isCaseSensitive)
         {
             return Array.Find(typeof(TEntity).GetProperties(), p =>
-                {
+            {
-                    return p.GetCustomAttribute(typeof(SieveAttribute)) is SieveAttribute sieveAttribute
+                return p.GetCustomAttribute(typeof(SieveAttribute)) is SieveAttribute sieveAttribute
-                    && (canSortRequired ? sieveAttribute.CanSort : true)
+                && (!canSortRequired || sieveAttribute.CanSort)
-                    && (canFilterRequired ? sieveAttribute.CanFilter : true)
+                && (!canFilterRequired || sieveAttribute.CanFilter)
-                    && ((sieveAttribute.Name ?? p.Name).Equals(name, isCaseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase));
+                && (sieveAttribute.Name ?? p.Name).Equals(name, isCaseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase);
-                });
+            });
         }
 
         private IQueryable<TEntity> ApplyCustomMethod<TEntity>(IQueryable<TEntity> result, string name, object parent, object[] parameters, object[] optionalParameters = null)

SieveUnitTests/General.cs

@@ -466,5 +466,67 @@ namespace SieveUnitTests
             result = _processor.Apply(model, _posts);
             Assert.AreEqual(1, result.Count());
         }
+
+        [TestMethod]
+        public void FilteringNullsWorks()
+        {
+            var posts = new List<Post>
+            {
+                new Post() {
+                    Id = 1,
+                    Title = null,
+                    LikeCount = 0,
+                    IsDraft = false,
+                    CategoryId = null,
+                    TopComment = null,
+                    FeaturedComment = null
+                },
+            }.AsQueryable();
+
+            var model = new SieveModel()
+            {
+                Filters = "FeaturedComment.Text!@=Some value",
+            };
+
+            var result = _processor.Apply(model, posts);
+            Assert.AreEqual(0, result.Count());
+        }
+
+        [TestMethod]
+        public void SortingNullsWorks()
+        {
+            var posts = new List<Post>
+            {
+                new Post() {
+                    Id = 1,
+                    Title = null,
+                    LikeCount = 0,
+                    IsDraft = false,
+                    CategoryId = null,
+                    TopComment =  new Comment { Id = 1 },
+                    FeaturedComment = null
+                },
+                new Post() {
+                    Id = 2,
+                    Title = null,
+                    LikeCount = 0,
+                    IsDraft = false,
+                    CategoryId = null,
+                    TopComment = null,
+                    FeaturedComment = null
+                },
+            }.AsQueryable();
+
+            var model = new SieveModel()
+            {
+                Sorts = "TopComment.Id",
+            };
+
+            var result = _processor.Apply(model, posts);
+            Assert.AreEqual(2, result.Count());
+            var sortedPosts = result.ToList();
+            Assert.AreEqual(sortedPosts[0].Id, 2);
+            Assert.AreEqual(sortedPosts[1].Id, 1);
+        }
     }
 }