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

Added Dependency Injection to all Module Patterns and Removed Circular Dependencies in Module Pipeline #2774

Merged
merged 41 commits into from May 14, 2019

Conversation

Projects
None yet
8 participants
@ahoefling
Copy link
Contributor

commented May 10, 2019

UPDATED 5/13/2019 @ 09:54 EDT

Fixes: #2761

First major improvement towards .NET Core

Summary

Removed Reflection calls in the ModuleControlFactory (now the ModuleControlPipeline) by removing Circular Dependencies and adding Dependency Injection.

Module Pipeline

The different module patterns were invoked in the ModuleControlFactory which has been moved to a new library called DotNetNuke.ModulePipeline. The root level ModuleControlFactory (Yes, there were more than 1) has been renamed to ModuleControlPipeline which contains an interface IModuleControlPipeline which makes it easy to register into the Dependency Injection Container.

Since all the independent module patterns have the exact same interface of IModuleControlFactory we were able to register each Module Pattern with the Dependency Injection container and then inject the different Factories into the ModuleControlPipeline where it determines which one to use.

Prior to this change the following libraries where instantiated by reflection with each module load which created a performance hit

  • DotNetNuke.Web.Mvc
  • DotNetNuke.Web.Razor

Dependency Injection

Added the ASP.NET Core Dependency Injection libraries

  • Microsoft.Extensions.DependencyInjection
  • Microsoft.Extensions.DependencyInjection.Abstractions

New Dependency Injection Library specific for DNN

  • DotNetNuke.DependencyInjection

