Skip to content

CustomFilterProvider #1

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

Open
omar opened this issue Nov 26, 2016 · 5 comments
Open

CustomFilterProvider #1

omar opened this issue Nov 26, 2016 · 5 comments

Comments

@omar
Copy link

omar commented Nov 26, 2016

Thank you for creating this repo and pushing Microsoft to add more flexibility in choice of containers.

Using the sample Ninject, project, I set everything up but I'm getting an error when I try to resolve/active a filter of type IAuthorizationFilter which depends on a types in my container.

Is there a way to replace the activator for filters? I tried to replace the IFilterProvider via:

public static void AddCustomFilterProvider(this IServiceCollection services, 
    Func<Type, object> provider)
{
    if (services == null) throw new ArgumentNullException(nameof(services));
    if (provider == null) throw new ArgumentNullException(nameof(provider));

    services.AddSingleton<IFilterProvider>(new DelegatingFilterProvider(provider));
}

But I wasn't able to get a breakpoint in DelegatingFilterProvider to hit. DelegatingFilterProvider just inherits from DefaultFilterProvider at the moment.

Here's the relevant part of the exception:

System.InvalidOperationException: Unable to resolve service for type 'Omar.IMyService' while attempting to activate 'Omar.MyFilter'.
   at Microsoft.Extensions.Internal.ActivatorUtilities.GetService(IServiceProvider sp, Type type, Type requiredBy, Boolean isDefaultParameterRequired)
   at lambda_method(Closure , IServiceProvider , Object[] )
   at Microsoft.AspNetCore.Mvc.TypeFilterAttribute.CreateInstance(IServiceProvider serviceProvider)
   at Microsoft.AspNetCore.Mvc.Internal.DefaultFilterProvider.ProvideFilter(FilterProviderContext context, FilterItem filterItem)
   at Microsoft.AspNetCore.Mvc.Internal.DefaultFilterProvider.OnProvidersExecuting(FilterProviderContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvokerCache.GetFilters(ActionContext actionContext, List`1 filterItems)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvokerCache.GetState(ControllerContext controllerContext)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker..ctor(ControllerActionInvokerCache cache, IControllerFactory controllerFactory, IControllerArgumentBinder controllerArgumentBinder, ILogger logger, DiagnosticSource diagnosticSource, ActionContext actionContext, IReadOnlyList`1 valueProviderFactories, Int32 maxModelValidationErrors)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvokerProvider.OnProvidersExecuting(ActionInvokerProviderContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ActionInvokerFactory.CreateInvoker(ActionContext actionContext)
   at Microsoft.AspNetCore.Mvc.Internal.MvcRouteHandler.<>c__DisplayClass8_0.<RouteAsync>b__0(HttpContext c)
   at Microsoft.AspNetCore.Builder.RouterMiddleware.<Invoke>d__4.MoveNext()

Any thoughts on how to do that?

@omar
Copy link
Author

omar commented Nov 26, 2016

Looks like adding the line

services.Remove(new ServiceDescriptor(typeof(IFilterProvider), typeof(DefaultFilterProvider), 
    ServiceLifetime.Singleton));

before adding the custom FilterProvider replaced the default provider. I'm now working on making it the DelegatingFilterProvider call out to Ninject for resolution.

The full method is now:

public static void AddCustomFilterProvider(this IServiceCollection services, 
    Func<Type, object> provider)
{
    if (services == null) throw new ArgumentNullException(nameof(services));
    if (provider == null) throw new ArgumentNullException(nameof(provider));
        
    services.Remove(new ServiceDescriptor(typeof(IFilterProvider), typeof(DefaultFilterProvider),
        ServiceLifetime.Singleton));
    services.TryAddEnumerable(ServiceDescriptor.Singleton<IFilterProvider>(
        new DelegatingFilterProvider(provider)));
}

@omar
Copy link
Author

omar commented Nov 26, 2016

I've been digging into the ASP.NET Core code, but I have yet to grok how the filter activation is happening. There's a lot of layers of misdirection/abstraction and I can only gather that IServiceProvider is tightly coupled to creating a new filter object.

Any help would be appreciated.

@dotnetjunkie
Copy link
Owner

This whole model of adding, replacing and trying to replace registrations is built on quick sand. The order in which you make the calls is crucial, but completely depending on the internal implementation. Most of the time you can simply make your replacing registration after calling AddMvc. In that case what happens is that your registration becomes the last registration of a collection and the built-in container will automatically pick the last registration for that given type (IFilterProvider in your case).

This model however completely breaks in case the framework resolved the abstraction as a collection, as is the case with IFilterProvider where the framework resolves an IEnumerable<IFilterProvider>. In that case, the container will simply resolve the complete list. This list will contain both the framework's DefaultFilterProvider and your custom DelegatingFilterProvider.

What happens when you have two filter providers is unclear to me. What probably happens is that the DefaultFilterProvider alteady sets all FilterItem.Filter properties causing the next filter provider to skip them. If you look in the DefaultFilterProvider source code you see the following:

        public virtual void ProvideFilter(FilterProviderContext context, FilterItem filterItem)
        {
            if (filterItem.Filter != null)
            {
                return;
            }

I must admit that I have no experience with filter providers in ASP.NET Core myself, so I'm not sure whether I can provide you with any information that is of any use to you.

@omar
Copy link
Author

omar commented Nov 27, 2016

Thanks for the input @dotnetjunkie. I spent a lot of time trying to replace the built-in DI framework in ASP.NET Core using the files in this repo, but there seems to be a number of issues as you outlined, including filter injection.

Honestly, at this point, I think the only reliable route is to follow in the footsteps of Autofac and other DI containers and create an adapter. Unfortunately, Ninject doesn't seem to be actively maintained and we might have to write a bit of code to get it working with ASP.NET Core.

@dotnetjunkie
Copy link
Owner

at this point, I think the only reliable route is to follow in the footsteps of Autofac and other DI containers and create an adapter

I surely don't advice you to go this route. Having seen the amount of pain the Autofac maintainers have gone through, I think it is safe to say that it is undoable for a non-maintainer to build a reliable adapter; at this point, it's unclear whether the maintainers of Ninject would be able to succeed.

replace the built-in DI framework in ASP.NET Core using the files in this repo

Don't try to "replace the built-in DI framework". You should leave it as-is. Leave it in for all framework stuff and only plugin your container to resolve application components. I agree this isn't ideal at the moment, due to the lacking design of ASP.NET Core, but going the Conforming Container root will not solve your problems at all. I'm afraid we will have to bite the bullet on this one. After we know where the flaws and shortcomings are, we can communicate this to Microsoft and they can improve their design to minimize the pain for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants