From b6f14532c3955af2b497fea44ad136d992b10c96 Mon Sep 17 00:00:00 2001 From: Damon Tivel Date: Tue, 13 Nov 2018 11:13:00 -0800 Subject: [PATCH 1/3] Extract parallelized commit processing into common utility class. --- src/Catalog/BatchProcessingException.cs | 2 +- src/Catalog/CatalogCommit.cs | 41 ++ src/Catalog/CatalogCommitBatchTask.cs | 54 ++ src/Catalog/CatalogCommitBatchTasks.cs | 23 + src/Catalog/CatalogCommitItem.cs | 99 +++ src/Catalog/CatalogCommitItemBatch.cs | 47 ++ src/Catalog/CatalogCommitUtilities.cs | 457 ++++++++++++ src/Catalog/CatalogIndexEntry.cs | 113 +-- src/Catalog/CatalogIndexReader.cs | 7 +- src/Catalog/CommitCollector.cs | 63 +- src/Catalog/CreateCommitItemBatchesAsync.cs | 11 + src/Catalog/Dnx/DnxCatalogCollector.cs | 40 +- src/Catalog/Dnx/DnxMaker.cs | 2 +- src/Catalog/FetchCatalogCommitsAsync.cs | 14 + src/Catalog/GetCatalogCommitItemKey.cs | 7 + src/Catalog/Helpers/Utils.cs | 35 +- .../NuGet.Services.Metadata.Catalog.csproj | 10 + src/Catalog/ProcessCommitItemBatchAsync.cs | 17 + .../Registration/RegistrationCollector.cs | 270 +------- src/Catalog/SortingCollector.cs | 16 +- src/Catalog/SortingGraphCollector.cs | 19 +- src/Catalog/SortingIdCollector.cs | 10 +- src/Catalog/SortingIdVersionCollector.cs | 7 +- src/Catalog/Strings.Designer.cs | 18 + src/Catalog/Strings.resx | 6 + src/Ng/Jobs/MonitoringProcessorJob.cs | 4 +- src/Ng/SearchIndexFromCatalogCollector.cs | 18 +- .../ValidationCollector.cs | 6 +- src/V3PerPackage/EnqueueCollector.cs | 20 +- .../BatchProcessingExceptionTests.cs | 30 + .../CatalogCommitBatchTaskTests.cs | 79 +++ .../CatalogCommitBatchTasksTests.cs | 22 + .../CatalogCommitItemBatchTests.cs | 62 ++ tests/CatalogTests/CatalogCommitItemTests.cs | 84 +++ tests/CatalogTests/CatalogCommitTests.cs | 37 + .../CatalogCommitUtilitiesTests.cs | 653 ++++++++++++++++++ tests/CatalogTests/CatalogIndexEntryTests.cs | 152 ++-- tests/CatalogTests/CatalogTests.csproj | 15 +- tests/NgTests/DnxCatalogCollectorTests.cs | 31 +- tests/NgTests/Infrastructure/TestUtility.cs | 48 ++ .../FixPackageHashHandlerFacts.cs | 18 +- .../PackagesContainerCatalogProcessorFacts.cs | 7 +- .../ValidatePackageHashHandlerFacts.cs | 20 +- .../PackageMonitoringStatusServiceTests.cs | 5 +- .../NgTests/PackageTimestampMetadataTests.cs | 14 +- .../NgTests/SortingIdVersionCollectorTests.cs | 39 +- .../CatalogAggregateValidatorFacts.cs | 3 +- .../PackageHasSignatureValidatorFacts.cs | 41 +- ...PackageIsRepositorySignedValidatorFacts.cs | 9 +- .../PackageValidatorContextTests.cs | 4 +- 50 files changed, 2180 insertions(+), 629 deletions(-) create mode 100644 src/Catalog/CatalogCommit.cs create mode 100644 src/Catalog/CatalogCommitBatchTask.cs create mode 100644 src/Catalog/CatalogCommitBatchTasks.cs create mode 100644 src/Catalog/CatalogCommitItem.cs create mode 100644 src/Catalog/CatalogCommitItemBatch.cs create mode 100644 src/Catalog/CatalogCommitUtilities.cs create mode 100644 src/Catalog/CreateCommitItemBatchesAsync.cs create mode 100644 src/Catalog/FetchCatalogCommitsAsync.cs create mode 100644 src/Catalog/GetCatalogCommitItemKey.cs create mode 100644 src/Catalog/ProcessCommitItemBatchAsync.cs create mode 100644 tests/CatalogTests/BatchProcessingExceptionTests.cs create mode 100644 tests/CatalogTests/CatalogCommitBatchTaskTests.cs create mode 100644 tests/CatalogTests/CatalogCommitBatchTasksTests.cs create mode 100644 tests/CatalogTests/CatalogCommitItemBatchTests.cs create mode 100644 tests/CatalogTests/CatalogCommitItemTests.cs create mode 100644 tests/CatalogTests/CatalogCommitTests.cs create mode 100644 tests/CatalogTests/CatalogCommitUtilitiesTests.cs diff --git a/src/Catalog/BatchProcessingException.cs b/src/Catalog/BatchProcessingException.cs index 23a587b63..cdc135cf1 100644 --- a/src/Catalog/BatchProcessingException.cs +++ b/src/Catalog/BatchProcessingException.cs @@ -8,7 +8,7 @@ namespace NuGet.Services.Metadata.Catalog public sealed class BatchProcessingException : Exception { public BatchProcessingException(Exception inner) - : base(Strings.BatchProcessingFailure) + : base(Strings.BatchProcessingFailure, inner ?? throw new ArgumentNullException(nameof(inner))) { } } diff --git a/src/Catalog/CatalogCommit.cs b/src/Catalog/CatalogCommit.cs new file mode 100644 index 000000000..e20f3073c --- /dev/null +++ b/src/Catalog/CatalogCommit.cs @@ -0,0 +1,41 @@ +// 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 Newtonsoft.Json.Linq; + +namespace NuGet.Services.Metadata.Catalog +{ + /// + /// Represents a single catalog commit. + /// + public sealed class CatalogCommit : IComparable + { + private CatalogCommit(DateTime commitTimeStamp, Uri uri) + { + CommitTimeStamp = commitTimeStamp; + Uri = uri; + } + + public DateTime CommitTimeStamp { get; } + public Uri Uri { get; } + + public int CompareTo(object obj) + { + return CommitTimeStamp.CompareTo(((CatalogCommit)obj).CommitTimeStamp); + } + + public static CatalogCommit Create(JObject commit) + { + if (commit == null) + { + throw new ArgumentNullException(nameof(commit)); + } + + var commitTimeStamp = Utils.Deserialize(commit, "commitTimeStamp"); + var uri = Utils.Deserialize(commit, "@id"); + + return new CatalogCommit(commitTimeStamp, uri); + } + } +} \ No newline at end of file diff --git a/src/Catalog/CatalogCommitBatchTask.cs b/src/Catalog/CatalogCommitBatchTask.cs new file mode 100644 index 000000000..ba320c4e5 --- /dev/null +++ b/src/Catalog/CatalogCommitBatchTask.cs @@ -0,0 +1,54 @@ +// 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.Threading.Tasks; + +namespace NuGet.Services.Metadata.Catalog +{ + /// + /// Represents an asynchrononous task associated with catalog changes for a specific commit item key + /// and potentially spanning multiple commits. + /// + public sealed class CatalogCommitBatchTask + { + /// + /// Initializes a instance. + /// + /// The minimum commit timestamp for commit items with . + /// A unique key. + /// Thrown if is null, empty, + /// or whitespace. + public CatalogCommitBatchTask(DateTime minCommitTimeStamp, string key) + { + if (string.IsNullOrWhiteSpace(key)) + { + throw new ArgumentException(Strings.ArgumentMustNotBeNullEmptyOrWhitespace, nameof(key)); + } + + MinCommitTimeStamp = minCommitTimeStamp; + Key = key; + } + + public DateTime MinCommitTimeStamp { get; } + public string Key { get; } + public Task Task { get; set; } + + public override int GetHashCode() + { + return Key.GetHashCode(); + } + + public override bool Equals(object obj) + { + var other = obj as CatalogCommitBatchTask; + + if (ReferenceEquals(other, null)) + { + return false; + } + + return GetHashCode() == other.GetHashCode(); + } + } +} \ No newline at end of file diff --git a/src/Catalog/CatalogCommitBatchTasks.cs b/src/Catalog/CatalogCommitBatchTasks.cs new file mode 100644 index 000000000..44b573643 --- /dev/null +++ b/src/Catalog/CatalogCommitBatchTasks.cs @@ -0,0 +1,23 @@ +// 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; + +namespace NuGet.Services.Metadata.Catalog +{ + /// + /// Represents a set of that share the same minimum commit timestamp. + /// + public sealed class CatalogCommitBatchTasks + { + public CatalogCommitBatchTasks(DateTime commitTimeStamp) + { + BatchTasks = new HashSet(); + CommitTimeStamp = commitTimeStamp; + } + + public HashSet BatchTasks { get; } + public DateTime CommitTimeStamp { get; } + } +} \ No newline at end of file diff --git a/src/Catalog/CatalogCommitItem.cs b/src/Catalog/CatalogCommitItem.cs new file mode 100644 index 000000000..059be9e90 --- /dev/null +++ b/src/Catalog/CatalogCommitItem.cs @@ -0,0 +1,99 @@ +// 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.Globalization; +using System.Linq; +using Newtonsoft.Json.Linq; +using NuGet.Packaging.Core; +using NuGet.Versioning; + +namespace NuGet.Services.Metadata.Catalog +{ + /// + /// Represents a single item in a catalog commit. + /// + public sealed class CatalogCommitItem : IComparable + { + private const string _typeKeyword = "@type"; + + private CatalogCommitItem( + Uri uri, + string commitId, + DateTime commitTimeStamp, + IReadOnlyList types, + IReadOnlyList typeUris, + PackageIdentity packageIdentity) + { + Uri = uri; + CommitId = commitId; + CommitTimeStamp = commitTimeStamp; + PackageIdentity = packageIdentity; + Types = types; + TypeUris = typeUris; + } + + public Uri Uri { get; } + public DateTime CommitTimeStamp { get; } + public string CommitId { get; } + public PackageIdentity PackageIdentity { get; } + public IReadOnlyList Types { get; } + public IReadOnlyList TypeUris { get; } + + public int CompareTo(object obj) + { + return CommitTimeStamp.CompareTo(((CatalogCommitItem)obj).CommitTimeStamp); + } + + public static CatalogCommitItem Create(JObject context, JObject commitItem) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (commitItem == null) + { + throw new ArgumentNullException(nameof(commitItem)); + } + + var commitTimeStamp = Utils.Deserialize(commitItem, "commitTimeStamp"); + var commitId = Utils.Deserialize(commitItem, "commitId"); + var idUri = Utils.Deserialize(commitItem, "@id"); + var packageId = Utils.Deserialize(commitItem, "nuget:id"); + var packageVersion = Utils.Deserialize(commitItem, "nuget:version"); + var packageIdentity = new PackageIdentity(packageId, new NuGetVersion(packageVersion)); + var types = GetTypes(commitItem).ToArray(); + + if (!types.Any()) + { + throw new ArgumentException( + string.Format(CultureInfo.InvariantCulture, Strings.NonEmptyPropertyValueRequired, _typeKeyword), + nameof(commitItem)); + } + + var typeUris = types.Select(type => Utils.Expand(context, type)).ToArray(); + + return new CatalogCommitItem(idUri, commitId, commitTimeStamp, types, typeUris, packageIdentity); + } + + private static IEnumerable GetTypes(JObject commitItem) + { + if (commitItem.TryGetValue(_typeKeyword, out var value)) + { + if (value is JArray) + { + foreach (JToken typeToken in ((JArray)value).Values()) + { + yield return typeToken.ToString(); + } + } + else + { + yield return value.ToString(); + } + } + } + } +} \ No newline at end of file diff --git a/src/Catalog/CatalogCommitItemBatch.cs b/src/Catalog/CatalogCommitItemBatch.cs new file mode 100644 index 000000000..63bb43141 --- /dev/null +++ b/src/Catalog/CatalogCommitItemBatch.cs @@ -0,0 +1,47 @@ +// 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; + +namespace NuGet.Services.Metadata.Catalog +{ + /// + /// Represents a group of . + /// Items may span multiple commits but are grouped on common criteria (e.g.: lower-cased package ID). + /// + public sealed class CatalogCommitItemBatch + { + /// + /// Initializes a instance. + /// + /// A commit timestamp relevant to . + /// For example, the minimum or maximum commit timestamp amongst all , + /// depending on the . + /// A unique key for all items in a batch. This is used for parallelization and may be + /// null if parallelization is not used. + /// An enumerable of . Items may span multiple commits. + /// Thrown if is either null or empty. + public CatalogCommitItemBatch(DateTime commitTimeStamp, string key, IEnumerable items) + { + if (items == null || !items.Any()) + { + throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(items)); + } + + CommitTimeStamp = commitTimeStamp; + Key = key; + + var list = items.ToList(); + + list.Sort(); + + Items = list; + } + + public DateTime CommitTimeStamp { get; } + public IReadOnlyList Items { get; } + public string Key { get; } + } +} \ No newline at end of file diff --git a/src/Catalog/CatalogCommitUtilities.cs b/src/Catalog/CatalogCommitUtilities.cs new file mode 100644 index 000000000..008a7065d --- /dev/null +++ b/src/Catalog/CatalogCommitUtilities.cs @@ -0,0 +1,457 @@ +// 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 System.Threading; +using System.Threading.Tasks; +using Newtonsoft.Json.Linq; +using NuGet.Common; + +namespace NuGet.Services.Metadata.Catalog +{ + public static class CatalogCommitUtilities + { + /// + /// Creates an enumerable of instances. + /// + /// + /// A instance contains only the latest commit for each package identity. + /// + /// An enumerable of . + /// A function that returns a key for a . + /// An enumerable of . + /// Thrown if is null. + /// Thrown if is null. + public static IEnumerable CreateCommitItemBatches( + IEnumerable catalogItems, + GetCatalogCommitItemKey getCatalogCommitItemKey) + { + if (catalogItems == null) + { + throw new ArgumentNullException(nameof(catalogItems)); + } + + if (getCatalogCommitItemKey == null) + { + throw new ArgumentNullException(nameof(getCatalogCommitItemKey)); + } + + var catalogItemsGroups = catalogItems + .GroupBy(catalogItem => getCatalogCommitItemKey(catalogItem)); + + foreach (var catalogItemsGroup in catalogItemsGroups) + { + var key = catalogItemsGroup.Key; + + // Before filtering out all but the latest commit for each package identity, determine the earliest + // commit timestamp for all items in this batch. This timestamp is important for processing commits + // in chronological order. + var minCommitTimeStamp = catalogItemsGroup.Select(commitItem => commitItem.CommitTimeStamp).Min(); + var catalogItemsWithOnlyLatestCommitForEachPackageIdentity = catalogItemsGroup + .GroupBy(commitItem => new + { + PackageId = commitItem.PackageIdentity.Id.ToLowerInvariant(), + PackageVersion = commitItem.PackageIdentity.Version.ToNormalizedString().ToLowerInvariant() + }) + .Select(group => group.OrderBy(item => item.CommitTimeStamp).Last()); + + yield return new CatalogCommitItemBatch( + minCommitTimeStamp, + key, + catalogItemsWithOnlyLatestCommitForEachPackageIdentity); + } + } + + /// + /// Generate a map of commit timestamps to commit batch tasks. + /// + /// + /// Where P represents a package and T a commit timestamp, suppose the following catalog commits: + /// + /// P₀ P₁ P₂ P₃ + /// ^ ------------------ + /// | T₃ T₃ + /// | T₂ + /// | T₁ T₁ + /// time T₀ T₀ + /// + /// For a fixed catalog commit timestamp range (e.g.: T₀-T₃), each column will contain all relevant + /// catalog commits for its corresponding package. + /// + /// Each column is represented by a instance. Each group of columns + /// sharing the same minimum commit timestamp is represented by a instance. + /// + /// For the example above, this method would return a map with the following entries + /// + /// { T₀, new CatalogCommitBatchTasks(T₀, + /// new[] + /// { + /// new CatalogCommitBatchTask(T₀, P₀, [T₀, T₁, T₃]), + /// new CatalogCommitBatchTask(T₀, P₁, [T₀, T₃]) + /// } + /// }, + /// { T₁, new CatalogCommitBatchTasks(T₁, + /// new[] + /// { + /// new CatalogCommitBatchTask(T₁, P₂, [T₁]) + /// } + /// }, + /// { T₂, new CatalogCommitBatchTasks(T₂, + /// new[] + /// { + /// new CatalogCommitBatchTask(T₂, P₃, [P₃]) + /// } + /// } + /// + /// Note #1: typically only the latest commit for each package identity need be processed. This is true for + /// Catalog2Dnx and Catalog2Registration jobs. In those cases all but the latest commits for each package + /// identity should be excluded BEFORE calling this method. However, ... + /// + /// Note #2: it is assumed that each is the minimum + /// unprocessed commit timestamp for package ID (not identity), even + /// if contains no item for that commit timestamp (because of Note #1 above). + /// + /// For the example above with these notes applied, this method would return a map with the following entries: + /// + /// { T₀, new CatalogCommitBatchTasks(T₀, + /// new[] + /// { + /// new CatalogCommitBatchTask(T₀, P₀, [T₃]), // P₀-T₀ and P₀-T₁ have been skipped + /// new CatalogCommitBatchTask(T₀, P₁, [T₃])] }, // P₁-T₀ has been skipped + /// } + /// }, + /// { T₁, new CatalogCommitBatchTasks(T₁, + /// new[] + /// { + /// new CatalogCommitBatchTask(T₁, P₂, [T₁]) + /// } + /// }, + /// { T₂, new CatalogCommitBatchTasks(T₂, + /// new[] + /// { + /// new CatalogCommitBatchTask(T₂, P₃, [P₃]) + /// } + /// } + /// + /// An enumerable of instances. + /// A map of commit timestamps to commit batch tasks. + /// Thrown if is either null or empty. + public static SortedDictionary CreateCommitBatchTasksMap( + IEnumerable batches) + { + if (batches == null || !batches.Any()) + { + throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(batches)); + } + + var map = new SortedDictionary(); + + foreach (var batch in batches) + { + var minCommitTimeStamp = batch.CommitTimeStamp; + var batchTask = new CatalogCommitBatchTask(minCommitTimeStamp, batch.Key); + + CatalogCommitBatchTasks commitBatchTasks; + + if (!map.TryGetValue(minCommitTimeStamp, out commitBatchTasks)) + { + commitBatchTasks = new CatalogCommitBatchTasks(minCommitTimeStamp); + + map[minCommitTimeStamp] = commitBatchTasks; + } + + commitBatchTasks.BatchTasks.Add(batchTask); + } + + return map; + } + + public static void DequeueBatchesWhileMatches( + Queue batches, + Func isMatch) + { + if (batches == null) + { + throw new ArgumentNullException(nameof(batches)); + } + + if (isMatch == null) + { + throw new ArgumentNullException(nameof(isMatch)); + } + + CatalogCommitBatchTask batch; + + while ((batch = batches.FirstOrDefault()) != null) + { + if (isMatch(batch)) + { + batches.Dequeue(); + } + else + { + break; + } + } + } + + public static void EnqueueBatchesIfNoFailures( + CollectorHttpClient client, + JToken context, + SortedDictionary commitBatchTasksMap, + Queue unprocessedBatches, + Queue processingBatches, + CatalogCommitItemBatch lastBatch, + int maxConcurrentBatches, + ProcessCommitItemBatchAsync processCommitItemBatchAsync, + CancellationToken cancellationToken) + { + if (client == null) + { + throw new ArgumentNullException(nameof(client)); + } + + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (commitBatchTasksMap == null) + { + throw new ArgumentNullException(nameof(commitBatchTasksMap)); + } + + if (unprocessedBatches == null) + { + throw new ArgumentNullException(nameof(unprocessedBatches)); + } + + if (processingBatches == null) + { + throw new ArgumentNullException(nameof(processingBatches)); + } + + if (lastBatch == null) + { + throw new ArgumentNullException(nameof(lastBatch)); + } + + if (maxConcurrentBatches < 1) + { + throw new ArgumentOutOfRangeException( + nameof(maxConcurrentBatches), + maxConcurrentBatches, + string.Format(Strings.ArgumentOutOfRange, 1, int.MaxValue)); + } + + if (processCommitItemBatchAsync == null) + { + throw new ArgumentNullException(nameof(processCommitItemBatchAsync)); + } + + var hasAnyBatchFailed = processingBatches.Any(batch => batch.Task.IsFaulted || batch.Task.IsCanceled); + + if (hasAnyBatchFailed) + { + return; + } + + var batchesToEnqueue = Math.Min( + maxConcurrentBatches - processingBatches.Count(batch => !batch.Task.IsCompleted), + unprocessedBatches.Count); + + for (var i = 0; i < batchesToEnqueue; ++i) + { + var batch = unprocessedBatches.Dequeue(); + + var batchTask = commitBatchTasksMap[batch.CommitTimeStamp].BatchTasks + .Single(bt => bt.Key == batch.Key); + + batchTask.Task = processCommitItemBatchAsync(client, context, batch.Key, batch, lastBatch, cancellationToken); + + processingBatches.Enqueue(batchTask); + } + } + + internal static async Task FetchAsync( + CollectorHttpClient client, + ReadWriteCursor front, + ReadCursor back, + FetchCatalogCommitsAsync fetchCatalogCommitsAsync, + GetCatalogCommitItemKey getCatalogCommitItemKey, + CreateCommitItemBatchesAsync createCommitItemBatchesAsync, + ProcessCommitItemBatchAsync processCommitItemBatchAsync, + int maxConcurrentBatches, + string typeName, + CancellationToken cancellationToken) + { + IEnumerable rootItems = await fetchCatalogCommitsAsync(client, front, cancellationToken); + + var hasAnyBatchFailed = false; + var hasAnyBatchBeenProcessed = false; + + foreach (CatalogCommit rootItem in rootItems) + { + JObject page = await client.GetJObjectAsync(rootItem.Uri, cancellationToken); + var context = (JObject)page["@context"]; + CatalogCommitItemBatch[] batches = await CreateBatchesForAllAvailableItemsInPageAsync(front, back, page, context, createCommitItemBatchesAsync); + + if (!batches.Any()) + { + continue; + } + + DateTime maxCommitTimeStamp = GetMaxCommitTimeStamp(batches); + SortedDictionary commitBatchTasksMap = CreateCommitBatchTasksMap(batches); + + var unprocessedBatches = new Queue(batches); + var processingBatches = new Queue(); + + CatalogCommitItemBatch lastBatch = unprocessedBatches.LastOrDefault(); + var exceptions = new List(); + + EnqueueBatchesIfNoFailures( + client, + context, + commitBatchTasksMap, + unprocessedBatches, + processingBatches, + lastBatch, + maxConcurrentBatches, + processCommitItemBatchAsync, + cancellationToken); + + while (processingBatches.Any()) + { + var activeTasks = processingBatches.Where(batch => !batch.Task.IsCompleted) + .Select(batch => batch.Task) + .DefaultIfEmpty(Task.CompletedTask); + + await Task.WhenAny(activeTasks); + + DateTime? newCommitTimeStamp = null; + + while (!hasAnyBatchFailed && commitBatchTasksMap.Any()) + { + var commitBatchTasks = commitBatchTasksMap.First().Value; + var isCommitFullyProcessed = commitBatchTasks.BatchTasks.All(batch => batch.Task != null && batch.Task.IsCompleted); + + if (!isCommitFullyProcessed) + { + break; + } + + var isCommitSuccessfullyProcessed = commitBatchTasks.BatchTasks.All(batch => batch.Task.Status == TaskStatus.RanToCompletion); + + if (isCommitSuccessfullyProcessed) + { + // If there were multiple successfully processed commits, keep track of the most recent one + // and update the front cursor later after determining the latest successful commit timestamp + // to use. Updating the cursor here for each commit can be very slow. + newCommitTimeStamp = commitBatchTasks.CommitTimeStamp; + + DequeueBatchesWhileMatches(processingBatches, batch => batch.MinCommitTimeStamp == newCommitTimeStamp.Value); + + commitBatchTasksMap.Remove(newCommitTimeStamp.Value); + + if (!commitBatchTasksMap.Any()) + { + if (maxCommitTimeStamp > newCommitTimeStamp) + { + // Although all commits for the current page have been successfully processed, the + // current CatalogCommitBatchTasks.CommitTimeStamp value is not the maximum commit + // timestamp processed. + newCommitTimeStamp = maxCommitTimeStamp; + } + } + } + else // Canceled or Failed + { + hasAnyBatchFailed = true; + + exceptions.AddRange( + commitBatchTasks.BatchTasks + .Select(batch => batch.Task) + .Where(task => (task.IsFaulted || task.IsCanceled) && task.Exception != null) + .Select(task => ExceptionUtilities.Unwrap(task.Exception))); + } + } + + if (newCommitTimeStamp.HasValue) + { + front.Value = newCommitTimeStamp.Value; + + await front.SaveAsync(cancellationToken); + + Trace.TraceInformation($"{typeName}.{nameof(FetchAsync)} {nameof(front)}.{nameof(front.Value)} saved since timestamp changed from previous: {{0}}", front); + } + + if (hasAnyBatchFailed) + { + DequeueBatchesWhileMatches(processingBatches, batch => batch.Task.IsCompleted); + } + + hasAnyBatchBeenProcessed = true; + + EnqueueBatchesIfNoFailures( + client, + context, + commitBatchTasksMap, + unprocessedBatches, + processingBatches, + lastBatch, + maxConcurrentBatches, + processCommitItemBatchAsync, + cancellationToken); + } + + if (hasAnyBatchFailed) + { + var innerException = exceptions.Count == 1 ? exceptions.Single() : new AggregateException(exceptions); + + throw new BatchProcessingException(innerException); + } + } + + return hasAnyBatchBeenProcessed; + } + + public static string GetPackageIdKey(CatalogCommitItem item) + { + if (item == null) + { + throw new ArgumentNullException(nameof(item)); + } + + return item.PackageIdentity.Id.ToLowerInvariant(); + } + + private static async Task CreateBatchesForAllAvailableItemsInPageAsync( + ReadWriteCursor front, + ReadCursor back, + JObject page, + JObject context, + CreateCommitItemBatchesAsync createCommitItemBatchesAsync) + { + IEnumerable commitItems = page["items"] + .Select(item => CatalogCommitItem.Create(context, (JObject)item)) + .Where(item => item.CommitTimeStamp > front.Value && item.CommitTimeStamp <= back.Value); + + IEnumerable batches = await createCommitItemBatchesAsync(commitItems); + + return batches + .OrderBy(batch => batch.CommitTimeStamp) + .ToArray(); + } + + private static DateTime GetMaxCommitTimeStamp(CatalogCommitItemBatch[] batches) + { + return batches.SelectMany(batch => batch.Items) + .Select(item => item.CommitTimeStamp) + .Max(); + } + } +} \ No newline at end of file diff --git a/src/Catalog/CatalogIndexEntry.cs b/src/Catalog/CatalogIndexEntry.cs index 3ece5ce4a..6715bc46a 100644 --- a/src/Catalog/CatalogIndexEntry.cs +++ b/src/Catalog/CatalogIndexEntry.cs @@ -3,51 +3,44 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.Linq; using Newtonsoft.Json; -using Newtonsoft.Json.Linq; +using NuGet.Packaging.Core; using NuGet.Versioning; namespace NuGet.Services.Metadata.Catalog { public sealed class CatalogIndexEntry : IComparable { - private static readonly CatalogIndexEntryDateComparer _commitTimeStampComparer = new CatalogIndexEntryDateComparer(); - [JsonConstructor] private CatalogIndexEntry() { Types = Enumerable.Empty(); } - public CatalogIndexEntry(Uri uri, string type, string commitId, DateTime commitTs, string id, NuGetVersion version) + public CatalogIndexEntry( + Uri uri, + string type, + string commitId, + DateTime commitTs, + PackageIdentity packageIdentity) { - Uri = uri ?? throw new ArgumentNullException(nameof(uri)); - if (string.IsNullOrWhiteSpace(type)) { - throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(type)); - } - - Types = new[] { type }; - IsDelete = type == "nuget:PackageDelete"; - - if (string.IsNullOrWhiteSpace(commitId)) - { - throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(commitId)); + throw new ArgumentException(Strings.ArgumentMustNotBeNullEmptyOrWhitespace, nameof(type)); } - CommitId = commitId; - CommitTimeStamp = commitTs; - - if (string.IsNullOrWhiteSpace(id)) - { - throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(id)); - } + Initialize(uri, new[] { type }, commitId, commitTs, packageIdentity); + } - Id = id; - Version = version ?? throw new ArgumentNullException(nameof(version)); + public CatalogIndexEntry( + Uri uri, + IReadOnlyList types, + string commitId, + DateTime commitTs, + PackageIdentity packageIdentity) + { + Initialize(uri, types, commitId, commitTs, packageIdentity); } [JsonProperty("@id")] @@ -76,7 +69,7 @@ public CatalogIndexEntry(Uri uri, string type, string commitId, DateTime commitT public NuGetVersion Version { get; private set; } [JsonIgnore] - public bool IsDelete { get; } + public bool IsDelete { get; private set; } public int CompareTo(CatalogIndexEntry other) { @@ -85,37 +78,61 @@ public int CompareTo(CatalogIndexEntry other) throw new ArgumentNullException(nameof(other)); } - return _commitTimeStampComparer.Compare(this, other); + return CommitTimeStamp.CompareTo(other.CommitTimeStamp); } - public static CatalogIndexEntry Create(JToken token) + public static CatalogIndexEntry Create(CatalogCommitItem commitItem) { - if (token == null) + if (commitItem == null) { - throw new ArgumentNullException(nameof(token)); + throw new ArgumentNullException(nameof(commitItem)); } - var uri = new Uri(token["@id"].ToString()); - var type = token["@type"].ToString(); - var commitId = token["commitId"].ToString(); - var commitTimeStamp = DateTime.ParseExact( - token["commitTimeStamp"].ToString(), - "yyyy-MM-ddTHH:mm:ss.FFFFFFFZ", - DateTimeFormatInfo.CurrentInfo, - DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal); - - var packageId = token["nuget:id"].ToString(); - var packageVersion = NuGetVersion.Parse(token["nuget:version"].ToString()); - - return new CatalogIndexEntry(uri, type, commitId, commitTimeStamp, packageId, packageVersion); + return new CatalogIndexEntry( + commitItem.Uri, + commitItem.Types, + commitItem.CommitId, + commitItem.CommitTimeStamp, + commitItem.PackageIdentity); } - } - public class CatalogIndexEntryDateComparer : IComparer - { - public int Compare(CatalogIndexEntry x, CatalogIndexEntry y) + private void Initialize( + Uri uri, + IReadOnlyList types, + string commitId, + DateTime commitTs, + PackageIdentity packageIdentity) { - return x.CommitTimeStamp.CompareTo(y.CommitTimeStamp); + Uri = uri ?? throw new ArgumentNullException(nameof(uri)); + + if (types == null || !types.Any()) + { + throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(types)); + } + + if (types.Any(type => string.IsNullOrWhiteSpace(type))) + { + throw new ArgumentException(Strings.ArgumentMustNotBeNullEmptyOrWhitespace, nameof(types)); + } + + Types = types; + IsDelete = types.Any(type => type == "nuget:PackageDelete"); + + if (string.IsNullOrWhiteSpace(commitId)) + { + throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(commitId)); + } + + CommitId = commitId; + CommitTimeStamp = commitTs; + + if (packageIdentity == null) + { + throw new ArgumentNullException(nameof(packageIdentity)); + } + + Id = packageIdentity.Id; + Version = packageIdentity.Version; } } } \ No newline at end of file diff --git a/src/Catalog/CatalogIndexReader.cs b/src/Catalog/CatalogIndexReader.cs index 0aaa60d02..e33b5a4a9 100644 --- a/src/Catalog/CatalogIndexReader.cs +++ b/src/Catalog/CatalogIndexReader.cs @@ -9,6 +9,7 @@ using System.Net; using System.Threading.Tasks; using Newtonsoft.Json.Linq; +using NuGet.Packaging.Core; using NuGet.Versioning; namespace NuGet.Services.Metadata.Catalog @@ -84,6 +85,7 @@ private async Task ProcessPageUris(ConcurrentBag pageUriBag, ConcurrentBag< var commitId = interner.Intern(item["commitId"].ToString()); var nugetId = interner.Intern(item["nuget:id"].ToString()); var nugetVersion = interner.Intern(item["nuget:version"].ToString()); + var packageIdentity = new PackageIdentity(nugetId, NuGetVersion.Parse(nugetVersion)); // No string is directly operated on here. var commitTimeStamp = item["commitTimeStamp"].ToObject(); @@ -93,12 +95,11 @@ private async Task ProcessPageUris(ConcurrentBag pageUriBag, ConcurrentBag< type, commitId, commitTimeStamp, - nugetId, - NuGetVersion.Parse(nugetVersion)); + packageIdentity); entries.Add(entry); } } } } -} +} \ No newline at end of file diff --git a/src/Catalog/CommitCollector.cs b/src/Catalog/CommitCollector.cs index d483d7d1f..e55c87034 100644 --- a/src/Catalog/CommitCollector.cs +++ b/src/Catalog/CommitCollector.cs @@ -29,19 +29,19 @@ protected override async Task FetchAsync( ReadCursor back, CancellationToken cancellationToken) { - IEnumerable catalogItems = await FetchCatalogItemsAsync(client, front, cancellationToken); + IEnumerable commits = await FetchCatalogCommitsAsync(client, front, cancellationToken); bool acceptNextBatch = false; - foreach (CatalogItem catalogItem in catalogItems) + foreach (CatalogCommit commit in commits) { - JObject page = await client.GetJObjectAsync(catalogItem.Uri, cancellationToken); + JObject page = await client.GetJObjectAsync(commit.Uri, cancellationToken); JToken context = null; page.TryGetValue("@context", out context); var batches = await CreateBatchesAsync(page["items"] - .Select(item => new CatalogItem((JObject)item)) + .Select(item => CatalogCommitItem.Create((JObject)context, (JObject)item)) .Where(item => item.CommitTimeStamp > front.Value && item.CommitTimeStamp <= back.Value)); var orderedBatches = batches @@ -70,7 +70,7 @@ protected override async Task FetchAsync( { acceptNextBatch = await OnProcessBatchAsync( client, - batch.Items.Select(item => item.Value), + batch.Items, context, batch.CommitTimeStamp, batch.CommitTimeStamp == lastBatch.CommitTimeStamp, @@ -104,7 +104,7 @@ protected override async Task FetchAsync( return acceptNextBatch; } - protected async Task> FetchCatalogItemsAsync( + protected async Task> FetchCatalogCommitsAsync( CollectorHttpClient client, ReadWriteCursor front, CancellationToken cancellationToken) @@ -118,67 +118,32 @@ protected async Task> FetchCatalogItemsAsync( root = await client.GetJObjectAsync(Index, cancellationToken); } - IEnumerable rootItems = root["items"] - .Select(item => new CatalogItem((JObject)item)) + IEnumerable commits = root["items"] + .Select(item => CatalogCommit.Create((JObject)item)) .Where(item => item.CommitTimeStamp > front.Value) .OrderBy(item => item.CommitTimeStamp); - return rootItems; + return commits; } - protected virtual Task> CreateBatchesAsync(IEnumerable catalogItems) + protected virtual Task> CreateBatchesAsync(IEnumerable catalogItems) { + const string NullKey = null; + var batches = catalogItems .GroupBy(item => item.CommitTimeStamp) .OrderBy(group => group.Key) - .Select(group => new CatalogItemBatch(group.Key, group)); + .Select(group => new CatalogCommitItemBatch(group.Key, NullKey, group)); return Task.FromResult(batches); } protected abstract Task OnProcessBatchAsync( CollectorHttpClient client, - IEnumerable items, + IEnumerable items, JToken context, DateTime commitTimeStamp, bool isLastBatch, CancellationToken cancellationToken); - - protected class CatalogItemBatch : IComparable - { - public CatalogItemBatch(DateTime commitTimeStamp, IEnumerable items) - { - CommitTimeStamp = commitTimeStamp; - Items = items.ToList(); - Items.Sort(); - } - - public DateTime CommitTimeStamp { get; } - public List Items { get; } - - public int CompareTo(object obj) - { - return CommitTimeStamp.CompareTo(((CatalogItem)obj).CommitTimeStamp); - } - } - - protected class CatalogItem : IComparable - { - public CatalogItem(JObject value) - { - CommitTimeStamp = value["commitTimeStamp"].ToObject(); - Uri = value["@id"].ToObject(); - Value = value; - } - - public DateTime CommitTimeStamp { get; } - public Uri Uri { get; } - public JObject Value { get; } - - public int CompareTo(object obj) - { - return CommitTimeStamp.CompareTo(((CatalogItem)obj).CommitTimeStamp); - } - } } } \ No newline at end of file diff --git a/src/Catalog/CreateCommitItemBatchesAsync.cs b/src/Catalog/CreateCommitItemBatchesAsync.cs new file mode 100644 index 000000000..d95efee77 --- /dev/null +++ b/src/Catalog/CreateCommitItemBatchesAsync.cs @@ -0,0 +1,11 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace NuGet.Services.Metadata.Catalog +{ + internal delegate Task> CreateCommitItemBatchesAsync( + IEnumerable catalogItems); +} \ No newline at end of file diff --git a/src/Catalog/Dnx/DnxCatalogCollector.cs b/src/Catalog/Dnx/DnxCatalogCollector.cs index 3af28b824..d7feb131a 100644 --- a/src/Catalog/Dnx/DnxCatalogCollector.cs +++ b/src/Catalog/Dnx/DnxCatalogCollector.cs @@ -57,18 +57,13 @@ public DnxCatalogCollector( protected override async Task OnProcessBatchAsync( CollectorHttpClient client, - IEnumerable items, + IEnumerable items, JToken context, DateTime commitTimeStamp, bool isLastBatch, CancellationToken cancellationToken) { - var catalogEntries = items.Select( - item => new CatalogEntry( - item["nuget:id"].ToString().ToLowerInvariant(), - NuGetVersionUtility.NormalizeVersion(item["nuget:version"].ToString()).ToLowerInvariant(), - item["@type"].ToString().Replace("nuget:", Schema.Prefixes.NuGet), - item)) + var catalogEntries = items.Select(item => CatalogEntry.Create(item)) .ToList(); // Sanity check: a single catalog batch should not contain multiple entries for the same package ID and version. @@ -95,7 +90,7 @@ await catalogEntries.ForEachAsync(_maxDegreeOfParallelism, async catalogEntry => var packageId = catalogEntry.PackageId; var normalizedPackageVersion = catalogEntry.NormalizedPackageVersion; - if (catalogEntry.EntryType == Schema.DataTypes.PackageDetails.ToString()) + if (catalogEntry.Type.AbsoluteUri == Schema.DataTypes.PackageDetails.AbsoluteUri) { var properties = GetTelemetryProperties(catalogEntry); @@ -138,7 +133,7 @@ await catalogEntries.ForEachAsync(_maxDegreeOfParallelism, async catalogEntry => } } } - else if (catalogEntry.EntryType == Schema.DataTypes.PackageDelete.ToString()) + else if (catalogEntry.Type.AbsoluteUri == Schema.DataTypes.PackageDelete.AbsoluteUri) { var properties = GetTelemetryProperties(catalogEntry); @@ -190,11 +185,11 @@ await _dnxMaker.UpdatePackageVersionIndexAsync(packageId, versions => { foreach (var catalogEntry in catalogEntryGroup) { - if (catalogEntry.EntryType == Schema.DataTypes.PackageDetails.ToString()) + if (catalogEntry.Type.AbsoluteUri == Schema.DataTypes.PackageDetails.AbsoluteUri) { versions.Add(NuGetVersion.Parse(catalogEntry.NormalizedPackageVersion)); } - else if (catalogEntry.EntryType == Schema.DataTypes.PackageDelete.ToString()) + else if (catalogEntry.Type.AbsoluteUri == Schema.DataTypes.PackageDelete.AbsoluteUri) { versions.Remove(NuGetVersion.Parse(catalogEntry.NormalizedPackageVersion)); } @@ -445,17 +440,30 @@ private static Dictionary GetTelemetryProperties(string packageI private sealed class CatalogEntry { + internal DateTime CommitTimeStamp { get; } internal string PackageId { get; } internal string NormalizedPackageVersion { get; } - internal string EntryType { get; } - internal JToken Entry { get; } + internal Uri Type { get; } - internal CatalogEntry(string packageId, string normalizedPackageVersion, string entryType, JToken entry) + private CatalogEntry(DateTime commitTimeStamp, string packageId, string normalizedPackageVersion, Uri type) { + CommitTimeStamp = commitTimeStamp; PackageId = packageId; NormalizedPackageVersion = normalizedPackageVersion; - EntryType = entryType; - Entry = entry; + Type = type; + } + + internal static CatalogEntry Create(CatalogCommitItem item) + { + var typeUri = item.TypeUris.Single(uri => + uri.AbsoluteUri == Schema.DataTypes.PackageDetails.AbsoluteUri || + uri.AbsoluteUri == Schema.DataTypes.PackageDelete.AbsoluteUri); + + return new CatalogEntry( + item.CommitTimeStamp, + item.PackageIdentity.Id.ToLowerInvariant(), + item.PackageIdentity.Version.ToNormalizedString().ToLowerInvariant(), + typeUri); } } } diff --git a/src/Catalog/Dnx/DnxMaker.cs b/src/Catalog/Dnx/DnxMaker.cs index 478d7a509..732b19e71 100644 --- a/src/Catalog/Dnx/DnxMaker.cs +++ b/src/Catalog/Dnx/DnxMaker.cs @@ -173,7 +173,7 @@ public async Task UpdatePackageVersionIndexAsync(string id, Action result = new List(versions); + var result = new List(versions); if (result.Any()) { diff --git a/src/Catalog/FetchCatalogCommitsAsync.cs b/src/Catalog/FetchCatalogCommitsAsync.cs new file mode 100644 index 000000000..cde21651b --- /dev/null +++ b/src/Catalog/FetchCatalogCommitsAsync.cs @@ -0,0 +1,14 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +namespace NuGet.Services.Metadata.Catalog +{ + internal delegate Task> FetchCatalogCommitsAsync( + CollectorHttpClient client, + ReadWriteCursor front, + CancellationToken cancellationToken); +} \ No newline at end of file diff --git a/src/Catalog/GetCatalogCommitItemKey.cs b/src/Catalog/GetCatalogCommitItemKey.cs new file mode 100644 index 000000000..d28bf0659 --- /dev/null +++ b/src/Catalog/GetCatalogCommitItemKey.cs @@ -0,0 +1,7 @@ +// 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. + +namespace NuGet.Services.Metadata.Catalog +{ + public delegate string GetCatalogCommitItemKey(CatalogCommitItem item); +} \ No newline at end of file diff --git a/src/Catalog/Helpers/Utils.cs b/src/Catalog/Helpers/Utils.cs index 795cd4941..3bbcdb01d 100644 --- a/src/Catalog/Helpers/Utils.cs +++ b/src/Catalog/Helpers/Utils.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; +using System.Globalization; using System.IO; using System.IO.Compression; using System.Linq; @@ -346,21 +347,13 @@ public static bool IsType(JToken context, JToken obj, Uri type) return false; } - public static bool IsType(JToken context, JObject obj, Uri[] types) + public static Uri Expand(JToken context, JToken token) { - foreach (Uri type in types) - { - if (IsType(context, obj, type)) - { - return true; - } - } - return false; + return Expand(context, token.ToString()); } - public static Uri Expand(JToken context, JToken token) + public static Uri Expand(JToken context, string term) { - string term = token.ToString(); if (term.StartsWith("http:", StringComparison.OrdinalIgnoreCase)) { return new Uri(term); @@ -563,5 +556,25 @@ public static void TraceException(Exception e) } } } + + internal static T Deserialize(JObject jObject, string propertyName) + { + if (jObject == null) + { + throw new ArgumentNullException(nameof(jObject)); + } + + if (string.IsNullOrEmpty(propertyName)) + { + throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(propertyName)); + } + + if (!jObject.TryGetValue(propertyName, out var value) || value == null) + { + throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, Strings.PropertyRequired, propertyName)); + } + + return value.ToObject(); + } } } \ No newline at end of file diff --git a/src/Catalog/NuGet.Services.Metadata.Catalog.csproj b/src/Catalog/NuGet.Services.Metadata.Catalog.csproj index c346b91b9..b27c14fe7 100644 --- a/src/Catalog/NuGet.Services.Metadata.Catalog.csproj +++ b/src/Catalog/NuGet.Services.Metadata.Catalog.csproj @@ -82,16 +82,25 @@ + + + + + + + + + @@ -114,6 +123,7 @@ + diff --git a/src/Catalog/ProcessCommitItemBatchAsync.cs b/src/Catalog/ProcessCommitItemBatchAsync.cs new file mode 100644 index 000000000..71664dc0e --- /dev/null +++ b/src/Catalog/ProcessCommitItemBatchAsync.cs @@ -0,0 +1,17 @@ +// 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.Threading; +using System.Threading.Tasks; +using Newtonsoft.Json.Linq; + +namespace NuGet.Services.Metadata.Catalog +{ + public delegate Task ProcessCommitItemBatchAsync( + CollectorHttpClient client, + JToken context, + string packageId, + CatalogCommitItemBatch batch, + CatalogCommitItemBatch lastBatch, + CancellationToken cancellationToken); +} \ No newline at end of file diff --git a/src/Catalog/Registration/RegistrationCollector.cs b/src/Catalog/Registration/RegistrationCollector.cs index 7a318af27..44f1a2206 100644 --- a/src/Catalog/Registration/RegistrationCollector.cs +++ b/src/Catalog/Registration/RegistrationCollector.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Net.Http; using System.Threading; @@ -28,9 +27,6 @@ public class RegistrationCollector : SortingGraphCollector private readonly ShouldIncludeRegistrationPackage _shouldIncludeSemVer2; private readonly int _maxConcurrentBatches; - // Doesn't exist until .NET 4.6 - private static readonly Task CompletedTask = Task.FromResult(0); - public RegistrationCollector( Uri index, StorageFactory legacyStorageFactory, @@ -59,7 +55,8 @@ public RegistrationCollector( public Uri ContentBaseAddress { get; } - protected override Task> CreateBatchesAsync(IEnumerable catalogItems) + protected override Task> CreateBatchesAsync( + IEnumerable catalogItems) { // Grouping batches by commit is slow if it contains // the same package registration id over and over again. @@ -80,31 +77,14 @@ protected override Task> CreateBatchesAsync(IEnume // from the correct location (even though we may have // a little rework). - var batches = catalogItems - .GroupBy(item => GetKey(item.Value)) - .Select(group => new CatalogItemBatch( - group.Min(item => item.CommitTimeStamp), - group)); + var batches = CatalogCommitUtilities.CreateCommitItemBatches(catalogItems, GetKey); return Task.FromResult(batches); } - private async Task CreateBatchesAsync(ReadWriteCursor front, ReadCursor back, JObject page) + protected override string GetKey(CatalogCommitItem item) { - IEnumerable pageItems = page["items"] - .Select(item => new CatalogItem((JObject)item)) - .Where(item => item.CommitTimeStamp > front.Value && item.CommitTimeStamp <= back.Value); - - IEnumerable batches = await CreateBatchesAsync(pageItems); - - return batches - .OrderBy(batch => batch.CommitTimeStamp) - .ToArray(); - } - - protected override string GetKey(JObject item) - { - return item["nuget:id"].ToString().ToLowerInvariant(); + return CatalogCommitUtilities.GetPackageIdKey(item); } // Summary: @@ -120,111 +100,23 @@ protected override string GetKey(JObject item) // been transactional. Actively cancelling tasks would make an inconsistent registration more likely. // 6. Update the cursor if and only if all preceding commits and the current (oldest) commit have been // fully and successfully processed. - protected override async Task FetchAsync( + protected override Task FetchAsync( CollectorHttpClient client, ReadWriteCursor front, ReadCursor back, CancellationToken cancellationToken) { - IEnumerable catalogItems = await FetchCatalogItemsAsync(client, front, cancellationToken); - - var hasAnyBatchFailed = false; - var hasAnyBatchBeenProcessed = false; - - foreach (CatalogItem catalogItem in catalogItems) - { - JObject page = await client.GetJObjectAsync(catalogItem.Uri, cancellationToken); - JToken context = page["@context"]; - CatalogItemBatch[] batches = await CreateBatchesAsync(front, back, page); - SortedDictionary commitBatchTasksMap = CreateCommitBatchTasksMap(batches); - - var unprocessedBatches = new Queue(batches); - var processingBatches = new Queue(); - - CatalogItemBatch lastBatch = unprocessedBatches.LastOrDefault(); - var exceptions = new List(); - - EnqueueBatchesIfNoFailures( - client, - context, - commitBatchTasksMap, - unprocessedBatches, - processingBatches, - lastBatch, - cancellationToken); - - while (processingBatches.Any()) - { - var activeTasks = processingBatches.Where(batch => !batch.Task.IsCompleted) - .Select(batch => batch.Task) - .DefaultIfEmpty(CompletedTask); - - await Task.WhenAny(activeTasks); - - while (!hasAnyBatchFailed && commitBatchTasksMap.Any()) - { - var commitBatchTasks = commitBatchTasksMap.First().Value; - var isCommitFullyProcessed = commitBatchTasks.BatchTasks.All(batch => batch.Task != null && batch.Task.IsCompleted); - - if (!isCommitFullyProcessed) - { - break; - } - - var isCommitSuccessfullyProcessed = commitBatchTasks.BatchTasks.All(batch => batch.Task.Status == TaskStatus.RanToCompletion); - - if (isCommitSuccessfullyProcessed) - { - var commitTimeStamp = commitBatchTasks.CommitTimeStamp; - - front.Value = commitTimeStamp; - - await front.SaveAsync(cancellationToken); - - Trace.TraceInformation($"{nameof(RegistrationCollector)}.{nameof(FetchAsync)} {nameof(front)}.{nameof(front.Value)} saved since timestamp changed from previous: {{0}}", front); - - DequeueBatchesWhileMatches(processingBatches, batch => batch.CommitTimeStamp == commitTimeStamp); - - commitBatchTasksMap.Remove(commitTimeStamp); - } - else // Canceled or Failed - { - hasAnyBatchFailed = true; - - exceptions.AddRange( - commitBatchTasks.BatchTasks - .Select(batch => batch.Task) - .Where(task => (task.IsFaulted || task.IsCanceled) && task.Exception != null) - .Select(task => task.Exception)); - } - } - - if (hasAnyBatchFailed) - { - DequeueBatchesWhileMatches(processingBatches, batch => batch.Task.IsCompleted); - } - - hasAnyBatchBeenProcessed = true; - - EnqueueBatchesIfNoFailures( - client, - context, - commitBatchTasksMap, - unprocessedBatches, - processingBatches, - lastBatch, - cancellationToken); - } - - if (hasAnyBatchFailed) - { - var innerException = exceptions.Count == 1 ? exceptions.Single() : new AggregateException(exceptions); - - throw new BatchProcessingException(innerException); - } - } - - return hasAnyBatchBeenProcessed; + return CatalogCommitUtilities.FetchAsync( + client, + front, + back, + FetchCatalogCommitsAsync, + CatalogCommitUtilities.GetPackageIdKey, + CreateBatchesAsync, + ProcessBatchAsync, + _maxConcurrentBatches, + nameof(RegistrationCollector), + cancellationToken); } protected override async Task ProcessGraphsAsync( @@ -282,64 +174,12 @@ public static ShouldIncludeRegistrationPackage GetShouldIncludeRegistrationPacka return (k, u, g) => !NuGetVersionUtility.IsGraphSemVer2(k.Version, u, g); } - private static void DequeueBatchesWhileMatches(Queue batches, Func isMatch) - { - BatchTask batch; - - while ((batch = batches.FirstOrDefault()) != null) - { - if (isMatch(batch)) - { - batches.Dequeue(); - } - else - { - break; - } - } - } - - private void EnqueueBatchesIfNoFailures( - CollectorHttpClient client, - JToken context, - SortedDictionary commitBatchTasksMap, - Queue unprocessedBatches, - Queue processingBatches, - CatalogItemBatch lastBatch, - CancellationToken cancellationToken) - { - var hasAnyBatchFailed = processingBatches.Any(batch => batch.Task.IsFaulted || batch.Task.IsCanceled); - - if (hasAnyBatchFailed) - { - return; - } - - var batchesToEnqueue = Math.Min( - _maxConcurrentBatches - processingBatches.Count(batch => !batch.Task.IsCompleted), - unprocessedBatches.Count); - - for (var i = 0; i < batchesToEnqueue; ++i) - { - var batch = unprocessedBatches.Dequeue(); - var batchItem = batch.Items.First(); - var packageId = GetKey(batchItem.Value); - - var batchTask = commitBatchTasksMap[batchItem.CommitTimeStamp].BatchTasks - .Single(bt => bt.PackageId == packageId); - - batchTask.Task = ProcessBatchAsync(client, context, packageId, batch, lastBatch, cancellationToken); - - processingBatches.Enqueue(batchTask); - } - } - private async Task ProcessBatchAsync( CollectorHttpClient client, JToken context, string packageId, - CatalogItemBatch batch, - CatalogItemBatch lastBatch, + CatalogCommitItemBatch batch, + CatalogCommitItemBatch lastBatch, CancellationToken cancellationToken) { await Task.Yield(); @@ -354,82 +194,12 @@ private async Task ProcessBatchAsync( { await OnProcessBatchAsync( client, - batch.Items.Select(item => item.Value), + batch.Items, context, batch.CommitTimeStamp, batch.CommitTimeStamp == lastBatch.CommitTimeStamp, cancellationToken); } } - - private SortedDictionary CreateCommitBatchTasksMap(CatalogItemBatch[] batches) - { - var map = new SortedDictionary(); - - foreach (var batch in batches) - { - var jObject = batch.Items.First().Value; - var packageId = GetKey(jObject); - var batchTask = new BatchTask(batch.CommitTimeStamp, packageId); - - foreach (var commitTimeStamp in batch.Items.Select(item => item.CommitTimeStamp)) - { - CommitBatchTasks commitBatchTasks; - - if (!map.TryGetValue(commitTimeStamp, out commitBatchTasks)) - { - commitBatchTasks = new CommitBatchTasks(commitTimeStamp); - - map[commitTimeStamp] = commitBatchTasks; - } - - commitBatchTasks.BatchTasks.Add(batchTask); - } - } - - return map; - } - - private sealed class BatchTask - { - internal BatchTask(DateTime commitTimeStamp, string packageId) - { - CommitTimeStamp = commitTimeStamp; - PackageId = packageId; - } - - internal DateTime CommitTimeStamp { get; } - internal string PackageId { get; } - internal Task Task { get; set; } - - public override int GetHashCode() - { - return PackageId.GetHashCode(); - } - - public override bool Equals(object obj) - { - var other = obj as BatchTask; - - if (ReferenceEquals(other, null)) - { - return false; - } - - return GetHashCode() == other.GetHashCode(); - } - } - - private sealed class CommitBatchTasks - { - internal CommitBatchTasks(DateTime commitTimeStamp) - { - BatchTasks = new HashSet(); - CommitTimeStamp = commitTimeStamp; - } - - internal HashSet BatchTasks { get; } - internal DateTime CommitTimeStamp { get; } - } } } \ No newline at end of file diff --git a/src/Catalog/SortingCollector.cs b/src/Catalog/SortingCollector.cs index 7524c18b4..c32d8998f 100644 --- a/src/Catalog/SortingCollector.cs +++ b/src/Catalog/SortingCollector.cs @@ -20,22 +20,22 @@ public SortingCollector(Uri index, ITelemetryService telemetryService, Func OnProcessBatchAsync( CollectorHttpClient client, - IEnumerable items, + IEnumerable items, JToken context, DateTime commitTimeStamp, bool isLastBatch, CancellationToken cancellationToken) { - IDictionary> sortedItems = new Dictionary>(); + var sortedItems = new Dictionary>(); - foreach (JObject item in items) + foreach (CatalogCommitItem item in items) { T key = GetKey(item); - IList itemList; + IList itemList; if (!sortedItems.TryGetValue(key, out itemList)) { - itemList = new List(); + itemList = new List(); sortedItems.Add(key, itemList); } @@ -44,7 +44,7 @@ protected override async Task OnProcessBatchAsync( IList tasks = new List(); - foreach (KeyValuePair> sortedBatch in sortedItems) + foreach (KeyValuePair> sortedBatch in sortedItems) { Task task = ProcessSortedBatchAsync(client, sortedBatch, context, cancellationToken); @@ -56,11 +56,11 @@ protected override async Task OnProcessBatchAsync( return true; } - protected abstract T GetKey(JObject item); + protected abstract T GetKey(CatalogCommitItem item); protected abstract Task ProcessSortedBatchAsync( CollectorHttpClient client, - KeyValuePair> sortedBatch, + KeyValuePair> sortedBatch, JToken context, CancellationToken cancellationToken); } diff --git a/src/Catalog/SortingGraphCollector.cs b/src/Catalog/SortingGraphCollector.cs index af1034fa1..bbccb842e 100644 --- a/src/Catalog/SortingGraphCollector.cs +++ b/src/Catalog/SortingGraphCollector.cs @@ -28,7 +28,7 @@ public SortingGraphCollector( protected override async Task ProcessSortedBatchAsync( CollectorHttpClient client, - KeyValuePair> sortedBatch, + KeyValuePair> sortedBatch, JToken context, CancellationToken cancellationToken) { @@ -37,16 +37,25 @@ protected override async Task ProcessSortedBatchAsync( foreach (var item in sortedBatch.Value) { - if (Utils.IsType((JObject)context, item, _types)) + var isMatch = false; + + foreach (Uri type in _types) { - var itemUri = item["@id"].ToString(); + if (item.TypeUris.Any(typeUri => typeUri.AbsoluteUri == type.AbsoluteUri)) + { + isMatch = true; + break; + } + } + if (isMatch) + { // Load package details from catalog. // Download the graph to a read-only container. This allows operations on each graph to be safely // parallelized. - var task = client.GetGraphAsync(new Uri(itemUri), readOnly: true, token: cancellationToken); + var task = client.GetGraphAsync(item.Uri, readOnly: true, token: cancellationToken); - graphTasks.Add(itemUri, task); + graphTasks.Add(item.Uri.AbsoluteUri, task); } } diff --git a/src/Catalog/SortingIdCollector.cs b/src/Catalog/SortingIdCollector.cs index b39ba1ac6..bd2925fb0 100644 --- a/src/Catalog/SortingIdCollector.cs +++ b/src/Catalog/SortingIdCollector.cs @@ -3,19 +3,19 @@ using System; using System.Net.Http; -using Newtonsoft.Json.Linq; namespace NuGet.Services.Metadata.Catalog { public abstract class SortingIdCollector : SortingCollector { - public SortingIdCollector(Uri index, ITelemetryService telemetryService, Func handlerFunc = null) : base(index, telemetryService, handlerFunc) + public SortingIdCollector(Uri index, ITelemetryService telemetryService, Func handlerFunc = null) + : base(index, telemetryService, handlerFunc) { } - protected override string GetKey(JObject item) + protected override string GetKey(CatalogCommitItem item) { - return item["nuget:id"].ToString(); + return item.PackageIdentity.Id; } } -} +} \ No newline at end of file diff --git a/src/Catalog/SortingIdVersionCollector.cs b/src/Catalog/SortingIdVersionCollector.cs index 8911e7221..b21fd0354 100644 --- a/src/Catalog/SortingIdVersionCollector.cs +++ b/src/Catalog/SortingIdVersionCollector.cs @@ -3,7 +3,6 @@ using System; using System.Net.Http; -using Newtonsoft.Json.Linq; using NuGet.Services.Metadata.Catalog.Helpers; namespace NuGet.Services.Metadata.Catalog @@ -15,9 +14,9 @@ public SortingIdVersionCollector(Uri index, ITelemetryService telemetryService, { } - protected override FeedPackageIdentity GetKey(JObject item) + protected override FeedPackageIdentity GetKey(CatalogCommitItem item) { - return new FeedPackageIdentity(item["nuget:id"].ToString(), item["nuget:version"].ToString()); + return new FeedPackageIdentity(item.PackageIdentity); } } -} +} \ No newline at end of file diff --git a/src/Catalog/Strings.Designer.cs b/src/Catalog/Strings.Designer.cs index 3a222b5de..07fe7f669 100644 --- a/src/Catalog/Strings.Designer.cs +++ b/src/Catalog/Strings.Designer.cs @@ -95,5 +95,23 @@ internal static string BatchProcessingFailure { return ResourceManager.GetString("BatchProcessingFailure", resourceCulture); } } + + /// + /// Looks up a localized string similar to The value of property '{0}' must be non-null and non-empty.. + /// + internal static string NonEmptyPropertyValueRequired { + get { + return ResourceManager.GetString("NonEmptyPropertyValueRequired", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The property '{0}' is required and is value must not be null.. + /// + internal static string PropertyRequired { + get { + return ResourceManager.GetString("PropertyRequired", resourceCulture); + } + } } } diff --git a/src/Catalog/Strings.resx b/src/Catalog/Strings.resx index 81e1cc1ad..ede92d8e0 100644 --- a/src/Catalog/Strings.resx +++ b/src/Catalog/Strings.resx @@ -129,4 +129,10 @@ A failure occurred while processing a catalog batch. + + The value of property '{0}' must be non-null and non-empty. + + + The property '{0}' is required and is value must not be null. + \ No newline at end of file diff --git a/src/Ng/Jobs/MonitoringProcessorJob.cs b/src/Ng/Jobs/MonitoringProcessorJob.cs index 815e62169..c1f58d2de 100644 --- a/src/Ng/Jobs/MonitoringProcessorJob.cs +++ b/src/Ng/Jobs/MonitoringProcessorJob.cs @@ -207,6 +207,7 @@ private async Task> FetchCatalogIndexEntriesFromR var catalogPageUri = new Uri(leafBlob["@id"].ToString()); var catalogPage = await _client.GetJObjectAsync(catalogPageUri, token); + return new CatalogIndexEntry[] { new CatalogIndexEntry( @@ -214,8 +215,7 @@ private async Task> FetchCatalogIndexEntriesFromR Schema.DataTypes.PackageDetails.ToString(), catalogPage["catalog:commitId"].ToString(), DateTime.Parse(catalogPage["catalog:commitTimeStamp"].ToString()), - id, - version) + new PackageIdentity(id, version)) }; } diff --git a/src/Ng/SearchIndexFromCatalogCollector.cs b/src/Ng/SearchIndexFromCatalogCollector.cs index 1cf34388c..3f281fe23 100644 --- a/src/Ng/SearchIndexFromCatalogCollector.cs +++ b/src/Ng/SearchIndexFromCatalogCollector.cs @@ -50,7 +50,13 @@ public SearchIndexFromCatalogCollector( _logger = logger; } - protected override async Task OnProcessBatchAsync(CollectorHttpClient client, IEnumerable items, JToken context, DateTime commitTimeStamp, bool isLastBatch, CancellationToken cancellationToken) + protected override async Task OnProcessBatchAsync( + CollectorHttpClient client, + IEnumerable items, + JToken context, + DateTime commitTimeStamp, + bool isLastBatch, + CancellationToken cancellationToken) { JObject catalogIndex = null; if (_baseAddress != null) @@ -152,16 +158,14 @@ private async Task CommitIndexAsync() private static async Task> FetchCatalogItemsAsync( CollectorHttpClient client, - IEnumerable items, + IEnumerable items, CancellationToken cancellationToken) { - IList> tasks = new List>(); + var tasks = new List>(); - foreach (JToken item in items) + foreach (var item in items) { - Uri catalogItemUri = item["@id"].ToObject(); - - tasks.Add(client.GetJObjectAsync(catalogItemUri, cancellationToken)); + tasks.Add(client.GetJObjectAsync(item.Uri, cancellationToken)); } await Task.WhenAll(tasks); diff --git a/src/NuGet.Services.Metadata.Catalog.Monitoring/ValidationCollector.cs b/src/NuGet.Services.Metadata.Catalog.Monitoring/ValidationCollector.cs index d34692e1d..d89471ef5 100644 --- a/src/NuGet.Services.Metadata.Catalog.Monitoring/ValidationCollector.cs +++ b/src/NuGet.Services.Metadata.Catalog.Monitoring/ValidationCollector.cs @@ -35,7 +35,11 @@ public ValidationCollector( _logger = logger; } - protected override async Task ProcessSortedBatchAsync(CollectorHttpClient client, KeyValuePair> sortedBatch, JToken context, CancellationToken cancellationToken) + protected override async Task ProcessSortedBatchAsync( + CollectorHttpClient client, + KeyValuePair> sortedBatch, + JToken context, + CancellationToken cancellationToken) { var packageId = sortedBatch.Key.Id; var packageVersion = sortedBatch.Key.Version; diff --git a/src/V3PerPackage/EnqueueCollector.cs b/src/V3PerPackage/EnqueueCollector.cs index c4a91f39e..796bf60dd 100644 --- a/src/V3PerPackage/EnqueueCollector.cs +++ b/src/V3PerPackage/EnqueueCollector.cs @@ -11,7 +11,6 @@ using Microsoft.Extensions.Options; using Newtonsoft.Json.Linq; using NuGet.Services.Metadata.Catalog; -using NuGet.Services.Metadata.Catalog.Helpers; using NuGet.Services.Storage; namespace NuGet.Services.V3PerPackage @@ -32,24 +31,24 @@ public EnqueueCollector( _queue = queue ?? throw new ArgumentNullException(nameof(queue)); } - protected override Task> CreateBatchesAsync(IEnumerable catalogItems) + protected override Task> CreateBatchesAsync(IEnumerable catalogItems) { var catalogItemList = catalogItems.ToList(); var maxCommitTimestamp = catalogItems.Max(x => x.CommitTimeStamp); - return Task.FromResult(new[] { new CatalogItemBatch(maxCommitTimestamp, catalogItemList) }.AsEnumerable()); + return Task.FromResult(new[] { new CatalogCommitItemBatch(maxCommitTimestamp, key: null, items: catalogItemList) }.AsEnumerable()); } protected override async Task OnProcessBatchAsync( CollectorHttpClient client, - IEnumerable items, + IEnumerable items, JToken context, DateTime commitTimeStamp, bool isLastBatch, CancellationToken cancellationToken) { - var itemBag = new ConcurrentBag(items); + var itemBag = new ConcurrentBag(items); var tasks = Enumerable .Range(0, 16) @@ -61,19 +60,18 @@ protected override async Task OnProcessBatchAsync( return true; } - private async Task EnqueueAsync(ConcurrentBag itemBag, CancellationToken cancellationToken) + private async Task EnqueueAsync(ConcurrentBag itemBag, CancellationToken cancellationToken) { while (itemBag.TryTake(out var item)) { - var id = item["nuget:id"].ToString().ToLowerInvariant(); - var version = NuGetVersionUtility.NormalizeVersion(item["nuget:version"].ToString().ToLowerInvariant()); - var type = item["@type"].ToString().Replace("nuget:", Schema.Prefixes.NuGet); - - if (type != Schema.DataTypes.PackageDetails.ToString()) + if (!item.TypeUris.Any(itemType => itemType.AbsoluteUri != Schema.DataTypes.PackageDetails.AbsoluteUri)) { continue; } + var id = item.PackageIdentity.Id.ToLowerInvariant(); + var version = item.PackageIdentity.Version.ToNormalizedString().ToLowerInvariant(); + await _queue.AddAsync(new PackageMessage(id, version), cancellationToken); } } diff --git a/tests/CatalogTests/BatchProcessingExceptionTests.cs b/tests/CatalogTests/BatchProcessingExceptionTests.cs new file mode 100644 index 000000000..35b5a0133 --- /dev/null +++ b/tests/CatalogTests/BatchProcessingExceptionTests.cs @@ -0,0 +1,30 @@ +// 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 NuGet.Services.Metadata.Catalog; +using Xunit; + +namespace CatalogTests +{ + public class BatchProcessingExceptionTests + { + [Fact] + public void Constructor_WhenExceptionIsNull_Throws() + { + var exception = Assert.Throws(() => new BatchProcessingException(inner: null)); + + Assert.Equal("inner", exception.ParamName); + } + + [Fact] + public void Constructor_WhenArgumentIsValid_ReturnsInstance() + { + var innerException = new Exception(); + var exception = new BatchProcessingException(innerException); + + Assert.Equal("A failure occurred while processing a catalog batch.", exception.Message); + Assert.Same(innerException, exception.InnerException); + } + } +} \ No newline at end of file diff --git a/tests/CatalogTests/CatalogCommitBatchTaskTests.cs b/tests/CatalogTests/CatalogCommitBatchTaskTests.cs new file mode 100644 index 000000000..c111b79b1 --- /dev/null +++ b/tests/CatalogTests/CatalogCommitBatchTaskTests.cs @@ -0,0 +1,79 @@ +// 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 NuGet.Services.Metadata.Catalog; +using Xunit; + +namespace CatalogTests +{ + public class CatalogCommitBatchTaskTests + { + private readonly DateTime _minCommitTimeStamp = DateTime.UtcNow; + private const string _key = "a"; + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Constructor_WhenKeyIsNullEmptyOrWhitespace_Throws(string key) + { + var exception = Assert.Throws( + () => new CatalogCommitBatchTask(_minCommitTimeStamp, key)); + + Assert.Equal("key", exception.ParamName); + } + + [Fact] + public void Constructor_WhenArgumentsAreValid_ReturnsInstance() + { + var commitBatchTask = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); + + Assert.Equal(_minCommitTimeStamp, commitBatchTask.MinCommitTimeStamp); + Assert.Equal(_key, commitBatchTask.Key); + Assert.Null(commitBatchTask.Task); + } + + [Fact] + public void GetHashCode_Always_ReturnsKeyHashCode() + { + var commitBatchTask = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); + + Assert.Equal(_key.GetHashCode(), commitBatchTask.GetHashCode()); + } + + [Fact] + public void Equals_WhenObjectIsNull_ReturnsFalse() + { + var commitBatchTask = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); + + Assert.False(commitBatchTask.Equals(obj: null)); + } + + [Fact] + public void Equals_WhenObjectIsNotCatalogCommitBatchTask_ReturnsFalse() + { + var commitBatchTask = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); + + Assert.False(commitBatchTask.Equals(new object())); + } + + [Fact] + public void Equals_WhenObjectIsSameInstance_ReturnsTrue() + { + var commitBatchTask = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); + + Assert.True(commitBatchTask.Equals(commitBatchTask)); + } + + [Fact] + public void Equals_WhenObjectHasSamePackageId_ReturnsTrue() + { + var commitBatchTask0 = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); + var commitBatchTask1 = new CatalogCommitBatchTask(_minCommitTimeStamp.AddMinutes(1), _key); + + Assert.True(commitBatchTask0.Equals(commitBatchTask1)); + Assert.True(commitBatchTask1.Equals(commitBatchTask0)); + } + } +} \ No newline at end of file diff --git a/tests/CatalogTests/CatalogCommitBatchTasksTests.cs b/tests/CatalogTests/CatalogCommitBatchTasksTests.cs new file mode 100644 index 000000000..3c1a7e654 --- /dev/null +++ b/tests/CatalogTests/CatalogCommitBatchTasksTests.cs @@ -0,0 +1,22 @@ +// 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 NuGet.Services.Metadata.Catalog; +using Xunit; + +namespace CatalogTests +{ + public class CatalogCommitBatchTasksTests + { + [Fact] + public void Constructor_Always_ReturnsInstance() + { + var commitTimeStamp = DateTime.UtcNow; + var commitBatchTasks = new CatalogCommitBatchTasks(commitTimeStamp); + + Assert.Equal(commitTimeStamp, commitBatchTasks.CommitTimeStamp); + Assert.Empty(commitBatchTasks.BatchTasks); + } + } +} \ No newline at end of file diff --git a/tests/CatalogTests/CatalogCommitItemBatchTests.cs b/tests/CatalogTests/CatalogCommitItemBatchTests.cs new file mode 100644 index 000000000..2e4e0fc0f --- /dev/null +++ b/tests/CatalogTests/CatalogCommitItemBatchTests.cs @@ -0,0 +1,62 @@ +// 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.Linq; +using NgTests.Infrastructure; +using NuGet.Packaging.Core; +using NuGet.Services.Metadata.Catalog; +using NuGet.Versioning; +using Xunit; + +namespace CatalogTests +{ + public class CatalogCommitItemBatchTests + { + private static readonly PackageIdentity _packageIdentity = new PackageIdentity( + id: "a", + version: new NuGetVersion("1.0.0")); + private const string _nullKey = null; + + [Fact] + public void Constructor_WhenItemsIsNull_Throws() + { + var exception = Assert.Throws( + () => new CatalogCommitItemBatch(DateTime.MinValue, _nullKey, items: null)); + + Assert.Equal("items", exception.ParamName); + } + + [Fact] + public void Constructor_WhenItemsIsEmpty_Throws() + { + var exception = Assert.Throws( + () => new CatalogCommitItemBatch(DateTime.MinValue, _nullKey, Enumerable.Empty())); + + Assert.Equal("items", exception.ParamName); + } + + [Theory] + [InlineData(null)] + [InlineData("a")] + public void Constructor_WhenCommitsAreUnordered_OrdersCommitsInChronologicallyAscendingOrder(string key) + { + var commitTimeStamp = DateTime.UtcNow; + var commitItem0 = TestUtility.CreateCatalogCommitItem(commitTimeStamp, _packageIdentity); + var commitItem1 = TestUtility.CreateCatalogCommitItem(commitTimeStamp.AddMinutes(1), _packageIdentity); + var commitItem2 = TestUtility.CreateCatalogCommitItem(commitTimeStamp.AddMinutes(2), _packageIdentity); + + var commitItemBatch = new CatalogCommitItemBatch( + commitTimeStamp, + key, + new[] { commitItem1, commitItem0, commitItem2 }); + + Assert.Equal(commitTimeStamp, commitItemBatch.CommitTimeStamp); + Assert.Equal(3, commitItemBatch.Items.Count); + Assert.Equal(key, commitItemBatch.Key); + Assert.Same(commitItem0, commitItemBatch.Items[0]); + Assert.Same(commitItem1, commitItemBatch.Items[1]); + Assert.Same(commitItem2, commitItemBatch.Items[2]); + } + } +} \ No newline at end of file diff --git a/tests/CatalogTests/CatalogCommitItemTests.cs b/tests/CatalogTests/CatalogCommitItemTests.cs new file mode 100644 index 000000000..f91202278 --- /dev/null +++ b/tests/CatalogTests/CatalogCommitItemTests.cs @@ -0,0 +1,84 @@ +// 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.Linq; +using Newtonsoft.Json.Linq; +using NgTests; +using NgTests.Infrastructure; +using NuGet.Packaging.Core; +using NuGet.Services.Metadata.Catalog; +using NuGet.Versioning; +using Xunit; + +namespace CatalogTests +{ + public class CatalogCommitItemTests + { + private static readonly PackageIdentity _packageIdentity = new PackageIdentity(id: "A", version: new NuGetVersion("1.0.0")); + private readonly DateTime _now = DateTime.UtcNow; + private readonly JObject _context; + private readonly JObject _commitItem; + + public CatalogCommitItemTests() + { + _context = TestUtility.CreateCatalogContextJObject(); + _commitItem = TestUtility.CreateCatalogCommitItemJObject(_now, _packageIdentity); + } + + [Fact] + public void Create_WhenContextIsNull_Throws() + { + const JObject context = null; + + var exception = Assert.Throws(() => CatalogCommitItem.Create(context, _commitItem)); + + Assert.Equal("context", exception.ParamName); + } + + [Fact] + public void Create_WhenCommitItemIsNull_Throws() + { + var exception = Assert.Throws(() => CatalogCommitItem.Create(_context, commitItem: null)); + + Assert.Equal("commitItem", exception.ParamName); + } + + [Fact] + public void Create_WhenTypeIsEmpty_Throws() + { + _commitItem[CatalogConstants.TypeKeyword] = new JArray(); + + var exception = Assert.Throws(() => CatalogCommitItem.Create(_context, _commitItem)); + + Assert.Equal("commitItem", exception.ParamName); + Assert.StartsWith($"The value of property '{CatalogConstants.TypeKeyword}' must be non-null and non-empty.", exception.Message); + } + + [Fact] + public void Create_WhenArgumentsAreValid_ReturnsInstance() + { + var commitItem = CatalogCommitItem.Create(_context, _commitItem); + + Assert.Equal($"https://nuget.test/{_packageIdentity.Id}", commitItem.Uri.AbsoluteUri); + Assert.Equal(_now, commitItem.CommitTimeStamp.ToUniversalTime()); + Assert.True(Guid.TryParse(commitItem.CommitId, out var commitId)); + Assert.Equal(_packageIdentity, commitItem.PackageIdentity); + Assert.Equal(CatalogConstants.NuGetPackageDetails, commitItem.Types.Single()); + Assert.Equal(Schema.DataTypes.PackageDetails.AbsoluteUri, commitItem.TypeUris.Single().AbsoluteUri); + } + + [Fact] + public void CompareTo_WhenArgumentIsValid_ReturnsValue() + { + var commitTimeStamp1 = DateTime.UtcNow; + var commitTimeStamp2 = DateTime.UtcNow.AddMinutes(1); + var commitItem0 = TestUtility.CreateCatalogCommitItem(commitTimeStamp1, _packageIdentity); + var commitItem1 = TestUtility.CreateCatalogCommitItem(commitTimeStamp2, _packageIdentity); + + Assert.Equal(0, commitItem0.CompareTo(commitItem0)); + Assert.Equal(-1, commitItem0.CompareTo(commitItem1)); + Assert.Equal(1, commitItem1.CompareTo(commitItem0)); + } + } +} \ No newline at end of file diff --git a/tests/CatalogTests/CatalogCommitTests.cs b/tests/CatalogTests/CatalogCommitTests.cs new file mode 100644 index 000000000..099472e2e --- /dev/null +++ b/tests/CatalogTests/CatalogCommitTests.cs @@ -0,0 +1,37 @@ +// 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 Newtonsoft.Json.Linq; +using NgTests; +using NuGet.Services.Metadata.Catalog; +using Xunit; + +namespace CatalogTests +{ + public class CatalogCommitTests + { + [Fact] + public void Create_WhenCommitIsNull_Throws() + { + var exception = Assert.Throws(() => CatalogCommit.Create(commit: null)); + + Assert.Equal("commit", exception.ParamName); + } + + [Fact] + public void Create_WhenArgumentIsValid_ReturnsInstance() + { + var idKeyword = "https://nuget.test/a"; + var commitTimeStamp = DateTime.UtcNow.ToString("O"); + var jObject = new JObject( + new JProperty(CatalogConstants.IdKeyword, idKeyword), + new JProperty(CatalogConstants.CommitTimeStamp, commitTimeStamp)); + + var commit = CatalogCommit.Create(jObject); + + Assert.Equal(idKeyword, commit.Uri.AbsoluteUri); + Assert.Equal(commitTimeStamp, commit.CommitTimeStamp.ToUniversalTime().ToString("O")); + } + } +} \ No newline at end of file diff --git a/tests/CatalogTests/CatalogCommitUtilitiesTests.cs b/tests/CatalogTests/CatalogCommitUtilitiesTests.cs new file mode 100644 index 000000000..959128674 --- /dev/null +++ b/tests/CatalogTests/CatalogCommitUtilitiesTests.cs @@ -0,0 +1,653 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Newtonsoft.Json.Linq; +using NgTests.Infrastructure; +using NuGet.Packaging.Core; +using NuGet.Services.Metadata.Catalog; +using NuGet.Versioning; +using Xunit; + +namespace CatalogTests +{ + public class CatalogCommitUtilitiesTests + { + private const int _maxConcurrentBatches = 1; + private static readonly CatalogCommitItemBatch _lastBatch; + private static readonly Task FailedTask = Task.FromException(new Exception()); + private static readonly PackageIdentity _packageIdentitya = new PackageIdentity(id: "a", version: new NuGetVersion("1.0.0")); + private static readonly PackageIdentity _packageIdentityb = new PackageIdentity(id: "b", version: new NuGetVersion("1.0.0")); + + static CatalogCommitUtilitiesTests() + { + var commitTimeStamp = DateTime.UtcNow; + var commit = TestUtility.CreateCatalogCommitItem(commitTimeStamp, _packageIdentitya); + + _lastBatch = new CatalogCommitItemBatch(DateTime.UtcNow, _packageIdentitya.Id, new[] { commit }); + } + + [Fact] + public void CreateCommitItemBatches_WhenCatalogItemsIsNull_Throws() + { + IEnumerable catalogItems = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.CreateCommitItemBatches( + catalogItems, + CatalogCommitUtilities.GetPackageIdKey) + .ToArray()); // force evaluation + + Assert.Equal("catalogItems", exception.ParamName); + } + + [Fact] + public void CreateCommitItemBatches_WhenGetCatalogCommitItemKeyIsNull_Throws() + { + var exception = Assert.Throws( + () => CatalogCommitUtilities.CreateCommitItemBatches( + Enumerable.Empty(), + getCatalogCommitItemKey: null) + .ToArray()); // force evaluation + + Assert.Equal("getCatalogCommitItemKey", exception.ParamName); + } + + [Fact] + public void CreateCommitItemBatches_WhenPackageIdsVaryInCasing_GroupsUsingProvidedGetKeyFunction() + { + var now = DateTime.UtcNow; + var packageIdentityA0 = new PackageIdentity(id: "a", version: new NuGetVersion("1.0.0")); + var packageIdentityA1 = new PackageIdentity(id: "A", version: new NuGetVersion("2.0.0")); + var packageIdentityA2 = new PackageIdentity(id: "A", version: new NuGetVersion("3.0.0")); + var packageIdentityB0 = new PackageIdentity(id: "b", version: new NuGetVersion("1.0.0")); + var packageIdentityB1 = new PackageIdentity(id: "B", version: new NuGetVersion("2.0.0")); + var commit0 = TestUtility.CreateCatalogCommitItem(now, packageIdentityA0); + var commit1 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), packageIdentityA1); + var commit2 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(2), packageIdentityA2); + var commit3 = TestUtility.CreateCatalogCommitItem(now, packageIdentityB0); + var commit4 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), packageIdentityB1); + var commits = new[] { commit4, commit2, commit0, commit3, commit1 }; // not in alphanumeric or chronological order + + var batches = CatalogCommitUtilities.CreateCommitItemBatches(commits, CatalogCommitUtilities.GetPackageIdKey); + + Assert.Collection( + batches, + batch => + { + Assert.Equal(commit3.CommitTimeStamp, batch.CommitTimeStamp); + Assert.Collection( + batch.Items, + commit => Assert.True(ReferenceEquals(commit, commit3)), + commit => Assert.True(ReferenceEquals(commit, commit4))); + }, + batch => + { + Assert.Equal(commit0.CommitTimeStamp, batch.CommitTimeStamp); + Assert.Collection( + batch.Items, + commit => Assert.True(ReferenceEquals(commit, commit0)), + commit => Assert.True(ReferenceEquals(commit, commit1)), + commit => Assert.True(ReferenceEquals(commit, commit2))); + }); + } + + [Fact] + public void CreateCommitItemBatches_WhenCommitItemsContainMultipleCommitsForSamePackageIdentity_ReturnsOnlyLatestCommitForEachPackageIdentity() + { + var now = DateTime.UtcNow; + var commit0 = TestUtility.CreateCatalogCommitItem(now, _packageIdentitya); + var commit1 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), _packageIdentitya); + var commit2 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(2), _packageIdentitya); + var commits = new[] { commit0, commit1, commit2 }; + + var batches = CatalogCommitUtilities.CreateCommitItemBatches(commits, CatalogCommitUtilities.GetPackageIdKey); + + Assert.Collection( + batches, + batch => + { + Assert.Equal(commit0.CommitTimeStamp, batch.CommitTimeStamp); + Assert.Collection( + batch.Items, + commit => Assert.True(ReferenceEquals(commit, commit2))); + }); + } + + [Fact] + public void CreateCommitBatchTasksMap_WhenBatchesIsNull_Throws() + { + IEnumerable batches = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.CreateCommitBatchTasksMap(batches)); + + Assert.Equal("batches", exception.ParamName); + } + + [Fact] + public void CreateCommitBatchTasksMap_WhenBatchesIsEmpty_Throws() + { + var batches = Enumerable.Empty(); + + var exception = Assert.Throws( + () => CatalogCommitUtilities.CreateCommitBatchTasksMap(batches)); + + Assert.Equal("batches", exception.ParamName); + } + + [Fact] + public void CreateCommitBatchTasksMap_WhenArgumentsAreValid_ReturnsMap() + { + var commitTimeStamp = DateTime.UtcNow; + + var commit0 = TestUtility.CreateCatalogCommitItem(commitTimeStamp, _packageIdentitya); + var commit1 = TestUtility.CreateCatalogCommitItem(commitTimeStamp.AddMinutes(1), _packageIdentitya); + var commitBatch0 = new CatalogCommitItemBatch(commit0.CommitTimeStamp, _packageIdentitya.Id, new[] { commit0, commit1 }); + + var commit2 = TestUtility.CreateCatalogCommitItem(commitTimeStamp.AddMinutes(1), _packageIdentityb); + var commit3 = TestUtility.CreateCatalogCommitItem(commitTimeStamp.AddMinutes(2), _packageIdentityb); + var commitBatch1 = new CatalogCommitItemBatch(commit2.CommitTimeStamp, _packageIdentityb.Id, new[] { commit2, commit3 }); + + var commitBatches = new[] { commitBatch0, commitBatch1 }; + + var map = CatalogCommitUtilities.CreateCommitBatchTasksMap(commitBatches); + + Assert.Collection( + map, + element => + { + Assert.Equal(commitTimeStamp, element.Key.ToUniversalTime()); + Assert.Equal(commitTimeStamp, element.Value.CommitTimeStamp.ToUniversalTime()); + Assert.Single(element.Value.BatchTasks); + + var batchTask = element.Value.BatchTasks.Single(); + + Assert.Equal(commitBatch0.CommitTimeStamp, batchTask.MinCommitTimeStamp); + Assert.Equal(_packageIdentitya.Id, batchTask.Key); + }, + element => + { + var expectedCommitTimeStamp = commitTimeStamp.AddMinutes(1); + + Assert.Equal(expectedCommitTimeStamp, element.Key.ToUniversalTime()); + Assert.Equal(expectedCommitTimeStamp, element.Value.CommitTimeStamp.ToUniversalTime()); + Assert.Single(element.Value.BatchTasks); + + var batchTask = element.Value.BatchTasks.Single(); + + Assert.Equal(commitBatch1.CommitTimeStamp, batchTask.MinCommitTimeStamp); + Assert.Equal(_packageIdentityb.Id, batchTask.Key); + }); + } + + [Fact] + public void DequeueBatchesWhileMatches_WhenBatchesIsNull_Throws() + { + Queue batches = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.DequeueBatchesWhileMatches(batches, _ => true)); + + Assert.Equal("batches", exception.ParamName); + } + + [Fact] + public void DequeueBatchesWhileMatches_WhenIsMatchIsNull_Throws() + { + var exception = Assert.Throws( + () => CatalogCommitUtilities.DequeueBatchesWhileMatches( + new Queue(), + isMatch: null)); + + Assert.Equal("isMatch", exception.ParamName); + } + + [Fact] + public void DequeueBatchesWhileMatches_WhenQueueIsEmpty_NoOps() + { + var batches = new Queue(); + + CatalogCommitUtilities.DequeueBatchesWhileMatches(batches, batch => true); + + Assert.Empty(batches); + } + + [Fact] + public void DequeueBatchesWhileMatches_WhenNoMatchIsFound_NoOps() + { + var now = DateTime.UtcNow; + var id0 = "a"; + var id1 = "b"; + var commitBatchTask0 = new CatalogCommitBatchTask(now, id0); + var commitBatchTask1 = new CatalogCommitBatchTask(now.AddMinutes(1), id1); + var commitBatchTask2 = new CatalogCommitBatchTask(now.AddMinutes(2), id0); + + var batches = new Queue(); + + batches.Enqueue(commitBatchTask0); + batches.Enqueue(commitBatchTask1); + batches.Enqueue(commitBatchTask2); + + CatalogCommitUtilities.DequeueBatchesWhileMatches(batches, batch => false); + + Assert.Equal(3, batches.Count); + } + + [Fact] + public void DequeueBatchesWhileMatches_WhenMatchIsFound_Dequeues() + { + var now = DateTime.UtcNow; + var id0 = "a"; + var id1 = "b"; + var commitBatchTask0 = new CatalogCommitBatchTask(now, id0); + var commitBatchTask1 = new CatalogCommitBatchTask(now.AddMinutes(1), id1); + var commitBatchTask2 = new CatalogCommitBatchTask(now.AddMinutes(2), id0); + + var batches = new Queue(); + + batches.Enqueue(commitBatchTask0); + batches.Enqueue(commitBatchTask1); + batches.Enqueue(commitBatchTask2); + + CatalogCommitUtilities.DequeueBatchesWhileMatches(batches, batch => batch.Key == id0); + + Assert.Equal(2, batches.Count); + Assert.Same(commitBatchTask1, batches.Dequeue()); + Assert.Same(commitBatchTask2, batches.Dequeue()); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenClientIsNull_Throws() + { + const CollectorHttpClient client = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + client, + new JObject(), + new SortedDictionary(), + new Queue(), + new Queue(), + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None)); + + Assert.Equal("client", exception.ParamName); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenContextIsNull_Throws() + { + const JToken context = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + context, + new SortedDictionary(), + new Queue(), + new Queue(), + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None)); + + Assert.Equal("context", exception.ParamName); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenCommitBatchTasksMapIsNull_Throws() + { + const SortedDictionary commitBatchTasksMap = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + commitBatchTasksMap, + new Queue(), + new Queue(), + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None)); + + Assert.Equal("commitBatchTasksMap", exception.ParamName); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenUnprocessedBatchesIsNull_Throws() + { + const Queue unprocessedBatches = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + new SortedDictionary(), + unprocessedBatches, + new Queue(), + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None)); + + Assert.Equal("unprocessedBatches", exception.ParamName); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenProcessingBatchesIsNull_Throws() + { + const Queue processingBatches = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + new SortedDictionary(), + new Queue(), + processingBatches, + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None)); + + Assert.Equal("processingBatches", exception.ParamName); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenLastBatchIsNull_Throws() + { + const CatalogCommitItemBatch lastBatch = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + new SortedDictionary(), + new Queue(), + new Queue(), + lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None)); + + Assert.Equal("lastBatch", exception.ParamName); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenMaxConcurrentBatchesIsLessThanOne_Throws() + { + const int maxConcurrentBatches = 0; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + new SortedDictionary(), + new Queue(), + new Queue(), + _lastBatch, + maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None)); + + Assert.Equal("maxConcurrentBatches", exception.ParamName); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenProcessCommitItemBatchAsyncIsNull_Throws() + { + const ProcessCommitItemBatchAsync processCommitItemBatchAsync = null; + + var exception = Assert.Throws( + () => CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + new SortedDictionary(), + new Queue(), + new Queue(), + _lastBatch, + _maxConcurrentBatches, + processCommitItemBatchAsync, + CancellationToken.None)); + + Assert.Equal("processCommitItemBatchAsync", exception.ParamName); + } + + public class EnqueueBatchesIfNoFailures + { + private PackageIdentity _packageIdentityc = new PackageIdentity(id: "c", version: new NuGetVersion("1.0.0")); + private PackageIdentity _packageIdentityd = new PackageIdentity(id: "d", version: new NuGetVersion("1.0.0")); + private readonly DateTime _now = DateTime.UtcNow; + private readonly CatalogCommitItem _commitItem0; + private readonly CatalogCommitItem _commitItem1; + private readonly CatalogCommitItem _commitItem2; + private readonly CatalogCommitItem _commitItem3; + + public EnqueueBatchesIfNoFailures() + { + _commitItem0 = TestUtility.CreateCatalogCommitItem(_now, _packageIdentitya); + _commitItem1 = TestUtility.CreateCatalogCommitItem(_now, _packageIdentityb); + _commitItem2 = TestUtility.CreateCatalogCommitItem(_now.AddMinutes(1), _packageIdentityc); + _commitItem3 = TestUtility.CreateCatalogCommitItem(_now.AddMinutes(2), _packageIdentityd); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenAnyBatchIsFailed_DoesNotEnqueue() + { + var commitItemBatch = new CatalogCommitItemBatch(_now, _packageIdentitya.Id, new[] { _commitItem0, _commitItem1 }); + var commitBatchMap = CatalogCommitUtilities.CreateCommitBatchTasksMap(new[] { commitItemBatch }); + var batchTasks = new CatalogCommitBatchTasks(_now); + var failedBatchTask = new CatalogCommitBatchTask(_now, _packageIdentitya.Id) { Task = FailedTask }; + + batchTasks.BatchTasks.Add(failedBatchTask); + + var unprocessedBatches = new Queue(); + + unprocessedBatches.Enqueue(commitItemBatch); + + var processingBatches = new Queue(); + + processingBatches.Enqueue(failedBatchTask); + + CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + commitBatchMap, + unprocessedBatches, + processingBatches, + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None); + + Assert.Equal(1, unprocessedBatches.Count); + Assert.Equal(1, processingBatches.Count); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenNoBatchIsCancelled_DoesNotEnqueue() + { + var commitItemBatch = new CatalogCommitItemBatch(_now, _packageIdentitya.Id, new[] { _commitItem0, _commitItem1 }); + var commitBatchMap = CatalogCommitUtilities.CreateCommitBatchTasksMap(new[] { commitItemBatch }); + var batchTasks = new CatalogCommitBatchTasks(_now); + var cancelledBatchTask = new CatalogCommitBatchTask(_now, _packageIdentitya.Id) + { + Task = Task.FromCanceled(new CancellationToken(canceled: true)) + }; + + batchTasks.BatchTasks.Add(cancelledBatchTask); + + var unprocessedBatches = new Queue(); + + unprocessedBatches.Enqueue(commitItemBatch); + + var processingBatches = new Queue(); + + processingBatches.Enqueue(cancelledBatchTask); + + CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + commitBatchMap, + unprocessedBatches, + processingBatches, + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None); + + Assert.Equal(1, unprocessedBatches.Count); + Assert.Equal(1, processingBatches.Count); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenMaxConcurrencyLimitHit_DoesNotEnqueue() + { + var commitItemBatch0 = new CatalogCommitItemBatch(_now, _packageIdentitya.Id, new[] { _commitItem0 }); + var commitItemBatch1 = new CatalogCommitItemBatch(_now, _packageIdentityc.Id, new[] { _commitItem2 }); + var commitBatchMap = CatalogCommitUtilities.CreateCommitBatchTasksMap(new[] { commitItemBatch0, commitItemBatch1 }); + var batchTasks = new CatalogCommitBatchTasks(_now); + + using (var cancellationTokenSource = new CancellationTokenSource()) + { + var inprocessTask = new CatalogCommitBatchTask(_now, _packageIdentitya.Id) + { + Task = Task.Delay(TimeSpan.FromMilliseconds(-1), cancellationTokenSource.Token) + }; + + var unprocessedBatches = new Queue(); + + unprocessedBatches.Enqueue(commitItemBatch1); + + var processingBatches = new Queue(); + + processingBatches.Enqueue(inprocessTask); + + const int maxConcurrentBatches = 1; + + CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + commitBatchMap, + unprocessedBatches, + processingBatches, + _lastBatch, + maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None); + + Assert.Equal(1, unprocessedBatches.Count); + Assert.Equal(1, processingBatches.Count); + } + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenCanEnqueue_Enqueues() + { + var commitItemBatch = new CatalogCommitItemBatch(_now, _packageIdentitya.Id, new[] { _commitItem0 }); + var commitBatchMap = CatalogCommitUtilities.CreateCommitBatchTasksMap(new[] { commitItemBatch }); + var batchTasks = new CatalogCommitBatchTasks(_now); + + var unprocessedBatches = new Queue(); + + unprocessedBatches.Enqueue(commitItemBatch); + + var processingBatches = new Queue(); + + CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + commitBatchMap, + unprocessedBatches, + processingBatches, + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None); + + Assert.Equal(0, unprocessedBatches.Count); + Assert.Equal(1, processingBatches.Count); + } + + [Fact] + public void EnqueueBatchesIfNoFailures_WhenProcessingQueueContainsCompletedTasks_Enqueues() + { + var commitItemBatch0 = new CatalogCommitItemBatch(_now, _packageIdentitya.Id, new[] { _commitItem0 }); + var commitItemBatch1 = new CatalogCommitItemBatch(_now, _packageIdentityb.Id, new[] { _commitItem1 }); + var commitBatchMap = CatalogCommitUtilities.CreateCommitBatchTasksMap(new[] { commitItemBatch0, commitItemBatch1 }); + var batchTasks = new CatalogCommitBatchTasks(_now); + var completedTask = new CatalogCommitBatchTask(_now, _packageIdentitya.Id) { Task = Task.CompletedTask }; + + var unprocessedBatches = new Queue(); + + unprocessedBatches.Enqueue(commitItemBatch1); + + var processingBatches = new Queue(); + + processingBatches.Enqueue(completedTask); + + CatalogCommitUtilities.EnqueueBatchesIfNoFailures( + new CollectorHttpClient(), + new JObject(), + commitBatchMap, + unprocessedBatches, + processingBatches, + _lastBatch, + _maxConcurrentBatches, + NoOpProcessBatchAsync, + CancellationToken.None); + + Assert.Equal(0, unprocessedBatches.Count); + Assert.Equal(2, processingBatches.Count); + } + } + + [Fact] + public void GetPackageIdKey_WhenItemIsNull_Throws() + { + var exception = Assert.Throws( + () => CatalogCommitUtilities.GetPackageIdKey(item: null)); + + Assert.Equal("item", exception.ParamName); + } + + [Theory] + [InlineData("a")] + [InlineData("A")] + public void GetPackageIdKey_WhenPackageIdVariesInCase_ReturnsLowerCase(string packageId) + { + var commitItem = TestUtility.CreateCatalogCommitItem(DateTime.UtcNow, new PackageIdentity(packageId, new NuGetVersion("1.0.0"))); + var key = CatalogCommitUtilities.GetPackageIdKey(commitItem); + + Assert.Equal(packageId.ToLowerInvariant(), key); + } + + private static CatalogCommitItemBatch CreateCatalogCommitBatch( + DateTime commitTimeStamp, + PackageIdentity packageIdentity) + { + var commit = TestUtility.CreateCatalogCommitItem(commitTimeStamp, packageIdentity); + + return new CatalogCommitItemBatch(commitTimeStamp, packageIdentity.Id, new[] { commit }); + } + + private static Task NoOpProcessBatchAsync( + CollectorHttpClient client, + JToken context, + string packageId, + CatalogCommitItemBatch batch, + CatalogCommitItemBatch lastBatch, + CancellationToken cancellationToken) + { + return Task.CompletedTask; + } + } +} \ No newline at end of file diff --git a/tests/CatalogTests/CatalogIndexEntryTests.cs b/tests/CatalogTests/CatalogIndexEntryTests.cs index 8890fb2cf..ef1286e33 100644 --- a/tests/CatalogTests/CatalogIndexEntryTests.cs +++ b/tests/CatalogTests/CatalogIndexEntryTests.cs @@ -2,12 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Globalization; using System.Linq; using Newtonsoft.Json; using Newtonsoft.Json.Converters; using Newtonsoft.Json.Linq; using NgTests; +using NgTests.Infrastructure; +using NuGet.Packaging.Core; using NuGet.Protocol; using NuGet.Services.Metadata.Catalog; using NuGet.Versioning; @@ -21,8 +22,9 @@ public class CatalogIndexEntryTests private readonly string _commitId; private readonly DateTime _commitTimeStamp; - private const string _packageId = "A"; + private const string _packageId = "a"; private readonly NuGetVersion _packageVersion; + private readonly PackageIdentity _packageIdentity; private readonly Uri _uri; static CatalogIndexEntryTests() @@ -39,6 +41,7 @@ public CatalogIndexEntryTests() _commitTimeStamp = DateTime.UtcNow; _packageVersion = new NuGetVersion("1.2.3"); _uri = new Uri("https://nuget.test/a"); + _packageIdentity = new PackageIdentity(_packageId, _packageVersion); } [Fact] @@ -52,8 +55,7 @@ public void Constructor_WhenUriIsNull_Throws() CatalogConstants.NuGetPackageDetails, _commitId, _commitTimeStamp, - _packageId, - _packageVersion)); + _packageIdentity)); Assert.Equal("uri", exception.ParamName); } @@ -70,61 +72,70 @@ public void Constructor_WhenTypeIsNullEmptyOrWhitespace_Throws(string type) type, _commitId, _commitTimeStamp, - _packageId, - _packageVersion)); + _packageIdentity)); Assert.Equal("type", exception.ParamName); } - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - public void Constructor_WhenCommitIdIsNullEmptyOrWhitespace_Throws(string commitId) + [Fact] + public void Constructor_WhenTypesIsNull_Throws() { + string[] types = null; + var exception = Assert.Throws( () => new CatalogIndexEntry( _uri, - CatalogConstants.NuGetPackageDelete, - commitId, + types, + _commitId, _commitTimeStamp, - _packageId, - _packageVersion)); + _packageIdentity)); - Assert.Equal("commitId", exception.ParamName); + Assert.Equal("types", exception.ParamName); + } + + [Fact] + public void Constructor_WhenTypesIsEmpty_Throws() + { + var exception = Assert.Throws( + () => new CatalogIndexEntry( + _uri, + Array.Empty(), + _commitId, + _commitTimeStamp, + _packageIdentity)); + + Assert.Equal("types", exception.ParamName); } [Theory] [InlineData(null)] [InlineData("")] [InlineData(" ")] - public void Constructor_WhenIdIsNullEmptyOrWhitespace_Throws(string id) + public void Constructor_WhenCommitIdIsNullEmptyOrWhitespace_Throws(string commitId) { var exception = Assert.Throws( () => new CatalogIndexEntry( _uri, CatalogConstants.NuGetPackageDelete, - _commitId, + commitId, _commitTimeStamp, - id, - _packageVersion)); + _packageIdentity)); - Assert.Equal("id", exception.ParamName); + Assert.Equal("commitId", exception.ParamName); } [Fact] - public void Constructor_WhenVersionIsNull_Throws() + public void Constructor_WhenPackageIdentityIsNull_Throws() { var exception = Assert.Throws( () => new CatalogIndexEntry( _uri, - CatalogConstants.NuGetPackageDetails, + CatalogConstants.NuGetPackageDelete, _commitId, _commitTimeStamp, - _packageId, - version: null)); + packageIdentity: null)); - Assert.Equal("version", exception.ParamName); + Assert.Equal("packageIdentity", exception.ParamName); } [Fact] @@ -135,8 +146,7 @@ public void Constructor_WhenArgumentsAreValid_ReturnsInstance() CatalogConstants.NuGetPackageDetails, _commitId, _commitTimeStamp, - _packageId, - _packageVersion); + _packageIdentity); Assert.Equal(_uri.AbsoluteUri, entry.Uri.AbsoluteUri); Assert.Equal(CatalogConstants.NuGetPackageDetails, entry.Types.Single()); @@ -147,57 +157,30 @@ public void Constructor_WhenArgumentsAreValid_ReturnsInstance() } [Fact] - public void Create_WhenTokenIsNull_Throws() + public void Create_WhenCommitItemIsNull_Throws() { - var exception = Assert.Throws(() => CatalogIndexEntry.Create(token: null)); + var exception = Assert.Throws(() => CatalogIndexEntry.Create(commitItem: null)); - Assert.Equal("token", exception.ParamName); + Assert.Equal("commitItem", exception.ParamName); } [Fact] public void Create_WhenArgumentIsValid_ReturnsInstance() { - var jObject = CreateCatalogIndexJObject(); + var contextJObject = TestUtility.CreateCatalogContextJObject(); + var commitItemJObject = TestUtility.CreateCatalogCommitItemJObject(_commitTimeStamp, _packageIdentity); + var commitItem = CatalogCommitItem.Create(contextJObject, commitItemJObject); - var entry = CatalogIndexEntry.Create(jObject); + var entry = CatalogIndexEntry.Create(commitItem); Assert.Equal(_uri.AbsoluteUri, entry.Uri.AbsoluteUri); Assert.Equal(CatalogConstants.NuGetPackageDetails, entry.Types.Single()); - Assert.Equal(_commitId, entry.CommitId); - Assert.Equal(_commitTimeStamp, entry.CommitTimeStamp); + Assert.Equal(commitItemJObject[CatalogConstants.CommitId].ToString(), entry.CommitId); + Assert.Equal(_commitTimeStamp, entry.CommitTimeStamp.ToUniversalTime()); Assert.Equal(_packageId, entry.Id); Assert.Equal(_packageVersion, entry.Version); } - [Theory] - [InlineData("2018-10-15T01:12:40.1234567Z")] - [InlineData("2018-10-15T01:12:40.123456Z")] - [InlineData("2018-10-15T01:12:40.12345Z")] - [InlineData("2018-10-15T01:12:40.1234Z")] - [InlineData("2018-10-15T01:12:40.123Z")] - [InlineData("2018-10-15T01:12:40.12Z")] - [InlineData("2018-10-15T01:12:40.1Z")] - public void Create_WhenCommitTimeStampVariesInSignificantDigits_DeserializesCorrectly(string commitTimeStamp) - { - var jObject = new JObject( - new JProperty(CatalogConstants.IdKeyword, _uri), - new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.NuGetPackageDetails), - new JProperty(CatalogConstants.CommitId, _commitId), - new JProperty(CatalogConstants.CommitTimeStamp, commitTimeStamp), - new JProperty(CatalogConstants.NuGetId, _packageId), - new JProperty(CatalogConstants.NuGetVersion, _packageVersion.ToNormalizedString())); - - var entry = CatalogIndexEntry.Create(jObject); - - var expectedCommitTimeStamp = DateTime.ParseExact( - commitTimeStamp, - CatalogConstants.CommitTimeStampFormat, - DateTimeFormatInfo.CurrentInfo, - DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal); - - Assert.Equal(expectedCommitTimeStamp, entry.CommitTimeStamp); - } - [Fact] public void CompareTo_WhenOtherIsNull_Throws() { @@ -206,8 +189,7 @@ public void CompareTo_WhenOtherIsNull_Throws() CatalogConstants.NuGetPackageDetails, _commitId, _commitTimeStamp, - _packageId, - _packageVersion); + _packageIdentity); var exception = Assert.Throws(() => entry.CompareTo(other: null)); @@ -223,15 +205,13 @@ public void CompareTo_WhenCommitTimeStampsAreNotEqual_ReturnsNonZero() CatalogConstants.NuGetPackageDetails, _commitId, now.AddHours(-1), - _packageId, - _packageVersion); + _packageIdentity); var newerEntry = new CatalogIndexEntry( _uri, CatalogConstants.NuGetPackageDetails, _commitId, now, - _packageId, - _packageVersion); + _packageIdentity); Assert.Equal(-1, olderEntry.CompareTo(newerEntry)); Assert.Equal(1, newerEntry.CompareTo(olderEntry)); @@ -240,23 +220,22 @@ public void CompareTo_WhenCommitTimeStampsAreNotEqual_ReturnsNonZero() [Fact] public void CompareTo_WhenCommitTimeStampsAreEqual_ReturnsZero() { - var entry1 = new CatalogIndexEntry( - _uri, + var entry0 = new CatalogIndexEntry( + new Uri("https://nuget.test/a"), CatalogConstants.NuGetPackageDetails, _commitId, _commitTimeStamp, - _packageId, - _packageVersion); - var entry2 = new CatalogIndexEntry( - _uri, - CatalogConstants.NuGetPackageDetails, - _commitId, + _packageIdentity); + var entry1 = new CatalogIndexEntry( + new Uri("https://nuget.test/b"), + CatalogConstants.NuGetPackageDelete, + Guid.NewGuid().ToString(), _commitTimeStamp, - _packageId, - _packageVersion); + new PackageIdentity(id: "b", version: new NuGetVersion("4.5.6"))); - Assert.Equal(0, entry1.CompareTo(entry2)); - Assert.Equal(0, entry2.CompareTo(entry1)); + Assert.Equal(0, entry0.CompareTo(entry0)); + Assert.Equal(0, entry0.CompareTo(entry1)); + Assert.Equal(0, entry1.CompareTo(entry0)); } [Fact] @@ -267,8 +246,7 @@ public void IsDelete_WhenTypeIsNotPackageDelete_ReturnsFalse() CatalogConstants.NuGetPackageDetails, _commitId, _commitTimeStamp, - _packageId, - _packageVersion); + _packageIdentity); Assert.False(entry.IsDelete); } @@ -281,8 +259,7 @@ public void IsDelete_WhenTypeIsPackageDelete_ReturnsTrue() CatalogConstants.NuGetPackageDelete, _commitId, _commitTimeStamp, - _packageId, - _packageVersion); + _packageIdentity); Assert.True(entry.IsDelete); } @@ -295,8 +272,7 @@ public void JsonSerialization_ReturnsCorrectJson() CatalogConstants.NuGetPackageDetails, _commitId, _commitTimeStamp, - _packageId, - _packageVersion); + _packageIdentity); var jObject = CreateCatalogIndexJObject(); diff --git a/tests/CatalogTests/CatalogTests.csproj b/tests/CatalogTests/CatalogTests.csproj index c77efa1f3..f333dfcc1 100644 --- a/tests/CatalogTests/CatalogTests.csproj +++ b/tests/CatalogTests/CatalogTests.csproj @@ -71,8 +71,15 @@ + + + + + + + @@ -177,16 +184,16 @@ PreserveNewest - PreserveNewest + PreserveNewest - PreserveNewest + PreserveNewest - PreserveNewest + PreserveNewest - PreserveNewest + PreserveNewest PreserveNewest diff --git a/tests/NgTests/DnxCatalogCollectorTests.cs b/tests/NgTests/DnxCatalogCollectorTests.cs index 4a03bbfec..fb5dbd3a3 100644 --- a/tests/NgTests/DnxCatalogCollectorTests.cs +++ b/tests/NgTests/DnxCatalogCollectorTests.cs @@ -305,14 +305,13 @@ public async Task RunAsync_WithValidPackage_CreatesFlatContainer() var nupkgUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.0/listedpackage.1.0.0.nupkg"); var nuspecUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.0/listedpackage.nuspec"); var catalogStorage = Catalogs.CreateTestCatalogWithThreePackagesAndDelete(); - var nupkgStream = File.OpenRead("Packages\\ListedPackage.1.0.0.zip"); - var expectedNupkg = GetStreamBytes(nupkgStream); + var expectedNupkg = File.ReadAllBytes("Packages\\ListedPackage.1.0.0.zip"); await _mockServer.AddStorageAsync(catalogStorage); _mockServer.SetAction( "/packages/listedpackage.1.0.0.nupkg", - request => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StreamContent(nupkgStream) })); + request => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(expectedNupkg) })); var front = new DurableCursor(_cursorJsonUri, _catalogToDnxStorage, MemoryCursor.MinValue); ReadCursor back = MemoryCursor.CreateMax(); @@ -409,7 +408,6 @@ public async Task RunAsync_WithNonIAzureStorage_WhenPackageIsAlreadySynchronized var nupkgUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.1/listedpackage.1.0.1.nupkg"); var nuspecUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.1/listedpackage.nuspec"); var nupkgStream = File.OpenRead("Packages\\ListedPackage.1.0.1.zip"); - var expectedNupkg = GetStreamBytes(nupkgStream); await _catalogToDnxStorage.SaveAsync( new Uri("http://tempuri.org/listedpackage/index.json"), @@ -464,7 +462,6 @@ public async Task RunAsync_WithIAzureStorage_WhenPackageIsAlreadySynchronizedAnd var nupkgUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.0/listedpackage.1.0.0.nupkg"); var nuspecUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.0/listedpackage.nuspec"); var nupkgStream = File.OpenRead("Packages\\ListedPackage.1.0.0.zip"); - var expectedNupkg = GetStreamBytes(nupkgStream); await _catalogToDnxStorage.SaveAsync( new Uri("http://tempuri.org/listedpackage/index.json"), @@ -518,8 +515,7 @@ public async Task RunAsync_WithFakeIAzureStorage_WhenPackageIsAlreadySynchronize var indexJsonUri = _catalogToDnxStorage.ResolveUri("/listedpackage/index.json"); var nupkgUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.0/listedpackage.1.0.0.nupkg"); var nuspecUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.0/listedpackage.nuspec"); - var nupkgStream = File.OpenRead("Packages\\ListedPackage.1.0.0.zip"); - var expectedNupkg = GetStreamBytes(nupkgStream); + var expectedNupkg = File.ReadAllBytes("Packages\\ListedPackage.1.0.0.zip"); await _catalogToDnxStorage.SaveAsync( new Uri("http://tempuri.org/listedpackage/index.json"), @@ -541,7 +537,7 @@ await _catalogToDnxStorage.SaveAsync( _mockServer.SetAction( "/packages/listedpackage.1.0.0.nupkg", - request => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StreamContent(nupkgStream) })); + request => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(expectedNupkg) })); var front = new DurableCursor(_cursorJsonUri, _catalogToDnxStorage, MemoryCursor.MinValue); ReadCursor back = MemoryCursor.CreateMax(); @@ -753,15 +749,14 @@ public async Task RunAsync_WhenMultipleEntriesWithSamePackageIdentityInSameBatch var indexJsonUri = _catalogToDnxStorage.ResolveUri("/listedpackage/index.json"); var nupkgUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.0/listedpackage.1.0.0.nupkg"); var nuspecUri = _catalogToDnxStorage.ResolveUri("/listedpackage/1.0.0/listedpackage.nuspec"); - var nupkgStream = File.OpenRead(@"Packages\ListedPackage.1.0.1.zip"); - var expectedNupkg = GetStreamBytes(nupkgStream); + var expectedNupkg = File.ReadAllBytes(@"Packages\ListedPackage.1.0.0.zip"); var catalogStorage = Catalogs.CreateTestCatalogWithMultipleEntriesWithSamePackageIdentityInSameBatch(); await _mockServer.AddStorageAsync(catalogStorage); _mockServer.SetAction( "/packages/listedpackage.1.0.0.nupkg", - request => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StreamContent(nupkgStream) })); + request => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(expectedNupkg) })); var front = new DurableCursor(_cursorJsonUri, _catalogToDnxStorage, MemoryCursor.MinValue); ReadCursor back = MemoryCursor.CreateMax(); @@ -772,20 +767,6 @@ public async Task RunAsync_WhenMultipleEntriesWithSamePackageIdentityInSameBatch Assert.Equal("The catalog batch 10/13/2015 6:40:07 AM contains multiple entries for the same package identity. Package(s): listedpackage 1.0.0", exception.Message); } - private static byte[] GetStreamBytes(Stream srcStream) - { - using (var memoryStream = new MemoryStream()) - { - srcStream.Position = 0; - - srcStream.CopyTo(memoryStream); - - srcStream.Position = 0; - - return memoryStream.ToArray(); - } - } - private static string GetExpectedIndexJsonContent(string version) { return $"{{\r\n \"versions\": [\r\n \"{version}\"\r\n ]\r\n}}"; diff --git a/tests/NgTests/Infrastructure/TestUtility.cs b/tests/NgTests/Infrastructure/TestUtility.cs index 752978d1e..710b0dcec 100644 --- a/tests/NgTests/Infrastructure/TestUtility.cs +++ b/tests/NgTests/Infrastructure/TestUtility.cs @@ -3,6 +3,9 @@ using System; using System.Linq; +using Newtonsoft.Json.Linq; +using NuGet.Packaging.Core; +using NuGet.Services.Metadata.Catalog; namespace NgTests.Infrastructure { @@ -24,5 +27,50 @@ public static string CreateRandomAlphanumericString(Random random) .Select(s => s[random.Next(s.Length)]) .ToArray()); } + + public static JObject CreateCatalogContextJObject() + { + return new JObject( + new JProperty(CatalogConstants.VocabKeyword, CatalogConstants.NuGetSchemaUri), + new JProperty(CatalogConstants.NuGet, CatalogConstants.NuGetSchemaUri), + new JProperty(CatalogConstants.Items, + new JObject( + new JProperty(CatalogConstants.IdKeyword, CatalogConstants.Item), + new JProperty(CatalogConstants.ContainerKeyword, CatalogConstants.SetKeyword))), + new JProperty(CatalogConstants.Parent, + new JObject( + new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.IdKeyword))), + new JProperty(CatalogConstants.CommitTimeStamp, + new JObject( + new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.XsdDateTime))), + new JProperty(CatalogConstants.NuGetLastCreated, + new JObject( + new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.XsdDateTime))), + new JProperty(CatalogConstants.NuGetLastEdited, + new JObject( + new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.XsdDateTime))), + new JProperty(CatalogConstants.NuGetLastDeleted, + new JObject( + new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.XsdDateTime)))); + } + + public static JObject CreateCatalogCommitItemJObject(DateTime commitTimeStamp, PackageIdentity packageIdentity) + { + return new JObject( + new JProperty(CatalogConstants.IdKeyword, $"https://nuget.test/{packageIdentity.Id}"), + new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.NuGetPackageDetails), + new JProperty(CatalogConstants.CommitTimeStamp, commitTimeStamp.ToString("O")), + new JProperty(CatalogConstants.CommitId, Guid.NewGuid().ToString()), + new JProperty(CatalogConstants.NuGetId, packageIdentity.Id), + new JProperty(CatalogConstants.NuGetVersion, packageIdentity.Version.ToNormalizedString())); + } + + public static CatalogCommitItem CreateCatalogCommitItem(DateTime commitTimeStamp, PackageIdentity packageIdentity) + { + var context = CreateCatalogContextJObject(); + var commitItem = CreateCatalogCommitItemJObject(commitTimeStamp, packageIdentity); + + return CatalogCommitItem.Create(context, commitItem); + } } } \ No newline at end of file diff --git a/tests/NgTests/PackageFixup/FixPackageHashHandlerFacts.cs b/tests/NgTests/PackageFixup/FixPackageHashHandlerFacts.cs index 48174a863..145ec7be3 100644 --- a/tests/NgTests/PackageFixup/FixPackageHashHandlerFacts.cs +++ b/tests/NgTests/PackageFixup/FixPackageHashHandlerFacts.cs @@ -9,9 +9,9 @@ using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.WindowsAzure.Storage; -using Microsoft.WindowsAzure.Storage.Blob; using Moq; using NgTests.Infrastructure; +using NuGet.Packaging.Core; using NuGet.Services.Metadata.Catalog; using NuGet.Services.Metadata.Catalog.Persistence; using NuGet.Versioning; @@ -21,8 +21,7 @@ namespace NgTests.PackageFixup { public class FixPackageHashHandlerFacts { - private const string PackageId = "TestUnsigned"; - private static readonly NuGetVersion PackageVersion = new NuGetVersion("1.0.0"); + private static readonly PackageIdentity PackageIdentity = new PackageIdentity("TestUnsigned", new NuGetVersion("1.0.0")); private readonly CatalogIndexEntry _packageEntry; private readonly Mock _blob; @@ -46,8 +45,7 @@ public FixPackageHashHandlerFacts() "nuget:PackageDetails", "123", DateTime.UtcNow, - PackageId, - PackageVersion); + PackageIdentity); _blob = new Mock(); _blob.Setup(b => b.Uri).Returns(new Uri("http://localhost/packages/testunsigned.1.0.0.nupkg")); @@ -70,8 +68,8 @@ public async Task SkipsPackagesThatAlreadyHaveAHash() // Assert _blob.Verify(b => b.FetchAttributesAsync(It.IsAny()), Times.Once); - _telemetryService.Verify(t => t.TrackPackageAlreadyHasHash(PackageId, PackageVersion), Times.Once); - _telemetryService.Verify(t => t.TrackPackageHashFixed(PackageId, PackageVersion), Times.Never); + _telemetryService.Verify(t => t.TrackPackageAlreadyHasHash(PackageIdentity.Id, PackageIdentity.Version), Times.Once); + _telemetryService.Verify(t => t.TrackPackageHashFixed(PackageIdentity.Id, PackageIdentity.Version), Times.Never); } [Fact] @@ -98,8 +96,8 @@ public async Task AddsHashIfPackageIsMissingHash() null), Times.Once); - _telemetryService.Verify(t => t.TrackPackageAlreadyHasHash(PackageId, PackageVersion), Times.Never); - _telemetryService.Verify(t => t.TrackPackageHashFixed(PackageId, PackageVersion), Times.Once); + _telemetryService.Verify(t => t.TrackPackageAlreadyHasHash(PackageIdentity.Id, PackageIdentity.Version), Times.Never); + _telemetryService.Verify(t => t.TrackPackageHashFixed(PackageIdentity.Id, PackageIdentity.Version), Times.Once); } } -} +} \ No newline at end of file diff --git a/tests/NgTests/PackageFixup/PackagesContainerCatalogProcessorFacts.cs b/tests/NgTests/PackageFixup/PackagesContainerCatalogProcessorFacts.cs index 7688301a6..b9dc67666 100644 --- a/tests/NgTests/PackageFixup/PackagesContainerCatalogProcessorFacts.cs +++ b/tests/NgTests/PackageFixup/PackagesContainerCatalogProcessorFacts.cs @@ -11,6 +11,7 @@ using Microsoft.WindowsAzure.Storage; using Microsoft.WindowsAzure.Storage.Blob; using Moq; +using NuGet.Packaging.Core; using NuGet.Services.Metadata.Catalog; using NuGet.Services.Metadata.Catalog.Persistence; using NuGet.Versioning; @@ -20,8 +21,7 @@ namespace NgTests.PackageFixup { public class PackagesContainerCatalogProcessorFacts { - private const string PackageId = "TestPackage"; - private static readonly NuGetVersion PackageVersion = new NuGetVersion("1.0.0"); + private static readonly PackageIdentity PackageIdentity = new PackageIdentity("TestPackage", new NuGetVersion("1.0.0")); private readonly CatalogIndexEntry _catalogEntry; private readonly Mock _rawBlob; @@ -41,8 +41,7 @@ public PackagesContainerCatalogProcessorFacts() "nuget:PackageDetails", "123", DateTime.UtcNow, - PackageId, - PackageVersion); + PackageIdentity); _handler = new Mock(); _telemetryService = new Mock(); diff --git a/tests/NgTests/PackageFixup/ValidatePackageHashHandlerFacts.cs b/tests/NgTests/PackageFixup/ValidatePackageHashHandlerFacts.cs index ff1f683cc..27d7284ed 100644 --- a/tests/NgTests/PackageFixup/ValidatePackageHashHandlerFacts.cs +++ b/tests/NgTests/PackageFixup/ValidatePackageHashHandlerFacts.cs @@ -8,9 +8,9 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; -using Microsoft.WindowsAzure.Storage.Blob; using Moq; using NgTests.Infrastructure; +using NuGet.Packaging.Core; using NuGet.Services.Metadata.Catalog; using NuGet.Services.Metadata.Catalog.Persistence; using NuGet.Versioning; @@ -20,8 +20,7 @@ namespace NgTests.PackageFixup { public class ValidatePackageHashHandlerFacts { - private const string PackageId = "TestUnsigned"; - private static readonly NuGetVersion PackageVersion = new NuGetVersion("1.0.0"); + private static readonly PackageIdentity PackageIdentity = new PackageIdentity("TestUnsigned", new NuGetVersion("1.0.0")); private readonly CatalogIndexEntry _packageEntry; private readonly Mock _blob; @@ -45,8 +44,7 @@ public ValidatePackageHashHandlerFacts() "nuget:PackageDetails", "123", DateTime.UtcNow, - PackageId, - PackageVersion); + PackageIdentity); _blob = new Mock(); _blob.Setup(b => b.Uri).Returns(new Uri("http://localhost/packages/testunsigned.1.0.0.nupkg")); @@ -69,8 +67,8 @@ public async Task ReportsPackagesThatAreMissingAHash() // Assert _blob.Verify(b => b.FetchAttributesAsync(It.IsAny()), Times.Once); - _telemetryService.Verify(t => t.TrackPackageMissingHash(PackageId, PackageVersion), Times.Once); - _telemetryService.Verify(t => t.TrackPackageHasIncorrectHash(PackageId, PackageVersion), Times.Never); + _telemetryService.Verify(t => t.TrackPackageMissingHash(PackageIdentity.Id, PackageIdentity.Version), Times.Once); + _telemetryService.Verify(t => t.TrackPackageHasIncorrectHash(PackageIdentity.Id, PackageIdentity.Version), Times.Never); } [Fact] @@ -84,8 +82,8 @@ public async Task ReportsPackagesWithIncorrectHash() // Assert _blob.Verify(b => b.FetchAttributesAsync(It.IsAny()), Times.Once); - _telemetryService.Verify(t => t.TrackPackageMissingHash(PackageId, PackageVersion), Times.Never); - _telemetryService.Verify(t => t.TrackPackageHasIncorrectHash(PackageId, PackageVersion), Times.Once); + _telemetryService.Verify(t => t.TrackPackageMissingHash(PackageIdentity.Id, PackageIdentity.Version), Times.Never); + _telemetryService.Verify(t => t.TrackPackageHasIncorrectHash(PackageIdentity.Id, PackageIdentity.Version), Times.Once); } [Fact] @@ -99,8 +97,8 @@ public async Task ReportsNothingIfPackageHasCorrectHash() // Assert _blob.Verify(b => b.FetchAttributesAsync(It.IsAny()), Times.Once); - _telemetryService.Verify(t => t.TrackPackageMissingHash(PackageId, PackageVersion), Times.Never); - _telemetryService.Verify(t => t.TrackPackageHasIncorrectHash(PackageId, PackageVersion), Times.Never); + _telemetryService.Verify(t => t.TrackPackageMissingHash(PackageIdentity.Id, PackageIdentity.Version), Times.Never); + _telemetryService.Verify(t => t.TrackPackageHasIncorrectHash(PackageIdentity.Id, PackageIdentity.Version), Times.Never); } } } diff --git a/tests/NgTests/PackageMonitoringStatusServiceTests.cs b/tests/NgTests/PackageMonitoringStatusServiceTests.cs index 9db1ff76d..c08e25a0c 100644 --- a/tests/NgTests/PackageMonitoringStatusServiceTests.cs +++ b/tests/NgTests/PackageMonitoringStatusServiceTests.cs @@ -123,7 +123,10 @@ private static CatalogIndexEntry CreateCatalogIndexEntry(string id, string versi { return new CatalogIndexEntry( new UriBuilder() { Path = $"{id.ToLowerInvariant()}/{id.ToLowerInvariant()}.{version.ToLowerInvariant()}" }.Uri, - "nuget:PackageDetails", Guid.NewGuid().ToString(), DateTime.UtcNow, id, new NuGetVersion(version)); + CatalogConstants.NuGetPackageDetails, + Guid.NewGuid().ToString(), + DateTime.UtcNow, + new PackageIdentity(id, new NuGetVersion(version))); } private static DeletionAuditEntry CreateDeletionAuditEntry(string id, string version) diff --git a/tests/NgTests/PackageTimestampMetadataTests.cs b/tests/NgTests/PackageTimestampMetadataTests.cs index da94288bd..72014a753 100644 --- a/tests/NgTests/PackageTimestampMetadataTests.cs +++ b/tests/NgTests/PackageTimestampMetadataTests.cs @@ -4,12 +4,12 @@ using System; using System.Collections.Generic; using System.Globalization; -using System.Linq; using System.Net; using System.Net.Http; using System.Threading.Tasks; using NgTests.Data; using NgTests.Infrastructure; +using NuGet.Packaging.Core; using NuGet.Services.Metadata.Catalog; using NuGet.Services.Metadata.Catalog.Monitoring; using NuGet.Versioning; @@ -107,8 +107,7 @@ public async Task FromCatalogEntry_HandlesCreatedLastEdited() CatalogConstants.NuGetPackageDetails, "9a37734f-1960-4c07-8934-c8bc797e35c1", commitTimeStamp1, - "ListedPackage", - new NuGetVersion("1.0.0"))), + new PackageIdentity("ListedPackage", new NuGetVersion("1.0.0")))), PackageTimestampMetadata.FromCatalogEntry( client, new CatalogIndexEntry( @@ -116,8 +115,7 @@ public async Task FromCatalogEntry_HandlesCreatedLastEdited() CatalogConstants.NuGetPackageDetails, "9a37734f-1960-4c07-8934-c8bc797e35c1", commitTimeStamp1, - "UnlistedPackage", - new NuGetVersion("1.0.0"))), + new PackageIdentity("UnlistedPackage", new NuGetVersion("1.0.0")))), PackageTimestampMetadata.FromCatalogEntry( client, new CatalogIndexEntry( @@ -125,8 +123,7 @@ public async Task FromCatalogEntry_HandlesCreatedLastEdited() CatalogConstants.NuGetPackageDetails, "8a9e7694-73d4-4775-9b7a-20aa59b9773e", commitTimeStamp2, - "ListedPackage", - new NuGetVersion("1.0.1"))), + new PackageIdentity("ListedPackage", new NuGetVersion("1.0.1")))) }; // Act @@ -162,8 +159,7 @@ public async Task FromCatalogEntry_HandlesDeleted_NotNull() CatalogConstants.CommitTimeStampFormat, DateTimeFormatInfo.CurrentInfo, DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal), - "OtherPackage", - new NuGetVersion("1.0.0")); + new PackageIdentity("OtherPackage", new NuGetVersion("1.0.0"))); // Act var entry = await PackageTimestampMetadata.FromCatalogEntry(client, catalogIndexEntry); diff --git a/tests/NgTests/SortingIdVersionCollectorTests.cs b/tests/NgTests/SortingIdVersionCollectorTests.cs index 8db71735b..71f9ac00f 100644 --- a/tests/NgTests/SortingIdVersionCollectorTests.cs +++ b/tests/NgTests/SortingIdVersionCollectorTests.cs @@ -7,8 +7,11 @@ using System.Threading.Tasks; using Moq; using Newtonsoft.Json.Linq; +using NgTests.Infrastructure; +using NuGet.Packaging.Core; using NuGet.Services.Metadata.Catalog; using NuGet.Services.Metadata.Catalog.Helpers; +using NuGet.Versioning; using Xunit; namespace NgTests @@ -22,7 +25,7 @@ public static IEnumerable BatchesData // Different id, same version yield return new object[] { - new List + new List { CreatePackage("test1", "1.0.0"), CreatePackage("test2", "1.0.0"), @@ -34,7 +37,7 @@ public static IEnumerable BatchesData // Same id, different version yield return new object[] { - new List + new List { CreatePackage("test1", "1.0.0"), CreatePackage("test1", "2.0.0"), @@ -46,7 +49,7 @@ public static IEnumerable BatchesData // Same id, same version yield return new object[] { - new List + new List { CreatePackage("test1", "1.0.0"), CreatePackage("test1", "1.0.0"), @@ -59,7 +62,7 @@ public static IEnumerable BatchesData [Theory] [MemberData(nameof(BatchesData))] - public async Task OnProcessBatch_BatchesCorrectly(IEnumerable items) + public async Task OnProcessBatch_BatchesCorrectly(IEnumerable items) { // Arrange var collectorMock = new Mock() @@ -70,8 +73,8 @@ public async Task OnProcessBatch_BatchesCorrectly(IEnumerable items) var seenPackages = new List(); collectorMock - .Setup(x => x.OverridableProcessSortedBatch(It.IsAny>>())) - .Returns>>( + .Setup(x => x.OverridableProcessSortedBatch(It.IsAny>>())) + .Returns>>( (pair) => { // Assert @@ -91,33 +94,33 @@ public async Task OnProcessBatch_BatchesCorrectly(IEnumerable items) var result = await collectorMock.Object.OnProcessBatchAsync(items); } - private static JToken CreatePackage(string id, string version) + private static CatalogCommitItem CreatePackage(string id, string version) { - return new JObject - { - { "nuget:id", id }, - { "nuget:version", version } - }; + var context = TestUtility.CreateCatalogContextJObject(); + var packageIdentity = new PackageIdentity(id, new NuGetVersion(version)); + var commitItem = TestUtility.CreateCatalogCommitItemJObject(DateTime.UtcNow, packageIdentity); + + return CatalogCommitItem.Create(context, commitItem); } public class TestableSortingIdVersionCollector : SortingIdVersionCollector { public TestableSortingIdVersionCollector() : base( - new Uri("https://www.microsoft.com"), - new Mock().Object, - null) + new Uri("https://nuget.test"), + Mock.Of(), + handlerFunc: null) { } - public Task OnProcessBatchAsync(IEnumerable items) + public Task OnProcessBatchAsync(IEnumerable items) { return base.OnProcessBatchAsync(null, items, null, DateTime.MinValue, false, CancellationToken.None); } protected override Task ProcessSortedBatchAsync( CollectorHttpClient client, - KeyValuePair> sortedBatch, + KeyValuePair> sortedBatch, JToken context, CancellationToken cancellationToken) { @@ -125,7 +128,7 @@ protected override Task ProcessSortedBatchAsync( } public virtual Task OverridableProcessSortedBatch( - KeyValuePair> sortedBatch) + KeyValuePair> sortedBatch) { return Task.FromResult(0); } diff --git a/tests/NgTests/Validation/CatalogAggregateValidatorFacts.cs b/tests/NgTests/Validation/CatalogAggregateValidatorFacts.cs index f53fe87b8..24201a1dc 100644 --- a/tests/NgTests/Validation/CatalogAggregateValidatorFacts.cs +++ b/tests/NgTests/Validation/CatalogAggregateValidatorFacts.cs @@ -190,8 +190,7 @@ private static ValidationContext CreateContext(CollectorHttpClient client, DateT CatalogConstants.NuGetPackageDetails, Guid.NewGuid().ToString(), commitTimeStamp, - _packageIdentity.Id, - _packageIdentity.Version) + _packageIdentity) }; return new ValidationContext( diff --git a/tests/NgTests/Validation/PackageHasSignatureValidatorFacts.cs b/tests/NgTests/Validation/PackageHasSignatureValidatorFacts.cs index 3da2fc742..880b3b2d7 100644 --- a/tests/NgTests/Validation/PackageHasSignatureValidatorFacts.cs +++ b/tests/NgTests/Validation/PackageHasSignatureValidatorFacts.cs @@ -96,7 +96,7 @@ public void SkipsIfNoEntries() public void SkipsIfLatestEntryIsDelete() { var target = CreateTarget(); - var uri = new Uri($"https://nuget.test/{PackageId}"); + var uri = new Uri($"https://nuget.test/{PackageIdentity.Id}"); var context = CreateValidationContext( catalogEntries: new[] { @@ -105,15 +105,13 @@ public void SkipsIfLatestEntryIsDelete() type: CatalogConstants.NuGetPackageDetails, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue, - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), new CatalogIndexEntry( uri, type: CatalogConstants.NuGetPackageDelete, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue.AddDays(1), - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), }); Assert.False(target.ShouldRunValidator(context)); @@ -123,7 +121,7 @@ public void SkipsIfLatestEntryIsDelete() public void RunsIfLatestEntryIsntDelete() { var target = CreateTarget(); - var uri = new Uri($"https://nuget.test/{PackageId}"); + var uri = new Uri($"https://nuget.test/{PackageIdentity.Id}"); var context = CreateValidationContext( catalogEntries: new[] { @@ -132,15 +130,13 @@ public void RunsIfLatestEntryIsntDelete() type: CatalogConstants.NuGetPackageDelete, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue, - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), new CatalogIndexEntry( uri, type: CatalogConstants.NuGetPackageDetails, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue.AddDays(1), - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), }); Assert.True(target.ShouldRunValidator(context)); @@ -162,15 +158,13 @@ public async Task ReturnsGracefullyIfLatestLeafHasSignatureFile() type: CatalogConstants.NuGetPackageDetails, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue, - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), new CatalogIndexEntry( uri: new Uri("https://nuget.test/b.json"), type: CatalogConstants.NuGetPackageDetails, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue.AddDays(1), - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), }); AddCatalogLeaf("/a.json", new CatalogLeaf @@ -209,15 +203,13 @@ public async Task ThrowsIfLatestLeafIsMissingASignatureFile() type: CatalogConstants.NuGetPackageDetails, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue, - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), new CatalogIndexEntry( uri: malformedUri, type: CatalogConstants.NuGetPackageDetails, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue.AddDays(1), - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), }); AddCatalogLeaf("/a.json", new CatalogLeaf @@ -257,8 +249,7 @@ public async Task ThrowsIfLeafPackageEntriesIsMissing() type: CatalogConstants.NuGetPackageDetails, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue, - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), }); AddCatalogLeaf("/a.json", "{ 'this': 'is missing the packageEntries field' }"); @@ -284,8 +275,7 @@ public async Task ThrowsIfLeafPackageEntriesIsMalformed() type: CatalogConstants.NuGetPackageDetails, commitId: Guid.NewGuid().ToString(), commitTs: DateTime.MinValue, - id: PackageId, - version: PackageNuGetVersion), + packageIdentity: PackageIdentity), }); AddCatalogLeaf("/a.json", "{ 'packageEntries': 'malformed'}"); @@ -299,10 +289,7 @@ public async Task ThrowsIfLeafPackageEntriesIsMalformed() public class FactsBase { - public const string PackageId = "TestPackage"; - public const string PackageVersion = "1.0.0"; - - public static readonly NuGetVersion PackageNuGetVersion = NuGetVersion.Parse(PackageVersion); + public static readonly PackageIdentity PackageIdentity = new PackageIdentity("TestPackage", NuGetVersion.Parse("1.0.0")); protected readonly Mock> _logger; private readonly MockServerHttpClientHandler _mockServer; @@ -320,7 +307,7 @@ protected ValidationContext CreateValidationContext(IEnumerable _catalogEntries; @@ -205,8 +203,7 @@ public FactsBase() CatalogConstants.NuGetPackageDetails, Guid.NewGuid().ToString(), DateTime.UtcNow, - PackageId, - PackageNuGetVersion) + PackageIdentity) }; AddCatalogLeafToMockServer("/catalog/leaf.json", new CatalogLeaf @@ -252,7 +249,7 @@ protected ValidationContext CreateValidationContext(string packageResource = nul var httpClient = new CollectorHttpClient(_mockServer); return new ValidationContext( - new PackageIdentity(PackageId, PackageNuGetVersion), + PackageIdentity, _catalogEntries, new DeletionAuditEntry[0], httpClient, diff --git a/tests/NgTests/Validation/PackageValidatorContextTests.cs b/tests/NgTests/Validation/PackageValidatorContextTests.cs index c60b0b347..83083e29a 100644 --- a/tests/NgTests/Validation/PackageValidatorContextTests.cs +++ b/tests/NgTests/Validation/PackageValidatorContextTests.cs @@ -3,6 +3,7 @@ using System; using System.Linq; +using NuGet.Packaging.Core; using NuGet.Services.Metadata.Catalog; using NuGet.Services.Metadata.Catalog.Helpers; using NuGet.Services.Metadata.Catalog.Monitoring; @@ -47,8 +48,7 @@ public void Constructor_WhenArgumentsAreValid_InitializesInstance() CatalogConstants.NuGetPackageDetails, Guid.NewGuid().ToString(), DateTime.UtcNow, - "a", - new NuGetVersion("1.0.0")) + new PackageIdentity(id: "a", version: new NuGetVersion("1.0.0"))) }; var context = new PackageValidatorContext(_package, catalogEntries); From 133992b4e516595f6b1d0fdca9cc9916103ce1d8 Mon Sep 17 00:00:00 2001 From: Damon Tivel Date: Tue, 13 Nov 2018 14:27:52 -0800 Subject: [PATCH 2/3] Apply feedback. --- src/Catalog/CatalogCommit.cs | 12 ++++- src/Catalog/CatalogCommitBatchTask.cs | 9 ++-- src/Catalog/CatalogCommitItem.cs | 11 ++++- src/Catalog/CatalogCommitUtilities.cs | 5 +- .../Registration/RegistrationCollector.cs | 1 - src/Catalog/Strings.Designer.cs | 9 ++++ src/Catalog/Strings.resx | 3 ++ .../CatalogCommitBatchTaskTests.cs | 24 ++++++++-- tests/CatalogTests/CatalogCommitItemTests.cs | 20 ++++++++ tests/CatalogTests/CatalogCommitTests.cs | 48 +++++++++++++++++++ 10 files changed, 128 insertions(+), 14 deletions(-) diff --git a/src/Catalog/CatalogCommit.cs b/src/Catalog/CatalogCommit.cs index e20f3073c..366eefd98 100644 --- a/src/Catalog/CatalogCommit.cs +++ b/src/Catalog/CatalogCommit.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Globalization; using Newtonsoft.Json.Linq; namespace NuGet.Services.Metadata.Catalog @@ -22,7 +23,16 @@ private CatalogCommit(DateTime commitTimeStamp, Uri uri) public int CompareTo(object obj) { - return CommitTimeStamp.CompareTo(((CatalogCommit)obj).CommitTimeStamp); + var other = obj as CatalogCommit; + + if (ReferenceEquals(other, null)) + { + throw new ArgumentException( + string.Format(CultureInfo.InvariantCulture, Strings.ArgumentMustBeInstanceOfType, nameof(CatalogCommit)), + nameof(obj)); + } + + return CommitTimeStamp.CompareTo(other.CommitTimeStamp); } public static CatalogCommit Create(JObject commit) diff --git a/src/Catalog/CatalogCommitBatchTask.cs b/src/Catalog/CatalogCommitBatchTask.cs index ba320c4e5..7c85c3a05 100644 --- a/src/Catalog/CatalogCommitBatchTask.cs +++ b/src/Catalog/CatalogCommitBatchTask.cs @@ -10,7 +10,7 @@ namespace NuGet.Services.Metadata.Catalog /// Represents an asynchrononous task associated with catalog changes for a specific commit item key /// and potentially spanning multiple commits. /// - public sealed class CatalogCommitBatchTask + public sealed class CatalogCommitBatchTask : IEquatable { /// /// Initializes a instance. @@ -41,14 +41,17 @@ public override int GetHashCode() public override bool Equals(object obj) { - var other = obj as CatalogCommitBatchTask; + return Equals(obj as CatalogCommitBatchTask); + } + public bool Equals(CatalogCommitBatchTask other) + { if (ReferenceEquals(other, null)) { return false; } - return GetHashCode() == other.GetHashCode(); + return string.Equals(Key, other.Key); } } } \ No newline at end of file diff --git a/src/Catalog/CatalogCommitItem.cs b/src/Catalog/CatalogCommitItem.cs index 059be9e90..a240f8e5f 100644 --- a/src/Catalog/CatalogCommitItem.cs +++ b/src/Catalog/CatalogCommitItem.cs @@ -43,7 +43,16 @@ private CatalogCommitItem( public int CompareTo(object obj) { - return CommitTimeStamp.CompareTo(((CatalogCommitItem)obj).CommitTimeStamp); + var other = obj as CatalogCommitItem; + + if (ReferenceEquals(other, null)) + { + throw new ArgumentException( + string.Format(CultureInfo.InvariantCulture, Strings.ArgumentMustBeInstanceOfType, nameof(CatalogCommitItem)), + nameof(obj)); + } + + return CommitTimeStamp.CompareTo(other.CommitTimeStamp); } public static CatalogCommitItem Create(JObject context, JObject commitItem) diff --git a/src/Catalog/CatalogCommitUtilities.cs b/src/Catalog/CatalogCommitUtilities.cs index 008a7065d..9b25cc461 100644 --- a/src/Catalog/CatalogCommitUtilities.cs +++ b/src/Catalog/CatalogCommitUtilities.cs @@ -44,8 +44,6 @@ public static IEnumerable CreateCommitItemBatches( foreach (var catalogItemsGroup in catalogItemsGroups) { - var key = catalogItemsGroup.Key; - // Before filtering out all but the latest commit for each package identity, determine the earliest // commit timestamp for all items in this batch. This timestamp is important for processing commits // in chronological order. @@ -60,7 +58,7 @@ public static IEnumerable CreateCommitItemBatches( yield return new CatalogCommitItemBatch( minCommitTimeStamp, - key, + catalogItemsGroup.Key, catalogItemsWithOnlyLatestCommitForEachPackageIdentity); } } @@ -281,7 +279,6 @@ internal static async Task FetchAsync( ReadWriteCursor front, ReadCursor back, FetchCatalogCommitsAsync fetchCatalogCommitsAsync, - GetCatalogCommitItemKey getCatalogCommitItemKey, CreateCommitItemBatchesAsync createCommitItemBatchesAsync, ProcessCommitItemBatchAsync processCommitItemBatchAsync, int maxConcurrentBatches, diff --git a/src/Catalog/Registration/RegistrationCollector.cs b/src/Catalog/Registration/RegistrationCollector.cs index 44f1a2206..d2f465347 100644 --- a/src/Catalog/Registration/RegistrationCollector.cs +++ b/src/Catalog/Registration/RegistrationCollector.cs @@ -111,7 +111,6 @@ protected override Task FetchAsync( front, back, FetchCatalogCommitsAsync, - CatalogCommitUtilities.GetPackageIdKey, CreateBatchesAsync, ProcessBatchAsync, _maxConcurrentBatches, diff --git a/src/Catalog/Strings.Designer.cs b/src/Catalog/Strings.Designer.cs index 07fe7f669..7a32863a4 100644 --- a/src/Catalog/Strings.Designer.cs +++ b/src/Catalog/Strings.Designer.cs @@ -60,6 +60,15 @@ internal Strings() { } } + /// + /// Looks up a localized string similar to The argument must be an instance of type {0}.. + /// + internal static string ArgumentMustBeInstanceOfType { + get { + return ResourceManager.GetString("ArgumentMustBeInstanceOfType", resourceCulture); + } + } + /// /// Looks up a localized string similar to The argument must not be null, empty, or whitespace.. /// diff --git a/src/Catalog/Strings.resx b/src/Catalog/Strings.resx index ede92d8e0..c3b9e6f78 100644 --- a/src/Catalog/Strings.resx +++ b/src/Catalog/Strings.resx @@ -117,6 +117,9 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + The argument must be an instance of type {0}. + The argument must not be null, empty, or whitespace. diff --git a/tests/CatalogTests/CatalogCommitBatchTaskTests.cs b/tests/CatalogTests/CatalogCommitBatchTaskTests.cs index c111b79b1..d678f6550 100644 --- a/tests/CatalogTests/CatalogCommitBatchTaskTests.cs +++ b/tests/CatalogTests/CatalogCommitBatchTaskTests.cs @@ -48,6 +48,7 @@ public void Equals_WhenObjectIsNull_ReturnsFalse() var commitBatchTask = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); Assert.False(commitBatchTask.Equals(obj: null)); + Assert.False(commitBatchTask.Equals(other: null)); } [Fact] @@ -55,7 +56,7 @@ public void Equals_WhenObjectIsNotCatalogCommitBatchTask_ReturnsFalse() { var commitBatchTask = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); - Assert.False(commitBatchTask.Equals(new object())); + Assert.False(commitBatchTask.Equals(obj: new object())); } [Fact] @@ -63,7 +64,8 @@ public void Equals_WhenObjectIsSameInstance_ReturnsTrue() { var commitBatchTask = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); - Assert.True(commitBatchTask.Equals(commitBatchTask)); + Assert.True(commitBatchTask.Equals(obj: commitBatchTask)); + Assert.True(commitBatchTask.Equals(other: commitBatchTask)); } [Fact] @@ -72,8 +74,22 @@ public void Equals_WhenObjectHasSamePackageId_ReturnsTrue() var commitBatchTask0 = new CatalogCommitBatchTask(_minCommitTimeStamp, _key); var commitBatchTask1 = new CatalogCommitBatchTask(_minCommitTimeStamp.AddMinutes(1), _key); - Assert.True(commitBatchTask0.Equals(commitBatchTask1)); - Assert.True(commitBatchTask1.Equals(commitBatchTask0)); + Assert.True(commitBatchTask0.Equals(obj: commitBatchTask1)); + Assert.True(commitBatchTask1.Equals(obj: commitBatchTask0)); + Assert.True(commitBatchTask0.Equals(other: commitBatchTask1)); + Assert.True(commitBatchTask1.Equals(other: commitBatchTask0)); + } + + [Fact] + public void Equals_WhenObjectHasDifferentPackageId_ReturnsFalse() + { + var commitBatchTask0 = new CatalogCommitBatchTask(_minCommitTimeStamp, key: "a"); + var commitBatchTask1 = new CatalogCommitBatchTask(_minCommitTimeStamp, key: "b"); + + Assert.False(commitBatchTask0.Equals(obj: commitBatchTask1)); + Assert.False(commitBatchTask1.Equals(obj: commitBatchTask0)); + Assert.False(commitBatchTask0.Equals(other: commitBatchTask1)); + Assert.False(commitBatchTask1.Equals(other: commitBatchTask0)); } } } \ No newline at end of file diff --git a/tests/CatalogTests/CatalogCommitItemTests.cs b/tests/CatalogTests/CatalogCommitItemTests.cs index f91202278..a2e41c96f 100644 --- a/tests/CatalogTests/CatalogCommitItemTests.cs +++ b/tests/CatalogTests/CatalogCommitItemTests.cs @@ -68,6 +68,26 @@ public void Create_WhenArgumentsAreValid_ReturnsInstance() Assert.Equal(Schema.DataTypes.PackageDetails.AbsoluteUri, commitItem.TypeUris.Single().AbsoluteUri); } + [Fact] + public void CompareTo_WhenObjIsNull_Throws() + { + var commitItem = CatalogCommitItem.Create(_context, _commitItem); + + var exception = Assert.Throws(() => commitItem.CompareTo(obj: null)); + + Assert.Equal("obj", exception.ParamName); + } + + [Fact] + public void CompareTo_WhenObjIsNotCatalogCommit_Throws() + { + var commitItem = CatalogCommitItem.Create(_context, _commitItem); + + var exception = Assert.Throws(() => commitItem.CompareTo(new object())); + + Assert.Equal("obj", exception.ParamName); + } + [Fact] public void CompareTo_WhenArgumentIsValid_ReturnsValue() { diff --git a/tests/CatalogTests/CatalogCommitTests.cs b/tests/CatalogTests/CatalogCommitTests.cs index 099472e2e..8b8439507 100644 --- a/tests/CatalogTests/CatalogCommitTests.cs +++ b/tests/CatalogTests/CatalogCommitTests.cs @@ -33,5 +33,53 @@ public void Create_WhenArgumentIsValid_ReturnsInstance() Assert.Equal(idKeyword, commit.Uri.AbsoluteUri); Assert.Equal(commitTimeStamp, commit.CommitTimeStamp.ToUniversalTime().ToString("O")); } + + [Fact] + public void CompareTo_WhenObjIsNull_Throws() + { + var commit = CreateCatalogCommit(); + + var exception = Assert.Throws(() => commit.CompareTo(obj: null)); + + Assert.Equal("obj", exception.ParamName); + } + + [Fact] + public void CompareTo_WhenObjIsNotCatalogCommit_Throws() + { + var commit = CreateCatalogCommit(); + + var exception = Assert.Throws(() => commit.CompareTo(new object())); + + Assert.Equal("obj", exception.ParamName); + } + + [Fact] + public void CompareTo_WhenObjIsCatalogCommit_ReturnsValue() + { + var jObject0 = new JObject( + new JProperty(CatalogConstants.IdKeyword, "https://nuget.test/a"), + new JProperty(CatalogConstants.CommitTimeStamp, DateTime.UtcNow.ToString("O"))); + + var jObject1 = new JObject( + new JProperty(CatalogConstants.IdKeyword, "https://nuget.test/b"), + new JProperty(CatalogConstants.CommitTimeStamp, DateTime.UtcNow.AddHours(1).ToString("O"))); + + var commit0 = CatalogCommit.Create(jObject0); + var commit1 = CatalogCommit.Create(jObject1); + + Assert.Equal(-1, commit0.CompareTo(commit1)); + Assert.Equal(0, commit0.CompareTo(commit0)); + Assert.Equal(1, commit1.CompareTo(commit0)); + } + + private static CatalogCommit CreateCatalogCommit() + { + var jObject = new JObject( + new JProperty(CatalogConstants.IdKeyword, "https://nuget.test/a"), + new JProperty(CatalogConstants.CommitTimeStamp, DateTime.UtcNow.ToString("O"))); + + return CatalogCommit.Create(jObject); + } } } \ No newline at end of file From 141ba285117dd9b043592d26eebcb6a97faad60c Mon Sep 17 00:00:00 2001 From: Damon Tivel Date: Tue, 13 Nov 2018 15:58:57 -0800 Subject: [PATCH 3/3] Apply feedback. --- src/Catalog/CatalogCommitUtilities.cs | 33 +++++++- .../Registration/RegistrationCollector.cs | 6 +- src/Catalog/Strings.Designer.cs | 9 +++ src/Catalog/Strings.resx | 3 + src/Ng/Jobs/Catalog2RegistrationJob.cs | 1 + src/V3PerPackage/PerBatchProcessor.cs | 1 + .../CatalogCommitUtilitiesTests.cs | 77 +++++++++++++------ .../RegistrationCollectorTests.cs | 7 +- tests/NgTests/Infrastructure/TestUtility.cs | 14 +++- 9 files changed, 121 insertions(+), 30 deletions(-) diff --git a/src/Catalog/CatalogCommitUtilities.cs b/src/Catalog/CatalogCommitUtilities.cs index 9b25cc461..7e3c45a33 100644 --- a/src/Catalog/CatalogCommitUtilities.cs +++ b/src/Catalog/CatalogCommitUtilities.cs @@ -4,16 +4,20 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; -using NuGet.Common; +using ExceptionUtilities = NuGet.Common.ExceptionUtilities; namespace NuGet.Services.Metadata.Catalog { public static class CatalogCommitUtilities { + private static readonly EventId _eventId = new EventId(id: 0); + /// /// Creates an enumerable of instances. /// @@ -39,6 +43,8 @@ public static IEnumerable CreateCommitItemBatches( throw new ArgumentNullException(nameof(getCatalogCommitItemKey)); } + AssertNotMoreThanOneCommitIdPerCommitTimeStamp(catalogItems); + var catalogItemsGroups = catalogItems .GroupBy(catalogItem => getCatalogCommitItemKey(catalogItem)); @@ -63,6 +69,25 @@ public static IEnumerable CreateCommitItemBatches( } } + private static void AssertNotMoreThanOneCommitIdPerCommitTimeStamp(IEnumerable catalogItems) + { + var commitsWithDifferentCommitIds = catalogItems.GroupBy(catalogItem => catalogItem.CommitTimeStamp) + .Where(group => group.Select(item => item.CommitId).Distinct().Count() > 1); + + if (commitsWithDifferentCommitIds.Any()) + { + var commits = commitsWithDifferentCommitIds.SelectMany(group => group) + .Select(commit => $"{{ CommitId = {commit.CommitId}, CommitTimeStamp = {commit.CommitTimeStamp.ToString("O")} }}"); + + throw new ArgumentException( + string.Format( + CultureInfo.InvariantCulture, + Strings.MultipleCommitIdsForSameCommitTimeStamp, + string.Join(", ", commits)), + nameof(catalogItems)); + } + } + /// /// Generate a map of commit timestamps to commit batch tasks. /// @@ -283,6 +308,7 @@ internal static async Task FetchAsync( ProcessCommitItemBatchAsync processCommitItemBatchAsync, int maxConcurrentBatches, string typeName, + ILogger logger, CancellationToken cancellationToken) { IEnumerable rootItems = await fetchCatalogCommitsAsync(client, front, cancellationToken); @@ -407,6 +433,11 @@ internal static async Task FetchAsync( if (hasAnyBatchFailed) { + foreach (var exception in exceptions) + { + logger.LogError(_eventId, exception, Strings.BatchProcessingFailure); + } + var innerException = exceptions.Count == 1 ? exceptions.Single() : new AggregateException(exceptions); throw new BatchProcessingException(innerException); diff --git a/src/Catalog/Registration/RegistrationCollector.cs b/src/Catalog/Registration/RegistrationCollector.cs index d2f465347..ed265bdf8 100644 --- a/src/Catalog/Registration/RegistrationCollector.cs +++ b/src/Catalog/Registration/RegistrationCollector.cs @@ -3,10 +3,10 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Net.Http; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; using NuGet.Services.Metadata.Catalog.Helpers; using NuGet.Services.Metadata.Catalog.Persistence; @@ -26,6 +26,7 @@ public class RegistrationCollector : SortingGraphCollector private readonly StorageFactory _semVer2StorageFactory; private readonly ShouldIncludeRegistrationPackage _shouldIncludeSemVer2; private readonly int _maxConcurrentBatches; + private readonly ILogger _logger; public RegistrationCollector( Uri index, @@ -33,11 +34,13 @@ public RegistrationCollector( StorageFactory semVer2StorageFactory, Uri contentBaseAddress, ITelemetryService telemetryService, + ILogger logger, Func handlerFunc = null, int maxConcurrentBatches = DefaultMaxConcurrentBatches) : base(index, new Uri[] { Schema.DataTypes.PackageDetails, Schema.DataTypes.PackageDelete }, telemetryService, handlerFunc) { _legacyStorageFactory = legacyStorageFactory ?? throw new ArgumentNullException(nameof(legacyStorageFactory)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _semVer2StorageFactory = semVer2StorageFactory; _shouldIncludeSemVer2 = GetShouldIncludeRegistrationPackage(_semVer2StorageFactory); ContentBaseAddress = contentBaseAddress; @@ -115,6 +118,7 @@ protected override Task FetchAsync( ProcessBatchAsync, _maxConcurrentBatches, nameof(RegistrationCollector), + _logger, cancellationToken); } diff --git a/src/Catalog/Strings.Designer.cs b/src/Catalog/Strings.Designer.cs index 7a32863a4..aa4f2a080 100644 --- a/src/Catalog/Strings.Designer.cs +++ b/src/Catalog/Strings.Designer.cs @@ -105,6 +105,15 @@ internal static string BatchProcessingFailure { } } + /// + /// Looks up a localized string similar to Multiple commits exist with the same commit timestamp but different commit ID's: {0}.. + /// + internal static string MultipleCommitIdsForSameCommitTimeStamp { + get { + return ResourceManager.GetString("MultipleCommitIdsForSameCommitTimeStamp", resourceCulture); + } + } + /// /// Looks up a localized string similar to The value of property '{0}' must be non-null and non-empty.. /// diff --git a/src/Catalog/Strings.resx b/src/Catalog/Strings.resx index c3b9e6f78..56bc19ae3 100644 --- a/src/Catalog/Strings.resx +++ b/src/Catalog/Strings.resx @@ -132,6 +132,9 @@ A failure occurred while processing a catalog batch. + + Multiple commits exist with the same commit timestamp but different commit ID's: {0}. + The value of property '{0}' must be non-null and non-empty. diff --git a/src/Ng/Jobs/Catalog2RegistrationJob.cs b/src/Ng/Jobs/Catalog2RegistrationJob.cs index 65e9e47ae..6ac270905 100644 --- a/src/Ng/Jobs/Catalog2RegistrationJob.cs +++ b/src/Ng/Jobs/Catalog2RegistrationJob.cs @@ -104,6 +104,7 @@ protected override void Init(IDictionary arguments, Cancellation storageFactories.SemVer2StorageFactory, contentBaseAddress == null ? null : new Uri(contentBaseAddress), TelemetryService, + Logger, CommandHelpers.GetHttpMessageHandlerFactory(TelemetryService, verbose)); var cursorStorage = storageFactories.LegacyStorageFactory.Create(); diff --git a/src/V3PerPackage/PerBatchProcessor.cs b/src/V3PerPackage/PerBatchProcessor.cs index beac15a4c..215571c5c 100644 --- a/src/V3PerPackage/PerBatchProcessor.cs +++ b/src/V3PerPackage/PerBatchProcessor.cs @@ -255,6 +255,7 @@ private async Task ExecuteCatalog2RegistrationAsync(PerBatchContext context, Uri registrationStorageFactories.SemVer2StorageFactory, context.Global.ContentBaseAddress, serviceProvider.GetRequiredService(), + _logger, () => serviceProvider.GetRequiredService()); await collector.RunAsync(CancellationToken.None); diff --git a/tests/CatalogTests/CatalogCommitUtilitiesTests.cs b/tests/CatalogTests/CatalogCommitUtilitiesTests.cs index 959128674..40761a13e 100644 --- a/tests/CatalogTests/CatalogCommitUtilitiesTests.cs +++ b/tests/CatalogTests/CatalogCommitUtilitiesTests.cs @@ -57,6 +57,32 @@ public void CreateCommitItemBatches_WhenGetCatalogCommitItemKeyIsNull_Throws() Assert.Equal("getCatalogCommitItemKey", exception.ParamName); } + [Fact] + public void CreateCommitItemBatches_WhenMultipleCommitsShareCommitTimeStampButNotCommitId_Throws() + { + var commitTimeStamp = DateTime.UtcNow; + var context = TestUtility.CreateCatalogContextJObject(); + var commitItem0 = CatalogCommitItem.Create( + context, + TestUtility.CreateCatalogCommitItemJObject(commitTimeStamp, _packageIdentitya)); + var commitItem1 = CatalogCommitItem.Create( + context, + TestUtility.CreateCatalogCommitItemJObject(commitTimeStamp, _packageIdentityb)); + var commitItems = new[] { commitItem0, commitItem1 }; + + var exception = Assert.Throws(() => + CatalogCommitUtilities.CreateCommitItemBatches( + commitItems, + CatalogCommitUtilities.GetPackageIdKey) + .ToArray()); // force evaluation + + Assert.Equal("catalogItems", exception.ParamName); + Assert.StartsWith("Multiple commits exist with the same commit timestamp but different commit ID's: " + + $"{{ CommitId = {commitItem0.CommitId}, CommitTimeStamp = {commitItem0.CommitTimeStamp.ToString("O")} }}, " + + $"{{ CommitId = {commitItem1.CommitId}, CommitTimeStamp = {commitItem1.CommitTimeStamp.ToString("O")} }}.", + exception.Message); + } + [Fact] public void CreateCommitItemBatches_WhenPackageIdsVaryInCasing_GroupsUsingProvidedGetKeyFunction() { @@ -66,33 +92,40 @@ public void CreateCommitItemBatches_WhenPackageIdsVaryInCasing_GroupsUsingProvid var packageIdentityA2 = new PackageIdentity(id: "A", version: new NuGetVersion("3.0.0")); var packageIdentityB0 = new PackageIdentity(id: "b", version: new NuGetVersion("1.0.0")); var packageIdentityB1 = new PackageIdentity(id: "B", version: new NuGetVersion("2.0.0")); - var commit0 = TestUtility.CreateCatalogCommitItem(now, packageIdentityA0); - var commit1 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), packageIdentityA1); - var commit2 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(2), packageIdentityA2); - var commit3 = TestUtility.CreateCatalogCommitItem(now, packageIdentityB0); - var commit4 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), packageIdentityB1); - var commits = new[] { commit4, commit2, commit0, commit3, commit1 }; // not in alphanumeric or chronological order - - var batches = CatalogCommitUtilities.CreateCommitItemBatches(commits, CatalogCommitUtilities.GetPackageIdKey); + var commitId0 = Guid.NewGuid().ToString(); + var commitId1 = Guid.NewGuid().ToString(); + var commitId2 = Guid.NewGuid().ToString(); + var commitItem0 = TestUtility.CreateCatalogCommitItem(now, packageIdentityA0, commitId0); + var commitItem1 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), packageIdentityA1, commitId1); + var commitItem2 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(2), packageIdentityA2, commitId2); + var commitItem3 = TestUtility.CreateCatalogCommitItem(now, packageIdentityB0, commitId0); + var commitItem4 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), packageIdentityB1, commitId1); + + // not in alphanumeric or chronological order + var commitItems = new[] { commitItem4, commitItem2, commitItem0, commitItem3, commitItem1 }; + + var batches = CatalogCommitUtilities.CreateCommitItemBatches( + commitItems, + CatalogCommitUtilities.GetPackageIdKey); Assert.Collection( batches, batch => { - Assert.Equal(commit3.CommitTimeStamp, batch.CommitTimeStamp); + Assert.Equal(commitItem3.CommitTimeStamp, batch.CommitTimeStamp); Assert.Collection( batch.Items, - commit => Assert.True(ReferenceEquals(commit, commit3)), - commit => Assert.True(ReferenceEquals(commit, commit4))); + commit => Assert.True(ReferenceEquals(commit, commitItem3)), + commit => Assert.True(ReferenceEquals(commit, commitItem4))); }, batch => { - Assert.Equal(commit0.CommitTimeStamp, batch.CommitTimeStamp); + Assert.Equal(commitItem0.CommitTimeStamp, batch.CommitTimeStamp); Assert.Collection( batch.Items, - commit => Assert.True(ReferenceEquals(commit, commit0)), - commit => Assert.True(ReferenceEquals(commit, commit1)), - commit => Assert.True(ReferenceEquals(commit, commit2))); + commit => Assert.True(ReferenceEquals(commit, commitItem0)), + commit => Assert.True(ReferenceEquals(commit, commitItem1)), + commit => Assert.True(ReferenceEquals(commit, commitItem2))); }); } @@ -100,21 +133,21 @@ public void CreateCommitItemBatches_WhenPackageIdsVaryInCasing_GroupsUsingProvid public void CreateCommitItemBatches_WhenCommitItemsContainMultipleCommitsForSamePackageIdentity_ReturnsOnlyLatestCommitForEachPackageIdentity() { var now = DateTime.UtcNow; - var commit0 = TestUtility.CreateCatalogCommitItem(now, _packageIdentitya); - var commit1 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), _packageIdentitya); - var commit2 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(2), _packageIdentitya); - var commits = new[] { commit0, commit1, commit2 }; + var commitItem0 = TestUtility.CreateCatalogCommitItem(now, _packageIdentitya); + var commitItem1 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(1), _packageIdentitya); + var commitItem2 = TestUtility.CreateCatalogCommitItem(now.AddMinutes(2), _packageIdentitya); + var commitItems = new[] { commitItem0, commitItem1, commitItem2 }; - var batches = CatalogCommitUtilities.CreateCommitItemBatches(commits, CatalogCommitUtilities.GetPackageIdKey); + var batches = CatalogCommitUtilities.CreateCommitItemBatches(commitItems, CatalogCommitUtilities.GetPackageIdKey); Assert.Collection( batches, batch => { - Assert.Equal(commit0.CommitTimeStamp, batch.CommitTimeStamp); + Assert.Equal(commitItem0.CommitTimeStamp, batch.CommitTimeStamp); Assert.Collection( batch.Items, - commit => Assert.True(ReferenceEquals(commit, commit2))); + commit => Assert.True(ReferenceEquals(commit, commitItem2))); }); } diff --git a/tests/CatalogTests/Registration/RegistrationCollectorTests.cs b/tests/CatalogTests/Registration/RegistrationCollectorTests.cs index faeef394b..c966d4f8a 100644 --- a/tests/CatalogTests/Registration/RegistrationCollectorTests.cs +++ b/tests/CatalogTests/Registration/RegistrationCollectorTests.cs @@ -9,6 +9,7 @@ using System.Threading; using System.Threading.Tasks; using CatalogTests.Helpers; +using Microsoft.Extensions.Logging; using Moq; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -77,7 +78,8 @@ private void SharedInit(bool useLegacy, bool useSemVer2, Uri baseUri = null, Uri _legacyStorageFactory, _semVer2StorageFactory, contentBaseUri ?? new Uri("http://tempuri.org/packages"), - new Mock().Object, + Mock.Of(), + Mock.Of(), handlerFunc: () => _mockServer); RegistrationMakerCatalogItem.PackagePathProvider = new PackagesFolderPackagePathProvider(); @@ -96,7 +98,8 @@ public void Constructor_WhenMaxConcurrentBatchesIsNotPositive_Throws(int maxConc storageFactory, storageFactory, _baseUri, - new Mock().Object, + Mock.Of(), + Mock.Of(), maxConcurrentBatches: maxConcurrentBatches)); Assert.Equal("maxConcurrentBatches", exception.ParamName); diff --git a/tests/NgTests/Infrastructure/TestUtility.cs b/tests/NgTests/Infrastructure/TestUtility.cs index 710b0dcec..b90767456 100644 --- a/tests/NgTests/Infrastructure/TestUtility.cs +++ b/tests/NgTests/Infrastructure/TestUtility.cs @@ -54,21 +54,27 @@ public static JObject CreateCatalogContextJObject() new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.XsdDateTime)))); } - public static JObject CreateCatalogCommitItemJObject(DateTime commitTimeStamp, PackageIdentity packageIdentity) + public static JObject CreateCatalogCommitItemJObject( + DateTime commitTimeStamp, + PackageIdentity packageIdentity, + string commitId = null) { return new JObject( new JProperty(CatalogConstants.IdKeyword, $"https://nuget.test/{packageIdentity.Id}"), new JProperty(CatalogConstants.TypeKeyword, CatalogConstants.NuGetPackageDetails), new JProperty(CatalogConstants.CommitTimeStamp, commitTimeStamp.ToString("O")), - new JProperty(CatalogConstants.CommitId, Guid.NewGuid().ToString()), + new JProperty(CatalogConstants.CommitId, commitId ?? Guid.NewGuid().ToString()), new JProperty(CatalogConstants.NuGetId, packageIdentity.Id), new JProperty(CatalogConstants.NuGetVersion, packageIdentity.Version.ToNormalizedString())); } - public static CatalogCommitItem CreateCatalogCommitItem(DateTime commitTimeStamp, PackageIdentity packageIdentity) + public static CatalogCommitItem CreateCatalogCommitItem( + DateTime commitTimeStamp, + PackageIdentity packageIdentity, + string commitId = null) { var context = CreateCatalogContextJObject(); - var commitItem = CreateCatalogCommitItemJObject(commitTimeStamp, packageIdentity); + var commitItem = CreateCatalogCommitItemJObject(commitTimeStamp, packageIdentity, commitId); return CatalogCommitItem.Create(context, commitItem); }