Is this an abuse of dependency injection? (when are dependencies not dependencies)

Question

We have a multi-tenant web application in which a many pages operate per-tenant. As a result many of our interfaces look like this

interface ISprocketDeployer
{
    void DeploySprocket(int tenantId);
}

It occurred to me that it might be better to simplify these interfaces to be unaware of the tenantId. The pages would also then be unaware of the tenantId, like so

[Inject] // Ninject
public ISprocketDeployer SprocketDeployer { get; set; }

private void _button_OnClick(object sender, EventArgs e)
{
    SprocketDeployer.DeploySprocket();
}

The dependency injection framework would then inject the tenant ID as a dependency by looking at the currently authenticated user. Is this a good idea or just an abuse of dependency injection?

It further occurred to me that many implementations also take additional dependencies just for looking up details about the tenant, and that I could reduce the number of dependencies further by just injecting in that detail directly, for example

class SprocketDeployer
{
    public SprocketDeployer(ITenantRepository tenantRepository)
    {
        _tenantRepository = tenantRepository;
    }

    void DeploySprocket(int tenantId)
    {
        var tenantName = _tenantRepository.GetTenant(tenantId).Name;
        // Do stuff with tenantName
    }
}

Would become

class SprocketDeployer
{
    public SprocketDeployer(Tenant tenant)
    {
        _tenant = tenant;
    }

    void DeploySprocket()
    {
        var tenantName = _tenant.Name;
        // Do stuff with tenantName
    }
}

I then realised that I could also inject in other "dependencies", such as details about the currently logged in user in the same way.

At that point I become unsure. While it seemed like a fantastic idea at first I realised that I wasn't sure when to stop adding extra "dependencies". How do I decide what should be a dependency and what should be a parameter?

Answer 1

I would stop short of calling it abuse, but that said:

The general use case of dependency injection (via a container) is to inject pure services that do not directly represent state. One of the immediate problems is informing the container of which instance of your object it should be injecting at run-time. If your SprocketDeployer requires a Tenant, and your system includes many Tenants, how does the container figure out which tenant to supply at runtime?

If you want to avoid passing Tenant around, consider using Thread Local Storage (TLS). However, there will still be some point in the pipeline where the Tenant needs to be added to TLS.

Edit

From your comment:

I solve the problem of figuring out which tenant to supply at runtime in Ninject by binding the type to a method which examines HttpContext.Current and using InRequestScope. It works fine, but I've not seen anything to indicate that this is (or isn't) a recommended practice.

If I understand you correctly, that sounds like a factory of sorts? If that's the case, I see nothing wrong with it.

A minor nitpick might be: it's nice to be able to not have to be concerned about how your services are scoped. When they are truly stateless services, you can view them as pure swappable components that have no side effects based on container configuration.

Answer 2

As with Phil, I would not call this dependency injection abuse, though it does feel a bit odd.

You have at least a few options. I'll detail a couple that seem the best from the detail you've provided, though these may have been what you were referring to when you said 'I then realised that I could also inject in other "dependencies", such as details about the currently logged in user in the same way.'

Option 1: Abstract tenant identification to a factory

It may make perfect sense to have an abstraction that represents the current tenant. This abstraction is a factory, but I prefer the term "provider" because factory connotes creation whereas a provider may simply retrieve an existing object (Note: I realize Microsoft introduced a provider pattern but that's not what I'm referring to). In this context you're not injecting data, instead you're injecting a service. I'd probably call it ICurrentTenantProvider. The implementation is frequently context specific. Right now, for example, it would come from your HttpContext object. But, you could decide a specific customer needed their own server and then inject an ICurrentTenantProvider that would retrieve it from your web.config file.

Option 2: Hide multitenancy entirely

Unless you ever have to do different things based on the tenant[1], it may be better to hide the multitenancy entirely. In this case you'd inject classes, that I'm going to call providers, that are context aware and the result of whose function calls would be based on the current tenant. For example, you might have an ICssProvider and an IImageProvider. These providers alone would be aware that the application supported multitenancy. They may use another abstraction such as the ICurrentTenantProvider referenced above or may use the HttpContxt directly. Regardless of the implementation, they would return context specific to the tenant.

In both cases, I'd recommend injecting a service instead of data. The service provides an abstraction layer and allows you to inject an implementation that's appropriately context aware.

Making the Decision

How do I decide what should be a dependency and what should be a parameter?

I generally only ever inject services and avoid injecting things like value objects. To decide you might ask yourself some questions:

1. Would it make sense to register this object type (e.g., int tenantId) in the IoC container?
2. Is this object/type consistent for the standard lifetime of the application (e.g., instance per http request), or does it change?
3. Will most objects end up dependent on this particular object/type?
4. Would this object need to be passed around a lot if made a parameter?

For (1), it doesn't make sense to inject value objects. For (2), if it is consistent, as the tenant would be, it may be better to inject a service that's aware of the tenant. If yes to (3), it may indicate a missing abstraction. If yes to (4) you may again be missing an abstraction.

In the vein of (3) and (4) and depending on the details of the application, I could see ICurrentTenantProvider being injected in a lot of places, which may indicate it's a little low level. At that point the ICssProvider or similar abstractions may be useful.

[1] - If you inject data, like an int, you're forced to query and you may end up in a situation where you'd want to replace conditional with polymorphism.