Fix, cleanup, and optimize new cloud provider code.

Author: JimBobSquarePants

src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs

@@ -14,18 +14,13 @@ internal static class AmazonS3ClientFactory
     /// with the same name does not already exist.
     /// </summary>
     /// <param name="options">The AWS S3 Storage cache options.</param>
-    /// <param name="serviceProvider">The current service provider.</param>
     /// <returns>
     /// A new <see cref="AmazonS3Client"/>.
     /// </returns>
     /// <exception cref="ArgumentException">Invalid configuration.</exception>
-    public static AmazonS3Client CreateClient(IAWSS3BucketClientOptions options, IServiceProvider serviceProvider)
+    public static AmazonS3Client CreateClient(IAWSS3BucketClientOptions options)
     {
-        if (options.S3ClientProvider != null)
-        {
-            return options.S3ClientProvider(options, serviceProvider);
-        }
-        else if (!string.IsNullOrWhiteSpace(options.Endpoint))
+        if (!string.IsNullOrWhiteSpace(options.Endpoint))
         {
             // AccessKey can be empty.
             // AccessSecret can be empty.

src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs

@@ -13,11 +13,12 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS;
 /// <summary>
 /// Implements an AWS S3 Storage based cache.
 /// </summary>
-public class AWSS3StorageCache : IImageCache
+public class AWSS3StorageCache : IImageCache, IDisposable
 {
     private readonly IAmazonS3 amazonS3Client;
     private readonly string bucketName;
     private readonly string cacheFolder;
+    private bool isDisposed;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="AWSS3StorageCache"/> class.
@@ -29,7 +30,11 @@ public AWSS3StorageCache(IOptions<AWSS3StorageCacheOptions> cacheOptions, IServi
         Guard.NotNull(cacheOptions, nameof(cacheOptions));
         AWSS3StorageCacheOptions options = cacheOptions.Value;
         this.bucketName = options.BucketName;
-        this.amazonS3Client = AmazonS3ClientFactory.CreateClient(options, serviceProvider);
+
+        this.amazonS3Client =
+            options.S3ClientFactory?.Invoke(options, serviceProvider)
+            ?? AmazonS3ClientFactory.CreateClient(options);
+
         this.cacheFolder = string.IsNullOrEmpty(options.CacheFolder)
             ? string.Empty
             : options.CacheFolder.Trim().Trim('/') + '/';
@@ -84,23 +89,41 @@ public Task SetAsync(string key, Stream stream, ImageCacheMetadata metadata)
     /// and object data. <see cref="S3CannedACL.Private"/> specifies that the bucket
     /// data is private to the account owner.
     /// </param>
-    /// <param name="serviceProvider">The current service provider.</param>
     /// <returns>
     /// If the bucket does not already exist, a <see cref="PutBucketResponse"/> describing the newly
     /// created bucket. If the container already exists, <see langword="null"/>.
     /// </returns>
-    public static PutBucketResponse? CreateIfNotExists(
-        AWSS3StorageCacheOptions options,
-        S3CannedACL acl,
-        IServiceProvider serviceProvider)
-        => AsyncHelper.RunSync(() => CreateIfNotExistsAsync(options, acl, serviceProvider));
-
-    private static async Task<PutBucketResponse?> CreateIfNotExistsAsync(
-        AWSS3StorageCacheOptions options,
-        S3CannedACL acl,
-        IServiceProvider serviceProvider)
+    public static PutBucketResponse? CreateIfNotExists(AWSS3StorageCacheOptions options, S3CannedACL acl)
+        => AsyncHelper.RunSync(() => CreateIfNotExistsAsync(options, acl));
+
+    /// <summary>
+    /// Releases the unmanaged resources used by the <see cref="AWSS3StorageCache"/> and optionally releases the managed resources.
+    /// </summary>
+    /// <param name="disposing">true to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
+    protected virtual void Dispose(bool disposing)
+    {
+        if (!this.isDisposed)
+        {
+            if (disposing)
+            {
+                this.amazonS3Client?.Dispose();
+            }
+
+            this.isDisposed = true;
+        }
+    }
+
+    /// <inheritdoc/>
+    public void Dispose()
+    {
+        // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
+        this.Dispose(disposing: true);
+        GC.SuppressFinalize(this);
+    }
+
+    private static async Task<PutBucketResponse?> CreateIfNotExistsAsync(AWSS3StorageCacheOptions options, S3CannedACL acl)
     {
-        AmazonS3Client client = AmazonS3ClientFactory.CreateClient(options, serviceProvider);
+        AmazonS3Client client = AmazonS3ClientFactory.CreateClient(options);
 
         bool foundBucket = false;
         ListBucketsResponse listBucketsResponse = await client.ListBucketsAsync();

src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs

@@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS;
 public class AWSS3StorageCacheOptions : IAWSS3BucketClientOptions
 {
     /// <inheritdoc/>
-    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientProvider { get; set; } = null!;
+    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientFactory { get; set; }
 
     /// <inheritdoc/>
     public string? Region { get; set; }

src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs

@@ -11,49 +11,53 @@ namespace SixLabors.ImageSharp.Web;
 public interface IAWSS3BucketClientOptions
 {
     /// <summary>
-    /// Gets or sets a custom Azure AmazonS3Client provider
+    /// Gets or sets a custom a factory method to create an <see cref="AmazonS3Client"/>.
     /// </summary>
-    Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientProvider { get; set; }
+    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientFactory { get; set; }
 
     /// <summary>
     /// Gets or sets the AWS region endpoint (us-east-1/us-west-1/ap-southeast-2).
     /// </summary>
-    string? Region { get; set; }
+    public string? Region { get; set; }
 
     /// <summary>
-    /// Gets or sets the AWS bucket name.
+    /// Gets or sets the AWS bucket name. Cannot be <see langword="null"/>.
     /// </summary>
-    string BucketName { get; set; }
+    public string BucketName { get; set; }
 
     /// <summary>
     /// Gets or sets the AWS key - Can be used to override keys provided by the environment.
     /// If deploying inside an EC2 instance AWS keys will already be available via environment
-    /// variables and don't need to be specified. Follow AWS best security practices on  <see href="https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html"/>.
+    /// variables and don't need to be specified. Follow AWS best security practices on
+    /// <see href="https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html"/>.
     /// </summary>
-    string? AccessKey { get; set; }
+    public string? AccessKey { get; set; }
 
     /// <summary>
     /// Gets or sets the AWS endpoint - used to override the default service endpoint.
     /// If deploying inside an EC2 instance AWS keys will already be available via environment
-    /// variables and don't need to be specified. Follow AWS best security practices on  <see href="https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html"/>.
+    /// variables and don't need to be specified. Follow AWS best security practices on
+    /// <see href="https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html"/>.
     /// </summary>
-    string? AccessSecret { get; set; }
+    public string? AccessSecret { get; set; }
 
     /// <summary>
     /// Gets or sets the AWS endpoint - used for testing to over region endpoint allowing it
     /// to be set to localhost.
     /// </summary>
-    string? Endpoint { get; set; }
+    public string? Endpoint { get; set; }
 
     /// <summary>
     /// Gets or sets a value indicating whether the S3 accelerate endpoint is used.
-    /// The feature must be enabled on the bucket. Follow AWS instruction on <see href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/transfer-acceleration.html"/>.
+    /// The feature must be enabled on the bucket. Follow AWS instruction on
+    /// <see href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/transfer-acceleration.html"/>.
     /// </summary>
-    bool UseAccelerateEndpoint { get; set; }
+    public bool UseAccelerateEndpoint { get; set; }
 
     /// <summary>
     /// Gets or sets a value indicating the timeout for the S3 client.
-    /// If the value is set, the value is assigned to the Timeout property of the HttpWebRequest/HttpClient object used to send requests.
+    /// If the value is set, the value is assigned to the Timeout property of the HttpWebRequest/HttpClient
+    /// object used to send requests.
     /// </summary>
-    TimeSpan? Timeout { get; set; }
+    public TimeSpan? Timeout { get; set; }
 }

src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs

@@ -14,7 +14,7 @@ namespace SixLabors.ImageSharp.Web.Providers.AWS;
 /// <summary>
 /// Returns images stored in AWS S3.
 /// </summary>
-public class AWSS3StorageImageProvider : IImageProvider
+public class AWSS3StorageImageProvider : IImageProvider, IDisposable
 {
     /// <summary>
     /// Character array to remove from paths.
@@ -29,6 +29,7 @@ private readonly Dictionary<string, AmazonS3Client> buckets
 
     private readonly AWSS3StorageImageProviderOptions storageOptions;
     private Func<HttpContext, bool>? match;
+    private bool isDisposed;
 
     /// <summary>
     /// Contains various helper methods based on the current configuration.
@@ -41,7 +42,10 @@ private readonly Dictionary<string, AmazonS3Client> buckets
     /// <param name="storageOptions">The S3 storage options</param>
     /// <param name="formatUtilities">Contains various format helper methods based on the current configuration.</param>
     /// <param name="serviceProvider">The current service provider.</param>
-    public AWSS3StorageImageProvider(IOptions<AWSS3StorageImageProviderOptions> storageOptions, FormatUtilities formatUtilities, IServiceProvider serviceProvider)
+    public AWSS3StorageImageProvider(
+        IOptions<AWSS3StorageImageProviderOptions> storageOptions,
+        FormatUtilities formatUtilities,
+        IServiceProvider serviceProvider)
     {
         Guard.NotNull(storageOptions, nameof(storageOptions));
 
@@ -51,7 +55,11 @@ public AWSS3StorageImageProvider(IOptions<AWSS3StorageImageProviderOptions> stor
 
         foreach (AWSS3BucketClientOptions bucket in this.storageOptions.S3Buckets)
         {
-            this.buckets.Add(bucket.BucketName, AmazonS3ClientFactory.CreateClient(bucket, serviceProvider));
+            AmazonS3Client s3Client =
+                bucket.S3ClientFactory?.Invoke(bucket, serviceProvider)
+                ?? AmazonS3ClientFactory.CreateClient(bucket);
+
+            this.buckets.Add(bucket.BucketName, s3Client);
         }
     }
 
@@ -88,7 +96,7 @@ public bool IsValidRequest(HttpContext context)
         }
 
         int index = path.IndexOfAny(SlashChars);
-        string nameToMatch = index != -1 ? path.Substring(0, index) : path;
+        string nameToMatch = index != -1 ? path[..index] : path;
 
         foreach (string k in this.buckets.Keys)
         {
@@ -107,7 +115,7 @@ public bool IsValidRequest(HttpContext context)
         }
 
         // Key should be the remaining path string.
-        string key = path.Substring(bucketName.Length).TrimStart(SlashChars);
+        string key = path[bucketName.Length..].TrimStart(SlashChars);
 
         if (string.IsNullOrWhiteSpace(key))
         {
@@ -125,7 +133,7 @@ public bool IsValidRequest(HttpContext context)
 
     private bool IsMatch(HttpContext context)
     {
-        // Only match loosly here for performance.
+        // Only match loosely here for performance.
         // Path matching conflicts should be dealt with by configuration.
         string? path = context.Request.Path.Value?.TrimStart(SlashChars);
 
@@ -178,6 +186,37 @@ private static async Task<KeyExistsResult> KeyExists(IAmazonS3 s3Client, string
         }
     }
 
+    /// <summary>
+    /// Releases the unmanaged resources used by the <see cref="AWSS3StorageImageProvider"/> and optionally releases the managed resources.
+    /// </summary>
+    /// <param name="disposing">true to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
+
+    protected virtual void Dispose(bool disposing)
+    {
+        if (!this.isDisposed)
+        {
+            if (disposing)
+            {
+                foreach (AmazonS3Client client in this.buckets.Values)
+                {
+                    client?.Dispose();
+                }
+
+                this.buckets.Clear();
+            }
+
+            this.isDisposed = true;
+        }
+    }
+
+    /// <inheritdoc/>
+    public void Dispose()
+    {
+        this.Dispose(true);
+
+        GC.SuppressFinalize(this);
+    }
+
     private readonly record struct KeyExistsResult(GetObjectMetadataResponse Metadata)
     {
         public bool Exists => this.Metadata is not null;

src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs

@@ -22,7 +22,7 @@ public class AWSS3StorageImageProviderOptions
 public class AWSS3BucketClientOptions : IAWSS3BucketClientOptions
 {
     /// <inheritdoc/>
-    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientProvider { get; set; } = null!;
+    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientFactory { get; set; }
 
     /// <inheritdoc/>
     public string? Region { get; set; }

src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCache.cs

@@ -28,10 +28,11 @@ public AzureBlobStorageCache(IOptions<AzureBlobStorageCacheOptions> cacheOptions
         Guard.NotNull(cacheOptions, nameof(cacheOptions));
         AzureBlobStorageCacheOptions options = cacheOptions.Value;
 
-        this.container = options.BlobContainerClientProvider == null
-            ? new BlobContainerClient(options.ConnectionString, options.ContainerName)
-            : options.BlobContainerClientProvider(options, serviceProvider);
-        this.cacheFolder = string.IsNullOrEmpty(options.CacheFolder)
+        this.container =
+            options.BlobContainerClientFactory?.Invoke(options, serviceProvider)
+            ?? new BlobContainerClient(options.ConnectionString, options.ContainerName);
+
+        this.cacheFolder = string.IsNullOrWhiteSpace(options.CacheFolder)
             ? string.Empty
             : options.CacheFolder.Trim().Trim('/') + '/';
     }

src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCacheOptions.cs

@@ -11,9 +11,9 @@ namespace SixLabors.ImageSharp.Web.Caching.Azure;
 public class AzureBlobStorageCacheOptions
 {
     /// <summary>
-    /// Gets or sets a custom Azure BlobContainerClient provider
+    /// Gets or sets a factory method to create an <see cref="BlobContainerClient"/>.
     /// </summary>
-    public Func<AzureBlobStorageCacheOptions, IServiceProvider, BlobContainerClient>? BlobContainerClientProvider { get; set; } = null!;
+    public Func<AzureBlobStorageCacheOptions, IServiceProvider, BlobContainerClient>? BlobContainerClientFactory { get; set; }
 
     /// <summary>
     /// Gets or sets the Azure Blob Storage connection string.

src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProvider.cs

@@ -53,11 +53,11 @@ public AzureBlobStorageImageProvider(
 
         foreach (AzureBlobContainerClientOptions container in storageOptions.Value.BlobContainers)
         {
-            BlobContainerClient containerClient = container.BlobContainerClientProvider == null
-                ? new BlobContainerClient(container.ConnectionString, container.ContainerName)
-                : container.BlobContainerClientProvider(container, serviceProvider);
+            BlobContainerClient client =
+                container.BlobContainerClientFactory?.Invoke(container, serviceProvider)
+                ?? new BlobContainerClient(container.ConnectionString, container.ContainerName);
 
-            this.containers.Add(container.ContainerName!, containerClient);
+            this.containers.Add(client.Name, client);
         }
     }
 
@@ -109,7 +109,7 @@ public Func<HttpContext, bool> Match
         }
 
         // Blob name should be the remaining path string.
-        string blobName = path.Substring(containerName.Length).TrimStart(SlashChars);
+        string blobName = path[containerName.Length..].TrimStart(SlashChars);
 
         if (string.IsNullOrWhiteSpace(blobName))
         {
@@ -132,7 +132,7 @@ public bool IsValidRequest(HttpContext context)
 
     private bool IsMatch(HttpContext context)
     {
-        // Only match loosly here for performance.
+        // Only match loosely here for performance.
         // Path matching conflicts should be dealt with by configuration.
         string? path = context.Request.Path.Value?.TrimStart(SlashChars);
 

src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProviderOptions.cs

@@ -22,20 +22,20 @@ public class AzureBlobStorageImageProviderOptions
 public class AzureBlobContainerClientOptions
 {
     /// <summary>
-    /// Gets or sets a custom Azure BlobServiceClient provider
+    /// Gets or sets a factory method to create an <see cref="BlobContainerClient"/>.
     /// </summary>
-    public Func<AzureBlobContainerClientOptions, IServiceProvider, BlobContainerClient>? BlobContainerClientProvider { get; set; } = null!;
+    public Func<AzureBlobContainerClientOptions, IServiceProvider, BlobContainerClient>? BlobContainerClientFactory { get; set; }
 
     /// <summary>
     /// Gets or sets the Azure Blob Storage connection string.
     /// <see href="https://docs.microsoft.com/en-us/azure/storage/common/storage-configure-connection-string."/>
     /// </summary>
-    public string? ConnectionString { get; set; }
+    public string ConnectionString { get; set; } = null!;
 
     /// <summary>
     /// Gets or sets the Azure Blob Storage container name.
-    /// Must conform to Azure Blob Storage container naming guidlines.
+    /// Must conform to Azure Blob Storage container naming guidelines.
     /// <see href="https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata#container-names"/>
     /// </summary>
-    public string? ContainerName { get; set; }
+    public string ContainerName { get; set; } = null!;
 }