From 52c75139772bcc85b564c9545b1ea2422aeecd9c Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Mon, 13 Apr 2020 12:47:37 -0700 Subject: [PATCH] Refactor download overrides Update tests Update comments Update comments Update Db2AzureSearchCommandFacts Update new package registration producer facts Add test Add tests for download transferrer Rename method Fix test Don't save popularity transfers for now Save popularity transfers data file Initial More tests Add tests Updates Clean Fix Undo change Undo change Add asserts Fix whitespace Tweak Bring test to live branch Start integration tests Update Remove old test Fix build Clean Fix whitespace --- .../UpdateDownloadsCommand.cs | 6 +- .../IPopularityTransferDataClient.cs | 2 +- .../AzureSearchScoringConfiguration.cs | 6 + .../Db2AzureSearch/Db2AzureSearchCommand.cs | 13 +- .../DownloadTransferrer.cs | 202 ++++- .../PopularityTransferIntegrationTests.cs | 412 ++++++++++ .../UpdateDownloadsCommandFacts.cs | 29 +- .../Db2AzureSearchCommandFacts.cs | 26 + .../DownloadTransferrerFacts.cs | 707 +++++++++++++++++- .../NuGet.Services.AzureSearch.Tests.csproj | 1 + .../Support/InMemoryCloudBlob.cs | 31 +- 11 files changed, 1409 insertions(+), 26 deletions(-) create mode 100644 tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/Integration/PopularityTransferIntegrationTests.cs diff --git a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateDownloadsCommand.cs b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateDownloadsCommand.cs index 586d55e3c..dbb5229a9 100644 --- a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateDownloadsCommand.cs +++ b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateDownloadsCommand.cs @@ -164,8 +164,10 @@ await ParallelAsync.Repeat( _logger.LogInformation("Uploading the new download count data to blob storage."); await _downloadDataClient.ReplaceLatestIndexedAsync(newData, oldResult.Metadata.GetIfMatchCondition()); - // TODO: Upload the new popularity transfer data to blob storage. - // See: https://github.com/NuGet/NuGetGallery/issues/7898 + _logger.LogInformation("Uploading the new popularity transfer data to blob storage."); + await _popularityTransferDataClient.ReplaceLatestIndexedAsync( + newTransfers, + oldTransfers.AccessCondition); return true; } diff --git a/src/NuGet.Services.AzureSearch/AuxiliaryFiles/IPopularityTransferDataClient.cs b/src/NuGet.Services.AzureSearch/AuxiliaryFiles/IPopularityTransferDataClient.cs index e31751f2a..209920666 100644 --- a/src/NuGet.Services.AzureSearch/AuxiliaryFiles/IPopularityTransferDataClient.cs +++ b/src/NuGet.Services.AzureSearch/AuxiliaryFiles/IPopularityTransferDataClient.cs @@ -8,7 +8,7 @@ namespace NuGet.Services.AzureSearch.AuxiliaryFiles { /// - /// The purpose of this interface is allow reading and writing populairty transfer information from storage. + /// The purpose of this interface is allow reading and writing popularity transfer information from storage. /// The Auxiliary2AzureSearch job does a comparison of latest popularity transfer data from the database with /// a snapshot of information stored in Azure Blob Storage. This interface handles the reading and writing of /// that snapshot from storage. diff --git a/src/NuGet.Services.AzureSearch/AzureSearchScoringConfiguration.cs b/src/NuGet.Services.AzureSearch/AzureSearchScoringConfiguration.cs index 30a4364f5..da6ee58f5 100644 --- a/src/NuGet.Services.AzureSearch/AzureSearchScoringConfiguration.cs +++ b/src/NuGet.Services.AzureSearch/AzureSearchScoringConfiguration.cs @@ -15,6 +15,12 @@ public class AzureSearchScoringConfiguration /// public Dictionary FieldWeights { get; set; } + /// + /// The percentage of downloads that should be transferred by the popularity transfer feature. + /// Values range from 0 to 1. + /// + public double PopularityTransfer { get; set; } + /// /// The magnitude boost. /// This boosts packages with many downloads. diff --git a/src/NuGet.Services.AzureSearch/Db2AzureSearch/Db2AzureSearchCommand.cs b/src/NuGet.Services.AzureSearch/Db2AzureSearch/Db2AzureSearchCommand.cs index 83c7603b7..294bfa226 100644 --- a/src/NuGet.Services.AzureSearch/Db2AzureSearch/Db2AzureSearchCommand.cs +++ b/src/NuGet.Services.AzureSearch/Db2AzureSearch/Db2AzureSearchCommand.cs @@ -117,8 +117,8 @@ private async Task ExecuteAsync(CancellationToken token) // Write the verified packages data file. await WriteVerifiedPackagesDataAsync(initialAuxiliaryData.VerifiedPackages); - // TODO: Write popularity transfers data file. - // See: https://github.com/NuGet/NuGetGallery/issues/7898 + // Write popularity transfers data file. + await WritePopularityTransfersDataAsync(initialAuxiliaryData.PopularityTransfers); // Write the cursor. _logger.LogInformation("Writing the initial cursor value to be {CursorValue:O}.", initialCursorValue); @@ -201,6 +201,15 @@ await _verifiedPackagesDataClient.ReplaceLatestAsync( _logger.LogInformation("Done uploading the initial verified packages data file."); } + private async Task WritePopularityTransfersDataAsync(SortedDictionary> popularityTransfers) + { + _logger.LogInformation("Writing the initial popularity transfers data file."); + await _popularityTransferDataClient.ReplaceLatestIndexedAsync( + popularityTransfers, + AccessConditionWrapper.GenerateIfNotExistsCondition()); + _logger.LogInformation("Done uploading the initial popularity transfers data file."); + } + private async Task ProduceWorkAsync( ConcurrentBag allWork, CancellationTokenSource produceWorkCts, diff --git a/src/NuGet.Services.AzureSearch/DownloadTransferrer.cs b/src/NuGet.Services.AzureSearch/DownloadTransferrer.cs index 7bda6c365..6c74562a0 100644 --- a/src/NuGet.Services.AzureSearch/DownloadTransferrer.cs +++ b/src/NuGet.Services.AzureSearch/DownloadTransferrer.cs @@ -4,17 +4,27 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using NuGet.Services.AzureSearch.Auxiliary2AzureSearch; using NuGet.Services.AzureSearch.AuxiliaryFiles; namespace NuGet.Services.AzureSearch { public class DownloadTransferrer : IDownloadTransferrer { + private readonly IDataSetComparer _dataComparer; + private readonly IOptionsSnapshot _options; private readonly ILogger _logger; - public DownloadTransferrer(ILogger logger) + public DownloadTransferrer( + IDataSetComparer dataComparer, + IOptionsSnapshot options, + ILogger logger) { + _dataComparer = dataComparer ?? throw new ArgumentNullException(nameof(dataComparer)); + _options = options ?? throw new ArgumentNullException(nameof(options)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } @@ -23,9 +33,25 @@ public SortedDictionary InitializeDownloadTransfers( SortedDictionary> outgoingTransfers, IReadOnlyDictionary downloadOverrides) { - // TODO: Add download changes due to popularity transfers. - // See: https://github.com/NuGet/NuGetGallery/issues/7898 - var downloadTransfers = new SortedDictionary(StringComparer.OrdinalIgnoreCase); + Guard.Assert( + outgoingTransfers.Comparer == StringComparer.OrdinalIgnoreCase, + $"Popularity transfer should have comparer {nameof(StringComparer.OrdinalIgnoreCase)}"); + + // Downloads are transferred from a "from" package to one or more "to" packages. + // The "outgoingTransfers" maps "from" packages to their corresponding "to" packages. + // The "incomingTransfers" maps "to" packages to their corresponding "from" packages. + var incomingTransfers = GetIncomingTransfers(outgoingTransfers); + + // Get the transfer changes for all packages that have popularity transfers. + var packageIds = new HashSet(StringComparer.OrdinalIgnoreCase); + packageIds.UnionWith(outgoingTransfers.Keys); + packageIds.UnionWith(incomingTransfers.Keys); + + var downloadTransfers = ApplyDownloadTransfers( + downloads, + outgoingTransfers, + incomingTransfers, + packageIds); // TODO: Remove download overrides. // See: https://github.com/NuGet/Engineering/issues/3089 @@ -49,13 +75,36 @@ public SortedDictionary UpdateDownloadTransfers( oldTransfers.Comparer == StringComparer.OrdinalIgnoreCase, $"Old popularity transfer should have comparer {nameof(StringComparer.OrdinalIgnoreCase)}"); + Guard.Assert( + newTransfers.Comparer == StringComparer.OrdinalIgnoreCase, + $"New popularity transfer should have comparer {nameof(StringComparer.OrdinalIgnoreCase)}"); + Guard.Assert( downloadChanges.All(x => downloads.GetDownloadCount(x.Key) == x.Value), "The download changes should match the latest downloads"); - // TODO: Add download changes due to popularity transfers. - // See: https://github.com/NuGet/NuGetGallery/issues/7898 - var downloadTransfers = new SortedDictionary(StringComparer.OrdinalIgnoreCase); + // Downloads are transferred from a "from" package to one or more "to" packages. + // The "oldTransfers" and "newTransfers" maps "from" packages to their corresponding "to" packages. + // The "incomingTransfers" maps "to" packages to their corresponding "from" packages. + var incomingTransfers = GetIncomingTransfers(newTransfers); + + _logger.LogInformation("Detecting changes in popularity transfers."); + var transferChanges = _dataComparer.ComparePopularityTransfers(oldTransfers, newTransfers); + _logger.LogInformation("{Count} popularity transfers have changed.", transferChanges.Count); + + // Get the transfer changes for packages affected by the download and transfer changes. + var affectedPackages = GetPackagesAffectedByChanges( + oldTransfers, + newTransfers, + incomingTransfers, + transferChanges, + downloadChanges); + + var downloadTransfers = ApplyDownloadTransfers( + downloads, + newTransfers, + incomingTransfers, + affectedPackages); // TODO: Remove download overrides. // See: https://github.com/NuGet/Engineering/issues/3089 @@ -64,6 +113,143 @@ public SortedDictionary UpdateDownloadTransfers( return downloadTransfers; } + private SortedDictionary ApplyDownloadTransfers( + DownloadData downloads, + SortedDictionary> outgoingTransfers, + SortedDictionary> incomingTransfers, + HashSet packageIds) + { + _logger.LogInformation( + "{Count} package IDs have download changes due to popularity transfers.", + packageIds.Count); + + var result = new SortedDictionary(StringComparer.OrdinalIgnoreCase); + foreach (var packageId in packageIds) + { + result[packageId] = TransferPackageDownloads( + packageId, + outgoingTransfers, + incomingTransfers, + downloads); + } + + return result; + } + + private SortedDictionary> GetIncomingTransfers( + SortedDictionary> outgoingTransfers) + { + var result = new SortedDictionary>(StringComparer.OrdinalIgnoreCase); + + foreach (var outgoingTransfer in outgoingTransfers) + { + var fromPackage = outgoingTransfer.Key; + + foreach (var toPackage in outgoingTransfer.Value) + { + if (!result.TryGetValue(toPackage, out var incomingTransfer)) + { + incomingTransfer = new SortedSet(StringComparer.OrdinalIgnoreCase); + result.Add(toPackage, incomingTransfer); + } + + incomingTransfer.Add(fromPackage); + } + } + + return result; + } + + private HashSet GetPackagesAffectedByChanges( + SortedDictionary> oldOutgoingTransfers, + SortedDictionary> outgoingTransfers, + SortedDictionary> incomingTransfers, + SortedDictionary transferChanges, + SortedDictionary downloadChanges) + { + var affectedPackages = new HashSet(StringComparer.OrdinalIgnoreCase); + + // If a package adds, changes, or removes outgoing transfers: + // Update "from" package + // Update all new "to" packages + // Update all old "to" packages (in case "to" packages were removed) + foreach (var transferChange in transferChanges) + { + var fromPackage = transferChange.Key; + var toPackages = transferChange.Value; + + affectedPackages.Add(fromPackage); + affectedPackages.UnionWith(toPackages); + + if (oldOutgoingTransfers.TryGetValue(fromPackage, out var oldToPackages)) + { + affectedPackages.UnionWith(oldToPackages); + } + } + + // If a package has download changes and outgoing transfers + // Update "from" package + // Update all "to" packages + // + // If a package has download changes and incoming transfers + // Update "to" package + foreach (var packageId in downloadChanges.Keys) + { + if (outgoingTransfers.TryGetValue(packageId, out var toPackages)) + { + affectedPackages.Add(packageId); + affectedPackages.UnionWith(toPackages); + } + + if (incomingTransfers.ContainsKey(packageId)) + { + affectedPackages.Add(packageId); + } + } + + return affectedPackages; + } + + private long TransferPackageDownloads( + string packageId, + SortedDictionary> outgoingTransfers, + SortedDictionary> incomingTransfers, + DownloadData downloads) + { + var originalDownloads = downloads.GetDownloadCount(packageId); + var transferPercentage = _options.Value.Scoring.PopularityTransfer; + + // Calculate packages with outgoing transfers first. These packages transfer a percentage + // or their downloads equally to a set of "incoming" packages. Packages with both outgoing + // and incoming transfers "reject" the incoming transfers. + if (outgoingTransfers.ContainsKey(packageId)) + { + var keepPercentage = 1 - transferPercentage; + + return (long)(originalDownloads * keepPercentage); + } + + // Next, calculate packages with incoming transfers. These packages receive downloads + // from one or more "outgoing" packages. + if (incomingTransfers.TryGetValue(packageId, out var incomingTransferIds)) + { + var result = originalDownloads; + + foreach (var incomingTransferId in incomingTransferIds) + { + var incomingDownloads = downloads.GetDownloadCount(incomingTransferId); + var incomingSplit = outgoingTransfers[incomingTransferId].Count; + + result += (long)(incomingDownloads * transferPercentage / incomingSplit); + } + + return result; + } + + // The package has no outgoing or incoming transfers. Return its downloads unchanged. + return originalDownloads; + } + private void ApplyDownloadOverrides( DownloadData downloads, IReadOnlyDictionary downloadOverrides, @@ -102,4 +288,4 @@ private void ApplyDownloadOverrides( } } } -} \ No newline at end of file +} diff --git a/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/Integration/PopularityTransferIntegrationTests.cs b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/Integration/PopularityTransferIntegrationTests.cs new file mode 100644 index 000000000..8e4e7f54d --- /dev/null +++ b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/Integration/PopularityTransferIntegrationTests.cs @@ -0,0 +1,412 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.Azure.Search.Models; +using Microsoft.Extensions.Options; +using Moq; +using NuGet.Services.AzureSearch.AuxiliaryFiles; +using NuGet.Services.AzureSearch.Wrappers; +using Xunit; +using Xunit.Abstractions; + +namespace NuGet.Services.AzureSearch.Auxiliary2AzureSearch.Integration +{ + public class PopularityTransferIntegrationTests + { + private readonly InMemoryCloudBlobClient _blobClient; + private readonly InMemoryCloudBlobContainer _auxilliaryContainer; + private readonly InMemoryCloudBlobContainer _storageContainer; + + private readonly Mock _searchOperations; + private readonly Auxiliary2AzureSearchConfiguration _config; + private readonly AzureSearchJobDevelopmentConfiguration _developmentConfig; + private readonly Mock _telemetry; + private readonly UpdateDownloadsCommand _target; + + private readonly SortedDictionary> _newPopularityTransfers; + + private IndexBatch _indexedBatch; + + public PopularityTransferIntegrationTests(ITestOutputHelper output) + { + _telemetry = new Mock(); + + _config = new Auxiliary2AzureSearchConfiguration + { + AuxiliaryDataStorageContainer = "auxiliary-container", + StorageContainer = "storage-container", + Scoring = new AzureSearchScoringConfiguration() + }; + + var options = new Mock>(); + options + .Setup(x => x.Value) + .Returns(_config); + + _developmentConfig = new AzureSearchJobDevelopmentConfiguration(); + var developmentOptions = new Mock>(); + developmentOptions + .Setup(x => x.Value) + .Returns(_developmentConfig); + + var auxiliaryConfig = new AuxiliaryDataStorageConfiguration + { + AuxiliaryDataStorageContainer = "auxiliary-container", + AuxiliaryDataStorageDownloadsPath = "downloads.json", + AuxiliaryDataStorageDownloadOverridesPath = "downloadOverrides.json", + AuxiliaryDataStorageExcludedPackagesPath = "excludedPackages.json", + }; + + var auxiliaryOptions = new Mock>(); + auxiliaryOptions + .Setup(x => x.Value) + .Returns(auxiliaryConfig); + + _auxilliaryContainer = new InMemoryCloudBlobContainer(); + _storageContainer = new InMemoryCloudBlobContainer(); + + _blobClient = new InMemoryCloudBlobClient(); + _blobClient.Containers["auxiliary-container"] = _auxilliaryContainer; + _blobClient.Containers["storage-container"] = _storageContainer; + + var auxiliaryFileClient = new AuxiliaryFileClient( + _blobClient, + auxiliaryOptions.Object, + _telemetry.Object, + output.GetLogger()); + + _newPopularityTransfers = new SortedDictionary>(StringComparer.OrdinalIgnoreCase); + var databaseFetcher = new Mock(); + databaseFetcher + .Setup(x => x.GetPackageIdToPopularityTransfersAsync()) + .ReturnsAsync(_newPopularityTransfers); + + var downloadDataClient = new DownloadDataClient( + _blobClient, + options.Object, + _telemetry.Object, + output.GetLogger()); + + var popularityTransferDataClient = new PopularityTransferDataClient( + _blobClient, + options.Object, + _telemetry.Object, + output.GetLogger()); + + var versionListDataClient = new VersionListDataClient( + _blobClient, + options.Object, + output.GetLogger()); + + var downloadComparer = new DownloadSetComparer( + _telemetry.Object, + options.Object, + output.GetLogger()); + + var dataComparer = new DataSetComparer( + _telemetry.Object, + output.GetLogger()); + + var downloadTransferrer = new DownloadTransferrer( + dataComparer, + options.Object, + output.GetLogger()); + + var baseDocumentBuilder = new BaseDocumentBuilder(options.Object); + var searchDocumentBuilder = new SearchDocumentBuilder(baseDocumentBuilder); + var searchIndexActionBuilder = new SearchIndexActionBuilder( + versionListDataClient, + output.GetLogger()); + + _searchOperations = new Mock(); + _searchOperations + .Setup(x => x.IndexAsync(It.IsAny>())) + .Callback>(batch => + { + _indexedBatch = batch; + }) + .ReturnsAsync(new DocumentIndexResult()); + + var hijackIndexClient = new Mock(); + var searchIndexClient = new Mock(); + searchIndexClient + .Setup(x => x.Documents) + .Returns(_searchOperations.Object); + + var batchPusher = new BatchPusher( + searchIndexClient.Object, + hijackIndexClient.Object, + versionListDataClient, + options.Object, + developmentOptions.Object, + _telemetry.Object, + output.GetLogger()); + + Func batchPusherFactory = () => batchPusher; + + var time = new Mock(); + + _target = new UpdateDownloadsCommand( + auxiliaryFileClient, + databaseFetcher.Object, + downloadDataClient, + downloadComparer, + downloadTransferrer, + popularityTransferDataClient, + searchDocumentBuilder, + searchIndexActionBuilder, + batchPusherFactory, + time.Object, + options.Object, + _telemetry.Object, + output.GetLogger()); + } + + [Fact] + public async Task FirstPopularityTransferChangesDownloads() + { + SetDownloadOverrides("{}"); + SetExcludedPackagesJson("{}"); + + AddVersionList("A", "1.0.0"); + AddVersionList("B", "1.0.0"); + + SetOldDownloadsJson(@" +{ + ""A"": { ""1.0.0"": 100 }, + ""B"": { ""1.0.0"": 1 } +}"); + SetNewDownloadsJson(@" +[ + [ ""A"", [ ""1.0.0"", 100 ] ], + [ ""B"", [ ""1.0.0"", 1 ] ], +]"); + + // Old: no rename + // New: A -> B rename + SetOldPopularityTransfersJson(@"{}"); + AddNewPopularityTransfer("A", "B"); + + _config.Scoring.PopularityTransfer = 0.5; + + await _target.ExecuteAsync(); + + Assert.NotNull(_indexedBatch); + var actions = _indexedBatch.Actions.OrderBy(x => x.Document.Key).ToList(); + Assert.Equal(8, actions.Count); + + VerifyUpdateDownloadCountAction("A", 50, actions[0]); + VerifyUpdateDownloadCountAction("A", 50, actions[1]); + VerifyUpdateDownloadCountAction("A", 50, actions[2]); + VerifyUpdateDownloadCountAction("A", 50, actions[3]); + VerifyUpdateDownloadCountAction("B", 51, actions[4]); + VerifyUpdateDownloadCountAction("B", 51, actions[5]); + VerifyUpdateDownloadCountAction("B", 51, actions[6]); + VerifyUpdateDownloadCountAction("B", 51, actions[7]); + } + + [Fact] + public async Task NewPopularityTransferChangesDownlaods() + { + SetDownloadOverrides("{}"); + SetExcludedPackagesJson("{}"); + + AddVersionList("A", "1.0.0"); + AddVersionList("B", "1.0.0"); + AddVersionList("C", "1.0.0"); + AddVersionList("D", "1.0.0"); + + SetOldDownloadsJson(@" +{ + ""A"": { ""1.0.0"": 100 }, + ""B"": { ""1.0.0"": 50 }, + ""C"": { ""1.0.0"": 20 }, + ""D"": { ""1.0.0"": 1 } +}"); + SetNewDownloadsJson(@" +[ + [ ""A"", [ ""1.0.0"", 100 ] ], + [ ""B"", [ ""1.0.0"", 50 ] ], + [ ""C"", [ ""1.0.0"", 20 ] ], + [ ""D"", [ ""1.0.0"", 1 ] ] +]"); + + // Old: A -> B rename + // New: A -> B, C -> D rename + SetOldPopularityTransfersJson(@"{ ""A"": [ ""B"" ] }"); + AddNewPopularityTransfer("A", "B"); + AddNewPopularityTransfer("C", "D"); + + _config.Scoring.PopularityTransfer = 0.5; + + await _target.ExecuteAsync(); + + Assert.NotNull(_indexedBatch); + var actions = _indexedBatch.Actions.OrderBy(x => x.Document.Key).ToList(); + Assert.Equal(8, actions.Count); + + VerifyUpdateDownloadCountAction("C", 10, actions[0]); + VerifyUpdateDownloadCountAction("C", 10, actions[1]); + VerifyUpdateDownloadCountAction("C", 10, actions[2]); + VerifyUpdateDownloadCountAction("C", 10, actions[3]); + VerifyUpdateDownloadCountAction("D", 11, actions[4]); + VerifyUpdateDownloadCountAction("D", 11, actions[5]); + VerifyUpdateDownloadCountAction("D", 11, actions[6]); + VerifyUpdateDownloadCountAction("D", 11, actions[7]); + } + + [Fact] + public async Task UpdatedPopularityTransferChangesDownloads() + { + SetDownloadOverrides("{}"); + SetExcludedPackagesJson("{}"); + + AddVersionList("A", "1.0.0"); + AddVersionList("B", "1.0.0"); + AddVersionList("C", "1.0.0"); + + SetOldDownloadsJson(@" +{ + ""A"": { ""1.0.0"": 100 }, + ""B"": { ""1.0.0"": 20 }, + ""C"": { ""1.0.0"": 1 }, +}"); + SetNewDownloadsJson(@" +[ + [ ""A"", [ ""1.0.0"", 100 ] ], + [ ""B"", [ ""1.0.0"", 20 ] ], + [ ""C"", [ ""1.0.0"", 1 ] ], +]"); + + // Old: A -> B rename + // New: A -> C rename + SetOldPopularityTransfersJson(@"{ ""A"": [ ""B"" ] }"); + AddNewPopularityTransfer("A", "C"); + + _config.Scoring.PopularityTransfer = 0.5; + + await _target.ExecuteAsync(); + + Assert.NotNull(_indexedBatch); + var actions = _indexedBatch.Actions.OrderBy(x => x.Document.Key).ToList(); + Assert.Equal(12, actions.Count); + + VerifyUpdateDownloadCountAction("A", 50, actions[0]); + VerifyUpdateDownloadCountAction("A", 50, actions[1]); + VerifyUpdateDownloadCountAction("A", 50, actions[2]); + VerifyUpdateDownloadCountAction("A", 50, actions[3]); + VerifyUpdateDownloadCountAction("B", 20, actions[4]); + VerifyUpdateDownloadCountAction("B", 20, actions[5]); + VerifyUpdateDownloadCountAction("B", 20, actions[6]); + VerifyUpdateDownloadCountAction("B", 20, actions[7]); + VerifyUpdateDownloadCountAction("C", 51, actions[8]); + VerifyUpdateDownloadCountAction("C", 51, actions[9]); + VerifyUpdateDownloadCountAction("C", 51, actions[10]); + VerifyUpdateDownloadCountAction("C", 51, actions[11]); + } + + [Fact] + public async Task ReverseTransferChangesDownloads() + { + SetDownloadOverrides("{}"); + SetExcludedPackagesJson("{}"); + + AddVersionList("A", "1.0.0"); + AddVersionList("B", "1.0.0"); + + SetOldDownloadsJson(@" +{ + ""A"": { ""1.0.0"": 100 }, + ""B"": { ""1.0.0"": 20 } +}"); + SetNewDownloadsJson(@" +[ + [ ""A"", [ ""1.0.0"", 100 ] ], + [ ""B"", [ ""1.0.0"", 20 ] ] +]"); + + // Old: A -> B rename + // New: B -> A rename + SetOldPopularityTransfersJson(@"{ ""A"": [ ""B"" ] }"); + AddNewPopularityTransfer("B", "A"); + + _config.Scoring.PopularityTransfer = 0.5; + + await _target.ExecuteAsync(); + + Assert.NotNull(_indexedBatch); + var actions = _indexedBatch.Actions.OrderBy(x => x.Document.Key).ToList(); + Assert.Equal(8, actions.Count); + + VerifyUpdateDownloadCountAction("A", 110, actions[0]); + VerifyUpdateDownloadCountAction("A", 110, actions[1]); + VerifyUpdateDownloadCountAction("A", 110, actions[2]); + VerifyUpdateDownloadCountAction("A", 110, actions[3]); + VerifyUpdateDownloadCountAction("B", 10, actions[4]); + VerifyUpdateDownloadCountAction("B", 10, actions[5]); + VerifyUpdateDownloadCountAction("B", 10, actions[6]); + VerifyUpdateDownloadCountAction("B", 10, actions[7]); + } + + private void SetOldDownloadsJson(string json) + { + _storageContainer.Blobs["downloads/downloads.v2.json"] = new InMemoryCloudBlob(json); + } + + private void SetNewDownloadsJson(string json) + { + _auxilliaryContainer.Blobs["downloads.json"] = new InMemoryCloudBlob(json); + } + + private void SetDownloadOverrides(string json) + { + _auxilliaryContainer.Blobs["downloadOverrides.json"] = new InMemoryCloudBlob(json); + } + + private void SetExcludedPackagesJson(string json) + { + _auxilliaryContainer.Blobs["excludedPackages.json"] = new InMemoryCloudBlob(json); + } + + private void SetOldPopularityTransfersJson(string json) + { + _storageContainer.Blobs["popularity-transfers/popularity-transfers.v1.json"] + = new InMemoryCloudBlob(json); + } + + private void AddVersionList(string id, string version) + { + _storageContainer.Blobs[$"version-lists/{id.ToLowerInvariant()}.json"] = new InMemoryCloudBlob(@" +{ + ""VersionProperties"": { + """ + version + @""": { ""Listed"": true } + } +}"); + } + + private void AddNewPopularityTransfer(string fromId, string toId) + { + if (!_newPopularityTransfers.TryGetValue(fromId, out var popularityTransfers)) + { + popularityTransfers = new SortedSet(StringComparer.OrdinalIgnoreCase); + _newPopularityTransfers[fromId] = popularityTransfers; + } + + popularityTransfers.Add(toId); + } + + private void VerifyUpdateDownloadCountAction( + string expectedId, + long expectedDownloads, + IndexAction action) + { + var document = action.Document as SearchDocument.UpdateDownloadCount; + + Assert.NotNull(document); + Assert.Equal(IndexActionType.Merge, action.ActionType); + Assert.StartsWith(expectedId, document.Key, StringComparison.OrdinalIgnoreCase); + Assert.Equal(expectedDownloads, document.TotalDownloadCount); + } + } +} diff --git a/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/UpdateDownloadsCommandFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/UpdateDownloadsCommandFacts.cs index 2f632fdfb..57ebac98c 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/UpdateDownloadsCommandFacts.cs +++ b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/UpdateDownloadsCommandFacts.cs @@ -221,12 +221,19 @@ public async Task AppliesTransferChanges() It.IsAny()), Times.Once); - // TODO: Popularity transfers auxiliary file should have new data. - // See: https://github.com/NuGet/NuGetGallery/issues/7898 + // Popularity transfers auxiliary file should have new data. + PopularityTransferDataClient.Verify( + c => c.ReplaceLatestIndexedAsync( + It.Is>>(d => + d.Count == 1 && + d["Package1"].Count == 1 && + d["Package1"].Contains("Package2")), + It.IsAny()), + Times.Once); } [Fact] - public async Task TransferChangesOverideDownloadChanges() + public async Task TransferChangesOverrideDownloadChanges() { DownloadSetComparer .Setup(c => c.Compare(It.IsAny(), It.IsAny())) @@ -249,9 +256,9 @@ public async Task TransferChangesOverideDownloadChanges() TransferChanges["A"] = 55; TransferChanges["b"] = 66; - NewTransfers["FromPackage"] = new SortedSet(StringComparer.OrdinalIgnoreCase) + NewTransfers["A"] = new SortedSet(StringComparer.OrdinalIgnoreCase) { - "ToPackage" + "b" }; await Target.ExecuteAsync(); @@ -288,8 +295,15 @@ public async Task TransferChangesOverideDownloadChanges() It.IsAny()), Times.Once); - // TODO: Popularity transfers auxiliary file should have new data. - // See: https://github.com/NuGet/NuGetGallery/issues/7898 + // Popularity transfers auxiliary file should have new data. + PopularityTransferDataClient.Verify( + c => c.ReplaceLatestIndexedAsync( + It.Is>>(d => + d.Count == 1 && + d["A"].Count == 1 && + d["A"].Contains("b")), + It.IsAny()), + Times.Once); } } @@ -334,7 +348,6 @@ public Facts(ITestOutputHelper output) .Returns(() => Changes); OldTransfers = new SortedDictionary>(StringComparer.OrdinalIgnoreCase); - OldTransferResult = new ResultAndAccessCondition>>( OldTransfers, Mock.Of()); diff --git a/tests/NuGet.Services.AzureSearch.Tests/Db2AzureSearch/Db2AzureSearchCommandFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/Db2AzureSearch/Db2AzureSearchCommandFacts.cs index 4aeba00fa..13f420690 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/Db2AzureSearch/Db2AzureSearchCommandFacts.cs +++ b/tests/NuGet.Services.AzureSearch.Tests/Db2AzureSearch/Db2AzureSearchCommandFacts.cs @@ -326,5 +326,31 @@ public async Task PushesVerifiedPackagesData() x => x.ReplaceLatestAsync(It.IsAny>(), It.IsAny()), Times.Once); } + + [Fact] + public async Task PushesPopularityTransferData() + { + SortedDictionary> data = null; + IAccessCondition accessCondition = null; + _popularityTransferDataClient + .Setup(x => x.ReplaceLatestIndexedAsync(It.IsAny>>(), It.IsAny())) + .Returns(Task.CompletedTask) + .Callback>, IAccessCondition>((d, a) => + { + data = d; + accessCondition = a; + }); + + await _target.ExecuteAsync(); + + Assert.Same(_initialAuxiliaryData.PopularityTransfers, data); + + Assert.Equal("*", accessCondition.IfNoneMatchETag); + Assert.Null(accessCondition.IfMatchETag); + + _popularityTransferDataClient.Verify( + x => x.ReplaceLatestIndexedAsync(It.IsAny>>(), It.IsAny()), + Times.Once); + } } } diff --git a/tests/NuGet.Services.AzureSearch.Tests/DownloadTransferrerFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/DownloadTransferrerFacts.cs index 7b3447d6e..023b055c7 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/DownloadTransferrerFacts.cs +++ b/tests/NuGet.Services.AzureSearch.Tests/DownloadTransferrerFacts.cs @@ -28,6 +28,217 @@ public void ReturnsEmptyResult() Assert.Empty(result); } + [Fact] + public void DoesNothingIfNoTransfers() + { + PopularityTransfer = 0.5; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Empty(result); + } + + [Fact] + public void TransfersPopularity() + { + PopularityTransfer = 0.5; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + + AddPopularityTransfer("A", "B"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(50, result["A"]); + Assert.Equal(55, result["B"]); + } + + [Fact] + public void SplitsPopularity() + { + PopularityTransfer = 0.5; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + DownloadData.SetDownloadCount("C", "1.0.0", 1); + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("A", "C"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(50, result["A"]); + Assert.Equal(30, result["B"]); + Assert.Equal(26, result["C"]); + } + + [Fact] + public void PopularityTransferRoundsDown() + { + PopularityTransfer = 0.5; + + DownloadData.SetDownloadCount("A", "1.0.0", 3); + DownloadData.SetDownloadCount("B", "1.0.0", 0); + + AddPopularityTransfer("A", "B"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(1, result["A"]); + Assert.Equal(1, result["B"]); + } + + [Fact] + public void AcceptsPopularityFromMultipleSources() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 20); + DownloadData.SetDownloadCount("C", "1.0.0", 1); + + AddPopularityTransfer("A", "C"); + AddPopularityTransfer("B", "C"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(0, result["B"]); + Assert.Equal(121, result["C"]); + } + + [Fact] + public void SupportsZeroPopularityTransfer() + { + PopularityTransfer = 0; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + + AddPopularityTransfer("A", "B"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(100, result["A"]); + Assert.Equal(5, result["B"]); + } + + [Fact] + public void PackageWithOutgoingTransferRejectsIncomingTransfer() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 0); + DownloadData.SetDownloadCount("C", "1.0.0", 0); + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("A", "C"); + AddPopularityTransfer("B", "C"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + // B has incoming and outgoing popularity transfers. It should reject the incoming transfer. + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(0, result["B"]); + Assert.Equal(50, result["C"]); + } + + [Fact] + public void PopularityTransfersAreNotTransitive() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 100); + DownloadData.SetDownloadCount("C", "1.0.0", 100); + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("B", "C"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + // A transfers downloads to B. + // B transfers downloads to C. + // B and C should reject downloads from A. + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(0, result["B"]); + Assert.Equal(200, result["C"]); + } + + [Fact] + public void RejectsCyclicalPopularityTransfers() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 100); + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("B", "A"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(0, result["B"]); + } + + [Fact] + public void UnknownPackagesTransferZeroDownloads() + { + PopularityTransfer = 1; + + AddPopularityTransfer("A", "B"); + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(0, result["B"]); + } + [Fact] public void AppliesDownloadOverrides() { @@ -45,6 +256,34 @@ public void AppliesDownloadOverrides() Assert.Equal(1000, result["A"]); } + [Fact] + public void OverridesPopularityTransfer() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("From1", "1.0.0", 2); + DownloadData.SetDownloadCount("From2", "1.0.0", 2); + DownloadData.SetDownloadCount("To1", "1.0.0", 0); + DownloadData.SetDownloadCount("To2", "1.0.0", 0); + + AddPopularityTransfer("From1", "To1"); + AddPopularityTransfer("From2", "To2"); + + DownloadOverrides["From1"] = 1000; + DownloadOverrides["To2"] = 1000; + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "From1", "From2", "To1", "To2" }, result.Keys); + Assert.Equal(1000, result["From1"]); + Assert.Equal(0, result["From2"]); + Assert.Equal(2, result["To1"]); + Assert.Equal(1000, result["To2"]); + } + [Fact] public void DoesNotOverrideGreaterOrEqualDownloads() { @@ -61,6 +300,34 @@ public void DoesNotOverrideGreaterOrEqualDownloads() Assert.Empty(result); } + + [Fact] + public void DoesNotOverrideGreaterPopularityTransfer() + { + PopularityTransfer = 0.5; + + DownloadData.SetDownloadCount("From1", "1.0.0", 100); + DownloadData.SetDownloadCount("From2", "1.0.0", 100); + DownloadData.SetDownloadCount("To1", "1.0.0", 0); + DownloadData.SetDownloadCount("To2", "1.0.0", 0); + + AddPopularityTransfer("From1", "To1"); + AddPopularityTransfer("From2", "To2"); + + DownloadOverrides["From1"] = 1; + DownloadOverrides["To2"] = 1; + + var result = Target.InitializeDownloadTransfers( + DownloadData, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "From1", "From2", "To1", "To2" }, result.Keys); + Assert.Equal(50, result["From1"]); + Assert.Equal(50, result["From2"]); + Assert.Equal(50, result["To1"]); + Assert.Equal(50, result["To2"]); + } } public class GetUpdatedTransferChanges : Facts @@ -111,6 +378,373 @@ public void ReturnsEmptyResult() Assert.Empty(result); } + [Fact] + public void DoesNothingIfNoTransfers() + { + PopularityTransfer = 0.5; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + + DownloadChanges["A"] = 100; + DownloadChanges["B"] = 5; + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Empty(result); + } + + [Fact] + public void DoesNothingIfNoChanges() + { + PopularityTransfer = 0.5; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + + AddPopularityTransfer("A", "B"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Empty(result); + } + + [Fact] + public void OutgoingTransfersNewDownloads() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 20); + DownloadData.SetDownloadCount("C", "1.0.0", 1); + + DownloadChanges["A"] = 100; + + AddPopularityTransfer("A", "C"); + AddPopularityTransfer("B", "C"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + // C receives downloads from A and B + // A has download changes + // B has no changes + Assert.Equal(new[] { "A", "C" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(121, result["C"]); + } + + [Fact] + public void OutgoingTransfersSplitsNewDownloads() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + DownloadData.SetDownloadCount("C", "1.0.0", 0); + + DownloadChanges["A"] = 100; + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("A", "C"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(55, result["B"]); + Assert.Equal(50, result["C"]); + } + + [Fact] + public void PopularityTransferRoundsDown() + { + PopularityTransfer = 0.5; + + DownloadData.SetDownloadCount("A", "1.0.0", 3); + DownloadData.SetDownloadCount("B", "1.0.0", 0); + + DownloadChanges["A"] = 3; + + AddPopularityTransfer("A", "B"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(1, result["A"]); + Assert.Equal(1, result["B"]); + } + + [Fact] + public void IncomingTransfersAddedToNewDownloads() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + DownloadData.SetDownloadCount("C", "1.0.0", 0); + + DownloadChanges["B"] = 5; + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("A", "C"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + // B has new downloads and receives downloads from A. + Assert.Equal(new[] { "B" }, result.Keys); + Assert.Equal(55, result["B"]); + } + + [Fact] + public void NewOrUpdatedPopularityTransfer() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + + AddPopularityTransfer("A", "B"); + + TransferChanges["A"] = new[] { "B" }; + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(105, result["B"]); + } + + [Fact] + public void NewOrUpdatedSplitsPopularityTransfer() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + DownloadData.SetDownloadCount("C", "1.0.0", 0); + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("A", "C"); + + TransferChanges["A"] = new[] { "B", "C" }; + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(55, result["B"]); + Assert.Equal(50, result["C"]); + } + + [Fact] + public void RemovesIncomingPopularityTransfer() + { + // A used to transfer to both B and C. + // A now transfers to just B. + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + DownloadData.SetDownloadCount("C", "1.0.0", 0); + + AddPopularityTransfer("A", "B"); + + TransferChanges["A"] = new[] { "B" }; + OldTransfers["A"] = new SortedSet(StringComparer.OrdinalIgnoreCase) + { + "B", "C" + }; + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(105, result["B"]); + Assert.Equal(0, result["C"]); + } + + [Fact] + public void RemovePopularityTransfer() + { + // A used to transfer to both B and C. + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + DownloadData.SetDownloadCount("C", "1.0.0", 0); + + TransferChanges["A"] = new string[0]; + OldTransfers["A"] = new SortedSet(StringComparer.OrdinalIgnoreCase) + { + "B", "C" + }; + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(100, result["A"]); + Assert.Equal(5, result["B"]); + Assert.Equal(0, result["C"]); + } + + [Fact] + public void SupportsZeroPopularityTransfer() + { + PopularityTransfer = 0; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 5); + + DownloadChanges["A"] = 100; + + AddPopularityTransfer("A", "B"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(100, result["A"]); + Assert.Equal(5, result["B"]); + } + + [Fact] + public void PackageWithOutgoingTransferRejectsIncomingTransfer() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 0); + DownloadData.SetDownloadCount("C", "1.0.0", 0); + + DownloadChanges["A"] = 100; + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("A", "C"); + AddPopularityTransfer("B", "C"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + // B has incoming and outgoing popularity transfers. It should reject the incoming transfer. + Assert.Equal(new[] { "A", "B", "C" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(0, result["B"]); + Assert.Equal(50, result["C"]); + } + + [Fact] + public void PopularityTransfersAreNotTransitive() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 100); + DownloadData.SetDownloadCount("C", "1.0.0", 100); + + DownloadChanges["A"] = 100; + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("B", "C"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + // A transfers downloads to B. + // B transfers downloads to C. + // B and C should reject downloads from A. + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(0, result["B"]); + } + + [Fact] + public void RejectsCyclicalPopularityTransfers() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 100); + DownloadData.SetDownloadCount("B", "1.0.0", 100); + + DownloadChanges["A"] = 100; + DownloadChanges["B"] = 100; + + AddPopularityTransfer("A", "B"); + AddPopularityTransfer("B", "A"); + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(0, result["B"]); + } + [Fact] public void AppliesDownloadOverrides() { @@ -161,6 +795,58 @@ public void DoesNotOverrideGreaterOrEqualDownloads() Assert.Empty(result); } + [Fact] + public void OverridesPopularityTransfer() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 1); + DownloadData.SetDownloadCount("B", "1.0.0", 0); + + DownloadChanges["A"] = 1; + + AddPopularityTransfer("A", "B"); + + DownloadOverrides["B"] = 1000; + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(1000, result["B"]); + } + + [Fact] + public void DoesNotOverrideGreaterPopularityTransfer() + { + PopularityTransfer = 1; + + DownloadData.SetDownloadCount("A", "1.0.0", 1000); + DownloadData.SetDownloadCount("B", "1.0.0", 0); + + DownloadChanges["A"] = 1000; + + AddPopularityTransfer("A", "B"); + + DownloadOverrides["B"] = 1; + + var result = Target.UpdateDownloadTransfers( + DownloadData, + DownloadChanges, + OldTransfers, + PopularityTransfers, + DownloadOverrides); + + Assert.Equal(new[] { "A", "B" }, result.Keys); + Assert.Equal(0, result["A"]); + Assert.Equal(1000, result["B"]); + } + public GetUpdatedTransferChanges() { DownloadChanges = new SortedDictionary(StringComparer.OrdinalIgnoreCase); @@ -176,13 +862,32 @@ public class Facts public Facts() { TransferChanges = new SortedDictionary(); + DataComparer = new Mock(); + DataComparer + .Setup(x => x.ComparePopularityTransfers( + It.IsAny>>(), + It.IsAny>>())) + .Returns(TransferChanges); DownloadOverrides = new Dictionary(); PopularityTransfers = new SortedDictionary>(StringComparer.OrdinalIgnoreCase); + var options = new Mock>(); + options + .Setup(x => x.Value) + .Returns(() => new AzureSearchJobConfiguration + { + Scoring = new AzureSearchScoringConfiguration + { + PopularityTransfer = PopularityTransfer + } + }); + DownloadData = new DownloadData(); Target = new DownloadTransferrer( + DataComparer.Object, + options.Object, Mock.Of>()); } @@ -207,4 +912,4 @@ public void AddPopularityTransfer(string fromPackageId, string toPackageId) } } } -} \ No newline at end of file +} diff --git a/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj b/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj index aa5a4b381..b767ddc03 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj +++ b/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj @@ -37,6 +37,7 @@ + diff --git a/tests/NuGet.Services.V3.Tests/Support/InMemoryCloudBlob.cs b/tests/NuGet.Services.V3.Tests/Support/InMemoryCloudBlob.cs index 1f8ff663d..524dceaa4 100644 --- a/tests/NuGet.Services.V3.Tests/Support/InMemoryCloudBlob.cs +++ b/tests/NuGet.Services.V3.Tests/Support/InMemoryCloudBlob.cs @@ -20,12 +20,23 @@ public class InMemoryCloudBlob : ISimpleCloudBlob private readonly object _lock = new object(); private string _etag; + public InMemoryCloudBlob() + { + } + + public InMemoryCloudBlob(string content) + { + Bytes = Encoding.ASCII.GetBytes(content); + Exists = true; + _etag = Interlocked.Increment(ref _nextETag).ToString(); + } + public BlobProperties Properties { get; } = new CloudBlockBlob(new Uri("https://example/blob")).Properties; public IDictionary Metadata => throw new NotImplementedException(); public CopyState CopyState => throw new NotImplementedException(); public Uri Uri => throw new NotImplementedException(); public string Name => throw new NotImplementedException(); - public DateTime LastModifiedUtc => throw new NotImplementedException(); + public DateTime LastModifiedUtc { get; private set; } = DateTime.UtcNow; public bool IsSnapshot => throw new NotImplementedException(); public string ETag @@ -123,9 +134,14 @@ public Task OpenReadStreamAsync(TimeSpan serverTimeout, TimeSpan maxExec throw new NotImplementedException(); } - public Task OpenWriteAsync(AccessCondition accessCondition) + public async Task OpenWriteAsync(AccessCondition accessCondition) { - throw new NotImplementedException(); + await Task.Yield(); + + return new RecordingStream(bytes => + { + UploadFromBytes(bytes, accessCondition); + }); } public Task SetMetadataAsync(AccessCondition accessCondition) @@ -174,9 +190,15 @@ public async Task UploadFromStreamAsync(Stream source, AccessCondition accessCon var buffer = new MemoryStream(); await source.CopyToAsync(buffer); - var newETag = Interlocked.Increment(ref _nextETag).ToString(); var newBytes = buffer.ToArray(); + UploadFromBytes(newBytes, accessCondition); + } + + private void UploadFromBytes(byte[] newBytes, AccessCondition accessCondition) + { + var newETag = Interlocked.Increment(ref _nextETag).ToString(); + lock (_lock) { if (Exists) @@ -202,6 +224,7 @@ public async Task UploadFromStreamAsync(Stream source, AccessCondition accessCon _etag = newETag; Bytes = newBytes; Exists = true; + LastModifiedUtc = DateTime.UtcNow; } } }