Skip to content

Add support for JSInvokable methods on generic types #2342

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.

Sign up for GitHub

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

Merged
pranavkm merged 4 commits into from
Sep 19, 2019
Merged

Add support for JSInvokable methods on generic types #2342

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b5ecf8
Add support for JSInvokable methods on generic types
pranavkm Sep 13, 2019
098be37
Add some more tests
pranavkm Sep 13, 2019
d9edf12
Merge branch 'release/3.1' into prkrishn/generic-jsinterop
pranavkm Sep 16, 2019
a55cdc6
Merge branch 'release/3.1' into prkrishn/generic-jsinterop
pranavkm Sep 19, 2019
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
83 changes: 57 additions & 26 deletions src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ public static class DotNetDispatcher
private static readonly ConcurrentDictionary<AssemblyKey, IReadOnlyDictionary<string, (MethodInfo, Type[])>> _cachedMethodsByAssembly
= new ConcurrentDictionary<AssemblyKey, IReadOnlyDictionary<string, (MethodInfo, Type[])>>();

private static readonly ConcurrentDictionary<Type, IReadOnlyDictionary<string, (MethodInfo, Type[])>> _cachedMethodsByType
= new ConcurrentDictionary<Type, IReadOnlyDictionary<string, (MethodInfo, Type[])>>();

/// <summary>
/// Receives a call from JS to .NET, locating and invoking the specified method.
/// </summary>
Expand Down Expand Up @@ -129,9 +132,12 @@ private static object InvokeSynchronously(JSRuntime jsRuntime, in DotNetInvocati
var methodIdentifier = callInfo.MethodIdentifier;

AssemblyKey assemblyKey;
MethodInfo methodInfo;
Type[] parameterTypes;
if (objectReference is null)
{
assemblyKey = new AssemblyKey(assemblyName);
(methodInfo, parameterTypes) = GetCachedMethodInfo(assemblyKey, methodIdentifier);
}
else
{
Expand All @@ -147,11 +153,9 @@ private static object InvokeSynchronously(JSRuntime jsRuntime, in DotNetInvocati
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
Expand Down Expand Up @@ -301,7 +305,47 @@ private static (MethodInfo, Type[]) GetCachedMethodInfo(AssemblyKey assemblyKey,
}
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}\")].");
Copy link
Author

@pranavkm pranavkm Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would exposing the type name be problematic in any way? For instance methods, reporting the object id isn't useful.

}

static Dictionary<string, (MethodInfo, Type[])> ScanTypeForCallableMethods(Type type)
{
var result = new Dictionary<string, (MethodInfo, Type[])>(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<JSInvokableAttribute>(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;
}
}

Expand All @@ -312,35 +356,22 @@ private static (MethodInfo, Type[]) GetCachedMethodInfo(AssemblyKey assemblyKey,
var result = new Dictionary<string, (MethodInfo, Type[])>(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));
Copy link
Author

@pranavkm pranavkm Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://docs.microsoft.com/en-us/dotnet/api/system.reflection.methodbase.containsgenericparameters?view=netframework-4.8, methods containing generic parameters cannot be invoked:

If the ContainsGenericParameters property returns true, the method cannot be invoked.

This gives us the opportunity to treat these pretty much the same as non-public methods with JsInvokable and provide a more meaningful message. Without this check, you get strange errors from System.Text.Json.

foreach (var method in invokableMethods)
{
var identifier = method.GetCustomAttribute<JSInvokableAttribute>(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;
Expand Down
87 changes: 85 additions & 2 deletions src/JSInterop/Microsoft.JSInterop/test/Infrastructure/DotNetDispatcherTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Linq;
using System.Runtime.ExceptionServices;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -70,7 +69,7 @@ public void CannotInvokeUnsuitableMethods(string methodIdentifier)
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]
Expand Down Expand Up @@ -355,6 +354,78 @@ public void CanInvokeInstanceMethodWithParams()
Assert.Equal("MY STRING", resultDto.StringVal);
}

[Fact]
public void CanInvokeNonGenericInstanceMethodOnGenericType()
{
var jsRuntime = new TestJSRuntime();
var targetInstance = new GenericType<int>();
jsRuntime.Invoke<object>("_setup",
DotNetObjectReference.Create(targetInstance));
var argsJson = "[\"hello world\"]";

// Act
var resultJson = DotNetDispatcher.Invoke(jsRuntime, new DotNetInvocationInfo(null, nameof(GenericType<int>.EchoStringParameter), 1, default), argsJson);

// Assert
Assert.Equal("\"hello world\"", resultJson);
}

[Fact]
public void CanInvokeMethodsThatAcceptGenericParametersOnGenericTypes()
{
var jsRuntime = new TestJSRuntime();
var targetInstance = new GenericType<string>();
jsRuntime.Invoke<object>("_setup",
DotNetObjectReference.Create(targetInstance));
var argsJson = "[\"hello world\"]";

// Act
var resultJson = DotNetDispatcher.Invoke(jsRuntime, new DotNetInvocationInfo(null, nameof(GenericType<string>.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<ArgumentException>(() => 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<int>();
var jsRuntime = new TestJSRuntime();
jsRuntime.Invoke<object>("_setup",
DotNetObjectReference.Create(targetInstance));
var argsJson = "[\"hello world\"]";

// Act
var ex = Assert.Throws<ArgumentException>(() => 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<int>();
jsRuntime.Invoke<object>("_setup",
DotNetObjectReference.Create(targetInstance));
var argsJson = "[\"hello world\"]";

// Act & Assert
Assert.Throws<JsonException>(() =>
DotNetDispatcher.Invoke(jsRuntime, new DotNetInvocationInfo(null, nameof(GenericType<int>.EchoParameter), 1, default), argsJson));
}

[Fact]
public void CannotInvokeWithFewerNumberOfParameters()
{
Expand Down Expand Up @@ -790,6 +861,18 @@ public static async Task<string> AsyncThrowingMethod()
}
}

public class GenericType<TValue>
Copy link
Author

@pranavkm pranavkm Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These scenarios work ✔️

{
[JSInvokable] public string EchoStringParameter(string input) => input;
[JSInvokable] public TValue EchoParameter(TValue input) => input;
}

public class GenericMethodClass
Copy link
Author

@pranavkm pranavkm Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These scenarios throw ❌

{
[JSInvokable("StaticGenericMethod")] public static string StaticGenericMethod<TValue>(TValue input) => input.ToString();
[JSInvokable("InstanceGenericMethod")] public string GenericMethod<TValue>(TValue input) => input.ToString();
}

public class TestJSRuntime : JSInProcessRuntime
{
private TaskCompletionSource<object> _nextInvocationTcs = new TaskCompletionSource<object>();
Expand Down