Skip to content

Commit

Permalink
Fix: 889 DryIoC creating instances at point of registration (#934)
Browse files Browse the repository at this point in the history
  • Loading branch information
dpvreony authored Jul 14, 2022
1 parent 36af7e2 commit 8b4e557
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 41 deletions.
4 changes: 2 additions & 2 deletions src/Directory.build.props
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
<PackageReference Include="FluentAssertions" Version="6.7.0" />
<PackageReference Include="Microsoft.Reactive.Testing" Version="5.0.0" />
<PackageReference Include="PublicApiGenerator" Version="10.3.0" />
<PackageReference Include="Verify.Xunit" Version="17.1.6" />
<PackageReference Include="Verify.Xunit" Version="17.2.1" />
</ItemGroup>

<ItemGroup Condition="$(IsTestProject)">
Expand All @@ -70,7 +70,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Nerdbank.GitVersioning" Version="3.5.107" PrivateAssets="all" />
<PackageReference Include="Nerdbank.GitVersioning" Version="3.5.108" PrivateAssets="all" />
</ItemGroup>

<ItemGroup>
Expand Down
10 changes: 6 additions & 4 deletions src/ReactiveUI.DI.Tests/DryIocReactiveUIDependencyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -20,15 +20,17 @@ namespace ReactiveUI.DI.Tests
public class DryIocReactiveUIDependencyTests
{
/// <summary>
/// Dries the ioc dependency resolver should register reactive UI creates command binding.
/// DyyIoC dependency resolver should register reactive UI creates command binding.
/// </summary>
[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<IEnumerable<ICreatesCommandBinding>>().ToList();

Expand Down
8 changes: 4 additions & 4 deletions src/ReactiveUI.DI.Tests/Mocks/ActivatingView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace ReactiveUI.DI.Tests.Mocks
public sealed class ActivatingView : ReactiveObject, IViewFor<ActivatingViewModel>, IDisposable
{
private int _count;
private ActivatingViewModel _viewModel;
private ActivatingViewModel? _viewModel;

/// <summary>
/// Initializes a new instance of the <see cref="ActivatingView"/> class.
Expand Down Expand Up @@ -53,7 +53,7 @@ public ActivatingView()
/// <summary>
/// Gets or sets the view model.
/// </summary>
public ActivatingViewModel ViewModel
public ActivatingViewModel? ViewModel
{
get => _viewModel;
set => this.RaiseAndSetIfChanged(ref _viewModel, value);
Expand All @@ -62,10 +62,10 @@ public ActivatingViewModel ViewModel
/// <summary>
/// Gets or sets the view model.
/// </summary>
object IViewFor.ViewModel
object? IViewFor.ViewModel
{
get => ViewModel;
set => ViewModel = (ActivatingViewModel)value;
set => ViewModel = (ActivatingViewModel?)value;
}

/// <summary>
Expand Down
10 changes: 7 additions & 3 deletions src/ReactiveUI.DI.Tests/ReactiveUI.DI.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
<NoWarn>$(NoWarn);1591;CA1707;SA1633;CA2000</NoWarn>
<IsPackable>false</IsPackable>
<LangVersion>latest</LangVersion>
<Nullable>enable</Nullable>
</PropertyGroup>

<Choose>
<When Condition="$(TargetFramework.StartsWith('net472'))">
<ItemGroup>
<Compile Remove="DryIocReactiveUIDependencyTests.cs" />
<!--<Compile Remove="AutoFacReactiveUIDependencyTests.cs" />-->
<Compile Remove="NinjectReactiveUIDependencyTests.cs" />
<Compile Remove="SimpleInjectorReactiveUIDependencyTests.cs" />
Expand All @@ -19,7 +19,6 @@
</When>
<When Condition="$(TargetFramework.StartsWith('net6.0'))">
<ItemGroup>
<Compile Remove="DryIocReactiveUIDependencyTests.cs" />
<Compile Remove="AutoFacReactiveUIDependencyTests.cs" />
<Compile Remove="NinjectReactiveUIDependencyTests.cs" />
<!--<Compile Remove="SimpleInjectorReactiveUIDependencyTests.cs" />-->
Expand All @@ -29,7 +28,12 @@
</Choose>

<ItemGroup>
<PackageReference Include="ReactiveUI" Version="18.2.5" />
<PackageReference Include="ReactiveUI" Version="18.2.9" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Splat.Common.Test\Splat.Common.Test.csproj" />
<ProjectReference Include="..\Splat.DryIoc\Splat.DryIoc.csproj" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -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<IBindingTypeConverter>().ToList();
initializer.InitializeReactiveUI();
var converters = initializer.GetServices<IBindingTypeConverter>().ToList();

converters.Should().NotBeNull();
converters.Should().Contain(x => x.GetType() == typeof(StringConverter));
Expand All @@ -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<ICreatesCommandBinding>().ToList();
var converters = initializer.GetServices<ICreatesCommandBinding>().ToList();

converters.Should().NotBeNull();
converters.Should().Contain(x => x.GetType() == typeof(CreatesCommandBindingViaEvent));
Expand Down
32 changes: 32 additions & 0 deletions src/ReactiveUI.DI.Tests/ViewWithViewContractThatShouldNotLoad.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System;
using Splat.Common.Test;

namespace ReactiveUI.DI.Tests
{
/// <summary>
/// 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.
/// </summary>
[ViewContract("somecontract")]
public sealed class ViewWithViewContractThatShouldNotLoad : IViewFor<ViewModelOne>
{
/// <summary>
/// Initializes a new instance of the <see cref="ViewWithViewContractThatShouldNotLoad"/> class.
/// </summary>
public ViewWithViewContractThatShouldNotLoad()
{
throw new InvalidOperationException("This view should not be created.");
}

/// <inheritdoc />
object? IViewFor.ViewModel
{
get => ViewModel;
set => ViewModel = (ViewModelOne?)value;
}

/// <inheritdoc />
public ViewModelOne? ViewModel { get; set; }
}
}
35 changes: 35 additions & 0 deletions src/Splat.Common.Test/ViewThatShouldNotLoad.cs
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// 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.
/// </summary>
public sealed class ViewThatShouldNotLoad : IViewFor<ViewModelOne>
{
/// <summary>
/// Initializes a new instance of the <see cref="ViewThatShouldNotLoad"/> class.
/// </summary>
public ViewThatShouldNotLoad()
{
throw new InvalidOperationException("This view should not be created.");
}

/// <inheritdoc />
object? IViewFor.ViewModel
{
get => ViewModel;
set => ViewModel = (ViewModelOne?)value;
}

/// <inheritdoc />
public ViewModelOne? ViewModel { get; set; }
}
}
37 changes: 34 additions & 3 deletions src/Splat.DryIoc.Tests/DependencyResolverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@ public class DependencyResolverTests
/// <summary>
/// Shoulds the resolve nulls.
/// </summary>
[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<ArgumentNullException>(() => 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);
Expand Down Expand Up @@ -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
}

/// <summary>
/// Should resolve the views.
/// </summary>
[Fact]
public void DryIocDependencyResolver_Should_Register_But_Not_Create_Views()
{
var container = new Container();
container.UseDryIocDependencyResolver();

Splat.Locator.CurrentMutable.Register(() => new ViewThatShouldNotLoad(), typeof(IViewFor<ViewModelOne>));
Assert.Throws<InvalidOperationException>(() => Locator.Current.GetService<IViewFor<ViewModelOne>>());
}

/// <summary>
/// Should resolve the views.
/// </summary>
[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<ViewModelOne>), "name");
Assert.Throws<InvalidOperationException>(() => Locator.Current.GetService<IViewFor<ViewModelOne>>("name"));
}

/// <summary>
Expand Down Expand Up @@ -108,6 +137,8 @@ public void DryIocDependencyResolver_Should_Resolve_View_Models()
container.Register<ViewModelTwo>();
container.UseDryIocDependencyResolver();

Splat.Locator.CurrentMutable.Register(() => new ViewThatShouldNotLoad(), typeof(IViewFor<ViewModelOne>), "name");

var vmOne = Locator.Current.GetService<ViewModelOne>();
var vmTwo = Locator.Current.GetService<ViewModelTwo>();

Expand Down Expand Up @@ -255,7 +286,7 @@ public void DryIocDependencyResolver_PreInit_Should_ReturnRegisteredLogger()
/// DryIoc dependency resolver should resolve after duplicate keyed registratoion.
/// </summary>
[Fact]
public void DryIocDependencyResolver_Should_Resolve_AfterDuplicateKeyedRegistratoion()
public void DryIocDependencyResolver_Should_Resolve_AfterDuplicateKeyedRegistration()
{
var container = new Container();
container.UseDryIocDependencyResolver();
Expand Down
Loading

0 comments on commit 8b4e557

Please sign in to comment.