From e06dac58d98358bea2b6c4adbbe6735685c422f1 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 1 Aug 2025 13:02:50 +1000 Subject: [PATCH 1/3] Fix, cleanup, and optimize new cloud provider code. --- .../AmazonS3ClientFactory.cs | 9 +--- .../Caching/AWSS3StorageCache.cs | 51 ++++++++++++++----- .../Caching/AWSS3StorageCacheOptions.cs | 2 +- .../IAWSS3BucketClientOptions.cs | 32 +++++++----- .../Providers/AWSS3StorageImageProvider.cs | 51 ++++++++++++++++--- .../AWSS3StorageImageProviderOptions.cs | 2 +- .../Caching/AzureBlobStorageCache.cs | 9 ++-- .../Caching/AzureBlobStorageCacheOptions.cs | 4 +- .../AzureBlobStorageImageProvider.cs | 12 ++--- .../AzureBlobStorageImageProviderOptions.cs | 10 ++-- ...torageCacheCacheFolderTestServerFixture.cs | 2 +- .../AWSS3StorageCacheTestServerFixture.cs | 2 +- .../AWSS3StorageImageProviderFactory.cs | 4 +- .../TestUtilities/ServerTestBase.cs | 8 +-- 14 files changed, 130 insertions(+), 68 deletions(-) diff --git a/src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs b/src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs index bbbf557c..d3589384 100644 --- a/src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs +++ b/src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs @@ -14,18 +14,13 @@ internal static class AmazonS3ClientFactory /// with the same name does not already exist. /// /// The AWS S3 Storage cache options. - /// The current service provider. /// /// A new . /// /// Invalid configuration. - 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. diff --git a/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs b/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs index a22db3f5..a4e6e065 100644 --- a/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs +++ b/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs @@ -13,11 +13,12 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS; /// /// Implements an AWS S3 Storage based cache. /// -public class AWSS3StorageCache : IImageCache +public class AWSS3StorageCache : IImageCache, IDisposable { private readonly IAmazonS3 amazonS3Client; private readonly string bucketName; private readonly string cacheFolder; + private bool isDisposed; /// /// Initializes a new instance of the class. @@ -29,7 +30,11 @@ public AWSS3StorageCache(IOptions 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. specifies that the bucket /// data is private to the account owner. /// - /// The current service provider. /// /// If the bucket does not already exist, a describing the newly /// created bucket. If the container already exists, . /// - public static PutBucketResponse? CreateIfNotExists( - AWSS3StorageCacheOptions options, - S3CannedACL acl, - IServiceProvider serviceProvider) - => AsyncHelper.RunSync(() => CreateIfNotExistsAsync(options, acl, serviceProvider)); - - private static async Task CreateIfNotExistsAsync( - AWSS3StorageCacheOptions options, - S3CannedACL acl, - IServiceProvider serviceProvider) + public static PutBucketResponse? CreateIfNotExists(AWSS3StorageCacheOptions options, S3CannedACL acl) + => AsyncHelper.RunSync(() => CreateIfNotExistsAsync(options, acl)); + + /// + /// Releases the unmanaged resources used by the and optionally releases the managed resources. + /// + /// true to release both managed and unmanaged resources; false to release only unmanaged resources. + protected virtual void Dispose(bool disposing) + { + if (!this.isDisposed) + { + if (disposing) + { + this.amazonS3Client?.Dispose(); + } + + this.isDisposed = true; + } + } + + /// + 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 CreateIfNotExistsAsync(AWSS3StorageCacheOptions options, S3CannedACL acl) { - AmazonS3Client client = AmazonS3ClientFactory.CreateClient(options, serviceProvider); + AmazonS3Client client = AmazonS3ClientFactory.CreateClient(options); bool foundBucket = false; ListBucketsResponse listBucketsResponse = await client.ListBucketsAsync(); diff --git a/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs b/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs index 68d5c94c..ef0d7f69 100644 --- a/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs +++ b/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs @@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS; public class AWSS3StorageCacheOptions : IAWSS3BucketClientOptions { /// - public Func? S3ClientProvider { get; set; } = null!; + public Func? S3ClientFactory { get; set; } /// public string? Region { get; set; } diff --git a/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs b/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs index 25310fa3..00b38998 100644 --- a/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs +++ b/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs @@ -11,49 +11,53 @@ namespace SixLabors.ImageSharp.Web; public interface IAWSS3BucketClientOptions { /// - /// Gets or sets a custom Azure AmazonS3Client provider + /// Gets or sets a custom a factory method to create an . /// - Func? S3ClientProvider { get; set; } + public Func? S3ClientFactory { get; set; } /// /// Gets or sets the AWS region endpoint (us-east-1/us-west-1/ap-southeast-2). /// - string? Region { get; set; } + public string? Region { get; set; } /// - /// Gets or sets the AWS bucket name. + /// Gets or sets the AWS bucket name. Cannot be . /// - string BucketName { get; set; } + public string BucketName { get; set; } /// /// 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 . + /// variables and don't need to be specified. Follow AWS best security practices on + /// . /// - string? AccessKey { get; set; } + public string? AccessKey { get; set; } /// /// 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 . + /// variables and don't need to be specified. Follow AWS best security practices on + /// . /// - string? AccessSecret { get; set; } + public string? AccessSecret { get; set; } /// /// Gets or sets the AWS endpoint - used for testing to over region endpoint allowing it /// to be set to localhost. /// - string? Endpoint { get; set; } + public string? Endpoint { get; set; } /// /// 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 . + /// The feature must be enabled on the bucket. Follow AWS instruction on + /// . /// - bool UseAccelerateEndpoint { get; set; } + public bool UseAccelerateEndpoint { get; set; } /// /// 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. /// - TimeSpan? Timeout { get; set; } + public TimeSpan? Timeout { get; set; } } diff --git a/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs b/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs index 4c3d5b24..b6dae293 100644 --- a/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs +++ b/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs @@ -14,7 +14,7 @@ namespace SixLabors.ImageSharp.Web.Providers.AWS; /// /// Returns images stored in AWS S3. /// -public class AWSS3StorageImageProvider : IImageProvider +public class AWSS3StorageImageProvider : IImageProvider, IDisposable { /// /// Character array to remove from paths. @@ -29,6 +29,7 @@ private readonly Dictionary buckets private readonly AWSS3StorageImageProviderOptions storageOptions; private Func? match; + private bool isDisposed; /// /// Contains various helper methods based on the current configuration. @@ -41,7 +42,10 @@ private readonly Dictionary buckets /// The S3 storage options /// Contains various format helper methods based on the current configuration. /// The current service provider. - public AWSS3StorageImageProvider(IOptions storageOptions, FormatUtilities formatUtilities, IServiceProvider serviceProvider) + public AWSS3StorageImageProvider( + IOptions storageOptions, + FormatUtilities formatUtilities, + IServiceProvider serviceProvider) { Guard.NotNull(storageOptions, nameof(storageOptions)); @@ -51,7 +55,11 @@ public AWSS3StorageImageProvider(IOptions 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 KeyExists(IAmazonS3 s3Client, string } } + /// + /// Releases the unmanaged resources used by the and optionally releases the managed resources. + /// + /// true to release both managed and unmanaged resources; false to release only unmanaged resources. + + 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; + } + } + + /// + public void Dispose() + { + this.Dispose(true); + + GC.SuppressFinalize(this); + } + private readonly record struct KeyExistsResult(GetObjectMetadataResponse Metadata) { public bool Exists => this.Metadata is not null; diff --git a/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs b/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs index 00c97a53..152e2c1e 100644 --- a/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs +++ b/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs @@ -22,7 +22,7 @@ public class AWSS3StorageImageProviderOptions public class AWSS3BucketClientOptions : IAWSS3BucketClientOptions { /// - public Func? S3ClientProvider { get; set; } = null!; + public Func? S3ClientFactory { get; set; } /// public string? Region { get; set; } diff --git a/src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCache.cs b/src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCache.cs index 089062dd..e49d281a 100644 --- a/src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCache.cs +++ b/src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCache.cs @@ -28,10 +28,11 @@ public AzureBlobStorageCache(IOptions 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('/') + '/'; } diff --git a/src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCacheOptions.cs b/src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCacheOptions.cs index c74ae302..ab119aa6 100644 --- a/src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCacheOptions.cs +++ b/src/ImageSharp.Web.Providers.Azure/Caching/AzureBlobStorageCacheOptions.cs @@ -11,9 +11,9 @@ namespace SixLabors.ImageSharp.Web.Caching.Azure; public class AzureBlobStorageCacheOptions { /// - /// Gets or sets a custom Azure BlobContainerClient provider + /// Gets or sets a factory method to create an . /// - public Func? BlobContainerClientProvider { get; set; } = null!; + public Func? BlobContainerClientFactory { get; set; } /// /// Gets or sets the Azure Blob Storage connection string. diff --git a/src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProvider.cs b/src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProvider.cs index 41da7281..60069a55 100644 --- a/src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProvider.cs +++ b/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 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); diff --git a/src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProviderOptions.cs b/src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProviderOptions.cs index aa5e3164..9b1b8a1b 100644 --- a/src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProviderOptions.cs +++ b/src/ImageSharp.Web.Providers.Azure/Providers/AzureBlobStorageImageProviderOptions.cs @@ -22,20 +22,20 @@ public class AzureBlobStorageImageProviderOptions public class AzureBlobContainerClientOptions { /// - /// Gets or sets a custom Azure BlobServiceClient provider + /// Gets or sets a factory method to create an . /// - public Func? BlobContainerClientProvider { get; set; } = null!; + public Func? BlobContainerClientFactory { get; set; } /// /// Gets or sets the Azure Blob Storage connection string. /// /// - public string? ConnectionString { get; set; } + public string ConnectionString { get; set; } = null!; /// /// 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. /// /// - public string? ContainerName { get; set; } + public string ContainerName { get; set; } = null!; } diff --git a/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageCacheCacheFolderTestServerFixture.cs b/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageCacheCacheFolderTestServerFixture.cs index 89c7726d..d452ef6b 100644 --- a/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageCacheCacheFolderTestServerFixture.cs +++ b/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageCacheCacheFolderTestServerFixture.cs @@ -35,7 +35,7 @@ protected override void ConfigureCustomServices(IServiceCollection services, IIm o.Timeout = TestConstants.AWSTimeout; o.CacheFolder = TestConstants.AWSCacheFolder; - AWSS3StorageCache.CreateIfNotExists(o, S3CannedACL.Private, services.BuildServiceProvider()); + AWSS3StorageCache.CreateIfNotExists(o, S3CannedACL.Private); }) .SetCache(); } diff --git a/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageCacheTestServerFixture.cs b/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageCacheTestServerFixture.cs index c3ac3ac8..cb016f22 100644 --- a/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageCacheTestServerFixture.cs +++ b/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageCacheTestServerFixture.cs @@ -34,7 +34,7 @@ protected override void ConfigureCustomServices(IServiceCollection services, IIm o.Region = TestConstants.AWSRegion; o.Timeout = TestConstants.AWSTimeout; - AWSS3StorageCache.CreateIfNotExists(o, S3CannedACL.Private, services.BuildServiceProvider()); + AWSS3StorageCache.CreateIfNotExists(o, S3CannedACL.Private); }) .SetCache(); } diff --git a/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs b/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs index 297c1cb7..dee5dc9f 100644 --- a/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs +++ b/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs @@ -27,7 +27,7 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A { // Upload an image to the AWS Test Storage; AWSS3BucketClientOptions bucketOptions = options.S3Buckets.First(); - AmazonS3Client amazonS3Client = AmazonS3ClientFactory.CreateClient(bucketOptions, services); + AmazonS3Client amazonS3Client = AmazonS3ClientFactory.CreateClient(bucketOptions); ListBucketsResponse listBucketsResponse = await amazonS3Client.ListBucketsAsync(); bool foundBucket = false; @@ -56,7 +56,7 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A catch (AmazonS3Exception e) { // CI tests are run in parallel and can sometimes return a - // false negative for the existance of a bucket. + // false negative for the existence of a bucket. if (string.Equals(e.ErrorCode, "BucketAlreadyExists", StringComparison.Ordinal)) { return; diff --git a/tests/ImageSharp.Web.Tests/TestUtilities/ServerTestBase.cs b/tests/ImageSharp.Web.Tests/TestUtilities/ServerTestBase.cs index 89bb989b..3cce43db 100644 --- a/tests/ImageSharp.Web.Tests/TestUtilities/ServerTestBase.cs +++ b/tests/ImageSharp.Web.Tests/TestUtilities/ServerTestBase.cs @@ -23,11 +23,11 @@ protected ServerTestBase(TFixture fixture, ITestOutputHelper outputHelper, strin this.OutputHelper.WriteLine(typeof(TFixture).Name); - //this.OutputHelper.WriteLine("EnvironmentalVariables"); - //foreach (DictionaryEntry item in Environment.GetEnvironmentVariables()) - //{ + // this.OutputHelper.WriteLine("EnvironmentalVariables"); + // foreach (DictionaryEntry item in Environment.GetEnvironmentVariables()) + // { // this.OutputHelper.WriteLine($"Key = {item.Key}, Value = {item.Value}"); - //} + // } } public TFixture Fixture { get; } From 4fa5aad189690f80e7d5813750b594d75339773b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 1 Aug 2025 14:16:58 +1000 Subject: [PATCH 2/3] Fix memory leak in AWS library. --- .../AmazonS3BucketClient.cs | 56 +++++++++++++++++++ .../AmazonS3ClientFactory.cs | 8 +-- .../Caching/AWSS3StorageCache.cs | 14 +++-- .../Caching/AWSS3StorageCacheOptions.cs | 4 +- .../IAWSS3BucketClientOptions.cs | 5 +- .../Providers/AWSS3StorageImageProvider.cs | 13 ++--- .../AWSS3StorageImageProviderOptions.cs | 4 +- .../AWSS3StorageImageProviderFactory.cs | 10 ++-- 8 files changed, 84 insertions(+), 30 deletions(-) create mode 100644 src/ImageSharp.Web.Providers.AWS/AmazonS3BucketClient.cs diff --git a/src/ImageSharp.Web.Providers.AWS/AmazonS3BucketClient.cs b/src/ImageSharp.Web.Providers.AWS/AmazonS3BucketClient.cs new file mode 100644 index 00000000..d8940666 --- /dev/null +++ b/src/ImageSharp.Web.Providers.AWS/AmazonS3BucketClient.cs @@ -0,0 +1,56 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using Amazon.S3; + +namespace SixLabors.ImageSharp.Web; + +/// +/// Represents a scoped Amazon S3 client instance that is explicitly associated with a single S3 bucket. +/// This wrapper provides a strongly-typed link between the client and the bucket it operates on, +/// and optionally manages the lifetime of the underlying . +/// +public sealed class AmazonS3BucketClient : IDisposable +{ + private readonly bool disposeClient; + + /// + /// Initializes a new instance of the class. + /// + /// + /// The bucket name associated with this client instance. + /// + /// + /// The underlying Amazon S3 client instance. This should be an already configured instance of . + /// + /// + /// A value indicating whether the underlying client should be disposed when this instance is disposed. + /// + public AmazonS3BucketClient(string bucketName, AmazonS3Client client, bool disposeClient = true) + { + Guard.NotNullOrWhiteSpace(bucketName, nameof(bucketName)); + Guard.NotNull(client, nameof(client)); + this.BucketName = bucketName; + this.Client = client; + this.disposeClient = disposeClient; + } + + /// + /// Gets the bucket name associated with this client instance. + /// + public string BucketName { get; } + + /// + /// Gets the underlying Amazon S3 client instance. + /// + public AmazonS3Client Client { get; } + + /// + public void Dispose() + { + if (this.disposeClient) + { + this.Client.Dispose(); + } + } +} diff --git a/src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs b/src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs index d3589384..ff21b56a 100644 --- a/src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs +++ b/src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs @@ -18,7 +18,7 @@ internal static class AmazonS3ClientFactory /// A new . /// /// Invalid configuration. - public static AmazonS3Client CreateClient(IAWSS3BucketClientOptions options) + public static AmazonS3BucketClient CreateClient(IAWSS3BucketClientOptions options) { if (!string.IsNullOrWhiteSpace(options.Endpoint)) { @@ -27,7 +27,7 @@ public static AmazonS3Client CreateClient(IAWSS3BucketClientOptions options) // PathStyle endpoint doesn't support AccelerateEndpoint. AmazonS3Config config = new() { ServiceURL = options.Endpoint, ForcePathStyle = true, AuthenticationRegion = options.Region }; SetTimeout(config, options.Timeout); - return new AmazonS3Client(options.AccessKey, options.AccessSecret, config); + return new(options.BucketName, new AmazonS3Client(options.AccessKey, options.AccessSecret, config)); } else if (!string.IsNullOrWhiteSpace(options.AccessKey)) { @@ -36,14 +36,14 @@ public static AmazonS3Client CreateClient(IAWSS3BucketClientOptions options) RegionEndpoint region = RegionEndpoint.GetBySystemName(options.Region); AmazonS3Config config = new() { RegionEndpoint = region, UseAccelerateEndpoint = options.UseAccelerateEndpoint }; SetTimeout(config, options.Timeout); - return new AmazonS3Client(options.AccessKey, options.AccessSecret, config); + return new(options.BucketName, new AmazonS3Client(options.AccessKey, options.AccessSecret, config)); } else if (!string.IsNullOrWhiteSpace(options.Region)) { RegionEndpoint region = RegionEndpoint.GetBySystemName(options.Region); AmazonS3Config config = new() { RegionEndpoint = region, UseAccelerateEndpoint = options.UseAccelerateEndpoint }; SetTimeout(config, options.Timeout); - return new AmazonS3Client(config); + return new(options.BucketName, new AmazonS3Client(config)); } else { diff --git a/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs b/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs index a4e6e065..aef41ac7 100644 --- a/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs +++ b/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs @@ -15,7 +15,7 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS; /// public class AWSS3StorageCache : IImageCache, IDisposable { - private readonly IAmazonS3 amazonS3Client; + private readonly AmazonS3BucketClient amazonS3Client; private readonly string bucketName; private readonly string cacheFolder; private bool isDisposed; @@ -29,12 +29,13 @@ public AWSS3StorageCache(IOptions cacheOptions, IServi { Guard.NotNull(cacheOptions, nameof(cacheOptions)); AWSS3StorageCacheOptions options = cacheOptions.Value; - this.bucketName = options.BucketName; this.amazonS3Client = options.S3ClientFactory?.Invoke(options, serviceProvider) ?? AmazonS3ClientFactory.CreateClient(options); + this.bucketName = this.amazonS3Client.BucketName; + this.cacheFolder = string.IsNullOrEmpty(options.CacheFolder) ? string.Empty : options.CacheFolder.Trim().Trim('/') + '/'; @@ -48,8 +49,8 @@ public AWSS3StorageCache(IOptions cacheOptions, IServi try { // HEAD request throws a 404 if not found. - MetadataCollection metadata = (await this.amazonS3Client.GetObjectMetadataAsync(request)).Metadata; - return new AWSS3StorageCacheResolver(this.amazonS3Client, this.bucketName, keyWithFolder, metadata); + MetadataCollection metadata = (await this.amazonS3Client.Client.GetObjectMetadataAsync(request)).Metadata; + return new AWSS3StorageCacheResolver(this.amazonS3Client.Client, this.bucketName, keyWithFolder, metadata); } catch { @@ -75,7 +76,7 @@ public Task SetAsync(string key, Stream stream, ImageCacheMetadata metadata) request.Metadata.Add(d.Key, d.Value); } - return this.amazonS3Client.PutObjectAsync(request); + return this.amazonS3Client.Client.PutObjectAsync(request); } /// @@ -123,7 +124,8 @@ public void Dispose() private static async Task CreateIfNotExistsAsync(AWSS3StorageCacheOptions options, S3CannedACL acl) { - AmazonS3Client client = AmazonS3ClientFactory.CreateClient(options); + using AmazonS3BucketClient bucketClient = AmazonS3ClientFactory.CreateClient(options); + AmazonS3Client client = bucketClient.Client; bool foundBucket = false; ListBucketsResponse listBucketsResponse = await client.ListBucketsAsync(); diff --git a/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs b/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs index ef0d7f69..e9c4a606 100644 --- a/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs +++ b/src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs @@ -1,8 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using Amazon.S3; - namespace SixLabors.ImageSharp.Web.Caching.AWS; /// @@ -11,7 +9,7 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS; public class AWSS3StorageCacheOptions : IAWSS3BucketClientOptions { /// - public Func? S3ClientFactory { get; set; } + public Func? S3ClientFactory { get; set; } /// public string? Region { get; set; } diff --git a/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs b/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs index 00b38998..93b8493a 100644 --- a/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs +++ b/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs @@ -13,7 +13,7 @@ public interface IAWSS3BucketClientOptions /// /// Gets or sets a custom a factory method to create an . /// - public Func? S3ClientFactory { get; set; } + public Func? S3ClientFactory { get; set; } /// /// Gets or sets the AWS region endpoint (us-east-1/us-west-1/ap-southeast-2). @@ -21,7 +21,8 @@ public interface IAWSS3BucketClientOptions public string? Region { get; set; } /// - /// Gets or sets the AWS bucket name. Cannot be . + /// Gets or sets the AWS bucket name. + /// Cannot be when is not set. /// public string BucketName { get; set; } diff --git a/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs b/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs index b6dae293..20ab9b15 100644 --- a/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs +++ b/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs @@ -24,7 +24,7 @@ public class AWSS3StorageImageProvider : IImageProvider, IDisposable /// /// The containers for the blob services. /// - private readonly Dictionary buckets + private readonly Dictionary buckets = new(); private readonly AWSS3StorageImageProviderOptions storageOptions; @@ -55,11 +55,11 @@ public AWSS3StorageImageProvider( foreach (AWSS3BucketClientOptions bucket in this.storageOptions.S3Buckets) { - AmazonS3Client s3Client = + AmazonS3BucketClient s3Client = bucket.S3ClientFactory?.Invoke(bucket, serviceProvider) ?? AmazonS3ClientFactory.CreateClient(bucket); - this.buckets.Add(bucket.BucketName, s3Client); + this.buckets.Add(s3Client.BucketName, s3Client); } } @@ -84,7 +84,7 @@ public bool IsValidRequest(HttpContext context) // the remaining path string as the key. // Path has already been correctly parsed before here. string bucketName = string.Empty; - IAmazonS3? s3Client = null; + AmazonS3Client? s3Client = null; // We want an exact match here to ensure that bucket names starting with // the same prefix are not mixed up. @@ -103,7 +103,7 @@ public bool IsValidRequest(HttpContext context) if (nameToMatch.Equals(k, StringComparison.OrdinalIgnoreCase)) { bucketName = k; - s3Client = this.buckets[k]; + s3Client = this.buckets[k].Client; break; } } @@ -190,14 +190,13 @@ private static async Task KeyExists(IAmazonS3 s3Client, string /// Releases the unmanaged resources used by the and optionally releases the managed resources. /// /// true to release both managed and unmanaged resources; false to release only unmanaged resources. - protected virtual void Dispose(bool disposing) { if (!this.isDisposed) { if (disposing) { - foreach (AmazonS3Client client in this.buckets.Values) + foreach (AmazonS3BucketClient client in this.buckets.Values) { client?.Dispose(); } diff --git a/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs b/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs index 152e2c1e..187aec7e 100644 --- a/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs +++ b/src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs @@ -1,8 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using Amazon.S3; - namespace SixLabors.ImageSharp.Web.Providers.AWS; /// @@ -22,7 +20,7 @@ public class AWSS3StorageImageProviderOptions public class AWSS3BucketClientOptions : IAWSS3BucketClientOptions { /// - public Func? S3ClientFactory { get; set; } + public Func? S3ClientFactory { get; set; } /// public string? Region { get; set; } diff --git a/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs b/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs index dee5dc9f..58ef0d87 100644 --- a/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs +++ b/tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs @@ -27,8 +27,8 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A { // Upload an image to the AWS Test Storage; AWSS3BucketClientOptions bucketOptions = options.S3Buckets.First(); - AmazonS3Client amazonS3Client = AmazonS3ClientFactory.CreateClient(bucketOptions); - ListBucketsResponse listBucketsResponse = await amazonS3Client.ListBucketsAsync(); + using AmazonS3BucketClient amazonS3Client = AmazonS3ClientFactory.CreateClient(bucketOptions); + ListBucketsResponse listBucketsResponse = await amazonS3Client.Client.ListBucketsAsync(); bool foundBucket = false; foreach (S3Bucket b in listBucketsResponse.Buckets) @@ -51,7 +51,7 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A CannedACL = S3CannedACL.PublicRead }; - await amazonS3Client.PutBucketAsync(putBucketRequest); + await amazonS3Client.Client.PutBucketAsync(putBucketRequest); } catch (AmazonS3Exception e) { @@ -76,7 +76,7 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A Key = TestConstants.ImagePath }; - await amazonS3Client.GetObjectAsync(request); + await amazonS3Client.Client.GetObjectAsync(request); } catch { @@ -105,7 +105,7 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A UseChunkEncoding = false, }; - await amazonS3Client.PutObjectAsync(putRequest); + await amazonS3Client.Client.PutObjectAsync(putRequest); } } } From ce6e4482d26f94107f39dc7c6f24d64964ca0dbb Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 1 Aug 2025 15:44:45 +1000 Subject: [PATCH 3/3] Update IAWSS3BucketClientOptions.cs --- src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs b/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs index 93b8493a..62bd79fe 100644 --- a/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs +++ b/src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs @@ -1,8 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using Amazon.S3; - namespace SixLabors.ImageSharp.Web; /// @@ -11,7 +9,7 @@ namespace SixLabors.ImageSharp.Web; public interface IAWSS3BucketClientOptions { /// - /// Gets or sets a custom a factory method to create an . + /// Gets or sets a custom factory method to create an . /// public Func? S3ClientFactory { get; set; }