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

DI: Allow overriding MVC/WebApi Controller registrations. #3543

Closed
5 tasks
dimarobert opened this issue Feb 5, 2020 · 2 comments · Fixed by #3544
Closed
5 tasks

DI: Allow overriding MVC/WebApi Controller registrations. #3543

dimarobert opened this issue Feb 5, 2020 · 2 comments · Fixed by #3544

Comments

@dimarobert
Copy link
Contributor

dimarobert commented Feb 5, 2020

Description of problem

With the current way of registering the services in the DI Container it is a bit difficult (even impossible in some cases) to override Controller registrations.
In the case of WebApi Controllers, they are registered after all IDnnStartup implementations, at the end of DotNetNuke.Web.Startup.ConfigureServices().
As for the MVC Controllers, they are registered in the MVC IDnnStartup. The iteration over the IDnnStartup implementations does not have a specific ordering, which means that most likely they will be called based on the name of the containing assembly. Because of this, the MVC Startup might be executed after some other vendors Startup's.
This ordering issue might affect other areas as well.

Description of solution

The direct solution for the issue at hand would be to register the controllers using the TryAddScoped(controllerType) IServiceCollection extension method, which will add the controller to the collection only if the controller hasn't already been registered.

Description of alternatives considered

A solution to the broader issue, would be to either define an ordering/priority scheme, that could span multiple levels, either just calling the DNN IDnnStartup implementations first and then the rest.

Affected browser

  • Chrome
  • Firefox
  • Safari
  • Internet Explorer
  • Edge
@bdukes
Copy link
Contributor

bdukes commented Feb 5, 2020

Is this a duplicate of #3335? If we fixed the ordering to load DNN startups first, would that fix this issue completely, or is there more to it?

@dimarobert
Copy link
Contributor Author

@bdukes Yes, this appears be a duplicate of #3335. From what I can see there is no PR for that issue and solving the priority might not be trivial. With the fix in the PR that I've opened for this issue the controllers (which are the entry point for the vendors) can be easily overridden no matter the ordering of the IDnnStartups. I was hoping that the PR could be merged for the 9.5.0 release as the changes are minimal and reverting back from the TryAdd is non-breaking once the ordering is in place.

@valadas valadas added this to the 9.5.1 milestone Feb 18, 2020
@valadas valadas modified the milestones: 9.5.1, 9.6.0 Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants