Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

Commit

Permalink
Move IVersionListDataClient down to CatalogIndexActionBuilder (#503)
Browse files Browse the repository at this point in the history 
Progress on NuGet/NuGetGallery#6475
  • Loading branch information
@joelverhagen
joelverhagen committed May 1, 2019
1 parent 909391c commit e22af44
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 40 deletions.
6 changes: 0 additions & 6 deletions src/NuGet.Services.AzureSearch/Catalog2AzureSearch/AzureSearchCollectorLogic.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,19 @@ namespace NuGet.Services.AzureSearch.Catalog2AzureSearch
public class AzureSearchCollectorLogic : ICommitCollectorLogic
{
private readonly ICatalogClient _catalogClient;
private readonly IVersionListDataClient _versionListDataClient;
private readonly ICatalogIndexActionBuilder _indexActionBuilder;
private readonly Func<IBatchPusher> _batchPusherFactory;
private readonly IOptionsSnapshot<Catalog2AzureSearchConfiguration> _options;
private readonly ILogger<AzureSearchCollectorLogic> _logger;

public AzureSearchCollectorLogic(
ICatalogClient catalogClient,
IVersionListDataClient versionListDataClient,
ICatalogIndexActionBuilder indexActionBuilder,
Func<IBatchPusher> batchPusherFactory,
IOptionsSnapshot<Catalog2AzureSearchConfiguration> options,
ILogger<AzureSearchCollectorLogic> logger)
{
_catalogClient = catalogClient ?? throw new ArgumentNullException(nameof(catalogClient));
_versionListDataClient = versionListDataClient ?? throw new ArgumentNullException(nameof(versionListDataClient));
_indexActionBuilder = indexActionBuilder ?? throw new ArgumentNullException(nameof(indexActionBuilder));
_batchPusherFactory = batchPusherFactory ?? throw new ArgumentNullException(nameof(batchPusherFactory));
_options = options ?? throw new ArgumentNullException(nameof(options));
Expand Down Expand Up @@ -208,11 +205,8 @@ private async Task<IndexActions> GetPackageIdIndexActionsAsync(
.Distinct(StringComparer.OrdinalIgnoreCase)
.Single();

var versionListDataResult = await _versionListDataClient.ReadAsync(packageId);

return await _indexActionBuilder.AddCatalogEntriesAsync(
packageId,
versionListDataResult,
entries,
entryToLeaf);
}
Expand Down
7 changes: 6 additions & 1 deletion src/NuGet.Services.AzureSearch/Catalog2AzureSearch/CatalogIndexActionBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,21 @@ namespace NuGet.Services.AzureSearch.Catalog2AzureSearch
public class CatalogIndexActionBuilder : ICatalogIndexActionBuilder
{
private static readonly int SearchFiltersCount = Enum.GetValues(typeof(SearchFilters)).Length;

private readonly IVersionListDataClient _versionListDataClient;
private readonly ICatalogLeafFetcher _leafFetcher;
private readonly ISearchDocumentBuilder _search;
private readonly IHijackDocumentBuilder _hijack;
private readonly ILogger<CatalogIndexActionBuilder> _logger;

public CatalogIndexActionBuilder(
IVersionListDataClient versionListDataClient,
ICatalogLeafFetcher leafFetcher,
ISearchDocumentBuilder search,
IHijackDocumentBuilder hijack,
ILogger<CatalogIndexActionBuilder> logger)
{
_versionListDataClient = versionListDataClient ?? throw new ArgumentNullException(nameof(versionListDataClient));
_leafFetcher = leafFetcher ?? throw new ArgumentNullException(nameof(leafFetcher));
_search = search ?? throw new ArgumentNullException(nameof(search));
_hijack = hijack ?? throw new ArgumentNullException(nameof(hijack));
Expand All @@ -36,7 +40,6 @@ public CatalogIndexActionBuilder(

public async Task<IndexActions> AddCatalogEntriesAsync(
string packageId,
ResultAndAccessCondition<VersionListData> versionListDataResult,
IReadOnlyList<CatalogCommitItem> latestEntries,
IReadOnlyDictionary<CatalogCommitItem, PackageDetailsCatalogLeaf> entryToLeaf)
{
Expand All @@ -45,6 +48,8 @@ public async Task<IndexActions> AddCatalogEntriesAsync(
throw new ArgumentException("There must be at least one catalog item to process.", nameof(latestEntries));
}

var versionListDataResult = await _versionListDataClient.ReadAsync(packageId);

var context = new Context(
packageId,
versionListDataResult,
Expand Down
1 change: 0 additions & 1 deletion src/NuGet.Services.AzureSearch/Catalog2AzureSearch/ICatalogIndexActionBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ public interface ICatalogIndexActionBuilder
{
Task<IndexActions> AddCatalogEntriesAsync(
string packageId,
ResultAndAccessCondition<VersionListData> versionListDataResult,
IReadOnlyList<CatalogCommitItem> latestEntries,
IReadOnlyDictionary<CatalogCommitItem, PackageDetailsCatalogLeaf> entryToLeaf);
}
Expand Down
15 changes: 0 additions & 15 deletions tests/NuGet.Services.AzureSearch.Tests/Catalog2AzureSearch/AzureSearchCollectorLogicFacts.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,9 @@ public async Task DoesNotFetchLeavesForDeleteEntries()
_catalogClient.Verify(x => x.GetPackageDetailsLeafAsync(It.IsAny<string>()), Times.Exactly(1));
_catalogClient.Verify(x => x.GetPackageDeleteLeafAsync(It.IsAny<string>()), Times.Never);

_versionListDataClient.Verify(x => x.ReadAsync("NuGet.Versioning"), Times.Once);
_versionListDataClient.Verify(x => x.ReadAsync("NuGet.Frameworks"), Times.Once);
_versionListDataClient.Verify(x => x.ReadAsync(It.IsAny<string>()), Times.Exactly(2));

_catalogIndexActionBuilder.Verify(
x => x.AddCatalogEntriesAsync(
"NuGet.Versioning",
It.IsAny<ResultAndAccessCondition<VersionListData>>(),
It.Is<IReadOnlyList<CatalogCommitItem>>(
y => y.Count == 1),
It.Is<IReadOnlyDictionary<CatalogCommitItem, PackageDetailsCatalogLeaf>>(
Expand All @@ -114,7 +109,6 @@ public async Task DoesNotFetchLeavesForDeleteEntries()
_catalogIndexActionBuilder.Verify(
x => x.AddCatalogEntriesAsync(
"NuGet.Frameworks",
It.IsAny<ResultAndAccessCondition<VersionListData>>(),
It.Is<IReadOnlyList<CatalogCommitItem>>(
y => y.Count == 1),
It.Is<IReadOnlyDictionary<CatalogCommitItem, PackageDetailsCatalogLeaf>>(
Expand Down Expand Up @@ -176,14 +170,9 @@ public async Task OperatesOnLatestPerPackageIdentityAndGroupsById()
_catalogClient.Verify(x => x.GetPackageDetailsLeafAsync("https://example/3"), Times.Once);
_catalogClient.Verify(x => x.GetPackageDetailsLeafAsync(It.IsAny<string>()), Times.Exactly(3));

_versionListDataClient.Verify(x => x.ReadAsync("NuGet.Versioning"), Times.Once);
_versionListDataClient.Verify(x => x.ReadAsync("NuGet.Frameworks"), Times.Once);
_versionListDataClient.Verify(x => x.ReadAsync(It.IsAny<string>()), Times.Exactly(2));

_catalogIndexActionBuilder.Verify(
x => x.AddCatalogEntriesAsync(
"NuGet.Versioning",
It.IsAny<ResultAndAccessCondition<VersionListData>>(),
It.Is<IReadOnlyList<CatalogCommitItem>>(
y => y.Count == 2),
It.Is<IReadOnlyDictionary<CatalogCommitItem, PackageDetailsCatalogLeaf>>(
Expand All @@ -192,7 +181,6 @@ public async Task OperatesOnLatestPerPackageIdentityAndGroupsById()
_catalogIndexActionBuilder.Verify(
x => x.AddCatalogEntriesAsync(
"NuGet.Frameworks",
It.IsAny<ResultAndAccessCondition<VersionListData>>(),
It.Is<IReadOnlyList<CatalogCommitItem>>(
y => y.Count == 1),
It.Is<IReadOnlyDictionary<CatalogCommitItem, PackageDetailsCatalogLeaf>>(
Expand Down Expand Up @@ -248,7 +236,6 @@ public async Task RejectsMultipleLeavesForTheSamePackageAtTheSameTime()
public abstract class BaseFacts
{
protected readonly Mock<ICatalogClient> _catalogClient;
protected readonly Mock<IVersionListDataClient> _versionListDataClient;
protected readonly Mock<ICatalogIndexActionBuilder> _catalogIndexActionBuilder;
protected readonly Mock<IBatchPusher> _batchPusher;
protected readonly Mock<IOptionsSnapshot<Catalog2AzureSearchConfiguration>> _options;
Expand All @@ -259,7 +246,6 @@ public abstract class BaseFacts
public BaseFacts(ITestOutputHelper output)
{
_catalogClient = new Mock<ICatalogClient>();
_versionListDataClient = new Mock<IVersionListDataClient>();
_catalogIndexActionBuilder = new Mock<ICatalogIndexActionBuilder>();
_batchPusher = new Mock<IBatchPusher>();
_options = new Mock<IOptionsSnapshot<Catalog2AzureSearchConfiguration>>();
Expand All @@ -271,7 +257,6 @@ public BaseFacts(ITestOutputHelper output)

_target = new AzureSearchCollectorLogic(
_catalogClient.Object,
_versionListDataClient.Object,
_catalogIndexActionBuilder.Object,
() => _batchPusher.Object,
_options.Object,
Expand Down
18 changes: 7 additions & 11 deletions tests/NuGet.Services.AzureSearch.Tests/Catalog2AzureSearch/CatalogIndexActionBuilderFacts.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public async Task ThrowsWithNoEntries()

var ex = await Assert.ThrowsAsync<ArgumentException>(() => _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf));
Assert.Contains("There must be at least one catalog item to process.", ex.Message);
Expand All @@ -46,7 +45,6 @@ public async Task AddFirstVersion()
{
var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -80,7 +78,6 @@ public async Task AddNewLatestVersion()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -120,7 +117,6 @@ public async Task AddNewNonLatestVersion()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -177,7 +173,6 @@ public async Task Downgrade()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -253,7 +248,6 @@ public async Task DowngradeToDifferent()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -321,7 +315,6 @@ public async Task DowngradeToUnlist()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -419,7 +412,6 @@ public async Task DowngradeUnlistsOtherSearchFilterLatest()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -478,7 +470,6 @@ public async Task DowngradeToDelete()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -522,7 +513,6 @@ public async Task DetectsSemVer2()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand Down Expand Up @@ -553,7 +543,6 @@ public async Task DetectsUnlisted()

var indexActions = await _target.AddCatalogEntriesAsync(
_packageId,
_versionListDataResult,
_latestEntries,
_entryToLeaf);

Expand All @@ -577,6 +566,7 @@ public async Task DetectsUnlisted()

public abstract class BaseFacts
{
protected readonly Mock<IVersionListDataClient> _versionListDataClient;
protected readonly Mock<ICatalogLeafFetcher> _fetcher;
protected readonly Mock<ISearchDocumentBuilder> _search;
protected readonly Mock<IHijackDocumentBuilder> _hijack;
Expand All @@ -593,6 +583,7 @@ public abstract class BaseFacts

public BaseFacts(ITestOutputHelper output)
{
_versionListDataClient = new Mock<IVersionListDataClient>();
_fetcher = new Mock<ICatalogLeafFetcher>();
_search = new Mock<ISearchDocumentBuilder>();
_hijack = new Mock<IHijackDocumentBuilder>();
Expand Down Expand Up @@ -621,6 +612,10 @@ public BaseFacts(ITestOutputHelper output)
new HashSet<NuGetVersion>(),
new Dictionary<NuGetVersion, PackageDetailsCatalogLeaf>());

_versionListDataClient
.Setup(x => x.ReadAsync(It.IsAny<string>()))
.ReturnsAsync(() => _versionListDataResult);

_search
.Setup(x => x.LatestFlagsOrNull(It.IsAny<VersionLists>(), It.IsAny<SearchFilters>()))
.Returns<VersionLists, SearchFilters>((vl, sf) => new SearchDocument.LatestFlags(
Expand Down Expand Up @@ -677,6 +672,7 @@ public BaseFacts(ITestOutputHelper output)
.ReturnsAsync(() => _latestCatalogLeaves);

_target = new CatalogIndexActionBuilder(
_versionListDataClient.Object,
_fetcher.Object,
_search.Object,
_hijack.Object,
Expand Down
12 changes: 6 additions & 6 deletions ...Search.Tests/Catalog2AzureSearch/Integration/AzureSearchCollectorLogicIntegrationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ public AzureSearchCollectorLogicIntegrationTests(ITestOutputHelper output)
_options = new Mock<IOptionsSnapshot<Catalog2AzureSearchConfiguration>>();
_options.Setup(x => x.Value).Returns(() => _config);

_cloudBlobClient = new InMemoryCloudBlobClient();
_versionListDataClient = new VersionListDataClient(
_cloudBlobClient,
_options.Object,
output.GetLogger<VersionListDataClient>());
_registrationClient = new InMemoryRegistrationClient();
_catalogClient = new InMemoryCatalogClient();
_fetcher = new CatalogLeafFetcher(
Expand All @@ -61,15 +66,11 @@ public AzureSearchCollectorLogicIntegrationTests(ITestOutputHelper output)
_search = new SearchDocumentBuilder();
_hijack = new HijackDocumentBuilder();
_builder = new CatalogIndexActionBuilder(
_versionListDataClient,
_fetcher,
_search,
_hijack,
output.GetLogger<CatalogIndexActionBuilder>());
_cloudBlobClient = new InMemoryCloudBlobClient();
_versionListDataClient = new VersionListDataClient(
_cloudBlobClient,
_options.Object,
output.GetLogger<VersionListDataClient>());

_searchIndex = new Mock<ISearchIndexClientWrapper>();
_searchDocuments = new InMemoryDocumentsOperations();
Expand All @@ -80,7 +81,6 @@ public AzureSearchCollectorLogicIntegrationTests(ITestOutputHelper output)

_collector = new AzureSearchCollectorLogic(
_catalogClient,
_versionListDataClient,
_builder,
() => new BatchPusher(
_searchIndex.Object,
Expand Down

0 comments on commit e22af44

Please sign in to comment.