Change the way Twitch accounts are stored in AccountManager

This is done in a way which should simplify abstracting it to other
types of accounts if needed in the future

Remove comment about removing singletons - we're keeping them (and probably restoring some)

IrcManager now updates its "account" reference automatically through the
AccountManager.Twitch.userChanged-signal

Remove unused IrcManager getUser-method

IrcManager::beginConnecting is no longer called asynchronously. This
might want to be reverted in a more controlled asynchronous manner.

User Accounts are now stored as Shared Pointers instead of using
references/copies everywhere

src/accountmanager.cpp

@@ -1,7 +1,6 @@
 #include "accountmanager.hpp"
 #include "common.hpp"
-
-#include <pajlada/settings/setting.hpp>
+#include "debug/log.hpp"
 
 namespace chatterino {
 
@@ -19,18 +18,84 @@ inline QString getEnvString(const char *target)
 
 }  // namespace
 
-AccountManager::AccountManager()
-    : currentUser("/accounts/current", "")
-    , twitchAnonymousUser("justinfan64537", "", "")
+std::shared_ptr<twitch::TwitchUser> TwitchAccountManager::getCurrent()
 {
+    if (!this->currentUser) {
+        return this->anonymousUser;
+    }
+
+    return this->currentUser;
+}
+
+std::vector<QString> TwitchAccountManager::getUsernames() const
+{
+    std::vector<QString> userNames;
+
+    std::lock_guard<std::mutex> lock(this->mutex);
+
+    for (const auto &user : this->users) {
+        userNames.push_back(user->getUserName());
+    }
+
+    return userNames;
+}
+
+std::shared_ptr<twitch::TwitchUser> TwitchAccountManager::findUserByUsername(
+    const QString &username) const
+{
+    std::lock_guard<std::mutex> lock(this->mutex);
+
+    for (const auto &user : this->users) {
+        if (username.compare(user->getUserName(), Qt::CaseInsensitive) == 0) {
+            return user;
+        }
+    }
+
+    return nullptr;
+}
+
+bool TwitchAccountManager::userExists(const QString &username) const
+{
+    return this->findUserByUsername(username) != nullptr;
+}
+
+bool TwitchAccountManager::addUser(std::shared_ptr<twitch::TwitchUser> user)
+{
+    if (this->userExists(user->getNickName())) {
+        // User already exists in user list
+        return false;
+    }
+
+    std::lock_guard<std::mutex> lock(this->mutex);
+
+    this->users.push_back(user);
+
+    return true;
+}
+
+AccountManager::AccountManager()
+{
+    this->Twitch.anonymousUser.reset(new twitch::TwitchUser("justinfan64537", "", ""));
+
+    this->Twitch.currentUsername.getValueChangedSignal().connect([this](const auto &newValue) {
+        QString newUsername(QString::fromStdString(newValue));
+        auto user = this->Twitch.findUserByUsername(newUsername);
+        if (user) {
+            debug::Log("[AccountManager:currentUsernameChanged] User successfully updated to {}",
+                       newUsername);
+            // XXX: Should we set the user regardless if the username is found or not?
+            // I can see the logic in setting it to nullptr if the `currentUsername` value has been
+            // set to "" or an invalid username
+            this->Twitch.currentUser = user;
+            this->Twitch.userChanged.invoke();
+        }
+    });
 }
 
 void AccountManager::load()
 {
     auto keys = pajlada::Settings::SettingManager::getObjectKeys("/accounts");
 
-    bool first = true;
-
     for (const auto &uid : keys) {
         if (uid == "current") {
             continue;
@@ -49,74 +114,20 @@ void AccountManager::load()
             continue;
         }
 
-        if (first) {
-            this->setCurrentTwitchUser(qS(username));
-            first = false;
-        }
+        auto user =
+            std::make_shared<twitch::TwitchUser>(qS(username), qS(oauthToken), qS(clientID));
 
-        twitch::TwitchUser user(qS(username), qS(oauthToken), qS(clientID));
-
-        this->addTwitchUser(user);
+        this->Twitch.addUser(user);
 
         printf("Adding user %s(%s)\n", username.c_str(), userID.c_str());
     }
-}
 
