refactor: Make ChatterinoBadges less of a singleton (#5103)

CHANGELOG.md

@@ -108,6 +108,7 @@
 - Dev: Refactored the Image Uploader feature. (#4971)
 - Dev: Refactored the SplitOverlay code. (#5082)
 - Dev: Refactored the TwitchBadges structure, making it less of a singleton. (#5096)
+- Dev: Refactored the ChatterinoBadges structure, making it less of a singleton. (#5103)
 - Dev: Moved the Network files to their own folder. (#5089)
 - Dev: Fixed deadlock and use-after-free in tests. (#4981)
 - Dev: Moved all `.clang-format` files to the root directory. (#5037)

mocks/include/mocks/ChatterinoBadges.hpp

@@ -0,0 +1,17 @@
+#pragma once
+
+#include "providers/chatterino/ChatterinoBadges.hpp"
+
+namespace chatterino::mock {
+
+class ChatterinoBadges : public IChatterinoBadges
+{
+public:
+    std::optional<EmotePtr> getBadge(const UserId &id) override
+    {
+        (void)id;
+        return std::nullopt;
+    }
+};
+
+}  // namespace chatterino::mock

mocks/include/mocks/EmptyApplication.hpp

@@ -134,7 +134,7 @@ public:
         return nullptr;
     }
 
-    ChatterinoBadges *getChatterinoBadges() override
+    IChatterinoBadges *getChatterinoBadges() override
     {
         assert(false && "EmptyApplication::getChatterinoBadges was called "
                         "without being initialized");

src/Application.cpp

@@ -127,7 +127,6 @@ Application::Application(Settings &_settings, const Paths &paths,
     , notifications(&this->emplace<NotificationController>())
     , highlights(&this->emplace<HighlightController>())
     , twitch(&this->emplace<TwitchIrcServer>())
-    , chatterinoBadges(&this->emplace<ChatterinoBadges>())
     , ffzBadges(&this->emplace<FfzBadges>())
     , seventvBadges(&this->emplace<SeventvBadges>())
     , userData(&this->emplace(new UserDataController(paths)))
@@ -135,6 +134,7 @@ Application::Application(Settings &_settings, const Paths &paths,
     , twitchLiveController(&this->emplace<TwitchLiveController>())
     , twitchPubSub(new PubSub(TWITCH_PUBSUB_URL))
     , twitchBadges(new TwitchBadges)
+    , chatterinoBadges(new ChatterinoBadges)
     , logging(new Logging(_settings))
 #ifdef CHATTERINO_HAVE_PLUGINS
     , plugins(&this->emplace(new PluginController(paths)))
@@ -156,6 +156,7 @@ void Application::fakeDtor()
 {
     this->twitchPubSub.reset();
     this->twitchBadges.reset();
+    this->chatterinoBadges.reset();
 }
 
 void Application::initialize(Settings &settings, const Paths &paths)
@@ -339,6 +340,14 @@ TwitchBadges *Application::getTwitchBadges()
     return this->twitchBadges.get();
 }
 
+IChatterinoBadges *Application::getChatterinoBadges()
+{
+    assertInGuiThread();
+    assert(this->chatterinoBadges);
+
+    return this->chatterinoBadges.get();
+}
+
 ITwitchIrcServer *Application::getTwitch()
 {
     assertInGuiThread();

src/Application.hpp

@@ -44,6 +44,7 @@ class IEmotes;
 class Settings;
 class Fonts;
 class Toasts;
+class IChatterinoBadges;
 class ChatterinoBadges;
 class FfzBadges;
 class SeventvBadges;
@@ -75,7 +76,7 @@ public:
     virtual ITwitchIrcServer *getTwitch() = 0;
     virtual PubSub *getTwitchPubSub() = 0;
     virtual Logging *getChatLogger() = 0;
-    virtual ChatterinoBadges *getChatterinoBadges() = 0;
+    virtual IChatterinoBadges *getChatterinoBadges() = 0;
     virtual FfzBadges *getFfzBadges() = 0;
     virtual SeventvBadges *getSeventvBadges() = 0;
     virtual IUserDataController *getUserData() = 0;
@@ -136,7 +137,6 @@ public:
     NotificationController *const notifications{};
     HighlightController *const highlights{};
     TwitchIrcServer *const twitch{};
-    ChatterinoBadges *const chatterinoBadges{};
     FfzBadges *const ffzBadges{};
     SeventvBadges *const seventvBadges{};
     UserDataController *const userData{};
@@ -146,6 +146,7 @@ private:
     TwitchLiveController *const twitchLiveController{};
     std::unique_ptr<PubSub> twitchPubSub;
     std::unique_ptr<TwitchBadges> twitchBadges;
+    std::unique_ptr<ChatterinoBadges> chatterinoBadges;
     const std::unique_ptr<Logging> logging;
 
 public:
@@ -225,12 +226,6 @@ public:
     ITwitchIrcServer *getTwitch() override;
     PubSub *getTwitchPubSub() override;
     Logging *getChatLogger() override;
-    ChatterinoBadges *getChatterinoBadges() override
-    {
-        assertInGuiThread();
-
-        return this->chatterinoBadges;
-    }
     FfzBadges *getFfzBadges() override
     {
         assertInGuiThread();
@@ -247,6 +242,7 @@ public:
     ISoundController *getSound() override;
     ITwitchLiveController *getTwitchLiveController() override;
     TwitchBadges *getTwitchBadges() override;
+    IChatterinoBadges *getChatterinoBadges() override;
     ImageUploader *getImageUploader() override
     {
         assertInGuiThread();

src/providers/chatterino/ChatterinoBadges.cpp

@@ -1,4 +1,4 @@
-#include "ChatterinoBadges.hpp"
+#include "providers/chatterino/ChatterinoBadges.hpp"
 
 #include "common/network/NetworkRequest.hpp"
 #include "common/network/NetworkResult.hpp"
@@ -7,17 +7,13 @@
 #include <QJsonArray>
 #include <QJsonObject>
 #include <QJsonValue>
-#include <QThread>
 #include <QUrl>
 
 namespace chatterino {
-void ChatterinoBadges::initialize(Settings &settings, const Paths &paths)
-{
-    this->loadChatterinoBadges();
-}
 
 ChatterinoBadges::ChatterinoBadges()
 {
+    this->loadChatterinoBadges();
 }
 
 std::optional<EmotePtr> ChatterinoBadges::getBadge(const UserId &id)
@@ -44,15 +40,19 @@ void ChatterinoBadges::loadChatterinoBadges()
             std::unique_lock lock(this->mutex_);
 
             int index = 0;
-            for (const auto &jsonBadge_ : jsonRoot.value("badges").toArray())
+            for (const auto &jsonBadgeValue :
+                 jsonRoot.value("badges").toArray())
             {
-                auto jsonBadge = jsonBadge_.toObject();
+                auto jsonBadge = jsonBadgeValue.toObject();
                 auto emote = Emote{
-                    EmoteName{},
+                    .name = EmoteName{},
-                    ImageSet{Url{jsonBadge.value("image1").toString()},
+                    .images =
-                             Url{jsonBadge.value("image2").toString()},
+                        ImageSet{Url{jsonBadge.value("image1").toString()},
-                             Url{jsonBadge.value("image3").toString()}},
+                                 Url{jsonBadge.value("image2").toString()},
-                    Tooltip{jsonBadge.value("tooltip").toString()}, Url{}};
+                                 Url{jsonBadge.value("image3").toString()}},
+                    .tooltip = Tooltip{jsonBadge.value("tooltip").toString()},
+                    .homePage = Url{},
+                };
 
                 emotes.push_back(
                     std::make_shared<const Emote>(std::move(emote)));

src/providers/chatterino/ChatterinoBadges.hpp

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "common/Aliases.hpp"
-#include "common/Singleton.hpp"
 #include "util/QStringHash.hpp"
 
 #include <memory>
@@ -15,20 +14,48 @@ namespace chatterino {
 struct Emote;
 using EmotePtr = std::shared_ptr<const Emote>;
 
-class ChatterinoBadges : public Singleton
+class IChatterinoBadges
 {
 public:
-    void initialize(Settings &settings, const Paths &paths) override;
+    IChatterinoBadges() = default;
+    virtual ~IChatterinoBadges() = default;
+
+    IChatterinoBadges(const IChatterinoBadges &) = delete;
+    IChatterinoBadges(IChatterinoBadges &&) = delete;
+    IChatterinoBadges &operator=(const IChatterinoBadges &) = delete;
+    IChatterinoBadges &operator=(IChatterinoBadges &&) = delete;
+
+    virtual std::optional<EmotePtr> getBadge(const UserId &id) = 0;
+};
+
+class ChatterinoBadges : public IChatterinoBadges
+{
+public:
+    /**
+     * Makes a network request to load Chatterino user badges
+     */
     ChatterinoBadges();
 
-    std::optional<EmotePtr> getBadge(const UserId &id);
+    /**
+     * Returns the Chatterino badge for the given user
+     */
+    std::optional<EmotePtr> getBadge(const UserId &id) override;
 
 private:
     void loadChatterinoBadges();
 
     std::shared_mutex mutex_;
 
+    /**
+     * Maps Twitch user IDs to their badge index
+     * Guarded by mutex_
+     */
     std::unordered_map<QString, int> badgeMap;
+
+    /**
+     * Keeps a list of badges.
+     * Indexes in here are referred to by badgeMap
+     */
     std::vector<EmotePtr> emotes;
 };
 

tests/src/Filters.cpp

@@ -4,10 +4,10 @@
 #include "controllers/filters/lang/Types.hpp"
 #include "controllers/highlights/HighlightController.hpp"
 #include "mocks/Channel.hpp"
+#include "mocks/ChatterinoBadges.hpp"
 #include "mocks/EmptyApplication.hpp"
 #include "mocks/TwitchIrcServer.hpp"
 #include "mocks/UserData.hpp"
-#include "providers/chatterino/ChatterinoBadges.hpp"
 #include "providers/ffz/FfzBadges.hpp"
 #include "providers/seventv/SeventvBadges.hpp"
 #include "providers/twitch/TwitchBadge.hpp"
@@ -49,7 +49,7 @@ public:
         return &this->twitch;
     }
 
-    ChatterinoBadges *getChatterinoBadges() override
+    IChatterinoBadges *getChatterinoBadges() override
     {
         return &this->chatterinoBadges;
     }
@@ -73,7 +73,7 @@ public:
     Emotes emotes;
     mock::UserDataController userData;
     mock::MockTwitchIrcServer twitch;
-    ChatterinoBadges chatterinoBadges;
+    mock::ChatterinoBadges chatterinoBadges;
     FfzBadges ffzBadges;
     SeventvBadges seventvBadges;
     HighlightController highlights;

tests/src/TwitchMessageBuilder.cpp

@@ -5,10 +5,10 @@
 #include "controllers/highlights/HighlightController.hpp"
 #include "messages/MessageBuilder.hpp"
 #include "mocks/Channel.hpp"
+#include "mocks/ChatterinoBadges.hpp"
 #include "mocks/EmptyApplication.hpp"
 #include "mocks/TwitchIrcServer.hpp"
 #include "mocks/UserData.hpp"
-#include "providers/chatterino/ChatterinoBadges.hpp"
 #include "providers/ffz/FfzBadges.hpp"
 #include "providers/seventv/SeventvBadges.hpp"
 #include "providers/twitch/TwitchBadge.hpp"
@@ -50,7 +50,7 @@ public:
         return &this->twitch;
     }
 
-    ChatterinoBadges *getChatterinoBadges() override
+    IChatterinoBadges *getChatterinoBadges() override
     {
         return &this->chatterinoBadges;
     }
@@ -74,7 +74,7 @@ public:
     Emotes emotes;
     mock::UserDataController userData;
     mock::MockTwitchIrcServer twitch;
-    ChatterinoBadges chatterinoBadges;
+    mock::ChatterinoBadges chatterinoBadges;
     FfzBadges ffzBadges;
     SeventvBadges seventvBadges;
     HighlightController highlights;