Increase max number of blocked users loaded from 100 to 1,000 (#4721)

Also includes a little refactor of how the requests are made & how the blocked users are stored

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)

mocks/include/mocks/Helix.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "providers/twitch/api/Helix.hpp"
+#include "util/CancellationToken.hpp"
 
 #include <gmock/gmock.h>
 #include <QString>
@@ -107,7 +108,8 @@ public:
     MOCK_METHOD(void, loadBlocks,
                 (QString userId,
                  ResultCallback<std::vector<HelixBlock>> successCallback,
-                 HelixFailureCallback failureCallback),
+                 FailureCallback<QString> failureCallback,
+                 CancellationToken &&token),
                 (override));
 
     MOCK_METHOD(void, blockUser,

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

src/controllers/ignores/IgnoreController.cpp

@@ -32,10 +32,10 @@ bool isIgnoredMessage(IgnoredMessageParameters &&params)
     {
         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<ShowIgnoredUsersMessages>(
                 getSettings()->showBlockedUsersMessages.getValue()))

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<HelixBlock> blocks) {
-            auto ignores = this->ignores_.access();
-            auto userIds = this->ignoresUserIds_.access();
-            ignores->clear();
-            userIds->clear();
+        [this](const std::vector<HelixBlock> &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<void()> onSuccess,
                               std::function<void()> 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<void()> onSuccess,
                                 std::function<void()> 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<const std::set<TwitchUser>> TwitchAccount::accessBlocks()
-    const
+const std::unordered_set<TwitchUser> &TwitchAccount::blocks() const
 {
-    return this->ignores_.accessConst();
+    assertInGuiThread();
+    return this->ignores_;
 }
 
-SharedAccessGuard<const std::set<QString>> TwitchAccount::accessBlockedUserIds()
-    const
+const std::unordered_set<QString> &TwitchAccount::blockedUserIds() const
 {
-    return this->ignoresUserIds_.accessConst();
+    assertInGuiThread();
+    return this->ignoresUserIds_;
 }
 
 void TwitchAccount::loadEmotes(std::weak_ptr<Channel> weakChannel)

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 <QColor>
@@ -15,12 +17,11 @@
 
 #include <functional>
 #include <mutex>
-#include <set>
+#include <unordered_set>
 #include <vector>
 
 namespace chatterino {
 
-struct TwitchUser;
 class Channel;
 using ChannelPtr = std::shared_ptr<Channel>;
 
@@ -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<void()> onSuccess,
                    std::function<void()> onFailure);
-    void unblockUser(QString userId, const QObject *caller,
+    void unblockUser(const QString &userId, const QObject *caller,
                      std::function<void()> onSuccess,
                      std::function<void()> onFailure);
 
-    SharedAccessGuard<const std::set<QString>> accessBlockedUserIds() const;
-    SharedAccessGuard<const std::set<TwitchUser>> accessBlocks() const;
+    [[nodiscard]] const std::unordered_set<TwitchUser> &blocks() const;
+    [[nodiscard]] const std::unordered_set<QString> &blockedUserIds() const;
 
     void loadEmotes(std::weak_ptr<Channel> weakChannel = {});
     // loadUserstateEmotes loads emote sets that are part of the USERSTATE emote-sets key
@@ -105,10 +106,11 @@ private:
     const bool isAnon_;
     Atomic<QColor> color_;
 
-    mutable std::mutex ignoresMutex_;
     QStringList userstateEmoteSets_;
-    UniqueAccess<std::set<TwitchUser>> ignores_;
-    UniqueAccess<std::set<QString>> ignoresUserIds_;
+
+    ScopedCancellationToken blockToken_;
+    std::unordered_set<TwitchUser> ignores_;
+    std::unordered_set<QString> ignoresUserIds_;
 
     //    std::map<UserId, TwitchAccountEmoteData> emotes;
     UniqueAccess<TwitchAccountEmoteData> emotes_;

src/providers/twitch/TwitchUser.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "util/QStringHash.hpp"
 #include "util/RapidjsonHelpers.hpp"
 
 #include <pajlada/serialize.hpp>
@@ -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<chatterino::TwitchUser> {
 };
 
 }  // namespace pajlada
+
+template <>
+struct std::hash<chatterino::TwitchUser> {
+    inline size_t operator()(const chatterino::TwitchUser &user) const noexcept
+    {
+        return std::hash<QString>{}(user.id);
+    }
+};

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 <magic_enum.hpp>
 #include <QJsonDocument>
@@ -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<std::vector<HelixBlock>> successCallback,
-                       HelixFailureCallback failureCallback)
+                       ResultCallback<std::vector<HelixBlock>> pageCallback,
+                       FailureCallback<QString> 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<HelixBlock> 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<bool(const QJsonObject &)> onPage,
+                     std::function<void(NetworkResult)> onError,
+                     CancellationToken &&cancellationToken)
+{
+    auto onSuccess =
+        std::make_shared<std::function<Outcome(NetworkResult)>>(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);

src/providers/twitch/api/Helix.hpp

@@ -24,6 +24,8 @@ using HelixFailureCallback = std::function<void()>;
 template <typename... T>
 using ResultCallback = std::function<void(T...)>;
 
+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<std::vector<HelixBlock>> successCallback,
-        HelixFailureCallback failureCallback) = 0;
+        QString userId, ResultCallback<std::vector<HelixBlock>> pageCallback,
+        FailureCallback<QString> 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<std::vector<HelixBlock>> successCallback,
-                    HelixFailureCallback failureCallback) final;
+                    ResultCallback<std::vector<HelixBlock>> pageCallback,
+                    FailureCallback<QString> 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<bool(const QJsonObject &)> onPage,
+                  std::function<void(NetworkResult)> onError,
+                  CancellationToken &&token);
+
     QString clientId;
     QString oauthToken;
 };

src/util/CancellationToken.hpp

@@ -0,0 +1,82 @@
+#pragma once
+
+#include <atomic>
+#include <memory>
+
+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<bool>(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<std::atomic<bool>> 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

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;

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;
     }