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

Consider way to control injection for one parameter of a constructor #67

Open
YairHalberstadt opened this issue Oct 15, 2020 · 10 comments
Labels
feature-request We're considering this issue, but have not decided what to do about it yet
Milestone

Comments

@YairHalberstadt
Copy link
Owner

If a constructor has multiple parameters, one of which needs special treatment, you need to create a factory method which takes all the other parameters as dependencies and manually calls the constructor.

This means if a new parameter is added to the constructor, we have to update the factory as well.

If there was some way to modify a single parameter of a constructor, that would avoid this issue.

@YairHalberstadt YairHalberstadt added the feature-request We're considering this issue, but have not decided what to do about it yet label Oct 15, 2020
@YairHalberstadt YairHalberstadt added this to Backlog in Product Roadmap Oct 18, 2020
@YairHalberstadt
Copy link
Owner Author

We could add an attribute:

public class ParameterFactoryAttribute
{
    public ParameterFactoryAttribute(Type forType, int? parameterIndex = null, string? parameterName = null, bool useForAnyParametersOfSameTypeAsReturnType = true)
}

Then it could be used as follows:

public class MyService { public MyService(Dep1 dep, Dep2 dep2, Dep3 dep3, Dep4 dep4, string connectionString) }

[Register(typeof(MyService))]
[Register(typeof(Dep1))]
[Register(typeof(Dep2))]
[Register(typeof(Dep3))]
[Register(typeof(Dep4))]
public class Container : IContainer<MyService>
{
    [ParameterFactory(typeof(MyService))] string GetConnectionString() => LoadConnectionStringFromConfig();
}

@YairHalberstadt YairHalberstadt added this to the 2.0 milestone Dec 2, 2020
@Stroniax
Copy link

This would be particularly useful for registrations of HttpClient. Currently to have differently configured HttpClient instances one must use a factory for each consumer of HttpClient. The problem here is that the owner (Owned<T> or container) won't dispose of the HttpClient because it doesn't know about it, so the consumer type must do so. The alternative to this for my HttpClient concern is a custom factory to create and dispose of HttpClient instances, and inject that into each consumer. This is made more annoying because Microsoft's version of IHttpClientFactory is strongly coupled to their DI system because the implementation type is not public, so I would have to fully implement my own or wrap their DI.

Unless there's a better workaround for HttpClient configuration with StrongInject?

@YairHalberstadt
Copy link
Owner Author

@Stroniax I'm not sure I fully understand the issue. Would you be able to provide a code sample with what you are doing now, and explain what the issue is with that solution, and how this proposal would help? Thanks!

@Stroniax
Copy link

Stroniax commented Jan 23, 2023

https://gist.github.com/Stroniax/c6ffd55b9506d4bfb7feb4036bc895b5

The HttpClient is created in the factory method, and therefore the container will not dispose of it. With this suggestion, I would be able to use something more like this:

// Specific configuration for clients that need it
[ParameterFactory(typeof(GlobalStateHttpClient))]
public static HttpClient CreateHttpClientForGlobalStateService() => throw new NotImplementedException();

[ParameterFactory(typeof(FileContentHttpClient))]
public static HttpClient CreateHttpClientForFileContentService() => throw new NotImplementedException();

// Could also set a default for clients that don't need special configuration
[Factory]
public static HttpClient CreateDefaultHttpClient() => new();

This may be a contrived example, but it demonstrates the issue of configured parameters required for injection, especially when they are disposable.

This also allows a container to override the client for a single service without overriding configuration everywhere.

@YairHalberstadt
Copy link
Owner Author

If GlobalStateHttpClient is disposable stronginject will dispose of it. If that isn't possible you can implement IFactory<T> to control lifetime of your dependencies.

@YairHalberstadt
Copy link
Owner Author

Sorry I misunderstood. Ignore my previous comment.

@YairHalberstadt
Copy link
Owner Author

My current advice would be to make GlobalStateHttpClient disposable and have it dispose of the httpclient.

Alternatively you could make the Factory take as a parameter a Func<Config, HttpClient> as a parameter and call it to get the HttpClient. Then stronginject would manage the lifetime of the httpclient.

However I agree that this proposal would be useful. I don't actively maintain stronginject at the moment, but if you would like to contribute a solution I would be very happy to review it and provide guidance.

@jnm2
Copy link
Contributor

jnm2 commented Jan 23, 2023

The problem here is that the owner (Owned<T> or container) won't dispose of the HttpClient because it doesn't know about it, so the consumer type must do so.

HttpClient instances obtained from HttpClientFactory do not need to be disposed at all. https://stackoverflow.com/a/54326424/521757

@Janek91
Copy link

Janek91 commented Jun 1, 2023

Is this project still maintained?

@YairHalberstadt
Copy link
Owner Author

I do not currently have time to maintain this project, but I am happy to review pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request We're considering this issue, but have not decided what to do about it yet
Projects
Development

No branches or pull requests

4 participants