unosquare / embedio

A tiny, cross-platform, module based web server for .NET
http://unosquare.github.io/embedio
Other
1.46k stars 176 forks source link

Cannot resolve web API controller with IoC container #507

Closed steve-bate closed 3 years ago

steve-bate commented 3 years ago

Describe the bug RegisterControllerType cannot register subclass of WebApiController.

To Reproduce Steps to reproduce the behavior:

  1. Define WebApiController subclass, e.g., MyController : WebApiController
  2. Attempt to register the controller using
    WebApiController myController = ... // From dependency injection framework, for example
    // Also fails if myController type is MyController
    server.WithWebApi(url, MySerializer, m => m.WithController(typeof(MyController), () => myControllerInstance)
  3. ArgumentException is thrown in WebApiModuleBase.RegisterControllerType.

Expected behavior Registration is successful.

Additional context The function is documented with the following requirements:

Since the factory return type is always WebApiController (Func declaration), it seems these constraints cannot be satisfied. If Controller is a subclass of WebApiController (constraint #1), it cannot also be a superclass of WebApiController (implied by constraint #3 since factory return type is WebApiController and it must be the controller type or a subclass of controller type).

rdeago commented 3 years ago

Hello @steve-bate, thanks for using EmbedIO!

I think the problem may be that the automatically inferred return type of your factory lambda is WebApiController. RegisterControllerType uses reflection to validate the factory return type before actually calling it

Can you try this small modification?

WebApiController myController = ... // From dependency injection framework, for example
// Shouldn't fail because now the lambda returns MyController.
server.WithWebApi(url, MySerializer, m =>
    m.WithController(
        typeof(MyController),
        () => myControllerInstance as MyController); // <-- Added " as MyController"

EDIT 1: As @steve-bate atates in the following comment, this does not change the return type of the lambda.

A possible alternative is to first call the factory function and then examine the type of the returned controller. This shouldn't be a breaking change, unless some factory function does really weird things... but all succeeding use cases would continue to succeed, so I don't see any big trouble there.

Would it work better that way? Or am I overlooking some other usage scenario?

EDIT 2: Calling the factory function is actually a bad idea. There's no guarantee that it will always return an object of the same type.

Picking some people's brains, in rigorously random order: @geoperez @mariodivece @bufferUnderrun @AbeniMatteo @madnik7 @rocketraman @bdurrer @Kaliumhexacyanoferrat

steve-bate commented 3 years ago

Hello @rdeago. Thanks for the quick response. I did try what you suggested before I posted the issue. The .NET type inferencing seemed to still interpret the lambda as a Func, possibly because of the WithController signature. That surprised me a bit. For now, I've worked around it using reflection to dynamically create a Func as I iterate over the collection of controllers from the DI framework (AutoFac). I can pass a Func to a Func argument, but reflection still reports the return type of the lambda as SomeController.

rdeago commented 3 years ago

I deleted my comment because I suddenly realized that we're down the wrong path here.

You cannot use the same controller object for all requests, which is what you apparently want to do. Should two or more simultaneous requests arrive for the same controller type, EmbedIO would invoke the factory function once per request, obtain the same object, and overwrite its HttpContext and Route properties. Then, when the function invoked to serve the first request uses e.g. the Response property, it will respond to the last request. Besides, if a controller is disposable, its Dispose method will be called after every controller method invocation.

You don't need a factory function that returns a pre-made singleton controller, but a factory function that calls AutoFac every time to obtain a new controller and returns it.

steve-bate commented 3 years ago

Ah, that's why I couldn't find the comment on GitHub. :-) In my case, each controller type is registered with Autofac and has metadata specifying the base URL for that controller. Assuming the base URLs are correct, the request should be routed to only one controller.

steve-bate commented 3 years ago

I've reread your comment again and I'll need to do more investigation. Initial testing appeared to be working but maybe I just haven't seen the problem yet. Thanks for the suggestion.

rdeago commented 3 years ago

Intrigued by the implications of this issue, I made a little research of my own. I was starting a comment here, but pretty soon it expanded into a wiki article.

https://github.com/unosquare/embedio/wiki/EmbedIO-and-IoC-containers

@steve-bate will you be so kind as to review the article and make sure I didn't write a ton of BS? 😁 I've never actually used Autofac, so it's entirely possible that something silly has slipped through the cracks.

steve-bate commented 3 years ago

I've read the article. I think it's very useful for providing ideas about how to do the integration. Our case is a little simpler since we don't need request-scope services. I may have misunderstood something (and I haven't tried to compile the code), but it doesn't appear the container built in Main is passed through RunWebServerAsync to CreateWebServer.

rdeago commented 3 years ago

Thanks for pointing out the error, I've corrected the sample code.

If you only need to use IoC-provided singletons, you probably only need to copy and paste the first block of code in the article.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.