Castle Windsor DI installer: dependency factory me

问题:

I am trying to implement DI with Castle Windsor. Currently I have a controller with overloaded constructors like this (this is an antipattern as described here: https://www.cuttingedge.it/blogs/steven/pivot/entry.php?id=97):

public class MyController : ApiController
{
    protected IStorageService StorageService;

    protected MyController()
    {
        StorageService = StorageServiceFactory.CreateStorageService(User.Identity as ClaimsIdentity);
    }

    protected MyController(IStorageService storageService)
    {
        StorageService = storageService;
    }
}

I am trying to get rid of the first constructor and have Castle Windsor handle the resolution of the storage service dependency.

I created a Castle Windsor installer class like this:

public class StorageServiceInstaller : IWindsorInstaller
{
    public void Install(IWindsorContainer container, IConfigurationStore store)
    {
        container.Register(
            Component.For<IStorageService>()
                     .UsingFactoryMethod(
                         () => StorageServiceFactory.CreateStorageService(User.Identity as ClaimsIdentity)));
    }
}

The problem is that User (which has type IPrincipal) is a property on ApiController, so it's not accessible from the installer. How can I make this work?

---

Update:

@PatrickQuirk seems to be implying that there is a better way to do this using Castle Windsor without needing a factory at all.

My StorageServiceFactory looks like this:

public static class StorageServiceFactory
{
    public static IStorageService CreateStorageService(ClaimsIdentity identity)
    {
        if (identity == null)
        {
            return null;
        }

        Claim providerKeyClaim = identity.FindFirst(ClaimTypes.NameIdentifier);
        if (providerKeyClaim == null || string.IsNullOrEmpty(providerKeyClaim.Value))
        {
            return null;
        }

        StorageProviderType storageProviderType;
        string storageProviderString = identity.FindFirstValue("storage_provider");
        if (string.IsNullOrWhiteSpace(storageProviderString) || !Enum.TryParse(storageProviderString, out storageProviderType))
        {
            return null;
        }

        string accessToken = identity.FindFirstValue("access_token");
        if (string.IsNullOrWhiteSpace(accessToken))
        {
            return null;
        }

        switch (storageProviderType)
        {
            // Return IStorageService implementation based on the type...
        }
    }
}

Is there a way to incorporate selecting the correct IStorageService into Windsor's dependency resolution and avoid the factory altogether? Or do I still need it?

I like @PatrickQuirk's solution, except that it seems odd to have to create a wrapper and corresponding wrapper interface for the factory just for the sake of dependency injection. Ideally I'd have the api controller's constructor take in an IStorageService as a parameter, which seems more intuitive/consistent with the field that actually needs to be set.

回答1:

I don't think the multiple constructors is as much of a sin as the hidden dependency on StorageServiceFactory is, but I agree with your approach for the most part.

Instead of a factory method, pass a factory object into the class and have it create the storage service:

public class MyController : ApiController
{
    protected IStorageService StorageService;

    protected MyController(IStorageServiceFactory storageServiceFactory)
    {
        StorageService = storageServiceFactory.CreateStorageService(User.Identity as ClaimsIdentity);
    }
}

And then define your factory interface and implementation:

public interface IStorageServiceFactory
{
    IStorageService Create(ClaimsIdentity claimsIdentity);
}

public class StorageServiceFactoryImpl : IStorageServiceFactory
{
    public IStorageService Create(ClaimsIdentity claimsIdentity)
    {
        return StorageServiceFactory.CreateStorageService(claimsIdentity);
    }
}

This way, you have a single constructor and the dependency on the storage service factory is explicit.

---

Regarding your update:

...it seems odd to have to create a wrapper and corresponding wrapper interface for the factory just for the sake of dependency injection.

Well, that's kind of the point of dependency injection.

The wrapper I propose is solving two problems: it removes the need to call a static method from inside your class (hiding a dependency), and allows for delayed resolution (because your dependency relies on member data to be created).

If you have a way to change the dependencies of creating an IStorageService to not rely on a member of the class you're giving it to, then you could pass one in directly (provided you can tell Windsor how to create one).