Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve dependencies requiring an implementation created by a parameter factory #477

Closed
BrutalSimplicity opened this issue Feb 21, 2019 · 8 comments

Comments

@BrutalSimplicity
Copy link

BrutalSimplicity commented Feb 21, 2019

Hi there!! I'm trying to use LightInject to resolve the dependencies of implementations that are being resolved on a per request basis. One of the dependencies is a request context that nearly every object in the dependency graph must use, but it also is created upon request based on the parameters passed into the method.

Here is an example of the setup.

void Main()
{
	var container = new ServiceContainer();
	container.Register<int, Context>((factory, @param) =>
	{
		return new Context { Data = @param };
	});
	container.Register<IRepository, Repository>(new PerScopeLifetime());
	container.Register<IService, Service>(new PerScopeLifetime());
	
	using (container.BeginScope())
	{
		// Here, I'd like the "Context" to be resolved and passed to the
		// rest of the dependencies
		var service = container.GetInstance<int, Service>(3);
	}
}

// Define other methods and classes here
public interface IService
{
	void DoSomeStuff();
}

public interface IRepository
{
	int Get();
	void Set(int i);
}

public class Service : IService
{
	IRepository _repository;
	
	public Service(
		Context context,
		IRepository repository)
	{
		_repository = repository;
	}
	
	public void DoSomeStuff()
	{
		_repository.Get();	
		_repository.Set(10);
	}
}

public class Repository : IRepository
{
	Context _context;
	
	public Repository(Context context)
	{
		_context = context;
	}
	
	public int Get()
	{
		return _context.Data;
	}

	public void Set(int i)
	{
		_context.Data = i;
	}
}

public class Context
{
	public int Data { get; set; }
}

The idea here is that the "context" object is passed around to all of the dependencies, but is scoped and resolved dynamically via each request. I thought the idea here, #237, was a fairly similar situation, but I'm not sure it solves my particular problem.

Is there a way to do this in LightInject?

@BrutalSimplicity BrutalSimplicity changed the title Resolve implementations of dependencies resolved through parameter factories Resolve dependencies requiring an implementation created by a parameter factory Feb 22, 2019
@BrutalSimplicity
Copy link
Author

BrutalSimplicity commented Feb 25, 2019

So, I've found a way of handling this that works around this issue. Still wondering if there is a way to do this in LightInject.

async Task Main()
{
	var container = new ServiceContainer();
	container.Register<IRepository, Repository>(new PerScopeLifetime());
	container.Register<IService, Service>(new PerScopeLifetime());
	container.Register<IContext, Context>(new PerScopeLifetime());
	container.Register<IContextFactory, ContextFactory>(new PerScopeLifetime());

	// excuse this grossness. Just trying to test scope and concurrency
	var values = await Task.WhenAll(Enumerable.Range(0, 1000).Select(i => new Func<Task<int>>(() =>
	{
		using (var scope = container.BeginScope())
		{
			var contextFactory = (ContextFactory)container.GetInstance<IContextFactory>();
			contextFactory.SetContext(new Context { Data = i });
			
			var service = scope.GetInstance<IService>();
			return Task.FromResult(service.DoSomeStuff());
		}
	})()));
	
	values.Dump();
}

// Define other methods and classes here
public interface IService
{
	int DoSomeStuff();
}

public interface IRepository
{
	int Get();
	void Set(int i);
}

public interface IContext
{
	int Data { get; set; }
}

public class Service : IService
{
	IRepository _repository;
	IContext _context;
	
	public Service(
		IContextFactory contextFactory,
		IRepository repository)
	{
		_context = contextFactory.GetContext();
		_repository = repository;
	}
	
	public int DoSomeStuff()
	{
		return _repository.Get();
	}
}

public class Repository : IRepository
{
	IContext _context;
	
	public Repository(IContextFactory contextFactory)
	{
		_context = contextFactory.GetContext();
	}
	
	public int Get()
	{
		return _context.Data;
	}

	public void Set(int i)
	{
		_context.Data = i;
	}
}

public class Context : IContext
{
	public int Data { get; set; }
}

public interface IContextFactory
{
	IContext GetContext();
}

public class ContextFactory : IContextFactory
{
	IContext _context;
	
	public void SetContext(Context context)
	{
		_context = context; 
	}
	
	public IContext GetContext()
	{
		return _context;	
	}
}

@seesharper
Copy link
Owner

I think you should stick with the workaround since mixing runtime data with services is sort of an anti-pattern. Take a look at this comment for more information about the topic.
#435 (comment)

@BrutalSimplicity
Copy link
Author

Thank you for that insight, I think everything was starting to look like a nail that DI could resolve. The post from Steven van Deursen, made me think a bit more about it, and I'm still a bit on the fence as to whether I should refactor to pass this data at runtime.

