From 7d6f7ac75e496c6cff379c7f471e805267e02047 Mon Sep 17 00:00:00 2001 From: Opelkuh <25430283+Opelkuh@users.noreply.github.com> Date: Sun, 15 Aug 2021 02:57:11 +0200 Subject: [PATCH 1/4] Fix mark channel as read error --- osu.Game/Online/Chat/ChannelManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 3136a3960d..1937019ef6 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -553,7 +553,7 @@ namespace osu.Game.Online.Chat if (channel.LastMessageId == channel.LastReadId) return; - var message = channel.Messages.LastOrDefault(); + var message = channel.Messages.FindLast(msg => !(msg is LocalMessage)); if (message == null) return; From 2f9f1ba862cf7c54dca3063e0c33dee9fef55c01 Mon Sep 17 00:00:00 2001 From: Opelkuh <25430283+Opelkuh@users.noreply.github.com> Date: Sun, 15 Aug 2021 15:44:23 +0200 Subject: [PATCH 2/4] Add test for `ChannelManager.MarkChannelAsRead` --- .../Chat/TestSceneChannelManager.cs | 57 ++++++++++++++++++- .../API/Requests/MarkChannelAsReadRequest.cs | 10 ++-- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Chat/TestSceneChannelManager.cs b/osu.Game.Tests/Chat/TestSceneChannelManager.cs index 0ec21a4c7b..7b6d2078ca 100644 --- a/osu.Game.Tests/Chat/TestSceneChannelManager.cs +++ b/osu.Game.Tests/Chat/TestSceneChannelManager.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -19,6 +20,7 @@ namespace osu.Game.Tests.Chat { private ChannelManager channelManager; private int currentMessageId; + List sentMessages; [SetUp] public void Setup() => Schedule(() => @@ -34,6 +36,7 @@ namespace osu.Game.Tests.Chat AddStep("register request handling", () => { currentMessageId = 0; + sentMessages = new List(); ((DummyAPIAccess)API).HandleRequest = req => { @@ -44,7 +47,7 @@ namespace osu.Game.Tests.Chat return true; case PostMessageRequest postMessage: - postMessage.TriggerSuccess(new Message(++currentMessageId) + var message = new Message(++currentMessageId) { IsAction = postMessage.Message.IsAction, ChannelId = postMessage.Message.ChannelId, @@ -52,7 +55,10 @@ namespace osu.Game.Tests.Chat Links = postMessage.Message.Links, Timestamp = postMessage.Message.Timestamp, Sender = postMessage.Message.Sender - }); + }; + + sentMessages.Add(message); + postMessage.TriggerSuccess(message); return true; } @@ -83,12 +89,59 @@ namespace osu.Game.Tests.Chat AddAssert("/np command received by channel 2", () => channel2.Messages.Last().Content.Contains("is listening to")); } + [Test] + public void TestMarkAsReadIgnoringLocalMessages() { + Channel channel = null; + + AddStep("join channel and select it", () => + { + channelManager.JoinChannel(channel = createChannel(1, ChannelType.Public)); + + channelManager.CurrentChannel.Value = channel; + }); + + AddStep("post message", () => channelManager.PostMessage("Something interesting")); + AddUntilStep("wait until the message is posted", () => channel.Messages.Count == sentMessages.Count); + + AddStep("post /help command", () => channelManager.PostCommand("help", channel)); + AddStep("post /me command with no action", () => channelManager.PostCommand("me", channel)); + AddStep("post /join command with no channel", () => channelManager.PostCommand("join", channel)); + AddStep("post /join command with non-existent channel", () => channelManager.PostCommand("join i-dont-exist", channel)); + AddStep("post non-existent command", () => channelManager.PostCommand("non-existent-cmd arg", channel)); + + AddStep("register mark channel as read request handler", () => { + ((DummyAPIAccess)API).HandleRequest = req => + { + switch (req) + { + case MarkChannelAsReadRequest markRead: + var isSentMessage = sentMessages.Contains(markRead.Message); + + AddAssert("mark channel as read called with a real message", () => isSentMessage); + + if(isSentMessage) { + markRead.TriggerSuccess(); + } else { + markRead.TriggerFailure(new APIException("unknown message!", null)); + } + + return true; + } + + return false; + }; + }); + + AddStep("mark channel as read", () => channelManager.MarkChannelAsRead(channel)); + } + private Channel createChannel(int id, ChannelType type) => new Channel(new User()) { Id = id, Name = $"Channel {id}", Topic = $"Topic of channel {id} with type {type}", Type = type, + LastMessageId = 0, }; private class ChannelManagerContainer : CompositeDrawable diff --git a/osu.Game/Online/API/Requests/MarkChannelAsReadRequest.cs b/osu.Game/Online/API/Requests/MarkChannelAsReadRequest.cs index 95a5d0acbd..594ab2accc 100644 --- a/osu.Game/Online/API/Requests/MarkChannelAsReadRequest.cs +++ b/osu.Game/Online/API/Requests/MarkChannelAsReadRequest.cs @@ -9,16 +9,16 @@ namespace osu.Game.Online.API.Requests { public class MarkChannelAsReadRequest : APIRequest { - private readonly Channel channel; - private readonly Message message; + public readonly Channel Channel; + public readonly Message Message; public MarkChannelAsReadRequest(Channel channel, Message message) { - this.channel = channel; - this.message = message; + this.Channel = channel; + this.Message = message; } - protected override string Target => $"chat/channels/{channel.Id}/mark-as-read/{message.Id}"; + protected override string Target => $"chat/channels/{Channel.Id}/mark-as-read/{Message.Id}"; protected override WebRequest CreateWebRequest() { From 7d7c5c06f06a3d5f987623defcdc878e518fed26 Mon Sep 17 00:00:00 2001 From: Opelkuh <25430283+Opelkuh@users.noreply.github.com> Date: Sun, 15 Aug 2021 16:02:25 +0200 Subject: [PATCH 3/4] Fix code formatting --- .../Chat/TestSceneChannelManager.cs | 41 ++++++++++--------- .../API/Requests/MarkChannelAsReadRequest.cs | 4 +- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/Chat/TestSceneChannelManager.cs b/osu.Game.Tests/Chat/TestSceneChannelManager.cs index 7b6d2078ca..fcb602cb04 100644 --- a/osu.Game.Tests/Chat/TestSceneChannelManager.cs +++ b/osu.Game.Tests/Chat/TestSceneChannelManager.cs @@ -20,7 +20,7 @@ namespace osu.Game.Tests.Chat { private ChannelManager channelManager; private int currentMessageId; - List sentMessages; + private List sentMessages; [SetUp] public void Setup() => Schedule(() => @@ -90,46 +90,49 @@ namespace osu.Game.Tests.Chat } [Test] - public void TestMarkAsReadIgnoringLocalMessages() { + public void TestMarkAsReadIgnoringLocalMessages() + { Channel channel = null; AddStep("join channel and select it", () => { channelManager.JoinChannel(channel = createChannel(1, ChannelType.Public)); - channelManager.CurrentChannel.Value = channel; }); AddStep("post message", () => channelManager.PostMessage("Something interesting")); AddUntilStep("wait until the message is posted", () => channel.Messages.Count == sentMessages.Count); - AddStep("post /help command", () => channelManager.PostCommand("help", channel)); AddStep("post /me command with no action", () => channelManager.PostCommand("me", channel)); AddStep("post /join command with no channel", () => channelManager.PostCommand("join", channel)); AddStep("post /join command with non-existent channel", () => channelManager.PostCommand("join i-dont-exist", channel)); AddStep("post non-existent command", () => channelManager.PostCommand("non-existent-cmd arg", channel)); - AddStep("register mark channel as read request handler", () => { + AddStep("register mark channel as read request handler", () => + { ((DummyAPIAccess)API).HandleRequest = req => + { + switch (req) { - switch (req) - { - case MarkChannelAsReadRequest markRead: - var isSentMessage = sentMessages.Contains(markRead.Message); + case MarkChannelAsReadRequest markRead: + var isSentMessage = sentMessages.Contains(markRead.Message); - AddAssert("mark channel as read called with a real message", () => isSentMessage); + AddAssert("mark channel as read called with a real message", () => isSentMessage); - if(isSentMessage) { - markRead.TriggerSuccess(); - } else { - markRead.TriggerFailure(new APIException("unknown message!", null)); - } + if (isSentMessage) + { + markRead.TriggerSuccess(); + } + else + { + markRead.TriggerFailure(new APIException("unknown message!", null)); + } - return true; - } + return true; + } - return false; - }; + return false; + }; }); AddStep("mark channel as read", () => channelManager.MarkChannelAsRead(channel)); diff --git a/osu.Game/Online/API/Requests/MarkChannelAsReadRequest.cs b/osu.Game/Online/API/Requests/MarkChannelAsReadRequest.cs index 594ab2accc..b24669e6d5 100644 --- a/osu.Game/Online/API/Requests/MarkChannelAsReadRequest.cs +++ b/osu.Game/Online/API/Requests/MarkChannelAsReadRequest.cs @@ -14,8 +14,8 @@ namespace osu.Game.Online.API.Requests public MarkChannelAsReadRequest(Channel channel, Message message) { - this.Channel = channel; - this.Message = message; + Channel = channel; + Message = message; } protected override string Target => $"chat/channels/{Channel.Id}/mark-as-read/{Message.Id}"; From 8f698a42f727472723668931713ae4671af2ace0 Mon Sep 17 00:00:00 2001 From: Opelkuh <25430283+Opelkuh@users.noreply.github.com> Date: Sun, 15 Aug 2021 20:35:52 +0200 Subject: [PATCH 4/4] Refactor `MarkChannelAsRead` test assert --- .../Chat/TestSceneChannelManager.cs | 75 +++++++++---------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/osu.Game.Tests/Chat/TestSceneChannelManager.cs b/osu.Game.Tests/Chat/TestSceneChannelManager.cs index fcb602cb04..5e22101e5c 100644 --- a/osu.Game.Tests/Chat/TestSceneChannelManager.cs +++ b/osu.Game.Tests/Chat/TestSceneChannelManager.cs @@ -47,19 +47,11 @@ namespace osu.Game.Tests.Chat return true; case PostMessageRequest postMessage: - var message = new Message(++currentMessageId) - { - IsAction = postMessage.Message.IsAction, - ChannelId = postMessage.Message.ChannelId, - Content = postMessage.Message.Content, - Links = postMessage.Message.Links, - Timestamp = postMessage.Message.Timestamp, - Sender = postMessage.Message.Sender - }; - - sentMessages.Add(message); - postMessage.TriggerSuccess(message); + handlePostMessageRequest(postMessage); + return true; + case MarkChannelAsReadRequest markRead: + handleMarkChannelAsReadRequest(markRead); return true; } @@ -101,41 +93,44 @@ namespace osu.Game.Tests.Chat }); AddStep("post message", () => channelManager.PostMessage("Something interesting")); - AddUntilStep("wait until the message is posted", () => channel.Messages.Count == sentMessages.Count); + AddStep("post /help command", () => channelManager.PostCommand("help", channel)); AddStep("post /me command with no action", () => channelManager.PostCommand("me", channel)); AddStep("post /join command with no channel", () => channelManager.PostCommand("join", channel)); AddStep("post /join command with non-existent channel", () => channelManager.PostCommand("join i-dont-exist", channel)); AddStep("post non-existent command", () => channelManager.PostCommand("non-existent-cmd arg", channel)); - AddStep("register mark channel as read request handler", () => - { - ((DummyAPIAccess)API).HandleRequest = req => - { - switch (req) - { - case MarkChannelAsReadRequest markRead: - var isSentMessage = sentMessages.Contains(markRead.Message); - - AddAssert("mark channel as read called with a real message", () => isSentMessage); - - if (isSentMessage) - { - markRead.TriggerSuccess(); - } - else - { - markRead.TriggerFailure(new APIException("unknown message!", null)); - } - - return true; - } - - return false; - }; - }); - AddStep("mark channel as read", () => channelManager.MarkChannelAsRead(channel)); + AddAssert("channel's last read ID is set to the latest message", () => channel.LastReadId == sentMessages.Last().Id); + } + + private void handlePostMessageRequest(PostMessageRequest request) + { + var message = new Message(++currentMessageId) + { + IsAction = request.Message.IsAction, + ChannelId = request.Message.ChannelId, + Content = request.Message.Content, + Links = request.Message.Links, + Timestamp = request.Message.Timestamp, + Sender = request.Message.Sender + }; + + sentMessages.Add(message); + request.TriggerSuccess(message); + } + + private void handleMarkChannelAsReadRequest(MarkChannelAsReadRequest request) + { + // only accept messages that were sent through the API + if (sentMessages.Contains(request.Message)) + { + request.TriggerSuccess(); + } + else + { + request.TriggerFailure(new APIException("unknown message!", null)); + } } private Channel createChannel(int id, ChannelType type) => new Channel(new User())