From 895f444e3e48a354e39ea5dbbfe077f7c5486ead Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Sun, 21 Jul 2019 15:56:08 -0700 Subject: [PATCH] Optimize packageid:{id} version:{version} ignoreFilter=true case (#605) This case covered the gallery hijack for OData Packages(Id=,Version=). This accounts for 22.47% of all requests in the past 90 days. As with the previous optimization PR (#602) switching from a search to a document lookup cut the query time in roughly a half. Progress on https://github.com/NuGet/NuGetGallery/issues/6466 --- .../AzureSearchTelemetryService.cs | 14 ++ .../IAzureSearchTelemetryService.cs | 2 + .../SearchService/AzureSearchService.cs | 50 +++++ .../SearchService/ISearchResponseBuilder.cs | 10 + .../SearchService/IndexOperationBuilder.cs | 53 +++++ .../SearchService/SearchResponseBuilder.cs | 76 +++++++ .../Constants.cs | 7 +- .../BasicTests/V2SearchProtocolTests.cs | 134 ++++++++++- .../SearchService/AzureSearchServiceFacts.cs | 70 ++++++ .../IndexOperationBuilderFacts.cs | 209 +++++++++++++++++- .../SearchService/SearchTextBuilderFacts.cs | 5 - 11 files changed, 619 insertions(+), 11 deletions(-) diff --git a/src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs b/src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs index 89386e00d..6ed492880 100644 --- a/src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs +++ b/src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs @@ -373,5 +373,19 @@ public void TrackV3GetDocument(TimeSpan elapsed) Prefix + "V3GetDocumentMs", elapsed.TotalMilliseconds); } + + public void TrackV2GetDocumentWithSearchIndex(TimeSpan elapsed) + { + _telemetryClient.TrackMetric( + Prefix + "V2GetDocumentWithSearchIndexMs", + elapsed.TotalMilliseconds); + } + + public void TrackV2GetDocumentWithHijackIndex(TimeSpan elapsed) + { + _telemetryClient.TrackMetric( + Prefix + "V2GetDocumentWithHijackIndexMs", + elapsed.TotalMilliseconds); + } } } diff --git a/src/NuGet.Services.AzureSearch/IAzureSearchTelemetryService.cs b/src/NuGet.Services.AzureSearch/IAzureSearchTelemetryService.cs index 207c680a3..67da6832d 100644 --- a/src/NuGet.Services.AzureSearch/IAzureSearchTelemetryService.cs +++ b/src/NuGet.Services.AzureSearch/IAzureSearchTelemetryService.cs @@ -49,5 +49,7 @@ void TrackDownloadCountDecrease( void TrackAuxiliary2AzureSearchCompleted(JobOutcome outcome, TimeSpan elapsed); IDisposable TrackUploadDownloadsSnapshot(int packageIdCount); void TrackV3GetDocument(TimeSpan elapsed); + void TrackV2GetDocumentWithSearchIndex(TimeSpan elapsed); + void TrackV2GetDocumentWithHijackIndex(TimeSpan elapsed); } } \ No newline at end of file diff --git a/src/NuGet.Services.AzureSearch/SearchService/AzureSearchService.cs b/src/NuGet.Services.AzureSearch/SearchService/AzureSearchService.cs index 32f755b90..dbf54d5be 100644 --- a/src/NuGet.Services.AzureSearch/SearchService/AzureSearchService.cs +++ b/src/NuGet.Services.AzureSearch/SearchService/AzureSearchService.cs @@ -4,6 +4,7 @@ using System; using System.Threading.Tasks; using NuGet.Services.AzureSearch.Wrappers; +using NuGetGallery; namespace NuGet.Services.AzureSearch.SearchService { @@ -119,6 +120,42 @@ private async Task UseHijackIndexAsync(V2SearchRequest request V2SearchResponse output; switch (operation.Type) { + case IndexOperationType.Get: + var documentResult = await Measure.DurationWithValueAsync( + () => _hijackIndex.Documents.GetOrNullAsync(operation.DocumentKey)); + + // If the request is excluding SemVer 2.0.0 packages and the document is SemVer 2.0.0, filter it + // out. The must be done after fetching the document because some SemVer 2.0.0 packages are + // SemVer 2.0.0 because of a dependency version or because of build metadata (e.g. 1.0.0+metadata). + // Neither of these reasons is apparent from the request. Build metadata is not used for comparison + // so if someone searchs for "version:1.0.0+foo" and the actual package version is "1.0.0" or + // "1.0.0+bar" the document will still be returned. + // + // A request looking for a specific package version that is SemVer 2.0.0 due to dots in the + // prerelease label (e.g. 1.0.0-beta.1) could no-op if the request is not including SemVer 2.0.0 + // but that's not worth the effort since we can catch that case after fetching the document anyway. + // It's more consistent with the search operation path to make the SemVer 2.0.0 filtering decision + // solely based on the document data. + // + // Note that the prerelease filter is ignored here by design. This is legacy behavior from the + // previous search implementation. + var document = documentResult.Value; + if (document != null + && !request.IncludeSemVer2 + && document.SemVerLevel == SemVerLevelKey.SemVer2) + { + document = null; + } + + output = _responseBuilder.V2FromHijackDocument( + request, + operation.DocumentKey, + document, + documentResult.Duration); + + _telemetryService.TrackV2GetDocumentWithHijackIndex(documentResult.Duration); + break; + case IndexOperationType.Search: var result = await Measure.DurationWithValueAsync(() => _hijackIndex.Documents.SearchAsync( operation.SearchText, @@ -148,6 +185,19 @@ private async Task UseSearchIndexAsync(V2SearchRequest request V2SearchResponse output; switch (operation.Type) { + case IndexOperationType.Get: + var documentResult = await Measure.DurationWithValueAsync( + () => _searchIndex.Documents.GetOrNullAsync(operation.DocumentKey)); + + output = _responseBuilder.V2FromSearchDocument( + request, + operation.DocumentKey, + documentResult.Value, + documentResult.Duration); + + _telemetryService.TrackV2GetDocumentWithSearchIndex(documentResult.Duration); + break; + case IndexOperationType.Search: var result = await Measure.DurationWithValueAsync(() => _searchIndex.Documents.SearchAsync( operation.SearchText, diff --git a/src/NuGet.Services.AzureSearch/SearchService/ISearchResponseBuilder.cs b/src/NuGet.Services.AzureSearch/SearchService/ISearchResponseBuilder.cs index e5032135d..ce67e41ba 100644 --- a/src/NuGet.Services.AzureSearch/SearchService/ISearchResponseBuilder.cs +++ b/src/NuGet.Services.AzureSearch/SearchService/ISearchResponseBuilder.cs @@ -20,12 +20,22 @@ V2SearchResponse V2FromSearch( SearchParameters parameters, DocumentSearchResult result, TimeSpan duration); + V2SearchResponse V2FromHijackDocument( + V2SearchRequest request, + string documentKey, + HijackDocument.Full document, + TimeSpan duration); V3SearchResponse V3FromSearch( V3SearchRequest request, string text, SearchParameters parameters, DocumentSearchResult result, TimeSpan duration); + V2SearchResponse V2FromSearchDocument( + V2SearchRequest request, + string documentKey, + SearchDocument.Full document, + TimeSpan duration); V3SearchResponse V3FromSearchDocument( V3SearchRequest request, string documentKey, diff --git a/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs b/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs index 14fadcc08..6770739f3 100644 --- a/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs +++ b/src/NuGet.Services.AzureSearch/SearchService/IndexOperationBuilder.cs @@ -5,6 +5,7 @@ using System.Linq; using NuGet.Indexing; using NuGet.Packaging; +using NuGet.Versioning; namespace NuGet.Services.AzureSearch.SearchService { @@ -39,6 +40,13 @@ public IndexOperation V3Search(V3SearchRequest request) public IndexOperation V2SearchWithSearchIndex(V2SearchRequest request) { var parsed = _textBuilder.ParseV2Search(request); + + IndexOperation indexOperation; + if (TryGetSearchDocumentByKey(request, parsed, out indexOperation)) + { + return indexOperation; + } + var text = _textBuilder.Build(parsed); var parameters = _parametersBuilder.V2Search(request); return IndexOperation.Search(text, parameters); @@ -47,6 +55,13 @@ public IndexOperation V2SearchWithSearchIndex(V2SearchRequest request) public IndexOperation V2SearchWithHijackIndex(V2SearchRequest request) { var parsed = _textBuilder.ParseV2Search(request); + + IndexOperation indexOperation; + if (TryGetHijackDocumentByKey(request, parsed, out indexOperation)) + { + return indexOperation; + } + var text = _textBuilder.Build(parsed); var parameters = _parametersBuilder.V2Search(request); return IndexOperation.Search(text, parameters); @@ -79,6 +94,26 @@ private bool TryGetSearchDocumentByKey( return false; } + private bool TryGetHijackDocumentByKey( + SearchRequest request, + ParsedQuery parsed, + out IndexOperation indexOperation) + { + if (PagedToFirstItem(request) + && parsed.Grouping.Count == 2 + && TryGetSinglePackageId(parsed, out var packageId) + && TryGetSingleVersion(parsed, out var normalizedVersion)) + { + var documentKey = DocumentUtilities.GetHijackDocumentKey(packageId, normalizedVersion); + + indexOperation = IndexOperation.Get(documentKey); + return true; + } + + indexOperation = null; + return false; + } + private bool TryGetSinglePackageId( ParsedQuery parsed, out string packageId) @@ -98,6 +133,24 @@ private bool TryGetSinglePackageId( return false; } + private bool TryGetSingleVersion( + ParsedQuery parsed, + out string normalizedVersion) + { + if (parsed.Grouping.TryGetValue(QueryField.Version, out var terms) + && terms.Count == 1) + { + if (NuGetVersion.TryParse(terms.First(), out var parsedVersion)) + { + normalizedVersion = parsedVersion.ToNormalizedString(); + return true; + } + } + + normalizedVersion = null; + return false; + } + private static bool PagedToFirstItem(SearchRequest request) { return request.Skip <= 0 && request.Take >= 1; diff --git a/src/NuGet.Services.AzureSearch/SearchService/SearchResponseBuilder.cs b/src/NuGet.Services.AzureSearch/SearchService/SearchResponseBuilder.cs index e5fc9ca1e..0c8072b45 100644 --- a/src/NuGet.Services.AzureSearch/SearchService/SearchResponseBuilder.cs +++ b/src/NuGet.Services.AzureSearch/SearchService/SearchResponseBuilder.cs @@ -72,6 +72,36 @@ public V2SearchResponse V2FromSearch( p => ToV2SearchPackage(p)); } + public V2SearchResponse V2FromSearchDocument( + V2SearchRequest request, + string documentKey, + SearchDocument.Full document, + TimeSpan duration) + { + return ToResponse( + request, + _options.Value.SearchIndexName, + documentKey, + document, + duration, + p => ToV2SearchPackage(p)); + } + + public V2SearchResponse V2FromHijackDocument( + V2SearchRequest request, + string documentKey, + HijackDocument.Full document, + TimeSpan duration) + { + return ToResponse( + request, + _options.Value.HijackIndexName, + documentKey, + document, + duration, + p => ToV2SearchPackage(p, request.IncludeSemVer2)); + } + public V3SearchResponse V3FromSearchDocument( V3SearchRequest request, string documentKey, @@ -206,6 +236,52 @@ private static string TitleThenId(IBaseMetadataDocument document) return document.PackageId; } + private V2SearchResponse ToResponse( + V2SearchRequest request, + string indexName, + string documentKey, + T document, + TimeSpan duration, + Func toPackage) + where T : class + { + var data = new List(); + if (document != null) + { + var package = toPackage(document); + package.Debug = request.ShowDebug ? new DebugDocumentResult { Document = document } : null; + data.Add(package); + } + + if (request.CountOnly) + { + return new V2SearchResponse + { + TotalHits = data.Count, + Debug = DebugInformation.CreateFromGetOrNull( + request, + indexName, + documentKey, + duration, + AuxiliaryData.Metadata), + }; + } + else + { + return new V2SearchResponse + { + TotalHits = data.Count, + Data = data, + Debug = DebugInformation.CreateFromGetOrNull( + request, + indexName, + documentKey, + duration, + AuxiliaryData.Metadata), + }; + } + } + private V2SearchResponse ToResponse( V2SearchRequest request, SearchParameters parameters, diff --git a/tests/BasicSearchTests.FunctionalTests.Core/Constants.cs b/tests/BasicSearchTests.FunctionalTests.Core/Constants.cs index 35676ee91..3e8159659 100644 --- a/tests/BasicSearchTests.FunctionalTests.Core/Constants.cs +++ b/tests/BasicSearchTests.FunctionalTests.Core/Constants.cs @@ -7,8 +7,13 @@ public static class Constants { // Predefined Texts public const string TestPackageId = "BaseTestPackage"; - public const string TestPackageIdSemVer2 = "BaseTestPackage.SemVer2"; + public const string TestPackageId_SearchFilters = "BaseTestPackage.SearchFilters"; + public const string TestPackageId_SemVer2 = "BaseTestPackage.SemVer2"; public const string TestPackageVersion = "1.0.0"; + public const string TestPackageVersion_SearchFilters_Default = "1.1.0"; + public const string TestPackageVersion_SearchFilters_Prerel = "1.2.0-beta"; + public const string TestPackageVersion_SearchFilters_SemVer2 = "1.3.0+metadata"; + public const string TestPackageVersion_SearchFilters_PrerelSemVer2 = "1.4.0-delta.4"; public const string TestPackageTitle = "BaseTestPackage"; public const string TestPackageDescription = "Package description"; public const string TestPackageSummary = ""; diff --git a/tests/NuGet.Services.AzureSearch.FunctionalTests/BasicTests/V2SearchProtocolTests.cs b/tests/NuGet.Services.AzureSearch.FunctionalTests/BasicTests/V2SearchProtocolTests.cs index df3fc7b84..d6fd2c8ca 100644 --- a/tests/NuGet.Services.AzureSearch.FunctionalTests/BasicTests/V2SearchProtocolTests.cs +++ b/tests/NuGet.Services.AzureSearch.FunctionalTests/BasicTests/V2SearchProtocolTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -167,10 +168,137 @@ public async Task ResultsHonorPreReleaseField() Assert.False(hasPrereleaseVersions, $"The search query returned results with prerelease versions when queried for Prerelease = false"); } + [Theory] + [InlineData(false, false, Constants.TestPackageVersion_SearchFilters_Default)] + [InlineData(true, false, Constants.TestPackageVersion_SearchFilters_Prerel)] + [InlineData(false, true, Constants.TestPackageVersion_SearchFilters_SemVer2)] + [InlineData(true, true, Constants.TestPackageVersion_SearchFilters_PrerelSemVer2)] + public async Task LatestVersionChangesWithRespectToSearchFilters(bool prerelease, bool includeSemVer2, string version) + { + var searchBuilder = new V2SearchBuilder + { + Query = $"packageid:{Constants.TestPackageId_SearchFilters}", + Prerelease = prerelease, + IncludeSemVer2 = includeSemVer2, + }; + + var results = await V2SearchAsync(searchBuilder); + + var package = Assert.Single(results.Data); + Assert.Equal(Constants.TestPackageId_SearchFilters, package.PackageRegistration.Id); + Assert.Equal(version, package.Version); + } + + public static IEnumerable IgnoreFilterTrueData => new[] + { + new + { + Prerelease = false, + IncludeSemVer2 = false, + ExpectedVersions = new[] // prerelease is always returned with ignoreFilter=true + { + Constants.TestPackageVersion_SearchFilters_Default, + Constants.TestPackageVersion_SearchFilters_Prerel, + }, + }, + new + { + Prerelease = true, + IncludeSemVer2 = false, + ExpectedVersions = new[] + { + Constants.TestPackageVersion_SearchFilters_Default, + Constants.TestPackageVersion_SearchFilters_Prerel, + }, + }, + new + { + Prerelease = false, + IncludeSemVer2 = true, + ExpectedVersions = new[] // prerelease is always returned with ignoreFilter=true + { + Constants.TestPackageVersion_SearchFilters_Default, + Constants.TestPackageVersion_SearchFilters_Prerel, + Constants.TestPackageVersion_SearchFilters_SemVer2, + Constants.TestPackageVersion_SearchFilters_PrerelSemVer2, + }, + }, + new + { + Prerelease = true, + IncludeSemVer2 = true, + ExpectedVersions = new[] + { + Constants.TestPackageVersion_SearchFilters_Default, + Constants.TestPackageVersion_SearchFilters_Prerel, + Constants.TestPackageVersion_SearchFilters_SemVer2, + Constants.TestPackageVersion_SearchFilters_PrerelSemVer2, + }, + }, + }.Select(x => new object[] { x.Prerelease, x.IncludeSemVer2, x.ExpectedVersions }); + + [Theory] + [MemberData(nameof(IgnoreFilterTrueData))] + public async Task IgnoreFilterTrueAlwaysIncludesPrerelease(bool prerelease, bool includeSemVer2, string[] expectedVersions) + { + var searchBuilder = new V2SearchBuilder + { + Query = $"packageid:{Constants.TestPackageId_SearchFilters}", + Prerelease = prerelease, + IncludeSemVer2 = includeSemVer2, + IgnoreFilter = true, + }; + + var results = await V2SearchAsync(searchBuilder); + + Assert.Equal( + expectedVersions, + results.Data.Select(x => x.Version).OrderBy(x => x).ToArray()); + } + + [Theory] + [InlineData(false, false, false)] + [InlineData(true, false, false)] + [InlineData(false, true, true)] + [InlineData(true, true, true)] + public async Task IgnoreFilterTrueWithSpecificIdVersionAlwaysIncludesPrerelease(bool prerelease, bool includeSemVer2, bool returned) + { + var searchBuilder = new V2SearchBuilder + { + Query = $"packageid:{Constants.TestPackageId_SearchFilters} version:{Constants.TestPackageVersion_SearchFilters_PrerelSemVer2}", + Prerelease = prerelease, + IncludeSemVer2 = includeSemVer2, + IgnoreFilter = true, + }; + + var results = await V2SearchAsync(searchBuilder); + + if (returned) + { + var package = Assert.Single(results.Data); + Assert.Equal(Constants.TestPackageId_SearchFilters, package.PackageRegistration.Id); + Assert.Equal(Constants.TestPackageVersion_SearchFilters_PrerelSemVer2, package.Version); + } + else + { + Assert.Empty(results.Data); + } + } + + [Fact] + public async Task SemVer2IsHiddenByDefault() + { + var searchTerm = "packageId:" + Constants.TestPackageId_SemVer2; + var results = await V2SearchAsync(new V2SearchBuilder { Query = searchTerm }); + + Assert.NotNull(results); + Assert.Empty(results.Data); + } + [Fact] - public async Task ResultsHonorSemverLevel() + public async Task SemVerLevel2AllowsSemVer2Packages() { - var searchTerm = "packageId:" + Constants.TestPackageIdSemVer2; + var searchTerm = "packageId:" + Constants.TestPackageId_SemVer2; var results = await V2SearchAsync(new V2SearchBuilder { Query = searchTerm, IncludeSemVer2 = true }); Assert.NotNull(results); @@ -323,6 +451,6 @@ public static IEnumerable GetSortByData yield return new object[] { "title-asc", (Func)((V2SearchResultEntry data) => { return data.Title; }), true }; yield return new object[] { "title-desc", (Func)((V2SearchResultEntry data) => { return data.Title; }) }; } - } + } } } diff --git a/tests/NuGet.Services.AzureSearch.Tests/SearchService/AzureSearchServiceFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/SearchService/AzureSearchServiceFacts.cs index e0f086a51..60acb9326 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/SearchService/AzureSearchServiceFacts.cs +++ b/tests/NuGet.Services.AzureSearch.Tests/SearchService/AzureSearchServiceFacts.cs @@ -7,6 +7,7 @@ using Microsoft.Azure.Search.Models; using Moq; using NuGet.Services.AzureSearch.Wrappers; +using NuGetGallery; using Xunit; namespace NuGet.Services.AzureSearch.SearchService @@ -37,6 +38,29 @@ public async Task SearchIndexAndSearchOperation() Times.Once); } + [Fact] + public async Task SearchIndexAndGetOperation() + { + _v2Request.IgnoreFilter = false; + _operation = IndexOperation.Get(_key); + + var response = await _target.V2SearchAsync(_v2Request); + + Assert.Same(_v2Response, response); + _operationBuilder.Verify( + x => x.V2SearchWithSearchIndex(_v2Request), + Times.Once); + _searchOperations.Verify( + x => x.GetOrNullAsync(_key), + Times.Once); + _responseBuilder.Verify( + x => x.V2FromSearchDocument(_v2Request, _key, _searchDocument, It.Is(t => t > TimeSpan.Zero)), + Times.Once); + _telemetryService.Verify( + x => x.TrackV2GetDocumentWithSearchIndex(It.Is(t => t > TimeSpan.Zero)), + Times.Once); + } + [Fact] public async Task HijackIndexAndSearchOperation() { @@ -58,6 +82,38 @@ public async Task HijackIndexAndSearchOperation() x => x.TrackV2SearchQueryWithHijackIndex(It.Is(t => t > TimeSpan.Zero)), Times.Once); } + + [Theory] + [InlineData(false, false, false)] + [InlineData(true, false, false)] + [InlineData(false, true, true)] + [InlineData(true, true, true)] + public async Task HijackIndexAndGetOperation(bool includePrerelease, bool includeSemVer2, bool returned) + { + _v2Request.IgnoreFilter = true; + _v2Request.IncludePrerelease = includePrerelease; + _v2Request.IncludeSemVer2 = includeSemVer2; + _hijackDocument.Prerelease = true; + _hijackDocument.SemVerLevel = SemVerLevelKey.SemVer2; + _operation = IndexOperation.Get(_key); + var expectedDocument = returned ? _hijackDocument : null; + + var response = await _target.V2SearchAsync(_v2Request); + + Assert.Same(_v2Response, response); + _operationBuilder.Verify( + x => x.V2SearchWithHijackIndex(_v2Request), + Times.Once); + _hijackOperations.Verify( + x => x.GetOrNullAsync(_key), + Times.Once); + _responseBuilder.Verify( + x => x.V2FromHijackDocument(_v2Request, _key, expectedDocument, It.Is(t => t > TimeSpan.Zero)), + Times.Once); + _telemetryService.Verify( + x => x.TrackV2GetDocumentWithHijackIndex(It.Is(t => t > TimeSpan.Zero)), + Times.Once); + } } public class V3SearchAsync : BaseFacts @@ -235,6 +291,13 @@ public BaseFacts() It.IsAny>(), It.IsAny())) .Returns(() => _v2Response); + _responseBuilder + .Setup(x => x.V2FromHijackDocument( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(() => _v2Response); _responseBuilder .Setup(x => x.V2FromSearch( It.IsAny(), @@ -243,6 +306,13 @@ public BaseFacts() It.IsAny>(), It.IsAny())) .Returns(() => _v2Response); + _responseBuilder + .Setup(x => x.V2FromSearchDocument( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(() => _v2Response); _responseBuilder .Setup(x => x.V3FromSearch( It.IsAny(), diff --git a/tests/NuGet.Services.AzureSearch.Tests/SearchService/IndexOperationBuilderFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/SearchService/IndexOperationBuilderFacts.cs index 9c715ab7c..0464beb96 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/SearchService/IndexOperationBuilderFacts.cs +++ b/tests/NuGet.Services.AzureSearch.Tests/SearchService/IndexOperationBuilderFacts.cs @@ -51,23 +51,37 @@ public void CallsDependenciesForSearchOperation() Build(); TextBuilder.Verify(x => x.ParseV3Search(V3SearchRequest), Times.Once); + TextBuilder.Verify(x => x.Build(ParsedQuery), Times.Once); ParametersBuilder.Verify(x => x.V3Search(V3SearchRequest), Times.Once); } } - public class V2SearchWithSearchIndex : Facts + public class V2SearchWithSearchIndex : SearchIndexFacts { - public IndexOperation Build() + public override IndexOperation Build() { return Target.V2SearchWithSearchIndex(V2SearchRequest); } + [Fact] + public void CallsDependenciesForGetOperation() + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + + Build(); + + TextBuilder.Verify(x => x.ParseV2Search(V2SearchRequest), Times.Once); + TextBuilder.Verify(x => x.Build(It.IsAny()), Times.Never); + ParametersBuilder.Verify(x => x.V2Search(It.IsAny()), Times.Never); + } + [Fact] public void CallsDependenciesForSearchOperation() { Build(); TextBuilder.Verify(x => x.ParseV2Search(V2SearchRequest), Times.Once); + TextBuilder.Verify(x => x.Build(ParsedQuery), Times.Once); ParametersBuilder.Verify(x => x.V2Search(V2SearchRequest), Times.Once); } } @@ -79,12 +93,203 @@ public IndexOperation Build() return Target.V2SearchWithHijackIndex(V2SearchRequest); } + [Theory] + [MemberData(nameof(ValidIdData))] + public void BuildsGetOperationForSingleValidPackageIdAndSingleValidVersion(string id) + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Get, actual.Type); + Assert.Equal( + DocumentUtilities.GetHijackDocumentKey(id, Version), + actual.DocumentKey); + } + + [Theory] + [MemberData(nameof(InvalidIdData))] + public void DoesNotBuildGetOperationForSingleInvalidPackageIdAndSingleValidVersion(string id) + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Theory] + [InlineData("\"1.0.0\"")] + [InlineData("1.0.0.0.0")] + [InlineData("1.0.0.a")] + [InlineData("1.0.0.-alpha")] + [InlineData("1.0.0-beta.01")] + [InlineData("alpha")] + [InlineData("")] + public void DoesNotBuildGetOperationForSingleValidPackageIdAndSingleInvalidVersion(string version) + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Theory] + [InlineData("1.0.0", "1.0.0")] + [InlineData("1.0.0-BETA", "1.0.0-BETA")] + [InlineData("1.0.0-beta01", "1.0.0-beta01")] + [InlineData("1.0.0-beta.2", "1.0.0-beta.2")] + [InlineData("1.0.0.0", "1.0.0")] + [InlineData("1.0.0-ALPHA+git", "1.0.0-ALPHA")] + [InlineData("1.0.0-alpha+git", "1.0.0-alpha")] + [InlineData("1.0.00-alpha", "1.0.0-alpha")] + [InlineData("1.0.01-alpha", "1.0.1-alpha")] + [InlineData(" 1.0.0 ", "1.0.0")] + public void NormalizesVersion(string version, string normalized) + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Get, actual.Type); + Assert.Equal( + DocumentUtilities.GetHijackDocumentKey(Id, normalized), + actual.DocumentKey); + } + + [Fact] + public void IgnoresFiltersWithSpecificPackageIdAndVersion() + { + V2SearchRequest.IncludePrerelease = false; + V2SearchRequest.IncludeSemVer2 = false; + var prereleaseSemVer2 = "1.0.0-beta.1"; + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { prereleaseSemVer2 }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Get, actual.Type); + Assert.Equal( + DocumentUtilities.GetHijackDocumentKey(Id, prereleaseSemVer2), + actual.DocumentKey); + } + + [Fact] + public void DoesNotBuildGetOperationForNonPackageIdAndVersion() + { + ParsedQuery.Grouping[QueryField.Id] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Fact] + public void DoesNotBuildGetOperationForMultiplePackageIds() + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id, "A" }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Fact] + public void DoesNotBuildGetOperationForMultipleVersions() + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version, "9.9.9" }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Fact] + public void DoesNotBuildGetOperationForPackageIdVersionAndExtra() + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + ParsedQuery.Grouping[QueryField.Description] = new HashSet(new[] { "hi" }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Fact] + public void DoesNotBuildGetOperationForEmptyPackageId() + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Fact] + public void DoesNotBuildGetOperationForEmptyVersion() + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Fact] + public void DoesNotBuildGetOperationForSkippingFirstItem() + { + V2SearchRequest.Skip = 1; + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Fact] + public void DoesNotBuildGetOperationForNoTake() + { + V2SearchRequest.Take = 0; + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + + var actual = Build(); + + Assert.Equal(IndexOperationType.Search, actual.Type); + } + + [Fact] + public void CallsDependenciesForGetOperation() + { + ParsedQuery.Grouping[QueryField.PackageId] = new HashSet(new[] { Id }); + ParsedQuery.Grouping[QueryField.Version] = new HashSet(new[] { Version }); + + Build(); + + TextBuilder.Verify(x => x.ParseV2Search(V2SearchRequest), Times.Once); + TextBuilder.Verify(x => x.Build(It.IsAny()), Times.Never); + ParametersBuilder.Verify(x => x.V2Search(It.IsAny()), Times.Never); + } + [Fact] public void CallsDependenciesForSearchOperation() { Build(); TextBuilder.Verify(x => x.ParseV2Search(V2SearchRequest), Times.Once); + TextBuilder.Verify(x => x.Build(ParsedQuery), Times.Once); ParametersBuilder.Verify(x => x.V2Search(V2SearchRequest), Times.Once); } } diff --git a/tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchTextBuilderFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchTextBuilderFacts.cs index ef4138a6d..e4b7aa0c9 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchTextBuilderFacts.cs +++ b/tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchTextBuilderFacts.cs @@ -11,11 +11,6 @@ public class SearchTextBuilderFacts { public class V2Search : FactsBase { - /// - /// 100 characters is the maximum length for package IDs. - /// - private const string MaxId = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; - [Theory] [MemberData(nameof(CommonAzureSearchQueryData))] public void GeneratesAzureSearchQuery(string input, string expected)