Skip to content

Commit

Permalink
Container.Update marked obsolete. Containers should be considered imm…
Browse files Browse the repository at this point in the history
…utable.
  • Loading branch information
Travis Illig committed Nov 14, 2016
1 parent bbea0e0 commit 8a89e94
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 4 deletions.
18 changes: 18 additions & 0 deletions src/Autofac/ContainerBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ private static void StartStartableComponents(IComponentContext componentContext)
/// </remarks>
/// <param name="container">An existing container to make the registrations in.</param>
[SuppressMessage("Microsoft.Design", "CA1011:ConsiderPassingBaseTypesAsParameters", Justification = "You can't update any arbitrary context, only containers.")]
[Obsolete("Containers should generally be considered immutable. Register all of your dependencies before building/resolving. If you need to change the contents of a container, you technically should rebuild the container. This method may be removed in a future major release.")]

This comment has been minimized.

Copy link
@osoykan

osoykan Nov 15, 2016

I really need Update the container on my project. How can i handle this, i mean, how rebuild works ?

This comment has been minimized.

Copy link
@MpDzik

MpDzik Nov 15, 2016

I agree, I also rely on this feature from time to time. While I understand that updating an existing container is not considered best practice, however, having this feature is really helpful, especially when integrating Autofac with legacy application which were not necessarily build with dependency injection in mind.

This comment has been minimized.

Copy link
@tillig

tillig Nov 15, 2016

Member

This is good feedback - we'd love to hear more specifics about what the technical requirement is and, for example, why passing a ContainerBuilder around isn't possible. I've started a discussion issue here as a place to explain what your needs are to see if we can help you out and get you what you need while at the same time removing Update in a future (year-or-more in the future) release.

This comment has been minimized.

Copy link
@logiclrd

logiclrd Jan 17, 2017

Another place this is used is with Nancy.Bootstrappers.Autofac. This package provides a NancyFx bootstrapper that sets up an Autofac container, and has a virtual method ConfigureApplicationContainer that receives an ILifetimeScope. I don't know the internal architecture, so I don't know if it is as simple as deferring the initial creation of the container and passing a ContainerBuilder instead of an ILifetimeScope, but even if it is, it will still be a breaking change to every single Nancy web app built on Autofac.

This comment has been minimized.

Copy link
@tillig

tillig Jan 17, 2017

Member

We're aware of the Nancy issue as are the maintainers of that project. It would be good to follow up with them there.

For further discussion on this - please do it in the discussion issue and not on this code line. It will help to avoid covering different topics in different places and ensure we don't lose discussion points. Thanks!

