From 3e1e400e3e3008e8078d667ef0ce8a5f03e6f27a Mon Sep 17 00:00:00 2001 From: pajlada Date: Sat, 12 Aug 2023 13:34:59 +0200 Subject: [PATCH] Refactor recent messages API (#4763) This exposes internal functions for testing by splitting the implementation & internal API into separate files --- CHANGELOG.md | 1 + src/CMakeLists.txt | 7 +- src/providers/RecentMessagesApi.cpp | 240 ------------------------- src/providers/RecentMessagesApi.hpp | 33 ---- src/providers/recentmessages/Api.cpp | 91 ++++++++++ src/providers/recentmessages/Api.hpp | 35 ++++ src/providers/recentmessages/Impl.cpp | 162 +++++++++++++++++ src/providers/recentmessages/Impl.hpp | 35 ++++ src/providers/twitch/TwitchChannel.cpp | 6 +- 9 files changed, 332 insertions(+), 278 deletions(-) delete mode 100644 src/providers/RecentMessagesApi.cpp delete mode 100644 src/providers/RecentMessagesApi.hpp create mode 100644 src/providers/recentmessages/Api.cpp create mode 100644 src/providers/recentmessages/Api.hpp create mode 100644 src/providers/recentmessages/Impl.cpp create mode 100644 src/providers/recentmessages/Impl.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ae837eff..bd6501366 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ - Dev: Replace our QObjectRef class with Qt's QPointer class. (#4666) - Dev: Fixed warnings about QWidgets already having a QLayout. (#4672) - Dev: Fixed undefined behavior when loading non-existant credentials. (#4673) +- Dev: Small refactor of the recent-messages API, splitting its internal API and its internal implementation up into separate files. (#4763) - Dev: Added support for compiling with `sccache`. (#4678) - Dev: Added `sccache` in Windows CI. (#4678) - Dev: Moved preprocessor Git and date definitions to executables only. (#4681) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 77d5bd282..7b5a11bdd 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -238,8 +238,6 @@ set(SOURCE_FILES providers/LinkResolver.hpp providers/NetworkConfigurationProvider.cpp providers/NetworkConfigurationProvider.hpp - providers/RecentMessagesApi.cpp - providers/RecentMessagesApi.hpp providers/bttv/BttvEmotes.cpp providers/bttv/BttvEmotes.hpp @@ -288,6 +286,11 @@ set(SOURCE_FILES providers/liveupdates/BasicPubSubManager.hpp providers/liveupdates/BasicPubSubWebsocket.hpp + providers/recentmessages/Api.cpp + providers/recentmessages/Api.hpp + providers/recentmessages/Impl.cpp + providers/recentmessages/Impl.hpp + providers/seventv/SeventvAPI.cpp providers/seventv/SeventvAPI.hpp providers/seventv/SeventvBadges.cpp diff --git a/src/providers/RecentMessagesApi.cpp b/src/providers/RecentMessagesApi.cpp deleted file mode 100644 index 05ee6e8c8..000000000 --- a/src/providers/RecentMessagesApi.cpp +++ /dev/null @@ -1,240 +0,0 @@ -#include "providers/RecentMessagesApi.hpp" - -#include "common/Channel.hpp" -#include "common/Common.hpp" -#include "common/Env.hpp" -#include "common/NetworkRequest.hpp" -#include "common/NetworkResult.hpp" -#include "common/QLogging.hpp" -#include "messages/Message.hpp" -#include "providers/twitch/IrcMessageHandler.hpp" -#include "providers/twitch/TwitchChannel.hpp" -#include "providers/twitch/TwitchMessageBuilder.hpp" -#include "singletons/Settings.hpp" -#include "util/FormatTime.hpp" -#include "util/PostToThread.hpp" - -#include -#include -#include -#include -#include - -namespace chatterino { - -namespace { - - // convertClearchatToNotice takes a Communi::IrcMessage that is a CLEARCHAT - // command and converts it to a readable NOTICE message. This has - // historically been done in the Recent Messages API, but this functionality - // has been moved to Chatterino instead. - auto convertClearchatToNotice(Communi::IrcMessage *message) - { - auto channelName = message->parameter(0); - QString noticeMessage{}; - if (message->tags().contains("target-user-id")) - { - auto target = message->parameter(1); - - if (message->tags().contains("ban-duration")) - { - // User was timed out - noticeMessage = - QString("%1 has been timed out for %2.") - .arg(target) - .arg(formatTime( - message->tag("ban-duration").toString())); - } - else - { - // User was permanently banned - noticeMessage = - QString("%1 has been permanently banned.").arg(target); - } - } - else - { - // Chat was cleared - noticeMessage = "Chat has been cleared by a moderator."; - } - - // rebuild the raw IRC message so we can convert it back to an ircmessage again! - // this could probably be done in a smarter way - - auto s = QString(":tmi.twitch.tv NOTICE %1 :%2") - .arg(channelName) - .arg(noticeMessage); - - auto newMessage = Communi::IrcMessage::fromData(s.toUtf8(), nullptr); - newMessage->setTags(message->tags()); - - return newMessage; - } - - // Parse the IRC messages returned in JSON form into Communi messages - std::vector parseRecentMessages( - const QJsonObject &jsonRoot) - { - QJsonArray jsonMessages = jsonRoot.value("messages").toArray(); - std::vector messages; - - if (jsonMessages.empty()) - return messages; - - for (const auto jsonMessage : jsonMessages) - { - auto content = jsonMessage.toString(); - - // For explanation of why this exists, see src/providers/twitch/TwitchChannel.hpp, - // where these constants are defined - content.replace(COMBINED_FIXER, ZERO_WIDTH_JOINER); - - auto message = - Communi::IrcMessage::fromData(content.toUtf8(), nullptr); - - if (message->command() == "CLEARCHAT") - { - message = convertClearchatToNotice(message); - } - - messages.emplace_back(std::move(message)); - } - - return messages; - } - - // Build Communi messages retrieved from the recent messages API into - // proper chatterino messages. - std::vector buildRecentMessages( - std::vector &messages, Channel *channel) - { - auto &handler = IrcMessageHandler::instance(); - std::vector allBuiltMessages; - - for (auto message : messages) - { - if (message->tags().contains("rm-received-ts")) - { - QDate msgDate = - QDateTime::fromMSecsSinceEpoch( - message->tags().value("rm-received-ts").toLongLong()) - .date(); - - // Check if we need to insert a message stating that a new day began - if (msgDate != channel->lastDate_) - { - channel->lastDate_ = msgDate; - auto msg = makeSystemMessage( - QLocale().toString(msgDate, QLocale::LongFormat), - QTime(0, 0)); - msg->flags.set(MessageFlag::RecentMessage); - allBuiltMessages.emplace_back(msg); - } - } - - auto builtMessages = handler.parseMessageWithReply( - channel, message, allBuiltMessages); - - for (auto builtMessage : builtMessages) - { - builtMessage->flags.set(MessageFlag::RecentMessage); - allBuiltMessages.emplace_back(builtMessage); - } - - message->deleteLater(); - } - - return allBuiltMessages; - } - - // Returns the URL to be used for querying the Recent Messages API for the - // given channel. - QUrl constructRecentMessagesUrl(const QString &name) - { - QUrl url(Env::get().recentMessagesApiUrl.arg(name)); - QUrlQuery urlQuery(url); - if (!urlQuery.hasQueryItem("limit")) - { - urlQuery.addQueryItem( - "limit", - QString::number(getSettings()->twitchMessageHistoryLimit)); - } - url.setQuery(urlQuery); - return url; - } - -} // namespace - -void RecentMessagesApi::loadRecentMessages(const QString &channelName, - std::weak_ptr channelPtr, - ResultCallback onLoaded, - ErrorCallback onError) -{ - qCDebug(chatterinoRecentMessages) - << "Loading recent messages for" << channelName; - - QUrl url = constructRecentMessagesUrl(channelName); - - NetworkRequest(url) - .onSuccess([channelPtr, onLoaded](NetworkResult result) -> Outcome { - auto shared = channelPtr.lock(); - if (!shared) - return Failure; - - qCDebug(chatterinoRecentMessages) - << "Successfully loaded recent messages for" - << shared->getName(); - - auto root = result.parseJson(); - auto parsedMessages = parseRecentMessages(root); - - // build the Communi messages into chatterino messages - auto builtMessages = - buildRecentMessages(parsedMessages, shared.get()); - - postToThread([shared = std::move(shared), root = std::move(root), - messages = std::move(builtMessages), - onLoaded]() mutable { - // Notify user about a possible gap in logs if it returned some messages - // but isn't currently joined to a channel - if (QString errorCode = root.value("error_code").toString(); - !errorCode.isEmpty()) - { - qCDebug(chatterinoRecentMessages) - << QString("Got error from API: error_code=%1, " - "channel=%2") - .arg(errorCode, shared->getName()); - if (errorCode == "channel_not_joined" && !messages.empty()) - { - shared->addMessage(makeSystemMessage( - "Message history service recovering, there may " - "be gaps in the message history.")); - } - } - - onLoaded(messages); - }); - - return Success; - }) - .onError([channelPtr, onError](const NetworkResult &result) { - auto shared = channelPtr.lock(); - if (!shared) - { - return; - } - - qCDebug(chatterinoRecentMessages) - << "Failed to load recent messages for" << shared->getName(); - - shared->addMessage(makeSystemMessage( - QStringLiteral( - "Message history service unavailable (Error: %1)") - .arg(result.formatError()))); - - onError(); - }) - .execute(); -} - -} // namespace chatterino diff --git a/src/providers/RecentMessagesApi.hpp b/src/providers/RecentMessagesApi.hpp deleted file mode 100644 index 30137d5d2..000000000 --- a/src/providers/RecentMessagesApi.hpp +++ /dev/null @@ -1,33 +0,0 @@ -#pragma once - -#include "ForwardDecl.hpp" - -#include - -#include -#include -#include - -namespace chatterino { - -class RecentMessagesApi -{ -public: - using ResultCallback = std::function &)>; - using ErrorCallback = std::function; - - /** - * @brief Loads recent messages for a channel using the Recent Messages API - * - * @param channelName Name of Twitch channel - * @param channelPtr Weak pointer to Channel to use to build messages - * @param onLoaded Callback taking the built messages as a const std::vector & - * @param onError Callback called when the network request fails - */ - static void loadRecentMessages(const QString &channelName, - std::weak_ptr channelPtr, - ResultCallback onLoaded, - ErrorCallback onError); -}; - -} // namespace chatterino diff --git a/src/providers/recentmessages/Api.cpp b/src/providers/recentmessages/Api.cpp new file mode 100644 index 000000000..a667c0cbb --- /dev/null +++ b/src/providers/recentmessages/Api.cpp @@ -0,0 +1,91 @@ +#include "providers/recentmessages/Api.hpp" + +#include "common/NetworkRequest.hpp" +#include "common/NetworkResult.hpp" +#include "common/QLogging.hpp" +#include "providers/recentmessages/Impl.hpp" +#include "providers/twitch/TwitchMessageBuilder.hpp" +#include "util/PostToThread.hpp" + +namespace { + +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +const auto &LOG = chatterinoRecentMessages; + +} // namespace + +namespace chatterino::recentmessages { + +using namespace recentmessages::detail; + +void load(const QString &channelName, std::weak_ptr channelPtr, + ResultCallback onLoaded, ErrorCallback onError) +{ + qCDebug(LOG) << "Loading recent messages for" << channelName; + + const auto url = constructRecentMessagesUrl(channelName); + + NetworkRequest(url) + .onSuccess([channelPtr, onLoaded](const auto &result) -> Outcome { + auto shared = channelPtr.lock(); + if (!shared) + { + return Failure; + } + + qCDebug(LOG) << "Successfully loaded recent messages for" + << shared->getName(); + + auto root = result.parseJson(); + auto parsedMessages = parseRecentMessages(root); + + // build the Communi messages into chatterino messages + auto builtMessages = + buildRecentMessages(parsedMessages, shared.get()); + + postToThread([shared = std::move(shared), root = std::move(root), + messages = std::move(builtMessages), + onLoaded]() mutable { + // Notify user about a possible gap in logs if it returned some messages + // but isn't currently joined to a channel + const auto errorCode = root.value("error_code").toString(); + if (!errorCode.isEmpty()) + { + qCDebug(LOG) + << QString("Got error from API: error_code=%1, " + "channel=%2") + .arg(errorCode, shared->getName()); + if (errorCode == "channel_not_joined" && !messages.empty()) + { + shared->addMessage(makeSystemMessage( + "Message history service recovering, there may " + "be gaps in the message history.")); + } + } + + onLoaded(messages); + }); + + return Success; + }) + .onError([channelPtr, onError](const NetworkResult &result) { + auto shared = channelPtr.lock(); + if (!shared) + { + return; + } + + qCDebug(LOG) << "Failed to load recent messages for" + << shared->getName(); + + shared->addMessage(makeSystemMessage( + QStringLiteral( + "Message history service unavailable (Error: %1)") + .arg(result.formatError()))); + + onError(); + }) + .execute(); +} + +} // namespace chatterino::recentmessages diff --git a/src/providers/recentmessages/Api.hpp b/src/providers/recentmessages/Api.hpp new file mode 100644 index 000000000..2a558010e --- /dev/null +++ b/src/providers/recentmessages/Api.hpp @@ -0,0 +1,35 @@ +#pragma once + +#include + +#include +#include +#include + +namespace chatterino { + +class Channel; +using ChannelPtr = std::shared_ptr; + +struct Message; +using MessagePtr = std::shared_ptr; + +} // namespace chatterino + +namespace chatterino::recentmessages { + +using ResultCallback = std::function &)>; +using ErrorCallback = std::function; + +/** + * @brief Loads recent messages for a channel using the Recent Messages API + * + * @param channelName Name of Twitch channel + * @param channelPtr Weak pointer to Channel to use to build messages + * @param onLoaded Callback taking the built messages as a const std::vector & + * @param onError Callback called when the network request fails + */ +void load(const QString &channelName, std::weak_ptr channelPtr, + ResultCallback onLoaded, ErrorCallback onError); + +} // namespace chatterino::recentmessages diff --git a/src/providers/recentmessages/Impl.cpp b/src/providers/recentmessages/Impl.cpp new file mode 100644 index 000000000..e271084b0 --- /dev/null +++ b/src/providers/recentmessages/Impl.cpp @@ -0,0 +1,162 @@ +#include "providers/recentmessages/Impl.hpp" + +#include "common/Env.hpp" +#include "common/QLogging.hpp" +#include "providers/twitch/IrcMessageHandler.hpp" +#include "providers/twitch/TwitchChannel.hpp" +#include "providers/twitch/TwitchMessageBuilder.hpp" +#include "singletons/Settings.hpp" +#include "util/FormatTime.hpp" + +#include +#include + +namespace { + +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +const auto &LOG = chatterinoRecentMessages; + +} // namespace + +namespace chatterino::recentmessages::detail { + +// convertClearchatToNotice takes a Communi::IrcMessage that is a CLEARCHAT +// command and converts it to a readable NOTICE message. This has +// historically been done in the Recent Messages API, but this functionality +// has been moved to Chatterino instead. +Communi::IrcMessage *convertClearchatToNotice(Communi::IrcMessage *message) +{ + auto channelName = message->parameter(0); + QString noticeMessage{}; + if (message->tags().contains("target-user-id")) + { + auto target = message->parameter(1); + + if (message->tags().contains("ban-duration")) + { + // User was timed out + noticeMessage = + QString("%1 has been timed out for %2.") + .arg(target) + .arg(formatTime(message->tag("ban-duration").toString())); + } + else + { + // User was permanently banned + noticeMessage = + QString("%1 has been permanently banned.").arg(target); + } + } + else + { + // Chat was cleared + noticeMessage = "Chat has been cleared by a moderator."; + } + + // rebuild the raw IRC message so we can convert it back to an ircmessage again! + // this could probably be done in a smarter way + + auto s = QString(":tmi.twitch.tv NOTICE %1 :%2") + .arg(channelName) + .arg(noticeMessage); + + auto *newMessage = Communi::IrcMessage::fromData(s.toUtf8(), nullptr); + newMessage->setTags(message->tags()); + + return newMessage; +} + +// Parse the IRC messages returned in JSON form into Communi messages +std::vector parseRecentMessages( + const QJsonObject &jsonRoot) +{ + const auto jsonMessages = jsonRoot.value("messages").toArray(); + std::vector messages; + + if (jsonMessages.empty()) + { + return messages; + } + + for (const auto &jsonMessage : jsonMessages) + { + auto content = jsonMessage.toString(); + + // For explanation of why this exists, see src/providers/twitch/TwitchChannel.hpp, + // where these constants are defined + content.replace(COMBINED_FIXER, ZERO_WIDTH_JOINER); + + auto *message = + Communi::IrcMessage::fromData(content.toUtf8(), nullptr); + + if (message->command() == "CLEARCHAT") + { + message = convertClearchatToNotice(message); + } + + messages.emplace_back(message); + } + + return messages; +} + +// Build Communi messages retrieved from the recent messages API into +// proper chatterino messages. +std::vector buildRecentMessages( + std::vector &messages, Channel *channel) +{ + auto &handler = IrcMessageHandler::instance(); + std::vector allBuiltMessages; + + for (auto *message : messages) + { + if (message->tags().contains("rm-received-ts")) + { + const auto msgDate = + QDateTime::fromMSecsSinceEpoch( + message->tags().value("rm-received-ts").toLongLong()) + .date(); + + // Check if we need to insert a message stating that a new day began + if (msgDate != channel->lastDate_) + { + channel->lastDate_ = msgDate; + auto msg = makeSystemMessage( + QLocale().toString(msgDate, QLocale::LongFormat), + QTime(0, 0)); + msg->flags.set(MessageFlag::RecentMessage); + allBuiltMessages.emplace_back(msg); + } + } + + auto builtMessages = + handler.parseMessageWithReply(channel, message, allBuiltMessages); + + for (const auto &builtMessage : builtMessages) + { + builtMessage->flags.set(MessageFlag::RecentMessage); + allBuiltMessages.emplace_back(builtMessage); + } + + message->deleteLater(); + } + + return allBuiltMessages; +} + +// Returns the URL to be used for querying the Recent Messages API for the +// given channel. +QUrl constructRecentMessagesUrl(const QString &name) +{ + QUrl url(Env::get().recentMessagesApiUrl.arg(name)); + QUrlQuery urlQuery(url); + if (!urlQuery.hasQueryItem("limit")) + { + urlQuery.addQueryItem( + "limit", QString::number(getSettings()->twitchMessageHistoryLimit)); + } + url.setQuery(urlQuery); + return url; +} + +} // namespace chatterino::recentmessages::detail diff --git a/src/providers/recentmessages/Impl.hpp b/src/providers/recentmessages/Impl.hpp new file mode 100644 index 000000000..3ff3bfb69 --- /dev/null +++ b/src/providers/recentmessages/Impl.hpp @@ -0,0 +1,35 @@ +#pragma once + +#include "common/Channel.hpp" +#include "messages/Message.hpp" + +#include +#include +#include +#include + +#include +#include + +namespace chatterino::recentmessages::detail { + +// convertClearchatToNotice takes a Communi::IrcMessage that is a CLEARCHAT +// command and converts it to a readable NOTICE message. This has +// historically been done in the Recent Messages API, but this functionality +// has been moved to Chatterino instead. +Communi::IrcMessage *convertClearchatToNotice(Communi::IrcMessage *message); + +// Parse the IRC messages returned in JSON form into Communi messages +std::vector parseRecentMessages( + const QJsonObject &jsonRoot); + +// Build Communi messages retrieved from the recent messages API into +// proper chatterino messages. +std::vector buildRecentMessages( + std::vector &messages, Channel *channel); + +// Returns the URL to be used for querying the Recent Messages API for the +// given channel. +QUrl constructRecentMessagesUrl(const QString &name); + +} // namespace chatterino::recentmessages::detail diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index c34a3a039..f8bbd1eec 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/src/providers/twitch/TwitchChannel.cpp @@ -18,7 +18,7 @@ #include "providers/bttv/BttvEmotes.hpp" #include "providers/bttv/BttvLiveUpdates.hpp" #include "providers/bttv/liveupdates/BttvLiveUpdateMessages.hpp" -#include "providers/RecentMessagesApi.hpp" +#include "providers/recentmessages/Api.hpp" #include "providers/seventv/eventapi/Dispatch.hpp" #include "providers/seventv/SeventvAPI.hpp" #include "providers/seventv/SeventvEmotes.hpp" @@ -1108,7 +1108,7 @@ void TwitchChannel::loadRecentMessages() } auto weak = weakOf(this); - RecentMessagesApi::loadRecentMessages( + recentmessages::load( this->getName(), weak, [weak](const auto &messages) { auto shared = weak.lock(); @@ -1163,7 +1163,7 @@ void TwitchChannel::loadRecentMessagesReconnect() } auto weak = weakOf(this); - RecentMessagesApi::loadRecentMessages( + recentmessages::load( this->getName(), weak, [weak](const auto &messages) { auto shared = weak.lock();