From 1dd82185a77fe712ef6fb094d9ae42e06f190e96 Mon Sep 17 00:00:00 2001 From: Will Sugarman Date: Mon, 13 Sep 2021 11:35:44 -0700 Subject: [PATCH] Update Blob Service Client Registration APIs and Configuration (#1001) - Update blob service client registration for the following reasons: 1. Integrate blob client and underlying http request pipeline with existing DI service container 2. Reduce boilerplate 3. Streamline configuration whose types integrate with existing APIs - Normalize JSON to 2 spaces for indent - Bump version of Azurite image such that we can leverage latest blob client API --- Directory.Build.props | 2 +- docker/docker-compose.dependencies.yml | 2 +- samples/docker/docker-compose.yaml | 2 +- .../Features/Storage/BlobFileStore.cs | 17 +- .../Microsoft.Health.Dicom.Blob.csproj | 1 + ...ServerBuilderBlobRegistrationExtensions.cs | 37 ++-- ...erBuilderMetadataRegistrationExtensions.cs | 35 ++-- .../appsettings.json | 173 ++++++++++-------- .../Persistence/DataStoreTestsFixture.cs | 2 +- 9 files changed, 127 insertions(+), 144 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 6a764a8a4b..70fc1ffd44 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -10,7 +10,7 @@ true 4.0.8 true - 3.1.23 + 3.2.1 true Latest MIT diff --git a/docker/docker-compose.dependencies.yml b/docker/docker-compose.dependencies.yml index 7d97551bc4..bea1eea537 100644 --- a/docker/docker-compose.dependencies.yml +++ b/docker/docker-compose.dependencies.yml @@ -12,7 +12,7 @@ services: - sql - azurite azurite: - image: mcr.microsoft.com/azure-storage/azurite:3.13.0@sha256:349f106b240c5fd5ecb146c3aa121c531c8a1080badb762099d886703bc39fa0 + image: mcr.microsoft.com/azure-storage/azurite:3.14.2@sha256:443c9b28466219e21005e638a28ac4bcc6d3860509d3fc9ec8753fce64424b29 # # These port bindings [source]:[dest] can be uncommented to connect to the storage emulator via Microsoft Azure Storage Explorer # # Note that the source ports may need to change if a storage emulator is already running on localhost # ports: diff --git a/samples/docker/docker-compose.yaml b/samples/docker/docker-compose.yaml index 28407d8d7f..c2837c446b 100644 --- a/samples/docker/docker-compose.yaml +++ b/samples/docker/docker-compose.yaml @@ -19,7 +19,7 @@ services: - azurite azurite: - image: mcr.microsoft.com/azure-storage/azurite:3.13.0@sha256:349f106b240c5fd5ecb146c3aa121c531c8a1080badb762099d886703bc39fa0 + image: mcr.microsoft.com/azure-storage/azurite:3.14.2@sha256:443c9b28466219e21005e638a28ac4bcc6d3860509d3fc9ec8753fce64424b29 restart: always # # Uncomment if you want to expose azure storage explorer against localhost # ports: diff --git a/src/Microsoft.Health.Dicom.Blob/Features/Storage/BlobFileStore.cs b/src/Microsoft.Health.Dicom.Blob/Features/Storage/BlobFileStore.cs index c0b5065f2c..61efdd9cbd 100644 --- a/src/Microsoft.Health.Dicom.Blob/Features/Storage/BlobFileStore.cs +++ b/src/Microsoft.Health.Dicom.Blob/Features/Storage/BlobFileStore.cs @@ -8,7 +8,6 @@ using System.Threading; using System.Threading.Tasks; using Azure; -using Azure.Storage; using Azure.Storage.Blobs; using Azure.Storage.Blobs.Models; using Azure.Storage.Blobs.Specialized; @@ -27,21 +26,21 @@ namespace Microsoft.Health.Dicom.Blob.Features.Storage public class BlobFileStore : IFileStore { private readonly BlobContainerClient _container; - private readonly BlobDataStoreConfiguration _blobDataStoreConfiguration; + private readonly BlobOperationOptions _options; public BlobFileStore( BlobServiceClient client, IOptionsMonitor namedBlobContainerConfigurationAccessor, - BlobDataStoreConfiguration blobDataStoreConfiguration) + IOptions options) { EnsureArg.IsNotNull(client, nameof(client)); EnsureArg.IsNotNull(namedBlobContainerConfigurationAccessor, nameof(namedBlobContainerConfigurationAccessor)); - EnsureArg.IsNotNull(blobDataStoreConfiguration, nameof(blobDataStoreConfiguration)); + EnsureArg.IsNotNull(options?.Value, nameof(options)); BlobContainerConfiguration containerConfiguration = namedBlobContainerConfigurationAccessor.Get(Constants.ContainerConfigurationName); _container = client.GetBlobContainerClient(containerConfiguration.ContainerName); - _blobDataStoreConfiguration = blobDataStoreConfiguration; + _options = options.Value; } /// @@ -56,13 +55,7 @@ public async Task StoreFileAsync( BlockBlobClient blob = GetInstanceBlockBlob(versionedInstanceIdentifier); stream.Seek(0, SeekOrigin.Begin); - var blobUploadOptions = new BlobUploadOptions() - { - TransferOptions = new StorageTransferOptions - { - MaximumConcurrency = _blobDataStoreConfiguration.RequestOptions.UploadMaximumConcurrency, - }, - }; + var blobUploadOptions = new BlobUploadOptions { TransferOptions = _options.Upload }; try { diff --git a/src/Microsoft.Health.Dicom.Blob/Microsoft.Health.Dicom.Blob.csproj b/src/Microsoft.Health.Dicom.Blob/Microsoft.Health.Dicom.Blob.csproj index 7f355db5a0..efe4de2dee 100644 --- a/src/Microsoft.Health.Dicom.Blob/Microsoft.Health.Dicom.Blob.csproj +++ b/src/Microsoft.Health.Dicom.Blob/Microsoft.Health.Dicom.Blob.csproj @@ -17,6 +17,7 @@ + diff --git a/src/Microsoft.Health.Dicom.Blob/Registration/DicomServerBuilderBlobRegistrationExtensions.cs b/src/Microsoft.Health.Dicom.Blob/Registration/DicomServerBuilderBlobRegistrationExtensions.cs index 48ef83d8dc..a82065a93d 100644 --- a/src/Microsoft.Health.Dicom.Blob/Registration/DicomServerBuilderBlobRegistrationExtensions.cs +++ b/src/Microsoft.Health.Dicom.Blob/Registration/DicomServerBuilderBlobRegistrationExtensions.cs @@ -5,10 +5,7 @@ using EnsureThat; using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; using Microsoft.Health.Blob.Configs; -using Microsoft.Health.Blob.Features.Storage; using Microsoft.Health.Dicom.Blob; using Microsoft.Health.Dicom.Blob.Features.Health; using Microsoft.Health.Dicom.Blob.Features.Storage; @@ -34,20 +31,27 @@ public static IDicomServerBuilder AddBlobStorageDataStore(this IDicomServerBuild EnsureArg.IsNotNull(configuration, nameof(configuration)); return serverBuilder - .AddBlobPersistence(configuration) - .AddBlobHealthCheck(); + .AddBlobPersistence(configuration) + .AddBlobHealthCheck(); } private static IDicomServerBuilder AddBlobPersistence(this IDicomServerBuilder serverBuilder, IConfiguration configuration) { IServiceCollection services = serverBuilder.Services; - services.AddBlobDataStore(); + IConfiguration blobConfig = configuration.GetSection(BlobServiceClientOptions.DefaultSectionName); + services + .AddBlobServiceClient(blobConfig) + .AddBlobContainerInitialization(x => blobConfig + .GetSection(BlobInitializerOptions.DefaultSectionName) + .Bind(x)) + .ConfigureContainer(Constants.ContainerConfigurationName, x => configuration + .GetSection(DicomServerBlobConfigurationSectionName) + .Bind(x)); - services.Configure( - Constants.ContainerConfigurationName, - containerConfiguration => configuration.GetSection(DicomServerBlobConfigurationSectionName) - .Bind(containerConfiguration)); + services + .AddOptions() + .Bind(blobConfig.GetSection(nameof(BlobServiceClientOptions.Operations))); services.Add() .Scoped() @@ -59,19 +63,6 @@ private static IDicomServerBuilder AddBlobPersistence(this IDicomServerBuilder s // so we need to register here. Need to some more investigation to see how we might be able to do this. services.Decorate(); - services.Add(sp => - { - ILoggerFactory loggerFactory = sp.GetService(); - IOptionsMonitor namedBlobContainerConfiguration = sp.GetService>(); - BlobContainerConfiguration blobContainerConfiguration = namedBlobContainerConfiguration.Get(Constants.ContainerConfigurationName); - - return new BlobContainerInitializer( - blobContainerConfiguration.ContainerName, - loggerFactory.CreateLogger()); - }) - .Singleton() - .AsService(); - return serverBuilder; } diff --git a/src/Microsoft.Health.Dicom.Metadata/Registration/DicomServerBuilderMetadataRegistrationExtensions.cs b/src/Microsoft.Health.Dicom.Metadata/Registration/DicomServerBuilderMetadataRegistrationExtensions.cs index 3da95a532c..505ea39b59 100644 --- a/src/Microsoft.Health.Dicom.Metadata/Registration/DicomServerBuilderMetadataRegistrationExtensions.cs +++ b/src/Microsoft.Health.Dicom.Metadata/Registration/DicomServerBuilderMetadataRegistrationExtensions.cs @@ -5,10 +5,7 @@ using EnsureThat; using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; using Microsoft.Health.Blob.Configs; -using Microsoft.Health.Blob.Features.Storage; using Microsoft.Health.Dicom.Core.Features.Common; using Microsoft.Health.Dicom.Core.Registration; using Microsoft.Health.Dicom.Metadata; @@ -34,33 +31,23 @@ public static IDicomServerBuilder AddMetadataStorageDataStore(this IDicomServerB EnsureArg.IsNotNull(configuration, nameof(configuration)); return serverBuilder - .AddMetadataPersistence(configuration) - .AddMetadataHealthCheck(); + .AddMetadataPersistence(configuration) + .AddMetadataHealthCheck(); } private static IDicomServerBuilder AddMetadataPersistence(this IDicomServerBuilder serverBuilder, IConfiguration configuration) { IServiceCollection services = serverBuilder.Services; - services.AddBlobDataStore(); - - services.Configure( - Constants.ContainerConfigurationName, - containerConfiguration => configuration.GetSection(DicomServerBlobConfigurationSectionName) - .Bind(containerConfiguration)); - - services.Add(sp => - { - ILoggerFactory loggerFactory = sp.GetService(); - IOptionsMonitor namedBlobContainerConfiguration = sp.GetService>(); - BlobContainerConfiguration blobContainerConfiguration = namedBlobContainerConfiguration.Get(Constants.ContainerConfigurationName); - - return new BlobContainerInitializer( - blobContainerConfiguration.ContainerName, - loggerFactory.CreateLogger()); - }) - .Singleton() - .AsService(); + IConfiguration blobConfig = configuration.GetSection(BlobServiceClientOptions.DefaultSectionName); + services + .AddBlobServiceClient(blobConfig) + .AddBlobContainerInitialization(x => blobConfig + .GetSection(BlobInitializerOptions.DefaultSectionName) + .Bind(x)) + .ConfigureContainer(Constants.ContainerConfigurationName, x => configuration + .GetSection(DicomServerBlobConfigurationSectionName) + .Bind(x)); services.Add() .Scoped() diff --git a/src/Microsoft.Health.Dicom.Web/appsettings.json b/src/Microsoft.Health.Dicom.Web/appsettings.json index 3a001d03ac..19b1b8bba4 100644 --- a/src/Microsoft.Health.Dicom.Web/appsettings.json +++ b/src/Microsoft.Health.Dicom.Web/appsettings.json @@ -1,90 +1,101 @@ { - "AllowedHosts": "*", - "DicomServer": { - "Security": { - "Enabled": false, - "Authentication": { - "Audience": null, - "Authority": null - } - }, - "Features": { - "EnableOhifViewer": false, - "EnableFullDicomItemValidation": false, - "EnableExtendedQueryTags": false - }, - "Services": { - "DeletedInstanceCleanup": { - "DeleteDelay": "3.00:00:00", - "MaxRetries": 5, - "RetryBackOff": "1.00:00:00", - "PollingInterval": "00:03:00", - "BatchSize": 10 - }, - "StoreServiceSettings": { - "MaxAllowedDicomFileSize": 2147483647 - } - }, - "Audit": { - "CustomAuditHeaderPrefix": "X-MS-AZUREDICOM-AUDIT-" - }, - "ServerIdentity": { - "UserAssignedAppId": null - }, - "Swagger": { - "License": { - "Name": "MIT License", - "Url": "https://github.com/microsoft/dicom-server/blob/main/LICENSE" - } - } + "AllowedHosts": "*", + "DicomServer": { + "Security": { + "Enabled": false, + "Authentication": { + "Audience": null, + "Authority": null + } }, - "Logging": { - "IncludeScopes": false, - "LogLevel": { - "Default": "Warning" - }, - "ApplicationInsights": { - "LogLevel": { - "Default": "Information", - "Microsoft.Health": "Information", - "Microsoft": "Warning", - "System": "Warning" - } - } + "Features": { + "EnableOhifViewer": false, + "EnableFullDicomItemValidation": false, + "EnableExtendedQueryTags": false + }, + "Services": { + "DeletedInstanceCleanup": { + "DeleteDelay": "3.00:00:00", + "MaxRetries": 5, + "RetryBackOff": "1.00:00:00", + "PollingInterval": "00:03:00", + "BatchSize": 10 + }, + "StoreServiceSettings": { + "MaxAllowedDicomFileSize": 2147483647 + } + }, + "Audit": { + "CustomAuditHeaderPrefix": "X-MS-AZUREDICOM-AUDIT-" + }, + "ServerIdentity": { + "UserAssignedAppId": null + }, + "Swagger": { + "License": { + "Name": "MIT License", + "Url": "https://github.com/microsoft/dicom-server/blob/main/LICENSE" + } + } + }, + "Logging": { + "IncludeScopes": false, + "LogLevel": { + "Default": "Warning" }, "ApplicationInsights": { - "InstrumentationKey": "" + "LogLevel": { + "Default": "Information", + "Microsoft.Health": "Information", + "Microsoft": "Warning", + "System": "Warning" + } + } + }, + "ApplicationInsights": { + "InstrumentationKey": "" + }, + "DicomWeb": { + "DicomStore": { + "ContainerName": "dicomwebcontainer" + }, + "MetadataStore": { + "ContainerName": "metadatacontainer" + } + }, + "BlobStore": { + "ConnectionString": null, + "Initialization": { + "RetryDelay": "00:00:15", + "Timeout": "00:06:00" }, - "DicomWeb": { - "DicomStore": { - "ContainerName": "dicomwebcontainer" - }, - "MetadataStore": { - "ContainerName": "metadatacontainer" - } + "Operations": { + "Download": { + "MaximumConcurrency": 5 + }, + "Upload": { + "MaximumConcurrency": 5 + } }, - "BlobStore": { - "ConnectionString": null, - "RequestOptions": { - "ExponentialRetryBackoffDeltaInSeconds": 4, - "ExponentialRetryMaxAttempts": 6, - "ServerTimeoutInMinutes": 2, - "DownloadMaximumConcurrency": 5, - "UploadMaximumConcurrency": 5 - } + "Retry": { + "Delay": "00:00:04", + "MaxRetries": 6, + "Mode": "Exponential", + "NetworkTimeout": "00:02:00" + } + }, + "SqlServer": { + "Initialize": "true", + "AllowDatabaseCreation": "true", + "ConnectionString": "server=(local);Initial Catalog=Dicom;Integrated Security=true", + "TransientFaultRetryPolicy": { + "InitialDelay": "00:00:00.100", + "RetryCount": 3, + "Factor": 2, + "FastFirst": true }, - "SqlServer": { - "Initialize": "true", - "AllowDatabaseCreation": "true", - "ConnectionString": "server=(local);Initial Catalog=Dicom;Integrated Security=true", - "TransientFaultRetryPolicy": { - "InitialDelay": "00:00:00.100", - "RetryCount": 3, - "Factor": 2, - "FastFirst": true - }, - "SchemaOptions": { - "AutomaticUpdatesEnabled": true - } + "SchemaOptions": { + "AutomaticUpdatesEnabled": true } + } } diff --git a/test/Microsoft.Health.Dicom.Tests.Integration/Persistence/DataStoreTestsFixture.cs b/test/Microsoft.Health.Dicom.Tests.Integration/Persistence/DataStoreTestsFixture.cs index 643eeb6c0a..bd65c86c4b 100644 --- a/test/Microsoft.Health.Dicom.Tests.Integration/Persistence/DataStoreTestsFixture.cs +++ b/test/Microsoft.Health.Dicom.Tests.Integration/Persistence/DataStoreTestsFixture.cs @@ -69,7 +69,7 @@ await blobClientInitializer.InitializeDataStoreAsync( var jsonSerializer = new JsonSerializer(); jsonSerializer.Converters.Add(new JsonDicomConverter()); - FileStore = new BlobFileStore(_blobClient, optionsMonitor, Substitute.For()); + FileStore = new BlobFileStore(_blobClient, optionsMonitor, Options.Create(Substitute.For())); MetadataStore = new BlobMetadataStore(_blobClient, jsonSerializer, optionsMonitor, RecyclableMemoryStreamManager); }