public void Update(IContainer container)
{
Update(container, ContainerBuildOptions.None);
Expand All @@ -188,6 +189,7 @@ public void Update(IContainer container)
/// <param name="container">An existing container to make the registrations in.</param>
/// <param name="options">Options that influence the way the container is updated.</param>
[SuppressMessage("Microsoft.Design", "CA1011:ConsiderPassingBaseTypesAsParameters", Justification = "You can't update any arbitrary context, only containers.")]
[Obsolete("Containers should generally be considered immutable. Register all of your dependencies before building/resolving. If you need to change the contents of a container, you technically should rebuild the container. This method may be removed in a future major release.")]
public void Update(IContainer container, ContainerBuildOptions options)
{
// Issue #462: The ContainerBuildOptions parameter is added here as an overload
Expand All @@ -208,7 +210,23 @@ public void Update(IContainer container, ContainerBuildOptions options)
/// - this prevents ownership issues for provided instances.
/// </remarks>
/// <param name="componentRegistry">An existing registry to make the registrations in.</param>
[Obsolete("Containers should generally be considered immutable. Register all of your dependencies before building/resolving. If you need to change the contents of a container, you technically should rebuild the container. This method may be removed in a future major release.")]
public void Update(IComponentRegistry componentRegistry)
{
this.UpdateRegistry(componentRegistry);
}

/// <summary>
/// Configure an existing registry with the component registrations
/// that have been made. Primarily useful in dynamically adding registrations
/// to a child lifetime scope.
/// </summary>
/// <remarks>
/// Update can only be called once per <see cref="ContainerBuilder"/>
/// - this prevents ownership issues for provided instances.
/// </remarks>
/// <param name="componentRegistry">An existing registry to make the registrations in.</param>
internal void UpdateRegistry(IComponentRegistry componentRegistry)
{
if (componentRegistry == null) throw new ArgumentNullException(nameof(componentRegistry));
Build(componentRegistry, true);
Expand Down
2 changes: 1 addition & 1 deletion src/Autofac/Core/Lifetime/LifetimeScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private ScopeRestrictedRegistry CreateScopeRestrictedRegistry(object tag, Action
configurationAction(builder);

var locals = new ScopeRestrictedRegistry(tag, builder.Properties);
builder.Update(locals);
builder.UpdateRegistry(locals);
return locals;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Autofac/Module.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void Configure(IComponentRegistry componentRegistry)
var moduleBuilder = new ContainerBuilder(componentRegistry.Properties);

Load(moduleBuilder);
moduleBuilder.Update(componentRegistry);
moduleBuilder.UpdateRegistry(componentRegistry);
AttachToRegistrations(componentRegistry);
AttachToSources(componentRegistry);
}
Expand Down
6 changes: 6 additions & 0 deletions test/Autofac.Test/ContainerBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,9 @@ public void WhenUpdating_DefaultModulesAreExcluded()
{
var builder = new ContainerBuilder();
var container = new Container();
#pragma warning disable CS0618
builder.Update(container);
#pragma warning restore CS0618
Assert.False(container.IsRegistered<IEnumerable<object>>());
}

Expand Down Expand Up @@ -445,7 +447,9 @@ public void WhenTheContainerIsUpdated_ExistingStartableComponentsAreNotReStarted

var builder2 = new ContainerBuilder();
builder2.RegisterInstance(startable2).As<IStartable>();
#pragma warning disable CS0618
builder2.Update(container);
#pragma warning restore CS0618

Assert.Equal(1, startable1.StartCount);
Assert.Equal(1, startable2.StartCount);
Expand All @@ -461,7 +465,9 @@ public void WhenTheContainerIsUpdated_NewStartableComponentsAreStarted()

var builder = new ContainerBuilder();
builder.RegisterInstance(startable).As<IStartable>();
#pragma warning disable CS0618
builder.Update(container);
#pragma warning restore CS0618

Assert.Equal(1, startable.StartCount);
}
Expand Down
2 changes: 1 addition & 1 deletion test/Autofac.Test/Core/Lifetime/LifetimeScopeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ internal void UpdateRegistry(object instance)
{
var builder = new ContainerBuilder();
builder.RegisterInstance(instance);
builder.Update(_registerContext.ComponentRegistry);
builder.UpdateRegistry(_registerContext.ComponentRegistry);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void RegistrationsMadeByUpdatingAChildScopeDoNotAppearInTheParentScope()
var childScope = container.BeginLifetimeScope();
var updater = new ContainerBuilder();
updater.RegisterType<object>();
updater.Update(childScope.ComponentRegistry);
updater.UpdateRegistry(childScope.ComponentRegistry);
Assert.True(childScope.IsRegistered<object>());
Assert.False(container.IsRegistered<object>());
}
Expand Down
4 changes: 4 additions & 0 deletions test/Autofac.Test/RegistrationExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ public void AutoActivate_ContainerUpdateAutoActivatesNewComponents()
var container = new ContainerBuilder().Build();
var builder = new ContainerBuilder();
builder.RegisterType<MyComponent2>().AutoActivate().OnActivated(e => instanceCount++);
#pragma warning disable CS0618
builder.Update(container);
#pragma warning restore CS0618
Assert.Equal(1, instanceCount);
}

Expand All @@ -245,7 +247,9 @@ public void AutoActivate_ContainerUpdateDoesNotAutoActivateExistingComponents()
int secondCount = 0;
var builder2 = new ContainerBuilder();
builder2.RegisterType<MyComponent>().AutoActivate().OnActivated(e => secondCount++);
#pragma warning disable CS0618
builder2.Update(container);
#pragma warning restore CS0618
Assert.Equal(1, firstCount);
Assert.Equal(1, secondCount);
}
Expand Down

0 comments on commit 8a89e94

Please sign in to comment.