Yes, I think you got the idea right. I like that you're separating concerns between the attribute and the filter implementation, and I like that you're using constructor DI rather than property DI.
Your approach works well if you only have one type of filter. I think the biggest potential area for improvement, if you had more than one type of filter, would be how the filter provider is implemented. Currently, the filter provider is tightly coupled to the attribute and filter instances it is providing.
If you're willing to combine the attribute with the filter and use property DI, there's a simple way to have a more decoupled filter provider. Here are two examples of that approach:
http://www.thecodinghumanist.com/blog/archives/2011/1/27/structuremap-action-filters-and-dependency-injection-in-asp-net-mvc-3
http://lozanotek.com/blog/archive/2010/10/12/dependency_injection_for_filters_in_mvc3.aspx
There are two challenges to solve with the current approach:
1. Injecting some, but not all, of the filter constructor parameters via DI.
2. Mapping from an attribute to a (dependency-injected) filter instance.
Currently, you're doing both manually, which is certainly fine when there's only one filter/attribute. If there were more, you'd probably want a more general approach for both parts.
For challenge #1, you could use something like a _container.Resolve overload that lets you pass in arguments. That solution is rather container-specific and probably a bit tricky.
Another solution, which I'll describe here, separates out a factory class that only takes dependencies in its constructor and produces a filter instance requiring both DI and non-DI arguments.
Here's what that factory might look like:
public interface IFilterInstanceFactory
{
object Create(Attribute attribute);
}
You'd then implement a factory for each attribute/filter pair:
public class MyAuthorizationFilterFactory : IFilterInstanceFactory
{
private readonly IAuthorizationProvider provider;
public MyAuthorizationFilterFactory(IAuthorizationProvider provider)
{
this.provider = provider;
}
public object Create(Attribute attribute)
{
MyAuthorizeAttribute authorizeAttribute = attribute as MyAuthorizeAttribute;
if (authorizeAttribute == null)
{
return null;
}
return new MyAuthorizationFilter(provider, authorizeAttribute.Code);
}
}
You can solve challenge #2 by just registering each implementation of IFilterInstanceFactory with CastleWindsor.
The filter provider can now be decoupled from any knowledge of specific attributes and filters:
public class MyFilterProvider : IFilterProvider
{
private IWindsorContainer _container;
public MyFilterProvider(IWindsorContainer container)
{
Contract.Requires(container != null);
_container = container;
}
public IEnumerable<Filter> GetFilters(ControllerContext controllerContext, ActionDescriptor actionDescriptor)
{
Type controllerType = controllerContext.Controller.GetType();
var authorizationProvider = _container.Resolve<IAuthorizationProvider>();
foreach (FilterAttribute attribute in controllerType.GetCustomAttributes(typeof(FilterAttribute), false))
{
object instance = Resolve(attribute);
yield return new Filter(instance, FilterScope.Controller, 0);
}
foreach (FilterAttribute attribute in actionDescriptor.GetCustomAttributes(typeof(FilterAttribute), false))
{
object instance = Resolve(attribute);
yield return new Filter(instance, FilterScope.Action, 0);
}
}
private object Resolve(Attribute attribute)
{
IFilterInstanceFactory[] factories = _container.ResolveAll<IFilterInstanceFactory>();
foreach (IFilterInstanceFactory factory in factories)
{
object dependencyInjectedInstance = factory.Create(attribute);
if (dependencyInjectedInstance != null)
{
return dependencyInjectedInstance;
}
}
return attribute;
}
}
David
