Hijack IsLatest(Stable)Version OData filter when semVerLevel=2.0.0 (#…

…3966)
NuGet · May 31, 2017 · 647c959 · 647c959
1 parent ac11c00
commit 647c959
Show file tree

Hide file tree

Showing 9 changed files with 329 additions and 54 deletions.
diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj
@@ -1475,6 +1475,7 @@
     <Compile Include="ViewModels\PackageListSearchViewModel.cs" />
     <Compile Include="WebApi\PlainTextResult.cs" />
     <Compile Include="WebApi\QueryResult.cs" />
+    <Compile Include="WebApi\QueryResultDefaults.cs" />
     <Compile Include="WebApi\QueryResultExtensions.cs" />
     <Content Include="App_Data\Files\Content\ReadOnly.md" />
     <Content Include="ApplicationInsights.config">

diff --git a/src/NuGetGallery/OData/QueryAllowed/ODataQueryFilter.cs b/src/NuGetGallery/OData/QueryAllowed/ODataQueryFilter.cs
@@ -31,6 +31,9 @@ public enum ODataOperators
             Top = 1 << 8
         }
 
+        internal const string IsLatestVersion = "IsLatestVersion";
+        internal const string IsAbsoluteLatestVersion = "IsAbsoluteLatestVersion";
+
         private static readonly string ResourcesNamespace = "NuGetGallery.OData.QueryAllowed.Data";
         private HashSet<ODataOperators> _allowedOperatorPatterns = null;
 

diff --git a/src/NuGetGallery/OData/SearchService/SearchAdaptor.cs b/src/NuGetGallery/OData/SearchService/SearchAdaptor.cs
@@ -12,6 +12,7 @@
 using System.Web.Routing;
 using NuGet.Services.Search.Models;
 using NuGetGallery.Infrastructure.Lucene;
+using NuGetGallery.OData.QueryFilter;
 using NuGetGallery.OData.QueryInterceptors;
 using QueryInterceptor;
 
