-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Added BlobFileStoreFactory to allow DefaultAzureCredential or Connectionstring authentication for Azure Blob storage #12874
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,17 @@ | ||
using Microsoft.Extensions.DependencyInjection; | ||
|
||
namespace OrchardCore.FileStorage.AzureBlob; | ||
public static class AzureBlobFileStorageOrchardCoreBuilderExtensions | ||
{ | ||
/// <summary> | ||
/// This registers the AzureBlobFileStorage components. | ||
/// Note: this method is safe to call more then once. | ||
/// </summary> | ||
/// <param name="builder"></param> | ||
/// <returns></returns> | ||
public static OrchardCoreBuilder AddAzureBlobFileStorage(this OrchardCoreBuilder builder) | ||
{ | ||
builder.ApplicationServices.AddAzureBlobFileStorage(); | ||
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. new line before return. |
||
return builder; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.Extensions.DependencyInjection.Extensions; | ||
|
||
namespace OrchardCore.FileStorage.AzureBlob; | ||
|
||
public static class AzureBlobFileStorageServiceCollectionExtensions | ||
{ | ||
/// <summary> | ||
/// Registers the BlobFileStorage services in the ServiceCollection. | ||
/// Note: this method can be called multiple times | ||
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. I would remove the comments here. |
||
/// </summary> | ||
/// <param name="services"></param> | ||
/// <returns></returns> | ||
public static IServiceCollection AddAzureBlobFileStorage(this IServiceCollection services) | ||
{ | ||
// always use TryXXX methods because this method can be called multiple times | ||
// by different modules (currently: Azure Media and Azure Shells module) | ||
services.TryAddSingleton<BlobFileStoreFactory>(); | ||
return services; | ||
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. New line before Then either remove the comment or change the comment block to something like this // TryAddSingleton is used to prevent registering the service again. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,63 @@ | ||||||
using System; | ||||||
using Azure.Storage.Blobs; | ||||||
using Microsoft.AspNetCore.StaticFiles; | ||||||
using Microsoft.Extensions.Azure; | ||||||
using Microsoft.Extensions.DependencyInjection; | ||||||
using OrchardCore.Modules; | ||||||
|
||||||
namespace OrchardCore.FileStorage.AzureBlob; | ||||||
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. Add new line after the namespace. |
||||||
public class BlobFileStoreFactory | ||||||
{ | ||||||
private readonly IAzureClientFactory<BlobServiceClient> _clientFactory; | ||||||
private readonly IServiceProvider _serviceProvider; | ||||||
|
||||||
private const string DefaultStorageName = "Default"; | ||||||
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. rename the variable to |
||||||
|
||||||
public BlobFileStoreFactory(IAzureClientFactory<BlobServiceClient> clientFactory, IServiceProvider serviceProvider) | ||||||
{ | ||||||
_clientFactory = clientFactory; | ||||||
_serviceProvider = serviceProvider; | ||||||
} | ||||||
|
||||||
/// <summary> | ||||||
/// Creates a <see cref="BlobFileStore"/> from <see cref="BlobStorageOptions"/> | ||||||
/// If a connectionstring is specified, it will use the connectionstring | ||||||
/// otherwise it will try to get the BlobServiceClient configured with the specified name in the BlobServiceName property | ||||||
/// </summary> | ||||||
/// <param name="options"></param> | ||||||
/// <returns></returns> | ||||||
public virtual BlobFileStore Create(BlobStorageOptions options) | ||||||
{ | ||||||
var containerClient = CreateContainerClientFromOptions(options); | ||||||
|
||||||
return new BlobFileStore( | ||||||
containerClient, | ||||||
options.BasePath, | ||||||
_serviceProvider.GetRequiredService<IClock>(), | ||||||
_serviceProvider.GetRequiredService<IContentTypeProvider>()); | ||||||
} | ||||||
|
||||||
|
||||||
private BlobContainerClient CreateContainerClientFromOptions(BlobStorageOptions options) | ||||||
{ | ||||||
// return a container client from connectionstring, if specified | ||||||
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.
Suggested change
|
||||||
if (!String.IsNullOrEmpty(options.ConnectionString)) | ||||||
{ | ||||||
return new BlobContainerClient(options.ConnectionString, options.ContainerName); | ||||||
} | ||||||
|
||||||
// else use the clientFactory to lookup a BlobService with the specified name | ||||||
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.
Suggested change
|
||||||
if (!String.IsNullOrEmpty(options.BlobServiceName)) | ||||||
{ | ||||||
return _clientFactory | ||||||
.CreateClient(options.BlobServiceName) | ||||||
.GetBlobContainerClient(options.ContainerName); | ||||||
} | ||||||
|
||||||
// fall-back to the default registered blob storage | ||||||
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.
Suggested change
|
||||||
return _clientFactory | ||||||
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. Why do we want to have a fall back option instead of logging an error? Maybe we should do option validation before? @jtkech, thoughts? |
||||||
.CreateClient(DefaultStorageName) | ||||||
.GetBlobContainerClient(options.ContainerName); | ||||||
} | ||||||
|
||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
using Azure.Core; | ||
|
||
namespace OrchardCore.FileStorage.AzureBlob | ||
{ | ||
public abstract class BlobStorageOptions | ||
|
@@ -7,6 +9,13 @@ public abstract class BlobStorageOptions | |
/// </summary> | ||
public string ConnectionString { get; set; } | ||
|
||
/// <summary> | ||
/// The reference to a named AzureClient. | ||
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. Simplify the comments here by writing something like this |
||
/// The client needs to be registered using the AddAzureClients extension method. | ||
/// more info: https://learn.microsoft.com/en-us/dotnet/azure/sdk/dependency-injection#configure-multiple-service-clients-with-different-names | ||
/// </summary> | ||
public string BlobServiceName { get; set; } | ||
|
||
/// <summary> | ||
/// The Azure Blob container name. | ||
/// </summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,9 @@ public static OrchardCoreBuilder AddAzureShellsConfiguration(this OrchardCoreBui | |
|
||
services.TryAddSingleton<IContentTypeProvider, FileExtensionContentTypeProvider>(); | ||
|
||
// register the azure blob file storage services | ||
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. I would just remove this comment. |
||
builder.AddAzureBlobFileStorage(); | ||
|
||
services.AddSingleton<IShellsFileStore>(sp => | ||
{ | ||
var configuration = sp.GetRequiredService<IConfiguration>(); | ||
|
@@ -34,10 +37,9 @@ public static OrchardCoreBuilder AddAzureShellsConfiguration(this OrchardCoreBui | |
"The 'OrchardCore.Shells.Azure' configuration section must be defined"); | ||
} | ||
|
||
var clock = sp.GetRequiredService<IClock>(); | ||
var contentTypeProvider = sp.GetRequiredService<IContentTypeProvider>(); | ||
var blobFileStoreFactory = sp.GetRequiredService<BlobFileStoreFactory>(); | ||
|
||
var fileStore = new BlobFileStore(blobOptions, clock, contentTypeProvider); | ||
var fileStore = blobFileStoreFactory.Create(blobOptions); | ||
|
||
return new BlobShellsFileStore(fileStore); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment isn't needed here