Skip to content

Commit de52f0a

Browse files
committed
Decouple notifications websocket handling from chat operations
This is a prerequisite for ppy#25480. The `WebSocketNotificationsClient` was tightly coupled to chat specifics making it difficult to use in the second factor verification flow. This commit's goal is to separate the websocket connection and message handling concerns from specific chat logic concerns.
1 parent baaf33d commit de52f0a

16 files changed

+330
-225
lines changed

osu.Game.Tests/Chat/TestSceneChannelManager.cs

-2
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ public void SetUpSteps()
7575
return false;
7676
};
7777
});
78-
79-
AddUntilStep("wait for notifications client", () => channelManager.NotificationsConnected);
8078
}
8179

8280
[Test]

osu.Game/Online/API/APIAccess.cs

+5-3
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
using osu.Game.Localisation;
2222
using osu.Game.Online.API.Requests;
2323
using osu.Game.Online.API.Requests.Responses;
24-
using osu.Game.Online.Notifications;
24+
using osu.Game.Online.Chat;
2525
using osu.Game.Online.Notifications.WebSocket;
2626
using osu.Game.Users;
2727

@@ -55,6 +55,8 @@ public partial class APIAccess : Component, IAPIProvider
5555
public IBindable<UserActivity> Activity => activity;
5656
public IBindable<UserStatistics> Statistics => statistics;
5757

58+
public INotificationsClient NotificationsClient { get; }
59+
5860
public Language Language => game.CurrentLanguage.Value;
5961

