Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On-demand SetupAllProperties #826

Merged
merged 18 commits into from
Jun 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).


## Unreleased

#### Changed

* Improved performance for `Mock.Of<T>` and `mock.SetupAllProperties()` as the latter now performs property setups just-in-time, instead of as an ahead-of-time batch operation. (@vanashimko, #826)


## 4.11.0 (2019-05-28)

Same as 4.11.0-rc2. See changelog entries for the below two pre-release versions.
Expand Down
6 changes: 6 additions & 0 deletions src/Moq/AsInterface.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ public override DefaultValueProvider DefaultValueProvider
set => this.owner.DefaultValueProvider = value;
}

internal override DefaultValueProvider AutoSetupPropertiesDefaultValueProvider
{
get => this.owner.AutoSetupPropertiesDefaultValueProvider;
set => this.owner.AutoSetupPropertiesDefaultValueProvider = value;
}

internal override EventHandlerCollection EventHandlers => this.owner.EventHandlers;

internal override Type[] InheritedInterfaces => this.owner.InheritedInterfaces;
Expand Down
60 changes: 10 additions & 50 deletions src/Moq/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ public static bool IsPropertySetter(this MethodInfo method)
return method.IsSpecialName && method.Name.StartsWith("set_", StringComparison.Ordinal);
}

public static bool IsPropertyAccessor(this MethodInfo method)
{
return method.IsPropertyGetter() || method.IsPropertySetter();
}

public static bool IsPropertyIndexerAccessor(this MethodInfo method)
{
return method.IsPropertyIndexerGetter() || method.IsPropertyIndexerSetter();
}

// NOTE: The following two methods used to first check whether `method.IsSpecialName` was set
// as a quick guard against non-event accessor methods. This was removed in commit 44070a90
// to "increase compatibility with F# and COM". More specifically:
Expand Down Expand Up @@ -223,56 +233,6 @@ private static MethodInfo GetInvokeMethodFromUntypedDelegateCallback(Delegate ca
}
}

/// <summary>
/// Gets all properties of the specified type in depth-first order.
/// That is, properties of the furthest ancestors are returned first,
/// and the type's own properties are returned last.
/// </summary>
/// <param name="type">The type whose properties are to be returned.</param>
internal static List<PropertyInfo> GetAllPropertiesInDepthFirstOrder(this Type type)
{
var properties = new List<PropertyInfo>();
var none = new HashSet<Type>();

type.AddPropertiesInDepthFirstOrderTo(properties, typesAlreadyVisited: none);

return properties;
}

/// <summary>
/// This is a helper method supporting <see cref="GetAllPropertiesInDepthFirstOrder(Type)"/>
/// and is not supposed to be called directly.
/// </summary>
private static void AddPropertiesInDepthFirstOrderTo(this Type type, List<PropertyInfo> properties, HashSet<Type> typesAlreadyVisited)
{
if (!typesAlreadyVisited.Contains(type))
{
// make sure we do not process properties of the current type twice:
typesAlreadyVisited.Add(type);

//// follow down axis 1: add properties of base class. note that this is currently
//// disabled, since it wasn't done previously and this can only result in changed
//// behavior.
//if (type.BaseType != null)
//{
// type.BaseType.AddPropertiesInDepthFirstOrderTo(properties, typesAlreadyVisited);
//}

// follow down axis 2: add properties of inherited / implemented interfaces:
var superInterfaceTypes = type.GetInterfaces();
foreach (var superInterfaceType in superInterfaceTypes)
{
superInterfaceType.AddPropertiesInDepthFirstOrderTo(properties, typesAlreadyVisited);
}

// add own properties:
foreach (var property in type.GetProperties())
{
properties.Add(property);
}
}
}

public static bool TryFind(this IEnumerable<Setup> innerMockSetups, InvocationShape expectation, out Setup setup)
{
Debug.Assert(innerMockSetups.All(s => s.ReturnsInnerMock(out _)));
Expand Down
95 changes: 91 additions & 4 deletions src/Moq/Interception/InterceptionAspects.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;

namespace Moq
Expand Down Expand Up @@ -121,10 +122,6 @@ public static bool Handle(Invocation invocation, Mock mock)
matchedSetup.Execute(invocation);
return true;
}
else if (mock.Behavior == MockBehavior.Strict)
{
throw MockException.NoSetup(invocation);
}
else
{
return false;
Expand Down Expand Up @@ -331,4 +328,94 @@ public static void Handle(Invocation invocation, Mock mock)
}
}
}

