diff --git a/src/Directory.build.props b/src/Directory.build.props index 110c63508..f50fd2145 100644 --- a/src/Directory.build.props +++ b/src/Directory.build.props @@ -48,7 +48,7 @@ - + @@ -70,7 +70,7 @@ - + diff --git a/src/ReactiveUI.DI.Tests/DryIocReactiveUIDependencyTests.cs b/src/ReactiveUI.DI.Tests/DryIocReactiveUIDependencyTests.cs index 9b0e2b385..27cfe2a9b 100644 --- a/src/ReactiveUI.DI.Tests/DryIocReactiveUIDependencyTests.cs +++ b/src/ReactiveUI.DI.Tests/DryIocReactiveUIDependencyTests.cs @@ -3,12 +3,12 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for full license information. -using System; using System.Collections.Generic; using System.Linq; using DryIoc; using FluentAssertions; using Splat; +using Splat.Common.Test; using Splat.DryIoc; using Xunit; @@ -20,15 +20,17 @@ namespace ReactiveUI.DI.Tests public class DryIocReactiveUIDependencyTests { /// - /// Dries the ioc dependency resolver should register reactive UI creates command binding. + /// DyyIoC dependency resolver should register reactive UI creates command binding. /// [Fact] public void DryIocDependencyResolverShouldRegisterReactiveUI() { // Invoke RxApp which initializes the ReactiveUI platform. var container = new Container(); - container.UseDryIocDependencyResolver(); - Locator.CurrentMutable.InitializeReactiveUI(); + + var locator = new DryIocDependencyResolver(container); + locator.RegisterViewsForViewModels(typeof(ViewWithViewContractThatShouldNotLoad).Assembly); + locator.InitializeReactiveUI(); var converters = container.Resolve>().ToList(); diff --git a/src/ReactiveUI.DI.Tests/Mocks/ActivatingView.cs b/src/ReactiveUI.DI.Tests/Mocks/ActivatingView.cs index 3061fc8a6..ef4a1fab9 100644 --- a/src/ReactiveUI.DI.Tests/Mocks/ActivatingView.cs +++ b/src/ReactiveUI.DI.Tests/Mocks/ActivatingView.cs @@ -18,7 +18,7 @@ namespace ReactiveUI.DI.Tests.Mocks public sealed class ActivatingView : ReactiveObject, IViewFor, IDisposable { private int _count; - private ActivatingViewModel _viewModel; + private ActivatingViewModel? _viewModel; /// /// Initializes a new instance of the class. @@ -53,7 +53,7 @@ public ActivatingView() /// /// Gets or sets the view model. /// - public ActivatingViewModel ViewModel + public ActivatingViewModel? ViewModel { get => _viewModel; set => this.RaiseAndSetIfChanged(ref _viewModel, value); @@ -62,10 +62,10 @@ public ActivatingViewModel ViewModel /// /// Gets or sets the view model. /// - object IViewFor.ViewModel + object? IViewFor.ViewModel { get => ViewModel; - set => ViewModel = (ActivatingViewModel)value; + set => ViewModel = (ActivatingViewModel?)value; } /// diff --git a/src/ReactiveUI.DI.Tests/ReactiveUI.DI.Tests.csproj b/src/ReactiveUI.DI.Tests/ReactiveUI.DI.Tests.csproj index 0b4cb5b75..d41b62559 100644 --- a/src/ReactiveUI.DI.Tests/ReactiveUI.DI.Tests.csproj +++ b/src/ReactiveUI.DI.Tests/ReactiveUI.DI.Tests.csproj @@ -5,12 +5,12 @@ $(NoWarn);1591;CA1707;SA1633;CA2000 false latest + enable - @@ -19,7 +19,6 @@ - @@ -29,7 +28,12 @@ - + + + + + + diff --git a/src/ReactiveUI.DI.Tests/SimpleInjectorReactiveUIDependencyTests.cs b/src/ReactiveUI.DI.Tests/SimpleInjectorReactiveUIDependencyTests.cs index bbd4dc162..9ac025389 100644 --- a/src/ReactiveUI.DI.Tests/SimpleInjectorReactiveUIDependencyTests.cs +++ b/src/ReactiveUI.DI.Tests/SimpleInjectorReactiveUIDependencyTests.cs @@ -27,10 +27,8 @@ public void SimpleInjectorDependencyResolverShouldRegisterReactiveUIBindingTypeC Container container = new(); SimpleInjectorInitializer initializer = new(); - Locator.SetLocator(initializer); - Locator.CurrentMutable.InitializeReactiveUI(); - container.UseSimpleInjectorDependencyResolver(initializer); - var converters = Locator.Current.GetServices().ToList(); + initializer.InitializeReactiveUI(); + var converters = initializer.GetServices().ToList(); converters.Should().NotBeNull(); converters.Should().Contain(x => x.GetType() == typeof(StringConverter)); @@ -47,11 +45,9 @@ public void SimpleInjectorDependencyResolverShouldRegisterReactiveUICreatesComma Container container = new(); SimpleInjectorInitializer initializer = new(); - Locator.SetLocator(initializer); - Locator.CurrentMutable.InitializeReactiveUI(); - container.UseSimpleInjectorDependencyResolver(initializer); + initializer.InitializeReactiveUI(); - var converters = Locator.Current.GetServices().ToList(); + var converters = initializer.GetServices().ToList(); converters.Should().NotBeNull(); converters.Should().Contain(x => x.GetType() == typeof(CreatesCommandBindingViaEvent)); diff --git a/src/ReactiveUI.DI.Tests/ViewWithViewContractThatShouldNotLoad.cs b/src/ReactiveUI.DI.Tests/ViewWithViewContractThatShouldNotLoad.cs new file mode 100644 index 000000000..f4bbfa11b --- /dev/null +++ b/src/ReactiveUI.DI.Tests/ViewWithViewContractThatShouldNotLoad.cs @@ -0,0 +1,32 @@ +using System; +using Splat.Common.Test; + +namespace ReactiveUI.DI.Tests +{ + /// + /// This is a test view relating to issue #889. + /// It's intended to ensure that view registration by different DI\IoC implementations + /// does not create an instance at the point of registration. + /// + [ViewContract("somecontract")] + public sealed class ViewWithViewContractThatShouldNotLoad : IViewFor + { + /// + /// Initializes a new instance of the class. + /// + public ViewWithViewContractThatShouldNotLoad() + { + throw new InvalidOperationException("This view should not be created."); + } + + /// + object? IViewFor.ViewModel + { + get => ViewModel; + set => ViewModel = (ViewModelOne?)value; + } + + /// + public ViewModelOne? ViewModel { get; set; } + } +} diff --git a/src/Splat.Common.Test/ViewThatShouldNotLoad.cs b/src/Splat.Common.Test/ViewThatShouldNotLoad.cs new file mode 100644 index 000000000..f49b05958 --- /dev/null +++ b/src/Splat.Common.Test/ViewThatShouldNotLoad.cs @@ -0,0 +1,35 @@ +// Copyright (c) 2021 .NET Foundation and Contributors. All rights reserved. +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System; + +namespace Splat.Common.Test +{ + /// + /// This is a test view relating to issue #889. + /// It's intended to ensure that view registration by different DI\IoC implementations + /// does not create an instance at the point of registration. + /// + public sealed class ViewThatShouldNotLoad : IViewFor + { + /// + /// Initializes a new instance of the class. + /// + public ViewThatShouldNotLoad() + { + throw new InvalidOperationException("This view should not be created."); + } + + /// + object? IViewFor.ViewModel + { + get => ViewModel; + set => ViewModel = (ViewModelOne?)value; + } + + /// + public ViewModelOne? ViewModel { get; set; } + } +} diff --git a/src/Splat.DryIoc.Tests/DependencyResolverTests.cs b/src/Splat.DryIoc.Tests/DependencyResolverTests.cs index 3c88f24de..b61830188 100644 --- a/src/Splat.DryIoc.Tests/DependencyResolverTests.cs +++ b/src/Splat.DryIoc.Tests/DependencyResolverTests.cs @@ -21,15 +21,17 @@ public class DependencyResolverTests /// /// Shoulds the resolve nulls. /// - [Fact(Skip = "Further investigation required")] + [Fact] //// (Skip = "Further investigation required")] public void Can_Register_And_Resolve_Null_Types() { var builder = new Container(); builder.UseDryIocDependencyResolver(); var foo = 5; - Locator.CurrentMutable.Register(() => foo, null); + Assert.Throws(() => Locator.CurrentMutable.Register(() => foo, null)); + // Tests skipped as functionality removed. +#if SKIP_TEST var bar = 4; var contract = "foo"; Locator.CurrentMutable.Register(() => bar, null, contract); @@ -59,6 +61,33 @@ public void Can_Register_And_Resolve_Null_Types() Locator.CurrentMutable.UnregisterAll(null, contract); valuesC = Locator.Current.GetServices(null, contract); Assert.Equal(0, valuesC.Count()); +#endif + } + + /// + /// Should resolve the views. + /// + [Fact] + public void DryIocDependencyResolver_Should_Register_But_Not_Create_Views() + { + var container = new Container(); + container.UseDryIocDependencyResolver(); + + Splat.Locator.CurrentMutable.Register(() => new ViewThatShouldNotLoad(), typeof(IViewFor)); + Assert.Throws(() => Locator.Current.GetService>()); + } + + /// + /// Should resolve the views. + /// + [Fact] + public void DryIocDependencyResolver_Should_Register_With_Contract_But_Not_Create_Views() + { + var container = new Container(); + container.UseDryIocDependencyResolver(); + + Splat.Locator.CurrentMutable.Register(() => new ViewThatShouldNotLoad(), typeof(IViewFor), "name"); + Assert.Throws(() => Locator.Current.GetService>("name")); } /// @@ -108,6 +137,8 @@ public void DryIocDependencyResolver_Should_Resolve_View_Models() container.Register(); container.UseDryIocDependencyResolver(); + Splat.Locator.CurrentMutable.Register(() => new ViewThatShouldNotLoad(), typeof(IViewFor), "name"); + var vmOne = Locator.Current.GetService(); var vmTwo = Locator.Current.GetService(); @@ -255,7 +286,7 @@ public void DryIocDependencyResolver_PreInit_Should_ReturnRegisteredLogger() /// DryIoc dependency resolver should resolve after duplicate keyed registratoion. /// [Fact] - public void DryIocDependencyResolver_Should_Resolve_AfterDuplicateKeyedRegistratoion() + public void DryIocDependencyResolver_Should_Resolve_AfterDuplicateKeyedRegistration() { var container = new Container(); container.UseDryIocDependencyResolver(); diff --git a/src/Splat.DryIoc/DryIocDependencyResolver.cs b/src/Splat.DryIoc/DryIocDependencyResolver.cs index 9ff613f72..91ebfe4c9 100644 --- a/src/Splat.DryIoc/DryIocDependencyResolver.cs +++ b/src/Splat.DryIoc/DryIocDependencyResolver.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Linq.Expressions; using DryIoc; namespace Splat.DryIoc @@ -36,26 +37,25 @@ public DryIocDependencyResolver(IContainer? container = null) /// public virtual IEnumerable GetServices(Type? serviceType, string? contract = null) { - var isNull = serviceType is null; if (serviceType is null) { - serviceType = typeof(NullServiceType); + throw new ArgumentNullException(nameof(serviceType)); } var key = (serviceType, contract ?? string.Empty); - var registeredinSplat = _container.ResolveMany(serviceType, behavior: ResolveManyBehavior.AsFixedArray, serviceKey: key).Select(x => isNull ? ((NullServiceType)x).Factory()! : x); + var registeredinSplat = _container.ResolveMany(serviceType, behavior: ResolveManyBehavior.AsFixedArray, serviceKey: key); if (registeredinSplat.Any()) { return registeredinSplat; } - var registeredWithContract = _container.ResolveMany(serviceType, behavior: ResolveManyBehavior.AsFixedArray, serviceKey: contract).Select(x => isNull ? ((NullServiceType)x).Factory()! : x); + var registeredWithContract = _container.ResolveMany(serviceType, behavior: ResolveManyBehavior.AsFixedArray, serviceKey: contract); if (registeredWithContract.Any()) { return registeredWithContract; } - return _container.ResolveMany(serviceType, behavior: ResolveManyBehavior.AsFixedArray).Select(x => isNull ? ((NullServiceType)x).Factory()! : x); + return _container.ResolveMany(serviceType, behavior: ResolveManyBehavior.AsFixedArray); } /// @@ -63,7 +63,7 @@ public bool HasRegistration(Type? serviceType, string? contract = null) { if (serviceType is null) { - serviceType = typeof(NullServiceType); + throw new ArgumentNullException(nameof(serviceType)); } return _container.GetServiceRegistrations().Any(x => @@ -95,18 +95,18 @@ public virtual void Register(Func factory, Type? serviceType, string? c throw new ArgumentNullException(nameof(factory)); } - var isNull = serviceType is null; if (serviceType is null) { - serviceType = typeof(NullServiceType); + throw new ArgumentNullException(nameof(serviceType)); } if (string.IsNullOrEmpty(contract)) { - _container.RegisterInstance( - serviceType, - isNull ? new NullServiceType(factory) : factory(), - IfAlreadyRegistered.AppendNewImplementation); + _container.RegisterDelegate( + serviceType, + context => CreateThenConvert(serviceType, factory), + ifAlreadyRegistered: IfAlreadyRegistered.AppendNewImplementation); + return; } @@ -118,10 +118,10 @@ public virtual void Register(Func factory, Type? serviceType, string? c } // Keyed instances can only have a single instance so keep latest - _container.RegisterInstance( + _container.RegisterDelegate( serviceType, - isNull ? new NullServiceType(factory) : factory(), - IfAlreadyRegistered.Replace, + context => CreateThenConvert(serviceType, factory), + ifAlreadyRegistered: IfAlreadyRegistered.Replace, serviceKey: key); } @@ -130,7 +130,7 @@ public virtual void UnregisterCurrent(Type? serviceType, string? contract = null { if (serviceType is null) { - serviceType = typeof(NullServiceType); + throw new ArgumentNullException(nameof(serviceType)); } var key = (serviceType, contract ?? string.Empty); @@ -169,7 +169,7 @@ public virtual void UnregisterAll(Type? serviceType, string? contract = null) { if (serviceType is null) { - serviceType = typeof(NullServiceType); + throw new ArgumentNullException(nameof(serviceType)); } var key = (serviceType, contract ?? string.Empty); @@ -224,5 +224,24 @@ protected virtual void Dispose(bool disposing) _container?.Dispose(); } } + + private static object? CreateThenConvert(Type serviceType, Func factory) + { + // we need to cast because we pass an object back and dryioc wants it explicitly cast. + // alternative (happy to be proven wrong) is to break the interface and add a Register(...) method? + var instance = factory(); + + return instance != null ? Cast(serviceType, instance) : null; + } + + private static object? Cast(Type type, object data) + { + // based upon https://stackoverflow.com/a/27584212 + var dataParam = Expression.Parameter(typeof(object), "data"); + var body = Expression.Block(Expression.Convert(Expression.Convert(dataParam, data.GetType()), type)); + + var run = Expression.Lambda(body, dataParam).Compile(); + return run.DynamicInvoke(data); + } } }