From 282a07a73328a49f9d2094af1d4c89776141adae Mon Sep 17 00:00:00 2001 From: Matej Hrlec Date: Fri, 18 Sep 2020 11:04:29 +0200 Subject: [PATCH 1/8] feature: Update Splat.Autofac to work with Autofac 5.0 (#465) Raises Autofac version to 5+, and deprecates or removes method that mutate the container (since that is supposed to be immutable from now on). Register and HasRegister are to be used only by the ReactiveUI initialization procedure, which modifies the container by creating a child scope for each service registration. Thanks to "skaman" for coming up with the workaround. --- .../AutofacDependencyResolver.cs | 324 +++++------------- src/Splat.Autofac/Splat.Autofac.csproj | 2 +- src/Splat.Autofac/SplatAutofacExtensions.cs | 20 +- 3 files changed, 87 insertions(+), 259 deletions(-) diff --git a/src/Splat.Autofac/AutofacDependencyResolver.cs b/src/Splat.Autofac/AutofacDependencyResolver.cs index 73be503f1..b59978e7a 100644 --- a/src/Splat.Autofac/AutofacDependencyResolver.cs +++ b/src/Splat.Autofac/AutofacDependencyResolver.cs @@ -6,11 +6,9 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Linq; using Autofac; using Autofac.Core; -using Autofac.Core.Registration; #pragma warning disable CS0618 // Obsolete values. @@ -22,15 +20,29 @@ namespace Splat.Autofac public class AutofacDependencyResolver : IDependencyResolver { private readonly object _lockObject = new object(); - private IComponentContext _componentContext; + private readonly ContainerBuilder _builder; + + /// + /// The internal container, which takes care of mutability needed for ReactiveUI initialization procedure. + /// It is disposed of once the user sets the actual lifetime scope from which to resolve by calling SetLifetimeScope. + /// + private IContainer _internalContainer; + + private ILifetimeScope _lifetimeScope; +#pragma warning disable CA2213 // _internalLifetimeScope will be disposed, because it is a child of _internalContainer + private ILifetimeScope _internalLifetimeScope; +#pragma warning restore CA2213 // Disposable fields should be disposed /// /// Initializes a new instance of the class. /// - /// The component context. - public AutofacDependencyResolver(IComponentContext componentContext) + /// Autofac container builder. + public AutofacDependencyResolver(ContainerBuilder containerBuilder) { - _componentContext = componentContext; + _builder = containerBuilder; + + _internalContainer = new ContainerBuilder().Build(); + _internalLifetimeScope = _internalContainer.BeginLifetimeScope(); } /// @@ -42,6 +54,21 @@ public virtual object GetService(Type serviceType, string contract = null) } } + /// + /// Sets the lifetime scope which will be used to resolve ReactiveUI services. + /// It should be set after Autofac application-wide container is built. + /// + /// Lifetime scope, which will be used to resolve ReactiveUI services. + public void SetLifetimeScope(ILifetimeScope lifetimeScope) + { + _lifetimeScope = lifetimeScope; + + // We dispose on the internal container, since it and its many child lifetime scopes are not needed anymore. + _internalContainer.Dispose(); + _internalContainer = null; + _internalLifetimeScope = null; + } + /// public virtual IEnumerable GetServices(Type serviceType, string contract = null) { @@ -65,183 +92,85 @@ public bool HasRegistration(Type serviceType, string contract = null) { lock (_lockObject) { - return _componentContext.ComponentRegistry.Registrations.Any(x => GetWhetherServiceRegistrationMatchesSearch( - x.Services, - serviceType, - contract)); + var lifeTimeScope = _lifetimeScope ?? _internalLifetimeScope; + + return string.IsNullOrEmpty(contract) ? + lifeTimeScope.IsRegistered(serviceType) : + lifeTimeScope.IsRegisteredWithName(contract, serviceType); } } /// - /// Register a function with the resolver which will generate a object - /// for the specified service type. - /// Optionally a contract can be registered which will indicate - /// that that registration will only work with that contract. - /// Most implementations will use a stack based approach to allow for multile items to be registered. + /// Important: Because Autofac 5+ containers are immutable, + /// this method should not be used by the end-user. + /// It is still needed to satisfy ReactiveUI initialization procedure. + /// Register a function with the resolver which will generate a object + /// for the specified service type. + /// Optionally a contract can be registered which will indicate + /// that that registration will only work with that contract. + /// Most implementations will use a stack based approach to allow for multiple items to be registered. /// /// The factory function which generates our object. /// The type which is used for the registration. /// A optional contract value which will indicates to only generate the value if this contract is specified. + [Obsolete("Because Autofac 5+ containers are immutable, this method should not be used by the end-user.")] public virtual void Register(Func factory, Type serviceType, string contract = null) { lock (_lockObject) { - var builder = new ContainerBuilder(); + // We register every ReactiveUI service twice. + // First to the application-wide container, which we are still building. + // Second to child lifetimes in a temporary container, that is used only to satisfy ReactiveUI dependencies. if (string.IsNullOrEmpty(contract)) { - builder.Register(x => factory()).As(serviceType).AsImplementedInterfaces(); + _builder.Register(_ => factory()) + .As(serviceType) + .AsImplementedInterfaces(); + _internalLifetimeScope = _internalLifetimeScope.BeginLifetimeScope(internalBuilder => + internalBuilder.Register(_ => factory()) + .As(serviceType) + .AsImplementedInterfaces()); } else { - builder.Register(x => factory()).Named(contract, serviceType).AsImplementedInterfaces(); + _builder.Register(_ => factory()) + .Named(contract, serviceType) + .AsImplementedInterfaces(); + _internalLifetimeScope = _internalLifetimeScope.BeginLifetimeScope(internalBuilder => + internalBuilder.Register(_ => factory()) + .Named(contract, serviceType) + .AsImplementedInterfaces()); } - - builder.Update(_componentContext.ComponentRegistry); } } /// - /// Unregisters the current item based on the specified type and contract. - /// https://stackoverflow.com/questions/5091101/is-it-possible-to-remove-an-existing-registration-from-autofac-container-builder. + /// Because Autofac 5+ containers are immutable, + /// UnregisterCurrent method is not available anymore. /// /// The service type to unregister. /// The optional contract value, which will only remove the value associated with the contract. /// This is not implemented by default. /// - public virtual void UnregisterCurrent(Type serviceType, string contract = null) - { - lock (_lockObject) - { - var registrations = _componentContext.ComponentRegistry.Registrations.ToList(); - var registrationCount = registrations.Count; - if (registrationCount < 1) - { - return; - } - - var candidatesForRemoval = new List(registrationCount); - var registrationIndex = 0; - while (registrationIndex < registrationCount) - { - var componentRegistration = registrations[registrationIndex]; - - var isCandidateForRemoval = GetWhetherServiceRegistrationMatchesSearch( - componentRegistration.Services, - serviceType, - contract); - if (isCandidateForRemoval) - { - registrations.RemoveAt(registrationIndex); - candidatesForRemoval.Add(componentRegistration); - registrationCount--; - } - else - { - registrationIndex++; - } - } - - if (candidatesForRemoval.Count == 0) - { - // nothing to remove - return; - } - - if (candidatesForRemoval.Count > 1) - { - // need to re-add some registrations - var reAdd = candidatesForRemoval.Take(candidatesForRemoval.Count - 1); - registrations.AddRange(reAdd); - - /* - // check for multi service registration - // in future might want to just remove a single service from a component - // rather than do the whole component. - var lastCandidate = candidatesForRemoval.Last(); - var lastCandidateRegisteredServices = lastCandidate.Services.ToArray(); - if (lastCandidateRegisteredServices.Length > 1) - { - // - // builder.RegisterType() - // .AsSelf() - // .As() - // .As(); - var survivingServices = lastCandidateRegisteredServices.Where(s => s.GetType() != serviceType); - var newRegistration = new ComponentRegistration( - lastCandidate.Id, - lastCandidate.Activator, - lastCandidate.Lifetime, - lastCandidate.Sharing, - lastCandidate.Ownership, - survivingServices, - lastCandidate.Metadata); - registrations.Add(newRegistration); - } - */ - } - - RemoveAndRebuild(registrations); - } - } + [Obsolete("Because Autofac 5+ containers are immutable, UnregisterCurrent method is not available anymore.")] + public virtual void UnregisterCurrent(Type serviceType, string contract = null) => + throw new NotImplementedException("Because Autofac 5+ containers are immutable, UnregisterCurrent method is not available anymore."); /// - /// Unregisters all the values associated with the specified type and contract. - /// https://stackoverflow.com/questions/5091101/is-it-possible-to-remove-an-existing-registration-from-autofac-container-builder. + /// Because Autofac 5+ containers are immutable, + /// UnregisterAll method is not available anymore. /// /// The service type to unregister. /// The optional contract value, which will only remove the value associated with the contract. /// This is not implemented by default. /// - public virtual void UnregisterAll(Type serviceType, string contract = null) - { - lock (_lockObject) - { - // prevent multiple enumerations - var registrations = _componentContext.ComponentRegistry.Registrations.ToList(); - var registrationCount = registrations.Count; - if (registrationCount < 1) - { - return; - } - - if (!string.IsNullOrEmpty(contract)) - { - RemoveAndRebuild( - registrationCount, - registrations, - x => x.Services.All(s => - { - if (!(s is TypedService typedService)) - { - return false; - } - - return typedService.ServiceType != serviceType || !HasMatchingContract(s, contract); - })); - return; - } - - RemoveAndRebuild( - registrationCount, - registrations, - x => x.Services.All(s => - { - if (!(s is TypedService typedService)) - { - return false; - } - - return typedService.ServiceType != serviceType; - })); - } - } + [Obsolete("Because Autofac 5+ containers are immutable, UnregisterAll method is not available anymore.")] + public virtual void UnregisterAll(Type serviceType, string contract = null) => + throw new NotImplementedException("Because Autofac 5+ containers are immutable, UnregisterAll method is not available anymore."); /// - public virtual IDisposable ServiceRegistrationCallback(Type serviceType, string contract, Action callback) - { - // this method is not used by RxUI + public virtual IDisposable ServiceRegistrationCallback(Type serviceType, string contract, Action callback) => throw new NotImplementedException(); - } /// public void Dispose() @@ -260,119 +189,28 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - _componentContext.ComponentRegistry?.Dispose(); - } - } - } - - private static bool GetWhetherServiceRegistrationMatchesSearch( - IEnumerable componentRegistrationServices, - Type serviceType, - string contract) - { - foreach (var componentRegistrationService in componentRegistrationServices) - { - if (!(componentRegistrationService is IServiceWithType keyedService)) - { - continue; - } - - if (keyedService.ServiceType != serviceType) - { - continue; - } - - // right type - if (string.IsNullOrEmpty(contract)) - { - if (!HasNoContract(componentRegistrationService)) - { - continue; - } - - // candidate for removal - return true; - } - - if (!HasMatchingContract(componentRegistrationService, contract)) - { - continue; + _lifetimeScope.ComponentRegistry.Dispose(); + _internalContainer?.Dispose(); } - - // candidate for removal - return true; - } - - return false; - } - - private static bool HasMatchingContract(Service service, string contract) - { - if (!(service is KeyedService keyedService)) - { - return false; } - - if (!(keyedService.ServiceKey is string stringServiceKey)) - { - return false; - } - - return stringServiceKey.Equals(contract, StringComparison.Ordinal); - } - - private static bool HasNoContract(Service service) - { - return !(service is KeyedService); } private object Resolve(Type serviceType, string contract) { object serviceInstance; + var lifeTimeScope = _lifetimeScope ?? _internalLifetimeScope; + if (string.IsNullOrEmpty(contract)) { - _componentContext.TryResolve(serviceType, out serviceInstance); + lifeTimeScope.TryResolve(serviceType, out serviceInstance); } else { - _componentContext.TryResolveNamed(contract, serviceType, out serviceInstance); + lifeTimeScope.TryResolveNamed(contract, serviceType, out serviceInstance); } return serviceInstance; } - - private void RemoveAndRebuild( - int registrationCount, - IList registrations, - Func predicate) - { - var survivingComponents = registrations.Where(predicate).ToArray(); - - if (survivingComponents.Length == registrationCount) - { - // not removing anything - // drop out - return; - } - - RemoveAndRebuild(survivingComponents); - } - - private void RemoveAndRebuild(IEnumerable survivingComponents) - { - var builder = new ContainerBuilder(); - foreach (var c in survivingComponents) - { - builder.RegisterComponent(c); - } - - foreach (var source in _componentContext.ComponentRegistry.Sources) - { - builder.RegisterSource(source); - } - - _componentContext = builder.Build(); - } } } diff --git a/src/Splat.Autofac/Splat.Autofac.csproj b/src/Splat.Autofac/Splat.Autofac.csproj index f68d836f8..6391120ce 100644 --- a/src/Splat.Autofac/Splat.Autofac.csproj +++ b/src/Splat.Autofac/Splat.Autofac.csproj @@ -7,7 +7,7 @@ - + diff --git a/src/Splat.Autofac/SplatAutofacExtensions.cs b/src/Splat.Autofac/SplatAutofacExtensions.cs index ae1439c47..58447c265 100644 --- a/src/Splat.Autofac/SplatAutofacExtensions.cs +++ b/src/Splat.Autofac/SplatAutofacExtensions.cs @@ -13,27 +13,17 @@ namespace Splat.Autofac /// public static class SplatAutofacExtensions { - /// - /// Initializes an instance of that overrides the default . - /// - /// Autofac component context. - [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Dispose handled by locator.")] - public static void UseAutofacDependencyResolver(this IComponentContext componentContext) => - Locator.SetLocator(new AutofacDependencyResolver(componentContext)); - /// /// Initializes an instance of that overrides the default . /// /// Autofac container builder. + /// The Autofac dependency resolver. [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Dispose handled by locator.")] - public static void UseAutofacDependencyResolver(this ContainerBuilder containerBuilder) + public static AutofacDependencyResolver UseAutofacDependencyResolver(this ContainerBuilder containerBuilder) { - if (containerBuilder is null) - { - throw new ArgumentNullException(nameof(containerBuilder)); - } - - Locator.SetLocator(new AutofacDependencyResolver(containerBuilder.Build())); + var autofacResolver = new AutofacDependencyResolver(containerBuilder); + Locator.SetLocator(autofacResolver); + return autofacResolver; } } } From 6017cad64361d377d91a68102be24c240f3b33df Mon Sep 17 00:00:00 2001 From: Matej Hrlec Date: Fri, 18 Sep 2020 11:40:19 +0200 Subject: [PATCH 2/8] UPDATE readme --- src/Splat.Autofac/README.md | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/Splat.Autofac/README.md b/src/Splat.Autofac/README.md index 53c6fffc1..4fa205ec7 100644 --- a/src/Splat.Autofac/README.md +++ b/src/Splat.Autofac/README.md @@ -7,17 +7,34 @@ Splat.Autofac is an adapter for `IMutableDependencyResolver`. It allows you to ### Register the Container ```cs -var container = new ContainerBuilder(); -container.RegisterType().As>(); -container.RegisterType().As>(); -container.RegisterType().AsSelf(); -container.RegisterType().AsSelf(); +var containerBuilder = new ContainerBuilder(); +containerBuilder.RegisterType().As>(); +containerBuilder.RegisterType().As>(); +containerBuilder.RegisterType().AsSelf(); +containerBuilder.RegisterType().AsSelf(); +// etc. ``` ### Register the Adapter to Splat ```cs -container.UseAutofacDependencyResolver(); +// Creates and sets the Autofac resolver as the Locator +var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + +// Register the resolver in Autofac so it can be later resolved +containerBuilder.RegisterInstance(autofacResolver); + +// Initialize ReactiveUI components +autofacResolver.InitializeReactiveUI(); +``` + +### Set Autofac Locator's lifetime after the ContainerBuilder has been built + +```cs +var autofacResolver = container.Resolve(); + +// Set a lifetime scope (either the root or any of the child ones) to Autofac resolver +autofacResolver.SetLifetimeScope(container);` ``` ### Use the Locator From 113ce6f01a381b7c93bab48a3ec4fe9573d2a15e Mon Sep 17 00:00:00 2001 From: Matej Hrlec Date: Fri, 18 Sep 2020 12:22:53 +0200 Subject: [PATCH 3/8] CHANGE Splat.Autofac tests, so they attempt to mutate the container anymore CHANGE Splat.Autofac tests, so they attempt to mutate the container anymore --- .../DependencyResolverTests.cs | 70 ++++++++++++------- .../Splat.Autofac.Tests.csproj | 4 -- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/src/Splat.Autofac.Tests/DependencyResolverTests.cs b/src/Splat.Autofac.Tests/DependencyResolverTests.cs index 34ca06aae..73fb2e08b 100644 --- a/src/Splat.Autofac.Tests/DependencyResolverTests.cs +++ b/src/Splat.Autofac.Tests/DependencyResolverTests.cs @@ -23,10 +23,12 @@ public class DependencyResolverTests : BaseDependencyResolverTests().As>(); - builder.RegisterType().As>(); - builder.Build().UseAutofacDependencyResolver(); + var containerBuilder = new ContainerBuilder(); + containerBuilder.RegisterType().As>(); + containerBuilder.RegisterType().As>(); + + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); var viewOne = Locator.Current.GetService(typeof(IViewFor)); var viewTwo = Locator.Current.GetService(typeof(IViewFor)); @@ -43,9 +45,11 @@ public void AutofacDependencyResolver_Should_Resolve_Views() [Fact] public void AutofacDependencyResolver_Should_Resolve_Named_View() { - var builder = new ContainerBuilder(); - builder.RegisterType().Named>("Other"); - builder.Build().UseAutofacDependencyResolver(); + var containerBuilder = new ContainerBuilder(); + containerBuilder.RegisterType().Named>("Other"); + + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); var viewTwo = Locator.Current.GetService(typeof(IViewFor), "Other"); @@ -59,10 +63,12 @@ public void AutofacDependencyResolver_Should_Resolve_Named_View() [Fact] public void AutofacDependencyResolver_Should_Resolve_View_Models() { - var builder = new ContainerBuilder(); - builder.RegisterType().AsSelf(); - builder.RegisterType().AsSelf(); - builder.Build().UseAutofacDependencyResolver(); + var containerBuilder = new ContainerBuilder(); + containerBuilder.RegisterType().AsSelf(); + containerBuilder.RegisterType().AsSelf(); + + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); var vmOne = Locator.Current.GetService(); var vmTwo = Locator.Current.GetService(); @@ -77,9 +83,11 @@ public void AutofacDependencyResolver_Should_Resolve_View_Models() [Fact] public void AutofacDependencyResolver_Should_Resolve_Screen() { - var builder = new ContainerBuilder(); - builder.RegisterType().As().SingleInstance(); - builder.Build().UseAutofacDependencyResolver(); + var containerBuilder = new ContainerBuilder(); + containerBuilder.RegisterType().As().SingleInstance(); + + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); var screen = Locator.Current.GetService(); @@ -93,8 +101,10 @@ public void AutofacDependencyResolver_Should_Resolve_Screen() [Fact] public void AutofacDependencyResolver_Should_Throw_If_ServiceRegistionCallback_Called() { - var container = new ContainerBuilder(); - container.UseAutofacDependencyResolver(); + var containerBuilder = new ContainerBuilder(); + + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); var result = Record.Exception(() => Locator.CurrentMutable.ServiceRegistrationCallback(typeof(IScreen), disposable => { })); @@ -111,15 +121,17 @@ public void AutofacDependencyResolver_Should_Throw_If_ServiceRegistionCallback_C [Fact] public void AutofacDependencyResolver_Should_ReturnRegisteredLogger() { - var container = new ContainerBuilder(); - container.UseAutofacDependencyResolver(); + var containerBuilder = new ContainerBuilder(); + + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); Locator.CurrentMutable.RegisterConstant( new FuncLogManager(type => new WrappingFullLogger(new ConsoleLogger())), typeof(ILogManager)); - var d = Splat.Locator.Current.GetService(); - Assert.IsType(d); + var logManager = Locator.Current.GetService(); + Assert.IsType(logManager); } /// @@ -131,21 +143,25 @@ public void AutofacDependencyResolver_Should_ReturnRegisteredLogger() [Fact] public void AutofacDependencyResolver_PreInit_Should_ReturnRegisteredLogger() { - var builder = new ContainerBuilder(); - builder.Register(_ => new FuncLogManager(type => new WrappingFullLogger(new ConsoleLogger()))).As(typeof(ILogManager)) + var containerBuilder = new ContainerBuilder(); + containerBuilder.Register(_ => new FuncLogManager(type => new WrappingFullLogger(new ConsoleLogger()))).As(typeof(ILogManager)) .AsImplementedInterfaces(); - builder.UseAutofacDependencyResolver(); + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); - var d = Splat.Locator.Current.GetService(); - Assert.IsType(d); + var logManager = Locator.Current.GetService(); + Assert.IsType(logManager); } /// protected override AutofacDependencyResolver GetDependencyResolver() { - var container = new ContainerBuilder(); - return new AutofacDependencyResolver(container.Build()); + var containerBuilder = new ContainerBuilder(); + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); + + return autofacResolver; } } } diff --git a/src/Splat.Autofac.Tests/Splat.Autofac.Tests.csproj b/src/Splat.Autofac.Tests/Splat.Autofac.Tests.csproj index cd7b9fa9b..4e829d514 100644 --- a/src/Splat.Autofac.Tests/Splat.Autofac.Tests.csproj +++ b/src/Splat.Autofac.Tests/Splat.Autofac.Tests.csproj @@ -8,10 +8,6 @@ $(NoWarn);1591;CA1707;SA1633;CA2000 - - - - From aea0bfd0faf0b5f051c9e3722f7f050e0e0df892 Mon Sep 17 00:00:00 2001 From: Matej Hrlec Date: Fri, 18 Sep 2020 13:00:18 +0200 Subject: [PATCH 4/8] FIX a few tests hopefully --- .../DependencyResolverTests.cs | 52 +++++++++++++++++-- .../BaseDependencyResolverTests.cs | 2 +- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/Splat.Autofac.Tests/DependencyResolverTests.cs b/src/Splat.Autofac.Tests/DependencyResolverTests.cs index 73fb2e08b..d12a54be7 100644 --- a/src/Splat.Autofac.Tests/DependencyResolverTests.cs +++ b/src/Splat.Autofac.Tests/DependencyResolverTests.cs @@ -124,12 +124,13 @@ public void AutofacDependencyResolver_Should_ReturnRegisteredLogger() var containerBuilder = new ContainerBuilder(); var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); Locator.CurrentMutable.RegisterConstant( new FuncLogManager(type => new WrappingFullLogger(new ConsoleLogger())), typeof(ILogManager)); + autofacResolver.SetLifetimeScope(containerBuilder.Build()); + var logManager = Locator.Current.GetService(); Assert.IsType(logManager); } @@ -144,24 +145,65 @@ public void AutofacDependencyResolver_Should_ReturnRegisteredLogger() public void AutofacDependencyResolver_PreInit_Should_ReturnRegisteredLogger() { var containerBuilder = new ContainerBuilder(); + + var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + containerBuilder.Register(_ => new FuncLogManager(type => new WrappingFullLogger(new ConsoleLogger()))).As(typeof(ILogManager)) .AsImplementedInterfaces(); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); autofacResolver.SetLifetimeScope(containerBuilder.Build()); var logManager = Locator.Current.GetService(); Assert.IsType(logManager); } + /// + /// Because Autofac 5+ containers are immutable, + /// UnregisterAll method is not available anymore. + /// + [Fact] + public override void UnregisterAll_UnregisterCurrent_Doesnt_Throw_When_List_Empty() + { + } + + /// + /// + /// + /// + [Fact] + public override void HasRegistration() + { + var type = typeof(string); + const string contractOne = "ContractOne"; + const string contractTwo = "ContractTwo"; + var resolver = GetDependencyResolver(); + + Assert.False(resolver.HasRegistration(type)); + Assert.False(resolver.HasRegistration(type, contractOne)); + Assert.False(resolver.HasRegistration(type, contractTwo)); + + resolver.Register(() => "unnamed", type); + Assert.True(resolver.HasRegistration(type)); + Assert.False(resolver.HasRegistration(type, contractOne)); + Assert.False(resolver.HasRegistration(type, contractTwo)); + + resolver.Register(() => contractOne, type, contractOne); + Assert.True(resolver.HasRegistration(type)); + Assert.True(resolver.HasRegistration(type, contractOne)); + Assert.False(resolver.HasRegistration(type, contractTwo)); + + resolver.Register(() => contractTwo, type, contractTwo); + Assert.True(resolver.HasRegistration(type)); + Assert.True(resolver.HasRegistration(type, contractOne)); + Assert.True(resolver.HasRegistration(type, contractTwo)); + } + /// protected override AutofacDependencyResolver GetDependencyResolver() { var containerBuilder = new ContainerBuilder(); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); - return autofacResolver; + return containerBuilder.UseAutofacDependencyResolver(); } } } diff --git a/src/Splat.Tests/ServiceLocation/BaseDependencyResolverTests.cs b/src/Splat.Tests/ServiceLocation/BaseDependencyResolverTests.cs index 9398af959..249cf1b50 100644 --- a/src/Splat.Tests/ServiceLocation/BaseDependencyResolverTests.cs +++ b/src/Splat.Tests/ServiceLocation/BaseDependencyResolverTests.cs @@ -108,7 +108,7 @@ public void GetServices_Should_Never_Return_Null() /// Tests for ensuring hasregistration behaves when using contracts. /// [Fact] - public void HasRegistration() + public virtual void HasRegistration() { var type = typeof(string); const string contractOne = "ContractOne"; From c0fd68d122783dd66edd9e9b8ae9b45b6e994f95 Mon Sep 17 00:00:00 2001 From: Matej Hrlec Date: Fri, 18 Sep 2020 13:43:30 +0200 Subject: [PATCH 5/8] FIX 4 more tests for Unregistered by disabling them --- .../DependencyResolverTests.cs | 35 +++++++++++++++++-- .../BaseDependencyResolverTests.cs | 8 ++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/Splat.Autofac.Tests/DependencyResolverTests.cs b/src/Splat.Autofac.Tests/DependencyResolverTests.cs index d12a54be7..13bb2a501 100644 --- a/src/Splat.Autofac.Tests/DependencyResolverTests.cs +++ b/src/Splat.Autofac.Tests/DependencyResolverTests.cs @@ -158,14 +158,45 @@ public void AutofacDependencyResolver_PreInit_Should_ReturnRegisteredLogger() } /// - /// Because Autofac 5+ containers are immutable, - /// UnregisterAll method is not available anymore. + /// + /// + [Fact] + public override void UnregisterCurrent_Doesnt_Throw_When_List_Empty() + { + } + + /// + /// + /// + [Fact] + public override void UnregisterCurrent_Remove_Last() + { + } + + /// + /// + /// + [Fact] + public override void UnregisterCurrentByName_Doesnt_Throw_When_List_Empty() + { + } + + /// + /// /// [Fact] public override void UnregisterAll_UnregisterCurrent_Doesnt_Throw_When_List_Empty() { } + /// + /// + /// + [Fact] + public override void UnregisterAllByContract_UnregisterCurrent_Doesnt_Throw_When_List_Empty() + { + } + /// /// /// diff --git a/src/Splat.Tests/ServiceLocation/BaseDependencyResolverTests.cs b/src/Splat.Tests/ServiceLocation/BaseDependencyResolverTests.cs index 249cf1b50..14ba2a757 100644 --- a/src/Splat.Tests/ServiceLocation/BaseDependencyResolverTests.cs +++ b/src/Splat.Tests/ServiceLocation/BaseDependencyResolverTests.cs @@ -17,7 +17,7 @@ public abstract class BaseDependencyResolverTests /// Test to ensure Unregister doesn't cause an IndexOutOfRangeException. /// [Fact] - public void UnregisterCurrent_Doesnt_Throw_When_List_Empty() + public virtual void UnregisterCurrent_Doesnt_Throw_When_List_Empty() { var resolver = GetDependencyResolver(); var type = typeof(ILogManager); @@ -31,7 +31,7 @@ public void UnregisterCurrent_Doesnt_Throw_When_List_Empty() /// Test to ensure UnregisterCurrent removes last entry. /// [Fact] - public void UnregisterCurrent_Remove_Last() + public virtual void UnregisterCurrent_Remove_Last() { var resolver = GetDependencyResolver(); var type = typeof(ILogManager); @@ -52,7 +52,7 @@ public void UnregisterCurrent_Remove_Last() /// Test to ensure Unregister doesn't cause an IndexOutOfRangeException. /// [Fact] - public void UnregisterCurrentByName_Doesnt_Throw_When_List_Empty() + public virtual void UnregisterCurrentByName_Doesnt_Throw_When_List_Empty() { var resolver = GetDependencyResolver(); var type = typeof(ILogManager); @@ -81,7 +81,7 @@ public virtual void UnregisterAll_UnregisterCurrent_Doesnt_Throw_When_List_Empty /// Test to ensure Unregister doesn't cause an IndexOutOfRangeException. /// [Fact] - public void UnregisterAllByContract_UnregisterCurrent_Doesnt_Throw_When_List_Empty() + public virtual void UnregisterAllByContract_UnregisterCurrent_Doesnt_Throw_When_List_Empty() { var resolver = GetDependencyResolver(); var type = typeof(ILogManager); From 7a71acbaee87d7e7c840a9398e01e32a55842610 Mon Sep 17 00:00:00 2001 From: Matej Hrlec Date: Fri, 18 Sep 2020 15:32:57 +0200 Subject: [PATCH 6/8] ADD check against adding registration to the dependency resolver after the container has been built --- .../DependencyResolverTests.cs | 2 +- .../AutofacDependencyResolver.cs | 30 +++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/Splat.Autofac.Tests/DependencyResolverTests.cs b/src/Splat.Autofac.Tests/DependencyResolverTests.cs index 13bb2a501..fc9d13f66 100644 --- a/src/Splat.Autofac.Tests/DependencyResolverTests.cs +++ b/src/Splat.Autofac.Tests/DependencyResolverTests.cs @@ -99,7 +99,7 @@ public void AutofacDependencyResolver_Should_Resolve_Screen() /// Should throw an exception if service registration call back called. /// [Fact] - public void AutofacDependencyResolver_Should_Throw_If_ServiceRegistionCallback_Called() + public void AutofacDependencyResolver_Should_Throw_If_ServiceRegistrationCallback_Called() { var containerBuilder = new ContainerBuilder(); diff --git a/src/Splat.Autofac/AutofacDependencyResolver.cs b/src/Splat.Autofac/AutofacDependencyResolver.cs index b59978e7a..725ba60fa 100644 --- a/src/Splat.Autofac/AutofacDependencyResolver.cs +++ b/src/Splat.Autofac/AutofacDependencyResolver.cs @@ -33,6 +33,12 @@ public class AutofacDependencyResolver : IDependencyResolver private ILifetimeScope _internalLifetimeScope; #pragma warning restore CA2213 // Disposable fields should be disposed + /// + /// Set to true, when SetLifetimeScope has been called. + /// Prevents mutating the ContainerBuilder or setting the lifetime again. + /// + private bool _lifetimeScopeSet; + /// /// Initializes a new instance of the class. /// @@ -61,12 +67,21 @@ public virtual object GetService(Type serviceType, string contract = null) /// Lifetime scope, which will be used to resolve ReactiveUI services. public void SetLifetimeScope(ILifetimeScope lifetimeScope) { - _lifetimeScope = lifetimeScope; + lock (_lockObject) + { + if (_lifetimeScopeSet) + { + throw new Exception("Lifetime scope of the Autofac resolver has already been set"); + } + + _lifetimeScopeSet = true; + _lifetimeScope = lifetimeScope; - // We dispose on the internal container, since it and its many child lifetime scopes are not needed anymore. - _internalContainer.Dispose(); - _internalContainer = null; - _internalLifetimeScope = null; + // We dispose on the internal container, since it and its many child lifetime scopes are not needed anymore. + _internalContainer.Dispose(); + _internalContainer = null; + _internalLifetimeScope = null; + } } /// @@ -118,6 +133,11 @@ public virtual void Register(Func factory, Type serviceType, string cont { lock (_lockObject) { + if (_lifetimeScopeSet) + { + throw new Exception("Container has already been built and the lifetime scope set, so it is not possible to modify it anymore."); + } + // We register every ReactiveUI service twice. // First to the application-wide container, which we are still building. // Second to child lifetimes in a temporary container, that is used only to satisfy ReactiveUI dependencies. From 3c2085588e588c2da90a8b3bbda2ba99b051a096 Mon Sep 17 00:00:00 2001 From: Matej Hrlec Date: Mon, 21 Sep 2020 09:11:29 +0200 Subject: [PATCH 7/8] CHANGE readme and container builder naming --- .../DependencyResolverTests.cs | 60 +++++++++---------- .../AutofacDependencyResolver.cs | 20 ++++--- src/Splat.Autofac/README.md | 20 ++++--- src/Splat.Autofac/SplatAutofacExtensions.cs | 6 +- 4 files changed, 59 insertions(+), 47 deletions(-) diff --git a/src/Splat.Autofac.Tests/DependencyResolverTests.cs b/src/Splat.Autofac.Tests/DependencyResolverTests.cs index fc9d13f66..478c71394 100644 --- a/src/Splat.Autofac.Tests/DependencyResolverTests.cs +++ b/src/Splat.Autofac.Tests/DependencyResolverTests.cs @@ -23,12 +23,12 @@ public class DependencyResolverTests : BaseDependencyResolverTests().As>(); - containerBuilder.RegisterType().As>(); + var builder = new ContainerBuilder(); + builder.RegisterType().As>(); + builder.RegisterType().As>(); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); + var autofacResolver = builder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(builder.Build()); var viewOne = Locator.Current.GetService(typeof(IViewFor)); var viewTwo = Locator.Current.GetService(typeof(IViewFor)); @@ -45,11 +45,11 @@ public void AutofacDependencyResolver_Should_Resolve_Views() [Fact] public void AutofacDependencyResolver_Should_Resolve_Named_View() { - var containerBuilder = new ContainerBuilder(); - containerBuilder.RegisterType().Named>("Other"); + var builder = new ContainerBuilder(); + builder.RegisterType().Named>("Other"); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); + var autofacResolver = builder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(builder.Build()); var viewTwo = Locator.Current.GetService(typeof(IViewFor), "Other"); @@ -63,12 +63,12 @@ public void AutofacDependencyResolver_Should_Resolve_Named_View() [Fact] public void AutofacDependencyResolver_Should_Resolve_View_Models() { - var containerBuilder = new ContainerBuilder(); - containerBuilder.RegisterType().AsSelf(); - containerBuilder.RegisterType().AsSelf(); + var builder = new ContainerBuilder(); + builder.RegisterType().AsSelf(); + builder.RegisterType().AsSelf(); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); + var autofacResolver = builder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(builder.Build()); var vmOne = Locator.Current.GetService(); var vmTwo = Locator.Current.GetService(); @@ -83,11 +83,11 @@ public void AutofacDependencyResolver_Should_Resolve_View_Models() [Fact] public void AutofacDependencyResolver_Should_Resolve_Screen() { - var containerBuilder = new ContainerBuilder(); - containerBuilder.RegisterType().As().SingleInstance(); + var builder = new ContainerBuilder(); + builder.RegisterType().As().SingleInstance(); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); + var autofacResolver = builder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(builder.Build()); var screen = Locator.Current.GetService(); @@ -101,10 +101,10 @@ public void AutofacDependencyResolver_Should_Resolve_Screen() [Fact] public void AutofacDependencyResolver_Should_Throw_If_ServiceRegistrationCallback_Called() { - var containerBuilder = new ContainerBuilder(); + var builder = new ContainerBuilder(); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); + var autofacResolver = builder.UseAutofacDependencyResolver(); + autofacResolver.SetLifetimeScope(builder.Build()); var result = Record.Exception(() => Locator.CurrentMutable.ServiceRegistrationCallback(typeof(IScreen), disposable => { })); @@ -121,15 +121,15 @@ public void AutofacDependencyResolver_Should_Throw_If_ServiceRegistrationCallbac [Fact] public void AutofacDependencyResolver_Should_ReturnRegisteredLogger() { - var containerBuilder = new ContainerBuilder(); + var builder = new ContainerBuilder(); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + var autofacResolver = builder.UseAutofacDependencyResolver(); Locator.CurrentMutable.RegisterConstant( new FuncLogManager(type => new WrappingFullLogger(new ConsoleLogger())), typeof(ILogManager)); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); + autofacResolver.SetLifetimeScope(builder.Build()); var logManager = Locator.Current.GetService(); Assert.IsType(logManager); @@ -144,14 +144,14 @@ public void AutofacDependencyResolver_Should_ReturnRegisteredLogger() [Fact] public void AutofacDependencyResolver_PreInit_Should_ReturnRegisteredLogger() { - var containerBuilder = new ContainerBuilder(); + var builder = new ContainerBuilder(); - var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); + var autofacResolver = builder.UseAutofacDependencyResolver(); - containerBuilder.Register(_ => new FuncLogManager(type => new WrappingFullLogger(new ConsoleLogger()))).As(typeof(ILogManager)) + builder.Register(_ => new FuncLogManager(type => new WrappingFullLogger(new ConsoleLogger()))).As(typeof(ILogManager)) .AsImplementedInterfaces(); - autofacResolver.SetLifetimeScope(containerBuilder.Build()); + autofacResolver.SetLifetimeScope(builder.Build()); var logManager = Locator.Current.GetService(); Assert.IsType(logManager); @@ -232,9 +232,9 @@ public override void HasRegistration() /// protected override AutofacDependencyResolver GetDependencyResolver() { - var containerBuilder = new ContainerBuilder(); + var builder = new ContainerBuilder(); - return containerBuilder.UseAutofacDependencyResolver(); + return builder.UseAutofacDependencyResolver(); } } } diff --git a/src/Splat.Autofac/AutofacDependencyResolver.cs b/src/Splat.Autofac/AutofacDependencyResolver.cs index 725ba60fa..ea00ea8e5 100644 --- a/src/Splat.Autofac/AutofacDependencyResolver.cs +++ b/src/Splat.Autofac/AutofacDependencyResolver.cs @@ -42,10 +42,10 @@ public class AutofacDependencyResolver : IDependencyResolver /// /// Initializes a new instance of the class. /// - /// Autofac container builder. - public AutofacDependencyResolver(ContainerBuilder containerBuilder) + /// Autofac container builder. + public AutofacDependencyResolver(ContainerBuilder builder) { - _builder = containerBuilder; + _builder = builder; _internalContainer = new ContainerBuilder().Build(); _internalLifetimeScope = _internalContainer.BeginLifetimeScope(); @@ -167,26 +167,32 @@ public virtual void Register(Func factory, Type serviceType, string cont /// /// Because Autofac 5+ containers are immutable, /// UnregisterCurrent method is not available anymore. + /// Instead, simply register your service after InitializeReactiveUI to override it. /// /// The service type to unregister. /// The optional contract value, which will only remove the value associated with the contract. /// This is not implemented by default. /// - [Obsolete("Because Autofac 5+ containers are immutable, UnregisterCurrent method is not available anymore.")] + [Obsolete("Because Autofac 5+ containers are immutable, UnregisterCurrent method is not available anymore. " + + "Instead, simply register your service after InitializeReactiveUI to override it https://autofaccn.readthedocs.io/en/latest/register/registration.html#default-registrations.")] public virtual void UnregisterCurrent(Type serviceType, string contract = null) => - throw new NotImplementedException("Because Autofac 5+ containers are immutable, UnregisterCurrent method is not available anymore."); + throw new NotImplementedException("Because Autofac 5+ containers are immutable, UnregisterCurrent method is not available anymore. " + + "Instead, simply register your service after InitializeReactiveUI to override it https://autofaccn.readthedocs.io/en/latest/register/registration.html#default-registrations."); /// /// Because Autofac 5+ containers are immutable, /// UnregisterAll method is not available anymore. + /// Instead, simply register your service after InitializeReactiveUI to override it. /// /// The service type to unregister. /// The optional contract value, which will only remove the value associated with the contract. /// This is not implemented by default. /// - [Obsolete("Because Autofac 5+ containers are immutable, UnregisterAll method is not available anymore.")] + [Obsolete("Because Autofac 5+ containers are immutable, UnregisterAll method is not available anymore. " + + "Instead, simply register your service after InitializeReactiveUI to override it https://autofaccn.readthedocs.io/en/latest/register/registration.html#default-registrations.")] public virtual void UnregisterAll(Type serviceType, string contract = null) => - throw new NotImplementedException("Because Autofac 5+ containers are immutable, UnregisterAll method is not available anymore."); + throw new NotImplementedException("Because Autofac 5+ containers are immutable, UnregisterAll method is not available anymore. " + + "Instead, simply register your service after InitializeReactiveUI to override it https://autofaccn.readthedocs.io/en/latest/register/registration.html#default-registrations."); /// public virtual IDisposable ServiceRegistrationCallback(Type serviceType, string contract, Action callback) => diff --git a/src/Splat.Autofac/README.md b/src/Splat.Autofac/README.md index 4fa205ec7..02fbebb04 100644 --- a/src/Splat.Autofac/README.md +++ b/src/Splat.Autofac/README.md @@ -7,11 +7,11 @@ Splat.Autofac is an adapter for `IMutableDependencyResolver`. It allows you to ### Register the Container ```cs -var containerBuilder = new ContainerBuilder(); -containerBuilder.RegisterType().As>(); -containerBuilder.RegisterType().As>(); -containerBuilder.RegisterType().AsSelf(); -containerBuilder.RegisterType().AsSelf(); +var builder = new ContainerBuilder(); +builder.RegisterType().As>(); +builder.RegisterType().As>(); +builder.RegisterType().AsSelf(); +builder.RegisterType().AsSelf(); // etc. ``` @@ -19,13 +19,17 @@ containerBuilder.RegisterType().AsSelf(); ```cs // Creates and sets the Autofac resolver as the Locator -var autofacResolver = containerBuilder.UseAutofacDependencyResolver(); +var autofacResolver = builder.UseAutofacDependencyResolver(); // Register the resolver in Autofac so it can be later resolved -containerBuilder.RegisterInstance(autofacResolver); +builder.RegisterInstance(autofacResolver); // Initialize ReactiveUI components autofacResolver.InitializeReactiveUI(); + +// If you need to override any service (such as the ViewLocator), register it after InitializeReactiveUI +// https://autofaccn.readthedocs.io/en/latest/register/registration.html#default-registrations +// builder.RegisterType().As().SingleInstance(); ``` ### Set Autofac Locator's lifetime after the ContainerBuilder has been built @@ -34,6 +38,8 @@ autofacResolver.InitializeReactiveUI(); var autofacResolver = container.Resolve(); // Set a lifetime scope (either the root or any of the child ones) to Autofac resolver +// This is needed, because the previous steps did not Update the ContainerBuilder since they became immutable in Autofac 5+ +// https://github.com/autofac/Autofac/issues/811 autofacResolver.SetLifetimeScope(container);` ``` diff --git a/src/Splat.Autofac/SplatAutofacExtensions.cs b/src/Splat.Autofac/SplatAutofacExtensions.cs index 58447c265..ef4ffa817 100644 --- a/src/Splat.Autofac/SplatAutofacExtensions.cs +++ b/src/Splat.Autofac/SplatAutofacExtensions.cs @@ -16,12 +16,12 @@ public static class SplatAutofacExtensions /// /// Initializes an instance of that overrides the default . /// - /// Autofac container builder. + /// Autofac container builder. /// The Autofac dependency resolver. [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Dispose handled by locator.")] - public static AutofacDependencyResolver UseAutofacDependencyResolver(this ContainerBuilder containerBuilder) + public static AutofacDependencyResolver UseAutofacDependencyResolver(this ContainerBuilder builder) { - var autofacResolver = new AutofacDependencyResolver(containerBuilder); + var autofacResolver = new AutofacDependencyResolver(builder); Locator.SetLocator(autofacResolver); return autofacResolver; } From f9d727c7f13b17cd809973e363a3574816176399 Mon Sep 17 00:00:00 2001 From: Matej Hrlec Date: Mon, 21 Sep 2020 09:19:55 +0200 Subject: [PATCH 8/8] CHANGE Autofac version specifier --- src/Splat.Autofac/Splat.Autofac.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Splat.Autofac/Splat.Autofac.csproj b/src/Splat.Autofac/Splat.Autofac.csproj index 6391120ce..f8064a5ff 100644 --- a/src/Splat.Autofac/Splat.Autofac.csproj +++ b/src/Splat.Autofac/Splat.Autofac.csproj @@ -7,7 +7,7 @@ - +