Refactor package mirroring (loic-sharma#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 loic-sharma#699

.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

src/BaGet.Azure/Table/TablePackageDatabase.cs

@@ -48,7 +48,7 @@ public async Task<PackageAddResult> AddAsync(Package package, CancellationToken
             return PackageAddResult.Success;
         }
 
-        public async Task<bool> AddDownloadAsync(
+        public async Task AddDownloadAsync(
             string id,
             NuGetVersion version,
             CancellationToken cancellationToken)
@@ -68,13 +68,13 @@ public async Task<bool> 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())

src/BaGet.Core/Content/DefaultPackageContentService.cs

@@ -9,22 +9,17 @@
 namespace BaGet.Core
 {
     /// <summary>
-    /// Implements the NuGet Package Content resource. Supports read-through caching.
-    /// Tracks state in a database (<see cref="IPackageDatabase"/>) and stores packages
-    /// using <see cref="IPackageStorageService"/>.
+    /// Implements the NuGet Package Content resource in NuGet's V3 protocol.
     /// </summary>
     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<PackageVersionsResponse> 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<Stream> 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<Stream> 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<Stream> GetPackageManifestStreamOrNullAsync(string id, NuGetVe
 
         public async Task<Stream> 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<Stream> GetPackageReadmeStreamOrNullAsync(string id, NuGetVers
 
         public async Task<Stream> 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;
             }

src/BaGet.Core/Extensions/DependencyInjectionExtensions.cs

@@ -92,22 +92,22 @@ private static void AddBaGetServices(this IServiceCollection services)
             services.TryAddTransient<IPackageDeletionService, PackageDeletionService>();
             services.TryAddTransient<IPackageIndexingService, PackageIndexingService>();
             services.TryAddTransient<IPackageMetadataService, DefaultPackageMetadataService>();
+            services.TryAddTransient<IPackageService, PackageService>();
             services.TryAddTransient<IPackageStorageService, PackageStorageService>();
             services.TryAddTransient<IServiceIndexService, BaGetServiceIndex>();
             services.TryAddTransient<ISymbolIndexingService, SymbolIndexingService>();
             services.TryAddTransient<ISymbolStorageService, SymbolStorageService>();
 
             services.TryAddTransient<DatabaseSearchService>();
             services.TryAddTransient<FileStorageService>();
-            services.TryAddTransient<MirrorService>();
+            services.TryAddTransient<PackageService>();
             services.TryAddTransient<V2UpstreamClient>();
             services.TryAddTransient<V3UpstreamClient>();
-            services.TryAddTransient<DisabledMirrorService>();
+            services.TryAddTransient<DisabledUpstreamClient>();
             services.TryAddSingleton<NullStorageService>();
             services.TryAddTransient<PackageDatabase>();
 
-            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<IOptionsSnapshot<MirrorOptions>>();
-            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<DisabledUpstreamClient>();
+            }
 
-        private static IUpstreamClient IMirrorClientFactory(IServiceProvider provider)
-        {
-            var options = provider.GetRequiredService<IOptionsSnapshot<MirrorOptions>>();
-            var service = options.Value.Legacy ? typeof(V2UpstreamClient) : typeof(V3UpstreamClient);
+            else if (options.Value.Legacy)
+            {
+                return provider.GetRequiredService<V2UpstreamClient>();
+            }
 
-            return (IUpstreamClient)provider.GetRequiredService(service);
+            else
+            {
+                return provider.GetRequiredService<V3UpstreamClient>();
+            }
         }
     }
 }

src/BaGet.Core/IPackageDatabase.cs

@@ -83,8 +83,8 @@ Task<Package> FindOrNullAsync(
         /// <param name="id">The id of the package to update.</param>
         /// <param name="version">The id of the package to update.</param>
         /// <param name="cancellationToken">A token to cancel the task.</param>
-        /// <returns>False if the package does not exist.</returns>
-        Task<bool> AddDownloadAsync(string id, NuGetVersion version, CancellationToken cancellationToken);
+        /// <returns>Task that completes when the package's download has been incremented.</returns>
+        Task AddDownloadAsync(string id, NuGetVersion version, CancellationToken cancellationToken);
 
         /// <summary>
         /// Completely remove the package from the database.

src/BaGet.Core/Metadata/DefaultPackageMetadataService.cs

@@ -10,16 +10,13 @@ namespace BaGet.Core
     /// <inheritdoc />
     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<BaGetRegistrationIndexResponse> 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<RegistrationLeafResponse> 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;

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
+{
+    /// <summary>
+    /// The service that combines the state of indexed packages and
+    /// upstream packages.
+    /// For upstream packages, see <see cref="IUpstreamClient"/>.
+    /// For indexed packages, see <see cref="IPackageDatabase"/>.
+    /// </summary>
+    public interface IPackageService
+    {
+        /// <summary>
+        /// Attempt to find a package's versions using mirroring. This will merge
+        /// results from the configured upstream source with the locally indexed packages.
+        /// </summary>
+        /// <param name="id">The package's id to lookup</param>
+        /// <param name="cancellationToken">The token to cancel the lookup</param>
+        /// <returns>
+        /// The package's versions, or an empty list if the package cannot be found.
+        /// This includes unlisted versions.
+        /// </returns>
+        Task<IReadOnlyList<NuGetVersion>> FindPackageVersionsAsync(string id, CancellationToken cancellationToken);
+
+        /// <summary>
+        /// Attempt to find a package's metadata using mirroring. This will merge
+        /// results from the configured upstream source with the locally indexed packages.
+        /// </summary>
+        /// <param name="id">The package's id to lookup</param>
+        /// <param name="cancellationToken">The token to cancel the lookup</param>
+        /// <returns>
+        /// The metadata for all versions of a package, including unlisted versions.
+        /// Returns an empty list if the package cannot be found.
+        /// </returns>
+        Task<IReadOnlyList<Package>> FindPackagesAsync(string id, CancellationToken cancellationToken);
+
+        /// <summary>
+        /// Attempt to find a package's metadata using mirroring. This will merge
+        /// results from the configured upstream source with the locally indexed packages.
+        /// </summary>
+        /// <param name="id">The package's id to lookup</param>
+        /// <param name="version">The package's version to lookup</param>
+        /// <param name="cancellationToken">The token to cancel the lookup</param>
+        /// <returns>
+        /// The metadata for single version of a package.
+        /// Returns null if the package does not exist.
+        /// </returns>
+        Task<Package> FindPackageOrNullAsync(string id, NuGetVersion version, CancellationToken cancellationToken);
+
+        /// <summary>
+        /// Determine whether a package exists locally or on the upstream source.
+        /// </summary>
+        /// <param name="id">The package id to search.</param>
+        /// <param name="version">The package version to search.</param>
+        /// <param name="cancellationToken">A token to cancel the task.</param>
+        /// <returns>Whether the package exists in the database.</returns>
+        Task<bool> ExistsAsync(string id, NuGetVersion version, CancellationToken cancellationToken);
+
+        /// <summary>
+        /// Increment a package's download count.
+        /// </summary>
+        /// <param name="packageId">The id of the package to update.</param>
+        /// <param name="version">The id of the package to update.</param>
+        /// <param name="cancellationToken">A token to cancel the task.</param>
+        Task AddDownloadAsync(string packageId, NuGetVersion version, CancellationToken cancellationToken);
+    }
+}