From 45016429da6d200172815fe0ea941281dfed6c18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= <737941+loic-sharma@users.noreply.github.com> Date: Sun, 12 Dec 2021 14:29:18 -0800 Subject: [PATCH] Refactor package mirroring (#711) After this change there's two services to access packages' state: * `IPackageDatabase` - This is a "low level" service for packages' state in BaGet's database * `IPackageService` - This is a "high level" service for packages' state, including both the database and the upstream feed if any This allows us to centralize the mirroring logic into the `IPackageService` (before mirroring logic was sprinkled throughout the codebase). Replaces https://github.com/loic-sharma/BaGet/pull/699 --- .gitignore | 4 +- src/BaGet.Azure/Table/TablePackageDatabase.cs | 6 +- .../Content/DefaultPackageContentService.cs | 36 ++--- .../DependencyInjectionExtensions.cs | 31 +++-- src/BaGet.Core/IPackageDatabase.cs | 4 +- .../Metadata/DefaultPackageMetadataService.cs | 14 +- .../Mirror/DisabledMirrorService.cs | 39 ------ src/BaGet.Core/Mirror/IMirrorService.cs | 46 ------- src/BaGet.Core/Mirror/IPackageService.cs | 70 ++++++++++ .../{MirrorService.cs => PackageService.cs} | 71 ++++++---- src/BaGet.Core/PackageDatabase.cs | 4 +- .../Clients/DisabledUpstreamClient.cs | 35 +++++ src/BaGet.Web/Pages/Package.cshtml.cs | 8 +- ...ServiceTests.cs => PackageServiceTests.cs} | 124 +++++++++++++++--- .../Pages/PackageModelFacts.cs | 28 ++-- 15 files changed, 312 insertions(+), 208 deletions(-) delete mode 100644 src/BaGet.Core/Mirror/DisabledMirrorService.cs delete mode 100644 src/BaGet.Core/Mirror/IMirrorService.cs create mode 100644 src/BaGet.Core/Mirror/IPackageService.cs rename src/BaGet.Core/Mirror/{MirrorService.cs => PackageService.cs} (59%) create mode 100644 src/BaGet.Core/Upstream/Clients/DisabledUpstreamClient.cs rename tests/BaGet.Core.Tests/Services/{MirrorServiceTests.cs => PackageServiceTests.cs} (73%) diff --git a/.gitignore b/.gitignore index 415b4f2e..2cec8475 100644 --- a/.gitignore +++ b/.gitignore @@ -268,5 +268,7 @@ __pycache__/ # Cake - Uncomment if you are using it # tools/ -# Ignore database file +# Ignore database files **/baget.db +**/baget.db-shm +**/baget.db-wal diff --git a/src/BaGet.Azure/Table/TablePackageDatabase.cs b/src/BaGet.Azure/Table/TablePackageDatabase.cs index f6bc1570..83174df4 100644 --- a/src/BaGet.Azure/Table/TablePackageDatabase.cs +++ b/src/BaGet.Azure/Table/TablePackageDatabase.cs @@ -48,7 +48,7 @@ public async Task AddAsync(Package package, CancellationToken return PackageAddResult.Success; } - public async Task AddDownloadAsync( + public async Task AddDownloadAsync( string id, NuGetVersion version, CancellationToken cancellationToken) @@ -68,13 +68,13 @@ public async Task AddDownloadAsync( if (entity == null) { - return false; + return; } entity.Downloads += 1; await _table.ExecuteAsync(TableOperation.Merge(entity), cancellationToken); - return true; + return; } catch (StorageException e) when (attempt < MaxPreconditionFailures && e.IsPreconditionFailedException()) diff --git a/src/BaGet.Core/Content/DefaultPackageContentService.cs b/src/BaGet.Core/Content/DefaultPackageContentService.cs index 6b7a1a8d..8d0a76d1 100644 --- a/src/BaGet.Core/Content/DefaultPackageContentService.cs +++ b/src/BaGet.Core/Content/DefaultPackageContentService.cs @@ -9,22 +9,17 @@ namespace BaGet.Core { /// - /// Implements the NuGet Package Content resource. Supports read-through caching. - /// Tracks state in a database () and stores packages - /// using . + /// Implements the NuGet Package Content resource in NuGet's V3 protocol. /// public class DefaultPackageContentService : IPackageContentService { - private readonly IMirrorService _mirror; - private readonly IPackageDatabase _packages; + private readonly IPackageService _packages; private readonly IPackageStorageService _storage; public DefaultPackageContentService( - IMirrorService mirror, - IPackageDatabase packages, + IPackageService packages, IPackageStorageService storage) { - _mirror = mirror ?? throw new ArgumentNullException(nameof(mirror)); _packages = packages ?? throw new ArgumentNullException(nameof(packages)); _storage = storage ?? throw new ArgumentNullException(nameof(storage)); } @@ -33,7 +28,7 @@ public async Task GetPackageVersionsOrNullAsync( string id, CancellationToken cancellationToken = default) { - var versions = await _mirror.FindPackageVersionsAsync(id, cancellationToken); + var versions = await _packages.FindPackageVersionsAsync(id, cancellationToken); if (!versions.Any()) { return null; @@ -53,22 +48,17 @@ public async Task GetPackageContentStreamOrNullAsync( NuGetVersion version, CancellationToken cancellationToken = default) { - // Allow read-through caching if it is configured. - await _mirror.MirrorAsync(id, version, cancellationToken); - - if (!await _packages.AddDownloadAsync(id, version, cancellationToken)) + if (!await _packages.ExistsAsync(id, version, cancellationToken)) { return null; } + await _packages.AddDownloadAsync(id, version, cancellationToken); return await _storage.GetPackageStreamAsync(id, version, cancellationToken); } public async Task GetPackageManifestStreamOrNullAsync(string id, NuGetVersion version, CancellationToken cancellationToken = default) { - // Allow read-through caching if it is configured. - await _mirror.MirrorAsync(id, version, cancellationToken); - if (!await _packages.ExistsAsync(id, version, cancellationToken)) { return null; @@ -79,11 +69,8 @@ public async Task GetPackageManifestStreamOrNullAsync(string id, NuGetVe public async Task GetPackageReadmeStreamOrNullAsync(string id, NuGetVersion version, CancellationToken cancellationToken = default) { - // Allow read-through caching if it is configured. - await _mirror.MirrorAsync(id, version, cancellationToken); - - var package = await _packages.FindOrNullAsync(id, version, includeUnlisted: true, cancellationToken); - if (!package.HasReadme) + var package = await _packages.FindPackageOrNullAsync(id, version, cancellationToken); + if (package == null || !package.HasReadme) { return null; } @@ -93,11 +80,8 @@ public async Task GetPackageReadmeStreamOrNullAsync(string id, NuGetVers public async Task GetPackageIconStreamOrNullAsync(string id, NuGetVersion version, CancellationToken cancellationToken = default) { - // Allow read-through caching if it is configured. - await _mirror.MirrorAsync(id, version, cancellationToken); - - var package = await _packages.FindOrNullAsync(id, version, includeUnlisted: true, cancellationToken); - if (!package.HasEmbeddedIcon) + var package = await _packages.FindPackageOrNullAsync(id, version, cancellationToken); + if (package == null || !package.HasEmbeddedIcon) { return null; } diff --git a/src/BaGet.Core/Extensions/DependencyInjectionExtensions.cs b/src/BaGet.Core/Extensions/DependencyInjectionExtensions.cs index 6a60208e..a56c41ea 100644 --- a/src/BaGet.Core/Extensions/DependencyInjectionExtensions.cs +++ b/src/BaGet.Core/Extensions/DependencyInjectionExtensions.cs @@ -92,6 +92,7 @@ private static void AddBaGetServices(this IServiceCollection services) services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); + services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); @@ -99,15 +100,14 @@ private static void AddBaGetServices(this IServiceCollection services) services.TryAddTransient(); services.TryAddTransient(); - services.TryAddTransient(); + services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); - services.TryAddTransient(); + services.TryAddTransient(); services.TryAddSingleton(); services.TryAddTransient(); - services.TryAddTransient(IMirrorServiceFactory); - services.TryAddTransient(IMirrorClientFactory); + services.TryAddTransient(UpstreamClientFactory); } private static void AddDefaultProviders(this IServiceCollection services) @@ -195,20 +195,25 @@ private static NuGetClientFactory NuGetClientFactoryFactory(IServiceProvider pro options.Value.PackageSource.ToString()); } - private static IMirrorService IMirrorServiceFactory(IServiceProvider provider) + private static IUpstreamClient UpstreamClientFactory(IServiceProvider provider) { var options = provider.GetRequiredService>(); - var service = options.Value.Enabled ? typeof(MirrorService) : typeof(DisabledMirrorService); - return (IMirrorService)provider.GetRequiredService(service); - } + // TODO: Convert to switch expression. + if (!options.Value.Enabled) + { + return provider.GetRequiredService(); + } - private static IUpstreamClient IMirrorClientFactory(IServiceProvider provider) - { - var options = provider.GetRequiredService>(); - var service = options.Value.Legacy ? typeof(V2UpstreamClient) : typeof(V3UpstreamClient); + else if (options.Value.Legacy) + { + return provider.GetRequiredService(); + } - return (IUpstreamClient)provider.GetRequiredService(service); + else + { + return provider.GetRequiredService(); + } } } } diff --git a/src/BaGet.Core/IPackageDatabase.cs b/src/BaGet.Core/IPackageDatabase.cs index a605ac12..38ec2966 100644 --- a/src/BaGet.Core/IPackageDatabase.cs +++ b/src/BaGet.Core/IPackageDatabase.cs @@ -83,8 +83,8 @@ Task FindOrNullAsync( /// The id of the package to update. /// The id of the package to update. /// A token to cancel the task. - /// False if the package does not exist. - Task AddDownloadAsync(string id, NuGetVersion version, CancellationToken cancellationToken); + /// Task that completes when the package's download has been incremented. + Task AddDownloadAsync(string id, NuGetVersion version, CancellationToken cancellationToken); /// /// Completely remove the package from the database. diff --git a/src/BaGet.Core/Metadata/DefaultPackageMetadataService.cs b/src/BaGet.Core/Metadata/DefaultPackageMetadataService.cs index 29653980..130e1e59 100644 --- a/src/BaGet.Core/Metadata/DefaultPackageMetadataService.cs +++ b/src/BaGet.Core/Metadata/DefaultPackageMetadataService.cs @@ -10,16 +10,13 @@ namespace BaGet.Core /// public class DefaultPackageMetadataService : IPackageMetadataService { - private readonly IMirrorService _mirror; - private readonly IPackageDatabase _packages; + private readonly IPackageService _packages; private readonly RegistrationBuilder _builder; public DefaultPackageMetadataService( - IMirrorService mirror, - IPackageDatabase packages, + IPackageService packages, RegistrationBuilder builder) { - _mirror = mirror ?? throw new ArgumentNullException(nameof(mirror)); _packages = packages ?? throw new ArgumentNullException(nameof(packages)); _builder = builder ?? throw new ArgumentNullException(nameof(builder)); } @@ -28,7 +25,7 @@ public async Task GetRegistrationIndexOrNullAsyn string packageId, CancellationToken cancellationToken = default) { - var packages = await _mirror.FindPackagesAsync(packageId, cancellationToken); + var packages = await _packages.FindPackagesAsync(packageId, cancellationToken); if (!packages.Any()) { return null; @@ -45,10 +42,7 @@ public async Task GetRegistrationLeafOrNullAsync( NuGetVersion version, CancellationToken cancellationToken = default) { - // Allow read-through caching to happen if it is configured. - await _mirror.MirrorAsync(id, version, cancellationToken); - - var package = await _packages.FindOrNullAsync(id, version, includeUnlisted: true, cancellationToken); + var package = await _packages.FindPackageOrNullAsync(id, version, cancellationToken); if (package == null) { return null; diff --git a/src/BaGet.Core/Mirror/DisabledMirrorService.cs b/src/BaGet.Core/Mirror/DisabledMirrorService.cs deleted file mode 100644 index b1f441c9..00000000 --- a/src/BaGet.Core/Mirror/DisabledMirrorService.cs +++ /dev/null @@ -1,39 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using NuGet.Versioning; - -namespace BaGet.Core -{ - /// - /// The mirror service used when mirroring has been disabled. - /// - public class DisabledMirrorService : IMirrorService - { - private readonly IPackageDatabase _packages; - - public DisabledMirrorService(IPackageDatabase packages) - { - _packages = packages ?? throw new ArgumentNullException(nameof(packages)); - } - - public async Task> FindPackageVersionsAsync(string id, CancellationToken cancellationToken) - { - var packages = await _packages.FindAsync(id, includeUnlisted: true, cancellationToken); - - return packages.Select(p => p.Version).ToList(); - } - - public async Task> FindPackagesAsync(string id, CancellationToken cancellationToken) - { - return await _packages.FindAsync(id, includeUnlisted: true, cancellationToken); - } - - public Task MirrorAsync(string id, NuGetVersion version, CancellationToken cancellationToken) - { - return Task.CompletedTask; - } - } -} diff --git a/src/BaGet.Core/Mirror/IMirrorService.cs b/src/BaGet.Core/Mirror/IMirrorService.cs deleted file mode 100644 index 704893c4..00000000 --- a/src/BaGet.Core/Mirror/IMirrorService.cs +++ /dev/null @@ -1,46 +0,0 @@ -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; -using NuGet.Versioning; - -namespace BaGet.Core -{ - /// - /// Indexes packages from an external source. - /// - public interface IMirrorService - { - /// - /// Attempt to find a package's versions using mirroring. This will merge - /// results from the configured upstream source with the locally indexed packages. - /// - /// The package's id to lookup - /// The token to cancel the lookup - /// - /// The package's versions, or an empty list if the package cannot be found. - /// This includes unlisted versions. - /// - Task> FindPackageVersionsAsync(string id, CancellationToken cancellationToken); - - /// - /// Attempt to find a package's metadata using mirroring. This will merge - /// results from the configured upstream source with the locally indexed packages. - /// - /// The package's id to lookup - /// The token to cancel the lookup - /// - /// The metadata for all versions of a package, including unlisted versions. - /// Returns an empty list if the package cannot be found. - /// - Task> FindPackagesAsync(string id, CancellationToken cancellationToken); - - /// - /// If the package is unknown, attempt to index it from an upstream source. - /// - /// The package's id - /// The package's version - /// The token to cancel the mirroring - /// A task that completes when the package has been mirrored. - Task MirrorAsync(string id, NuGetVersion version, CancellationToken cancellationToken); - } -} diff --git a/src/BaGet.Core/Mirror/IPackageService.cs b/src/BaGet.Core/Mirror/IPackageService.cs new file mode 100644 index 00000000..768813d2 --- /dev/null +++ b/src/BaGet.Core/Mirror/IPackageService.cs @@ -0,0 +1,70 @@ +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using NuGet.Versioning; + +namespace BaGet.Core +{ + /// + /// The service that combines the state of indexed packages and + /// upstream packages. + /// For upstream packages, see . + /// For indexed packages, see . + /// + public interface IPackageService + { + /// + /// Attempt to find a package's versions using mirroring. This will merge + /// results from the configured upstream source with the locally indexed packages. + /// + /// The package's id to lookup + /// The token to cancel the lookup + /// + /// The package's versions, or an empty list if the package cannot be found. + /// This includes unlisted versions. + /// + Task> FindPackageVersionsAsync(string id, CancellationToken cancellationToken); + + /// + /// Attempt to find a package's metadata using mirroring. This will merge + /// results from the configured upstream source with the locally indexed packages. + /// + /// The package's id to lookup + /// The token to cancel the lookup + /// + /// The metadata for all versions of a package, including unlisted versions. + /// Returns an empty list if the package cannot be found. + /// + Task> FindPackagesAsync(string id, CancellationToken cancellationToken); + + /// + /// Attempt to find a package's metadata using mirroring. This will merge + /// results from the configured upstream source with the locally indexed packages. + /// + /// The package's id to lookup + /// The package's version to lookup + /// The token to cancel the lookup + /// + /// The metadata for single version of a package. + /// Returns null if the package does not exist. + /// + Task FindPackageOrNullAsync(string id, NuGetVersion version, CancellationToken cancellationToken); + + /// + /// Determine whether a package exists locally or on the upstream source. + /// + /// The package id to search. + /// The package version to search. + /// A token to cancel the task. + /// Whether the package exists in the database. + Task ExistsAsync(string id, NuGetVersion version, CancellationToken cancellationToken); + + /// + /// Increment a package's download count. + /// + /// The id of the package to update. + /// The id of the package to update. + /// A token to cancel the task. + Task AddDownloadAsync(string packageId, NuGetVersion version, CancellationToken cancellationToken); + } +} diff --git a/src/BaGet.Core/Mirror/MirrorService.cs b/src/BaGet.Core/Mirror/PackageService.cs similarity index 59% rename from src/BaGet.Core/Mirror/MirrorService.cs rename to src/BaGet.Core/Mirror/PackageService.cs index 276847aa..1f176346 100644 --- a/src/BaGet.Core/Mirror/MirrorService.cs +++ b/src/BaGet.Core/Mirror/PackageService.cs @@ -8,20 +8,20 @@ namespace BaGet.Core { - public class MirrorService : IMirrorService + public class PackageService : IPackageService { - private readonly IPackageDatabase _localPackages; + private readonly IPackageDatabase _db; private readonly IUpstreamClient _upstream; private readonly IPackageIndexingService _indexer; - private readonly ILogger _logger; + private readonly ILogger _logger; - public MirrorService( - IPackageDatabase localPackages, + public PackageService( + IPackageDatabase db, IUpstreamClient upstream, IPackageIndexingService indexer, - ILogger logger) + ILogger logger) { - _localPackages = localPackages ?? throw new ArgumentNullException(nameof(localPackages)); + _db = db ?? throw new ArgumentNullException(nameof(db)); _upstream = upstream ?? throw new ArgumentNullException(nameof(upstream)); _indexer = indexer ?? throw new ArgumentNullException(nameof(indexer)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); @@ -34,7 +34,7 @@ public async Task> FindPackageVersionsAsync( var upstreamVersions = await _upstream.ListPackageVersionsAsync(id, cancellationToken); // Merge the local package versions into the upstream package versions. - var localPackages = await _localPackages.FindAsync(id, includeUnlisted: true, cancellationToken); + var localPackages = await _db.FindAsync(id, includeUnlisted: true, cancellationToken); var localVersions = localPackages.Select(p => p.Version); if (!upstreamVersions.Any()) return localVersions.ToList(); @@ -46,7 +46,7 @@ public async Task> FindPackageVersionsAsync( public async Task> FindPackagesAsync(string id, CancellationToken cancellationToken) { var upstreamPackages = await _upstream.ListPackagesAsync(id, cancellationToken); - var localPackages = await _localPackages.FindAsync(id, includeUnlisted: true, cancellationToken); + var localPackages = await _db.FindAsync(id, includeUnlisted: true, cancellationToken); if (!upstreamPackages.Any()) return localPackages; if (!localPackages.Any()) return upstreamPackages; @@ -63,32 +63,45 @@ public async Task> FindPackagesAsync(string id, Cancellat return result.Values.ToList(); } - public async Task MirrorAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + public async Task FindPackageOrNullAsync( + string id, + NuGetVersion version, + CancellationToken cancellationToken) { - if (await _localPackages.ExistsAsync(id, version, cancellationToken)) + if (!await MirrorAsync(id, version, cancellationToken)) { - return; + return null; } - _logger.LogInformation( - "Package {PackageId} {PackageVersion} does not exist locally. Indexing from upstream feed...", - id, - version); + return await _db.FindOrNullAsync(id, version, includeUnlisted: true, cancellationToken); + } - await IndexFromSourceAsync(id, version, cancellationToken); + public async Task ExistsAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + { + return await MirrorAsync(id, version, cancellationToken); + } - _logger.LogInformation( - "Finished indexing {PackageId} {PackageVersion} from the upstream feed", - id, - version); + public async Task AddDownloadAsync(string packageId, NuGetVersion version, CancellationToken cancellationToken) + { + await _db.AddDownloadAsync(packageId, version, cancellationToken); } - private async Task IndexFromSourceAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + /// + /// Index the package from an upstream if it does not exist locally. + /// + /// The package ID to index from an upstream. + /// The package version to index from an upstream. + /// + /// True if the package exists locally or was indexed from an upstream source. + private async Task MirrorAsync(string id, NuGetVersion version, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + if (await _db.ExistsAsync(id, version, cancellationToken)) + { + return true; + } _logger.LogInformation( - "Attempting to mirror package {PackageId} {PackageVersion}...", + "Package {PackageId} {PackageVersion} does not exist locally. Checking upstream feed...", id, version); @@ -99,10 +112,10 @@ private async Task IndexFromSourceAsync(string id, NuGetVersion version, Cancell if (packageStream == null) { _logger.LogWarning( - "Failed to download package {PackageId} {PackageVersion}", + "Upstream feed does not have package {PackageId} {PackageVersion}", id, version); - return; + return false; } _logger.LogInformation( @@ -113,10 +126,12 @@ private async Task IndexFromSourceAsync(string id, NuGetVersion version, Cancell var result = await _indexer.IndexAsync(packageStream, cancellationToken); _logger.LogInformation( - "Finished indexing package {PackageId} {PackageVersion} from upstream with result {Result}", + "Finished indexing package {PackageId} {PackageVersion} from upstream feed with result {Result}", id, version, result); + + return result == PackageIndexingResult.Success; } } catch (Exception e) @@ -126,6 +141,8 @@ private async Task IndexFromSourceAsync(string id, NuGetVersion version, Cancell "Failed to index package {PackageId} {PackageVersion} from upstream", id, version); + + return false; } } } diff --git a/src/BaGet.Core/PackageDatabase.cs b/src/BaGet.Core/PackageDatabase.cs index 0277ee34..98f5168c 100644 --- a/src/BaGet.Core/PackageDatabase.cs +++ b/src/BaGet.Core/PackageDatabase.cs @@ -97,9 +97,9 @@ public Task RelistPackageAsync(string id, NuGetVersion version, Cancellati return TryUpdatePackageAsync(id, version, p => p.Listed = true, cancellationToken); } - public Task AddDownloadAsync(string id, NuGetVersion version, CancellationToken cancellationToken) + public async Task AddDownloadAsync(string id, NuGetVersion version, CancellationToken cancellationToken) { - return TryUpdatePackageAsync(id, version, p => p.Downloads += 1, cancellationToken); + await TryUpdatePackageAsync(id, version, p => p.Downloads += 1, cancellationToken); } public async Task HardDeletePackageAsync(string id, NuGetVersion version, CancellationToken cancellationToken) diff --git a/src/BaGet.Core/Upstream/Clients/DisabledUpstreamClient.cs b/src/BaGet.Core/Upstream/Clients/DisabledUpstreamClient.cs new file mode 100644 index 00000000..482c46a6 --- /dev/null +++ b/src/BaGet.Core/Upstream/Clients/DisabledUpstreamClient.cs @@ -0,0 +1,35 @@ +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using NuGet.Versioning; + +namespace BaGet.Core +{ + /// + /// The client used when there are no upstream package sources. + /// + public class DisabledUpstreamClient : IUpstreamClient + { + private readonly IReadOnlyList _emptyVersionList = new List(); + private readonly IReadOnlyList _emptyPackageList = new List(); + + public Task> ListPackageVersionsAsync(string id, CancellationToken cancellationToken) + { + return Task.FromResult(_emptyVersionList); + } + + public Task> ListPackagesAsync(string id, CancellationToken cancellationToken) + { + return Task.FromResult(_emptyPackageList); + } + + public Task DownloadPackageOrNullAsync( + string id, + NuGetVersion version, + CancellationToken cancellationToken) + { + return Task.FromResult(null); + } + } +} diff --git a/src/BaGet.Web/Pages/Package.cshtml.cs b/src/BaGet.Web/Pages/Package.cshtml.cs index f647d5b4..8841648e 100644 --- a/src/BaGet.Web/Pages/Package.cshtml.cs +++ b/src/BaGet.Web/Pages/Package.cshtml.cs @@ -17,7 +17,7 @@ public class PackageModel : PageModel { private static readonly MarkdownPipeline MarkdownPipeline; - private readonly IMirrorService _mirror; + private readonly IPackageService _packages; private readonly IPackageContentService _content; private readonly ISearchService _search; private readonly IUrlGenerator _url; @@ -30,12 +30,12 @@ static PackageModel() } public PackageModel( - IMirrorService mirror, + IPackageService packages, IPackageContentService content, ISearchService search, IUrlGenerator url) { - _mirror = mirror ?? throw new ArgumentNullException(nameof(mirror)); + _packages = packages ?? throw new ArgumentNullException(nameof(packages)); _content = content ?? throw new ArgumentNullException(nameof(content)); _search = search ?? throw new ArgumentNullException(nameof(search)); _url = url ?? throw new ArgumentNullException(nameof(url)); @@ -62,7 +62,7 @@ public PackageModel( public async Task OnGetAsync(string id, string version, CancellationToken cancellationToken) { - var packages = await _mirror.FindPackagesAsync(id, cancellationToken); + var packages = await _packages.FindPackagesAsync(id, cancellationToken); var listedPackages = packages.Where(p => p.Listed).ToList(); // Try to find the requested version. diff --git a/tests/BaGet.Core.Tests/Services/MirrorServiceTests.cs b/tests/BaGet.Core.Tests/Services/PackageServiceTests.cs similarity index 73% rename from tests/BaGet.Core.Tests/Services/MirrorServiceTests.cs rename to tests/BaGet.Core.Tests/Services/PackageServiceTests.cs index 66442f76..8bbcb1d9 100644 --- a/tests/BaGet.Core.Tests/Services/MirrorServiceTests.cs +++ b/tests/BaGet.Core.Tests/Services/PackageServiceTests.cs @@ -11,7 +11,7 @@ namespace BaGet.Core.Tests { - public class MirrorServiceTests + public class PackageServiceTests { public class FindPackageVersionsAsync : FactsBase { @@ -97,7 +97,7 @@ private void Setup( localPackages = localPackages ?? new List(); upstreamPackages = upstreamPackages ?? new List(); - _packages + _db .Setup(p => p.FindAsync( "MyPackage", /*includeUnlisted: */ true, @@ -187,7 +187,7 @@ private void Setup( localPackages = localPackages ?? new List(); upstreamPackages = upstreamPackages ?? new List(); - _packages + _db .Setup(p => p.FindAsync( "MyPackage", /*includeUnlisted: */ true, @@ -202,19 +202,85 @@ private void Setup( } } - public class MirrorAsync : FactsBase + public class FindPackageOrNullAsync : MirrorAsync { - private readonly string _id = "MyPackage"; - private readonly NuGetVersion _version = new NuGetVersion("1.0.0"); + protected override async Task TargetAsync() + => await _target.FindPackageOrNullAsync(_id, _version, _cancellationToken); + + [Fact] + public async Task ExistsInDatabase() + { + var expected = new Package(); + + _db + .Setup(p => p.ExistsAsync(_id, _version, _cancellationToken)) + .ReturnsAsync(true); + _db + .Setup(p => p.FindOrNullAsync(_id, _version, /*includeUnlisted:*/ true, _cancellationToken)) + .ReturnsAsync(expected); + + var result = await _target.FindPackageOrNullAsync(_id, _version, _cancellationToken); + + Assert.Same(expected, result); + } + + [Fact] + public async Task DoesNotExistInDatabase() + { + _db + .Setup(p => p.FindOrNullAsync(_id, _version, /*includeUnlisted:*/ true, _cancellationToken)) + .ReturnsAsync((Package)null); + + var result = await _target.FindPackageOrNullAsync(_id, _version, _cancellationToken); + + Assert.Null(result); + } + } + + public class ExistsAsync : MirrorAsync + { + protected override async Task TargetAsync() => await _target.ExistsAsync(_id, _version, _cancellationToken); + + [Fact] + public async Task ExistsInDatabase() + { + _db + .Setup(p => p.ExistsAsync(_id, _version, _cancellationToken)) + .ReturnsAsync(true); + + var result = await _target.ExistsAsync(_id, _version, _cancellationToken); + + Assert.True(result); + } + + [Fact] + public async Task DoesNotExistInDatabase() + { + _db + .Setup(p => p.ExistsAsync(_id, _version, _cancellationToken)) + .ReturnsAsync(false); + + var result = await _target.ExistsAsync(_id, _version, _cancellationToken); + + Assert.False(result); + } + } + + public abstract class MirrorAsync : FactsBase + { + protected readonly string _id = "MyPackage"; + protected readonly NuGetVersion _version = new NuGetVersion("1.0.0"); + + protected abstract Task TargetAsync(); [Fact] public async Task SkipsIfAlreadyMirrored() { - _packages + _db .Setup(p => p.ExistsAsync(_id, _version, _cancellationToken)) .ReturnsAsync(true); - await _target.MirrorAsync(_id, _version, _cancellationToken); + await TargetAsync(); _indexer.Verify( i => i.IndexAsync(It.IsAny(), _cancellationToken), @@ -224,7 +290,7 @@ public async Task SkipsIfAlreadyMirrored() [Fact] public async Task SkipsIfUpstreamDoesntHavePackage() { - _packages + _db .Setup(p => p.ExistsAsync(_id, _version, _cancellationToken)) .ReturnsAsync(false); @@ -232,7 +298,7 @@ public async Task SkipsIfUpstreamDoesntHavePackage() .Setup(u => u.DownloadPackageOrNullAsync(_id, _version, _cancellationToken)) .ReturnsAsync((Stream)null); - await _target.MirrorAsync(_id, _version, _cancellationToken); + await TargetAsync(); _indexer.Verify( i => i.IndexAsync(It.IsAny(), _cancellationToken), @@ -242,7 +308,7 @@ public async Task SkipsIfUpstreamDoesntHavePackage() [Fact] public async Task SkipsIfUpstreamThrows() { - _packages + _db .Setup(p => p.ExistsAsync(_id, _version, _cancellationToken)) .ReturnsAsync(false); @@ -250,7 +316,7 @@ public async Task SkipsIfUpstreamThrows() .Setup(u => u.DownloadPackageOrNullAsync(_id, _version, _cancellationToken)) .ThrowsAsync(new InvalidOperationException("Hello world")); - await _target.MirrorAsync(_id, _version, _cancellationToken); + await TargetAsync(); _indexer.Verify( i => i.IndexAsync(It.IsAny(), _cancellationToken), @@ -260,7 +326,7 @@ public async Task SkipsIfUpstreamThrows() [Fact] public async Task MirrorsPackage() { - _packages + _db .Setup(p => p.ExistsAsync(_id, _version, _cancellationToken)) .ReturnsAsync(false); @@ -270,7 +336,7 @@ public async Task MirrorsPackage() .Setup(u => u.DownloadPackageOrNullAsync(_id, _version, _cancellationToken)) .ReturnsAsync(downloadStream); - await _target.MirrorAsync(_id, _version, _cancellationToken); + await TargetAsync(); _indexer.Verify( i => i.IndexAsync(It.IsAny(), _cancellationToken), @@ -279,26 +345,42 @@ public async Task MirrorsPackage() } } + public class AddDownloadAsync : FactsBase + { + [Fact] + public async Task AddsDownload() + { + var id = "Hello"; + var version = new NuGetVersion("1.2.3"); + + await _target.AddDownloadAsync(id, version, _cancellationToken); + + _db.Verify( + db => db.AddDownloadAsync(id, version, _cancellationToken), + Times.Once); + } + } + public class FactsBase { - protected readonly Mock _packages; + protected readonly Mock _db; protected readonly Mock _upstream; protected readonly Mock _indexer; protected readonly CancellationToken _cancellationToken = CancellationToken.None; - protected readonly MirrorService _target; + protected readonly PackageService _target; - public FactsBase() + protected FactsBase() { - _packages = new Mock(); + _db = new Mock(); _upstream = new Mock(); _indexer = new Mock(); - _target = new MirrorService( - _packages.Object, + _target = new PackageService( + _db.Object, _upstream.Object, _indexer.Object, - Mock.Of>()); + Mock.Of>()); } } } diff --git a/tests/BaGet.Web.Tests/Pages/PackageModelFacts.cs b/tests/BaGet.Web.Tests/Pages/PackageModelFacts.cs index cf802036..51a7d1cc 100644 --- a/tests/BaGet.Web.Tests/Pages/PackageModelFacts.cs +++ b/tests/BaGet.Web.Tests/Pages/PackageModelFacts.cs @@ -14,7 +14,7 @@ namespace BaGet.Web.Tests public class PackageModelFacts { private readonly Mock _content; - private readonly Mock _mirror; + private readonly Mock _packages; private readonly Mock _search; private readonly Mock _url; private readonly PackageModel _target; @@ -24,11 +24,11 @@ public class PackageModelFacts public PackageModelFacts() { _content = new Mock(); - _mirror = new Mock(); + _packages = new Mock(); _search = new Mock(); _url = new Mock(); _target = new PackageModel( - _mirror.Object, + _packages.Object, _content.Object, _search.Object, _url.Object); @@ -41,7 +41,7 @@ public PackageModelFacts() [Fact] public async Task ReturnsNotFound() { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List()); @@ -56,7 +56,7 @@ public async Task ReturnsNotFound() [Fact] public async Task ReturnsNotFoundIfAllUnlisted() { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -74,7 +74,7 @@ public async Task ReturnsNotFoundIfAllUnlisted() [Fact] public async Task ReturnsRequestedVersion() { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -101,7 +101,7 @@ public async Task ReturnsRequestedVersion() [Fact] public async Task ReturnsRequestedUnlistedVersion() { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -126,7 +126,7 @@ public async Task ReturnsRequestedUnlistedVersion() [Fact] public async Task FallsBackToLatestListedVersion() { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -156,7 +156,7 @@ public async Task FallsBackToLatestListedVersion() [InlineData(new[] { "tEmPlAte", "dOtNeTtOoL" }, /*expectDotnetTemplate: */ true, /*expectDotnetTool: */ true)] public async Task HandlesPackageTypes(IEnumerable packageTypes, bool expectDotnetTemplate, bool expectDotnetTool) { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -173,7 +173,7 @@ public async Task HandlesPackageTypes(IEnumerable packageTypes, bool exp [Fact] public async Task FindsDependentPackages() { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -201,7 +201,7 @@ public async Task FindsDependentPackages() [Fact] public async Task GroupsVersions() { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -256,7 +256,7 @@ public async Task GroupsVersions() [InlineData("net4.8", ".NET Framework 4.8")] public async Task PrettifiesTargetFramework(string targetFramework, string expectedResult) { - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -283,7 +283,7 @@ public async Task StatisticsIncludeUnlistedPackages() { var now = DateTime.Now; - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List { @@ -312,7 +312,7 @@ public async Task RendersReadme() readmeStream.Position = 0; - _mirror + _packages .Setup(m => m.FindPackagesAsync("testpackage", _cancellation)) .ReturnsAsync(new List {