Update Dependency Injection to v3.0.0

[SkyeHoefling]

Fixes: #3145 #3146

Summary

Updated Dependency Injection library to v3.0.0 and updated all packages.config files to targetFramework net472 which forces the NuGet to pull the correct version

Notable Change

The DotNetNuke.Web.Mvc library has been referencing an incompatible version of Microsoft.AspNet.Razor for awhile now. This causes problems when trying to update NuGets. This required a simple minor version update from 3.1.1 -> 3.1.2. If this change isn't a good idea, I can try and roll it back to 3.1.1

Feedback/Todo List

• Update Nuspec
• Update csproj files to v3.0.0
• Update Library.build files where appropriate
• Updated Microsoft.AspNet.Razor, Microsoft.AspNet.WebPages to v3.1.2
• Add binding redirect to installer

[mitchelsellers]

This change needs additionally changes to the .nuspec files to update the TargetFrameworks for them per this documentation: https://docs.microsoft.com/en-us/nuget/reference/target-frameworks

[SkyeHoefling]

That reminds me we need to double check the nuspec for the DotNetNuke.DependencyInjection library and make sure the Microsoft library reference is at v3.0.0

[SkyeHoefling]

@mitchelsellers is there any type of web.config or install/upgrade scenarios we should be concerned about with this change?

[SkyeHoefling]

I have started my implementation of Razor Pages Pipeline, in doing so I have been looking a lot at the Dependency Injection libs and how it relates to this PR. The Microsoft.AspNetCore.Mvc.Razor library has a dependency on Microsoft.Extensions.DependencyInjection >= 2.2.0 which first led me to upgrade the DI library to v3.0.0. The problem I am now running into is since that library wants version >= 2.2.0 in .NET Framework it is expecting 2.2.0 which means 3.0.0 does not work. After reading some forum posts about it, the recommended best practice for .NET Framework implementations is creating a binding redirect, which is easy enough to add to our upgrade scripts for the web.config.

My Questions

• Should we add a Binding Redirect for this specific change, or wait for the Razor Pages PR?
• When we make the follow-up PR targeting 9.x should we include the Binding Redirect change

I am in support of adding the binding redirect change with this PR because it will mitigate any side-effects of 3rd party modules. If someone builds a module that pulls in a library that requires Microsoft.Extension.DependencyInjection version >= 2.2.0 they will run into issues until they apply the Binding Redirect. I believe it is safe for us to add the Binding Redirect in to prevent these types of side-effects

[bdukes]

My opinion is that every strong-named assembly reference in DNN needs a binding redirect, otherwise we're just asking for conflicts when 3rd party extensions want to use the same dependencies that the platform uses.

[mitchelsellers]

What @bdukes said

[valadas]

Makes sense to me.

[SkyeHoefling]

I think the last thing I need to do is add a binding redirect for the Dependency Injection library to the web.config.

I plan to add a new 10.00.00.config file to DNN Platform\Website\Install\Config, is this the correct spot or is there another spot I also need to add it to?

cc: @bdukes

[SkyeHoefling]

My latest update includes many NuGet references to updating to version 3.1.2. This is something that really should have been done A LONG time ago with the Mvc project. There is a dependency on Microsoft.AspNet.Mvc v5.1.3 which is not compatible with Microsoft.AspNet.* v3.1.1, but v5.1.3 is compatible with v3.1.2 and greater.

I am not 100% sure how these dependencies got included in our Mvc project, maybe someone manually edited the packages.config and .csproj or it was included in a time before compatibility checks. Not really sure, but if you tried adding any NuGet or updating anything to the Mvc project you would see errors that would not let you proceed without manually editing files.

I believe making the patch upgrade to v3.1.2 is safe and necessary for the long-term maintainability of the Mvc project. Consolidating this change across the platform is significant since those NuGet are referenced just about everyone in the project

[SkyeHoefling]

The build is failing due to hardcoded references to the Microsoft.AspNet.WebPages assembly in the packages folder. This reference is not using NuGet, which makes it very difficult to maintain.

[SkyeHoefling]

I think this PR is ready to be reviewed again and merged if approved. Let me know if there is any other changes that I need to make.

@mitchelsellers once we approve this PR I can make a follow-up PR to branch 9.4.x. I think that PR will just be a minimal change without all the additional code clean-ups I did in this PR.

[valadas]

@ahoefling we do merge changes from 9.4.x into development (for 10) every couple weeks or when someone requests it for some reason. So I don't know if you need 2 PRs or if there is something different targetting 9.4.x and 10 ?

[SkyeHoefling]

I need this for Razor Pages so I would like it to go straight into development. This change as it is creates too much risk IMO to go to 9.4.x. I can make a minimal version of this change for 9.4.x

[SkyeHoefling]

If the reviewers group wants me to rebase this change to 9.4.x, I will but I think the side-effects of updating the Microsoft.AspNet.* to v3.1.2 and all the nugets to targetframework to net472 introduces risk. Granted this is stuff that should have been done a long time ago, so if the reviewers agree I will rebase this on 9.4.x

[bdukes]

No, I agree that the 3.0.0 upgrade should be held for version 10.

I pushed a small change to a couple of references that changed due to the 4.7.2 upgrade.

[SkyeHoefling]

@bdukes thanks for taking care of that!

If we make any changes to dependency injection for 9.4.x when we bring those changes over we will need to handle merge conflicts as well as unexpected projects that may be referencing the Dependency Injection Library. We should be able to handle that using the NuGet Package Manager using the consolidate tab, but we may still run into issues with the Mvc project.

This is probably going to be a problem as I am working on a PR to refactor the Globals.cs #3153 which adds the Dependency Injection lib to many other projects inside the DNN solution.

[valadas]

Please hold merging this until we re-update development with latest changes from 9.4.2

[mitchelsellers]

With the release of 3.1.0 - I would argue that we need to update this to that version

[valadas]

@ahoefling @mitchelsellers does this need only rebasing or a change of version or a new pr?

[mitchelsellers]

@ahoefling do you know what the status will be with this DLL in relation to the .NET Core 3.0 requirements re-.NET Framework?

[bdukes]

@ahoefling FYI, I've rebased this on future/10 (it was originally created against development, which we just deleted)

[stale]

We have detected this issue has not had any activity during the last 90 days. That could mean this issue is no longer relevant and/or nobody has found the necessary time to address the issue. We are trying to keep the list of open issues limited to those issues that are relevant to the majority and to close the ones that have become 'stale' (inactive). If no further activity is detected within the next 14 days, the issue will be closed automatically.
If new comments are are posted and/or a solution (pull request) is submitted for review that references this issue, the issue will not be closed. Closed issues can be reopened at any time in the future. Please remember those participating in this open source project are volunteers trying to help others and creating a better DNN Platform for all. Thank you for your continued involvement and contributions!

[stale]

We have detected this issue has not had any activity during the last 90 days. That could mean this issue is no longer relevant and/or nobody has found the necessary time to address the issue. We are trying to keep the list of open issues limited to those issues that are relevant to the majority and to close the ones that have become 'stale' (inactive). If no further activity is detected within the next 14 days, the issue will be closed automatically.
If new comments are are posted and/or a solution (pull request) is submitted for review that references this issue, the issue will not be closed. Closed issues can be reopened at any time in the future. Please remember those participating in this open source project are volunteers trying to help others and creating a better DNN Platform for all. Thank you for your continued involvement and contributions!

[valadas]

PR pinned and unstaled

[mitchelsellers]

Thank you for this contribution, we are going to look at creating a new PR that would incorporate these changes with the new 10.0 release cycle. We thank everyone on this chain for their contributions thus far.