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 | 56 +++--- src/providers/pronouns/Pronouns.hpp | 15 +- .../pronouns/alejo/PronounsAlejoApi.cpp | 177 ++++++++++-------- .../pronouns/alejo/PronounsAlejoApi.hpp | 30 +-- src/widgets/dialogs/UserInfoPopup.cpp | 10 +- 9 files changed, 164 insertions(+), 137 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4f6391de73..07bec5f95d4 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 994c8885857..24ebd127f82 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 @@ 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 tryReadJsonFile(const QString &path) diff --git a/src/Application.cpp b/src/Application.cpp index 7d26285f740..d781dbac7de 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 92f0dd7526f..2f6b11a3f33 100644 --- a/src/Application.hpp +++ b/src/Application.hpp @@ -173,7 +173,7 @@ class Application : public IApplication 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 8b37303dc19..b7e68fdb113 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()) + 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 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 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 adf2439e2eb..0f27a20cace 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); + void getUserPronoun( + const QString &username, + const std::function &callbackSuccess, + const std::function &callbackFail); +private: // Retrieve cached pronouns for user. - std::optional getForUsername(const QString &username); + std::optional getCachedUserPronoun(const QString &username); -private: // 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 31e606016e0..cdf8eaedaae 100644 --- a/src/providers/pronouns/alejo/PronounsAlejoApi.cpp +++ b/src/providers/pronouns/alejo/PronounsAlejoApi.cpp @@ -5,120 +5,147 @@ #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) +AlejoApi::AlejoApi() +{ + this->loadAvailablePronouns(); +} + +void AlejoApi::fetch( + const QString &username, + const std::function)> &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()}; - for (auto const &pronounId : object.keys()) + std::unordered_map 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)> 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 diff --git a/src/providers/pronouns/alejo/PronounsAlejoApi.hpp b/src/providers/pronouns/alejo/PronounsAlejoApi.hpp index a6e82c9bfac..267c7278d5c 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 6aac79d03b3..fabae271d11 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 {