chore: clean up some of the pronoun implementation (#5583)

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## Unversioned
 
-- Major: Add option to show pronouns in user card. (#5442)
+- Major: Add option to show pronouns in user card. (#5442, #5583)
 - Major: Release plugins alpha. (#5288)
 - Major: Improve high-DPI support on Windows. (#4868, #5391)
 - Minor: Removed the Ctrl+Shift+L hotkey for toggling the "live only" tab visibility state. (#5530)

benchmarks/src/RecentMessages.cpp

@@ -11,7 +11,6 @@
 #include "providers/chatterino/ChatterinoBadges.hpp"
 #include "providers/ffz/FfzBadges.hpp"
 #include "providers/ffz/FfzEmotes.hpp"
-#include "providers/pronouns/Pronouns.hpp"
 #include "providers/recentmessages/Impl.hpp"
 #include "providers/seventv/SeventvBadges.hpp"
 #include "providers/seventv/SeventvEmotes.hpp"
@@ -111,11 +110,6 @@ public:
         return &this->linkResolver;
     }
 
-    pronouns::Pronouns *getPronouns() override
-    {
-        return &this->pronouns;
-    }
-
     AccountController accounts;
     Emotes emotes;
     mock::UserDataController userData;
@@ -130,7 +124,6 @@ public:
     FfzEmotes ffzEmotes;
     SeventvEmotes seventvEmotes;
     DisabledStreamerMode streamerMode;
-    pronouns::Pronouns pronouns;
 };
 
 std::optional<QJsonDocument> tryReadJsonFile(const QString &path)

src/Application.cpp

@@ -179,7 +179,7 @@ Application::Application(Settings &_settings, const Paths &paths,
     , linkResolver(new LinkResolver)
     , streamerMode(new StreamerMode)
     , twitchUsers(new TwitchUsers)
-    , pronouns(std::make_shared<pronouns::Pronouns>())
+    , pronouns(new pronouns::Pronouns)
 #ifdef CHATTERINO_HAVE_PLUGINS
     , plugins(new PluginController(paths))
 #endif

src/Application.hpp

@@ -173,7 +173,7 @@ private:
     std::unique_ptr<ILinkResolver> linkResolver;
     std::unique_ptr<IStreamerMode> streamerMode;
     std::unique_ptr<ITwitchUsers> twitchUsers;
-    std::shared_ptr<pronouns::Pronouns> pronouns;
+    std::unique_ptr<pronouns::Pronouns> pronouns;
 #ifdef CHATTERINO_HAVE_PLUGINS
     std::unique_ptr<PluginController> plugins;
 #endif

src/providers/pronouns/Pronouns.cpp

@@ -6,51 +6,53 @@
 #include "providers/pronouns/UserPronouns.hpp"
 
 #include <mutex>
-#include <string>
 #include <unordered_map>
 
+namespace {
+
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
+const auto &LOG = chatterinoPronouns;
+
+}  // namespace
+
 namespace chatterino::pronouns {
 
-void Pronouns::fetch(const QString &username,
+void Pronouns::getUserPronoun(
+    const QString &username,
     const std::function<void(UserPronouns)> &callbackSuccess,
     const std::function<void()> &callbackFail)
 {
     // Only fetch pronouns if we haven't fetched before.
+    auto cachedPronoun = this->getCachedUserPronoun(username);
+    if (cachedPronoun.has_value())
     {
-        std::shared_lock lock(this->mutex);
-
-        auto iter = this->saved.find(username);
-        if (iter != this->saved.end())
-        {
-            callbackSuccess(iter->second);
+        callbackSuccess(*cachedPronoun);
         return;
     }
-    }  // unlock mutex
 
-    qCDebug(chatterinoPronouns)
-        << "Fetching pronouns from alejo.io for " << username;
-
-    alejoApi.fetch(username, [this, callbackSuccess, callbackFail,
-                              username](std::optional<UserPronouns> result) {
-        if (result.has_value())
-        {
-            {
-                std::unique_lock lock(this->mutex);
-                this->saved[username] = *result;
-            }  // unlock mutex
-            qCDebug(chatterinoPronouns)
-                << "Adding pronouns " << result->format() << " for user "
-                << username;
-            callbackSuccess(*result);
-        }
-        else
+    this->alejoApi.fetch(username, [this, callbackSuccess, callbackFail,
+                                    username](const auto &oUserPronoun) {
+        if (!oUserPronoun.has_value())
         {
             callbackFail();
+            return;
         }
+
+        const auto &userPronoun = *oUserPronoun;
+
+        qCDebug(LOG) << "Caching pronoun" << userPronoun.format() << "for user"
+                     << username;
+        {
+            std::unique_lock lock(this->mutex);
+            this->saved[username] = userPronoun;
+        }
+
+        callbackSuccess(userPronoun);
     });
 }
 
-std::optional<UserPronouns> Pronouns::getForUsername(const QString &username)
+std::optional<UserPronouns> Pronouns::getCachedUserPronoun(
+    const QString &username)
 {
     std::shared_lock lock(this->mutex);
     auto it = this->saved.find(username);

src/providers/pronouns/Pronouns.hpp

@@ -3,6 +3,7 @@
 #include "providers/pronouns/alejo/PronounsAlejoApi.hpp"
 #include "providers/pronouns/UserPronouns.hpp"
 
+#include <functional>
 #include <optional>
 #include <shared_mutex>
 #include <unordered_map>
@@ -12,20 +13,20 @@ namespace chatterino::pronouns {
 class Pronouns
 {
 public:
-    Pronouns() = default;
-
-    void fetch(const QString &username,
+    void getUserPronoun(
+        const QString &username,
         const std::function<void(UserPronouns)> &callbackSuccess,
         const std::function<void()> &callbackFail);
 
-    // Retrieve cached pronouns for user.
-    std::optional<UserPronouns> getForUsername(const QString &username);
-
 private:
+    // Retrieve cached pronouns for user.
+    std::optional<UserPronouns> getCachedUserPronoun(const QString &username);
+
     // mutex for editing the saved map.
     std::shared_mutex mutex;
     // Login name -> Pronouns
     std::unordered_map<QString, UserPronouns> saved;
     AlejoApi alejoApi;
 };
+
 }  // namespace chatterino::pronouns

src/providers/pronouns/alejo/PronounsAlejoApi.cpp

@@ -5,103 +5,52 @@
 #include "common/QLogging.hpp"
 #include "providers/pronouns/UserPronouns.hpp"
 
+#include <QStringBuilder>
+
+#include <mutex>
+#include <unordered_map>
+
+namespace {
+
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
+const auto &LOG = chatterinoPronouns;
+
+constexpr QStringView API_URL = u"https://api.pronouns.alejo.io/v1";
+constexpr QStringView API_USERS_ENDPOINT = u"/users";
+constexpr QStringView API_PRONOUNS_ENDPOINT = u"/pronouns";
+
+}  // namespace
+
 namespace chatterino::pronouns {
 
-UserPronouns AlejoApi::parse(const QJsonObject &object)
-{
-    if (!this->pronounsFromId.has_value())
-    {
-        return {};
-    }
-
-    auto pronoun = object["pronoun_id"];
-
-    if (!pronoun.isString())
-    {
-        return {};
-    }
-
-    auto pronounStr = pronoun.toString();
-    std::shared_lock lock(this->mutex);
-    auto iter = this->pronounsFromId->find(pronounStr);
-    if (iter != this->pronounsFromId->end())
-    {
-        return {iter->second};
-    }
-    return {};
-}
-
 AlejoApi::AlejoApi()
 {
-    std::shared_lock lock(this->mutex);
-    if (this->pronounsFromId)
-    {
-        return;
+    this->loadAvailablePronouns();
 }
 
-    qCDebug(chatterinoPronouns) << "Fetching available pronouns for alejo.io";
-    NetworkRequest(AlejoApi::API_URL + AlejoApi::API_PRONOUNS)
-        .concurrent()
-        .onSuccess([this](const auto &result) {
-            auto object = result.parseJson();
-            if (object.isEmpty())
+void AlejoApi::fetch(
+    const QString &username,
+    const std::function<void(std::optional<UserPronouns>)> &onDone)
 {
-                return;
-            }
-
-            std::unique_lock lock(this->mutex);
-            this->pronounsFromId = {std::unordered_map<QString, QString>()};
-            for (auto const &pronounId : object.keys())
-            {
-                if (!object[pronounId].isObject())
-                {
-                    continue;
-                };
-
-                const auto pronounObj = object[pronounId].toObject();
-
-                if (!pronounObj["subject"].isString())
-                {
-                    continue;
-                }
-
-                QString pronouns = pronounObj["subject"].toString();
-
-                auto singular = pronounObj["singular"];
-                if (singular.isBool() && !singular.toBool() &&
-                    pronounObj["object"].isString())
-                {
-                    pronouns += "/" + pronounObj["object"].toString();
-                }
-
-                this->pronounsFromId->insert_or_assign(pronounId,
-                                                       pronouns.toLower());
-            }
-        })
-        .execute();
-}
-
-void AlejoApi::fetch(const QString &username,
-                     std::function<void(std::optional<UserPronouns>)> onDone)
-{
-    bool havePronounList{true};
     {
         std::shared_lock lock(this->mutex);
-        havePronounList = this->pronounsFromId.has_value();
-    }  // unlock mutex
-
-    if (!havePronounList)
+        if (this->pronouns.empty())
         {
-        // Pronoun list not available yet, just fail and try again next time.
+            // Pronoun list not available yet, fail and try again next time.
             onDone({});
             return;
         }
+    }
 
-    NetworkRequest(AlejoApi::API_URL + AlejoApi::API_USERS + "/" + username)
+    qCDebug(LOG) << "Fetching pronouns from alejo.io for" << username;
+
+    QString endpoint = API_URL % API_USERS_ENDPOINT % "/" % username;
+
+    NetworkRequest(endpoint)
         .concurrent()
         .onSuccess([this, username, onDone](const auto &result) {
             auto object = result.parseJson();
-            auto parsed = this->parse(object);
+            auto parsed = this->parsePronoun(object);
             onDone({parsed});
         })
         .onError([onDone, username](auto result) {
@@ -113,12 +62,90 @@ void AlejoApi::fetch(const QString &username,
                 onDone({UserPronouns()});
                 return;
             }
-            qCWarning(chatterinoPronouns)
-                << "alejo.io returned " << status.value_or(-1)
+            qCWarning(LOG) << "alejo.io returned " << status.value_or(-1)
                            << " when fetching pronouns for " << username;
             onDone({});
         })
         .execute();
 }
 
+void AlejoApi::loadAvailablePronouns()
+{
+    qCDebug(LOG) << "Fetching available pronouns for alejo.io";
+
+    QString endpoint = API_URL % API_PRONOUNS_ENDPOINT;
+
+    NetworkRequest(endpoint)
+        .concurrent()
+        .onSuccess([this](const auto &result) {
+            auto root = result.parseJson();
+            if (root.isEmpty())
+            {
+                return;
+            }
+
+            std::unordered_map<QString, QString> newPronouns;
+
+            for (auto it = root.begin(); it != root.end(); ++it)
+            {
+                const auto &pronounId = it.key();
+                const auto &pronounObj = it.value().toObject();
+
+                const auto &subject = pronounObj["subject"].toString();
+                const auto &object = pronounObj["object"].toString();
+                const auto &singular = pronounObj["singular"].toBool();
+
+                if (subject.isEmpty() || object.isEmpty())
+                {
+                    qCWarning(LOG) << "Pronoun" << pronounId
+                                   << "was malformed:" << pronounObj;
+                    continue;
+                }
+
+                if (singular)
+                {
+                    newPronouns[pronounId] = subject;
+                }
+                else
+                {
+                    newPronouns[pronounId] = subject % "/" % object;
+                }
+            }
+
+            {
+                std::unique_lock lock(this->mutex);
+                this->pronouns = newPronouns;
+            }
+        })
+        .onError([](const NetworkResult &result) {
+            qCWarning(LOG) << "Failed to load pronouns from alejo.io"
+                           << result.formatError();
+        })
+        .execute();
+}
+
+UserPronouns AlejoApi::parsePronoun(const QJsonObject &object)
+{
+    if (this->pronouns.empty())
+    {
+        return {};
+    }
+
+    const auto &pronoun = object["pronoun_id"];
+
+    if (!pronoun.isString())
+    {
+        return {};
+    }
+
+    auto pronounStr = pronoun.toString();
+    std::shared_lock lock(this->mutex);
+    auto iter = this->pronouns.find(pronounStr);
+    if (iter != this->pronouns.end())
+    {
+        return {iter->second};
+    }
+    return {};
+}
+
 }  // namespace chatterino::pronouns

src/providers/pronouns/alejo/PronounsAlejoApi.hpp

@@ -5,31 +5,35 @@
 #include <QJsonObject>
 #include <QString>
 
-#include <mutex>
+#include <functional>
 #include <optional>
 #include <shared_mutex>
+#include <unordered_map>
 
 namespace chatterino::pronouns {
 
 class AlejoApi
 {
 public:
-    explicit AlejoApi();
-    /** Fetches pronouns from the alejo.io API for a username and calls onDone when done.
-        onDone can be invoked from any thread. The argument is std::nullopt if and only if the request failed.
-     */
+    AlejoApi();
+
+    /// Fetch the user's pronouns from the alejo.io API
+    ///
+    /// onDone can be invoked from any thread
+    ///
+    /// The argument is std::nullopt if and only if the request failed.
     void fetch(const QString &username,
-               std::function<void(std::optional<UserPronouns>)> onDone);
+               const std::function<void(std::optional<UserPronouns>)> &onDone);
 
 private:
+    void loadAvailablePronouns();
+
     std::shared_mutex mutex;
-    /** A map from alejo.io ids to human readable representation like theythem -> they/them, other -> other. */
-    std::optional<std::unordered_map<QString, QString>> pronounsFromId =
-        std::nullopt;
-    UserPronouns parse(const QJsonObject &object);
-    inline static const QString API_URL = "https://api.pronouns.alejo.io/v1";
-    inline static const QString API_USERS = "/users";
-    inline static const QString API_PRONOUNS = "/pronouns";
+    /// Maps alejo.io pronoun IDs to human readable representation like `they/them` or `other`
+    std::unordered_map<QString, QString> pronouns;
+
+    /// Parse a pronoun definition from the /users endpoint into a finished UserPronouns
+    UserPronouns parsePronoun(const QJsonObject &object);
 };
 
 }  // namespace chatterino::pronouns

src/widgets/dialogs/UserInfoPopup.cpp

@@ -962,19 +962,19 @@ void UserInfoPopup::updateUserData()
         // get pronouns
         if (getSettings()->showPronouns)
         {
-            getApp()->getPronouns()->fetch(
+            getApp()->getPronouns()->getUserPronoun(
                 user.login,
-                [this, hack](const auto pronouns) {
+                [this, hack](const auto userPronoun) {
                     runInGuiThread([this, hack,
-                                    pronouns = std::move(pronouns)]() {
+                                    userPronoun = std::move(userPronoun)]() {
                         if (!hack.lock() || this->ui_.pronounsLabel == nullptr)
                         {
                             return;
                         }
-                        if (!pronouns.isUnspecified())
+                        if (!userPronoun.isUnspecified())
                         {
                             this->ui_.pronounsLabel->setText(
-                                TEXT_PRONOUNS.arg(pronouns.format()));
+                                TEXT_PRONOUNS.arg(userPronoun.format()));
                         }
                         else
                         {