diff --git a/CHANGELOG.md b/CHANGELOG.md index d6070479d..e80658bdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Minor: Add accelerators to the right click menu for messages (#4705) - Minor: Add pin action to usercards and reply threads. (#4692) - Minor: Stream status requests are now batched. (#4713) +- Bugfix: Increased amount of blocked users loaded from 100 to 1,000. (#4721) - Bugfix: Fixed generation of crashdumps by the browser-extension process when the browser was closed. (#4667) - Bugfix: Fix spacing issue with mentions inside RTL text. (#4677) - Bugfix: Fixed a crash when opening and closing a reply thread and switching the user. (#4675) diff --git a/mocks/include/mocks/Helix.hpp b/mocks/include/mocks/Helix.hpp index 2d4d01aad..3b01c5b0d 100644 --- a/mocks/include/mocks/Helix.hpp +++ b/mocks/include/mocks/Helix.hpp @@ -1,6 +1,7 @@ #pragma once #include "providers/twitch/api/Helix.hpp" +#include "util/CancellationToken.hpp" #include #include @@ -107,7 +108,8 @@ public: MOCK_METHOD(void, loadBlocks, (QString userId, ResultCallback> successCallback, - HelixFailureCallback failureCallback), + FailureCallback failureCallback, + CancellationToken &&token), (override)); MOCK_METHOD(void, blockUser, diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4e30d67fd..89b05e429 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -385,6 +385,7 @@ set(SOURCE_FILES util/AttachToConsole.cpp util/AttachToConsole.hpp + util/CancellationToken.hpp util/Clipboard.cpp util/Clipboard.hpp util/ConcurrentMap.hpp diff --git a/src/controllers/ignores/IgnoreController.cpp b/src/controllers/ignores/IgnoreController.cpp index e3df12e47..6f3876401 100644 --- a/src/controllers/ignores/IgnoreController.cpp +++ b/src/controllers/ignores/IgnoreController.cpp @@ -32,10 +32,10 @@ bool isIgnoredMessage(IgnoredMessageParameters &¶ms) { auto sourceUserID = params.twitchUserID; - auto blocks = - getApp()->accounts->twitch.getCurrent()->accessBlockedUserIds(); - - if (auto it = blocks->find(sourceUserID); it != blocks->end()) + bool isBlocked = + getApp()->accounts->twitch.getCurrent()->blockedUserIds().contains( + sourceUserID); + if (isBlocked) { switch (static_cast( getSettings()->showBlockedUsersMessages.getValue())) diff --git a/src/providers/twitch/TwitchAccount.cpp b/src/providers/twitch/TwitchAccount.cpp index 0830fa15b..2ff1fe6fe 100644 --- a/src/providers/twitch/TwitchAccount.cpp +++ b/src/providers/twitch/TwitchAccount.cpp @@ -7,14 +7,15 @@ #include "common/Outcome.hpp" #include "common/QLogging.hpp" #include "controllers/accounts/AccountController.hpp" +#include "debug/AssertInGuiThread.hpp" #include "messages/Message.hpp" #include "messages/MessageBuilder.hpp" #include "providers/irc/IrcMessageBuilder.hpp" #include "providers/IvrApi.hpp" #include "providers/twitch/api/Helix.hpp" #include "providers/twitch/TwitchCommon.hpp" -#include "providers/twitch/TwitchUser.hpp" #include "singletons/Emotes.hpp" +#include "util/CancellationToken.hpp" #include "util/Helpers.hpp" #include "util/QStringHash.hpp" #include "util/RapidjsonHelpers.hpp" @@ -100,79 +101,79 @@ bool TwitchAccount::isAnon() const void TwitchAccount::loadBlocks() { + assertInGuiThread(); + + auto token = CancellationToken(false); + this->blockToken_ = token; + this->ignores_.clear(); + this->ignoresUserIds_.clear(); + getHelix()->loadBlocks( getIApp()->getAccounts()->twitch.getCurrent()->userId_, - [this](std::vector blocks) { - auto ignores = this->ignores_.access(); - auto userIds = this->ignoresUserIds_.access(); - ignores->clear(); - userIds->clear(); + [this](const std::vector &blocks) { + assertInGuiThread(); for (const HelixBlock &block : blocks) { TwitchUser blockedUser; blockedUser.fromHelixBlock(block); - ignores->insert(blockedUser); - userIds->insert(blockedUser.id); + this->ignores_.insert(blockedUser); + this->ignoresUserIds_.insert(blockedUser.id); } }, - [] { - qCWarning(chatterinoTwitch) << "Fetching blocks failed!"; - }); + [](auto error) { + qCWarning(chatterinoTwitch).noquote() + << "Fetching blocks failed:" << error; + }, + std::move(token)); } -void TwitchAccount::blockUser(QString userId, const QObject *caller, +void TwitchAccount::blockUser(const QString &userId, const QObject *caller, std::function onSuccess, std::function onFailure) { getHelix()->blockUser( userId, caller, - [this, userId, onSuccess] { + [this, userId, onSuccess = std::move(onSuccess)] { + assertInGuiThread(); + TwitchUser blockedUser; blockedUser.id = userId; - { - auto ignores = this->ignores_.access(); - auto userIds = this->ignoresUserIds_.access(); - - ignores->insert(blockedUser); - userIds->insert(blockedUser.id); - } + this->ignores_.insert(blockedUser); + this->ignoresUserIds_.insert(blockedUser.id); onSuccess(); }, std::move(onFailure)); } -void TwitchAccount::unblockUser(QString userId, const QObject *caller, +void TwitchAccount::unblockUser(const QString &userId, const QObject *caller, std::function onSuccess, std::function onFailure) { getHelix()->unblockUser( userId, caller, - [this, userId, onSuccess] { + [this, userId, onSuccess = std::move(onSuccess)] { + assertInGuiThread(); + TwitchUser ignoredUser; ignoredUser.id = userId; - { - auto ignores = this->ignores_.access(); - auto userIds = this->ignoresUserIds_.access(); - - ignores->erase(ignoredUser); - userIds->erase(ignoredUser.id); - } + this->ignores_.erase(ignoredUser); + this->ignoresUserIds_.erase(ignoredUser.id); onSuccess(); }, std::move(onFailure)); } -SharedAccessGuard> TwitchAccount::accessBlocks() - const +const std::unordered_set &TwitchAccount::blocks() const { - return this->ignores_.accessConst(); + assertInGuiThread(); + return this->ignores_; } -SharedAccessGuard> TwitchAccount::accessBlockedUserIds() - const +const std::unordered_set &TwitchAccount::blockedUserIds() const { - return this->ignoresUserIds_.accessConst(); + assertInGuiThread(); + return this->ignoresUserIds_; } void TwitchAccount::loadEmotes(std::weak_ptr weakChannel) diff --git a/src/providers/twitch/TwitchAccount.hpp b/src/providers/twitch/TwitchAccount.hpp index 154d307f5..f92a44279 100644 --- a/src/providers/twitch/TwitchAccount.hpp +++ b/src/providers/twitch/TwitchAccount.hpp @@ -5,6 +5,8 @@ #include "common/UniqueAccess.hpp" #include "controllers/accounts/Account.hpp" #include "messages/Emote.hpp" +#include "providers/twitch/TwitchUser.hpp" +#include "util/CancellationToken.hpp" #include "util/QStringHash.hpp" #include @@ -15,12 +17,11 @@ #include #include -#include +#include #include namespace chatterino { -struct TwitchUser; class Channel; using ChannelPtr = std::shared_ptr; @@ -72,15 +73,15 @@ public: bool isAnon() const; void loadBlocks(); - void blockUser(QString userId, const QObject *caller, + void blockUser(const QString &userId, const QObject *caller, std::function onSuccess, std::function onFailure); - void unblockUser(QString userId, const QObject *caller, + void unblockUser(const QString &userId, const QObject *caller, std::function onSuccess, std::function onFailure); - SharedAccessGuard> accessBlockedUserIds() const; - SharedAccessGuard> accessBlocks() const; + [[nodiscard]] const std::unordered_set &blocks() const; + [[nodiscard]] const std::unordered_set &blockedUserIds() const; void loadEmotes(std::weak_ptr weakChannel = {}); // loadUserstateEmotes loads emote sets that are part of the USERSTATE emote-sets key @@ -105,10 +106,11 @@ private: const bool isAnon_; Atomic color_; - mutable std::mutex ignoresMutex_; QStringList userstateEmoteSets_; - UniqueAccess> ignores_; - UniqueAccess> ignoresUserIds_; + + ScopedCancellationToken blockToken_; + std::unordered_set ignores_; + std::unordered_set ignoresUserIds_; // std::map emotes; UniqueAccess emotes_; diff --git a/src/providers/twitch/TwitchUser.hpp b/src/providers/twitch/TwitchUser.hpp index 7593abd3c..d615c95b0 100644 --- a/src/providers/twitch/TwitchUser.hpp +++ b/src/providers/twitch/TwitchUser.hpp @@ -1,5 +1,6 @@ #pragma once +#include "util/QStringHash.hpp" #include "util/RapidjsonHelpers.hpp" #include @@ -31,6 +32,16 @@ struct TwitchUser { { return this->id < rhs.id; } + + bool operator==(const TwitchUser &rhs) const + { + return this->id == rhs.id; + } + + bool operator!=(const TwitchUser &rhs) const + { + return !(*this == rhs); + } }; } // namespace chatterino @@ -75,3 +86,11 @@ struct Deserialize { }; } // namespace pajlada + +template <> +struct std::hash { + inline size_t operator()(const chatterino::TwitchUser &user) const noexcept + { + return std::hash{}(user.id); + } +}; diff --git a/src/providers/twitch/api/Helix.cpp b/src/providers/twitch/api/Helix.cpp index 3c23fcd7f..fac3b886c 100644 --- a/src/providers/twitch/api/Helix.cpp +++ b/src/providers/twitch/api/Helix.cpp @@ -1,9 +1,11 @@ #include "providers/twitch/api/Helix.hpp" +#include "common/Literals.hpp" #include "common/NetworkRequest.hpp" #include "common/NetworkResult.hpp" #include "common/Outcome.hpp" #include "common/QLogging.hpp" +#include "util/CancellationToken.hpp" #include #include @@ -20,6 +22,8 @@ static constexpr auto NUM_CHATTERS_TO_FETCH = 1000; namespace chatterino { +using namespace literals; + static IHelix *instance = nullptr; HelixChatters::HelixChatters(const QJsonObject &jsonObject) @@ -544,40 +548,53 @@ void Helix::createStreamMarker( }; void Helix::loadBlocks(QString userId, - ResultCallback> successCallback, - HelixFailureCallback failureCallback) + ResultCallback> pageCallback, + FailureCallback failureCallback, + CancellationToken &&token) { - QUrlQuery urlQuery; - urlQuery.addQueryItem("broadcaster_id", userId); - urlQuery.addQueryItem("first", "100"); + constexpr const size_t blockLimit = 1000; - this->makeGet("users/blocks", urlQuery) - .onSuccess([successCallback, failureCallback](auto result) -> Outcome { - auto root = result.parseJson(); - auto data = root.value("data"); + // TODO(Qt 5.13): use initializer list + QUrlQuery query; + query.addQueryItem(u"broadcaster_id"_s, userId); + query.addQueryItem(u"first"_s, u"100"_s); - if (!data.isArray()) + size_t receivedItems = 0; + this->paginate( + u"users/blocks"_s, query, + [pageCallback, receivedItems](const QJsonObject &json) mutable { + const auto data = json["data"_L1].toArray(); + + if (data.isEmpty()) { - failureCallback(); - return Failure; + return false; } std::vector ignores; + ignores.reserve(data.count()); - for (const auto &jsonStream : data.toArray()) + for (const auto &ignore : data) { - ignores.emplace_back(jsonStream.toObject()); + ignores.emplace_back(ignore.toObject()); } - successCallback(ignores); + pageCallback(ignores); - return Success; - }) - .onError([failureCallback](auto /*result*/) { - // TODO: make better xd - failureCallback(); - }) - .execute(); + receivedItems += data.count(); + + if (receivedItems >= blockLimit) + { + qCInfo(chatterinoTwitch) << "Reached the limit of" << blockLimit + << "Twitch blocks fetched"; + return false; + } + + return true; + }, + [failureCallback](const NetworkResult &result) { + failureCallback(result.formatError()); + }, + std::move(token)); } void Helix::blockUser(QString targetUserId, const QObject *caller, @@ -2931,6 +2948,59 @@ NetworkRequest Helix::makePatch(const QString &url, const QUrlQuery &urlQuery) return this->makeRequest(url, urlQuery, NetworkRequestType::Patch); } +void Helix::paginate(const QString &url, const QUrlQuery &baseQuery, + std::function onPage, + std::function onError, + CancellationToken &&cancellationToken) +{ + auto onSuccess = + std::make_shared>(nullptr); + // This is the actual callback passed to NetworkRequest. + // It wraps the shared-ptr. + auto onSuccessCb = [onSuccess](const auto &res) -> Outcome { + return (*onSuccess)(res); + }; + + *onSuccess = [this, onPage = std::move(onPage), onError, onSuccessCb, + url{url}, baseQuery{baseQuery}, + cancellationToken = std::move(cancellationToken)]( + const NetworkResult &res) -> Outcome { + if (cancellationToken.isCancelled()) + { + return Success; + } + + const auto json = res.parseJson(); + if (!onPage(json)) + { + // The consumer doesn't want any more pages + return Success; + } + + auto cursor = json["pagination"_L1]["cursor"_L1].toString(); + if (cursor.isEmpty()) + { + return Success; + } + + auto query = baseQuery; + query.removeAllQueryItems(u"after"_s); + query.addQueryItem(u"after"_s, cursor); + + this->makeGet(url, query) + .onSuccess(onSuccessCb) + .onError(onError) + .execute(); + + return Success; + }; + + this->makeGet(url, baseQuery) + .onSuccess(std::move(onSuccessCb)) + .onError(std::move(onError)) + .execute(); +} + void Helix::update(QString clientId, QString oauthToken) { this->clientId = std::move(clientId); diff --git a/src/providers/twitch/api/Helix.hpp b/src/providers/twitch/api/Helix.hpp index 6b3230fa8..dc8593815 100644 --- a/src/providers/twitch/api/Helix.hpp +++ b/src/providers/twitch/api/Helix.hpp @@ -24,6 +24,8 @@ using HelixFailureCallback = std::function; template using ResultCallback = std::function; +class CancellationToken; + struct HelixUser { QString id; QString login; @@ -807,8 +809,9 @@ public: // https://dev.twitch.tv/docs/api/reference#get-user-block-list virtual void loadBlocks( - QString userId, ResultCallback> successCallback, - HelixFailureCallback failureCallback) = 0; + QString userId, ResultCallback> pageCallback, + FailureCallback failureCallback, + CancellationToken &&token) = 0; // https://dev.twitch.tv/docs/api/reference#block-user virtual void blockUser(QString targetUserId, const QObject *caller, @@ -1126,8 +1129,9 @@ public: // https://dev.twitch.tv/docs/api/reference#get-user-block-list void loadBlocks(QString userId, - ResultCallback> successCallback, - HelixFailureCallback failureCallback) final; + ResultCallback> pageCallback, + FailureCallback failureCallback, + CancellationToken &&token) final; // https://dev.twitch.tv/docs/api/reference#block-user void blockUser(QString targetUserId, const QObject *caller, @@ -1406,6 +1410,13 @@ private: NetworkRequest makePut(const QString &url, const QUrlQuery &urlQuery); NetworkRequest makePatch(const QString &url, const QUrlQuery &urlQuery); + /// Paginate the `url` endpoint and use `baseQuery` as the starting point for pagination. + /// @param onPage returns true while a new page is expected. Once false is returned, pagination will stop. + void paginate(const QString &url, const QUrlQuery &baseQuery, + std::function onPage, + std::function onError, + CancellationToken &&token); + QString clientId; QString oauthToken; }; diff --git a/src/util/CancellationToken.hpp b/src/util/CancellationToken.hpp new file mode 100644 index 000000000..12c26f6b1 --- /dev/null +++ b/src/util/CancellationToken.hpp @@ -0,0 +1,82 @@ +#pragma once + +#include +#include + +namespace chatterino { + +/// The CancellationToken is a thread-safe way for worker(s) +/// to know if the task they want to continue doing should be cancelled. +class CancellationToken +{ +public: + CancellationToken() = default; + explicit CancellationToken(bool isCancelled) + : isCancelled_(new std::atomic(isCancelled)) + { + } + + CancellationToken(const CancellationToken &) = default; + CancellationToken(CancellationToken &&other) + : isCancelled_(std::move(other.isCancelled_)){}; + + CancellationToken &operator=(CancellationToken &&other) + { + this->isCancelled_ = std::move(other.isCancelled_); + return *this; + } + CancellationToken &operator=(const CancellationToken &) = default; + + void cancel() + { + if (this->isCancelled_ != nullptr) + { + this->isCancelled_->store(true, std::memory_order_release); + } + } + + bool isCancelled() const + { + return this->isCancelled_ == nullptr || + this->isCancelled_->load(std::memory_order_acquire); + } + +private: + std::shared_ptr> isCancelled_; +}; + +/// The ScopedCancellationToken is a way to automatically cancel a CancellationToken when it goes out of scope +class ScopedCancellationToken +{ +public: + ScopedCancellationToken() = default; + ScopedCancellationToken(CancellationToken &&backingToken) + : backingToken_(std::move(backingToken)) + { + } + ScopedCancellationToken(CancellationToken backingToken) + : backingToken_(std::move(backingToken)) + { + } + + ~ScopedCancellationToken() + { + this->backingToken_.cancel(); + } + + ScopedCancellationToken(const ScopedCancellationToken &) = delete; + ScopedCancellationToken(ScopedCancellationToken &&other) + : backingToken_(std::move(other.backingToken_)){}; + ScopedCancellationToken &operator=(ScopedCancellationToken &&other) + { + this->backingToken_ = std::move(other.backingToken_); + return *this; + } + ScopedCancellationToken &operator=(const ScopedCancellationToken &) = + delete; + +private: + CancellationToken backingToken_; +}; + +} // namespace chatterino diff --git a/src/widgets/dialogs/UserInfoPopup.cpp b/src/widgets/dialogs/UserInfoPopup.cpp index ab37c80c2..2c0fa10fd 100644 --- a/src/widgets/dialogs/UserInfoPopup.cpp +++ b/src/widgets/dialogs/UserInfoPopup.cpp @@ -836,13 +836,7 @@ void UserInfoPopup::updateUserData() }); // get ignore state - bool isIgnoring = false; - - if (auto blocks = currentUser->accessBlockedUserIds(); - blocks->find(user.id) != blocks->end()) - { - isIgnoring = true; - } + bool isIgnoring = currentUser->blockedUserIds().contains(user.id); // get ignoreHighlights state bool isIgnoringHighlights = false; diff --git a/src/widgets/settingspages/IgnoresPage.cpp b/src/widgets/settingspages/IgnoresPage.cpp index 9852a2f9c..d537265fe 100644 --- a/src/widgets/settingspages/IgnoresPage.cpp +++ b/src/widgets/settingspages/IgnoresPage.cpp @@ -126,10 +126,9 @@ void IgnoresPage::onShow() } QStringList users; + users.reserve(user->blocks().size()); - auto blocks = app->accounts->twitch.getCurrent()->accessBlocks(); - - for (const auto &blockedUser : *blocks) + for (const auto &blockedUser : user->blocks()) { users << blockedUser.name; }