@@ -209,7 +210,9 @@ private static bool TryReadSearchFilter(bool allVersionsInIndex, string url, boo
             string filter;
             if (queryTerms.TryGetValue("$filter", out filter))
             {
-                if (!ignoreLatestVersionFilter && !(filter.Equals("IsLatestVersion", StringComparison.Ordinal) || filter.Equals("IsAbsoluteLatestVersion", StringComparison.Ordinal)))
+                if (!ignoreLatestVersionFilter 
+                    && !(filter.Equals(ODataQueryFilter.IsLatestVersion, StringComparison.Ordinal) 
+                        || filter.Equals(ODataQueryFilter.IsAbsoluteLatestVersion, StringComparison.Ordinal)))
                 {
                     searchFilter = null;
                     return false;

diff --git a/src/NuGetGallery/WebApi/QueryResult.cs b/src/NuGetGallery/WebApi/QueryResult.cs
@@ -8,25 +8,17 @@
 using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
+using System.Web;
 using System.Web.Http;
 using System.Web.Http.OData;
 using System.Web.Http.OData.Query;
 using System.Web.Http.Results;
 using Microsoft.Data.OData;
 using Microsoft.Data.OData.Query;
+using NuGetGallery.OData.QueryFilter;
 
 namespace NuGetGallery.WebApi
 {
-    public static class QueryResultDefaults
-    {
-        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2211:NonConstantFieldsShouldNotBeVisible")]
-        public static ODataQuerySettings DefaultQuerySettings = new ODataQuerySettings()
-        {
-            HandleNullPropagation = HandleNullPropagationOption.False,
-            EnsureStableOrdering = true,
-            EnableConstantParameterization = false
-        };
-    }
 
     public class QueryResult<TModel>
         : IHttpActionResult
@@ -37,6 +29,7 @@ public class QueryResult<TModel>
         private readonly long? _totalResults;
         private readonly Func<ODataQueryOptions<TModel>, ODataQuerySettings, long?, Uri> _generateNextLink;
         private readonly bool _isPagedResult;
+        private readonly int? _semVerLevelKey;
 
         private readonly ODataValidationSettings _validationSettings;
         private readonly ODataQuerySettings _querySettings;
@@ -59,6 +52,9 @@ public QueryResult(
             _totalResults = totalResults;
             _generateNextLink = generateNextLink;
 
+            var queryDictionary = HttpUtility.ParseQueryString(queryOptions.Request.RequestUri.Query);
+            _semVerLevelKey = SemVerLevelKey.ForSemVerLevel(queryDictionary["semVerLevel"]);
+
             if (_totalResults.HasValue && generateNextLink != null)
             {
                 _isPagedResult = true;
@@ -92,7 +88,7 @@ public async Task<HttpResponseMessage> ExecuteAsync(CancellationToken cancellati
 
                 return response;
             }
-            catch(Exception e)
+            catch (Exception e)
             {
                 QuietLog.LogHandledException(e);
                 throw;
@@ -121,7 +117,22 @@ public IHttpActionResult GetInnerResult()
 
                 if (queryOptions.Filter != null)
                 {
-                    queryResults = queryOptions.Filter.ApplyTo(queryResults, _querySettings);
+                    if (_semVerLevelKey != SemVerLevelKey.Unknown
+                        && (string.Equals(queryOptions.Filter.RawValue, ODataQueryFilter.IsLatestVersion, StringComparison.OrdinalIgnoreCase)
+                            || string.Equals(queryOptions.Filter.RawValue, ODataQueryFilter.IsAbsoluteLatestVersion, StringComparison.OrdinalIgnoreCase)))
+                    {
+                        // The client uses IsLatestVersion and IsAbsoluteLatestVersion by default,
+                        // and just appends semVerLevel=2.0.0 to the query string.
+                        // When semVerLevel=2.0.0, we should not restrict the filter to only return IsLatest(Stable)=true,
+                        // but also include IsLatest(Stable)SemVer2=true. These additional properties are not exposed on the OData entities however.
+                        // As the proper filtering already should 've happened earlier in the pipeline (SQL or search service),
+                        // the OData filter is redundant, so all we need to do here is to avoid 
+                        // the OData filter to be applied on an already correctly filtered result set.
+                    }
+                    else
+                    {
+                        queryResults = queryOptions.Filter.ApplyTo(queryResults, _querySettings);
+                    }
                 }
 
                 if (queryOptions.OrderBy != null
@@ -302,10 +313,10 @@ private IQueryable ExecuteQuery(IQueryable<TModel> queryable, ODataQueryOptions<
             {
                 return projection;
             }
-            
+
             return queryResult;
         }
-        
+
         private BadRequestErrorMessageResult BadRequest(string message)
         {
             return new BadRequestErrorMessageResult(message, _controller);

diff --git a/src/NuGetGallery/WebApi/QueryResultDefaults.cs b/src/NuGetGallery/WebApi/QueryResultDefaults.cs
@@ -0,0 +1,18 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Web.Http.OData.Query;
+
+namespace NuGetGallery.WebApi
+{
+    public static class QueryResultDefaults
+    {
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2211:NonConstantFieldsShouldNotBeVisible")]
+        public static ODataQuerySettings DefaultQuerySettings = new ODataQuerySettings()
+        {
+            HandleNullPropagation = HandleNullPropagationOption.False,
+            EnsureStableOrdering = true,
+            EnableConstantParameterization = false
+        };
+    }
+}
diff --git a/tests/NuGetGallery.Facts/Controllers/ODataFeedControllerFactsBase.cs b/tests/NuGetGallery.Facts/Controllers/ODataFeedControllerFactsBase.cs
@@ -122,43 +122,58 @@ private static IQueryable<Package> CreatePackagesQueryable()
                 {
                     PackageRegistration = packageRegistration,
                     Version = "1.0.0.0",
-                    NormalizedVersion = "1.0.0.0",
+                    NormalizedVersion = "1.0.0",
                     SemVerLevelKey = SemVerLevelKey.Unknown
                 },
                 new Package
                 {
                     PackageRegistration = packageRegistration,
                     Version = "1.0.0.0-alpha",
-                    NormalizedVersion = "1.0.0.0-alpha",
+                    NormalizedVersion = "1.0.0-alpha",
+                    IsPrerelease = true,
                     SemVerLevelKey = SemVerLevelKey.Unknown
                 },
                 new Package
                 {
                     PackageRegistration = packageRegistration,
                     Version = "2.0.0",
                     NormalizedVersion = "2.0.0",
-                    SemVerLevelKey = SemVerLevelKey.Unknown
+                    SemVerLevelKey = SemVerLevelKey.Unknown,
+                    IsLatestStable = true
                 },
                 new Package
                 {
                     PackageRegistration = packageRegistration,
                     Version = "2.0.0-alpha",
                     NormalizedVersion = "2.0.0-alpha",
-                    SemVerLevelKey = SemVerLevelKey.SemVer2
+                    IsPrerelease = true,
+                    SemVerLevelKey = SemVerLevelKey.Unknown,
+                    IsLatest = true
                 },
                 new Package
                 {
                     PackageRegistration = packageRegistration,
                     Version = "2.0.0-alpha.1",
                     NormalizedVersion = "2.0.0-alpha.1",
+                    IsPrerelease = true,
                     SemVerLevelKey = SemVerLevelKey.SemVer2
                 },
                 new Package
                 {
                     PackageRegistration = packageRegistration,
                     Version = "2.0.0+metadata",
                     NormalizedVersion = "2.0.0",
-                    SemVerLevelKey = SemVerLevelKey.SemVer2
+                    SemVerLevelKey = SemVerLevelKey.SemVer2,
+                    IsLatestStableSemVer2 = true
+                },
+                new Package
+                {
+                    PackageRegistration = packageRegistration,
+                    Version = "2.0.1-alpha.1",
+                    NormalizedVersion = "2.0.1-alpha.1",
+                    IsPrerelease = true,
+                    SemVerLevelKey = SemVerLevelKey.SemVer2,
+                    IsLatestSemVer2 = true
                 }
             };
 

diff --git a/tests/NuGetGallery.Facts/Controllers/ODataV1FeedControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ODataV1FeedControllerFacts.cs
@@ -23,7 +23,7 @@ public async Task Get_FiltersSemVerV2PackageVersions()
 
             // Assert
             AssertSemVer2PackagesFilteredFromResult(resultSet);
-            Assert.Equal(NonSemVer2Packages.Count, resultSet.Count);
+            Assert.Equal(NonSemVer2Packages.Where(p => !p.IsPrerelease).Count(), resultSet.Count);
         }
 
         [Fact]
@@ -35,7 +35,7 @@ public async Task GetCount_FiltersSemVerV2PackageVersions()
                 "/api/v1/Packages/$count");
 
             // Assert
-            Assert.Equal(NonSemVer2Packages.Count, count);
+            Assert.Equal(NonSemVer2Packages.Where(p => !p.IsPrerelease).Count(), count);
         }
 
         [Fact]
@@ -48,7 +48,7 @@ public async Task FindPackagesById_FiltersSemVerV2PackageVersions()
 
             // Assert
             AssertSemVer2PackagesFilteredFromResult(resultSet);
-            Assert.Equal(NonSemVer2Packages.Count, resultSet.Count);
+            Assert.Equal(NonSemVer2Packages.Where(p => !p.IsPrerelease).Count(), resultSet.Count);
         }
 
         [Fact]
@@ -61,7 +61,7 @@ public async Task Search_FiltersSemVerV2PackageVersions()
 
             // Assert
             AssertSemVer2PackagesFilteredFromResult(resultSet);
-            Assert.Equal(NonSemVer2Packages.Count, resultSet.Count);
+            Assert.Equal(NonSemVer2Packages.Where(p => !p.IsPrerelease).Count(), resultSet.Count);
         }
 
         [Fact]
@@ -73,7 +73,7 @@ public async Task SearchCount_FiltersSemVerV2PackageVersions()
                 $"/api/v1/Search/$count?searchTerm='{TestPackageId}'");
 
             // Assert
-            Assert.Equal(NonSemVer2Packages.Count, searchCount);
+            Assert.Equal(NonSemVer2Packages.Where(p => !p.IsPrerelease).Count(), searchCount);
         }
 
         protected override ODataV1FeedController CreateController(IEntityRepository<Package> packagesRepository,