-twitch::TwitchUser &AccountManager::getTwitchAnon()
-{
-    return this->twitchAnonymousUser;
-}
-
-twitch::TwitchUser &AccountManager::getTwitchUser()
-{
-    std::lock_guard<std::mutex> lock(this->twitchUsersMutex);
-
-    if (this->twitchUsers.size() == 0) {
-        return this->getTwitchAnon();
+    auto currentUser = this->Twitch.findUserByUsername(
+        QString::fromStdString(this->Twitch.currentUsername.getValue()));
+    if (currentUser) {
+        this->Twitch.currentUser = currentUser;
+        this->Twitch.userChanged.invoke();
     }
-
-    QString currentUsername = QString::fromStdString(this->currentUser);
-
-    for (auto &user : this->twitchUsers) {
-        if (user.getUserName() == currentUsername) {
-            return user;
-        }
-    }
-
-    return this->twitchUsers.front();
-}
-
-void AccountManager::setCurrentTwitchUser(const QString &username)
-{
-    this->currentUser.setValue(username.toStdString());
-}
-
-std::vector<twitch::TwitchUser> AccountManager::getTwitchUsers()
-{
-    std::lock_guard<std::mutex> lock(this->twitchUsersMutex);
-
-    return std::vector<twitch::TwitchUser>(this->twitchUsers);
-}
-
-bool AccountManager::removeTwitchUser(const QString &userName)
-{
-    std::lock_guard<std::mutex> lock(this->twitchUsersMutex);
-
-    for (auto it = this->twitchUsers.begin(); it != this->twitchUsers.end(); it++) {
-        if ((*it).getUserName() == userName) {
-            this->twitchUsers.erase(it);
-            return true;
-        }
-    }
-
-    return false;
-}
-
-void AccountManager::addTwitchUser(const twitch::TwitchUser &user)
-{
-    std::lock_guard<std::mutex> lock(this->twitchUsersMutex);
-
-    this->twitchUsers.push_back(user);
 }
 
 }  // namespace chatterino

src/accountmanager.hpp

@@ -9,6 +9,34 @@
 
 namespace chatterino {
 
+class AccountManager;
+
+class TwitchAccountManager
+{
+public:
+    // Returns the current twitchUsers, or the anonymous user if we're not currently logged in
+    std::shared_ptr<twitch::TwitchUser> getCurrent();
+
+    std::vector<QString> getUsernames() const;
+
+    std::shared_ptr<twitch::TwitchUser> findUserByUsername(const QString &username) const;
+    bool userExists(const QString &username) const;
+
+    pajlada::Settings::Setting<std::string> currentUsername = {"/accounts/current", ""};
+    pajlada::Signals::NoArgSignal userChanged;
+
+private:
+    bool addUser(std::shared_ptr<twitch::TwitchUser> user);
+
+    std::shared_ptr<twitch::TwitchUser> currentUser;
+
+    std::shared_ptr<twitch::TwitchUser> anonymousUser;
+    std::vector<std::shared_ptr<twitch::TwitchUser>> users;
+    mutable std::mutex mutex;
+
+    friend class AccountManager;
+};
+
 class AccountManager
 {
 public:
@@ -20,30 +48,10 @@ public:
 
     void load();
 
-    twitch::TwitchUser &getTwitchAnon();
-
-    // Returns first user from twitchUsers, or twitchAnonymousUser if twitchUsers is empty
-    twitch::TwitchUser &getTwitchUser();
-
-    // Return a copy of the current available twitch users
-    std::vector<twitch::TwitchUser> getTwitchUsers();
-
-    // Remove twitch user with the given username
-    bool removeTwitchUser(const QString &userName);
-
-    void setCurrentTwitchUser(const QString &username);
-
-    // Add twitch user to the list of available twitch users
-    void addTwitchUser(const twitch::TwitchUser &user);
+    TwitchAccountManager Twitch;
 
 private:
     AccountManager();
-
-    pajlada::Settings::Setting<std::string> currentUser;
-
-    twitch::TwitchUser twitchAnonymousUser;
-    std::vector<twitch::TwitchUser> twitchUsers;
-    std::mutex twitchUsersMutex;
 };
 
 }  // namespace chatterino

src/application.cpp