6062
private Bindable<APIUser> localUser { get; } = new Bindable<APIUser>(createGuestUser());
@@ -82,6 +84,7 @@ public APIAccess(OsuGameBase game, OsuConfigManager config, EndpointConfiguratio
8284

8385
APIEndpointUrl = endpointConfiguration.APIEndpointUrl;
8486
WebsiteRootUrl = endpointConfiguration.WebsiteRootUrl;
87+
NotificationsClient = new WebSocketNotificationsClientConnector(this);
8588

8689
authentication = new OAuth(endpointConfiguration.APIClientID, endpointConfiguration.APIClientSecret, APIEndpointUrl);
8790
log = Logger.GetLogger(LoggingTarget.Network);
@@ -324,8 +327,7 @@ public void Login(string username, string password)
324327
public IHubClientConnector GetHubConnector(string clientName, string endpoint, bool preferMessagePack) =>
325328
new HubClientConnector(clientName, endpoint, this, versionHash, preferMessagePack);
326329

327-
public NotificationsClientConnector GetNotificationsConnector() =>
328-
new WebSocketNotificationsClientConnector(this);
330+
public IChatClient GetChatClient() => new WebSocketChatClient(this);
329331

330332
public RegistrationRequest.RegistrationRequestErrors CreateAccount(string email, string username, string password)
331333
{

osu.Game/Online/API/DummyAPIAccess.cs

+6-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
using osu.Framework.Graphics;
99
using osu.Game.Localisation;
1010
using osu.Game.Online.API.Requests.Responses;
11-
using osu.Game.Online.Notifications;
11+
using osu.Game.Online.Chat;
12+
using osu.Game.Online.Notifications.WebSocket;
1213
using osu.Game.Tests;
1314
using osu.Game.Users;
1415

@@ -30,6 +31,9 @@ public partial class DummyAPIAccess : Component, IAPIProvider
3031

3132
public Bindable<UserStatistics?> Statistics { get; } = new Bindable<UserStatistics?>();
3233

34+
public DummyNotificationsClient NotificationsClient { get; } = new DummyNotificationsClient();
35+
INotificationsClient IAPIProvider.NotificationsClient => NotificationsClient;
36+
3337
public Language Language => Language.en;
3438

3539
public string AccessToken => "token";
@@ -144,7 +148,7 @@ public void UpdateStatistics(UserStatistics newStatistics)
144148

145149
public IHubClientConnector? GetHubConnector(string clientName, string endpoint, bool preferMessagePack) => null;
146150

147-
public NotificationsClientConnector GetNotificationsConnector() => new PollingNotificationsClientConnector(this);
151+
public IChatClient GetChatClient() => new PollingChatClientConnector(this);
148152

149153
public RegistrationRequest.RegistrationRequestErrors? CreateAccount(string email, string username, string password)
150154
{

osu.Game/Online/API/IAPIProvider.cs

+5-5
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
using osu.Framework.Bindables;
77
using osu.Game.Localisation;
88
using osu.Game.Online.API.Requests.Responses;
9-
using osu.Game.Online.Notifications;
9+
using osu.Game.Online.Chat;
10+
using osu.Game.Online.Notifications.WebSocket;
1011
using osu.Game.Users;
1112

1213
namespace osu.Game.Online.API
@@ -129,10 +130,9 @@ public interface IAPIProvider
129130
/// <param name="preferMessagePack">Whether to use MessagePack for serialisation if available on this platform.</param>
130131
IHubClientConnector? GetHubConnector(string clientName, string endpoint, bool preferMessagePack = true);
131132

132-
/// <summary>
133-
/// Constructs a new <see cref="NotificationsClientConnector"/>.
134-
/// </summary>
135-
NotificationsClientConnector GetNotificationsConnector();
133+
INotificationsClient NotificationsClient { get; }
134+
135+
IChatClient GetChatClient();
136136

137137
/// <summary>
138138
/// Create a new user account. This is a blocking operation.

osu.Game/Online/Chat/ChannelManager.cs

+8-18
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
using osu.Game.Online.API;
1717
using osu.Game.Online.API.Requests;
1818
using osu.Game.Online.API.Requests.Responses;
19-
using osu.Game.Online.Notifications;
2019
using osu.Game.Overlays.Chat.Listing;
2120

2221
namespace osu.Game.Online.Chat
@@ -64,13 +63,8 @@ public partial class ChannelManager : CompositeComponent, IChannelPostTarget
6463
/// </summary>
6564
public IBindableList<Channel> AvailableChannels => availableChannels;
6665

67-
/// <summary>
68-
/// Whether the client responsible for channel notifications is connected.
69-
/// </summary>
70-
public bool NotificationsConnected => connector.IsConnected.Value;
71-
7266
private readonly IAPIProvider api;
73-
private readonly NotificationsClientConnector connector;
67+
private readonly IChatClient chatClient;
7468

7569
[Resolved]
7670
private UserLookupCache users { get; set; }
@@ -85,23 +79,19 @@ public ChannelManager(IAPIProvider api)
8579
{
8680
this.api = api;
8781

88-
connector = api.GetNotificationsConnector();
82+
chatClient = api.GetChatClient();
8983

9084
CurrentChannel.ValueChanged += currentChannelChanged;
9185
}
9286

9387
[BackgroundDependencyLoader]
9488
private void load()
9589
{
96-
connector.ChannelJoined += ch => Schedule(() => joinChannel(ch));
97-
98-
connector.ChannelParted += ch => Schedule(() => leaveChannel(getChannel(ch), false));
99-
100-
connector.NewMessages += msgs => Schedule(() => addMessages(msgs));
101-
102-
connector.PresenceReceived += () => Schedule(initializeChannels);
103-
104-
connector.Start();
90+
chatClient.ChannelJoined += ch => Schedule(() => joinChannel(ch));
91+
chatClient.ChannelParted += ch => Schedule(() => leaveChannel(getChannel(ch), false));
92+
chatClient.NewMessages += msgs => Schedule(() => addMessages(msgs));
93+
chatClient.PresenceReceived += () => Schedule(initializeChannels);
94+
chatClient.FetchInitialMessages();
10595

10696
apiState.BindTo(api.State);
10797
apiState.BindValueChanged(_ => SendAck(), true);
@@ -655,7 +645,7 @@ public void MarkChannelAsRead(Channel channel)
655645
protected override void Dispose(bool isDisposing)
656646
{
657647
base.Dispose(isDisposing);
658-
connector?.Dispose();
648+
chatClient?.Dispose();
659649
}
660650
}
661651

osu.Game/Online/Chat/IChatClient.cs

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
2+
// See the LICENCE file in the repository root for full licence text.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
7+
namespace osu.Game.Online.Chat
8+
{
9+
public interface IChatClient : IDisposable
10+
{
11+
event Action<Channel>? ChannelJoined;
12+
event Action<Channel>? ChannelParted;
13+
event Action<List<Message>>? NewMessages;
14+
event Action? PresenceReceived;
15+
16+
void FetchInitialMessages();
17+
}
18+
}
+148
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
2+
// See the LICENCE file in the repository root for full licence text.
3+
4+
using System;
5+
using System.Collections.Concurrent;
6+
using System.Collections.Generic;
7+
using System.Diagnostics;
8+
using Newtonsoft.Json;
9+
using osu.Framework.Bindables;
10+
using osu.Framework.Extensions;
11+
using osu.Framework.Logging;
12+
using osu.Game.Online.API;
13+
using osu.Game.Online.API.Requests;
14+
using osu.Game.Online.Notifications.WebSocket;
15+
16+
namespace osu.Game.Online.Chat
17+
{
18+
public class WebSocketChatClient : IChatClient
19+
{
20+
public event Action<Channel>? ChannelJoined;
21+
public event Action<Channel>? ChannelParted;
22+
public event Action<List<Message>>? NewMessages;
23+
public event Action? PresenceReceived;
24+
25+
private readonly IAPIProvider api;
26+
private readonly INotificationsClient client;
27+
private readonly ConcurrentDictionary<long, Channel> channelsMap = new ConcurrentDictionary<long, Channel>();
28+
29+
public WebSocketChatClient(IAPIProvider api)
30+
{
31+
this.api = api;
32+
client = api.NotificationsClient;
33+
client.IsConnected.BindValueChanged(start, true);
34+
}
35+
36+
private void start(ValueChangedEvent<bool> connected)
37+
{
38+
if (!connected.NewValue)
39+
return;
40+
41+
client.MessageReceived += onMessageReceived;
42+
client.SendAsync(new StartChatRequest()).WaitSafely();
43+
}
44+
45+
public void FetchInitialMessages()
46+
{
47+
api.Queue(createInitialFetchRequest());
48+
}
49+
50+
private APIRequest createInitialFetchRequest()
51+
{
52+
var fetchReq = new GetUpdatesRequest(0);
53+
54+
fetchReq.Success += updates =>
55+
{
56+
if (updates?.Presence != null)
57+
{
58+
foreach (var channel in updates.Presence)
59+
joinChannel(channel);
60+
61+
handleMessages(updates.Messages);
62+
}
63+
64+
PresenceReceived?.Invoke();
65+
};
66+
67+
return fetchReq;
68+
}
69+
70+
private void onMessageReceived(SocketMessage message)
71+
{
72+
switch (message.Event)
73+
{
74+
case @"chat.channel.join":
75+
Debug.Assert(message.Data != null);
76+
77+
Channel? joinedChannel = JsonConvert.DeserializeObject<Channel>(message.Data.ToString());
78+
Debug.Assert(joinedChannel != null);
79+
80+
joinChannel(joinedChannel);
81+
break;
82+
83+
case @"chat.channel.part":
84+
Debug.Assert(message.Data != null);
85+
86+
Channel? partedChannel = JsonConvert.DeserializeObject<Channel>(message.Data.ToString());
87+
Debug.Assert(partedChannel != null);
88+
89+
partChannel(partedChannel);
90+
break;
91+
92+
case @"chat.message.new":
93+
Debug.Assert(message.Data != null);
94+
95+
NewChatMessageData? messageData = JsonConvert.DeserializeObject<NewChatMessageData>(message.Data.ToString());
96+
Debug.Assert(messageData != null);
97+
98+
foreach (var msg in messageData.Messages)
99+
postToChannel(msg);
100+
101+
break;
102+
}
103+
}
104+
105+
private void postToChannel(Message message)
106+
{
107+
if (channelsMap.TryGetValue(message.ChannelId, out Channel? channel))
108+
{
109+
joinChannel(channel);
110+
NewMessages?.Invoke(new List<Message> { message });
111+
return;
112+
}
113+
114+
var req = new GetChannelRequest(message.ChannelId);
115+
116+
req.Success += response =>
117+
{
118+
joinChannel(channelsMap[message.ChannelId] = response.Channel);
119+
NewMessages?.Invoke(new List<Message> { message });
120+
};
121+
req.Failure += ex => Logger.Error(ex, "Failed to join channel");
122+
123+
api.Queue(req);
124+
}
125+
126+
private void joinChannel(Channel ch)
127+
{
128+
ch.Joined.Value = true;
129+
ChannelJoined?.Invoke(ch);
130+
}
131+
132+
private void partChannel(Channel channel) => ChannelParted?.Invoke(channel);
133+
134+
private void handleMessages(List<Message>? messages)
135+
{
136+
if (messages == null)
137+
return;
138+
139+
NewMessages?.Invoke(messages);
140+
}
141+
142+
public void Dispose()
143+
{
144+
client.IsConnected.ValueChanged -= start;
145+
client.MessageReceived -= onMessageReceived;
146+
}
147+
}
148+
}

osu.Game/Online/Notifications/NotificationsClientConnector.cs

-42
This file was deleted.

0 commit comments

Comments
 (0)