-
-
Notifications
You must be signed in to change notification settings - Fork 103
Remove nullable disable from aws #312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
65fb7bb
8ff6fd2
737eb59
b7e03f6
5341d87
a61e19d
7ee9e16
bbf7f47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
#nullable disable | ||
|
||
using System.Globalization; | ||
using Amazon.S3; | ||
|
@@ -17,7 +16,7 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS; | |
public class AWSS3StorageCache : IImageCache | ||
{ | ||
private readonly IAmazonS3 amazonS3Client; | ||
private readonly string bucket; | ||
private readonly string? bucket; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should do a guard check for the bucket name and make this non-nullable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is non nullable. But to achieve this we have the ugly hack with = null! in the options :( |
||
|
||
/// <summary> | ||
/// Initializes a new instance of the <see cref="AWSS3StorageCache"/> class. | ||
|
@@ -32,14 +31,14 @@ public AWSS3StorageCache(IOptions<AWSS3StorageCacheOptions> cacheOptions) | |
} | ||
|
||
/// <inheritdoc/> | ||
public async Task<IImageCacheResolver> GetAsync(string key) | ||
public async Task<IImageCacheResolver?> GetAsync(string key) | ||
{ | ||
GetObjectMetadataRequest request = new() { BucketName = this.bucket, Key = key }; | ||
try | ||
{ | ||
// HEAD request throws a 404 if not found. | ||
MetadataCollection metadata = (await this.amazonS3Client.GetObjectMetadataAsync(request)).Metadata; | ||
return new AWSS3StorageCacheResolver(this.amazonS3Client, this.bucket, key, metadata); | ||
return new AWSS3StorageCacheResolver(this.amazonS3Client, this.bucket!, key, metadata); | ||
} | ||
catch | ||
{ | ||
|
@@ -82,12 +81,12 @@ public Task SetAsync(string key, Stream stream, ImageCacheMetadata metadata) | |
/// 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( | ||
public static PutBucketResponse? CreateIfNotExists( | ||
AWSS3StorageCacheOptions options, | ||
S3CannedACL acl) | ||
=> AsyncHelper.RunSync(() => CreateIfNotExistsAsync(options, acl)); | ||
|
||
private static async Task<PutBucketResponse> CreateIfNotExistsAsync( | ||
private static async Task<PutBucketResponse?> CreateIfNotExistsAsync( | ||
AWSS3StorageCacheOptions options, | ||
S3CannedACL acl) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
#nullable disable | ||
|
||
namespace SixLabors.ImageSharp.Web.Providers.AWS; | ||
|
||
|
@@ -21,19 +20,19 @@ public class AWSS3StorageImageProviderOptions | |
public class AWSS3BucketClientOptions : IAWSS3BucketClientOptions | ||
{ | ||
/// <inheritdoc/> | ||
public string Region { get; set; } | ||
public string? Region { get; set; } | ||
|
||
/// <inheritdoc/> | ||
public string BucketName { get; set; } | ||
public string BucketName { get; set; } = null!; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we should switch to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes no difference. Init does not mean that he must set it. It just says that it can not be changed after constructing. For your desired behavior we need the required keyword and that is net7... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, you're right (we keep having this conversation 🤣) I do thoink for .NET 6 though, options-types should use this pattern to enforce immutability. |
||
|
||
/// <inheritdoc/> | ||
public string AccessKey { get; set; } | ||
public string? AccessKey { get; set; } | ||
|
||
/// <inheritdoc/> | ||
public string AccessSecret { get; set; } | ||
public string? AccessSecret { get; set; } | ||
|
||
/// <inheritdoc/> | ||
public string Endpoint { get; set; } | ||
public string? Endpoint { get; set; } | ||
|
||
/// <inheritdoc/> | ||
public bool UseAccelerateEndpoint { get; set; } | ||
|
Uh oh!
There was an error while loading. Please reload this page.