Feature/985 lifetime scopes and build callbacks

[weelink]

No description provided.

[weelink]

I have extracted all but the BuildCallback into LifetimeScopeBuilder and changed the references from ContainerBuilder to LifetimeScopeBuilder.

[alistairjevans]

This PR will remove the build callback methods from the lifetime scope builder parameter rather than actually supporting build callbacks on the lifetime scope creation event. Is that the way we want this to go? I saw that @tillig said it was by design on the issue thread to not call those callbacks, so maybe. I see the argument for wanting build callbacks, but the lifetime scope isn't 'built' per se.

I feel like the proposed module changes do put a significant responsibility on the developer to know which derived class to use (as you said in the raised issue).
I believe there will also be a significant documentation and integration impact since the extension method this type for a lot of methods will be changing because ContainerBuilder is no longer the base class.

If the aim is to tell developers their callbacks won't work, might it be simpler just to add an internal derived class for ContainerBuilder, something like NoCallbackContainerBuilder, that throws a descriptive NotSupportedException when someone tries to call RegisterBuildCallback on it? We could update the LifetimeScope.CreateScopeRestrictedRegistry to use that new type? Just a thought.

If the aim should be to actually support build callbacks for lifetime scopes, then we're going to need to go down a different path.

[weelink]

The problem is that the callback is for a IContainer and not for a ILifetimeScope. So when someone adds registrations when beginning a new lifetime scope, it is incorrect to pass in the IContainer. The new registrations aren't in that container.

Personally I don't like throwing exceptions because of a wrong API usage. This is what the compiler can do for you. I much rather have a compile time error than a runtime one. So either it should be impossible to add a build callback when starting a new lifetime scope, or have the build callback take a parameter of type ILifetimeScope instead. In that last case the question arises what the point of IContainer is going to be. It is just a ILifetimeScope. However that will drastically change Autofac.

That being said, this PR is mostly to see what it will take to actually fix this. There seems to be some underlying design issue with how Autofac works. I'm not entirely happy with the result. It is a lot of changes for a relatively simple problem. But I can't see a better one at the moment.

[alistairjevans]

Ah, yes, sorry. I forgot that the callback takes the container rather than the scope.

I agree with you about preferring compile-time errors over exceptions, I'm just trying to balance change complexity with what we're trying to achieve.

Hypothetically, if we switched the build callback to take an ILifetimeScope (even on container build), that will work; should only break those that use that callback with an explicit type. However, the list of callbacks is stored in a Properties object that is propagated down from the main registry, so it could accidentally re-run previously registered build callbacks; the lifetime scope ContainerBuilder would need to isolate its own callbacks, perhaps with an additional internal constructor.

The ContainerBuilder would then need a new internal BuildLifetimeScope method (or something), and build the scope inside there (as well as building the scoped registry) then call the build callbacks. Those are significant changes, but they are at least mostly internal rather than externally breaking.

[tillig]

Fixed by #1054.