refactor(chatlog): remove getInstance from ChatlogItem

src/model/chatlogitem.cpp

@@ -18,9 +18,6 @@
 */
 
 #include "chatlogitem.h"
-#include "src/core/core.h"
-#include "src/friendlist.h"
-#include "src/grouplist.h"
 #include "src/model/friend.h"
 #include "src/model/group.h"
 
@@ -39,56 +36,23 @@ struct ChatLogItemDeleter
         delete static_cast<T*>(ptr);
     }
 };
-
-QString resolveToxPk(const ToxPk& pk)
-{
-    Friend* f = FriendList::findFriend(pk);
-    if (f) {
-        return f->getDisplayedName();
-    }
-
-    for (Group* it : GroupList::getAllGroups()) {
-        QString res = it->resolveToxId(pk);
-        if (!res.isEmpty()) {
-            return res;
-        }
-    }
-
-    return pk.toString();
-}
-
-QString resolveSenderNameFromSender(const ToxPk& sender)
-{
-    // TODO: Remove core instance
-    const Core* core = Core::getInstance();
-
-    // In unit tests we don't have a core instance so we just stringize the key
-    if (!core) {
-        return sender.toString();
-    }
-
-    bool isSelf = sender == core->getSelfId().getPublicKey();
-    QString myNickName = core->getUsername().isEmpty() ? sender.toString() : core->getUsername();
-
-    return isSelf ? myNickName : resolveToxPk(sender);
-}
 } // namespace
 
-ChatLogItem::ChatLogItem(ToxPk sender_, ChatLogFile file_)
+ChatLogItem::ChatLogItem(ToxPk sender_, const QString& displayName, ChatLogFile file_)
-    : ChatLogItem(std::move(sender_), ContentType::fileTransfer,
+    : ChatLogItem(std::move(sender_), displayName, ContentType::fileTransfer,
                   ContentPtr(new ChatLogFile(std::move(file_)),
                              ChatLogItemDeleter<ChatLogFile>::doDelete))
 {}
 
-ChatLogItem::ChatLogItem(ToxPk sender_, ChatLogMessage message_)
+ChatLogItem::ChatLogItem(ToxPk sender_, const QString& displayName, ChatLogMessage message_)
-    : ChatLogItem(sender_, ContentType::message,
+    : ChatLogItem(sender_, displayName, ContentType::message,
                   ContentPtr(new ChatLogMessage(std::move(message_)),
                              ChatLogItemDeleter<ChatLogMessage>::doDelete))
 {}
 
-ChatLogItem::ChatLogItem(ToxPk sender_, ContentType contentType_, ContentPtr content_)
+ChatLogItem::ChatLogItem(ToxPk sender_, const QString& displayName_, ContentType contentType_, ContentPtr content_)
     : sender(std::move(sender_))
-    , displayName(resolveSenderNameFromSender(sender))
+    , displayName(displayName_)
     , contentType(contentType_)
     , content(std::move(content_))
 {}

src/model/chatlogitem.h

@@ -50,8 +50,8 @@ public:
         fileTransfer,
     };
 
-    ChatLogItem(ToxPk sender, ChatLogFile file);
+    ChatLogItem(ToxPk sender, const QString& displayName, ChatLogFile file);
-    ChatLogItem(ToxPk sender, ChatLogMessage message);
+    ChatLogItem(ToxPk sender, const QString& displayName, ChatLogMessage message);
     const ToxPk& getSender() const;
     ContentType getContentType() const;
     ChatLogFile& getContentAsFile();
@@ -63,7 +63,7 @@ public:
     const QString& getDisplayName() const;
 
 private:
-    ChatLogItem(ToxPk sender, ContentType contentType, ContentPtr content);
+    ChatLogItem(ToxPk sender, const QString &displayName_, ContentType contentType, ContentPtr content);
 
     ToxPk sender;
     QString displayName;

src/model/sessionchatlog.cpp

@@ -19,6 +19,7 @@
 
 #include "sessionchatlog.h"
 #include "src/friendlist.h"
+#include "src/grouplist.h"
 
 #include <QDebug>
 #include <QtGlobal>
@@ -105,6 +106,23 @@ firstItemAfterDate(QDate date, const std::map<ChatLogIdx, ChatLogItem>& items)
                                 return a.timestamp.date() < b.timestamp.date();
                             });
 }