New NUSPEC for 3rd-party libraries using dependency injection. The new library properly depends on the Microsoft NuGets so all the 3rd party developer has to do is include the following library.

  • `DotNetNuke.DependencyInjection

Added IDnnStartupto the DotNetNuke.Common.Globals which provides easy access anywhere in DNN to use the Service Locator pattern to resolve dependencies. It is always recommended to use constructor injection when possible, but the ServiceProvider provides a workaround if not possible.

Added IDnnStartup interface that exists in the DotNetNuke.DependencyInjection library. Any file that implements this interface is invoked at App Start and registers all services in the implemented class. (See Usage API for more details)

The IDnnStartup is configured at the beginning of the Application Startup logic and is one of the first things that is invoked.

Usage API

To use the new Dependency Injection Libraries you will add the DotNetNuke.DependencyInjection NuGet (which will be available after the next release).

Example
Suppose you have a simple interface of IMessageService and an implementation of MessageService

IMessageService

public interface IMessageService
{
    string GetMessage();
}

Create a Startup.cs file like below.

public class Startup : IDnnStartup
{
    public void ConfigureServices(IServiceCollection services)
    {
        services.AddScoped<IMessageService, MessageService>();
    }
}

MVC Modules

Create a Startup file like the above example and include it in the MVC Module assembly typically at the root level.

HomeController

public class HomeController : DnnController
{
    protected IMessageService MessageService { get; }
    public HomeController(IMessageService messageService)
    {
        MessageService = messageService;
    }

    public ActionResult Index()
    {
        var model = new HelloWorldModel
        {
            Message = MessageService.GetMessage()
        };
        return View(model);
    }
}

SPA Modules using Web API

Create a Startup file like the above example and include it in the MVC Module assembly typically at the root level.

HomeController

public class HomeController : DnnApiController
{
    protected IMessageService MessageService { get; }
    public HomeController(IMessageService messageService)
    {
        MessageService = messageService;
    }

    public string Get()
    {
        return MessageService.GetMessage();
    }
}

WebForms

Create a Startup file like the above example and include it in the Web Forms Module assembly typically at the root level.

Web forms doesn't support constructor injection but using the Service Locator pattern you can access the IMessageService

public partial class Index : PortalModuleBase
{
    protected IMessageService MessageService { get; }

    public Index()
    {
        MessageService = DotNetNuke.Common.Globals.DependencyProvider.GetService<IMessageService>();;
    }

    Message => MessageService.GetMessage();
}

Razor3 Modules

Create a Startup file like the above example and include it in the Razor3 Module assembly typically at the root level. Make sure to register your model or constructor injection will not work.

Startup.cs

public class Startup : IDnnStartup
{
    public void ConfigureServices(IServiceCollection services)
    {
        services.AddScoped<IMessageService, MessageService>();
        services.AddScoped<IndexModel>();
    }
}

IndexModel.cs

public class IndexModel
{
    protected IMessageService MessageService { get; }
    public IndexModel(IMessageService messageService)
    {
        MessageService = messageService;
    }

    public string Message => MessageService.GetMessage();
}

ahoefling added some commits May 7, 2019

Created Module Pipeline to containt the ModuleControlFactory that ini…
…tializes the different module patterns
Created new interface IModuleControlPipeline; Updated instantiation o…
…f the pipeline to be a singleton in the Core Library. This reduces reflection calls and the new Module Pipeline can directly reference the different module platforms which increases performance.
Created ModuleControlPipeline singleton for resolving the DotNetNuke.…
…ModulePipeline and reducing reflection calls
Added Microsoft.Extensions.DependencyInjection to the platform. Updat…
…ed the ModuleControlFactory to use DependencyInjection to better handle the reflection problems
Added local build scripts to properly copy assemblies for the new lib…
…raries (DotNetNuke.ModulePipeline and DotNetNuke.DependencyInjection)
Removed hard-coded project depenency on DotNetNuke.Web as DotNetNuke.…
…Web.Mvc does not use it. Updated ModulePipeline to use a project reference to DotNetNuke.Web.Mvc
Added dependency injection to MVC Modules by creating a custom IContr…
…ollerFactory that attempts to generate the controller via the DI Container. Added startup code that registers all MVC Controllers with the container
@ahoefling

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2019

@ohine can you take a look at the build considerations with this PR. I added 2 new projects and want to make sure that the installer will include the new assemblies correctly. Here is a list of the new assemblies that need to be included in the bin directory

  • DotNetNuke.DependencyInjection
  • DotNetNuke.ModulePipeline
  • Microsoft.Extensions.DependencyInjection
  • Microsoft.Extensions.DependencyINjection.Abstractions

As well, I create a nuspec file for DotNetNuke.DependencyInjection and want to make sure that file gets published to the nuget feed correctly for 9.4

@ohine

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

For sure, I’ll take a look the build and also get a review going. However first, thank you for this amazing contribution!!

@ohine

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

/azp run

@azure-pipelines

This comment has been minimized.

Copy link

commented May 10, 2019

Azure Pipelines successfully started running 2 pipeline(s).

@ahoefling

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

Update

Added Dependency Injection for Web Api Controllers in the DnnApiController. This is a controller that is commonly used in SPA Modules.

@ahoefling

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

@ohine can you look at the latest CI build. Looks like the failure is unrelated to my latest changes

@ohine

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

That's a weird failure for sure, it appears the devops build agent lost internet connectivity? Bizarre. Attempting to re-run that pipeline now...

@dnnsoftware dnnsoftware deleted a comment from azure-pipelines bot May 13, 2019

@CBPSC

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

EPIC!

@bdukes

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

This is great! For constructor injection in Web Forms, were you not able to turn that on for some reason? Now that we're targetting .NET 4.7.2, you should just be able to set HttpRuntime.WebObjectActivator. If there are issues with that, we don't need to hold up this PR, just wondering if that could be enabled, also.

@ahoefling

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

@bdukes thanks for bringing up the HttpRuntime.WebObjectActivator this is something I was trying to leverage especially for the WebForms modules. Considering the HttpRuntime.WebObjectActivator gives us access to the IServicePincipal this would be a great object to use for the service locator instead of our DotNetNuke.Globals.DependencyProvider. The HttpRuntime.WebObjectActivator also should provide dependency injection into WebForms pages, I'm not sure if it works for WebForms controls as many articles I ran across had issues with WebForms controls which is what our WebForms modules are built on top of.

When I tried using the HttpRuntime.WebObjectActivator I ran into a lot of problems with the website crashing. I had a theory that it was tied to the ClientDependency library but no real way to determine where the problem was because the Application Startup code wasn't running.

@bdukes

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

👍 When I had played with it, I got it working somewhat, but only after lots of crashing, so I'm happy to leave that out for now. Hopefully we can pinpoint the issue and get it working, but it's not a showstopper to get this in.

@bdukes bdukes requested a review from mitchelsellers May 13, 2019

ahoefling added some commits May 13, 2019

Added StringComprar.OrgindalCase to control factories dictionary. Thi…
…s ignores thes case when checking extensions
@ahoefling

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

@mtrutledge I have fixed your requested change, thanks for the feedback

@bdukes I have fixed some of your requested changes and have some follow up we need to finish in the review. Thanks for the feedback on these!

Show resolved Hide resolved DNN Platform/DotNetNuke.Web/Startup.cs Outdated
@@ -124,7 +124,7 @@
<trust level="Full" originUrl=".*" />
-->
<!-- set debugmode to false for running application -->
<compilation debug="false" strict="false" targetFramework="4.5">
<compilation debug="false" strict="false" targetFramework="4.7.2">

This comment has been minimized.

Copy link
@bdukes

bdukes May 13, 2019

Contributor

Do we already have code to update the web.config on upgrades?

This comment has been minimized.

Copy link
@ahoefling

ahoefling May 13, 2019

Author Contributor

This was missed when I put in the 4.7.2 PR. Where does the upgrade code for the web.config exist? If it is not in there we need to make sure it is included.

This comment has been minimized.

Copy link
@bdukes

bdukes May 13, 2019

Contributor

@ohine would this be Upgrade.cs code that calls an XML Merge template?

ahoefling added some commits May 13, 2019

Added defensive code the the Dependency Injection startup routine to …
…prevent the website from crashing if a 3rd party module crashes at instantiation
Added defensive code for executing ConfigureServices in case a 3rd pa…
…rty module throws an exception. This will prevent the website from crashing at Application Startup
@bdukes

bdukes approved these changes May 13, 2019

Updated GetModuleControlFactory so it properly falls through to the d…
…efault by adding a `TryGetValue`. Before the code would throw an exception and not use the fallthrough factory

@mitchelsellers mitchelsellers merged commit cb5c2d0 into dnnsoftware:development May 14, 2019

2 checks passed

DNN [CI] Build #9.4.0-pr2774.44.17034 succeeded
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.