internal static class HandleAutoSetupProperties
{
private static readonly int AccessorPrefixLength = "?et_".Length; // get_ or set_

public static bool Handle(Invocation invocation, Mock mock)
{
if (mock.AutoSetupPropertiesDefaultValueProvider == null)
{
return false;
}

MethodInfo invocationMethod = invocation.Method;
if (invocationMethod.IsPropertyAccessor() && !invocationMethod.IsPropertyIndexerAccessor())
{
string propertyNameToSearch = invocationMethod.Name.Substring(AccessorPrefixLength);
PropertyInfo property = invocationMethod.DeclaringType.GetProperty(propertyNameToSearch);

if (property == null)
{
return false;
}

// Should ignore write-only properties, as they will be handled by Return aspect
// unless if the mock is strict; for those, interception terminates by FailForStrictMock aspect.
if (invocationMethod.IsPropertySetter() && !property.CanRead)
{
return false;
}

var getter = property.GetGetMethod(true);
var expression = GetPropertyExpression(invocationMethod.DeclaringType, property);

object propertyValue = CreateInitialPropertyValue(mock, getter);

Setup getterSetup = new AutoImplementedPropertyGetterSetup(expression, getter, () => propertyValue);
mock.Setups.Add(getterSetup);

Setup setterSetup = null;
if (property.CanWrite)
{
MethodInfo setter = property.GetSetMethod(nonPublic: true);
setterSetup = new AutoImplementedPropertySetterSetup(expression, setter, (newValue) =>
{
propertyValue = newValue;
});
mock.Setups.Add(setterSetup);
}

Setup setupToExecute = invocationMethod.IsPropertyGetter() ? getterSetup : setterSetup;
setupToExecute.Execute(invocation);

return true;
}
else
{
return false;
}
}

private static object CreateInitialPropertyValue(Mock mock, MethodInfo getter)
{
object initialValue = mock.GetDefaultValue(getter, out Mock innerMock,
useAlternateProvider: mock.AutoSetupPropertiesDefaultValueProvider);

if (innerMock != null)
{
Mock.SetupAllProperties(innerMock, mock.AutoSetupPropertiesDefaultValueProvider);
}

return initialValue;
}

private static LambdaExpression GetPropertyExpression(Type mockType, PropertyInfo property)
{
var param = Expression.Parameter(mockType, "m");
return Expression.Lambda(Expression.MakeMemberAccess(param, property), param);
}
}

internal static class FailForStrictMock
{
public static void Handle(Invocation invocation, Mock mock)
{
if (mock.Behavior == MockBehavior.Strict)
{
throw MockException.NoSetup(invocation);
}
}
}
}
7 changes: 7 additions & 0 deletions src/Moq/Interception/Mock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ void IInterceptor.Intercept(Invocation invocation)
{
return;
}

if (HandleAutoSetupProperties.Handle(invocation, this))
{
return;
}

FailForStrictMock.Handle(invocation, this);

Return.Handle(invocation, this);
}
Expand Down
2 changes: 2 additions & 0 deletions src/Moq/Mock.Generic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ public override DefaultValueProvider DefaultValueProvider
set => this.defaultValueProvider = value ?? throw new ArgumentNullException(nameof(value));
}

internal override DefaultValueProvider AutoSetupPropertiesDefaultValueProvider { get; set; }

internal override EventHandlerCollection EventHandlers => this.eventHandlers;

internal override List<Type> AdditionalInterfaces => this.additionalInterfaces;
Expand Down
94 changes: 15 additions & 79 deletions src/Moq/Mock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@ public DefaultValue DefaultValue
/// </summary>
public abstract DefaultValueProvider DefaultValueProvider { get; set; }

/// <summary>
/// The <see cref="Moq.DefaultValueProvider"/> used to initialize automatically stubbed properties.
/// It is equal to the value of <see cref="Moq.Mock.DefaultValueProvider"/> at the time when
/// <see cref="SetupAllProperties"/> was last called.
/// </summary>
internal abstract DefaultValueProvider AutoSetupPropertiesDefaultValueProvider { get; set; }

internal abstract SetupCollection Setups { get; }