Basically, in my situation, I'm looking to refactor some code that is using the context to mostly pass around the state to every method, and I'd like to avoid every method having to have this context argument passed into it. It would seem from the post that he suggests using some kind of context provider to encapsulate the concrete implementation of the state, which is kind of what I was going for here, but I'm not sure if his intent was that you should just pass around the entire context, or only expose the information the client will need. Unfortunately, it seems that most of the information in the context object is being used by the clients.

I'm curious whether you think it would be better to add the context as a parameter to a method, or whether using a context provider like above would be more appropriate?

@seesharper
Copy link
Owner

seesharper commented Feb 26, 2019

Well, that depends 😀One example from AspNet.Core is the IHttpContextAccessor that provides access to the current HttpContext from outside your controllers.

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-context?view=aspnetcore-2.2

Personally, I am not a big fan of passing "contexts" around as methods parameters. I prefer them to be ambient and that I can inject them where they are needed, just like with the IHttpContextAccessor

Does that make any sense ? 😀

If you need to "capture" the context and have it flow across async/await points, you can do that by storing the context in an AsyncLocal<T>

@BrutalSimplicity
Copy link
Author

BrutalSimplicity commented Feb 27, 2019

It does make sense, and I do prefer the idea of passing in the context through the constructor via an interface, as opposed to methods, but wasn't sure whether I'd be shooting myself in the foot somehow. Definitely seems practical in this case.

To your second point about capturing the context in AsyncLocal, could you expand on that a bit more? I don't think in my case this would be necessary since the entire dependency graph is instantiated before the data flow, but it does make me wonder about one case in particular.

There is a portion of my code that needs to "fire and forget" a task at the end of the request flow. A snippet of the idea is below.

async Task Main()
{
	var container = new ServiceContainer();
	container.Register<IRepository, Repository>(new PerScopeLifetime());
	container.Register<IService, Service>(new PerScopeLifetime());
	container.Register<IContext, Context>(new PerScopeLifetime());
	container.Register<IContextFactory, ContextFactory>(new PerScopeLifetime());
	var bool someSetting = true;

	using (var scope = container.BeginScope())
	{
		var contextFactory = (ContextFactory)container.GetInstance<IContextFactory>();
		contextFactory.SetContext(new Context { Data = i });
		
		var service = scope.GetInstance<IService>();

		// My assumption here, is that even though the dependencies are "disposed"
		// at the end of the using block, the method should still have no problem
		// continuing on another thread since the dependency graph for this
		// object has already been instantiated. In this case, the only dependencies that may be
		// instantiated happen through message dispatch calls, which internally
		// use the container to create a new scope). Am I right about this assumption?
		if (someSetting)
			var fireAndForget = service.DoSomeStuff();

		var value = await service.DoSomeStuff();
	}
	
	values.Dump();
}

// Define other methods and classes here
public interface IService
{
	int DoSomeStuff();
}

public interface IRepository
{
	int Get();
	void Set(int i);
}

public interface IContext
{
	int Data { get; set; }
}

public class Service : IService
{
	IRepository _repository;
	IContext _context;
	
	public Service(
		IContextFactory contextFactory,
		IRepository repository)
	{
		_context = contextFactory.GetContext();
		_repository = repository;
	}
	
	public Task<int> DoSomeStuff()
	{
		return _repository.Get();
	}
}

public class Repository : IRepository
{
	IContext _context;
	
	public Repository(IContextFactory contextFactory)
	{
		_context = contextFactory.GetContext();
	}
	
	public async Task<int> Get()
	{
		await Task.Delay(500);
		return _context.Data;
	}

	public async Task Set(int i)
	{
		await Task.Delay(2000);
		_context.Data = i;
	}
}

public class Context : IContext
{
	public int Data { get; set; }
}

public interface IContextFactory
{
	IContext GetContext();
}

public class ContextFactory : IContextFactory
{
	IContext _context;
	
	public void SetContext(Context context)
	{
		_context = context; 
	}
	
	public IContext GetContext()
	{
		return _context;	
	}
}

So my thoughts are contained in the comment block above. Is this type of thing safe, since the dependency graph has already been created (no lazies or anything resolved on demand)?

@seesharper
Copy link
Owner

This code should be safe. All services are PerScopeLifeTime and are actually backed by an AsyncLocal<T> behind the scenes. So the ContextFactory that you inject are tied to the scope that you have started.

@BrutalSimplicity
Copy link
Author

@seesharper Thanks for the advice!! I think I've got a better understanding of the options available now, so you can close this ticket when ready.

@seesharper
Copy link
Owner

Great 👍👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants