Add package type filtering to the autocomplete endpoint (#717)

Progress on NuGet/NuGetGallery#7754
NuGet · Dec 17, 2019 · a21082b · a21082b
1 parent d03fd75
commit a21082b
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 14 deletions.
diff --git a/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs b/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs
@@ -30,10 +30,7 @@ public IndexOperationBuilder(
 
         public IndexOperation V3Search(V3SearchRequest request)
         {
-            // Requests with bad parameters yield no results. For the package type case, by specification a package type
-            // valid characters are the same as a package ID.
-            if (request.Skip > MaximumSkip
-                || (request.PackageType != null && !PackageIdValidator.IsValidPackageId(request.PackageType)))
+            if (HasInvalidParameters(request, request.PackageType))
             {
                 return IndexOperation.Empty();
             }
@@ -54,7 +51,7 @@ public IndexOperation V3Search(V3SearchRequest request)
 
         public IndexOperation V2SearchWithSearchIndex(V2SearchRequest request)
         {
-            if (request.Skip > MaximumSkip)
+            if (HasInvalidParameters(request, packageType: null))
             {
                 return IndexOperation.Empty();
             }
@@ -74,7 +71,7 @@ public IndexOperation V2SearchWithSearchIndex(V2SearchRequest request)
 
         public IndexOperation V2SearchWithHijackIndex(V2SearchRequest request)
         {
-            if (request.Skip > MaximumSkip)
+            if (HasInvalidParameters(request, packageType: null))
             {
                 return IndexOperation.Empty();
             }
@@ -94,7 +91,7 @@ public IndexOperation V2SearchWithHijackIndex(V2SearchRequest request)
 
         public IndexOperation Autocomplete(AutocompleteRequest request)
         {
-            if (request.Skip > MaximumSkip)
+            if (HasInvalidParameters(request, request.PackageType))
             {
                 return IndexOperation.Empty();
             }
@@ -181,6 +178,14 @@ private bool TryGetSingleVersion(
             return false;
         }
 
+        private static bool HasInvalidParameters(SearchRequest request, string packageType)
+        {
+            // Requests with bad parameters yield no results. For the package type case, by specification a package type
+            // valid characters are the same as a package ID.
+            return request.Skip > MaximumSkip
+                || (packageType != null && !PackageIdValidator.IsValidPackageId(packageType));
+        }
+
         private static bool PagedToFirstItem(SearchRequest request)
         {
             return request.Skip <= 0 && request.Take >= 1;

diff --git a/src/NuGet.Services.AzureSearch/SearchService/Models/AutocompleteRequest.cs b/src/NuGet.Services.AzureSearch/SearchService/Models/AutocompleteRequest.cs
@@ -9,5 +9,6 @@ namespace NuGet.Services.AzureSearch.SearchService
     public class AutocompleteRequest : SearchRequest
     {
         public AutocompleteRequestType Type { get; set; }
+        public string PackageType { get; set; }
     }
 }
diff --git a/src/NuGet.Services.AzureSearch/SearchService/SearchParametersBuilder.cs b/src/NuGet.Services.AzureSearch/SearchService/SearchParametersBuilder.cs
@@ -94,7 +94,7 @@ public SearchParameters Autocomplete(AutocompleteRequest request, bool isDefault
         {
             var searchParameters = NewSearchParameters();
 
-            ApplySearchIndexFilter(searchParameters, request, isDefaultSearch, packageType: null);
+            ApplySearchIndexFilter(searchParameters, request, isDefaultSearch, request.PackageType);
 
             switch (request.Type)
             {

diff --git a/src/NuGet.Services.SearchService/Controllers/SearchController.cs b/src/NuGet.Services.SearchService/Controllers/SearchController.cs
@@ -135,6 +135,7 @@ public async Task<AutocompleteResponse> AutocompleteAsync(
             string semVerLevel = null,
             string q = null,
             string id = null,
+            string packageType = null,
             bool? debug = false)
         {
             await EnsureInitializedAsync();
@@ -152,6 +153,7 @@ public async Task<AutocompleteResponse> AutocompleteAsync(
                 IncludeSemVer2 = GetIncludeSemVer2(semVerLevel),
                 Query = q ?? id,
                 Type = type,
+                PackageType = packageType,
                 ShowDebug = debug ?? false,
             };
 

diff --git a/tests/NuGet.Services.AzureSearch.Tests/SearchService/IndexOperationBuilderFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/SearchService/IndexOperationBuilderFacts.cs
@@ -27,6 +27,16 @@ public void BuildsSearchOperation()
                 TextBuilder.Verify(x => x.Autocomplete(AutocompleteRequest), Times.Once);
                 ParametersBuilder.Verify(x => x.Autocomplete(AutocompleteRequest, It.IsAny<bool>()), Times.Once);
             }
+
+            [Fact]
+            public void ReturnsEmptyQueryForInvalidPackageType()
+            {
+                AutocompleteRequest.PackageType = "invalid package type";
+
+                var actual = Build();
+
+                Assert.Equal(IndexOperationType.Empty, actual.Type);
+            }
         }
 
         public class V3Search : SearchIndexFacts

diff --git a/tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchParametersBuilderFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchParametersBuilderFacts.cs
@@ -239,12 +239,7 @@ public void Defaults()
             }
 
             [Theory]
-            [InlineData("Dependency")]
-            [InlineData("DotnetTool")]
-            [InlineData("Template")]
-            [InlineData("PackageType.With.Dots")]
-            [InlineData("PackageType-With-Hyphens")]
-            [InlineData("PackageType_With_Underscores")]
+            [MemberData(nameof(ValidPackageTypes))]
             public void PackageTypeFiltering(string packageType)
             {
                 var request = new V3SearchRequest
@@ -468,6 +463,33 @@ public void SearchFilters(bool includePrerelease, bool includeSemVer2, string fi
 
                 Assert.Equal(filter, output.Filter);
             }
+
+            [Theory]
+            [MemberData(nameof(ValidPackageTypes))]
+            public void PackageTypeFiltering(string packageType)
+            {
+                var request = new AutocompleteRequest
+                {
+                    PackageType = packageType,
+                };
+
+                var output = _target.Autocomplete(request, It.IsAny<bool>());
+
+                Assert.Equal($"searchFilters eq 'Default' and filterablePackageTypes/any(p: p eq '{packageType.ToLowerInvariant()}')", output.Filter);
+            }
+
+            [Fact]
+            public void InvalidPackageType()
+            {
+                var request = new AutocompleteRequest
+                {
+                    PackageType = "something's-weird",
+                };
+
+                var output = _target.Autocomplete(request, It.IsAny<bool>());
+
+                Assert.Equal("searchFilters eq 'Default'", output.Filter);
+            }
         }
 
         public abstract class BaseFacts
@@ -503,6 +525,17 @@ public abstract class BaseFacts
                 .Cast<V2SortBy>()
                 .Select(x => new object[] { x });
 
+
+            public static IEnumerable<object[]> ValidPackageTypes => new[]
+            {
+                new object[] { "Dependency" },
+                new object[] { "DotnetTool" },
+                new object[] { "Template" },
+                new object[] { "PackageType.With.Dots" },
+                new object[] { "PackageType-With-Hyphens" },
+                new object[] { "PackageType_With_Underscores" },
+            };
+
             public BaseFacts()
             {
                 _target = new SearchParametersBuilder();

diff --git a/tests/NuGet.Services.SearchService.Tests/Controllers/SearchControllerFacts.cs b/tests/NuGet.Services.SearchService.Tests/Controllers/SearchControllerFacts.cs
@@ -395,6 +395,7 @@ public async Task HasDefaultParameters()
                 Assert.False(lastRequest.IncludeSemVer2);
                 Assert.Null(lastRequest.Query);
                 Assert.False(lastRequest.ShowDebug);
+                Assert.Null(lastRequest.PackageType);
                 Assert.Equal(AutocompleteRequestType.PackageIds, lastRequest.Type);
             }
 
@@ -414,6 +415,7 @@ await _target.AutocompleteAsync(
                     semVerLevel: null,
                     q: null,
                     id: null,
+                    packageType: null,
                     debug: null);
 
                 _searchService.Verify(x => x.AutocompleteAsync(It.IsAny<AutocompleteRequest>()), Times.Once);
@@ -424,6 +426,7 @@ await _target.AutocompleteAsync(
                 Assert.False(lastRequest.IncludeSemVer2);
                 Assert.Null(lastRequest.Query);
                 Assert.False(lastRequest.ShowDebug);
+                Assert.Null(lastRequest.PackageType);
                 Assert.Equal(AutocompleteRequestType.PackageIds, lastRequest.Type);
             }
 
@@ -442,6 +445,7 @@ await _target.AutocompleteAsync(
                     prerelease: true,
                     semVerLevel: "2.0.0",
                     q: "windows azure storage",
+                    packageType: "DotnetTool",
                     debug: true);
 
                 _searchService.Verify(x => x.AutocompleteAsync(It.IsAny<AutocompleteRequest>()), Times.Once);
@@ -452,6 +456,7 @@ await _target.AutocompleteAsync(
                 Assert.True(lastRequest.IncludeSemVer2);
                 Assert.Equal("windows azure storage", lastRequest.Query);
                 Assert.True(lastRequest.ShowDebug);
+                Assert.Equal("DotnetTool", lastRequest.PackageType);
                 Assert.Equal(AutocompleteRequestType.PackageIds, lastRequest.Type);
             }