Merge pull request #826 from vanashimko/lazy-setup-all-properties

On-demand SetupAllProperties

CHANGELOG.md

@@ -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.

src/Moq/AsInterface.cs

@@ -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;

src/Moq/Extensions.cs

@@ -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:
@@ -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 _)));

src/Moq/Interception/InterceptionAspects.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using System.Linq.Expressions;
 using System.Reflection;
 
 namespace Moq
@@ -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;
@@ -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);
+			}
+		}
+	}
 }

src/Moq/Interception/Mock.cs

@@ -23,6 +23,13 @@ void IInterceptor.Intercept(Invocation invocation)
 			{
 				return;
 			}
+
+			if (HandleAutoSetupProperties.Handle(invocation, this))
+			{
+				return;
+			}
+
+			FailForStrictMock.Handle(invocation, this);
 
 			Return.Handle(invocation, this);
 		}

src/Moq/Mock.Generic.cs

@@ -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;

src/Moq/Mock.cs

@@ -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>
@@ -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)
 		{
-			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).
 		}
 
 		#endregion

tests/Moq.Tests/MockFixture.cs

@@ -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
 	}