Allow argument of roll referee hub operation to be nullable (#461)

* Make `RefereeHub` inherit `LoggingHub`

Mostly because I'm starting to look at sentry issues from the referee
hub and noticing that they're missing the user ID which is useful
information, and in other hubs it is supplied by `LoggingHub` and its
related hub filter. It'll also add some datadog monitoring which may
come in useful going forward as well.

Not sure why I never did this. I think I convinced myself that the
generics didn't work in my usage because of it being tied to
`StatefulUserHub`, but on closer inspection, no, that's not the case.

* Allow argument of `Roll` referee hub operation to be nullable

Spotted via #459.

I reproduced this by calling `Roll(roomId, null)` from a JS client.

You could argue that the user was at fault here for passing `null`
rather than `{}` or something, but you could also argue that there's no
reason not to make this work with a nullable for user convenience. While
I'm not clairvoyant I can't see a future where rolling has a required
parameter in it either.

Author: bdach

osu.Server.Spectator/Hubs/Referee/IRefereeHubServer.cs

@@ -141,7 +141,7 @@ public interface IRefereeHubServer
         /// Initiates a random roll in the room.
         /// Corresponds to the <c>!roll</c> command on bancho.
         /// </summary>
-        Task Roll(long roomId, RollRequest request);
+        Task Roll(long roomId, RollRequest? request);
 
         /// <summary>
         /// Moves the user to a different team in the given <paramref name="roomId"/>.

osu.Server.Spectator/Hubs/Referee/RefereeHub.cs

@@ -7,7 +7,6 @@
 using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authorization;
-using Microsoft.AspNetCore.SignalR;
 using Microsoft.Extensions.Logging;
 using osu.Game.Online.API.Requests.Responses;
 using osu.Game.Online.Multiplayer;
@@ -29,11 +28,10 @@
 namespace osu.Server.Spectator.Hubs.Referee
 {
     [Authorize(ConfigureJwtBearerOptions.REFEREE_CLIENT_SCHEME)]
-    public class RefereeHub : Hub, IRefereeHubServer
+    public class RefereeHub : LoggingHub<IRefereeHubClient>, IRefereeHubServer
     {
         private readonly IDatabaseFactory databaseFactory;
         private readonly ISharedInterop sharedInterop;
-        private readonly ILogger<RefereeHub> logger;
         private readonly IMultiplayerRoomController roomController;
         private readonly MultiplayerEventDispatcher eventDispatcher;
         private readonly EntityStore<RefereeClientState> refereeStates;
@@ -49,9 +47,9 @@ public RefereeHub(
             EntityStore<RefereeClientState> refereeStates,
             EntityStore<MultiplayerClientState> playerStates,
             ChatFilters chatFilters)
+            : base(loggerFactory)
         {
             this.databaseFactory = databaseFactory;
-            logger = loggerFactory.CreateLogger<RefereeHub>();
             this.sharedInterop = sharedInterop;
             this.roomController = roomController;
             this.eventDispatcher = eventDispatcher;
@@ -76,14 +74,15 @@ public override async Task OnConnectedAsync()
             await base.OnConnectedAsync();
         }
 
+        [Obsolete]
         public async Task Ping(string message)
         {
             string? username;
 
             using (var db = databaseFactory.GetInstance())
                 username = await db.GetUsernameAsync(Context.GetUserId());
 
-            await Clients.Caller.SendAsync(nameof(IRefereeHubClient.Pong), $"Hi {username}! Here's your message back: {message}");
+            await Clients.Caller.Pong($"Hi {username}! Here's your message back: {message}");
         }
 
         public async Task<RoomJoinedResponse> MakeRoom(MakeRoomRequest request)
@@ -632,7 +631,7 @@ private static void ensurePlaylistItemValid(MultiplayerPlaylistItem playlistItem
             }
         }
 
-        public async Task Roll(long roomId, RollRequest request)
+        public async Task Roll(long roomId, RollRequest? request)
         {
             using (var userUsage = await refereeStates.GetForUse(Context.GetUserId()))
             {
@@ -649,7 +648,7 @@ public async Task Roll(long roomId, RollRequest request)
                     if (user == null)
                         ThrowHelper.ThrowUserNotInRoom();
 
-                    await roomUsage.Item.MatchController.HandleUserRequest(user, new Game.Online.Multiplayer.RollRequest { Max = request.Max });
+                    await roomUsage.Item.MatchController.HandleUserRequest(user, new Game.Online.Multiplayer.RollRequest { Max = request?.Max });
                 }
             }
         }
@@ -823,6 +822,6 @@ private static void ensureIsReferee(long roomId, ItemUsage<RefereeClientState> u
         }
 
         private void log(string message, ServerMultiplayerRoom? room, LogLevel level = LogLevel.Information)
-            => logger.Log(level, "[user:{userId}] [room:{roomId}] {message}", Context.UserIdentifier, room?.RoomID.ToString() ?? "none", message);
+            => Log($"[room:{room?.RoomID.ToString() ?? "none"}] {message}", level);
     }
 }

osu.Server.Spectator/Startup.cs

@@ -55,6 +55,7 @@ public void ConfigureServices(IServiceCollection services)
                     .AddHubOptions<RefereeHub>(options =>
                     {
                         options.SupportedProtocols?.Remove("messagepack");
+                        options.AddFilter<LoggingHubFilter>();
                     });
 
             services.AddHubEntities()