Access just the types of the handlers

[svrooij]

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), 
     }; 
 } 

[baywet]

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 ?

[sebastienlevert]

Love this idea!

[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.

[baywet]

@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

[calebkiage]

@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?

[svrooij]

@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.

[baywet]

We could still enable the extensibility while enabling that scenario and without shipping a breaking change simply by:

1. introducing a derived handler to this one that fixes the option type
2. using that new type as the default instead of the generic one

Thoughts?

[calebkiage]

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

[baywet]

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>())