autofac / Autofac.WebApi

ASP.NET Web API integration for Autofac
Other
36 stars 27 forks source link

Can't add multiple filters to the same controller #7

Closed tillig closed 8 years ago

tillig commented 9 years ago

From @taschmidt on October 15, 2014 13:53

Given this code:

    builder.RegisterType<FilterA>().AsWebApiExceptionFilterFor<MyController>().InstancePerRequest();
    builder.RegisterType<FilterB>().AsWebApiExceptionFilterFor<MyController>().InstancePerRequest();

I get the following exception:

[ArgumentException: An item with the same key has already been added.]
   System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add) +14874155
   Autofac.Builder.RegistrationBuilder`3.WithMetadata(String key, Object value) +38
   Autofac.Integration.WebApi.RegistrationExtensions.AsFilterFor(IRegistrationBuilder`3 registration, String metadataKey) +294
   ...

We should definitely have the ability to register multiple filters on the same controller.

Copied from original issue: autofac/Autofac#586

tillig commented 9 years ago

From @taschmidt on October 15, 2014 13:57

A relatively easy solution would be to append the type of the filter to the key. The only limitation there would be if someone wanted to have multiple instances of a certain filter but with different parameters? I don't know how likely a scenario that is but from a philosophical standpoint, filters are meant to be a pipeline-type mechanism and Autofac really shouldn't stand in the way of that. But what's the alternative then? Just append a unique ID to each filter?

tillig commented 9 years ago

I think type is a good unique identifier unless it's an attribute-based filter, in which case we should use the TypeId value as the ID to follow the way MVC determines uniqueness.

tillig commented 9 years ago

From @alexmg on October 15, 2014 14:20

We need to know the key value in advance. Being able to add to the existing metadata value would solve the problem. The value with be a list of FilterMetadata instead of a single instance.

scottleyg commented 9 years ago

this also throws (using WebApi Integration nuget pkg Autofac.WebApi2 version 3.2.1) builder .RegisterType() .AsWebApiActionFilterFor(api => api.Action1())) .AsWebApiActionFilterFor(api => api.Action2()))
.InstancePerApiRequest();

tillig commented 8 years ago

I'm not currently able to reproduce this using the 3.4.0 version of the code. I explicitly put a test together with some test types that looks like this:

        [Fact]
        public void MultipleExceptionFiltersOnOneController()
        {
            var builder = new ContainerBuilder();
            builder.Register<ILogger>(c => new Logger()).InstancePerDependency();
            builder.RegisterType<TestExceptionFilter>().AsWebApiExceptionFilterFor<TestController>();
            builder.RegisterType<TestExceptionFilter2>().AsWebApiExceptionFilterFor<TestController>();
            var container = builder.Build();
            var provider = new AutofacWebApiFilterProvider(container);
            var configuration = new HttpConfiguration { DependencyResolver = new AutofacWebApiDependencyResolver(container) };
            var actionDescriptor = BuildActionDescriptorForGetMethod();

            var filterInfos = provider.GetFilters(configuration, actionDescriptor).ToArray();

            var wrapperType = GetWrapperType();
            var filters = filterInfos.Select(info => info.Instance).Where(i => i.GetType() == wrapperType).ToArray();
            Assert.Equal(1, filters.Length);
            Assert.IsType(wrapperType, filters[0]);
        }

We actually already have unit tests for multiple controllers per filter on all the filter types (so I don't need to keep this one-off, but I just wanted to manually verify).

I'm going to close this guessing that it was fixed sometime between the 3.2.1 version and now. If things are failing, upgrade to the latest.

If you can still repro this with the latest stable bits, please post that repro somewhere and let us know so we can re-open the issue.