bradwestness / collate-dot-net

Filtering, sorting and paging extensions for .NET IQueryables
MIT License
21 stars 4 forks source link

Null Reference Error when sorting (Issue with EF Core .NET 3.0) #7

Closed JaronrH closed 5 years ago

JaronrH commented 5 years ago

I'm currently working on updating an application to EF Core 3.0 and I found a possible issue with sorting with Collate.NET. SortExpressionBuilder.cs throws a null exception when trying to apply the first OrderBy/OrderByDescending operation. Apparently, the source being passed in can no longer be cast to IOrderedQueryable in QueryableExtensions.IOrderedQueryable Sort(this IQueryable source, IEnumerable sorts).

I was able to fix it by passing in the original Quaryable then checking to see if it could be cast correctly. The fix will require more testing but it appears to be working for me so I wanted to give you a heads up!

QueryableExtensions.cs (2 Lines Changed):

  /// <summary>
        /// Sorts an IQueryable by one or more fields.
        /// </summary>
        /// <typeparam name="T">The collection type.</typeparam>
        /// <param name="source">The collection to be sorted.</param>
        /// <param name="sorts">One or more ISort objects by which to sort the collection..</param>
        /// <returns>the IOrderedQueryable, sorted as specified by the ISort object(s).</returns>
        public static IOrderedQueryable<T> Sort<T>(this IQueryable<T> source, IEnumerable<ISort> sorts)
        {
            var dest = source as IOrderedQueryable<T>;

            if (sorts == null || !sorts.Any())
            {
                return dest;
            }

            SortExpressionBuilder.ApplySorts(ref source, sorts); // <-- Changed

            return source as IOrderedQueryable<T>; // <-- Changed
        }

SortExpressionBuilder.cs (3 Lines Changed/Added):

public static void ApplySorts<T>(ref IQueryable<T> source, IEnumerable<ISort> sorts)  // <-- Changed
        {
            if (sorts == null || !sorts.Any())
            {
                return;
            }

            var sortList = sorts.ToList();
            var itemType = typeof(T);
            var parameter = Expression.Parameter(itemType, "item");
            var methodCalls = new List<MethodCallExpression>();
            var isOrdered = source is IOrderedQueryable<T>; // <-- Added

            for (var i = 0; i < sortList.Count; i++)
            {
                var property = typeof(T).GetProperty(sortList[i].Field);
                var propertyAccess = Expression.MakeMemberAccess(parameter, property);
                var sortExpression = Expression.Lambda(propertyAccess, parameter);
                MethodCallExpression methodCall = null;

                if (!isOrdered && i == 0) // <-- Changed
                {
                    methodCall = (sortList[i].Direction == SortDirection.Ascending)
                        ? Expression.Call(typeof(Queryable), "OrderBy", new Type[] { itemType, property.PropertyType }, source.Expression, Expression.Quote(sortExpression))
                        : Expression.Call(typeof(Queryable), "OrderByDescending", new Type[] { itemType, property.PropertyType }, source.Expression, Expression.Quote(sortExpression));
                }
                else
                {
                    methodCall = (sortList[i].Direction == SortDirection.Ascending)
                        ? Expression.Call(typeof(Queryable), "ThenBy", new Type[] { itemType, property.PropertyType }, source.Expression, Expression.Quote(sortExpression))
                        : Expression.Call(typeof(Queryable), "ThenByDescending", new Type[] { itemType, property.PropertyType }, source.Expression, Expression.Quote(sortExpression));
                }

                source = source.Provider.CreateQuery<T>(methodCall) as IOrderedQueryable<T>;
            }
        }
bradwestness commented 5 years ago

Thanks for the heads up! I haven't gotten around to testing this on .NET Core 3 yet. I'll look into this ASAP.

bradwestness commented 5 years ago

Just pushed an update which resolves this issue and adds compatibility with Entity Framework Core 3.

JaronrH commented 5 years ago

Thanks for the update! (and your fix is even better then what I did!)

bradwestness commented 5 years ago

Haha yeah, there were a couple tests that didn't pass with your changes when running on full framework, but I definitely used your changes and just made a few minor modifications from there to get everything passing, so it was much appreciated!