From ef2dc5024feb965dbd1205c0904ca2114872420c Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 19 Sep 2019 10:48:09 -0700 Subject: [PATCH] Add support for JSInvokable methods on generic types (dotnet/extensions#2342) * Add support for JSInvokable methods on generic types Prior to this change, DotNetDispatcher cached the MethodInfo on the generic type definition. Using this would have required MethodInfo.MakeGenericMethod before the method was invoked. We could separately cache the result of this to avoid the reflection cost per invocation. Alternatively we could cache static and non-static MethodInfo instances separately which is what this change attempts to do. The big difference in the outcome is that this requires instance (non-static) JSInvokable methods to be only unique named within the type hierarchy as opposed to across all static and instance JSInvokable methods in an assembly. Fixes https://github.com/aspnet/Extensions/issues/1360 Fixes https://github.com/aspnet/AspNetCore/issues/9061 \n\nCommit migrated from https://github.com/dotnet/extensions/commit/659b604fb2e595d48a931a7ed831f6c38f035382 --- .../src/Infrastructure/DotNetDispatcher.cs | 83 ++++++++++++------ .../Infrastructure/DotNetDispatcherTest.cs | 87 ++++++++++++++++++- 2 files changed, 142 insertions(+), 28 deletions(-) diff --git a/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs b/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs index 6a3a4f8d5f..8d37905980 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs @@ -24,6 +24,9 @@ namespace Microsoft.JSInterop.Infrastructure private static readonly ConcurrentDictionary> _cachedMethodsByAssembly = new ConcurrentDictionary>(); + private static readonly ConcurrentDictionary> _cachedMethodsByType + = new ConcurrentDictionary>(); + /// /// Receives a call from JS to .NET, locating and invoking the specified method. /// @@ -129,9 +132,12 @@ namespace Microsoft.JSInterop.Infrastructure var methodIdentifier = callInfo.MethodIdentifier; AssemblyKey assemblyKey; + MethodInfo methodInfo; + Type[] parameterTypes; if (objectReference is null) { assemblyKey = new AssemblyKey(assemblyName); + (methodInfo, parameterTypes) = GetCachedMethodInfo(assemblyKey, methodIdentifier); } else { @@ -147,11 +153,9 @@ namespace Microsoft.JSInterop.Infrastructure return default; } - assemblyKey = new AssemblyKey(objectReference.Value.GetType().Assembly); + (methodInfo, parameterTypes) = GetCachedMethodInfo(objectReference, methodIdentifier); } - var (methodInfo, parameterTypes) = GetCachedMethodInfo(assemblyKey, methodIdentifier); - var suppliedArgs = ParseArguments(jsRuntime, methodIdentifier, argsJson, parameterTypes); try @@ -301,7 +305,47 @@ namespace Microsoft.JSInterop.Infrastructure } else { - throw new ArgumentException($"The assembly '{assemblyKey.AssemblyName}' does not contain a public method with [{nameof(JSInvokableAttribute)}(\"{methodIdentifier}\")]."); + throw new ArgumentException($"The assembly '{assemblyKey.AssemblyName}' does not contain a public invokable method with [{nameof(JSInvokableAttribute)}(\"{methodIdentifier}\")]."); + } + } + + private static (MethodInfo methodInfo, Type[] parameterTypes) GetCachedMethodInfo(IDotNetObjectReference objectReference, string methodIdentifier) + { + var type = objectReference.Value.GetType(); + var assemblyMethods = _cachedMethodsByType.GetOrAdd(type, ScanTypeForCallableMethods); + if (assemblyMethods.TryGetValue(methodIdentifier, out var result)) + { + return result; + } + else + { + throw new ArgumentException($"The type '{type.Name}' does not contain a public invokable method with [{nameof(JSInvokableAttribute)}(\"{methodIdentifier}\")]."); + } + + static Dictionary ScanTypeForCallableMethods(Type type) + { + var result = new Dictionary(StringComparer.Ordinal); + var invokableMethods = type + .GetMethods(BindingFlags.Public | BindingFlags.Instance) + .Where(method => !method.ContainsGenericParameters && method.IsDefined(typeof(JSInvokableAttribute), inherit: false)); + + foreach (var method in invokableMethods) + { + var identifier = method.GetCustomAttribute(false).Identifier ?? method.Name; + var parameterTypes = method.GetParameters().Select(p => p.ParameterType).ToArray(); + + if (result.ContainsKey(identifier)) + { + throw new InvalidOperationException($"The type {type.Name} contains more than one " + + $"[JSInvokable] method with identifier '{identifier}'. All [JSInvokable] methods within the same " + + $"type must have different identifiers. You can pass a custom identifier as a parameter to " + + $"the [JSInvokable] attribute."); + } + + result.Add(identifier, (method, parameterTypes)); + } + + return result; } } @@ -312,35 +356,22 @@ namespace Microsoft.JSInterop.Infrastructure var result = new Dictionary(StringComparer.Ordinal); var invokableMethods = GetRequiredLoadedAssembly(assemblyKey) .GetExportedTypes() - .SelectMany(type => type.GetMethods( - BindingFlags.Public | - BindingFlags.DeclaredOnly | - BindingFlags.Instance | - BindingFlags.Static)) - .Where(method => method.IsDefined(typeof(JSInvokableAttribute), inherit: false)); + .SelectMany(type => type.GetMethods(BindingFlags.Public | BindingFlags.Static)) + .Where(method => !method.ContainsGenericParameters && method.IsDefined(typeof(JSInvokableAttribute), inherit: false)); foreach (var method in invokableMethods) { var identifier = method.GetCustomAttribute(false).Identifier ?? method.Name; var parameterTypes = method.GetParameters().Select(p => p.ParameterType).ToArray(); - try + if (result.ContainsKey(identifier)) { - result.Add(identifier, (method, parameterTypes)); - } - catch (ArgumentException) - { - if (result.ContainsKey(identifier)) - { - throw new InvalidOperationException($"The assembly '{assemblyKey.AssemblyName}' contains more than one " + - $"[JSInvokable] method with identifier '{identifier}'. All [JSInvokable] methods within the same " + - $"assembly must have different identifiers. You can pass a custom identifier as a parameter to " + - $"the [JSInvokable] attribute."); - } - else - { - throw; - } + throw new InvalidOperationException($"The assembly '{assemblyKey.AssemblyName}' contains more than one " + + $"[JSInvokable] method with identifier '{identifier}'. All [JSInvokable] methods within the same " + + $"assembly must have different identifiers. You can pass a custom identifier as a parameter to " + + $"the [JSInvokable] attribute."); } + + result.Add(identifier, (method, parameterTypes)); } return result; diff --git a/src/JSInterop/Microsoft.JSInterop/test/Infrastructure/DotNetDispatcherTest.cs b/src/JSInterop/Microsoft.JSInterop/test/Infrastructure/DotNetDispatcherTest.cs index 7e82a47a89..c85565d5f2 100644 --- a/src/JSInterop/Microsoft.JSInterop/test/Infrastructure/DotNetDispatcherTest.cs +++ b/src/JSInterop/Microsoft.JSInterop/test/Infrastructure/DotNetDispatcherTest.cs @@ -3,7 +3,6 @@ using System; using System.Linq; -using System.Runtime.ExceptionServices; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -70,7 +69,7 @@ namespace Microsoft.JSInterop.Infrastructure DotNetDispatcher.Invoke(new TestJSRuntime(), new DotNetInvocationInfo(thisAssemblyName, methodIdentifier, default, default), null); }); - Assert.Equal($"The assembly '{thisAssemblyName}' does not contain a public method with [JSInvokableAttribute(\"{methodIdentifier}\")].", ex.Message); + Assert.Equal($"The assembly '{thisAssemblyName}' does not contain a public invokable method with [JSInvokableAttribute(\"{methodIdentifier}\")].", ex.Message); } [Fact] @@ -355,6 +354,78 @@ namespace Microsoft.JSInterop.Infrastructure Assert.Equal("MY STRING", resultDto.StringVal); } + [Fact] + public void CanInvokeNonGenericInstanceMethodOnGenericType() + { + var jsRuntime = new TestJSRuntime(); + var targetInstance = new GenericType(); + jsRuntime.Invoke("_setup", + DotNetObjectReference.Create(targetInstance)); + var argsJson = "[\"hello world\"]"; + + // Act + var resultJson = DotNetDispatcher.Invoke(jsRuntime, new DotNetInvocationInfo(null, nameof(GenericType.EchoStringParameter), 1, default), argsJson); + + // Assert + Assert.Equal("\"hello world\"", resultJson); + } + + [Fact] + public void CanInvokeMethodsThatAcceptGenericParametersOnGenericTypes() + { + var jsRuntime = new TestJSRuntime(); + var targetInstance = new GenericType(); + jsRuntime.Invoke("_setup", + DotNetObjectReference.Create(targetInstance)); + var argsJson = "[\"hello world\"]"; + + // Act + var resultJson = DotNetDispatcher.Invoke(jsRuntime, new DotNetInvocationInfo(null, nameof(GenericType.EchoParameter), 1, default), argsJson); + + // Assert + Assert.Equal("\"hello world\"", resultJson); + } + + [Fact] + public void CannotInvokeStaticOpenGenericMethods() + { + var methodIdentifier = "StaticGenericMethod"; + var jsRuntime = new TestJSRuntime(); + + // Act + var ex = Assert.Throws(() => DotNetDispatcher.Invoke(jsRuntime, new DotNetInvocationInfo(thisAssemblyName, methodIdentifier, 0, default), "[7]")); + Assert.Contains($"The assembly '{thisAssemblyName}' does not contain a public invokable method with [{nameof(JSInvokableAttribute)}(\"{methodIdentifier}\")].", ex.Message); + } + + [Fact] + public void CannotInvokeInstanceOpenGenericMethods() + { + var methodIdentifier = "InstanceGenericMethod"; + var targetInstance = new GenericType(); + var jsRuntime = new TestJSRuntime(); + jsRuntime.Invoke("_setup", + DotNetObjectReference.Create(targetInstance)); + var argsJson = "[\"hello world\"]"; + + // Act + var ex = Assert.Throws(() => DotNetDispatcher.Invoke(jsRuntime, new DotNetInvocationInfo(null, methodIdentifier, 1, default), argsJson)); + Assert.Contains($"The type 'GenericType`1' does not contain a public invokable method with [{nameof(JSInvokableAttribute)}(\"{methodIdentifier}\")].", ex.Message); + } + + [Fact] + public void CannotInvokeMethodsWithGenericParameters_IfTypesDoNotMatch() + { + var jsRuntime = new TestJSRuntime(); + var targetInstance = new GenericType(); + jsRuntime.Invoke("_setup", + DotNetObjectReference.Create(targetInstance)); + var argsJson = "[\"hello world\"]"; + + // Act & Assert + Assert.Throws(() => + DotNetDispatcher.Invoke(jsRuntime, new DotNetInvocationInfo(null, nameof(GenericType.EchoParameter), 1, default), argsJson)); + } + [Fact] public void CannotInvokeWithFewerNumberOfParameters() { @@ -790,6 +861,18 @@ namespace Microsoft.JSInterop.Infrastructure } } + public class GenericType + { + [JSInvokable] public string EchoStringParameter(string input) => input; + [JSInvokable] public TValue EchoParameter(TValue input) => input; + } + + public class GenericMethodClass + { + [JSInvokable("StaticGenericMethod")] public static string StaticGenericMethod(TValue input) => input.ToString(); + [JSInvokable("InstanceGenericMethod")] public string GenericMethod(TValue input) => input.ToString(); + } + public class TestJSRuntime : JSInProcessRuntime { private TaskCompletionSource _nextInvocationTcs = new TaskCompletionSource();