Use common ObjectMethodExecutor (#440)

src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs

@@ -8,9 +8,9 @@ using System.Linq;
 using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.SignalR.Internal;
 using Microsoft.AspNetCore.Sockets;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Internal;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
@@ -263,7 +263,7 @@ namespace Microsoft.AspNetCore.SignalR
                     object result = null;
                     if (methodExecutor.IsMethodAsync)
                     {
-                        if (methodExecutor.TaskGenericType == null)
+                        if (methodExecutor.MethodReturnType == typeof(Task))
                         {
                             await (Task)methodExecutor.Execute(hub, invocationDescriptor.Arguments);
                         }
@@ -366,7 +366,7 @@ namespace Microsoft.AspNetCore.SignalR
             public HubMethodDescriptor(ObjectMethodExecutor methodExecutor)
             {
                 MethodExecutor = methodExecutor;
-                ParameterTypes = methodExecutor.ActionParameters.Select(p => p.ParameterType).ToArray();
+                ParameterTypes = methodExecutor.MethodParameters.Select(p => p.ParameterType).ToArray();
             }
 
             public ObjectMethodExecutor MethodExecutor { get; }

src/Microsoft.AspNetCore.SignalR/Internal/ObjectMethodExecutor.cs

@@ -1,258 +0,0 @@
-// Copyright (c) .NET Foundation. All rights reserved.
-// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
-
-using System;
-using System.Collections.Generic;
-using System.ComponentModel;
-using System.Linq;
-using System.Linq.Expressions;
-using System.Reflection;
-using System.Threading.Tasks;
-using Microsoft.Extensions.Internal;
-
-namespace Microsoft.AspNetCore.SignalR.Internal
-{
-    internal class ObjectMethodExecutor
-    {
-        private readonly object[] _parameterDefaultValues;
-        private readonly ActionExecutorAsync _executorAsync;
-        private readonly ActionExecutor _executor;
-
-        private static readonly MethodInfo _convertOfTMethod =
-            typeof(ObjectMethodExecutor).GetRuntimeMethods().Single(methodInfo => methodInfo.Name == nameof(ObjectMethodExecutor.Convert));
-
-        private ObjectMethodExecutor(MethodInfo methodInfo, TypeInfo targetTypeInfo)
-        {
-            if (methodInfo == null)
-            {
-                throw new ArgumentNullException(nameof(methodInfo));
-            }
-
-            MethodInfo = methodInfo;
-            TargetTypeInfo = targetTypeInfo;
-            ActionParameters = methodInfo.GetParameters();
-            MethodReturnType = methodInfo.ReturnType;
-            IsMethodAsync = typeof(Task).IsAssignableFrom(MethodReturnType);
-            TaskGenericType = IsMethodAsync ? GetTaskInnerTypeOrNull(MethodReturnType) : null;
-
-            if (IsMethodAsync && TaskGenericType != null)
-            {
-                // For backwards compatibility we're creating a sync-executor for an async method.
-                _executor = GetExecutor(methodInfo, targetTypeInfo);
-                _executorAsync = GetExecutorAsync(TaskGenericType, methodInfo, targetTypeInfo);
-            }
-            else
-            {
-                _executor = GetExecutor(methodInfo, targetTypeInfo);
-            }
-
-            _parameterDefaultValues = GetParameterDefaultValues(ActionParameters);
-        }
-
-        private delegate Task<object> ActionExecutorAsync(object target, object[] parameters);
-
-        private delegate object ActionExecutor(object target, object[] parameters);
-
-        private delegate void VoidActionExecutor(object target, object[] parameters);
-
-        public MethodInfo MethodInfo { get; }
-
-        public ParameterInfo[] ActionParameters { get; }
-
-        public TypeInfo TargetTypeInfo { get; }
-
-        public Type TaskGenericType { get; }
-
-        // This field is made internal set because it is set in unit tests.
-        public Type MethodReturnType { get; internal set; }
-
-        public bool IsMethodAsync { get; }
-
-        public static ObjectMethodExecutor Create(MethodInfo methodInfo, TypeInfo targetTypeInfo)
-        {
-            var executor = new ObjectMethodExecutor(methodInfo, targetTypeInfo);
-            return executor;
-        }
-
-        public Task<object> ExecuteAsync(object target, object[] parameters)
-        {
-            return _executorAsync(target, parameters);
-        }
-
-        public object Execute(object target, object[] parameters)
-        {
-            return _executor(target, parameters);
-        }
-
-        public object GetDefaultValueForParameter(int index)
-        {
-            if (index < 0 || index > ActionParameters.Length - 1)
-            {
-                throw new ArgumentOutOfRangeException(nameof(index));
-            }
-
-            return _parameterDefaultValues[index];
-        }
-
-        private static ActionExecutor GetExecutor(MethodInfo methodInfo, TypeInfo targetTypeInfo)
-        {
-            // Parameters to executor
-            var targetParameter = Expression.Parameter(typeof(object), "target");
-            var parametersParameter = Expression.Parameter(typeof(object[]), "parameters");
-
-            // Build parameter list
-            var parameters = new List<Expression>();
-            var paramInfos = methodInfo.GetParameters();
-            for (int i = 0; i < paramInfos.Length; i++)
-            {
-                var paramInfo = paramInfos[i];
-                var valueObj = Expression.ArrayIndex(parametersParameter, Expression.Constant(i));
-                var valueCast = Expression.Convert(valueObj, paramInfo.ParameterType);
-
-                // valueCast is "(Ti) parameters[i]"
-                parameters.Add(valueCast);
-            }
-
-            // Call method
-            MethodCallExpression methodCall;
-
-            if (!methodInfo.IsStatic)
-            {
-                var instanceCast = Expression.Convert(targetParameter, targetTypeInfo.AsType());
-                methodCall = Expression.Call(instanceCast, methodInfo, parameters);
-            }
-            else
-            {
-                methodCall = Expression.Call(null, methodInfo, parameters);
-            }
-
-            // methodCall is "((Ttarget) target) method((T0) parameters[0], (T1) parameters[1], ...)"
-            // Create function
-            if (methodCall.Type == typeof(void))
-            {
-                var lambda = Expression.Lambda<VoidActionExecutor>(methodCall, targetParameter, parametersParameter);
-                var voidExecutor = lambda.Compile();
-                return WrapVoidAction(voidExecutor);
-            }
-            else
-            {
-                // must coerce methodCall to match ActionExecutor signature
-                var castMethodCall = Expression.Convert(methodCall, typeof(object));
-                var lambda = Expression.Lambda<ActionExecutor>(castMethodCall, targetParameter, parametersParameter);
-                return lambda.Compile();
-            }
-        }
-
-        private static ActionExecutor WrapVoidAction(VoidActionExecutor executor)
-        {
-            return delegate (object target, object[] parameters)
-            {
-                executor(target, parameters);
-                return null;
-            };
-        }
-
-        private static ActionExecutorAsync GetExecutorAsync(Type taskInnerType, MethodInfo methodInfo, TypeInfo targetTypeInfo)
-        {
-            // Parameters to executor
-            var targetParameter = Expression.Parameter(typeof(object), "target");
-            var parametersParameter = Expression.Parameter(typeof(object[]), "parameters");
-
-            // Build parameter list
-            var parameters = new List<Expression>();
-            var paramInfos = methodInfo.GetParameters();
-            for (int i = 0; i < paramInfos.Length; i++)
-            {
-                var paramInfo = paramInfos[i];
-                var valueObj = Expression.ArrayIndex(parametersParameter, Expression.Constant(i));
-                var valueCast = Expression.Convert(valueObj, paramInfo.ParameterType);
-
-                // valueCast is "(Ti) parameters[i]"
-                parameters.Add(valueCast);
-            }
-
-            // Call method
-            var instanceCast = Expression.Convert(targetParameter, targetTypeInfo.AsType());
-            var methodCall = Expression.Call(instanceCast, methodInfo, parameters);
-
-            var coerceMethodCall = GetCoerceMethodCallExpression(taskInnerType, methodCall, methodInfo);
-            var lambda = Expression.Lambda<ActionExecutorAsync>(coerceMethodCall, targetParameter, parametersParameter);
-            return lambda.Compile();
-        }
-
-        // We need to CoerceResult as the object value returned from methodInfo.Invoke has to be cast to a Task<T>.
-        // This is necessary to enable calling await on the returned task.
-        // i.e we need to write the following var result = await (Task<ActualType>)mInfo.Invoke.
-        // Returning Task<object> enables us to await on the result.
-        private static Expression GetCoerceMethodCallExpression(
-            Type taskValueType,
-            MethodCallExpression methodCall,
-            MethodInfo methodInfo)
-        {
-            var castMethodCall = Expression.Convert(methodCall, typeof(object));
-            // for: public Task<T> Action()
-            // constructs: return (Task<object>)Convert<T>((Task<T>)result)
-            var genericMethodInfo = _convertOfTMethod.MakeGenericMethod(taskValueType);
-            var genericMethodCall = Expression.Call(null, genericMethodInfo, castMethodCall);
-            var convertedResult = Expression.Convert(genericMethodCall, typeof(Task<object>));
-            return convertedResult;
-        }
-
-        /// <summary>
-        /// Cast Task of T to Task of object
-        /// </summary>
-        private static async Task<object> CastToObject<T>(Task<T> task)
-        {
-            return (object)await task;
-        }
-
-        private static Type GetTaskInnerTypeOrNull(Type type)
-        {
-            var genericType = ClosedGenericMatcher.ExtractGenericInterface(type, typeof(Task<>));
-
-            return genericType?.GenericTypeArguments[0];
-        }
-
-        private static Task<object> Convert<T>(object taskAsObject)
-        {
-            var task = (Task<T>)taskAsObject;
-            return CastToObject<T>(task);
-        }
-
-        private static object[] GetParameterDefaultValues(ParameterInfo[] parameters)
-        {
-            var values = new object[parameters.Length];
-
-            for (var i = 0; i < parameters.Length; i++)
-            {
-                var parameterInfo = parameters[i];
-                object defaultValue;
-
-                if (parameterInfo.HasDefaultValue)
-                {
-                    defaultValue = parameterInfo.DefaultValue;
-                }
-                else
-                {
-                    var defaultValueAttribute = parameterInfo
-                        .GetCustomAttribute<DefaultValueAttribute>(inherit: false);
- 
-                    if (defaultValueAttribute?.Value == null)
-                    {
-                        defaultValue = parameterInfo.ParameterType.GetTypeInfo().IsValueType
-                            ? Activator.CreateInstance(parameterInfo.ParameterType)
-                            : null;
-                    }
-                    else
-                    {
-                        defaultValue = defaultValueAttribute.Value;
-                    }
-                }
-
-                values[i] = defaultValue;
-            }
-
-            return values;
-        }
-    }
-}

src/Microsoft.AspNetCore.SignalR/Microsoft.AspNetCore.SignalR.csproj

@@ -16,6 +16,7 @@
     <ProjectReference Include="..\Microsoft.AspNetCore.SignalR.Common\Microsoft.AspNetCore.SignalR.Common.csproj" />
     <PackageReference Include="Microsoft.Extensions.TaskCache.Sources" Version="$(AspNetCoreVersion)" PrivateAssets="All" />
     <PackageReference Include="Microsoft.Extensions.ClosedGenericMatcher.Sources" Version="$(AspNetCoreVersion)" PrivateAssets="All" />
+    <PackageReference Include="Microsoft.Extensions.ObjectMethodExecutor.Sources" Version="$(AspNetCoreVersion)" PrivateAssets="All" />
     <PackageReference Include="Newtonsoft.Json" Version="$(JsonNetVersion)" />
   </ItemGroup>
 

test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs

@@ -185,28 +185,6 @@ namespace Microsoft.AspNetCore.SignalR.Tests
             }
         }
 
