simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 155 forks source link

Combining abstract factories with dependencies #188

Closed zedr0n closed 8 years ago

zedr0n commented 8 years ago

I'm trying to figure out a way to handle creating class instances with dependencies which can only be resolved at runtime and are also transient registrations. An example I have is

interface ILoginService {
    IDocument DoLogin();
}

interface ILoginServiceFactory {
    ILoginService Create(IDocument document);
}

public class LoginServiceFactory : ILoginServiceFactory {
    ILoginService Create(IDocument document) {
        return new LoginService(document);
    }  
}

interface IConnectionService {
    IDocument GetDocument(string url);
}

public class AuthenticatedConnectionService : ConnectionService {
    private readonly IConnectionService _connectionService;
    private readonly ILoginServiceFactory _factory;

    public IDocument GetDocument(string url) {
        var document = _connectionService.GetDocument(url);
        var loginService = _factory.Create(document);
        return loginService.DoLogin();
    }

    AuthenticatedConnectionService(IConnectionService connectionService, 
        ILoginServiceFactory factory) {
        _factory = factory;
        _connectionService = connectionService;
    }
}

Here I am using an abstract factory registration to create the object with a runtime dependency. The problem with this approach is that I have explicit dependency on a constructor of ILoginService. For simple cases I could move the parameter to property and initialise explicitly in the factory

public class LoginServiceFactory : ILoginServiceFactory {
    private readonly Container _container;

    ILoginService Create(IDocument document) {
        var loginService = _container.GetInstance<ILoginService>();
        loginService.Document = document;
    }

    LoginServiceFactory(Container container) {
        _container = container;
    }
}

But in my case ILoginService has properties injected which depend on IDocument so what I really need is to register the IDocument instance in the container so it can resolve all dependencies correctly. The reason I need to do this is that ILoginService has properties which depend on both IDocument instance plus primitives which are resolved from attributes implementing an aspect driven design:

public class LoginService : ILoginService {
    [Selector(".ul")]
    private IPageItem _list { get; set; } 
}

public class PageItem : IPageItem {
    PageItem(IDocument document, string selector) { }
}

The only way I could figure out how to do it is by using a temporary container for each resolution of the type:

public class LoginServiceFactory : ILoginServiceFactory {
    private readonly Container _innerContainer;

    ILoginService Create(IDocument document) {
        _innerContainer.Register(() => document);
        var loginService = _innerContainer.GetInstance<ILoginService>();
        return loginService;
    }

    LoginServiceFactory(Container container) {
        _innerContainer = new Container();
        foreach(var producer in container.GetAllRegistrations()) {
            _innerContainer.AddRegistration(producer.ServiceType, producer.Registration);
        }
    }
}

But that really seems quite ugly. I've read through all the docs and especially the design concepts/guidelines but I still can't figure out what would be a valid way to combine runtime dependencies with static dependencies. Any hints?

dotnetjunkie commented 8 years ago

The root of your problems is that you wish to inject runtime data (the document) into your components. This is an anti-pattern, as described here.

zedr0n commented 8 years ago

I see, if I move the runtime data to a method then I won't be able to inject properties which means I will have to manually initialise IPageItem avoiding which was the primary goal of using dependency injection. Now if I move the runtime behind an abstraction ( e.g. IDocumentProvider ) I will still have to somehow configure the IDocumentProvider to give me an instance I want so now we are back to square one where IDocumentProvider itself will depend on some runtime data which defines which document to create.

interface IDocumentProvider {
    IDocument Document { get; }
}

public class DocumentProvider : IDocumentProvider {
    IConnectionStack _connectionStack;
    private string _url;
    IDocument Document => _connectionStack[url];
}

That doesn't really help much as while in your examples the runtime data is in a sense a singleton here we cannot abstract runtime data by a service. So while I agree with what you've written in the blog post and the unit testing limitation specifically I would still want to be able to define a construction graph not fully resolvable statically. Isn't this why we can add factory delegates, Func<T> and register unresolved types in the first place?

qujck commented 8 years ago

What sort of app is it Mvc/WebApi/?

zedr0n commented 8 years ago

