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

Provide Dependency injection support #1882

Open
Bchir opened this issue Dec 16, 2019 · 16 comments
Open

Provide Dependency injection support #1882

Bchir opened this issue Dec 16, 2019 · 16 comments
Labels

Comments

@Bchir
Copy link

Bchir commented Dec 16, 2019

As a developer, I want to have an extension to IServiceCollection (Extensions.DependencyInjection.Abstractions) so that I can register all stripe services and inject them directly
Expected Method

services.AddStripe(ApiKey);

Notice that for HTTP client it is recommended to use services.AddHttpClient

@DanDepot
Copy link

Agreed, this would be a nice feature for .NET core applications.

@ob-stripe
Copy link
Contributor

Hi both. This seems like it would be an interesting addition to Stripe.net. I can't commit to an ETA, but I'll tag this as future for now and we will try to add support for this in a future version.

@Bchir Bchir changed the title Provide Dependenct injection support Provide Dependency injection support Dec 17, 2019
@clement911
Copy link
Contributor

One thing that would need to be sorted is how to inject services when different api keys are used.
Probably a good model is how HttpClient does it with named clients and a factory.

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-3.1#named-clients

@Bchir
Copy link
Author

Bchir commented Dec 19, 2019

@clement911 maybe for configuration as

services.AddStripeAccount(configuration, name);

and then two choices :

  • use Strategy Pattern in services to select the account / API key you want to use by passing the name that you defined (providing a way to provide a name for a config would be better for developers so that they can use business-related names, so more readability )

OR

  • if you are using a scoped lifetimes for the client so, at one request, any stripe service will use the same instance, that way you can add service for the client setup that would configure the client to use at that scope. It would mean that for any scope you have to start with
_stripeClientService.UseAccount(name);

@Simonl9l
Copy link

Simonl9l commented Sep 2, 2020

It's also important to ensure that one can control DI scopes. For example Blazor Server apps need services per session/circuit to be transient scoped, this also applied the the underlying HttpClient or one is likely to have severe issues.

@Bchir
Copy link
Author

Bchir commented Sep 14, 2020

@Simonl9l it is possible to use ServiceDescriptor Describe method to implement this , and the user will just have to pass the scope from the ServiceLifetime enum

@Simonl9l
Copy link

Simonl9l commented Mar 4, 2021

@Bchir I create a wrapper service that wraps the Stripe Client creating a new one each time, an hence its underlying http client, so each blazer session has its own stripe client Http connection.

It would be better if there was an option to leverage the underlying HttpClient Factory, especially since DotNet 5 is now in production and DotNet Framework support per the current implementation will at some point sunset.

@spencer741
Copy link

Currently I am doing it myself... sort of.

Can't attribute to what's going on under the hood with HttpClient in this library...

I assume HttpClientFactory is not being used in stripe-dotnet underneath... as such, the HttpClient used would be re-instantiated on every incoming request... but in reality, it was doing that anyway when I would instantiate a Stripe Service within a controller.

So, I guess you could say my code base is ready when DI is officially supported. Until then, when I use a stripe service, I just add it as transient to my extensions... not like it's doing anything different other than instantiating the service for me... haven't rigorously tested either.

using Stripe;

...

public static class BusinessServiceCollectionExtensions
{
    public static void AddStripeServices(this IServiceCollection services)
    {
        //https://github.com/stripe/stripe-dotnet/issues/1882
        services.AddTransient<CustomerService>();
        services.AddTransient<SubscriptionService>();
        services.AddTransient<InvoiceService>();
        services.AddTransient<PriceService>();
    }
}

Then in my Startup.cs, I just set the API key from appsettings.json, since the StripeConfiguration class is static.

StripeConfiguration.ApiKey = Configuration["Stripe:ApiKey"];

Makes it easy... only using one api key at the moment.

@cecilphillip-stripe
Copy link

@spencer741 I've been doing something similar.

Curious... Do you think all stripe services should be registered in the container?

@jaymedavis
Copy link
Contributor

fwiw, I don't think this should be implemented. Interfaces for services belong in business logic, not as part of this library. You may not need all functions in a service in your application. Therefore create an interface in your business logic (with all your other interfaces) and wire it up that way.

My $0.02 :)

@AraHaan
Copy link
Contributor

AraHaan commented Apr 4, 2023

I think this would be great for the following targets only:

The following can be done for the below targets just by using netstandard2.1 as a target:

  • net5.0 (out of support)
  • net6.0 (LTS)
  • net7.0 (limited support)
  • net8.0 (in development - LTS)

While the .NET Framework target (aka netstandard2.0) would need to stay the same as the Microsoft.AspNet.* packages are made specifically for .NET Framework which means no dependency injection support unless you manage to get AspNetCore to fully work in .NET Framework out of chance (yes, many colleges use framework and so that means no AspNetCore and likewise no DI and some of them might want to in the future require their students to learn how to use stripe's api (I feel like doing it should be a requirement as many companies love it when someone knows how to properly maintain their payment integrations)).

@drewid
Copy link

drewid commented Nov 12, 2024

What's the current recommended status on DI and stripe-dotnet? It's unclear from the comments and info online.

@Simonl9l
Copy link

@drewid as the years go by…

@jaymedavis
Copy link
Contributor

Nothing is preventing you from doing DI now. You can pass your dependencies via properties, methods, or constructors. DI is just a strategy you implement in your code by not hard coding other implementations in your classes.

If you're referring to interfaces, they were never designed to be a one to one match with every class in every library. Imo, that really overshadows the real use of interfaces, and leads to a lack of understanding why and when they should be used.

Your contract for your application, should be in your business logic, not in a third party library.

With that said, If you don't mind all the double work of defining everything twice for every little thing, go for it. Most of the lib is generated it seems now, so it's no where near the maintenance annoyance it used to be.

@Simonl9l
Copy link

@jaymedavis I’d suggest the primary need for DI support is totally related to the HttpClient and use of the HttpClientFactory as dotnet best practice.

@jaymedavis
Copy link
Contributor

You can set your own HttpClient here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants