Refactor recent messages API (#4763)

This exposes internal functions for testing by splitting the implementation & internal API into separate files

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)

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

src/providers/RecentMessagesApi.cpp

@@ -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 <IrcMessage>
-#include <QJsonArray>
-#include <QJsonObject>
-#include <QUrl>
-#include <QUrlQuery>
-
-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<Communi::IrcMessage *> parseRecentMessages(
-        const QJsonObject &jsonRoot)
-    {
-        QJsonArray jsonMessages = jsonRoot.value("messages").toArray();
-        std::vector<Communi::IrcMessage *> 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<MessagePtr> buildRecentMessages(
-        std::vector<Communi::IrcMessage *> &messages, Channel *channel)
-    {
-        auto &handler = IrcMessageHandler::instance();
-        std::vector<MessagePtr> 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<Channel> 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

src/providers/RecentMessagesApi.hpp

@@ -1,33 +0,0 @@
-#pragma once
-
-#include "ForwardDecl.hpp"
-
-#include <QString>
-
-#include <functional>
-#include <memory>
-#include <vector>
-
-namespace chatterino {
-
-class RecentMessagesApi
-{
-public:
-    using ResultCallback = std::function<void(const std::vector<MessagePtr> &)>;
-    using ErrorCallback = std::function<void()>;
-
-    /**
-     * @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<MessagePtr> &
-     * @param onError Callback called when the network request fails
-     */
-    static void loadRecentMessages(const QString &channelName,
-                                   std::weak_ptr<Channel> channelPtr,
-                                   ResultCallback onLoaded,
-                                   ErrorCallback onError);
-};
-
-}  // namespace chatterino

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<Channel> 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

src/providers/recentmessages/Api.hpp

@@ -0,0 +1,35 @@
+#pragma once
+
+#include <QString>
+
+#include <functional>
+#include <memory>
+#include <vector>
+
+namespace chatterino {
+
+class Channel;
+using ChannelPtr = std::shared_ptr<Channel>;
+
+struct Message;
+using MessagePtr = std::shared_ptr<const Message>;
+
+}  // namespace chatterino
+
+namespace chatterino::recentmessages {
+
+using ResultCallback = std::function<void(const std::vector<MessagePtr> &)>;
+using ErrorCallback = std::function<void()>;
+
+/**
+ * @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<MessagePtr> &
+ * @param onError Callback called when the network request fails
+ */
+void load(const QString &channelName, std::weak_ptr<Channel> channelPtr,
+          ResultCallback onLoaded, ErrorCallback onError);
+
+}  // namespace chatterino::recentmessages

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 <QJsonArray>
+#include <QUrlQuery>
+
+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<Communi::IrcMessage *> parseRecentMessages(
+    const QJsonObject &jsonRoot)
+{
+    const auto jsonMessages = jsonRoot.value("messages").toArray();
+    std::vector<Communi::IrcMessage *> 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<MessagePtr> buildRecentMessages(
+    std::vector<Communi::IrcMessage *> &messages, Channel *channel)
+{
+    auto &handler = IrcMessageHandler::instance();
+    std::vector<MessagePtr> 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

src/providers/recentmessages/Impl.hpp

@@ -0,0 +1,35 @@
+#pragma once
+
+#include "common/Channel.hpp"
+#include "messages/Message.hpp"
+
+#include <IrcMessage>
+#include <QJsonObject>
+#include <QString>
+#include <QUrl>
+
+#include <memory>
+#include <vector>
+
+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<Communi::IrcMessage *> parseRecentMessages(
+    const QJsonObject &jsonRoot);
+
+// Build Communi messages retrieved from the recent messages API into
+// proper chatterino messages.
+std::vector<MessagePtr> buildRecentMessages(
+    std::vector<Communi::IrcMessage *> &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

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<Channel>(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<Channel>(this);
-    RecentMessagesApi::loadRecentMessages(
+    recentmessages::load(
         this->getName(), weak,
         [weak](const auto &messages) {
             auto shared = weak.lock();