Prevent feed query from sorting on version.

OData sorts by Id, Version by default if it requires a stable sort and no
sort is explicitly specified. However sorting by version makes no sense
since SQL performs string sorts which is incorrect and very expensive.

Facts/Facts.csproj

@@ -140,6 +140,7 @@
     <Compile Include="Services\FileSystemFileStorageServiceFacts.cs" />
     <Compile Include="Services\MessageServiceFacts.cs" />
     <Compile Include="Services\NuGetExeDownloaderServiceFacts.cs" />
+    <Compile Include="Services\ODataRemoveVersionSorterFacts.cs" />
     <Compile Include="Services\PackageServiceFacts.cs" />
     <Compile Include="Services\UploadFileServiceFacts.cs" />
     <Compile Include="Services\UsersServiceFacts.cs" />

Facts/Services/ODataRemoveVersionSorterFacts.cs

@@ -0,0 +1,49 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using Xunit;
+
+namespace NuGetGallery
+{
+    public class ODataRemoveVersionSorterFacts
+    {
+        [Fact]
+        public void RemoveVersionSortRemovesThenByOnVersion()
+        {
+            // Arrange
+            var package_AB = new V2FeedPackage { Id = "A", Version = "B"};
+            var package_AA = new V2FeedPackage { Id = "A", Version = "A"};
+            var package_CA = new V2FeedPackage { Id = "C", Version = "A"};
+
+            var source = new[] { package_AB, package_CA, package_AA }.AsQueryable();
+
+            // Act
+            var resultA = source.OrderBy(p => p.Id).ThenBy(p => p.Version);
+            var resultB = source.WithoutVersionSort().OrderBy(p => p.Id).ThenBy(p => p.Version);
+
+            // Assert
+            Assert.Equal(new[] { package_AA, package_AB, package_CA }, resultA);
+            Assert.Equal(new[] { package_AB, package_AA, package_CA }, resultB);
+        }
+
+        [Fact]
+        public void RemoveVersionSortRemovesThenByDescendingOnVersion()
+        {
+            // Arrange
+            var package_AB = new V2FeedPackage { Id = "A", Version = "B" };
+            var package_AA = new V2FeedPackage { Id = "A", Version = "A" };
+            var package_AC = new V2FeedPackage { Id = "A", Version = "C" };
+
+            var source = new[] { package_AB, package_AC, package_AA }.AsQueryable();
+
+            // Act
+            var resultA = source.OrderBy(p => p.Id).ThenByDescending(p => p.Version);
+            var resultB = source.WithoutVersionSort().OrderBy(p => p.Id).ThenByDescending(p => p.Version);
+
+            // Assert
+            Assert.Equal(new[] { package_AC, package_AB, package_AA }, resultA);
+            Assert.Equal(new[] { package_AB, package_AC, package_AA }, resultB);
+        }
+    }
+}

Website/DataServices/ODataRemoveVersionSorter.cs

@@ -0,0 +1,50 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Linq.Expressions;
+using System.Web;
+
+namespace NuGetGallery
+{
+    public class ODataRemoveVersionSorter : ExpressionVisitor
+    {
+        protected override Expression VisitMethodCall(MethodCallExpression node)
+        {
+            if (IsSortingOnVersion(node))
+            {
+                // The expression is of the format Queryable.OrderBy(<Expression>, <Order-by-params>). To avoid performing the 
+                // method, we ignore it, traversing the passed in expression instead.
+                return Visit(node.Arguments[0]);
+            }
+            return base.VisitMethodCall(node);
+        }
+
+        private bool IsSortingOnVersion(MethodCallExpression expression)
+        {
+            var methodsToIgnore = new[] { "ThenBy", "ThenByDescending" };
+            var method = expression.Method;
+
+            return method.DeclaringType == typeof(Queryable) &&
+                   methodsToIgnore.Contains(method.Name, StringComparer.Ordinal) &&
+                   IsVersionArgument(expression);
+        }
+
+        private bool IsVersionArgument(MethodCallExpression expression)
+        {
+            if (expression.Arguments.Count == 2 && expression.Arguments[1].NodeType == ExpressionType.Quote)
+            {
+                var unaryExpression = expression.Arguments[1] as UnaryExpression;
+                if (unaryExpression != null)
+                {
+                    var lambdaExpression = unaryExpression.Operand as LambdaExpression;
+                    if (lambdaExpression != null)
+                    {
+                        var memberAccess = lambdaExpression.Body as MemberExpression;
+                        return memberAccess != null && memberAccess.Member.Name.Equals("Version", StringComparison.Ordinal);
+                    }
+                }
+            }
+            return false;
+        }
+    }
+}

Website/DataServices/PackageExtensions.cs

@@ -1,7 +1,10 @@
 using System;
+using System.Collections.ObjectModel;
 using System.Data.Entity;
 using System.Linq;
+using System.Linq.Expressions;
 using OData.Linq;
+using QueryInterceptor;
 
 namespace NuGetGallery
 {
@@ -14,6 +17,7 @@ public static IQueryable<V1FeedPackage> ToV1FeedPackageQuery(this IQueryable<Pac
             siteRoot = EnsureTrailingSlash(siteRoot);
             return packages
                      .WithoutNullPropagation()
+                     .WithoutVersionSort()
                      .Include(p => p.PackageRegistration)
                      .Select(p => new V1FeedPackage
                      {
@@ -52,6 +56,7 @@ public static IQueryable<V2FeedPackage> ToV2FeedPackageQuery(this IQueryable<Pac
             siteRoot = EnsureTrailingSlash(siteRoot);
             return packages
                      .WithoutNullPropagation()
+                     .WithoutVersionSort()
                      .Include(p => p.PackageRegistration)
                      .Select(p => new V2FeedPackage
                      {
@@ -86,6 +91,11 @@ public static IQueryable<V2FeedPackage> ToV2FeedPackageQuery(this IQueryable<Pac
                      });
         }
 
+        internal static IQueryable<TVal> WithoutVersionSort<TVal>(this IQueryable<TVal> feedQuery)
+        {
+            return feedQuery.InterceptWith(new ODataRemoveVersionSorter());
+        }
+
         private static string EnsureTrailingSlash(string siteRoot)
         {
             if (!siteRoot.EndsWith("/", StringComparison.Ordinal))

Website/Website.csproj

@@ -215,6 +215,7 @@
     <Compile Include="CuratedPackagesController.generated.cs">
       <DependentUpon>T4MVC.tt</DependentUpon>
     </Compile>
+    <Compile Include="DataServices\ODataRemoveVersionSorter.cs" />
     <Compile Include="DataServices\V2CuratedFeed.svc.cs">
       <DependentUpon>V2CuratedFeed.svc</DependentUpon>
     </Compile>
@@ -930,4 +931,4 @@
     </VisualStudio>
   </ProjectExtensions>
   <Import Project="$(SolutionDir)\.nuget\NuGet.targets" />
-</Project>
+</Project>