From ca32e77d7400e23a6a839f6a8d1f322bfe48bbf0 Mon Sep 17 00:00:00 2001 From: Anthony Bilinski Date: Mon, 23 Apr 2018 16:51:26 -0700 Subject: [PATCH] feat(history): load set number of messages from history Fix #3124 Fix #3004 Instead of loading a set 7 days of history. Better performance when there are lots of messages, and better context when friends haven't talked in over a week. Removed historyBaselineDate, introduced in deb8440c6a8b508da127cdac2c54a7d6491cdbbf to fix duplicate messages, but duplicate messages were very likely fixed by https://github.com/qTox/qTox/pull/4607. Also refactored history loading. --- src/chatlog/chatlog.cpp | 4 +- src/chatlog/chatlog.h | 2 +- src/persistence/history.cpp | 102 +++++++++++----- src/persistence/history.h | 6 +- src/widget/form/chatform.cpp | 180 ++++++++++++++++------------ src/widget/form/chatform.h | 26 +++- src/widget/form/genericchatform.cpp | 6 - src/widget/form/genericchatform.h | 1 - src/widget/widget.cpp | 3 +- 9 files changed, 204 insertions(+), 126 deletions(-) diff --git a/src/chatlog/chatlog.cpp b/src/chatlog/chatlog.cpp index 44bcbb36b..50ff2a6ae 100644 --- a/src/chatlog/chatlog.cpp +++ b/src/chatlog/chatlog.cpp @@ -380,10 +380,10 @@ void ChatLog::insertChatlineOnTop(ChatLine::Ptr l) if (!l.get()) return; - insertChatlineOnTop(QList() << l); + insertChatlinesOnTop(QList() << l); } -void ChatLog::insertChatlineOnTop(const QList& newLines) +void ChatLog::insertChatlinesOnTop(const QList& newLines) { if (newLines.isEmpty()) return; diff --git a/src/chatlog/chatlog.h b/src/chatlog/chatlog.h index be2a79aee..dfaba6390 100644 --- a/src/chatlog/chatlog.h +++ b/src/chatlog/chatlog.h @@ -43,7 +43,7 @@ public: void insertChatlineAtBottom(ChatLine::Ptr l); void insertChatlineOnTop(ChatLine::Ptr l); - void insertChatlineOnTop(const QList& newLines); + void insertChatlinesOnTop(const QList& newLines); void clearSelection(); void clear(); void copySelectedText(bool toSelectionBuffer = false) const; diff --git a/src/persistence/history.cpp b/src/persistence/history.cpp index c5c04f8cf..44ce96f52 100644 --- a/src/persistence/history.cpp +++ b/src/persistence/history.cpp @@ -34,6 +34,8 @@ * Caches mappings to speed up message saving. */ +static constexpr int NUM_MESSAGES_DEFAULT = 100; // arbitrary number of messages loaded when not loading by date + /** * @brief Prepares the database to work with the history. * @param db This database will be prepared for use with the history. @@ -249,46 +251,29 @@ void History::addNewMessage(const QString& friendPk, const QString& message, con * @param to End of period to fetch. * @return List of messages. */ -QList History::getChatHistory(const QString& friendPk, const QDateTime& from, +QList History::getChatHistoryFromDate(const QString& friendPk, const QDateTime& from, const QDateTime& to) { if (!isValid()) { return {}; } - - QList messages; - - auto rowCallback = [&messages](const QVector& row) { - // dispName and message could have null bytes, QString::fromUtf8 - // truncates on null bytes so we strip them - messages += {row[0].toLongLong(), - row[1].isNull(), - QDateTime::fromMSecsSinceEpoch(row[2].toLongLong()), - row[3].toString(), - QString::fromUtf8(row[4].toByteArray().replace('\0', "")), - row[5].toString(), - QString::fromUtf8(row[6].toByteArray().replace('\0', ""))}; - }; - - // Don't forget to update the rowCallback if you change the selected columns! - QString queryText = - QString("SELECT history.id, faux_offline_pending.id, timestamp, " - "chat.public_key, aliases.display_name, sender.public_key, " - "message FROM history " - "LEFT JOIN faux_offline_pending ON history.id = faux_offline_pending.id " - "JOIN peers chat ON chat_id = chat.id " - "JOIN aliases ON sender_alias = aliases.id " - "JOIN peers sender ON aliases.owner = sender.id " - "WHERE timestamp BETWEEN %1 AND %2 AND chat.public_key='%3';") - .arg(from.toMSecsSinceEpoch()) - .arg(to.toMSecsSinceEpoch()) - .arg(friendPk); - - db->execNow({queryText, rowCallback}); - - return messages; + return getChatHistory(friendPk, from, to, 0); } +/** + * @brief Fetches the latest set amount of messages from the database. + * @param friendPk Friend public key to fetch. + * @return List of messages. + */ +QList History::getChatHistoryDefaultNum(const QString& friendPk) +{ + if (!isValid()) { + return {}; + } + return getChatHistory(friendPk, QDateTime::fromMSecsSinceEpoch(0), QDateTime::currentDateTime(), NUM_MESSAGES_DEFAULT); +} + + /** * @brief Fetches chat messages counts for each day from the database. * @param friendPk Friend public key to fetch. @@ -375,3 +360,54 @@ void History::markAsSent(qint64 messageId) db->execLater(QString("DELETE FROM faux_offline_pending WHERE id=%1;").arg(messageId)); } + + +/** + * @brief Fetches chat messages from the database. + * @param friendPk Friend publick key to fetch. + * @param from Start of period to fetch. + * @param to End of period to fetch. + * @param numMessages max number of messages to fetch. + * @return List of messages. + */ +QList History::getChatHistory(const QString& friendPk, const QDateTime& from, + const QDateTime& to, int numMessages) +{ + QList messages; + + auto rowCallback = [&messages](const QVector& row) { + // dispName and message could have null bytes, QString::fromUtf8 + // truncates on null bytes so we strip them + messages += {row[0].toLongLong(), + row[1].isNull(), + QDateTime::fromMSecsSinceEpoch(row[2].toLongLong()), + row[3].toString(), + QString::fromUtf8(row[4].toByteArray().replace('\0', "")), + row[5].toString(), + QString::fromUtf8(row[6].toByteArray().replace('\0', ""))}; + }; + + // Don't forget to update the rowCallback if you change the selected columns! + QString queryText = + QString("SELECT history.id, faux_offline_pending.id, timestamp, " + "chat.public_key, aliases.display_name, sender.public_key, " + "message FROM history " + "LEFT JOIN faux_offline_pending ON history.id = faux_offline_pending.id " + "JOIN peers chat ON chat_id = chat.id " + "JOIN aliases ON sender_alias = aliases.id " + "JOIN peers sender ON aliases.owner = sender.id " + "WHERE timestamp BETWEEN %1 AND %2 AND chat.public_key='%3'") + .arg(from.toMSecsSinceEpoch()) + .arg(to.toMSecsSinceEpoch()) + .arg(friendPk); + if (numMessages) { + queryText = "SELECT * FROM (" + queryText + + QString(" ORDER BY history.id DESC limit %1) AS T1 ORDER BY T1.id ASC;").arg(numMessages); + } else { + queryText = queryText + ";"; + } + + db->execNow({queryText, rowCallback}); + + return messages; +} diff --git a/src/persistence/history.h b/src/persistence/history.h index e03c73131..c749b3774 100644 --- a/src/persistence/history.h +++ b/src/persistence/history.h @@ -78,9 +78,9 @@ public: const QDateTime& time, bool isSent, QString dispName, const std::function& insertIdCallback = {}); - QList getChatHistory(const QString& friendPk, const QDateTime& from, + QList getChatHistoryFromDate(const QString& friendPk, const QDateTime& from, const QDateTime& to); - + QList getChatHistoryDefaultNum(const QString& friendPk); QList getChatHistoryCounts(const ToxPk& friendPk, const QDate& from, const QDate& to); QDateTime getDateWhereFindPhrase(const QString& friendPk, const QDateTime& from, QString phrase); @@ -93,6 +93,8 @@ protected: QString dispName, std::function insertIdCallback = {}); private: + QList getChatHistory(const QString& friendPk, const QDateTime& from, + const QDateTime& to, int numMessages); std::shared_ptr db; QHash peers; }; diff --git a/src/widget/form/chatform.cpp b/src/widget/form/chatform.cpp index 32f566bd7..492a8d895 100644 --- a/src/widget/form/chatform.cpp +++ b/src/widget/form/chatform.cpp @@ -63,9 +63,9 @@ * @brief stopNotification Tell others to stop notification of a call. */ -static const int CHAT_WIDGET_MIN_HEIGHT = 50; -static const int SCREENSHOT_GRABBER_OPENING_DELAY = 500; -static const int TYPING_NOTIFICATION_DURATION = 3000; +static constexpr int CHAT_WIDGET_MIN_HEIGHT = 50; +static constexpr int SCREENSHOT_GRABBER_OPENING_DELAY = 500; +static constexpr int TYPING_NOTIFICATION_DURATION = 3000; const QString ChatForm::ACTION_PREFIX = QStringLiteral("/me "); @@ -205,7 +205,7 @@ ChatForm::ChatForm(Friend* chatFriend, History* history) updateCallButtons(); if (Nexus::getProfile()->isHistoryEnabled()) { - loadHistory(QDateTime::currentDateTime().addDays(-7), true); + loadHistoryDefaultNum(true); } setAcceptDrops(true); @@ -503,14 +503,14 @@ void ChatForm::onSearchUp(const QString& phrase) if (startLine == 0) { QString pk = f->getPublicKey().toString(); - QDateTime newBaseData = history->getDateWhereFindPhrase(pk, earliestMessage, phrase); + QDateTime newBaseDate = history->getDateWhereFindPhrase(pk, earliestMessage, phrase); - if (!newBaseData.isValid()) { + if (!newBaseDate.isValid()) { return; } searchAfterLoadHistory = true; - loadHistory(newBaseData); + loadHistoryByDateRange(newBaseDate); return; } @@ -519,15 +519,15 @@ void ChatForm::onSearchUp(const QString& phrase) if (!isSearch) { QString pk = f->getPublicKey().toString(); - QDateTime newBaseData = history->getDateWhereFindPhrase(pk, earliestMessage, phrase); + QDateTime newBaseDate = history->getDateWhereFindPhrase(pk, earliestMessage, phrase); - if (!newBaseData.isValid()) { + if (!newBaseDate.isValid()) { return; } searchPoint.setX(numLines); searchAfterLoadHistory = true; - loadHistory(newBaseData); + loadHistoryByDateRange(newBaseDate); } } @@ -693,13 +693,6 @@ void ChatForm::clearChatArea(bool notInForm) offlineEngine->removeAllReceipts(); } -void ChatForm::onLoadChatHistory() -{ - if (sender() == f) { - loadHistory(QDateTime::currentDateTime().addDays(-7), true); - } -} - QString getMsgAuthorDispName(const ToxPk& authorPk, const QString& dispName) { QString authorStr; @@ -716,10 +709,19 @@ QString getMsgAuthorDispName(const ToxPk& authorPk, const QString& dispName) return authorStr; } -// TODO: Split on smaller methods (style) -void ChatForm::loadHistory(const QDateTime& since, bool processUndelivered) +void ChatForm::loadHistoryDefaultNum(bool processUndelivered) { - QDateTime now = historyBaselineDate.addMSecs(-1); + QString pk = f->getPublicKey().toString(); + QList msgs = history->getChatHistoryDefaultNum(pk); + if (!msgs.isEmpty()) { + earliestMessage = msgs.back().timestamp; + } + handleLoadedMessages(msgs, processUndelivered); +} + +void ChatForm::loadHistoryByDateRange(const QDateTime& since, bool processUndelivered) +{ + QDateTime now = QDateTime::currentDateTime(); if (since > now) { return; } @@ -736,72 +738,96 @@ void ChatForm::loadHistory(const QDateTime& since, bool processUndelivered) } QString pk = f->getPublicKey().toString(); - QList msgs = history->getChatHistory(pk, since, now); + earliestMessage = since; + QList msgs = history->getChatHistoryFromDate(pk, since, now); + handleLoadedMessages(msgs, processUndelivered); +} +void ChatForm::handleLoadedMessages(QList newHistMsgs, bool processUndelivered) +{ ToxPk prevIdBackup = previousId; previousId = ToxPk{}; - - QList historyMessages; - + QList chatLines; + Core* core = Core::getInstance(); QDate lastDate(1, 0, 0); - for (const auto& it : msgs) { - // Show the date every new day - QDateTime msgDateTime = it.timestamp.toLocalTime(); - QDate msgDate = msgDateTime.date(); - - if (msgDate > lastDate) { - lastDate = msgDate; - QString dateText = msgDate.toString(Settings::getInstance().getDateFormat()); - auto msg = ChatMessage::createChatInfoMessage(dateText, ChatMessage::INFO, QDateTime()); - historyMessages.append(msg); + for (const auto& histMessage : newHistMsgs) { + MessageMetadata const metadata = getMessageMetadata(histMessage); + lastDate = addDateLineIfNeeded(chatLines, lastDate, histMessage, metadata); + auto msg = chatMessageFromHistMessage(histMessage, metadata); + if (processUndelivered) { + sendLoadedMessage(msg, metadata); } - - // Show each messages - const Core* core = Core::getInstance(); - ToxPk authorPk(ToxId(it.sender).getPublicKey()); - QString authorStr = getMsgAuthorDispName(authorPk, it.dispName); - bool isSelf = authorPk == core->getSelfId().getPublicKey(); - - bool isAction = it.message.startsWith(ACTION_PREFIX, Qt::CaseInsensitive); - bool needSending = !it.isSent && isSelf; - - QString messageText = isAction ? it.message.mid(ACTION_PREFIX.length()) : it.message; - ChatMessage::MessageType type = isAction ? ChatMessage::ACTION : ChatMessage::NORMAL; - QDateTime dateTime = needSending ? QDateTime() : msgDateTime; - auto msg = ChatMessage::createChatMessage(authorStr, messageText, type, isSelf, dateTime); - if (!isAction && needsToHideName(authorPk, msgDateTime)) { - msg->hideSender(); - } - - previousId = authorPk; - prevMsgDateTime = msgDateTime; - - if (needSending && processUndelivered) { - Core* core = Core::getInstance(); - uint32_t friendId = f->getId(); - QString stringMsg = msg->toString(); - int receipt = isAction ? core->sendAction(friendId, stringMsg) - : core->sendMessage(friendId, stringMsg); - getOfflineMsgEngine()->registerReceipt(receipt, it.id, msg); - } - - historyMessages.append(msg); + chatLines.append(msg); + previousId = metadata.authorPk; + prevMsgDateTime = metadata.msgDateTime; } - previousId = prevIdBackup; - earliestMessage = since; - - QScrollBar* verticalBar = chatWidget->verticalScrollBar(); - int savedSliderPos = verticalBar->maximum() - verticalBar->value(); - chatWidget->insertChatlineOnTop(historyMessages); - savedSliderPos = verticalBar->maximum() - savedSliderPos; - verticalBar->setValue(savedSliderPos); - - if (searchAfterLoadHistory && historyMessages.isEmpty()) { + insertChatlines(chatLines); + if (searchAfterLoadHistory && chatLines.isEmpty()) { onContinueSearch(); } } +void ChatForm::insertChatlines(QList chatLines) +{ + QScrollBar* verticalBar = chatWidget->verticalScrollBar(); + int savedSliderPos = verticalBar->maximum() - verticalBar->value(); + chatWidget->insertChatlinesOnTop(chatLines); + savedSliderPos = verticalBar->maximum() - savedSliderPos; + verticalBar->setValue(savedSliderPos); +} + +QDate ChatForm::addDateLineIfNeeded(QList msgs, QDate const& lastDate, History::HistMessage const& newMessage, MessageMetadata const& metadata) +{ + // Show the date every new day + QDate newDate = metadata.msgDateTime.date(); + if (newDate > lastDate) { + QString dateText = newDate.toString(Settings::getInstance().getDateFormat()); + auto msg = ChatMessage::createChatInfoMessage(dateText, ChatMessage::INFO, QDateTime()); + msgs.append(msg); + return newDate; + } + return lastDate; +} + +ChatForm::MessageMetadata ChatForm::getMessageMetadata(History::HistMessage const& histMessage) +{ + const ToxPk authorPk = ToxId(histMessage.sender).getPublicKey(); + const QDateTime msgDateTime = histMessage.timestamp.toLocalTime(); + const bool isSelf = Core::getInstance()->getSelfId().getPublicKey() == authorPk; + const bool needSending = !histMessage.isSent && isSelf; + const bool isAction = histMessage.message.startsWith(ACTION_PREFIX, Qt::CaseInsensitive); + const qint64 id = histMessage.id; + return {isSelf, needSending, isAction, id, authorPk, msgDateTime}; +} + +ChatMessage::Ptr ChatForm::chatMessageFromHistMessage(History::HistMessage const& histMessage, MessageMetadata const& metadata) +{ + ToxPk authorPk(ToxId(histMessage.sender).getPublicKey()); + QString authorStr = getMsgAuthorDispName(authorPk, histMessage.dispName); + QString messageText = metadata.isAction ? histMessage.message.mid(ACTION_PREFIX.length()) : histMessage.message; + ChatMessage::MessageType type = metadata.isAction ? ChatMessage::ACTION : ChatMessage::NORMAL; + QDateTime dateTime = metadata.needSending ? QDateTime() : metadata.msgDateTime; + auto msg = ChatMessage::createChatMessage(authorStr, messageText, type, metadata.isSelf, dateTime); + if (!metadata.isAction && needsToHideName(authorPk, metadata.msgDateTime)) { + msg->hideSender(); + } + return msg; +} + +void ChatForm::sendLoadedMessage(ChatMessage::Ptr chatMsg, MessageMetadata const& metadata) +{ + if (!metadata.needSending) { + return; + } + Core* core = Core::getInstance(); + uint32_t friendId = f->getId(); + QString stringMsg = chatMsg->toString(); + int receipt = metadata.isAction ? core->sendAction(friendId, stringMsg) + : core->sendMessage(friendId, stringMsg); + getOfflineMsgEngine()->registerReceipt(receipt, metadata.id, chatMsg); +} + void ChatForm::onScreenshotClicked() { doScreenshot(); @@ -853,7 +879,7 @@ void ChatForm::onLoadHistory() LoadHistoryDialog dlg(f->getPublicKey()); if (dlg.exec()) { QDateTime fromTime = dlg.getFromDate(); - loadHistory(fromTime); + loadHistoryByDateRange(fromTime); } } @@ -1022,7 +1048,7 @@ void ChatForm::onExportChat() QString pk = f->getPublicKey().toString(); QDateTime epochStart = QDateTime::fromMSecsSinceEpoch(0); QDateTime now = QDateTime::currentDateTime(); - QList msgs = history->getChatHistory(pk, epochStart, now); + QList msgs = history->getChatHistoryFromDate(pk, epochStart, now); QString path = QFileDialog::getSaveFileName(0, tr("Save chat log"), QString{}, QString{}, 0, QFileDialog::DontUseNativeDialog); diff --git a/src/widget/form/chatform.h b/src/widget/form/chatform.h index 6ea58c120..c5860f95c 100644 --- a/src/widget/form/chatform.h +++ b/src/widget/form/chatform.h @@ -27,6 +27,7 @@ #include "genericchatform.h" #include "src/core/core.h" +#include "src/persistence/history.h" #include "src/widget/tool/screenshotgrabber.h" class CallConfirmWidget; @@ -45,7 +46,8 @@ public: ChatForm(Friend* chatFriend, History* history); ~ChatForm(); void setStatusMessage(const QString& newMessage); - void loadHistory(const QDateTime& since, bool processUndelivered = false); + void loadHistoryByDateRange(const QDateTime& since, bool processUndelivered = false); + void loadHistoryDefaultNum(bool processUndelivered = false); void dischargeReceipt(int receipt); void setFriendTyping(bool isTyping); @@ -83,7 +85,6 @@ private slots: void onAttachClicked() override; void onScreenshotClicked() override; - void onLoadChatHistory(); void onTextEditChanged(); void onCallTriggered(); void onVideoCallTriggered(); @@ -107,6 +108,27 @@ private slots: void onExportChat(); private: + struct MessageMetadata { + const bool isSelf; + const bool needSending; + const bool isAction; + const qint64 id; + const ToxPk authorPk; + const QDateTime msgDateTime; + MessageMetadata(bool isSelf, bool needSending, bool isAction, qint64 id, ToxPk authorPk, QDateTime msgDateTime) : + needSending{needSending}, + isSelf{isSelf}, + isAction{isAction}, + id{id}, + authorPk{authorPk}, + msgDateTime{msgDateTime} {} + }; + void handleLoadedMessages(QList newHistMsgs, bool processUndelivered); + QDate addDateLineIfNeeded(QList msgs, QDate const& lastDate, History::HistMessage const& newMessage, MessageMetadata const& metadata); + MessageMetadata getMessageMetadata(History::HistMessage const& histMessage); + ChatMessage::Ptr chatMessageFromHistMessage(History::HistMessage const& histMessage, MessageMetadata const& metadata); + void sendLoadedMessage(ChatMessage::Ptr chatMsg, MessageMetadata const& metadata); + void insertChatlines(QList chatLines); void updateMuteMicButton(); void updateMuteVolButton(); void retranslateUi(); diff --git a/src/widget/form/genericchatform.cpp b/src/widget/form/genericchatform.cpp index 251f50368..382f8a541 100644 --- a/src/widget/form/genericchatform.cpp +++ b/src/widget/form/genericchatform.cpp @@ -50,11 +50,6 @@ * @class GenericChatForm * @brief Parent class for all chatforms. It's provide the minimum required UI * elements and methods to work with chat messages. - * - * TODO: reword - * @var GenericChatForm::historyBaselineDate - * @brief Used by HistoryKeeper to load messages from t to historyBaselineDate - * (excluded) */ #define SET_STYLESHEET(x) (x)->setStyleSheet(Style::getStylesheet(":/ui/" #x "/" #x ".css")) @@ -670,7 +665,6 @@ void GenericChatForm::clearChatArea(bool notinform) addSystemInfoMessage(tr("Cleared"), ChatMessage::INFO, QDateTime::currentDateTime()); earliestMessage = QDateTime(); // null - historyBaselineDate = QDateTime::currentDateTime(); emit chatAreaCleared(); } diff --git a/src/widget/form/genericchatform.h b/src/widget/form/genericchatform.h index d458f0342..2fc630947 100644 --- a/src/widget/form/genericchatform.h +++ b/src/widget/form/genericchatform.h @@ -151,7 +151,6 @@ protected: QDateTime prevMsgDateTime; QDateTime earliestMessage; - QDateTime historyBaselineDate = QDateTime::currentDateTime(); QMenu menu; diff --git a/src/widget/widget.cpp b/src/widget/widget.cpp index 000d363ab..999489cf4 100644 --- a/src/widget/widget.cpp +++ b/src/widget/widget.cpp @@ -933,9 +933,8 @@ void Widget::setStatusMessage(const QString& statusMessage) void Widget::reloadHistory() { - QDateTime weekAgo = QDateTime::currentDateTime().addDays(-7); for (auto f : FriendList::getAllFriends()) { - chatForms[f->getId()]->loadHistory(weekAgo, true); + chatForms[f->getId()]->loadHistoryDefaultNum(true); } }