Skip to content
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

[FEATURE REQ] Support keyed DI with AddAzureClients #40408

Open
Arithmomaniac opened this issue Nov 30, 2023 · 11 comments
Open

[FEATURE REQ] Support keyed DI with AddAzureClients #40408

Arithmomaniac opened this issue Nov 30, 2023 · 11 comments
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions feature-request This issue requires a new behavior in the product in order be resolved.
Milestone

Comments

@Arithmomaniac
Copy link
Contributor

Library name

Microsoft.Extensions.Azure

Please describe the feature.

When registering multiple services by name (using WithName()) with .AddAzureClients, you need to resolve the intended service via DI by injecting an IAzureClientFactory<TClient> so that you can invoke .CreateClient() on the named service.

It would be great if registering a dependency by name would register the client with the new keyed DI support so that instances of TClient can be injected directly. Or if that poses compatibility issues, add a new WithKey() method to do the same.

@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions needs-team-triage Workflow: This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Nov 30, 2023
@jsquire jsquire added feature-request This issue requires a new behavior in the product in order be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that needs-team-triage Workflow: This issue needs the team to triage. labels Nov 30, 2023
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Nov 30, 2023
@jsquire jsquire added this to the Backlog milestone Nov 30, 2023
@jsquire
Copy link
Member

jsquire commented Nov 30, 2023

Thank you for your feedback, @Arithmomaniac. We've added this to our backlog for consideration.

@jsquire jsquire removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Mar 4, 2024
@jsquire
Copy link
Member

jsquire commented Mar 4, 2024

@AlexanderSher: Would you please take a look at whether it is feasible to support the new keyed DI registration without breaking changes and advise?

@annelo-msft annelo-msft added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Oct 4, 2024
Copy link

github-actions bot commented Oct 4, 2024

Hi @Arithmomaniac. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@annelo-msft
Copy link
Member

@Arithmomaniac -- thanks for your feature request! As we look further into different approaches to enabling your scenario, it would be helpful for us to have a clear picture in our heads of what you'd like to do with keyed DI registration. Would it be possible to give us a sense of the type of app that you're building around the DI container and what scenarios/features you are looking to implement that you are unable to accomplish today without keyed registration support? Many thanks in advance for your help on this!

@Arithmomaniac
Copy link
Contributor Author

Arithmomaniac commented Oct 6, 2024

I've moved on to other projects, but IIRC the use case was pretty similar to the one provided in the DI documentation. Suppose the following DI configuration:

builder.Services.AddAzureClients(clientBuilder =>
{
    clientBuilder.AddBlobServiceClient(
            builder.Configuration.GetSection("PublicStorage"));
        .WithName("PublicStorage");

    clientBuilder.AddBlobServiceClient(
            builder.Configuration.GetSection("PrivateStorage"))
        .WithName("PrivateStorage");
});

Today you have to do the following:

public class HomeController : Controller
{
    private readonly ICache _bigCache;
    private readonly BlobServiceClient _publicStorage;
    private readonly BlobServiceClient _privateStorage;

    public HomeController(
        [FromKeyedServices("big")] ICache bigCache,
        IAzureClientFactory<BlobServiceClient> clientFactory)
    {
        _bigCache = bigCache;
        _publicStorage = clientFactory.CreateClient("PublicStorage");
        _privateStorage = clientFactory.CreateClient("PrivateStorage");
    }
}

I'd like to be able to do the following:

public class HomeController(
  [FromKeyedServices("big")] ICache bigCache,
  [FromKeyedServices("PublicStorage")] BlobServiceClient publicStorage,
  [FromKeyedServices("PrivateStorage")] BlobServiceClient privateStorage
) : Controller
{
}

Advantages:

  • Makes it easier to pass in test doubles during testing - don't need to mock a whole client factory to return two concrete instances
  • Makes it easier to use primary constructors (as shown)
  • Unified, idiomatic interface for "keying" Azure and non-Azure services

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Oct 6, 2024
@annelo-msft
Copy link
Member

Thank you so much for the explanation, @Arithmomaniac!

Just to confirm that I am understanding correctly -- Microsoft.Extensions.Azure APIs strictly-speaking support the scenario you're interested in (adding multiple instances of a single client type to the DI service collection) via the WithName extension method. But, if we were to better integrate it with the new-in-.NET8 keyed services feature, it would have the advantages you listed as a result of better ecosystem integration. Specifically, you could then make one or more keyed services (e.g. Azure.Storage.Blobs.BlobServiceClient instances) parameters to an ASP.NET controller, which would retrieve the client(s) from the DI service collection rather than requiring you to create them new via the call to the Azure client factory.

Is that correct?

@annelo-msft annelo-msft added needs-author-feedback Workflow: More information is needed from author to address the issue. and removed needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels Oct 7, 2024
Copy link

github-actions bot commented Oct 7, 2024

Hi @Arithmomaniac. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@Arithmomaniac
Copy link
Contributor Author

Precisely

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Oct 8, 2024
@annelo-msft
Copy link
Member

annelo-msft commented Oct 22, 2024

Looking at this as part of #46671

@annelo-msft annelo-msft removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Oct 22, 2024
@abatishchev
Copy link
Contributor

+1 for supporting keyed service by AddAzureClients et al

@Bouke
Copy link

Bouke commented Jan 30, 2025

While waiting for official support, here's how you can achieve the same. Instead of calling WithName, you'd call WithKey.

public static class AzureClientBuilderExtensions
{
    public static IAzureClientBuilder<TClient, TOptions> WithKey<TClient, TOptions>(
        this IAzureClientBuilder<TClient, TOptions> builder, string serviceKey)
        where TOptions : class where TClient : class
    {
        builder.WithName(serviceKey);

        var services = (IServiceCollection)builder.GetType().GetProperty("ServiceCollection")!.GetValue(builder)!;
        services.AddKeyedSingleton<TClient>(serviceKey,
            (provider, _) => provider.GetRequiredService<IAzureClientFactory<TClient>>().CreateClient(serviceKey));

        return builder;
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions feature-request This issue requires a new behavior in the product in order be resolved.
Projects
None yet
Development

No branches or pull requests

6 participants