@@ -18,7 +18,6 @@ Application::Application()
     , channelManager(this->windowManager, this->emoteManager, this->ircManager)
     , ircManager(this->channelManager, this->resources, this->emoteManager, this->windowManager)
 {
-    // TODO(pajlada): Get rid of all singletons
     logging::init();
     SettingsManager::getInstance().load();
 
@@ -29,8 +28,6 @@ Application::Application()
 
     AccountManager::getInstance().load();
 
-    this->ircManager.setUser(AccountManager::getInstance().getTwitchUser());
-
     // XXX
     SettingsManager::getInstance().updateWordTypeMask();
 

src/ircmanager.cpp

@@ -3,6 +3,7 @@
 #include "asyncexec.hpp"
 #include "channel.hpp"
 #include "channelmanager.hpp"
+#include "debug/log.hpp"
 #include "emotemanager.hpp"
 #include "messages/messageparseargs.hpp"
 #include "twitch/twitchmessagebuilder.hpp"
@@ -31,21 +32,18 @@ IrcManager::IrcManager(ChannelManager &_channelManager, Resources &_resources,
     , resources(_resources)
     , emoteManager(_emoteManager)
     , windowManager(_windowManager)
-    , account(AccountManager::getInstance().getTwitchAnon())
-    , currentUser("/accounts/current")
 {
-    this->currentUser.getValueChangedSignal().connect([](const auto &newUsername) {
-        // TODO: Implement
-        qDebug() << "Current user changed, fetch new credentials and reconnect";
+    AccountManager::getInstance().Twitch.userChanged.connect([this]() {
+        this->setUser(AccountManager::getInstance().Twitch.getCurrent());
+
+        debug::Log("[IrcManager] Reconnecting to Twitch IRC as new user {}",
+                   this->account->getUserName());
+
+        postToThread([this] { this->connect(); });
     });
 }
 
-const twitch::TwitchUser &IrcManager::getUser() const
-{
-    return this->account;
-}
-
-void IrcManager::setUser(const twitch::TwitchUser &account)
+void IrcManager::setUser(std::shared_ptr<twitch::TwitchUser> account)
 {
     this->account = account;
 }
@@ -54,11 +52,16 @@ void IrcManager::connect()
 {
     disconnect();
 
-    async_exec([this] { beginConnecting(); });
+    // XXX(pajlada): Disabled the async_exec for now, because if we happen to run the
+    // `beginConnecting` function in a different thread than last time, we won't be able to connect
+    // because we can't clean up the previous connection properly
+    // async_exec([this] { beginConnecting(); });
+    this->beginConnecting();
 }
 
 Communi::IrcConnection *IrcManager::createConnection(bool doRead)
 {
+    assert(this->account);
     Communi::IrcConnection *connection = new Communi::IrcConnection;
 
     if (doRead) {
@@ -68,9 +71,9 @@ Communi::IrcConnection *IrcManager::createConnection(bool doRead)
                          &IrcManager::privateMessageReceived);
     }
 
-    QString username = this->account.getUserName();
-    QString oauthClient = this->account.getOAuthClient();
-    QString oauthToken = this->account.getOAuthToken();
+    QString username = this->account->getUserName();
+    QString oauthClient = this->account->getOAuthClient();
+    QString oauthToken = this->account->getOAuthToken();
     if (!oauthToken.startsWith("oauth:")) {
         oauthToken.prepend("oauth:");
     }
@@ -79,7 +82,7 @@ Communi::IrcConnection *IrcManager::createConnection(bool doRead)
     connection->setNickName(username);
     connection->setRealName(username);
 
-    if (!this->account.isAnon()) {
+    if (!this->account->isAnon()) {
         connection->setPassword(oauthToken);
 
         this->refreshIgnoredUsers(username, oauthClient, oauthToken);
@@ -310,9 +313,11 @@ bool IrcManager::isTwitchBlockedUser(QString const &username)
 
 bool IrcManager::tryAddIgnoredUser(QString const &username, QString &errorMessage)
 {
-    QUrl url("https://api.twitch.tv/kraken/users/" + this->account.getUserName() + "/blocks/" +
-             username + "?oauth_token=" + this->account.getOAuthToken() +
-             "&client_id=" + this->account.getOAuthClient());
+    assert(this->account);
+
+    QUrl url("https://api.twitch.tv/kraken/users/" + this->account->getUserName() + "/blocks/" +
+             username + "?oauth_token=" + this->account->getOAuthToken() +
+             "&client_id=" + this->account->getOAuthClient());
 
     QNetworkRequest request(url);
     auto reply = this->networkAccessManager.put(request, QByteArray());
@@ -342,9 +347,10 @@ void IrcManager::addIgnoredUser(QString const &username)
 
 bool IrcManager::tryRemoveIgnoredUser(QString const &username, QString &errorMessage)
 {
-    QUrl url("https://api.twitch.tv/kraken/users/" + this->account.getUserName() + "/blocks/" +
-             username + "?oauth_token=" + this->account.getOAuthToken() +
-             "&client_id=" + this->account.getOAuthClient());
+    assert(this->account);
+    QUrl url("https://api.twitch.tv/kraken/users/" + this->account->getUserName() + "/blocks/" +
+             username + "?oauth_token=" + this->account->getOAuthToken() +
+             "&client_id=" + this->account->getOAuthClient());
 
     QNetworkRequest request(url);
     auto reply = this->networkAccessManager.deleteResource(request);

src/ircmanager.hpp

@@ -44,8 +44,7 @@ public:
     void joinChannel(const QString &channelName);
     void partChannel(const QString &channelName);
 
-    const twitch::TwitchUser &getUser() const;
-    void setUser(const twitch::TwitchUser &account);
+    void setUser(std::shared_ptr<twitch::TwitchUser> account);
 
     pajlada::Signals::Signal<Communi::IrcPrivateMessage *> onPrivateMessage;
 
@@ -56,9 +55,7 @@ public:
 
 private:
     // variables
-    twitch::TwitchUser account;
-
-    pajlada::Settings::Setting<std::string> currentUser;
+    std::shared_ptr<twitch::TwitchUser> account = nullptr;
 
     std::shared_ptr<Communi::IrcConnection> writeConnection = nullptr;
 

src/util/urlfetch.hpp

@@ -87,11 +87,18 @@ static void put(QUrl url, std::function<void(QJsonObject)> successCallback)
     auto manager = new QNetworkAccessManager();
     QNetworkRequest request(url);
 
+    auto &accountManager = AccountManager::getInstance();
+    auto currentTwitchUser = accountManager.Twitch.getCurrent();
+    QByteArray oauthToken;
+    if (currentTwitchUser) {
+        oauthToken = currentTwitchUser->getOAuthToken().toUtf8();
+    } else {
+        // XXX(pajlada): Bail out?
+    }
+
     request.setRawHeader("Client-ID", getDefaultClientID());
     request.setRawHeader("Accept", "application/vnd.twitchtv.v5+json");
-    request.setRawHeader(
-        "Authorization",
-        "OAuth " + AccountManager::getInstance().getTwitchUser().getOAuthToken().toUtf8());
+    request.setRawHeader("Authorization", "OAuth " + oauthToken);
 
     NetworkManager::urlPut(std::move(request), [=](QNetworkReply *reply) {
         if (reply->error() == QNetworkReply::NetworkError::NoError) {

src/widgets/accountpopup.cpp

@@ -60,6 +60,15 @@ AccountPopupWidget::AccountPopupWidget(std::shared_ptr<Channel> channel)
     sendCommand(this->_ui->mod, "/mod ");
     sendCommand(this->_ui->unMod, "/unmod ");
 
+    auto &accountManager = AccountManager::getInstance();
+    QString userId;
+    QString userNickname;
+    auto currentTwitchUser = accountManager.Twitch.getCurrent();
+    if (currentTwitchUser) {
+        userId = currentTwitchUser->getUserId();
+        userNickname = currentTwitchUser->getNickName();
+    }
+
     QObject::connect(this->_ui->profile, &QPushButton::clicked, this, [=](){
         QDesktopServices::openUrl(QUrl("https://twitch.tv/" +
                                        this->_ui->lblUsername->text()));
@@ -76,7 +85,7 @@ AccountPopupWidget::AccountPopupWidget(std::shared_ptr<Channel> channel)
 
     QObject::connect(this->_ui->follow, &QPushButton::clicked, this, [=](){
         QUrl requestUrl("https://api.twitch.tv/kraken/users/" +
-                        AccountManager::getInstance().getTwitchUser().getUserId() +
+                        userId +
                         "/follows/channels/" + this->userID);
 
         util::twitch::put(requestUrl,[](QJsonObject obj){
@@ -86,7 +95,7 @@ AccountPopupWidget::AccountPopupWidget(std::shared_ptr<Channel> channel)
 
     QObject::connect(this->_ui->ignore, &QPushButton::clicked, this, [=](){
         QUrl requestUrl("https://api.twitch.tv/kraken/users/" +
-                        AccountManager::getInstance().getTwitchUser().getUserId() +
+                        userId +
                         "/blocks/" + this->userID);
 
         util::twitch::put(requestUrl,[](QJsonObject obj){
@@ -121,9 +130,9 @@ AccountPopupWidget::AccountPopupWidget(std::shared_ptr<Channel> channel)
         hide();  //
     });
 
-    util::twitch::getUserID(AccountManager::getInstance().getTwitchUser().getNickName(), this,
+    util::twitch::getUserID(userNickname, this,
                             [=](const QString &id){
-        AccountManager::getInstance().getTwitchUser().setUserId(id);
+        currentTwitchUser->setUserId(id);
     });
 }
 
@@ -185,12 +194,20 @@ void AccountPopupWidget::loadAvatar(const QUrl &avatarUrl)
 
 void AccountPopupWidget::updatePermissions()
 {
-    if(this->_channel.get()->name == AccountManager::getInstance().getTwitchUser().getNickName())
+    AccountManager &accountManager = AccountManager::getInstance();
+    auto currentTwitchUser = accountManager.Twitch.getCurrent();
+    if (!currentTwitchUser) {
+        // No twitch user set (should never happen)
+        return;
+    }
+
+    if(this->_channel.get()->name == currentTwitchUser->getNickName())
     {
         permission = permissions::Owner;
     }
-    else if(this->_channel->modList.contains(AccountManager::getInstance().getTwitchUser().getNickName()))
+    else if(this->_channel->modList.contains(currentTwitchUser->getNickName()))
     {
+        // XXX(pajlada): This might always trigger if user is anonymous (if nickName is empty?)
         permission = permissions::Mod;
     }
 }
@@ -229,8 +246,15 @@ void AccountPopupWidget::focusOutEvent(QFocusEvent *event)
 
 void AccountPopupWidget::showEvent(QShowEvent *event)
 {
-    if(this->_ui->lblUsername->text() != AccountManager::getInstance().getTwitchUser().getNickName())
-    {    
+    AccountManager &accountManager = AccountManager::getInstance();
+    auto currentTwitchUser = accountManager.Twitch.getCurrent();
+    if (!currentTwitchUser) {
+        // No twitch user set (should never happen)
+        return;
+    }
+
+    if(this->_ui->lblUsername->text() != currentTwitchUser->getNickName())
+    {
         updateButtons(this->_ui->userLayout, true);
         if(permission != permissions::User)
         {

src/widgets/settingsdialog.cpp

@@ -129,13 +129,13 @@ QVBoxLayout *SettingsDialog::createAccountsTab()
     // listview
     auto listWidget = new QListWidget(this);
 
-    for (auto &user : AccountManager::getInstance().getTwitchUsers()) {
-        listWidget->addItem(user.getUserName());
+    for (const auto &userName : AccountManager::getInstance().Twitch.getUsernames()) {
+        listWidget->addItem(userName);
     }
 
+    // Select the currently logged in user
     if (listWidget->count() > 0) {
-        const auto &currentUser = AccountManager::getInstance().getTwitchUser();
-        QString currentUsername = currentUser.getUserName();
+        const QString &currentUsername = AccountManager::getInstance().Twitch.getCurrent()->getUserName();
         for (int i = 0; i < listWidget->count(); ++i) {
             QString itemText = listWidget->item(i)->text();
             if (itemText.compare(currentUsername, Qt::CaseInsensitive) == 0) {
@@ -147,7 +147,8 @@ QVBoxLayout *SettingsDialog::createAccountsTab()
 
     QObject::connect(listWidget, &QListWidget::clicked, this, [&, listWidget] {
         if (!listWidget->selectedItems().isEmpty()) {
-            AccountManager::getInstance().setCurrentTwitchUser(listWidget->currentItem()->text());
+            QString newUsername = listWidget->currentItem()->text();
+            AccountManager::getInstance().Twitch.currentUsername = newUsername.toStdString();
         }
     });