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 @@ class MockApplication : public mock::BaseApplication
         return &this->linkResolver;
     }
 
-    pronouns::Pronouns *getPronouns() override
-    {
-        return &this->pronouns;
-    }
-
     AccountController accounts;
     Emotes emotes;
     mock::UserDataController userData;
@@ -130,7 +124,6 @@ class MockApplication : public mock::BaseApplication
     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 @@ class Application : public IApplication
     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,
-                     const std::function<void(UserPronouns)> &callbackSuccess,
-                     const std::function<void()> &callbackFail)
+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);
+        callbackSuccess(*cachedPronoun);
+        return;
+    }
 
-        auto iter = this->saved.find(username);
-        if (iter != this->saved.end())
+    this->alejoApi.fetch(username, [this, callbackSuccess, callbackFail,
+                                    username](const auto &oUserPronoun) {
+        if (!oUserPronoun.has_value())
         {
-            callbackSuccess(iter->second);
+            callbackFail();
             return;
         }
-    }  // unlock mutex
 
-    qCDebug(chatterinoPronouns)
-        << "Fetching pronouns from alejo.io for " << username;
+        const auto &userPronoun = *oUserPronoun;
 
-    alejoApi.fetch(username, [this, callbackSuccess, callbackFail,
-                              username](std::optional<UserPronouns> result) {
-        if (result.has_value())
+        qCDebug(LOG) << "Caching pronoun" << userPronoun.format() << "for user"
+                     << username;
         {
-            {
-                std::unique_lock lock(this->mutex);
-                this->saved[username] = *result;
-            }  // unlock mutex
-            qCDebug(chatterinoPronouns)
-                << "Adding pronouns " << result->format() << " for user "
-                << username;
-            callbackSuccess(*result);
-        }
-        else
-        {
-            callbackFail();
+            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,
-               const std::function<void(UserPronouns)> &callbackSuccess,
-               const std::function<void()> &callbackFail);
+    void getUserPronoun(
+        const QString &username,
+        const std::function<void(UserPronouns)> &callbackSuccess,
+        const std::function<void()> &callbackFail);
 
+private:
     // Retrieve cached pronouns for user.
-    std::optional<UserPronouns> getForUsername(const QString &username);
+    std::optional<UserPronouns> getCachedUserPronoun(const QString &username);
 
-private:
     // 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,120 +5,147 @@
 #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)
+AlejoApi::AlejoApi()
+{
+    this->loadAvailablePronouns();
+}
+
+void AlejoApi::fetch(
+    const QString &username,
+    const std::function<void(std::optional<UserPronouns>)> &onDone)
 {
-    if (!this->pronounsFromId.has_value())
     {
-        return {};
+        std::shared_lock lock(this->mutex);
+        if (this->pronouns.empty())
+        {
+            // Pronoun list not available yet, fail and try again next time.
+            onDone({});
+            return;
+        }
     }
 
-    auto pronoun = object["pronoun_id"];
+    qCDebug(LOG) << "Fetching pronouns from alejo.io for" << username;
 
-    if (!pronoun.isString())
-    {
-        return {};
-    }
+    QString endpoint = API_URL % API_USERS_ENDPOINT % "/" % username;
 
-    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 {};
+    NetworkRequest(endpoint)
+        .concurrent()
+        .onSuccess([this, username, onDone](const auto &result) {
+            auto object = result.parseJson();
+            auto parsed = this->parsePronoun(object);
+            onDone({parsed});
+        })
+        .onError([onDone, username](auto result) {
+            auto status = result.status();
+            if (status.has_value() && status == 404)
+            {
+                // Alejo returns 404 if the user has no pronouns set.
+                // Return unspecified.
+                onDone({UserPronouns()});
+                return;
+            }
+            qCWarning(LOG) << "alejo.io returned " << status.value_or(-1)
+                           << " when fetching pronouns for " << username;
+            onDone({});
+        })
+        .execute();
 }
 
-AlejoApi::AlejoApi()
+void AlejoApi::loadAvailablePronouns()
 {
-    std::shared_lock lock(this->mutex);
-    if (this->pronounsFromId)
-    {
-        return;
-    }
+    qCDebug(LOG) << "Fetching available pronouns for alejo.io";
+
+    QString endpoint = API_URL % API_PRONOUNS_ENDPOINT;
 
-    qCDebug(chatterinoPronouns) << "Fetching available pronouns for alejo.io";
-    NetworkRequest(AlejoApi::API_URL + AlejoApi::API_PRONOUNS)
+    NetworkRequest(endpoint)
         .concurrent()
         .onSuccess([this](const auto &result) {
-            auto object = result.parseJson();
-            if (object.isEmpty())
+            auto root = result.parseJson();
+            if (root.isEmpty())
             {
                 return;
             }
 
-            std::unique_lock lock(this->mutex);
-            this->pronounsFromId = {std::unordered_map<QString, QString>()};
-            for (auto const &pronounId : object.keys())
+            std::unordered_map<QString, QString> newPronouns;
+
+            for (auto it = root.begin(); it != root.end(); ++it)
             {
-                if (!object[pronounId].isObject())
-                {
-                    continue;
-                };
+                const auto &pronounId = it.key();
+                const auto &pronounObj = it.value().toObject();
 
-                const auto pronounObj = object[pronounId].toObject();
+                const auto &subject = pronounObj["subject"].toString();
+                const auto &object = pronounObj["object"].toString();
+                const auto &singular = pronounObj["singular"].toBool();
 
-                if (!pronounObj["subject"].isString())
+                if (subject.isEmpty() || object.isEmpty())
                 {
+                    qCWarning(LOG) << "Pronoun" << pronounId
+                                   << "was malformed:" << pronounObj;
                     continue;
                 }
 
-                QString pronouns = pronounObj["subject"].toString();
-
-                auto singular = pronounObj["singular"];
-                if (singular.isBool() && !singular.toBool() &&
-                    pronounObj["object"].isString())
+                if (singular)
                 {
-                    pronouns += "/" + pronounObj["object"].toString();
+                    newPronouns[pronounId] = subject;
                 }
+                else
+                {
+                    newPronouns[pronounId] = subject % "/" % object;
+                }
+            }
 
-                this->pronounsFromId->insert_or_assign(pronounId,
-                                                       pronouns.toLower());
+            {
+                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();
 }
 
-void AlejoApi::fetch(const QString &username,
-                     std::function<void(std::optional<UserPronouns>)> onDone)
+UserPronouns AlejoApi::parsePronoun(const QJsonObject &object)
 {
-    bool havePronounList{true};
+    if (this->pronouns.empty())
     {
-        std::shared_lock lock(this->mutex);
-        havePronounList = this->pronounsFromId.has_value();
-    }  // unlock mutex
+        return {};
+    }
 
-    if (!havePronounList)
+    const auto &pronoun = object["pronoun_id"];
+
+    if (!pronoun.isString())
     {
-        // Pronoun list not available yet, just fail and try again next time.
-        onDone({});
-        return;
+        return {};
     }
 
-    NetworkRequest(AlejoApi::API_URL + AlejoApi::API_USERS + "/" + username)
-        .concurrent()
-        .onSuccess([this, username, onDone](const auto &result) {
-            auto object = result.parseJson();
-            auto parsed = this->parse(object);
-            onDone({parsed});
-        })
-        .onError([onDone, username](auto result) {
-            auto status = result.status();
-            if (status.has_value() && status == 404)
-            {
-                // Alejo returns 404 if the user has no pronouns set.
-                // Return unspecified.
-                onDone({UserPronouns()});
-                return;
-            }
-            qCWarning(chatterinoPronouns)
-                << "alejo.io returned " << status.value_or(-1)
-                << " when fetching pronouns for " << username;
-            onDone({});
-        })
-        .execute();
+    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