microsoft
Access just the types of the handlers
Kiota opted for it's own HttpClientFactory, which is fine. But I would like to integrate with dependency injection and start using Microsoft.Extensions.Http as a source for the HttpClient. To keep all the Kiota goodies like (header inspection) I need to register all the Kiota handlers as well.
For that I only need the types of the handlers, and not the instances. Would it be possible to have an extra static method just for that? Instead of this:
https://github.com/microsoft/kiota-http-dotnet/blob/0212e818931b31a2d9e0ab4d768b31f23fe5f386/src/KiotaClientFactory.cs#L50-L62
I would like to see, as this does not initialize those classes.
public static IList<Type> DefaultHandlerTypes()
{
return new List<Type>
{
//add the default middlewares as they are ready
typeof(UriReplacementHandler<UriReplacementHandlerOption>),
typeof(RetryHandler),
typeof(RedirectHandler),
typeof(ParametersNameDecodingHandler),
typeof(UserAgentHandler),
typeof(HeadersInspectionHandler),
};
}
For other readers, this is the context this request comes from I think this is a reasonable request, especially if we collocate the new method next to the create one. From a naming convention perspective, it should be prefixed with a Get.
Is this something you'd be willing to submit a pull request for @svrooij ?
And related to this, do you ever expect the UriReplacementHandler<UriReplacementHandlerOption> to have other options @sebastienlevert? The generic is a bit in the way to make this happen. And the constructor for this one is using = default while all the others use = null. The first one doesn't play nice with dependency injection, but that could also because of the generic.
@calebkiage might be able to add more context on why the option is a generic type parameter and what scenarios he was trying to enable making it generic here. #158
@svrooj, the IUriReplacementHandlerOption is an extensibility point. For example, a user could decide to implement it and then change IsEnabled to be conditional on different circumstances. The default implementation of UriReplacementHandlerOption has a fixed value for IsEnabled for each instance passed to the constructor. It's also possible to change the Replace function's logic through a different implementation. The UriReplacementHandler is generic to allow swapping out the type of IUriReplacementHandlerOption. What problems are you having with the DI setup?
@calebkiage I'm trying to document how you can use Microsoft.Extensions.Http and dependency injection in general with Kiota. @baywet suggested that it was a bad idea to document a static list of handlers. So I tried injecting the types, from CreateDefaultHandlers() using .Select(x => x.GetType()).
https://github.com/MicrosoftDocs/openapi-docs/pull/89/files#diff-cb6707ca7707710dabba9df2adea67478eb3301978ebd9f451e539ba99a98a60R160-R178
But it does not take it, won't resolve the UriReplacementHandler<UriReplacementHandlerOption>, not sure if it has to do with the constructor having = default instead of = null or with the generic being in the way.
Without the generic type and with =null constructor, it could resolve the handler without options unless the developer explicitly added the UriReplacementHandlerOption to the DI container as well.
And when thinking about it, if a developer wants to use the UriReplacementHandler with other options type, he/she can no longer use the CreateDefaultHandlers(), method because then he/she would have to inject another handler.
We could still enable the extensibility while enabling that scenario and without shipping a breaking change simply by:
- introducing a derived handler to this one that fixes the option type
- using that new type as the default instead of the generic one
Thoughts?
I suspect the root cause here might be the UriReplacementHandlerOption not having a default constructor. The dependency graph of UriReplacementHandler would be:
UriReplacementHandler
|
UriReplacementHandlerOption
|
----------------------------------------------
| |
bool IEnumerable<KeyValuePair<string, string>>
The DI library won't know how to create an instance of UriReplacementHandlerOption because it can't resolve the constructor parameters.
I'd need to create a project to reproduce this and investigate. @svrooij, if you have a project on GitHub, I can just use that to quickly reproduce the issue and provide my suggestions.
@baywet, your idea would work, but we also need the derived handler to provide a default value for the option type
Right, we could start with the constructor and see if that fixes things by just adding another constructor
public UriReplacementHandlerOption(bool isEnabled = true):this(isEnabled, new Dictionary<string, string>())