Remove Try from TryCreateDelegateToMixin

From the user's point of view, there's no reason to expect `TryCreate- DelegateToMixin` to fail once a proxy with `options.AddDelegate[Type]- Mixin` has been created. If the method *does* fail, there will be no indication what might have gone wrong, and user code likely won't be able to proceed; so why not just let DynamicProxy throw an exception.
castleproject · Apr 2, 2019 · f147680 · f147680
1 parent 2e0d8b2
commit f147680
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 50 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,7 +6,7 @@ Enhancements:
 - Added trace logging level below Debug; maps to Trace in log4net/NLog, and Verbose in Serilog (@pi3k14, #404)
 - Recognize read-only parameters by the `In` modreq (@zvirja, #406)
 - DictionaryAdapter: Exposed GetAdapter overloads with NameValueCollection parameter in .NET Standard (@rzontar, #423)
-- Ability to add delegate mixins to proxies using `ProxyGenerationOptions.AddDelegate[Type]Mixin`. You can bind to the mixed-in `Invoke` methods on the proxy using `ProxyUtil.TryCreateDelegateToMixin`. (@stakx, #436)
+- Ability to add delegate mixins to proxies using `ProxyGenerationOptions.AddDelegate[Type]Mixin`. You can bind to the mixed-in `Invoke` methods on the proxy using `ProxyUtil.CreateDelegateToMixin`. (@stakx, #436)
 - New `IInvocation.CaptureProceedInfo()` method to enable better implementations of asynchronous interceptors (@stakx, #439)
 
 Deprecations:

diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DelegateMixinTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DelegateMixinTestCase.cs
@@ -96,7 +96,8 @@ public void ProxyGenerator_CreateClassProxy_can_create_callable_delegate_proxy_w
 			var interceptor = new Interceptor();
 
 			var proxy = generator.CreateClassProxy(typeof(object), options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			action.Invoke();
 			Assert.AreSame(typeof(Action).GetTypeInfo().GetMethod("Invoke"), interceptor.LastInvocation.Method);
@@ -111,7 +112,8 @@ public void ProxyGenerator_CreateInterfaceProxyWithoutTarget_can_create_callable
 			var interceptor = new Interceptor();
 
 			var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IComparable), options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			action.Invoke();
 			Assert.AreSame(typeof(Action).GetTypeInfo().GetMethod("Invoke"), interceptor.LastInvocation.Method);
@@ -128,7 +130,8 @@ public void ProxyGenerator_CreateInterfaceProxyWithTarget_can_create_callable_de
 			var interceptor = new Interceptor();
 
 			var proxy = generator.CreateInterfaceProxyWithTarget(typeof(IComparable), target, options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			action.Invoke();
 			Assert.AreSame(typeof(Action).GetTypeInfo().GetMethod("Invoke"), interceptor.LastInvocation.Method);
@@ -143,7 +146,8 @@ public void ProxyGenerator_CreateClassProxy_cannot_proceed_to_delegate_type_mixi
 			var interceptor = new Interceptor(shouldProceed: true);
 
 			var proxy = generator.CreateClassProxy(typeof(object), options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			Assert.Throws<NotImplementedException>(() => action.Invoke());
 		}
@@ -157,7 +161,8 @@ public void ProxyGenerator_CreateInterfaceProxyWithoutTarget_cannot_proceed_to_d
 			var interceptor = new Interceptor(shouldProceed: true);
 
 			var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IComparable), options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			Assert.Throws<NotImplementedException>(() => action.Invoke());
 		}
@@ -173,7 +178,8 @@ public void ProxyGenerator_CreateInterfaceProxyWithTarget_cannot_proceed_to_dele
 			var interceptor = new Interceptor(shouldProceed: true);
 
 			var proxy = generator.CreateInterfaceProxyWithTarget(typeof(IComparable), target, options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			Assert.Throws<NotImplementedException>(() => action.Invoke());
 		}
@@ -189,7 +195,8 @@ public void ProxyGenerator_CreateClassProxy_can_proceed_to_delegate_mixin()
 			var interceptor = new Interceptor(shouldProceed: true);
 
 			var proxy = generator.CreateClassProxy(typeof(object), options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			action.Invoke();
 			Assert.True(target.MethodInvoked);
@@ -206,7 +213,8 @@ public void ProxyGenerator_CreateInterfaceProxyWithoutTarget_can_proceed_to_dele
 			var interceptor = new Interceptor(shouldProceed: true);
 
 			var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IComparable), options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			action.Invoke();
 			Assert.True(target.MethodInvoked);
@@ -223,7 +231,8 @@ public void ProxyGenerator_CreateInterfaceProxyWithTarget_can_proceed_to_delegat
 			var interceptor = new Interceptor(shouldProceed: true);
 
 			var proxy = generator.CreateInterfaceProxyWithTarget(typeof(IComparable), target, options, interceptor);
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 
 			action.Invoke();
 			Assert.True(target.MethodInvoked);
@@ -240,10 +249,12 @@ public void Can_mixin_several_different_delegate_types_simultaneously()
 
 			var proxy = generator.CreateClassProxy(typeof(object), options, interceptor);
 
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
+			Assert.NotNull(action);
 			action.Invoke();
 
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action<int> intAction));
+			var intAction = ProxyUtil.CreateDelegateToMixin<Action<int>>(proxy);
+			Assert.NotNull(action);
 			intAction.Invoke(42);
 		}
 

diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/ProxyUtilTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/ProxyUtilTestCase.cs
@@ -25,58 +25,58 @@ namespace Castle.DynamicProxy.Tests
 	public class ProxyUtilTestCase
 	{
 		[Test]
-		public void TryCreateDelegateToMixin_when_given_null_for_proxy_throws_ArgumentNullException()
+		public void CreateDelegateToMixin_when_given_null_for_proxy_throws_ArgumentNullException()
 		{
 			var _ = typeof(Action);
-			Assert.Throws<ArgumentNullException>(() => ProxyUtil.TryCreateDelegateToMixin(null, _, out var __));
+			Assert.Throws<ArgumentNullException>(() => ProxyUtil.CreateDelegateToMixin(null, _));
 		}
 
 		[Test]
-		public void TryCreateDelegateToMixin_when_given_null_for_delegateType_throws_ArgumentNullException()
+		public void CreateDelegateToMixin_when_given_null_for_delegateType_throws_ArgumentNullException()
 		{
 			var _ = new object();
-			Assert.Throws<ArgumentNullException>(() => ProxyUtil.TryCreateDelegateToMixin(_, null, out var __));
+			Assert.Throws<ArgumentNullException>(() => ProxyUtil.CreateDelegateToMixin(_, null));
 		}
 
 		[Test]
-		public void TryCreateDelegateToMixin_when_given_non_delegate_type_throws_ArgumentException()
+		public void CreateDelegateToMixin_when_given_non_delegate_type_throws_ArgumentException()
 		{
 			var _ = new object();
-			Assert.Throws<ArgumentException>(() => ProxyUtil.TryCreateDelegateToMixin(_, typeof(Exception), out var __));
+			Assert.Throws<ArgumentException>(() => ProxyUtil.CreateDelegateToMixin(_, typeof(Exception)));
 		}
 
 		[Test]
-		public void TryCreateDelegateToMixin_when_given_valid_arguments_succeeds()
+		public void CreateDelegateToMixin_when_given_valid_arguments_succeeds()
 		{
 			var proxy = new FakeProxyWithInvokeMethods();
-			Assert.True(ProxyUtil.TryCreateDelegateToMixin(proxy, typeof(Action), out _));
+			Assert.NotNull(ProxyUtil.CreateDelegateToMixin(proxy, typeof(Action)));
 		}
 
 		[Test]
-		public void TryCreateDelegateToMixin_returns_false_if_no_suitable_Invoke_method_found()
+		public void CreateDelegateToMixin_throws_MissingMethodException_if_no_suitable_Invoke_method_found()
 		{
 			var proxy = new FakeProxyWithInvokeMethods();
-			Assert.False(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action<bool> boolAction));
+			Assert.Throws<MissingMethodException>(() => ProxyUtil.CreateDelegateToMixin<Action<bool>>(proxy));
 		}
 
 		[Test]
-		public void TryCreateDelegateToMixin_returns_invokable_delegate()
+		public void CreateDelegateToMixin_returns_invokable_delegate()
 		{
 			var proxy = new FakeProxyWithInvokeMethods();
-			Assume.That(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
 			action.Invoke();
 		}
 
 		[Test]
-		public void TryCreateDelegateToMixin_can_deal_with_multiple_Invoke_overloads()
+		public void CreateDelegateToMixin_can_deal_with_multiple_Invoke_overloads()
 		{
 			var proxy = new FakeProxyWithInvokeMethods();
 
-			Assume.That(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action action));
+			var action = ProxyUtil.CreateDelegateToMixin<Action>(proxy);
 			action.Invoke();
 			Assert.AreEqual("Invoke()", proxy.LastInvocation);
 
-			Assume.That(ProxyUtil.TryCreateDelegateToMixin(proxy, out Action<int> intAction));
+			var intAction = ProxyUtil.CreateDelegateToMixin<Action<int>>(proxy);
 			intAction.Invoke(42);
 			Assert.AreEqual("Invoke(42)", proxy.LastInvocation);
 		}

diff --git a/src/Castle.Core/DynamicProxy/ProxyUtil.cs b/src/Castle.Core/DynamicProxy/ProxyUtil.cs
@@ -34,36 +34,31 @@ public static class ProxyUtil
 		private static readonly SynchronizedDictionary<Assembly, bool> internalsVisibleToDynamicProxy = new SynchronizedDictionary<Assembly, bool>();
 
 		/// <summary>
-		///   Attempts to create a delegate of the specified type to a suitable `Invoke` method
+		///   Creates a delegate of the specified type to a suitable `Invoke` method
 		///   on the given <paramref name="proxy"/> instance.
 		/// </summary>
 		/// <param name="proxy">The proxy instance to which the delegate should be bound.</param>
 		/// <typeparam name="TDelegate">The type of delegate that should be created.</typeparam>
-		/// <param name="delegate">The produced delegate if the method succeeds; otherwise <see langword="null"/>.</param>
-		/// <returns><see langword="true"/> if the method succeeds; otherwise <see langword="false"/>.</returns>
-		public static bool TryCreateDelegateToMixin<TDelegate>(object proxy, out TDelegate @delegate)
+		/// <exception cref="MissingMethodException">
+		///   The <paramref name="proxy"/> does not have an Invoke method that is compatible with
+		///   the requested <typeparamref name="TDelegate"/> type.
+		/// </exception>
+		public static TDelegate CreateDelegateToMixin<TDelegate>(object proxy)
 		{
-			if (TryCreateDelegateToMixin(proxy, typeof(TDelegate), out var d))
-			{
-				@delegate = (TDelegate)(object)d;
-				return true;
-			}
-			else
-			{
-				@delegate = default(TDelegate);
-				return false;
-			}
+			return (TDelegate)(object)CreateDelegateToMixin(proxy, typeof(TDelegate));
 		}
 
 		/// <summary>
-		///   Attempts to create a delegate of the specified type to a suitable `Invoke` method
+		///   Creates a delegate of the specified type to a suitable `Invoke` method
 		///   on the given <paramref name="proxy"/> instance.
 		/// </summary>
 		/// <param name="proxy">The proxy instance to which the delegate should be bound.</param>
 		/// <param name="delegateType">The type of delegate that should be created.</param>
-		/// <param name="delegate">The produced delegate if the method succeeds; otherwise <see langword="null"/>.</param>
-		/// <returns><see langword="true"/> if the method succeeds; otherwise <see langword="false"/>.</returns>
-		public static bool TryCreateDelegateToMixin(object proxy, Type delegateType, out Delegate @delegate)
+		/// <exception cref="MissingMethodException">
+		///   The <paramref name="proxy"/> does not have an Invoke method that is compatible with
+		///   the requested <paramref name="delegateType"/>.
+		/// </exception>
+		public static Delegate CreateDelegateToMixin(object proxy, Type delegateType)
 		{
 			if (proxy == null) throw new ArgumentNullException(nameof(proxy));
 			if (delegateType == null) throw new ArgumentNullException(nameof(delegateType));
@@ -79,17 +74,16 @@ public static bool TryCreateDelegateToMixin(object proxy, Type delegateType, out
 
 			if (proxiedInvokeMethod == null)
 			{
-				@delegate = null;
-				return false;
+				throw new MissingMethodException("The proxy does not have an Invoke method " +
+				                                 "that is compatible with the requested delegate type.");
 			}
 			else
 			{
 #if FEATURE_NETCORE_REFLECTION_API
-				@delegate = proxiedInvokeMethod.CreateDelegate(delegateType, proxy);
+				return proxiedInvokeMethod.CreateDelegate(delegateType, proxy);
 #else
-				@delegate = Delegate.CreateDelegate(delegateType, proxy, proxiedInvokeMethod);
+				return Delegate.CreateDelegate(delegateType, proxy, proxiedInvokeMethod);
 #endif
-				return true;
 			}
 		}