-        [Fact]
-        public async Task HubMethodCanBeStatic()
-        {
-            var serviceProvider = CreateServiceProvider();
-
-            var endPoint = serviceProvider.GetService<HubEndPoint<MethodHub>>();
-
-            using (var client = new TestClient(serviceProvider))
-            {
-                var endPointTask = endPoint.OnConnectedAsync(client.Connection);
-
-                var result = await client.Invoke<InvocationResultDescriptor>(nameof(MethodHub.StaticMethod)).OrTimeout();
-
-                Assert.Equal("fromStatic", result.Result);
-
-                // kill the connection
-                client.Dispose();
-
-                await endPointTask.OrTimeout();
-            }
-        }
-
         [Fact]
         public async Task HubMethodCanBeVoid()
         {
@@ -586,11 +564,6 @@ namespace Microsoft.AspNetCore.SignalR.Tests
                 return data;
             }
 
-            static public string StaticMethod()
-            {
-                return "fromStatic";
-            }
-
             public void VoidMethod()
             {
             }

test/Microsoft.AspNetCore.SignalR.Tests/ObjectMethodExecutorTests.cs

@@ -1,251 +0,0 @@
-// Copyright (c) .NET Foundation. All rights reserved.
-// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
-
-using System;
-using System.ComponentModel;
-using System.Reflection;
-using System.Threading.Tasks;
-using Microsoft.AspNetCore.SignalR.Internal;
-using Xunit;
-
-namespace Microsoft.AspNetCore.SignalR.Tests
-{
-    public class ObjectMethodExecutorTest
-    {
-        private TestObject _targetObject = new TestObject();
-        private TypeInfo targetTypeInfo = typeof(TestObject).GetTypeInfo();
-
-        [Fact]
-        public void ExecuteValueMethod()
-        {
-            var executor = GetExecutorForMethod("ValueMethod");
-            var result = executor.Execute(
-                _targetObject,
-                new object[] { 10, 20 });
-            Assert.Equal(30, (int)result);
-        }
-
-        [Fact]
-        public void ExecuteVoidValueMethod()
-        {
-            var executor = GetExecutorForMethod("VoidValueMethod");
-            var result = executor.Execute(
-                _targetObject,
-                new object[] { 10 });
-            Assert.Same(null, result);
-        }
-
-        [Fact]
-        public void ExecuteValueMethodWithReturnType()
-        {
-            var executor = GetExecutorForMethod("ValueMethodWithReturnType");
-            var result = executor.Execute(
-                _targetObject,
-                new object[] { 10 });
-            var resultObject = Assert.IsType<TestObject>(result);
-            Assert.Equal("Hello", resultObject.value);
-        }
-
-        [Fact]
-        public void ExecuteStaticValueMethodWithReturnType()
-        {
-            var executor = GetExecutorForMethod("StaticValueMethodWithReturnType");
-            var result = executor.Execute(
-                _targetObject,
-                new object[] { 10 });
-            var resultObject = Assert.IsType<TestObject>(result);
-            Assert.Equal("Hello", resultObject.value);
-        }
-
-        [Fact]
-        public void ExecuteValueMethodUpdateValue()
-        {
-            var executor = GetExecutorForMethod("ValueMethodUpdateValue");
-            var parameter = new TestObject();
-            var result = executor.Execute(
-                _targetObject,
-                new object[] { parameter });
-            var resultObject = Assert.IsType<TestObject>(result);
-            Assert.Equal("HelloWorld", resultObject.value);
-        }
-
-        [Fact]
-        public void ExecuteValueMethodWithReturnTypeThrowsException()
-        {
-            var executor = GetExecutorForMethod("ValueMethodWithReturnTypeThrowsException");
-            var parameter = new TestObject();
-            Assert.ThrowsAny<NotImplementedException>(
-                        () => executor.Execute(
-                            _targetObject,
-                            new object[] { parameter }));
-        }
-
-        [Fact]
-        public async Task ExecuteValueMethodAsync()
-        {
-            var executor = GetExecutorForMethod("ValueMethodAsync");
-            var result = await executor.ExecuteAsync(
-                _targetObject,
-                new object[] { 10, 20 });
-            Assert.Equal(30, (int)result);
-        }
-
-        [Fact]
-        public async Task ExecuteValueMethodWithReturnTypeAsync()
-        {
-            var executor = GetExecutorForMethod("ValueMethodWithReturnTypeAsync");
-            var result = await executor.ExecuteAsync(
-                _targetObject,
-                new object[] { 10 });
-            var resultObject = Assert.IsType<TestObject>(result);
-            Assert.Equal("Hello", resultObject.value);
-        }
-
-        [Fact]
-        public async Task ExecuteValueMethodUpdateValueAsync()
-        {
-            var executor = GetExecutorForMethod("ValueMethodUpdateValueAsync");
-            var parameter = new TestObject();
-            var result = await executor.ExecuteAsync(
-                _targetObject,
-                new object[] { parameter });
-            var resultObject = Assert.IsType<TestObject>(result);
-            Assert.Equal("HelloWorld", resultObject.value);
-        }
-
-        [Fact]
-        public async Task ExecuteValueMethodWithReturnTypeThrowsExceptionAsync()
-        {
-            var executor = GetExecutorForMethod("ValueMethodWithReturnTypeThrowsExceptionAsync");
-            var parameter = new TestObject();
-            await Assert.ThrowsAsync<NotImplementedException>(
-                    () => executor.ExecuteAsync(
-                            _targetObject,
-                            new object[] { parameter }));
-        }
-
-        [Theory]
-        [InlineData("EchoWithDefaultAttributes", new object[] { "hello", true, 10 })]
-        [InlineData("EchoWithDefaultValues", new object[] { "hello", true, 20 })]
-        [InlineData("EchoWithDefaultValuesAndAttributes", new object[] { "hello", 20 })]
-        [InlineData("EchoWithNoDefaultAttributesAndValues", new object[] { null, 0, false, null })]
-        [InlineData("StaticEchoWithDefaultVaules", new object[] { "hello", true, 20 })]
-        public void GetDefaultValueForParameters_ReturnsExpectedValues(string methodName, object[] expectedValues)
-        {
-            var executor = GetExecutorForMethod(methodName);
-            var defaultValues = new object[expectedValues.Length];
-
-            for (var index = 0; index < expectedValues.Length; index++)
-            {
-                defaultValues[index] = executor.GetDefaultValueForParameter(index);
-            }
-
-            Assert.Equal(expectedValues, defaultValues);
-        }
-
-        private ObjectMethodExecutor GetExecutorForMethod(string methodName)
-        {
-            var method = typeof(TestObject).GetMethod(methodName);
-            var executor = ObjectMethodExecutor.Create(method, targetTypeInfo);
-            return executor;
-        }
-
-        public class TestObject
-        {
-            public string value;
-            public int ValueMethod(int i, int j)
-            {
-                return i + j;
-            }
-
-            public void VoidValueMethod(int i)
-            {
-
-            }
-            public TestObject ValueMethodWithReturnType(int i)
-            {
-                return new TestObject() { value = "Hello" };
-            }
-
-            public TestObject ValueMethodWithReturnTypeThrowsException(TestObject i)
-            {
-                throw new NotImplementedException("Not Implemented Exception");
-            }
-
-            public TestObject ValueMethodUpdateValue(TestObject parameter)
-            {
-                parameter.value = "HelloWorld";
-                return parameter;
-            }
-
-            public Task<int> ValueMethodAsync(int i, int j)
-            {
-                return Task.FromResult<int>(i + j);
-            }
-
-            public async Task VoidValueMethodAsync(int i)
-            {
-                await ValueMethodAsync(3, 4);
-            }
-            public Task<TestObject> ValueMethodWithReturnTypeAsync(int i)
-            {
-                return Task.FromResult<TestObject>(new TestObject() { value = "Hello" });
-            }
-
-            public Task<TestObject> ValueMethodWithReturnTypeThrowsExceptionAsync(TestObject i)
-            {
-                throw new NotImplementedException("Not Implemented Exception");
-            }
-
-            public Task<TestObject> ValueMethodUpdateValueAsync(TestObject parameter)
-            {
-                parameter.value = "HelloWorld";
-                return Task.FromResult<TestObject>(parameter);
-            }
-
-            public static TestObject StaticValueMethodWithReturnType(int i)
-            {
-                return new TestObject() { value = "Hello" };
-            }
-
-            public string EchoWithDefaultAttributes(
-                [DefaultValue("hello")] string input1,
-                [DefaultValue(true)] bool input2,
-                [DefaultValue(10)] int input3)
-            {
-                return input1;
-            }
-
-            public string EchoWithDefaultValues(
-                string input1 = "hello",
-                bool input2 = true,
-                int input3 = 20)
-            {
-                return input1;
-            }
-
-            public string EchoWithDefaultValuesAndAttributes(
-                [DefaultValue("Hi")] string input1 = "hello",
-                [DefaultValue(10)] int input3 = 20)
-            {
-                return input1;
-            }
-
-            public string EchoWithNoDefaultAttributesAndValues(
-                string input1,
-                int input2,
-                bool input3,
-                TestObject input4)
-            {
-                return input1;
-            }
-
-            public static string StaticEchoWithDefaultVaules(string input1 = "hello",
-                bool input2 = true,
-                int input3 = 20)
-            {
-                return input1;
-            }
-        }
-    }
-}