I use a self-made "external" library to get some basic infrastructure in place when creating new web-applications. I recently made some changes to how my repositories work and came across this warning from Simple Injector Diagnostic Warnings:
SimpleInjector.DiagnosticVerificationException : The configuration is invalid. The following diagnostic warnings were reported: -[Torn Lifestyle] The registration for IUnitOfWork maps to the same implementation and lifestyle as the registrations for IUnitOfWork, IUnitOfWork, IUnitOfWork and IUnitOfWork do. They all map to EmptyUnitOfWork (Singleton). This will cause each registration to resolve to a different instance: each registration will have its own instance. See the Error property for detailed information about the warnings. Please see https://simpleinjector.org/diagnostics how to fix problems and how to suppress individual warnings.
The core assembly of my library has an empty implementation of IUnitOfWork called EmptyUnitOfWork which is just a simple no-op class:
internal sealed class EmptyUnitOfWork : IUnitOfWork
{
public void SaveChanges()
{
// Do nothing
}
}
This class is then registered in the container when there is no other unit of work avaliable. I do this by using the container.ResolveUnregisteredType(...) like so:
// Register an EmptyUnitOfWork to be returned when a IUnitOfWork is requested:
container.ResolveUnregisteredType += (sender, e) =>
{
if (e.UnregisteredServiceType == typeof(IUnitOfWork))
{
// Register the instance as singleton.
var registration = Lifestyle.Singleton.CreateRegistration<IUnitOfWork, EmptyUnitOfWork>(container);
e.Register(registration);
}
};
First time I've used above method - but works fine with this test:
[Fact]
public void RegistersEmptyUnitOfWork_AsSingleton_WhenIUnitOfWorkIsNotRegisted()
{
var instance = _container.GetInstance<IUnitOfWork>();
var registration = _container.GetRegistration(typeof (IUnitOfWork));
Assert.NotNull(instance);
Assert.IsType<EmptyUnitOfWork>(instance);
Assert.Equal(Lifestyle.Singleton, registration.Lifestyle);
}
Now for the fun part, I have an extension method in a supporting library which registers an EntityFramework IUnitOfWork if my application needs one:
public static void RegisterEntityFramework<TContext>(this Container container) where TContext : DbContext
{
if (container == null)
throw new ArgumentNullException(nameof(container));
var lifestyle = Lifestyle.CreateHybrid(() =>
HttpContext.Current != null,
new WebRequestLifestyle(),
new LifetimeScopeLifestyle()
);
container.Register<DbContext, TContext>(lifestyle);
container.Register<IUnitOfWork, EntityFrameworkUnitOfWork>(lifestyle);
container.Register(typeof (IRepository<>), typeof (EntityFrameworkRepository<>), lifestyle);
}
But somehow this throws the warning from Simple Injector - but I just injected the EntityFrameworkUnitOfWork so the EmptyUnitOfWork should not be triggered?
The reason for this design is that I have a CommandTransactionDecorator in my core library which uses the IUnitOfWork to save changes. I just want to have an empty one if a IUnitOfWork is not required by the application.
For reference this is the decorator:
internal sealed class CommandTransactionDecorator<TCommand> : IHandleCommand<TCommand> where TCommand : ICommand
{
private readonly IUnitOfWork _unitOfWork;
private readonly Func<IHandleCommand<TCommand>> _handlerFactory;
public CommandTransactionDecorator(IUnitOfWork unitOfWork, Func<IHandleCommand<TCommand>> handlerFactory)
{
_unitOfWork = unitOfWork;
_handlerFactory = handlerFactory;
}
public void Handle(TCommand command)
{
_handlerFactory().Handle(command);
_unitOfWork.SaveChanges();
}
}
UPDATE
It seems like this is the registration that makes the warning:
var registration = Lifestyle.Singleton.CreateRegistration<IUnitOfWork, EmptyUnitOfWork>(container);
e.Register(registration);
Changing it to e.Register(() => new EmptyUnitOfWork()); makes the warning go away, but then the lifestyle is not singleton?
What you are seeing is that ResolveUnregisteredType is called multiple times. This causes multiple singleton registrations for the same type to be made. Each registration gets its own instance. This will result in an application that consists of multiple instances of that type, which is usually not what you want to happen when you register a type as singleton. Since your EmptyUnitOfWork doesn't have any behavior, there is probably no problem, but Simple Injector can obviously not guess that this is the case, so it throws an exception.
What you are experiencing however is a breaking change / bug that was introduced in Simple Injector v3. In Simple Injector v1 and v2 the resulting registration of a ResolveUnregisteredType was cached; which meant that a call to Verify() would trigger your custom delegate just once. In Simple Injector v3.0 however, the resulting registration isn't cached anymore. This was an oversight, that has slipped through. The idea was to make ResolveUnregisteredType context aware. To be context aware, caching was not an option anymore. So caching was removed, but we eventually decided not to make ResolveUnregisteredType context aware, while we forgot to add the caching again.
The funny thing of this accidental behavior however is, that it expose a bug in your code. This bug already existed even when you used v2, but v3 now (accidentally) slaps you in the face with it. With v2, the correctness of your registration depended on the use of the Verify() method. Verify() builds all object graphs on a single thread. Without the use of Verify() however, the object graphs are compiled lazily and in case you are running a multi-threaded application, multiple threads can simultaneously call ResolveUnregisteredType; Simple Injector never locked ResolveUnregisteredType and this is documented.
So the result of this is that without a call to Verify() you could still end up in multiple registrations of that specific component, which of course again could lead to really ugly hard to find problems, that usually only appear once in a way in production.
This is how you should actually write that registration:
Lazy<Registration> registration = new Lazy<Registration>(() =>
Lifestyle.Singleton.CreateRegistration<IUnitOfWork, EmptyUnitOfWork>(container));
container.ResolveUnregisteredType += (sender, e) => {
if (e.UnregisteredServiceType == typeof(IUnitOfWork)) {
e.Register(registration.Value);
}
};
With Simple Injector v3 however, you hardly ever have to use the ResolveUnregisteredType event anymore. You can do make the following registration instead:
container.RegisterConditional<IUnitOfWork, EntityFrameworkUnitOfWork>(Lifestyle.Scoped,
c => true);
// NOTE: This registration must be made second
container.RegisterConditional<IUnitOfWork, EmptyUnitOfWork>(Lifestyle.Singleton,
c => !c.Handled);
This solves the problem of having to think about multi-threading completely. Here we make two conditional registrations, where the first is always applied (using the predicate c => true). You might be tempted to make the first registration unconditional using Register<IUnitOfWork, EFUoW>(), but that won't work, because Simple Injector will detect that the second registration can never be applied and an exception will be thrown. So the use of the c => true predicate suppresses this detection. I wouldn't usually advice such construct, because it blinds Simple Injector. In your case however it seems reasonable, because both registrations are made at different moment.
I now will have to think about whether or not I want to change this behavior in v3 and do caching. Advantage of caching is that it can improve performance, but the downside is that it hides bugs.