Skip to content

Commit

Permalink
Reverted caching in hub method resolution because of a regression.
Browse files Browse the repository at this point in the history
- Added test to verify overloads work.
Fixes #362
  • Loading branch information
davidfowl authored and jrsconfitto committed May 18, 2012
1 parent 9f73e43 commit a8120fe
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 26 deletions.
17 changes: 17 additions & 0 deletions SignalR.Tests/HubFacts.cs
Expand Up @@ -71,5 +71,22 @@ public void GenericTaskWithException()

Assert.Equal("Exception of type 'System.Exception' was thrown.", ex.GetBaseException().Message);
}

[Fact]
public void Overloads()
{
var host = new MemoryHost();
host.MapHubs();
var connection = new Client.Hubs.HubConnection("http://foo/");

var hub = connection.CreateProxy("demo");

connection.Start(host).Wait();

hub.Invoke("Overload").Wait();
int n = hub.Invoke<int>("Overload", 1).Result;

Assert.Equal(1, n);
}
}
}
38 changes: 12 additions & 26 deletions SignalR/Hubs/Lookup/ReflectedMethodDescriptorProvider.cs
Expand Up @@ -11,12 +11,10 @@ namespace SignalR.Hubs
public class ReflectedMethodDescriptorProvider : IMethodDescriptorProvider
{
private readonly ConcurrentDictionary<string, IDictionary<string, IEnumerable<MethodDescriptor>>> _methods;
private readonly ConcurrentDictionary<string, MethodDescriptor> _executableMethods;

public ReflectedMethodDescriptorProvider()
{
_methods = new ConcurrentDictionary<string, IDictionary<string, IEnumerable<MethodDescriptor>>>(StringComparer.OrdinalIgnoreCase);
_executableMethods = new ConcurrentDictionary<string, MethodDescriptor>(StringComparer.OrdinalIgnoreCase);
}

public IEnumerable<MethodDescriptor> GetMethods(HubDescriptor hub)
Expand Down Expand Up @@ -59,43 +57,31 @@ public IEnumerable<MethodDescriptor> GetMethods(HubDescriptor hub)
Invoker = oload.Invoke,
Parameters = oload.GetParameters()
.Select(p => new ParameterDescriptor
{
Name = p.Name,
Type = p.ParameterType,
})
{
Name = p.Name,
Type = p.ParameterType,
})
.ToList()
}),
StringComparer.OrdinalIgnoreCase);
}

public bool TryGetMethod(HubDescriptor hub, string method, out MethodDescriptor descriptor, params JToken[] parameters)
{
string hubMethodKey = hub.Name + "::" + method;
IEnumerable<MethodDescriptor> overloads;

if(!_executableMethods.TryGetValue(hubMethodKey, out descriptor))
if (FetchMethodsFor(hub).TryGetValue(method, out overloads))
{
IEnumerable<MethodDescriptor> overloads;

if(FetchMethodsFor(hub).TryGetValue(method, out overloads))
{
var matches = overloads.Where(o => o.Matches(parameters)).ToList();

// If only one match is found, that is the "executable" version, otherwise none of the methods can be returned because we don't know which one was actually being targeted
descriptor = matches.Count == 1 ? matches[0] : null;
}
else
{
descriptor = null;
}

// If an executable method was found, cache it for future lookups (NOTE: we don't cache null instances because it could be a surface area for DoS attack by supplying random method names to flood the cache)
if(descriptor != null)
var matches = overloads.Where(o => o.Matches(parameters)).ToList();
if (matches.Count == 1)
{
_executableMethods.TryAdd(hubMethodKey, descriptor);
descriptor = matches.First();
return true;
}
}

return descriptor != null;
descriptor = null;
return false;
}

private static string GetMethodName(MethodInfo method)
Expand Down

0 comments on commit a8120fe

Please sign in to comment.