/// <summary>
Expand Down Expand Up @@ -494,88 +501,17 @@ private static TSetup SetupRecursive<TSetup>(Mock mock, LambdaExpression express

internal static void SetupAllProperties(Mock mock)
{
Mock.SetupAllProperties(mock, mock.DefaultValueProvider);
// ^^^^^^^^^^^^^^^^^^^^^^^^^
// `SetupAllProperties` no longer performs eager recursive property setup like in previous versions.
// If `mock` uses `DefaultValue.Mock`, mocked sub-objects are now only constructed when queried for
// the first time. In order for `SetupAllProperties`'s new mode of operation to be indistinguishable
// from how it worked previously, it's important to capture the default value provider at this precise
// moment, since it might be changed later (before queries to properties of a mockable type).
SetupAllProperties(mock, mock.DefaultValueProvider);
}

private static void SetupAllProperties(Mock mock, DefaultValueProvider defaultValueProvider)
internal static void SetupAllProperties(Mock mock, DefaultValueProvider defaultValueProvider)
stakx marked this conversation as resolved.
Show resolved Hide resolved
{
var mockType = mock.MockedType;

var properties =
mockType
.GetAllPropertiesInDepthFirstOrder()
// ^ Depth-first traversal is important because properties in derived interfaces
// that shadow properties in base interfaces should be set up last. This
// enables the use case where a getter-only property is redeclared in a derived
// interface as a getter-and-setter property.
.Where(p =>
p.CanRead && p.CanOverrideGet() &&
p.CanWrite == p.CanOverrideSet() &&
// ^ This condition will be true for two kinds of properties:
// (a) those that are read-only; and
// (b) those that are writable and whose setter can be overridden.
p.GetIndexParameters().Length == 0 &&
ProxyFactory.Instance.IsMethodVisible(p.GetGetMethod(), out _))
.Distinct();

foreach (var property in properties)
{
var expression = GetPropertyExpression(mockType, property);
var getter = property.GetGetMethod(true);

object value = null;
bool valueNotSet = true;

mock.Setups.Add(new AutoImplementedPropertyGetterSetup(expression, getter, () =>
{
if (valueNotSet)
{
object initialValue;
Mock innerMock;
try
{
initialValue = mock.GetDefaultValue(getter, out innerMock, useAlternateProvider: defaultValueProvider);
}
catch
{
// Since this method performs a batch operation, a single failure of the default value
// provider should not tear down the whole operation. The empty default value provider
// is a safe fallback because it does not throw.
initialValue = mock.GetDefaultValue(getter, out innerMock, useAlternateProvider: DefaultValueProvider.Empty);
}

if (innerMock != null)
{
Mock.SetupAllProperties(innerMock, defaultValueProvider);
}

value = initialValue;
valueNotSet = false;
}
return value;
}));

if (property.CanWrite)
{
mock.Setups.Add(new AutoImplementedPropertySetterSetup(expression, property.GetSetMethod(true), (newValue) =>
{
value = newValue;
valueNotSet = false;
}));
}
}
}

private static LambdaExpression GetPropertyExpression(Type mockType, PropertyInfo property)
{
var param = Expression.Parameter(mockType, "m");
return Expression.Lambda(Expression.MakeMemberAccess(param, property), param);
mock.AutoSetupPropertiesDefaultValueProvider = defaultValueProvider;
// `SetupAllProperties` no longer performs properties setup like in previous versions.
// Instead it just enables a switch to setup properties on-demand at the moment of first access.
// In order for `SetupAllProperties`'s new mode of operation to be indistinguishable
// from how it worked previously, it's important to capture the default value provider at this precise
// moment, since it might be changed later (before queries to properties).
ishimko marked this conversation as resolved.
Show resolved Hide resolved
}

#endregion
Expand Down
4 changes: 2 additions & 2 deletions tests/Moq.Tests/MockFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1250,12 +1250,12 @@ public void Accessing_property_of_bad_serializable_type_throws()
}

[Fact]
public void Setting_up_property_of_bad_serializable_type_with_SetupAllProperties_does_not_throw()
public void Accessing_property_of_bad_serializable_type_after_SetupAllProperties_throws()
{
var mock = new Mock<IHaveBadSerializableProperty>() { DefaultValue = DefaultValue.Mock };
mock.SetupAllProperties();

Assert.Null(mock.Object.BadSerializable);
Assert.ThrowsAny<Exception>(() => mock.Object.BadSerializable);
}
#endif
}
Expand Down
Loading