+
+QString resolveToxPk(const ToxPk& pk)
+{
+    Friend* f = FriendList::findFriend(pk);
+    if (f) {
+        return f->getDisplayedName();
+    }
+
+    for (Group* it : GroupList::getAllGroups()) {
+        QString res = it->resolveToxId(pk);
+        if (!res.isEmpty()) {
+            return res;
+        }
+    }
+
+    return pk.toString();
+}
 } // namespace
 
 SessionChatLog::SessionChatLog(const ICoreIdHandler& coreIdHandler)
@@ -121,6 +139,14 @@ SessionChatLog::SessionChatLog(ChatLogIdx initialIdx, const ICoreIdHandler& core
 
 SessionChatLog::~SessionChatLog() = default;
 
+QString SessionChatLog::resolveSenderNameFromSender(const ToxPk& sender)
+{
+    bool isSelf = sender == coreIdHandler.getSelfPublicKey();
+    QString myNickName = coreIdHandler.getUsername().isEmpty() ? sender.toString() : coreIdHandler.getUsername();
+
+    return isSelf ? myNickName : resolveToxPk(sender);
+}
+
 const ChatLogItem& SessionChatLog::at(ChatLogIdx idx) const
 {
     auto item = items.find(idx);
@@ -293,29 +319,21 @@ std::vector<IChatLog::DateChatLogIdxPair> SessionChatLog::getDateIdxs(const QDat
     return ret;
 }
 
-void SessionChatLog::insertCompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, const QString& senderName,
+void SessionChatLog::insertCompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, QString senderName,
                                                 const ChatLogMessage& message)
 {
-    auto item = ChatLogItem(sender, message);
+    auto item = ChatLogItem(sender, senderName, message);
-
-    if (!senderName.isEmpty()) {
-        item.setDisplayName(senderName);
-    }
 
     assert(message.state == MessageState::complete);
 
     items.emplace(idx, std::move(item));
 }
 
-void SessionChatLog::insertIncompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, const QString& senderName,
+void SessionChatLog::insertIncompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, QString senderName,
                                                   const ChatLogMessage& message,
                                                   DispatchedMessageId dispatchId)
 {
-    auto item = ChatLogItem(sender, message);
+    auto item = ChatLogItem(sender, senderName, message);
-
-    if (!senderName.isEmpty()) {
-        item.setDisplayName(senderName);
-    }
 
     assert(message.state == MessageState::pending);
 
@@ -323,27 +341,19 @@ void SessionChatLog::insertIncompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& s
     outgoingMessages.insert(dispatchId, idx);
 }
 
-void SessionChatLog::insertBrokenMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, const QString& senderName,
+void SessionChatLog::insertBrokenMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, QString senderName,
                                               const ChatLogMessage& message)
 {
-    auto item = ChatLogItem(sender, message);
+    auto item = ChatLogItem(sender, senderName, message);
-
-    if (!senderName.isEmpty()) {
-        item.setDisplayName(senderName);
-    }
 
     assert(message.state == MessageState::broken);
 
     items.emplace(idx, std::move(item));
 }
 
-void SessionChatLog::insertFileAtIdx(ChatLogIdx idx, const ToxPk& sender, const QString& senderName, const ChatLogFile& file)
+void SessionChatLog::insertFileAtIdx(ChatLogIdx idx, const ToxPk& sender, QString senderName, const ChatLogFile& file)
 {
-    auto item = ChatLogItem(sender, file);
+    auto item = ChatLogItem(sender, senderName, file);
-
-    if (!senderName.isEmpty()) {
-        item.setDisplayName(senderName);
-    }
 
     items.emplace(idx, std::move(item));
 }
