aspnet / AspNetWebHooks

Libraries to create and consume web hooks on ASP.NET 4.x (Due to other priorities this project is currently in maintenance mode only. There are no planned releases at this time. No new features are planned and we are only addressing critical issues as required.)
Apache License 2.0
134 stars 103 forks source link

Issue with having Webhooks and 2.0 version of DependencyInjection in same project #5

Open rsvaidya opened 6 years ago

rsvaidya commented 6 years ago

Reopening the issue: https://github.com/aspnet/WebHooks/issues/229 in this repo.


Hi,

Microsoft.AspNet.WebHooks.Custom.AzureStorage.dll has a dependency on 1.0 version of Microsoft.Extensions.DependencyInjection.dll.

In our application we have a hard dependency on 2.0 version of Microsoft.Extensions.DependencyInjection. Because of this we get the following exception:

System.MissingMethodException: Method not found: 'System.IServiceProvider Microsoft.Extensions.DependencyInjection.ServiceCollectionContainerBuilderExtensions.BuildServiceProvider(Microsoft.Extensions.DependencyInjection.IServiceCollection)'. at Microsoft.AspNet.WebHooks.DataSecurity.GetDataProtector() at System.Web.Http.HttpConfigurationExtensions.InitializeCustomWebHooksAzureStorage(HttpConfiguration config, Boolean encryptData)

Other frameworks also seem to have similar issues. See https://github.com/dotnet/orleans/issues/3325. We raised this issue on the DependencyInjection repo first and the suggestion is to recompile WebHooks with 2.0 version of DependencyInjection. See https://github.com/aspnet/Home/issues/2788.

What is the best way to solve this problem?

Thanks, Raghavendra

dougbu commented 6 years ago

First action is to confirm bug exists and isn't specific to one machine.

Need to discuss downstream impact e.g. when using 3rd party DI providers. Looks like existing providers either cross-compile or have stuck with DI 1.0.

dougbu commented 6 years ago

Copying old comment…

Please move this issue to the aspnet/AspNetWebHooks repo. We just separated ASP.NET WebHooks from AS.NET Core WebHooks and the former now lives in that repo. (Will update the Readme files soon.)

Unfortunately, fixing this issue won't be as simple as "recompiling WebHooks with 2.0 version of DependencyInjection". Will likely need to bump the target framework of the Microsoft.AspNet.WebHooks.Custom.AzureStorage and Microsoft.AspNet.WebHooks.Custom.SqlStorage packages to .NET Framework 4.6.1. (The newer DependencyInjection package targets .NET Standard 2.0, not the current .NET Standard 1.1.) That's a breaking change.

Could target both .NET Framework 4.5 and 4.6.1 (with different DependencyInjection versions) in those projects. But, I'd prefer we avoid the multi-targeting complications. Is 4.6.1 is sufficient for everyone likely to upgrade their Microsoft.AspNet.WebHooks.Custom.AzureStorage or Microsoft.AspNet.WebHooks.Custom.SqlStorage dependencies?

dougbu commented 6 years ago

And another comment on aspnet/WebHooks#229:

By the way, are you using WebHooks sender or receiver packages in your application? I'm wondering if the ASP.NET Core WebHooks packages would be a better choice for your scenario.


Of course, I was misreading this issue. Are dealing with custom WebHooks sender packages here.

dougbu commented 6 years ago

Self-assigning temporarily for the confirmation.

dougbu commented 6 years ago

Confirmed the breaking change. Change leading to the MissingMethodException was introduced in aspnet/DependencyInjection@45bbdb5efc. The return value for the BuildServiceProvider(...) extension method was System.IServiceProvider and is now Microsoft.Extensions.DependencyInjection.ServiceProvider. DataSecurity (a class shared between the Microsoft.AspNet.WebHooks.Custom.AzureStorage and Microsoft.AspNet.WebHooks.Custom.SqlStorage projects) calls that extension directly.

dougbu commented 6 years ago

May have a compromise fix here: Move from Microsoft.Extensions.DependencyInjection v1.0.x to v1.1.x. That newer minor release still targets .NET Standard 1.1 but adds DefaultServiceProviderFactory. The new class wraps the service collection and has had no breaking changes since v1.1.x.

@davidfowl I realize this use case isn't the original purpose for DefaultServiceProviderFactory. But, would the following be reasonable?

Change ASP.NET WebHooks' DataSecurity from https://github.com/aspnet/AspNetWebHooks/blob/f702a66575d86a1c6c86dfbec6a6b97547d6b223/src/Common/DataSecurity.cs#L32-L34 to

var serviceCollection = new ServiceCollection();
serviceCollection.AddDataProtection();
var factory = new DefaultServiceProviderFactory();
var services = factory.CreateServiceProvider(serviceCollection);

Of course, may hit other 1.0 / 1.1 to 2.0 / 2.1 breaking changes if we go this route.

dougbu commented 6 years ago

Go with the v1.1 bump for the DependencyInjection package.

rsvaidya commented 6 years ago

Hi Doug,

We cannot go with 1.1 version of DependencyInjection package as there is a hard dependency on 2.0 (or higher) version of DependencyInjection.

dougbu commented 6 years ago

@rsvaidya the suggestion is about changing our dependencies so that your dependencies are more flexible. If we use v1.1 of DependencyInjection, your application should be able to use v2.0 of that package.

rsvaidya commented 6 years ago

@dougbu thanks for the clarification :+1: