From f45319de2524036d6fe375160e1c7bd204be956b Mon Sep 17 00:00:00 2001 From: blue Date: Fri, 7 May 2021 21:26:02 +0300 Subject: [PATCH] now instead of storing uploading message in ram I store them in database to be able to recover unsent ones on the next statrt. Found and fixed bug with spam repaints in feedview because of icons --- core/archive.cpp | 16 +++-- core/handlers/messagehandler.cpp | 115 ++++++++++++++++++++----------- core/handlers/messagehandler.h | 9 ++- core/rosteritem.cpp | 17 +++++ core/rosteritem.h | 2 + shared/message.cpp | 10 ++- ui/models/messagefeed.cpp | 8 +++ ui/utils/feedview.cpp | 4 +- ui/utils/messagedelegate.cpp | 24 ++++--- 9 files changed, 146 insertions(+), 59 deletions(-) diff --git a/core/archive.cpp b/core/archive.cpp index f18201b..96a8c0d 100644 --- a/core/archive.cpp +++ b/core/archive.cpp @@ -271,6 +271,8 @@ void Core::Archive::changeMessage(const QString& id, const QMap 0; QDateTime oTime = msg.getTime(); bool idChange = msg.change(data); + QDateTime nTime = msg.getTime(); + bool orderChange = oTime != nTime; MDB_val lmdbKey, lmdbData; QByteArray ba; @@ -280,15 +282,21 @@ void Core::Archive::changeMessage(const QString& id, const QMaprh->getRosterItem(jid); + bool sent = false; QMap changes; if (acc->state == Shared::ConnectionState::connected) { QXmppMessage msg(acc->getFullJid(), data.getTo(), data.getBody(), data.getThread()); @@ -267,20 +267,13 @@ void Core::MessageHandler::performSending(Shared::Message data) msg.setOutOfBandUrl(oob); msg.setReceiptRequested(true); - bool sent = acc->client.sendPacket(msg); + sent = acc->client.sendPacket(msg); if (sent) { data.setState(Shared::Message::State::sent); } else { data.setState(Shared::Message::State::error); - data.setErrorText("Couldn't send message via QXMPP library check out logs"); - } - - if (ri != 0) { - ri->appendMessageToArchive(data); - if (sent) { - pendingStateMessages.insert(std::make_pair(id, jid)); - } + data.setErrorText("Couldn't send message: internal QXMPP library error, probably need to check out the logs"); } } else { @@ -296,6 +289,22 @@ void Core::MessageHandler::performSending(Shared::Message data) if (oob.size() > 0) { changes.insert("outOfBandUrl", oob); } + if (!newMessage) { + changes.insert("stamp", data.getTime()); + } + + if (ri != 0) { + if (newMessage) { + ri->appendMessageToArchive(data); + } else { + ri->changeMessage(id, changes); + } + if (sent) { + pendingStateMessages.insert(std::make_pair(id, jid)); + } else { + pendingStateMessages.erase(id); + } + } emit acc->changeMessage(jid, id, changes); } @@ -303,27 +312,37 @@ void Core::MessageHandler::performSending(Shared::Message data) void Core::MessageHandler::prepareUpload(const Shared::Message& data) { if (acc->state == Shared::ConnectionState::connected) { + QString jid = data.getPenPalJid(); + QString id = data.getId(); + RosterItem* ri = acc->rh->getRosterItem(jid); + if (!ri) { + qDebug() << "An attempt to initialize upload in" << acc->name << "for pal" << jid << "but the object for this pal wasn't found, something went terrebly wrong, skipping send"; + return; + } QString path = data.getAttachPath(); QString url = acc->network->getFileRemoteUrl(path); if (url.size() != 0) { sendMessageWithLocalUploadedFile(data, url); } else { - if (acc->network->checkAndAddToUploading(acc->getName(), data.getPenPalJid(), data.getId(), path)) { - pendingMessages.emplace(data.getId(), data); + if (acc->network->checkAndAddToUploading(acc->getName(), jid, id, path)) { + ri->appendMessageToArchive(data); + pendingStateMessages.insert(std::make_pair(id, jid)); } else { if (acc->um->serviceFound()) { QFileInfo file(path); if (file.exists() && file.isReadable()) { - uploadingSlotsQueue.emplace_back(path, data); + ri->appendMessageToArchive(data); + pendingStateMessages.insert(std::make_pair(id, jid)); + uploadingSlotsQueue.emplace_back(path, id); if (uploadingSlotsQueue.size() == 1) { acc->um->requestUploadSlot(file); } } else { - handleUploadError(data.getPenPalJid(), data.getId(), "Uploading file no longer exists or your system user has no permission to read it"); + handleUploadError(jid, id, "Uploading file no longer exists or your system user has no permission to read it"); qDebug() << "Requested upload slot in account" << acc->name << "for file" << path << "but the file doesn't exist or is not readable"; } } else { - handleUploadError(data.getPenPalJid(), data.getId(), "Your server doesn't support file upload service, or it's prohibited for your account"); + handleUploadError(jid, id, "Your server doesn't support file upload service, or it's prohibited for your account"); qDebug() << "Requested upload slot in account" << acc->name << "for file" << path << "but upload manager didn't discover any upload services"; } } @@ -340,10 +359,10 @@ void Core::MessageHandler::onUploadSlotReceived(const QXmppHttpUploadSlotIq& slo if (uploadingSlotsQueue.size() == 0) { qDebug() << "HTTP Upload manager of account" << acc->name << "reports about success requesting upload slot, but none was requested"; } else { - const std::pair& pair = uploadingSlotsQueue.front(); - const QString& mId = pair.second.getId(); - acc->network->uploadFile({acc->name, pair.second.getPenPalJid(), mId}, pair.first, slot.putUrl(), slot.getUrl(), slot.putHeaders()); - pendingMessages.emplace(mId, pair.second); + const std::pair& pair = uploadingSlotsQueue.front(); + const QString& mId = pair.second; + QString palJid = pendingStateMessages.at(mId); + acc->network->uploadFile({acc->name, palJid, mId}, pair.first, slot.putUrl(), slot.getUrl(), slot.putHeaders()); uploadingSlotsQueue.pop_front(); if (uploadingSlotsQueue.size() > 0) { @@ -359,9 +378,9 @@ void Core::MessageHandler::onUploadSlotRequestFailed(const QXmppHttpUploadReques qDebug() << "HTTP Upload manager of account" << acc->name << "reports about an error requesting upload slot, but none was requested"; qDebug() << err; } else { - const std::pair& pair = uploadingSlotsQueue.front(); + const std::pair& pair = uploadingSlotsQueue.front(); qDebug() << "Error requesting upload slot for file" << pair.first << "in account" << acc->name << ":" << err; - emit acc->uploadFileError(pair.second.getPenPalJid(), pair.second.getId(), "Error requesting slot to upload file: " + err); + handleUploadError(pendingStateMessages.at(pair.second), pair.second, err); uploadingSlotsQueue.pop_front(); if (uploadingSlotsQueue.size() > 0) { @@ -400,47 +419,65 @@ void Core::MessageHandler::onLoadFileError(const std::list& void Core::MessageHandler::handleUploadError(const QString& jid, const QString& messageId, const QString& errorText) { - std::map::const_iterator itr = pendingMessages.find(messageId); - if (itr != pendingMessages.end()) { - pendingMessages.erase(itr); - //TODO move the storage of pending messages to the database and change them there - } + emit acc->uploadFileError(jid, messageId, "Error requesting slot to upload file: " + errorText); + pendingStateMessages.erase(jid); + requestChangeMessage(jid, messageId, { + {"state", static_cast(Shared::Message::State::error)}, + {"errorText", errorText} + }); } void Core::MessageHandler::onUploadFileComplete(const std::list& msgs, const QString& path) { for (const Shared::MessageInfo& info : msgs) { if (info.account == acc->getName()) { - std::map::const_iterator itr = pendingMessages.find(info.messageId); - if (itr != pendingMessages.end()) { - sendMessageWithLocalUploadedFile(itr->second, path); - pendingMessages.erase(itr); + RosterItem* ri = acc->rh->getRosterItem(info.jid); + if (ri != 0) { + Shared::Message msg = ri->getMessage(info.messageId); + sendMessageWithLocalUploadedFile(msg, path, false); + } else { + qDebug() << "A signal received about complete upload to" << acc->name << "for pal" << info.jid << "but the object for this pal wasn't found, something went terrebly wrong, skipping send"; } } } } -void Core::MessageHandler::sendMessageWithLocalUploadedFile(Shared::Message msg, const QString& url) +void Core::MessageHandler::sendMessageWithLocalUploadedFile(Shared::Message msg, const QString& url, bool newMessage) { msg.setOutOfBandUrl(url); - if (msg.getBody().size() == 0) { - msg.setBody(url); - } - performSending(msg); + if (msg.getBody().size() == 0) { //not sure why, but most messages do that + msg.setBody(url); //they duplicate oob in body, some of them wouldn't even show an attachment if you don't do that + } + performSending(msg, newMessage); //TODO removal/progress update } +static const std::set allowerToChangeKeys({ + "attachPath", + "outOfBandUrl", + "state", + "errorText" +}); + void Core::MessageHandler::requestChangeMessage(const QString& jid, const QString& messageId, const QMap& data) { RosterItem* cnt = acc->rh->getRosterItem(jid); if (cnt != 0) { - QMap::const_iterator itr = data.find("attachPath"); - if (data.size() == 1 && itr != data.end()) { + bool allSupported = true; + QString unsupportedString; + for (QMap::const_iterator itr = data.begin(); itr != data.end(); ++itr) { //I need all this madness + if (allowerToChangeKeys.count(itr.key()) != 1) { //to not allow this method + allSupported = false; //to make a message to look like if it was edited + unsupportedString = itr.key(); //basically I needed to control who exaclty calls this method + break; //because the underlying tech assumes that the change is initiated by user + } //not by system + } + if (allSupported) { cnt->changeMessage(messageId, data); emit acc->changeMessage(jid, messageId, data); } else { qDebug() << "A request to change message" << messageId << "of conversation" << jid << "with following data" << data; - qDebug() << "nothing but the changing of the local path is supported yet in this method, skipping"; + qDebug() << "only limited set of dataFields are supported yet here, and" << unsupportedString << "isn't one of them, skipping"; } } } diff --git a/core/handlers/messagehandler.h b/core/handlers/messagehandler.h index 28fc783..9138245 100644 --- a/core/handlers/messagehandler.h +++ b/core/handlers/messagehandler.h @@ -64,16 +64,15 @@ private: bool handleChatMessage(const QXmppMessage& msg, bool outgoing = false, bool forwarded = false, bool guessing = false); bool handleGroupMessage(const QXmppMessage& msg, bool outgoing = false, bool forwarded = false, bool guessing = false); void logMessage(const QXmppMessage& msg, const QString& reason = "Message wasn't handled: "); - void sendMessageWithLocalUploadedFile(Shared::Message msg, const QString& url); - void performSending(Shared::Message data); + void sendMessageWithLocalUploadedFile(Shared::Message msg, const QString& url, bool newMessage = true); + void performSending(Shared::Message data, bool newMessage = true); void prepareUpload(const Shared::Message& data); void handleUploadError(const QString& jid, const QString& messageId, const QString& errorText); private: Account* acc; - std::map pendingStateMessages; - std::map pendingMessages; - std::deque> uploadingSlotsQueue; + std::map pendingStateMessages; //key is message id, value is JID + std::deque> uploadingSlotsQueue; }; } diff --git a/core/rosteritem.cpp b/core/rosteritem.cpp index 9b121fb..b1951d6 100644 --- a/core/rosteritem.cpp +++ b/core/rosteritem.cpp @@ -569,3 +569,20 @@ void Core::RosterItem::downgradeDatabaseState() archiveState = ArchiveState::chunk; } } + +Shared::Message Core::RosterItem::getMessage(const QString& id) +{ + for (const Shared::Message& msg : appendCache) { + if (msg.getId() == id) { + return msg; + } + } + + for (Shared::Message& msg : hisoryCache) { + if (msg.getId() == id) { + return msg; + } + } + + return archive->getElement(id); +} diff --git a/core/rosteritem.h b/core/rosteritem.h index e744cac..237a46a 100644 --- a/core/rosteritem.h +++ b/core/rosteritem.h @@ -78,6 +78,8 @@ public: void clearArchiveRequests(); void downgradeDatabaseState(); + Shared::Message getMessage(const QString& id); + signals: void nameChanged(const QString& name); void subscriptionStateChanged(Shared::SubscriptionState state); diff --git a/shared/message.cpp b/shared/message.cpp index e63b7d2..e6b47b2 100644 --- a/shared/message.cpp +++ b/shared/message.cpp @@ -410,6 +410,14 @@ bool Shared::Message::change(const QMap& data) setEdited(true); } } + } else { + QMap::const_iterator dItr = data.find("stamp"); + if (dItr != data.end()) { + QDateTime ntime = dItr.value().toDateTime(); + if (time != ntime) { + setTime(ntime); + } + } } return idChanged; @@ -437,7 +445,7 @@ void Shared::Message::setOutOfBandUrl(const QString& url) bool Shared::Message::storable() const { - return id.size() > 0 && (body.size() > 0 || oob.size()) > 0; + return id.size() > 0 && (body.size() > 0 || oob.size() > 0 || attachPath.size() > 0); } void Shared::Message::setStanzaId(const QString& sid) diff --git a/ui/models/messagefeed.cpp b/ui/models/messagefeed.cpp index 743e64a..4187af8 100644 --- a/ui/models/messagefeed.cpp +++ b/ui/models/messagefeed.cpp @@ -194,6 +194,14 @@ std::set Models::MessageFeed::detectChanges(c roles.insert(MessageRoles::Text); roles.insert(MessageRoles::Correction); } + } else { + QMap::const_iterator dItr = data.find("stamp"); + if (dItr != data.end()) { + QDateTime ntime = dItr.value().toDateTime(); + if (msg.getTime() != ntime) { + roles.insert(MessageRoles::Date); + } + } } return roles; diff --git a/ui/utils/feedview.cpp b/ui/utils/feedview.cpp index fd9669e..22ef4c4 100644 --- a/ui/utils/feedview.cpp +++ b/ui/utils/feedview.cpp @@ -34,8 +34,8 @@ const std::set FeedView::geometryChangingRoles = { Models::MessageFeed::Attach, Models::MessageFeed::Text, Models::MessageFeed::Id, - Models::MessageFeed::Error - + Models::MessageFeed::Error, + Models::MessageFeed::Date }; FeedView::FeedView(QWidget* parent): diff --git a/ui/utils/messagedelegate.cpp b/ui/utils/messagedelegate.cpp index 0ea64d8..8db024d 100644 --- a/ui/utils/messagedelegate.cpp +++ b/ui/utils/messagedelegate.cpp @@ -397,13 +397,6 @@ QLabel * MessageDelegate::getStatusIcon(const Models::FeedItem& data) const std::map::const_iterator itr = statusIcons->find(data.id); QLabel* result = 0; - if (itr != statusIcons->end()) { - result = itr->second; - } else { - result = new QLabel(); - statusIcons->insert(std::make_pair(data.id, result)); - } - QIcon q(Shared::icon(Shared::messageStateThemeIcons[static_cast(data.state)])); QString tt = Shared::Global::getName(data.state); if (data.state == Shared::Message::State::error) { @@ -412,8 +405,23 @@ QLabel * MessageDelegate::getStatusIcon(const Models::FeedItem& data) const } } + if (itr != statusIcons->end()) { + result = itr->second; + if (result->toolTip() != tt) { //If i just assign pixmap every time unconditionally + result->setPixmap(q.pixmap(statusIconSize)); //it involves into an infinite cycle of repaint + result->setToolTip(tt); //may be it's better to subclass and store last condition in int? + } + } else { + result = new QLabel(); + statusIcons->insert(std::make_pair(data.id, result)); + result->setPixmap(q.pixmap(statusIconSize)); + result->setToolTip(tt); + } + + + result->setToolTip(tt); - result->setPixmap(q.pixmap(statusIconSize)); + //result->setText(std::to_string((int)data.state).c_str()); return result; }