@@ -359,7 +369,7 @@ void SessionChatLog::onMessageReceived(const ToxPk& sender, const Message& messa
     ChatLogMessage chatLogMessage;
     chatLogMessage.state = MessageState::complete;
     chatLogMessage.message = message;
-    items.emplace(messageIdx, ChatLogItem(sender, chatLogMessage));
+    items.emplace(messageIdx, ChatLogItem(sender, resolveSenderNameFromSender(sender), chatLogMessage));
 
     emit this->itemUpdated(messageIdx);
 }
@@ -375,7 +385,9 @@ void SessionChatLog::onMessageSent(DispatchedMessageId id, const Message& messag
     ChatLogMessage chatLogMessage;
     chatLogMessage.state = MessageState::pending;
     chatLogMessage.message = message;
-    items.emplace(messageIdx, ChatLogItem(coreIdHandler.getSelfPublicKey(), chatLogMessage));
+    const ToxPk selfPk = coreIdHandler.getSelfPublicKey();
+    const QString selfName = resolveSenderNameFromSender(selfPk);
+    items.emplace(messageIdx, ChatLogItem(selfPk, selfName, chatLogMessage));
 
     outgoingMessages.insert(id, messageIdx);
 
@@ -428,7 +440,7 @@ void SessionChatLog::onFileUpdated(const ToxPk& sender, const ToxFile& file)
         currentFileTransfers.push_back(currentTransfer);
 
         const auto chatLogFile = ChatLogFile{QDateTime::currentDateTime(), file};
-        items.emplace(currentTransfer.idx, ChatLogItem(sender, chatLogFile));
+        items.emplace(currentTransfer.idx, ChatLogItem(sender, resolveSenderNameFromSender(sender), chatLogFile));
         messageIdx = currentTransfer.idx;
     } else if (fileIt != currentFileTransfers.end()) {
         messageIdx = fileIt->idx;

src/model/sessionchatlog.h

@@ -45,13 +45,13 @@ public:
     ChatLogIdx getNextIdx() const override;
     std::vector<DateChatLogIdxPair> getDateIdxs(const QDate& startDate, size_t maxDates) const override;
 
-    void insertCompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, const QString& senderName,
+    void insertCompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, QString senderName,
                                     const ChatLogMessage& message);
-    void insertIncompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, const QString& senderName,
+    void insertIncompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, QString senderName,
                                       const ChatLogMessage& message, DispatchedMessageId dispatchId);
-    void insertBrokenMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, const QString& senderName,
+    void insertBrokenMessageAtIdx(ChatLogIdx idx, const ToxPk& sender, QString senderName,
                                   const ChatLogMessage& message);
-    void insertFileAtIdx(ChatLogIdx idx, const ToxPk& sender, const QString& senderName, const ChatLogFile& file);
+    void insertFileAtIdx(ChatLogIdx idx, const ToxPk& sender, QString senderName, const ChatLogFile& file);
 
 public slots:
     void onMessageReceived(const ToxPk& sender, const Message& message);
@@ -62,6 +62,10 @@ public slots:
     void onFileTransferRemotePausedUnpaused(const ToxPk& sender, const ToxFile& file, bool paused);
     void onFileTransferBrokenUnbroken(const ToxPk& sender, const ToxFile& file, bool broken);
 
+private:
+    QString resolveSenderNameFromSender(const ToxPk &sender);
+
+
 private:
     const ICoreIdHandler& coreIdHandler;
 

test/model/sessionchatlog_test.cpp

@@ -24,6 +24,8 @@
 #include <QtTest/QtTest>
 
 namespace {
+static const QString TEST_USERNAME = "qTox Tester #1";
+
 Message createMessage(const QString& content)
 {
     Message message;
@@ -50,8 +52,7 @@ public:
 
     QString getUsername() const override
     {
-        std::terminate();
+        return TEST_USERNAME;
-        return QString();
     }
 };
 } // namespace