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

Add support for using activators to resolve TContext #170

Merged

Conversation

mgoodfellow
Copy link
Contributor

This PR adds support for more complex use cases (#166) while maintaining backwards compatible support for existing/simple implementations.

New implementations can be added on a per IoC basis. For example the code for SimpleInjector would be:

public class SimpleInjectorCacheContextActivator<TContext> : ICacheContextActivator
	{
		private readonly Container Container;

		public SimpleInjectorCacheContextActivator(Container container)
		{
			Container = container;
		}

		public ICacheContextScope BeginScope()
		{
			return new SimpleInjectorCacheContextScope(Container, AsyncScopedLifestyle.BeginScope(Container));
		}
	}

With the scope similar to:

public class SimpleInjectorCacheContextScope<TContext> : ICacheContextScope
	{
		private readonly Container Container;
                private readonly Scope Scope;

		public SimpleInjectorCacheContextScope(Container container, Scope scope)
		{
			Resolver = resolver;
                        Scope = scope;
		}

		public object Resolve(Type type)
		{
			return Container.GetInstance(type);
		}

		public void Dispose()
		{
                       Scope?.Dispose();
		}
	}

Open to comments and thoughts - this was my initial implementation!

@Turnerj
Copy link
Member

Turnerj commented May 19, 2021

Hey @mgoodfellow ! First I want to thank you for submitting the PR - sorry I hadn't got to it myself, between being sick, doing a presentation and making sure my business keeps on spinning I just hadn't got around to it.

I do very much like this change for the reasons you highlighted in your original issue about this. Its kinda late where I am so I'll take another look in the morning but I don't see any obvious issues.

When I merge this, it might be a little bit longer before I release a new version as I need to re-familiarise myself with some serialization changes that are in the main branch. I want to make sure I've got everything done for it that is needed rather than release a half-implemented version. Hopefully that is OK!

@mgoodfellow
Copy link
Contributor Author

No worries at all! I had been meaning to do this as well and its been on my todo list since we last spoke, always happy to contribute back to projects as we use them internally :) I finally understand background refreshing, so that renewed my interest in adding this functionality.

No rush, speak soon!

@Turnerj Turnerj merged commit 3c8a9fa into TurnerSoftware:master May 21, 2021
@Turnerj
Copy link
Member

Turnerj commented May 21, 2021

All looks good - thanks again for the PR!

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

Successfully merging this pull request may close these issues.

None yet

2 participants