UWP app ( GUI is based on MVC / https://github.com/Windows-XAML/Template10) which is basically a wrapper around a website but I use https://github.com/AngleSharp/AngleSharp for connecting and parsing html elements. That's why I'm using dependency injection, I want the app classes to be descriptive of web pages, e.g. I have something like

[Url("http://")]
public class Profile
{
    [Selector("div.page-title")]
    public readonly IWebString Name;
    [Selector("div.page__lede")]
    public readonly IWebNumber Age;
}

Simple Injector is used to process the attributes and inject them as primitives to the actual parameters of underlying constructors. This should allow for very quick scaffolding and is primarily a framework I'm creating.

zedr0n commented 8 years ago

Now that I think about it there might be another way to handle this abstracting out IDocument using execution context scope

    public interface IDocumentProvider
    {
        IDocument Document { get; set; }
    }

    public class AuthenticatedConnectionService : IConnectionService
    {
        private readonly IHttpConnectionService _connectionService;
        private readonly Container _container;

        public async Task<IDocument> GetDocument(string url, bool alwaysReload = false)
        {
            var document = await _connectionService.GetDocument(url, alwaysReload);

            if (document == null)
                return null;

            using (_container.BeginExecutionContextScope())
            {
                var documentProvider = _container.GetInstance<IDocumentProvider>();
                documentProvider.Document = document;

                var loginService = _container.GetInstance<ILoginService>();

                // replace the result in the connection stack with the authentication result
                return await _connectionService.ConnectionStack
                    .GetConnection(url, loginService.DoLogin(), true);
            }
        }

Effectively the logjn service will extract the document only per particular connection requested. Does that make sense?

dotnetjunkie commented 8 years ago

Why can't the IDocumentProvider implementation itself call _connectionService.GetDocument? By setting the document after the provider is constructed, you cause temporal coupling and make it mutable. Perhaps it's my unfamiliarity with the technology, but what is this document a d why must it be get/set using a property. Why can't you pass it along with method calls? What is so special about this document object?

zedr0n commented 8 years ago

IDocument is just a parsed web page ( and the associated web request internally ), I can have IDocumentProvider get it again but that would require passing somehow the url string to the document provider which is also runtime data or relying on connection service knowing the last document retrieved ( but that's not really ideal considering that IConnectionService is a singleton and there might be multiple connections going on at the same time on different threads ) which would still have temporal coupling.

Also I need the same Idocument instance as I would be setting input fields and clicking a button on that instance so I would have to make IDocumentProvider a singleton but what happens then if at the same time a different thread does another connection requesting Idocument...

The problem with passing along with method calls is that I need an IDocument instance at login service construction to inject properties:

    public class DocumentLogin : DocumentLoginService
    {
        [Selector("input#email")]
        protected override IPageInputElement Username { get; }
        [Selector("input#password")]
        protected override IPageInputElement Password { get; }
        [Selector("button.btn")]
        protected override IPageButtonElement Button { get; }
    }

Here IPageInputElement is constructed by IoC by using IDocumentProvider instance and the attribute selector string using primitives injection as in your article. Now I could theoretically move Idocument from constructor into method calls on IPageElements but that means I would have to effectively add some Init(IDocument) logic to every single method and I thought that was an anti-pattern as well.

So it's really the fact that for login service Idocument is not a method parameter but a constructor parameter.

zedr0n commented 8 years ago

So in the end I've subverted scoping for something generic like this

public class Provider<T>
{
    public T Value { get; set; }
}

public class ScopeFactory<TInput,TService> : IScopeFactory<TInput,TService> where TService : class
{
    public IDisposable BeginScope() => _container.BeginExecutionContextScope();
    private readonly Container _container;

    public TService Create(TInput input)
    {
        if(_container.GetCurrentExecutionContextScope() == null)
            throw new ActivationException();

        var provider = _container.GetInstance<Provider<TInput>>();
        provider.Value = input;

        return _container.GetInstance<TService>();
    }

    public ScopeFactory(Container container)
    {
        _container = container;
    } 
}

which can then be used using scope

public async Task<IDocument> GetDocument(string url, bool alwaysReload = false)
{
    var document = await _connectionService.GetDocument(url, alwaysReload);

    if (document == null)
        return null;

    using (_loginServiceFactory.BeginScope())
    {
        var loginService = _loginServiceFactory.Create(document);

        // replace the result in the connection stack with the authentication result
        return await _connectionService.ConnectionStack.GetConnection(url, 
            loginService.DoLogin(), true);
    }
}