This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 21
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add OwnerIndexActionBuilder to build to proper set search index chang…
…es per ID (#504) Progress on NuGet/NuGetGallery#6475
- Loading branch information
1 parent
e22af44
commit 28edbb3
Showing
6 changed files
with
265 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 9 additions & 0 deletions
9
src/NuGet.Services.AzureSearch/Owners2AzureSearch/IOwnerIndexActionBuilder.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
using System.Threading.Tasks; | ||
|
||
namespace NuGet.Services.AzureSearch.Owners2AzureSearch | ||
{ | ||
public interface IOwnerIndexActionBuilder | ||
{ | ||
Task<IndexActions> UpdateOwnersAsync(string packageId, string[] owners); | ||
} | ||
} |
98 changes: 98 additions & 0 deletions
98
src/NuGet.Services.AzureSearch/Owners2AzureSearch/OwnerIndexActionBuilder.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
// 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; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Threading.Tasks; | ||
using Microsoft.Azure.Search.Models; | ||
using Microsoft.Extensions.Logging; | ||
|
||
namespace NuGet.Services.AzureSearch.Owners2AzureSearch | ||
{ | ||
public class OwnerIndexActionBuilder : IOwnerIndexActionBuilder | ||
{ | ||
private static readonly IReadOnlyList<SearchFilters> AllSearchFilters = Enum | ||
.GetValues(typeof(SearchFilters)) | ||
.Cast<SearchFilters>() | ||
.ToList(); | ||
|
||
private readonly IVersionListDataClient _versionListDataClient; | ||
private readonly ISearchDocumentBuilder _search; | ||
private readonly ILogger<OwnerIndexActionBuilder> _logger; | ||
|
||
public OwnerIndexActionBuilder( | ||
IVersionListDataClient versionListDataClient, | ||
ISearchDocumentBuilder search, | ||
ILogger<OwnerIndexActionBuilder> logger) | ||
{ | ||
_versionListDataClient = versionListDataClient ?? throw new ArgumentNullException(nameof(versionListDataClient)); | ||
_search = search ?? throw new ArgumentNullException(nameof(search)); | ||
_logger = logger ?? throw new ArgumentNullException(nameof(logger)); | ||
} | ||
|
||
public async Task<IndexActions> UpdateOwnersAsync(string packageId, string[] owners) | ||
{ | ||
var versionListDataResult = await _versionListDataClient.ReadAsync(packageId); | ||
var versionLists = new VersionLists(versionListDataResult.Result); | ||
|
||
/// Update all of the search documents that exist for this package ID with the provided owners. Note that | ||
/// this owner list can be empty (e.g. if the last owner was deleted). Here are some examples of different | ||
/// search filter combinations that could occur. | ||
/// | ||
/// Example #1: 1.0.0 (listed) | ||
/// | ||
/// A stable SemVer 1.0.0 package matches all search filters, so one index action will be produced for | ||
/// each search document. That is four in total. All owner fields will be set to the same thing. All of | ||
/// these search documents have the same latest version: 1.0.0. | ||
/// | ||
/// Example #2: 1.0.0 (unlisted), 2.0.0 (unlisted) | ||
/// | ||
/// There are no search documents at all in this case since there is no listed version. No index actions | ||
/// are produced in this case. | ||
/// | ||
/// Example #3: 1.0.0-beta (listed), 2.0.0-beta.1 (listed) | ||
/// | ||
/// All of the versions are prerelease so there are no search documents for "stable" search filters. There | ||
/// two search documents to be updated, one for <see cref="SearchFilters.IncludePrerelease"/> and one for | ||
/// <see cref="SearchFilters.IncludePrereleaseAndSemVer2"/>. The latest version for each of these two | ||
/// documents is different, but both have the same update with regards to the owners field. | ||
var search = new List<IndexAction<KeyedDocument>>(); | ||
var searchFilters = new List<SearchFilters>(); | ||
foreach (var searchFilter in AllSearchFilters) | ||
{ | ||
// Determine if there is a document for this ID and search filter. | ||
if (versionLists.GetLatestVersionInfoOrNull(searchFilter) == null) | ||
{ | ||
continue; | ||
} | ||
|
||
var document = _search.UpdateOwners(packageId, searchFilter, owners); | ||
var indexAction = IndexAction.Merge<KeyedDocument>(document); | ||
search.Add(indexAction); | ||
searchFilters.Add(searchFilter); | ||
} | ||
|
||
_logger.LogInformation( | ||
"Package ID {PackageId} has {Count} search document changes for search filters: {SearchFilters}", | ||
packageId, | ||
searchFilters.Count, | ||
searchFilters); | ||
|
||
// No changes are made to the hijack index. | ||
var hijack = new List<IndexAction<KeyedDocument>>(); | ||
|
||
// We never make any change to the version list but still want to push it back to storage. This will give | ||
// us an etag mismatch if the version list has changed. This is good because if the version list has | ||
// changed it's possible there is another search document that we have to update. If we didn't do this, | ||
// then a race condition could occur where one of the search documents for an ID would have the old owners | ||
// until the ownership changes again. | ||
var newVersionListDataResult = versionListDataResult; | ||
|
||
return new IndexActions( | ||
search, | ||
hijack, | ||
newVersionListDataResult); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
155 changes: 155 additions & 0 deletions
155
tests/NuGet.Services.AzureSearch.Tests/Owners2AzureSearch/OwnerIndexActionBuilderFacts.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
// 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.Collections.Generic; | ||
using System.Threading.Tasks; | ||
using Microsoft.Azure.Search.Models; | ||
using Moq; | ||
using NuGet.Services.AzureSearch.Support; | ||
using NuGetGallery; | ||
using Xunit; | ||
using Xunit.Abstractions; | ||
|
||
namespace NuGet.Services.AzureSearch.Owners2AzureSearch | ||
{ | ||
public class OwnerIndexActionBuilderFacts | ||
{ | ||
public class UpdateOwnersAsync : Facts | ||
{ | ||
public UpdateOwnersAsync(ITestOutputHelper output) : base(output) | ||
{ | ||
} | ||
|
||
[Fact] | ||
public async Task UpdatesSearchDocumentsWithVersionMatchingAllFilters() | ||
{ | ||
VersionListDataResult = new ResultAndAccessCondition<VersionListData>( | ||
new VersionListData(new Dictionary<string, VersionPropertiesData> | ||
{ | ||
{ | ||
"1.0.0", | ||
new VersionPropertiesData(listed: true, semVer2: false) | ||
}, | ||
}), | ||
AccessConditionWrapper.GenerateIfNotExistsCondition()); | ||
|
||
var indexActions = await Target.UpdateOwnersAsync( | ||
Data.PackageId, | ||
Data.Owners); | ||
|
||
Assert.Same(VersionListDataResult, indexActions.VersionListDataResult); | ||
Assert.Empty(indexActions.Hijack); | ||
|
||
Assert.Equal(4, indexActions.Search.Count); | ||
Assert.All(indexActions.Search, x => Assert.IsType<SearchDocument.UpdateOwners>(x.Document)); | ||
Assert.All(indexActions.Search, x => Assert.Equal(IndexActionType.Merge, x.ActionType)); | ||
|
||
Assert.Single(indexActions.Search, x => x.Document.Key == SearchFilters.Default.ToString()); | ||
Assert.Single(indexActions.Search, x => x.Document.Key == SearchFilters.IncludePrerelease.ToString()); | ||
Assert.Single(indexActions.Search, x => x.Document.Key == SearchFilters.IncludeSemVer2.ToString()); | ||
Assert.Single(indexActions.Search, x => x.Document.Key == SearchFilters.IncludePrereleaseAndSemVer2.ToString()); | ||
} | ||
|
||
[Fact] | ||
public async Task UpdatesSearchDocumentsWithVersionMatchingSomeFilters() | ||
{ | ||
VersionListDataResult = new ResultAndAccessCondition<VersionListData>( | ||
new VersionListData(new Dictionary<string, VersionPropertiesData> | ||
{ | ||
{ | ||
"1.0.0-beta", | ||
new VersionPropertiesData(listed: true, semVer2: false) | ||
}, | ||
}), | ||
AccessConditionWrapper.GenerateIfNotExistsCondition()); | ||
|
||
var indexActions = await Target.UpdateOwnersAsync( | ||
Data.PackageId, | ||
Data.Owners); | ||
|
||
Assert.Same(VersionListDataResult, indexActions.VersionListDataResult); | ||
Assert.Empty(indexActions.Hijack); | ||
|
||
Assert.Equal(2, indexActions.Search.Count); | ||
Assert.All(indexActions.Search, x => Assert.IsType<SearchDocument.UpdateOwners>(x.Document)); | ||
Assert.All(indexActions.Search, x => Assert.Equal(IndexActionType.Merge, x.ActionType)); | ||
|
||
Assert.Single(indexActions.Search, x => x.Document.Key == SearchFilters.IncludePrerelease.ToString()); | ||
Assert.Single(indexActions.Search, x => x.Document.Key == SearchFilters.IncludePrereleaseAndSemVer2.ToString()); | ||
} | ||
|
||
[Fact] | ||
public async Task UpdatesSearchDocumentsWithOnlyUnlistedVersions() | ||
{ | ||
VersionListDataResult = new ResultAndAccessCondition<VersionListData>( | ||
new VersionListData(new Dictionary<string, VersionPropertiesData> | ||
{ | ||
{ | ||
"1.0.0-beta", | ||
new VersionPropertiesData(listed: false, semVer2: false) | ||
}, | ||
}), | ||
AccessConditionWrapper.GenerateIfNotExistsCondition()); | ||
|
||
var indexActions = await Target.UpdateOwnersAsync( | ||
Data.PackageId, | ||
Data.Owners); | ||
|
||
Assert.Same(VersionListDataResult, indexActions.VersionListDataResult); | ||
Assert.Empty(indexActions.Hijack); | ||
Assert.Empty(indexActions.Search); | ||
} | ||
|
||
[Fact] | ||
public async Task UpdatesSearchDocumentsWithNoVersions() | ||
{ | ||
VersionListDataResult = new ResultAndAccessCondition<VersionListData>( | ||
new VersionListData(new Dictionary<string, VersionPropertiesData>()), | ||
AccessConditionWrapper.GenerateIfNotExistsCondition()); | ||
|
||
var indexActions = await Target.UpdateOwnersAsync( | ||
Data.PackageId, | ||
Data.Owners); | ||
|
||
Assert.Same(VersionListDataResult, indexActions.VersionListDataResult); | ||
Assert.Empty(indexActions.Hijack); | ||
Assert.Empty(indexActions.Search); | ||
} | ||
} | ||
|
||
public abstract class Facts | ||
{ | ||
public Facts(ITestOutputHelper output) | ||
{ | ||
VersionListDataClient = new Mock<IVersionListDataClient>(); | ||
Search = new Mock<ISearchDocumentBuilder>(); | ||
Logger = output.GetLogger<OwnerIndexActionBuilder>(); | ||
|
||
VersionListDataResult = new ResultAndAccessCondition<VersionListData>( | ||
new VersionListData(new Dictionary<string, VersionPropertiesData>()), | ||
AccessConditionWrapper.GenerateIfNotExistsCondition()); | ||
|
||
VersionListDataClient | ||
.Setup(x => x.ReadAsync(It.IsAny<string>())) | ||
.ReturnsAsync(() => VersionListDataResult); | ||
Search | ||
.Setup(x => x.UpdateOwners(It.IsAny<string>(), It.IsAny<SearchFilters>(), It.IsAny<string[]>())) | ||
.Returns<string, SearchFilters, string[]>((_, sf, __) => new SearchDocument.UpdateOwners | ||
{ | ||
Key = sf.ToString(), | ||
}); | ||
|
||
Target = new OwnerIndexActionBuilder( | ||
VersionListDataClient.Object, | ||
Search.Object, | ||
Logger); | ||
} | ||
|
||
public Mock<IVersionListDataClient> VersionListDataClient { get; } | ||
public Mock<ISearchDocumentBuilder> Search { get; } | ||
public RecordingLogger<OwnerIndexActionBuilder> Logger { get; } | ||
public ResultAndAccessCondition<VersionListData> VersionListDataResult { get; set; } | ||
public OwnerIndexActionBuilder Target { get; } | ||
} | ||
} | ||
} |
1 change: 0 additions & 1 deletion
1
tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchTextBuilderFacts.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters