From 981ec3c7ccdc85bdb492bd40025d64c451a123b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= Date: Tue, 14 Apr 2020 11:15:29 -0700 Subject: [PATCH] Add popularity transfers comparer (#766) The `auxiliary2azuresearch` job needs to know which popularity transfers have changed to properly update the search index. Previous change: https://github.com/NuGet/NuGet.Services.Metadata/pull/765 Part of https://github.com/nuget/nugetgallery/issues/7898 --- .../Auxiliary2AzureSearch/DataSetComparer.cs | 145 +++++++ ...wnerSetComparer.cs => IDataSetComparer.cs} | 18 +- .../Auxiliary2AzureSearch/OwnerSetComparer.cs | 111 ----- .../UpdateOwnersCommand.cs | 6 +- .../AzureSearchTelemetryService.cs | 13 + .../DependencyInjectionExtensions.cs | 2 +- .../IAzureSearchTelemetryService.cs | 1 + .../NuGet.Services.AzureSearch.csproj | 4 +- .../DataSetComparerFacts.cs | 404 ++++++++++++++++++ .../OwnerSetComparerFacts.cs | 161 ------- .../UpdateOwnersCommandFacts.cs | 10 +- .../NuGet.Services.AzureSearch.Tests.csproj | 2 +- 12 files changed, 590 insertions(+), 287 deletions(-) create mode 100644 src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/DataSetComparer.cs rename src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/{IOwnerSetComparer.cs => IDataSetComparer.cs} (53%) delete mode 100644 src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/OwnerSetComparer.cs create mode 100644 tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/DataSetComparerFacts.cs delete mode 100644 tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/OwnerSetComparerFacts.cs diff --git a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/DataSetComparer.cs b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/DataSetComparer.cs new file mode 100644 index 000000000..c78519261 --- /dev/null +++ b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/DataSetComparer.cs @@ -0,0 +1,145 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using Microsoft.Extensions.Logging; + +namespace NuGet.Services.AzureSearch.Auxiliary2AzureSearch +{ + public class DataSetComparer : IDataSetComparer + { + private static readonly string[] EmptyStringArray = new string[0]; + + private readonly IAzureSearchTelemetryService _telemetryService; + private readonly ILogger _logger; + + public DataSetComparer( + IAzureSearchTelemetryService telemetryService, + ILogger logger) + { + _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + } + + public SortedDictionary CompareOwners( + SortedDictionary> oldData, + SortedDictionary> newData) + { + // Use ordinal comparison to allow username case changes to flow through. + var stopwatch = Stopwatch.StartNew(); + var result = CompareData( + oldData, + newData, + "package ID", + "owners", + StringComparer.Ordinal); + + stopwatch.Stop(); + _telemetryService.TrackOwnerSetComparison(oldData.Count, newData.Count, result.Count, stopwatch.Elapsed); + + return result; + } + + public SortedDictionary ComparePopularityTransfers( + SortedDictionary> oldData, + SortedDictionary> newData) + { + // Ignore case changes in popularity transfers. + var stopwatch = Stopwatch.StartNew(); + var result = CompareData( + oldData, + newData, + "package ID", + "popularity transfers", + StringComparer.OrdinalIgnoreCase); + + stopwatch.Stop(); + _telemetryService.TrackPopularityTransfersSetComparison(oldData.Count, newData.Count, result.Count, stopwatch.Elapsed); + + return result; + } + + private SortedDictionary CompareData( + SortedDictionary> oldData, + SortedDictionary> newData, + string keyName, + string valuesName, + StringComparer valuesComparer) + { + if (oldData.Comparer != StringComparer.OrdinalIgnoreCase) + { + throw new ArgumentException("The old data should have a case-insensitive comparer.", nameof(oldData)); + } + + if (newData.Comparer != StringComparer.OrdinalIgnoreCase) + { + throw new ArgumentException("The new data should have a case-insensitive comparer.", nameof(newData)); + } + + // We use a very simplistic algorithm here. Perform one pass on the new data to find the added or changed + // values. Then perform a second pass on the old data to find removed keys. We can optimize + // this later if necessary. + // + // On the "changed" case, we emit all of the values instead of the delta. This is because Azure Search + // does not have a way to append or remove a specific item from a field that is an array. + // The entire new array needs to be provided. + var result = new SortedDictionary(StringComparer.OrdinalIgnoreCase); + + // First pass: find added or changed sets. + foreach (var pair in newData) + { + var key = pair.Key; + var newValues = pair.Value; + if (!oldData.TryGetValue(key, out var oldValues)) + { + // ADDED: The key does not exist in the old data, which means the key was added. + result.Add(key, newValues.ToArray()); + _logger.LogInformation( + $"The {keyName} {{Key}} has been added, with {{AddedCount}} {valuesName}.", + key, + newValues.Count); + } + else + { + // The key exists in the old data. We need to check if the values set has changed. + var removedValues = oldValues.Except(newValues, valuesComparer).ToList(); + var addedValues = newValues.Except(oldValues, valuesComparer).ToList(); + + if (removedValues.Any() || addedValues.Any()) + { + // CHANGED: The values set has changed. + result.Add(key, newValues.ToArray()); + _logger.LogInformation( + $"The {keyName} {{Key}} {valuesName} have changed, with {{RemovedCount}} {valuesName} removed and " + + $"{{AddedCount}} {valuesName} added.", + key, + removedValues.Count, + addedValues.Count); + } + } + } + + // Second pass: find removed sets. + foreach (var pair in oldData) + { + var key = pair.Key; + var oldValues = pair.Value; + + if (!newData.TryGetValue(key, out var newValues)) + { + // REMOVED: The key does not exist in the new data, which means the key was removed. + result.Add(key, EmptyStringArray); + _logger.LogInformation( + $"The {keyName} {{Key}} has been removed, with {{RemovedCount}} {valuesName}", + key, + oldValues.Count); + } + } + + return result; + } + } +} diff --git a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/IOwnerSetComparer.cs b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/IDataSetComparer.cs similarity index 53% rename from src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/IOwnerSetComparer.cs rename to src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/IDataSetComparer.cs index 038da5948..72c657e3f 100644 --- a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/IOwnerSetComparer.cs +++ b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/IDataSetComparer.cs @@ -6,9 +6,9 @@ namespace NuGet.Services.AzureSearch.Auxiliary2AzureSearch { /// - /// Used to compare two sets of owners to determine the changes. + /// Used to compare two sets of data to determine the changes. /// - public interface IOwnerSetComparer + public interface IDataSetComparer { /// /// Compares two sets of owners to determine the package IDs that have changed. The returned dictionary @@ -19,7 +19,19 @@ public interface IOwnerSetComparer /// /// The old owner information, typically from storage. /// The new owner information, typically from gallery DB. - SortedDictionary Compare( + SortedDictionary CompareOwners( + SortedDictionary> oldData, + SortedDictionary> newData); + + /// + /// Compares two sets of popularity transfers to determine changes. The two inputs are maps of package IDs that transfer + /// popularity away to package IDs that receive the popularity. The returned dictionary is subset of these inputs that + /// were added, removed, or changed. For the "added" and "changed" cases, the popularity transfer set is the new data. + /// For the "removed" case, the set is empty. + /// + /// The old popularity transfers, typically from storage. + /// The new popularity transfers, typically from gallery DB. + SortedDictionary ComparePopularityTransfers( SortedDictionary> oldData, SortedDictionary> newData); } diff --git a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/OwnerSetComparer.cs b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/OwnerSetComparer.cs deleted file mode 100644 index 8cef67759..000000000 --- a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/OwnerSetComparer.cs +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; -using Microsoft.Extensions.Logging; - -namespace NuGet.Services.AzureSearch.Auxiliary2AzureSearch -{ - public class OwnerSetComparer : IOwnerSetComparer - { - private static readonly string[] EmptyStringArray = new string[0]; - - private readonly IAzureSearchTelemetryService _telemetryService; - private readonly ILogger _logger; - - public OwnerSetComparer( - IAzureSearchTelemetryService telemetryService, - ILogger logger) - { - _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService)); - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - } - - public SortedDictionary Compare( - SortedDictionary> oldData, - SortedDictionary> newData) - { - if (oldData.Comparer != StringComparer.OrdinalIgnoreCase) - { - throw new ArgumentException("The old data should have a case-insensitive comparer.", nameof(oldData)); - } - - if (newData.Comparer != StringComparer.OrdinalIgnoreCase) - { - throw new ArgumentException("The new data should have a case-insensitive comparer.", nameof(newData)); - } - - var stopwatch = Stopwatch.StartNew(); - - // We use a very simplistic algorithm here. Perform one pass on the new data to find the added or changed - // package IDs. Then perform a second pass on the old data to find removed package IDs. We can optimize - // this later if necessary. - // - // We emit all of the usernames when a package ID's owners have changed instead of the delta. This is - // because Azure Search does not have a way to append or remove a specific item from a field that is an - // array. The entire new array needs to be provided. - var result = new SortedDictionary(StringComparer.OrdinalIgnoreCase); - - // First pass: find added or changed sets. - foreach (var pair in newData) - { - var id = pair.Key; - var newOwners = pair.Value; - if (!oldData.TryGetValue(id, out var oldOwners)) - { - // ADDED: The package ID does not exist in the old data, which means the package ID was added. - result.Add(id, newOwners.ToArray()); - _logger.LogInformation( - "The package ID {ID} has been added, with {AddedCount} owners.", - id, - newOwners.Count); - } - else - { - // The package ID exists in the old data. We need to check if the owner set has changed. Perform - // an ordinal comparison to allow username case changes to flow through. - var removedUsernames = oldOwners.Except(newOwners, StringComparer.Ordinal).ToList(); - var addedUsernames = newOwners.Except(oldOwners, StringComparer.Ordinal).ToList(); - - if (removedUsernames.Any() || addedUsernames.Any()) - { - // CHANGED: The username set has changed. - result.Add(id, newOwners.ToArray()); - _logger.LogInformation( - "The package ID {ID} has an ownership change, with {RemovedCount} owners removed and " + - "{AddedCount} owners added.", - id, - removedUsernames.Count, - addedUsernames.Count); - } - } - } - - // Second pass: find removed sets. - foreach (var pair in oldData) - { - var id = pair.Key; - var oldOwners = pair.Value; - - if (!newData.TryGetValue(id, out var newOwners)) - { - // REMOVED: The package ID does not exist in the new data, which means the package ID was removed. - result.Add(id, EmptyStringArray); - _logger.LogInformation( - "The package ID {ID} has been removed, with {RemovedCount} owners", - id, - oldOwners.Count); - } - } - - stopwatch.Stop(); - _telemetryService.TrackOwnerSetComparison(oldData.Count, newData.Count, result.Count, stopwatch.Elapsed); - - return result; - } - } -} - diff --git a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateOwnersCommand.cs b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateOwnersCommand.cs index 940fae196..3800125f1 100644 --- a/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateOwnersCommand.cs +++ b/src/NuGet.Services.AzureSearch/Auxiliary2AzureSearch/UpdateOwnersCommand.cs @@ -17,7 +17,7 @@ public class UpdateOwnersCommand : IAzureSearchCommand { private readonly IDatabaseAuxiliaryDataFetcher _databaseFetcher; private readonly IOwnerDataClient _ownerDataClient; - private readonly IOwnerSetComparer _ownerSetComparer; + private readonly IDataSetComparer _ownerSetComparer; private readonly ISearchDocumentBuilder _searchDocumentBuilder; private readonly ISearchIndexActionBuilder _searchIndexActionBuilder; private readonly Func _batchPusherFactory; @@ -28,7 +28,7 @@ public class UpdateOwnersCommand : IAzureSearchCommand public UpdateOwnersCommand( IDatabaseAuxiliaryDataFetcher databaseFetcher, IOwnerDataClient ownerDataClient, - IOwnerSetComparer ownerSetComparer, + IDataSetComparer ownerSetComparer, ISearchDocumentBuilder searchDocumentBuilder, ISearchIndexActionBuilder indexActionBuilder, Func batchPusherFactory, @@ -67,7 +67,7 @@ public async Task ExecuteAsync() var databaseResult = await _databaseFetcher.GetPackageIdToOwnersAsync(); _logger.LogInformation("Detecting owner changes."); - var changes = _ownerSetComparer.Compare(storageResult.Result, databaseResult); + var changes = _ownerSetComparer.CompareOwners(storageResult.Result, databaseResult); var changesBag = new ConcurrentBag>(changes.Select(x => new IdAndValue(x.Key, x.Value))); _logger.LogInformation("{Count} package IDs have owner changes.", changesBag.Count); diff --git a/src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs b/src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs index 4be4e22fc..c4c3b0bcf 100644 --- a/src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs +++ b/src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs @@ -189,6 +189,19 @@ public void TrackReadLatestIndexedPopularityTransfers(int outgoingTransfers, Tim }); } + public void TrackPopularityTransfersSetComparison(int oldCount, int newCount, int changeCount, TimeSpan elapsed) + { + _telemetryClient.TrackMetric( + Prefix + "PopularityTransfersSetComparisonSeconds", + elapsed.TotalSeconds, + new Dictionary + { + { "OldCount", oldCount.ToString() }, + { "NewCount", oldCount.ToString() }, + { "ChangeCount", oldCount.ToString() }, + }); + } + public IDisposable TrackReplaceLatestIndexedPopularityTransfers(int outogingTransfers) { return _telemetryClient.TrackDuration( diff --git a/src/NuGet.Services.AzureSearch/DependencyInjectionExtensions.cs b/src/NuGet.Services.AzureSearch/DependencyInjectionExtensions.cs index ac3a5d37c..cc34e6fcb 100644 --- a/src/NuGet.Services.AzureSearch/DependencyInjectionExtensions.cs +++ b/src/NuGet.Services.AzureSearch/DependencyInjectionExtensions.cs @@ -249,6 +249,7 @@ public static IServiceCollection AddAzureSearch( services.AddTransient(); services.AddTransient(); services.AddTransient(); + services.AddTransient(); services.AddTransient(); services.AddTransient(); services.AddTransient(); @@ -256,7 +257,6 @@ public static IServiceCollection AddAzureSearch( services.AddTransient(); services.AddTransient(); services.AddTransient(); - services.AddTransient(); services.AddTransient(); services.AddTransient(); services.AddTransient(); diff --git a/src/NuGet.Services.AzureSearch/IAzureSearchTelemetryService.cs b/src/NuGet.Services.AzureSearch/IAzureSearchTelemetryService.cs index 4c9f4050f..77d3bf183 100644 --- a/src/NuGet.Services.AzureSearch/IAzureSearchTelemetryService.cs +++ b/src/NuGet.Services.AzureSearch/IAzureSearchTelemetryService.cs @@ -20,6 +20,7 @@ public interface IAzureSearchTelemetryService void TrackOwnerSetComparison(int oldCount, int newCount, int changeCount, TimeSpan elapsed); void TrackReadLatestIndexedOwners(int packageIdCount, TimeSpan elapsed); void TrackReadLatestOwnersFromDatabase(int packageIdCount, TimeSpan elapsed); + void TrackPopularityTransfersSetComparison(int oldCount, int newCount, int changeCount, TimeSpan elapsed); void TrackReadLatestIndexedPopularityTransfers(int outgoingTransfers, TimeSpan elapsed); void TrackReadLatestVerifiedPackagesFromDatabase(int packageIdCount, TimeSpan elapsed); IDisposable TrackReplaceLatestIndexedOwners(int packageIdCount); diff --git a/src/NuGet.Services.AzureSearch/NuGet.Services.AzureSearch.csproj b/src/NuGet.Services.AzureSearch/NuGet.Services.AzureSearch.csproj index c469c6e33..61fd4951b 100644 --- a/src/NuGet.Services.AzureSearch/NuGet.Services.AzureSearch.csproj +++ b/src/NuGet.Services.AzureSearch/NuGet.Services.AzureSearch.csproj @@ -89,10 +89,10 @@ - + - + diff --git a/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/DataSetComparerFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/DataSetComparerFacts.cs new file mode 100644 index 000000000..e5f1f869d --- /dev/null +++ b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/DataSetComparerFacts.cs @@ -0,0 +1,404 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using Moq; +using Xunit; +using Xunit.Abstractions; + +namespace NuGet.Services.AzureSearch.Auxiliary2AzureSearch +{ + public class DataSetComparerFacts + { + public class CompareOwners : Facts + { + public CompareOwners(ITestOutputHelper output) : base(output) + { + } + + [Fact] + public void FindsAddedPackageIds() + { + var oldData = OwnersData("NuGet.Core: NuGet, Microsoft"); + var newData = OwnersData( + "NuGet.Core: NuGet, Microsoft", + "NuGet.Versioning: NuGet, Microsoft"); + + var changes = Target.CompareOwners(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("NuGet.Versioning", pair.Key); + Assert.Equal(new[] { "Microsoft", "NuGet" }, pair.Value); + + TelemetryService.Verify( + x => x.TrackOwnerSetComparison( + /*oldCount: */ 1, + /*newCount: */ 2, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsRemovedPackageIds() + { + var oldData = OwnersData( + "NuGet.Core: NuGet, Microsoft", + "NuGet.Versioning: NuGet, Microsoft"); + var newData = OwnersData("NuGet.Core: NuGet, Microsoft"); + + var changes = Target.CompareOwners(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("NuGet.Versioning", pair.Key); + Assert.Empty(pair.Value); + + TelemetryService.Verify( + x => x.TrackOwnerSetComparison( + /*oldCount: */ 2, + /*newCount: */ 1, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsAddedOwner() + { + var oldData = OwnersData("NuGet.Core: NuGet"); + var newData = OwnersData("NuGet.Core: NuGet, Microsoft"); + + var changes = Target.CompareOwners(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("NuGet.Core", pair.Key); + Assert.Equal(new[] { "Microsoft", "NuGet" }, pair.Value); + + TelemetryService.Verify( + x => x.TrackOwnerSetComparison( + /*oldCount: */ 1, + /*newCount: */ 1, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsRemovedOwner() + { + var oldData = OwnersData("NuGet.Core: NuGet, Microsoft"); + var newData = OwnersData("NuGet.Core: NuGet"); + + var changes = Target.CompareOwners(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("NuGet.Core", pair.Key); + Assert.Equal(new[] { "NuGet" }, pair.Value); + + TelemetryService.Verify( + x => x.TrackOwnerSetComparison( + /*oldCount: */ 1, + /*newCount: */ 1, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsOwnerWithChangedCase() + { + var oldData = OwnersData("NuGet.Core: NuGet, Microsoft"); + var newData = OwnersData("NuGet.Core: NuGet, microsoft"); + + var changes = Target.CompareOwners(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("NuGet.Core", pair.Key); + Assert.Equal(new[] { "microsoft", "NuGet" }, pair.Value); + + TelemetryService.Verify( + x => x.TrackOwnerSetComparison( + /*oldCount: */ 1, + /*newCount: */ 1, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsManyChangesAtOnce() + { + var oldData = OwnersData( + "NuGet.Core: NuGet, Microsoft", + "NuGet.Frameworks: NuGet", + "NuGet.Protocol: NuGet"); + var newData = OwnersData( + "NuGet.Core: NuGet, microsoft", + "NuGet.Versioning: NuGet", + "NuGet.Protocol: NuGet"); + + var changes = Target.CompareOwners(oldData, newData); + + Assert.Equal(3, changes.Count); + Assert.Equal(new[] { "NuGet.Core", "NuGet.Frameworks", "NuGet.Versioning" }, changes.Keys.ToArray()); + Assert.Equal(new[] { "microsoft", "NuGet" }, changes["NuGet.Core"]); + Assert.Empty(changes["NuGet.Frameworks"]); + Assert.Equal(new[] { "NuGet" }, changes["NuGet.Versioning"]); + + TelemetryService.Verify( + x => x.TrackOwnerSetComparison( + /*oldCount: */ 3, + /*newCount: */ 3, + /*changeCount: */ 3, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsNoChanges() + { + var oldData = OwnersData( + "NuGet.Core: NuGet, Microsoft", + "NuGet.Versioning: NuGet, Microsoft"); + var newData = OwnersData( + "NuGet.Core: NuGet, Microsoft", + "NuGet.Versioning: NuGet, Microsoft"); + + var changes = Target.CompareOwners(oldData, newData); + + Assert.Empty(changes); + + TelemetryService.Verify( + x => x.TrackOwnerSetComparison( + /*oldCount: */ 2, + /*newCount: */ 2, + /*changeCount: */ 0, + It.IsAny()), + Times.Once); + } + } + + public class ComparePopularityTransfers : Facts + { + public ComparePopularityTransfers(ITestOutputHelper output) : base(output) + { + } + + [Fact] + public void FindsNoChanges() + { + var oldData = TransfersData( + "PackageA: PackageB, PackageC", + "Package1: Package3, Package2"); + var newData = TransfersData( + "PackageA: PackageC, PackageB", + "Package1: Package2, Package3"); + + var changes = Target.ComparePopularityTransfers(oldData, newData); + + Assert.Empty(changes); + + TelemetryService.Verify( + x => x.TrackPopularityTransfersSetComparison( + /*oldCount: */ 2, + /*newCount: */ 2, + /*changeCount: */ 0, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsAddedTransfers() + { + var oldData = TransfersData("PackageA: PackageB, PackageC"); + var newData = TransfersData( + "PackageA: PackageB, PackageC", + "Package1: Package2, Package3"); + + var changes = Target.ComparePopularityTransfers(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("Package1", pair.Key); + Assert.Equal(new[] { "Package2", "Package3" }, pair.Value); + + TelemetryService.Verify( + x => x.TrackPopularityTransfersSetComparison( + /*oldCount: */ 1, + /*newCount: */ 2, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsRemovedTransfers() + { + var oldData = TransfersData( + "PackageA: PackageB, PackageC", + "Package1: Package2, Package3"); + var newData = TransfersData("PackageA: PackageB, PackageC"); + + var changes = Target.ComparePopularityTransfers(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("Package1", pair.Key); + Assert.Empty(pair.Value); + + TelemetryService.Verify( + x => x.TrackPopularityTransfersSetComparison( + /*oldCount: */ 2, + /*newCount: */ 1, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsAddedToPackage() + { + var oldData = TransfersData("PackageA: PackageB"); + var newData = TransfersData("PackageA: PackageB, PackageC"); + + var changes = Target.ComparePopularityTransfers(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("PackageA", pair.Key); + Assert.Equal(new[] { "PackageB", "PackageC" }, pair.Value); + + TelemetryService.Verify( + x => x.TrackPopularityTransfersSetComparison( + /*oldCount: */ 1, + /*newCount: */ 1, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsRemovedToPackage() + { + var oldData = TransfersData("PackageA: PackageB, PackageC"); + var newData = TransfersData("PackageA: PackageB"); + + var changes = Target.ComparePopularityTransfers(oldData, newData); + + var pair = Assert.Single(changes); + Assert.Equal("PackageA", pair.Key); + Assert.Equal(new[] { "PackageB" }, pair.Value); + + TelemetryService.Verify( + x => x.TrackPopularityTransfersSetComparison( + /*oldCount: */ 1, + /*newCount: */ 1, + /*changeCount: */ 1, + It.IsAny()), + Times.Once); + } + + [Fact] + public void IgnoresCaseChanges() + { + var oldData = TransfersData("PackageA: packageb, PackageC"); + var newData = TransfersData("packagea: PACKAGEB, packageC"); + + var changes = Target.ComparePopularityTransfers(oldData, newData); + + Assert.Empty(changes); + + TelemetryService.Verify( + x => x.TrackPopularityTransfersSetComparison( + /*oldCount: */ 1, + /*newCount: */ 1, + /*changeCount: */ 0, + It.IsAny()), + Times.Once); + } + + [Fact] + public void FindsManyChangesAtOnce() + { + var oldData = TransfersData( + "Package1: PackageA, PackageB", + "Package2: PackageC", + "Package3: PackageD"); + var newData = TransfersData( + "Package1: PackageA, PackageE", + "Package4: PackageC", + "Package3: Packaged"); + + var changes = Target.ComparePopularityTransfers(oldData, newData); + + Assert.Equal(3, changes.Count); + Assert.Equal(new[] { "Package1", "Package2", "Package4" }, changes.Keys.ToArray()); + Assert.Equal(new[] { "PackageA", "PackageE" }, changes["Package1"]); + Assert.Empty(changes["Package2"]); + Assert.Equal(new[] { "PackageC" }, changes["Package4"]); + + TelemetryService.Verify( + x => x.TrackPopularityTransfersSetComparison( + /*oldCount: */ 3, + /*newCount: */ 3, + /*changeCount: */ 3, + It.IsAny()), + Times.Once); + } + } + + public abstract class Facts + { + public Facts(ITestOutputHelper output) + { + TelemetryService = new Mock(); + Logger = output.GetLogger(); + + Target = new DataSetComparer( + TelemetryService.Object, + Logger); + } + + public Mock TelemetryService { get; } + public RecordingLogger Logger { get; } + public DataSetComparer Target { get; } + + /// + /// A helper to turn lines formatted like this "PackageId: OwnerA, OwnerB" into package ID to owners + /// dictionary. + /// + public SortedDictionary> OwnersData(params string[] lines) + { + var builder = new PackageIdToOwnersBuilder(Logger); + ParseData(lines, builder.Add); + return builder.GetResult(); + } + + /// + /// A helper to turn lines formatted like this "FromPackage1: ToPackage1, ToPackage2" into package ID to popularity + /// transfers dictionary. + /// + public SortedDictionary> TransfersData(params string[] lines) + { + var builder = new PackageIdToPopularityTransfersBuilder(Logger); + ParseData(lines, builder.Add); + return builder.GetResult(); + } + + private void ParseData(string[] lines, Action> add) + { + foreach (var line in lines) + { + var pieces = line.Split(new[] { ':' }, 2); + var key = pieces[0].Trim(); + var values = pieces[1] + .Split(',') + .Select(x => x.Trim()) + .Where(x => x.Length > 0) + .ToList(); + + add(key, values); + } + } + } + } +} diff --git a/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/OwnerSetComparerFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/OwnerSetComparerFacts.cs deleted file mode 100644 index 17d127952..000000000 --- a/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/OwnerSetComparerFacts.cs +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using System.Linq; -using Moq; -using Xunit; -using Xunit.Abstractions; - -namespace NuGet.Services.AzureSearch.Auxiliary2AzureSearch -{ - public class OwnerSetComparerFacts - { - public class Compare : Facts - { - public Compare(ITestOutputHelper output) : base(output) - { - } - - [Fact] - public void FindsAddedPackageIds() - { - var oldData = Data("NuGet.Core: NuGet, Microsoft"); - var newData = Data("NuGet.Core: NuGet, Microsoft", - "NuGet.Versioning: NuGet, Microsoft"); - - var changes = Target.Compare(oldData, newData); - - var pair = Assert.Single(changes); - Assert.Equal("NuGet.Versioning", pair.Key); - Assert.Equal(new[] { "Microsoft", "NuGet" }, pair.Value); - } - - - [Fact] - public void FindsRemovedPackageIds() - { - var oldData = Data("NuGet.Core: NuGet, Microsoft", - "NuGet.Versioning: NuGet, Microsoft"); - var newData = Data("NuGet.Core: NuGet, Microsoft"); - - var changes = Target.Compare(oldData, newData); - - var pair = Assert.Single(changes); - Assert.Equal("NuGet.Versioning", pair.Key); - Assert.Empty(pair.Value); - } - - [Fact] - public void FindsAddedOwner() - { - var oldData = Data("NuGet.Core: NuGet"); - var newData = Data("NuGet.Core: NuGet, Microsoft"); - - var changes = Target.Compare(oldData, newData); - - var pair = Assert.Single(changes); - Assert.Equal("NuGet.Core", pair.Key); - Assert.Equal(new[] { "Microsoft", "NuGet" }, pair.Value); - } - - [Fact] - public void FindsRemovedOwner() - { - var oldData = Data("NuGet.Core: NuGet, Microsoft"); - var newData = Data("NuGet.Core: NuGet"); - - var changes = Target.Compare(oldData, newData); - - var pair = Assert.Single(changes); - Assert.Equal("NuGet.Core", pair.Key); - Assert.Equal(new[] { "NuGet" }, pair.Value); - } - - [Fact] - public void FindsOwnerWithChangedCase() - { - var oldData = Data("NuGet.Core: NuGet, Microsoft"); - var newData = Data("NuGet.Core: NuGet, microsoft"); - - var changes = Target.Compare(oldData, newData); - - var pair = Assert.Single(changes); - Assert.Equal("NuGet.Core", pair.Key); - Assert.Equal(new[] { "microsoft", "NuGet" }, pair.Value); - } - - [Fact] - public void FindsManyChangesAtOnce() - { - var oldData = Data("NuGet.Core: NuGet, Microsoft", - "NuGet.Frameworks: NuGet", - "NuGet.Protocol: NuGet"); - var newData = Data("NuGet.Core: NuGet, microsoft", - "NuGet.Versioning: NuGet", - "NuGet.Protocol: NuGet"); - - var changes = Target.Compare(oldData, newData); - - Assert.Equal(3, changes.Count); - Assert.Equal(new[] { "NuGet.Core", "NuGet.Frameworks", "NuGet.Versioning" }, changes.Keys.ToArray()); - Assert.Equal(new[] { "microsoft", "NuGet" }, changes["NuGet.Core"]); - Assert.Empty(changes["NuGet.Frameworks"]); - Assert.Equal(new[] { "NuGet" }, changes["NuGet.Versioning"]); - } - - [Fact] - public void FindsNoChanges() - { - var oldData = Data("NuGet.Core: NuGet, Microsoft", - "NuGet.Versioning: NuGet, Microsoft"); - var newData = Data("NuGet.Core: NuGet, Microsoft", - "NuGet.Versioning: NuGet, Microsoft"); - - var changes = Target.Compare(oldData, newData); - - Assert.Empty(changes); - } - } - - public abstract class Facts - { - public Facts(ITestOutputHelper output) - { - TelemetryService = new Mock(); - Logger = output.GetLogger(); - - Target = new OwnerSetComparer( - TelemetryService.Object, - Logger); - } - - public Mock TelemetryService { get; } - public RecordingLogger Logger { get; } - public OwnerSetComparer Target { get; } - - /// - /// A helper to turn lines formatted like this "PackageId: OwnerA, OwnerB" into package ID to owners - /// dictionary. - /// - public SortedDictionary> Data(params string[] lines) - { - var builder = new PackageIdToOwnersBuilder(Logger); - foreach (var line in lines) - { - var pieces = line.Split(new[] { ':' }, 2); - var id = pieces[0].Trim(); - var usernames = pieces[1] - .Split(',') - .Select(x => x.Trim()) - .Where(x => x.Length > 0) - .ToList(); - - builder.Add(id, usernames); - } - - return builder.GetResult(); - } - } - } -} diff --git a/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/UpdateOwnersCommandFacts.cs b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/UpdateOwnersCommandFacts.cs index 0a50f66d9..6b9fe2675 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/UpdateOwnersCommandFacts.cs +++ b/tests/NuGet.Services.AzureSearch.Tests/Auxiliary2AzureSearch/UpdateOwnersCommandFacts.cs @@ -47,12 +47,12 @@ public async Task ComparesInTheRightOrder() await Target.ExecuteAsync(); OwnerSetComparer.Verify( - x => x.Compare( + x => x.CompareOwners( It.IsAny>>(), It.IsAny>>()), Times.Once); OwnerSetComparer.Verify( - x => x.Compare(StorageResult.Result, DatabaseResult), + x => x.CompareOwners(StorageResult.Result, DatabaseResult), Times.Once); } @@ -169,7 +169,7 @@ public Facts(ITestOutputHelper output) { DatabaseOwnerFetcher = new Mock(); OwnerDataClient = new Mock(); - OwnerSetComparer = new Mock(); + OwnerSetComparer = new Mock(); SearchDocumentBuilder = new Mock(); SearchIndexActionBuilder = new Mock(); Pusher = new Mock(); @@ -203,7 +203,7 @@ public Facts(ITestOutputHelper output) .Setup(x => x.ReadLatestIndexedAsync()) .ReturnsAsync(() => StorageResult); OwnerSetComparer - .Setup(x => x.Compare( + .Setup(x => x.CompareOwners( It.IsAny>>(), It.IsAny>>())) .Returns(() => Changes); @@ -225,7 +225,7 @@ public Facts(ITestOutputHelper output) public Mock DatabaseOwnerFetcher { get; } public Mock OwnerDataClient { get; } - public Mock OwnerSetComparer { get; } + public Mock OwnerSetComparer { get; } public Mock SearchDocumentBuilder { get; } public Mock SearchIndexActionBuilder { get; } public Mock Pusher { get; } 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 f62e56896..c3f5d1a9e 100644 --- a/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj +++ b/tests/NuGet.Services.AzureSearch.Tests/NuGet.Services.AzureSearch.Tests.csproj @@ -68,7 +68,7 @@ - +