From 336536c7614a6c3c7935ec9e016ab153d9796151 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sun, 8 Sep 2024 13:30:06 +0200 Subject: [PATCH] chore: clean up some of the pronoun implementation (#5583) --- CHANGELOG.md | 2 +- benchmarks/src/RecentMessages.cpp | 7 - src/Application.cpp | 2 +- src/Application.hpp | 2 +- src/providers/pronouns/Pronouns.cpp | 64 +++--- src/providers/pronouns/Pronouns.hpp | 17 +- .../pronouns/alejo/PronounsAlejoApi.cpp | 199 ++++++++++-------- .../pronouns/alejo/PronounsAlejoApi.hpp | 30 +-- src/widgets/dialogs/UserInfoPopup.cpp | 10 +- 9 files changed, 180 insertions(+), 153 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4f6391de..07bec5f95 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/benchmarks/src/RecentMessages.cpp b/benchmarks/src/RecentMessages.cpp index 994c88858..24ebd127f 100644 --- a/benchmarks/src/RecentMessages.cpp +++ b/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 tryReadJsonFile(const QString &path) diff --git a/src/Application.cpp b/src/Application.cpp index 7d26285f7..d781dbac7 100644 --- a/src/Application.cpp +++ b/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(new pronouns::Pronouns) #ifdef CHATTERINO_HAVE_PLUGINS , plugins(new PluginController(paths)) #endif diff --git a/src/Application.hpp b/src/Application.hpp index 92f0dd752..2f6b11a3f 100644 --- a/src/Application.hpp +++ b/src/Application.hpp @@ -173,7 +173,7 @@ private: std::unique_ptr linkResolver; std::unique_ptr streamerMode; std::unique_ptr twitchUsers; - std::shared_ptr pronouns; + std::unique_ptr pronouns; #ifdef CHATTERINO_HAVE_PLUGINS std::unique_ptr plugins; #endif diff --git a/src/providers/pronouns/Pronouns.cpp b/src/providers/pronouns/Pronouns.cpp index 8b37303dc..b7e68fdb1 100644 --- a/src/providers/pronouns/Pronouns.cpp +++ b/src/providers/pronouns/Pronouns.cpp @@ -6,51 +6,53 @@ #include "providers/pronouns/UserPronouns.hpp" #include -#include #include +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 &callbackSuccess, - const std::function &callbackFail) +void Pronouns::getUserPronoun( + const QString &username, + const std::function &callbackSuccess, + const std::function &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()) - { - callbackSuccess(iter->second); - return; - } - } // unlock mutex - - qCDebug(chatterinoPronouns) - << "Fetching pronouns from alejo.io for " << username; - - alejoApi.fetch(username, [this, callbackSuccess, callbackFail, - username](std::optional 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 Pronouns::getForUsername(const QString &username) +std::optional Pronouns::getCachedUserPronoun( + const QString &username) { std::shared_lock lock(this->mutex); auto it = this->saved.find(username); diff --git a/src/providers/pronouns/Pronouns.hpp b/src/providers/pronouns/Pronouns.hpp index adf2439e2..0f27a20ca 100644 --- a/src/providers/pronouns/Pronouns.hpp +++ b/src/providers/pronouns/Pronouns.hpp @@ -3,6 +3,7 @@ #include "providers/pronouns/alejo/PronounsAlejoApi.hpp" #include "providers/pronouns/UserPronouns.hpp" +#include #include #include #include @@ -12,20 +13,20 @@ namespace chatterino::pronouns { class Pronouns { public: - Pronouns() = default; - - void fetch(const QString &username, - const std::function &callbackSuccess, - const std::function &callbackFail); - - // Retrieve cached pronouns for user. - std::optional getForUsername(const QString &username); + void getUserPronoun( + const QString &username, + const std::function &callbackSuccess, + const std::function &callbackFail); private: + // Retrieve cached pronouns for user. + std::optional getCachedUserPronoun(const QString &username); + // mutex for editing the saved map. std::shared_mutex mutex; // Login name -> Pronouns std::unordered_map saved; AlejoApi alejoApi; }; + } // namespace chatterino::pronouns diff --git a/src/providers/pronouns/alejo/PronounsAlejoApi.cpp b/src/providers/pronouns/alejo/PronounsAlejoApi.cpp index 31e606016..cdf8eaeda 100644 --- a/src/providers/pronouns/alejo/PronounsAlejoApi.cpp +++ b/src/providers/pronouns/alejo/PronounsAlejoApi.cpp @@ -5,103 +5,52 @@ #include "common/QLogging.hpp" #include "providers/pronouns/UserPronouns.hpp" +#include + +#include +#include + +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; - } - - 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()) - { - return; - } - - std::unique_lock lock(this->mutex); - this->pronounsFromId = {std::unordered_map()}; - 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(); + this->loadAvailablePronouns(); } -void AlejoApi::fetch(const QString &username, - std::function)> onDone) +void AlejoApi::fetch( + const QString &username, + const std::function)> &onDone) { - bool havePronounList{true}; { std::shared_lock lock(this->mutex); - havePronounList = this->pronounsFromId.has_value(); - } // unlock mutex - - if (!havePronounList) - { - // Pronoun list not available yet, just fail and try again next time. - onDone({}); - return; + if (this->pronouns.empty()) + { + // 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) - << " when fetching pronouns for " << username; + 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 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 diff --git a/src/providers/pronouns/alejo/PronounsAlejoApi.hpp b/src/providers/pronouns/alejo/PronounsAlejoApi.hpp index a6e82c9bf..267c7278d 100644 --- a/src/providers/pronouns/alejo/PronounsAlejoApi.hpp +++ b/src/providers/pronouns/alejo/PronounsAlejoApi.hpp @@ -5,31 +5,35 @@ #include #include -#include +#include #include #include +#include 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)> onDone); + const std::function)> &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> 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 pronouns; + + /// Parse a pronoun definition from the /users endpoint into a finished UserPronouns + UserPronouns parsePronoun(const QJsonObject &object); }; } // namespace chatterino::pronouns diff --git a/src/widgets/dialogs/UserInfoPopup.cpp b/src/widgets/dialogs/UserInfoPopup.cpp index 6aac79d03..fabae271d 100644 --- a/src/widgets/dialogs/UserInfoPopup.cpp +++ b/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 {