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; } } }