Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/SignalR/server/Core/src/IHubFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public interface IHubFilter
/// <param name="invocationContext">The context for the method invocation that holds all the important information about the invoke.</param>
/// <param name="next">The next filter to run, and for the final one, the Hub invocation.</param>
/// <returns>Returns the result of the Hub method invoke.</returns>
ValueTask<object> InvokeMethodAsync(HubInvocationContext invocationContext, Func<HubInvocationContext, ValueTask<object>> next);
ValueTask<object> InvokeMethodAsync(HubInvocationContext invocationContext, Func<HubInvocationContext, ValueTask<object>> next) => next(invocationContext);
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth adding a simple test that verifies you don't have to implement InvokeMethodAsync.

Copy link
Member

Choose a reason for hiding this comment

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

Why are hub filters an interface with a default implementation rather than an abstract base class?

Copy link
Member

Choose a reason for hiding this comment

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

We're rebels. I actually think it's a good experiment to try this on

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not a fan of how people generally call base.FunctionName(); (and intellisense adds it automatically) with abstract base classes when the base class does nothing


/// <summary>
/// Allows handling of the <see cref="Hub.OnConnectedAsync"/> method.
Expand Down
35 changes: 34 additions & 1 deletion src/SignalR/server/SignalR/test/HubFilterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Diagnostics;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Internal;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -136,6 +135,40 @@ private async Task AssertMethodsCalled(IServiceProvider serviceProvider, TcsServ
}
}

[Fact]
public async Task HubFilterDoesNotNeedToImplementMethods()
{
using (StartVerifiableLog())
{
var tcsService = new TcsService();
var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services =>
{
services.AddSignalR().AddHubOptions<DynamicTestHub>(options =>
{
options.AddFilter(typeof(EmptyFilter));
});
}, LoggerFactory);


var connectionHandler = serviceProvider.GetService<HubConnectionHandler<DynamicTestHub>>();

using (var client = new TestClient())
{
var connectionHandlerTask = await client.ConnectAsync(connectionHandler);

await client.Connected.OrTimeout();

var completion = await client.InvokeAsync(nameof(DynamicTestHub.Echo), "hello");
Assert.Null(completion.Error);
Assert.Equal("hello", completion.Result);

client.Dispose();

await connectionHandlerTask.OrTimeout();
}
}
}

[Fact]
public async Task MutlipleFilters_MethodsAreCalled()
{
Expand Down
5 changes: 5 additions & 0 deletions src/SignalR/server/SignalR/test/TestFilters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,4 +233,9 @@ public ValueTask<object> InvokeMethodAsync(HubInvocationContext invocationContex
return next(context);
}
}

public class EmptyFilter : IHubFilter
{
// Purposefully not implementing any methods
}
}