From 4307262f6eb4a85b8996cf3bae91ea63976e1b7d Mon Sep 17 00:00:00 2001 From: blue Date: Fri, 14 May 2021 22:49:38 +0300 Subject: [PATCH] basic error download/upload files handling, need more testing --- core/networkaccess.cpp | 76 +++++++++++++++----- core/networkaccess.h | 1 + ui/models/messagefeed.cpp | 132 ++++++++++++++++++++++++++++------- ui/models/messagefeed.h | 6 +- ui/utils/feedview.cpp | 9 +-- ui/utils/feedview.h | 2 +- ui/utils/messagedelegate.cpp | 54 ++++++++------ ui/utils/messagedelegate.h | 3 +- 8 files changed, 206 insertions(+), 77 deletions(-) diff --git a/core/networkaccess.cpp b/core/networkaccess.cpp index eece379..69fe812 100644 --- a/core/networkaccess.cpp +++ b/core/networkaccess.cpp @@ -70,6 +70,9 @@ void Core::NetworkAccess::start() { if (!running) { manager = new QNetworkAccessManager(); +#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0) + manager->setTransferTimeout(); +#endif storage.open(); running = true; } @@ -99,31 +102,56 @@ void Core::NetworkAccess::onDownloadProgress(qint64 bytesReceived, qint64 bytesT qDebug() << "an error downloading" << url << ": the request had some progress but seems like no one is waiting for it, skipping"; } else { Transfer* dwn = itr->second; - qreal received = bytesReceived; - qreal total = bytesTotal; - qreal progress = received/total; - dwn->progress = progress; - emit loadFileProgress(dwn->messages, progress, false); + if (dwn->success) { + qreal received = bytesReceived; + qreal total = bytesTotal; + qreal progress = received/total; + dwn->progress = progress; + emit loadFileProgress(dwn->messages, progress, false); + } } } void Core::NetworkAccess::onDownloadError(QNetworkReply::NetworkError code) { + qDebug() << "DEBUG: DOWNLOAD ERROR"; QNetworkReply* rpl = static_cast(sender()); + qDebug() << rpl->errorString(); QString url = rpl->url().toString(); std::map::const_iterator itr = downloads.find(url); if (itr == downloads.end()) { qDebug() << "an error downloading" << url << ": the request is reporting an error but seems like no one is waiting for it, skipping"; } else { QString errorText = getErrorText(code); - if (errorText.size() > 0) { + //if (errorText.size() > 0) { itr->second->success = false; Transfer* dwn = itr->second; emit loadFileError(dwn->messages, errorText, false); - } + //} } } +void Core::NetworkAccess::onDownloadSSLError(const QList& errors) +{ + qDebug() << "DEBUG: DOWNLOAD SSL ERRORS"; + for (const QSslError& err : errors) { + qDebug() << err.errorString(); + } + QNetworkReply* rpl = static_cast(sender()); + QString url = rpl->url().toString(); + std::map::const_iterator itr = downloads.find(url); + if (itr == downloads.end()) { + qDebug() << "an SSL error downloading" << url << ": the request is reporting an error but seems like no one is waiting for it, skipping"; + } else { + //if (errorText.size() > 0) { + itr->second->success = false; + Transfer* dwn = itr->second; + emit loadFileError(dwn->messages, "SSL errors occured", false); + //} + } +} + + QString Core::NetworkAccess::getErrorText(QNetworkReply::NetworkError code) { QString errorText(""); @@ -146,7 +174,11 @@ QString Core::NetworkAccess::getErrorText(QNetworkReply::NetworkError code) errorText = "Connection was closed because it timed out"; break; case QNetworkReply::OperationCanceledError: - //this means I closed it myself by abort() or close(), don't think I need to notify here + //this means I closed it myself by abort() or close() + //I don't call them directory, but this is the error code + //Qt returns when it can not resume donwload after the network failure + //or when the download is canceled by the timout; + errorText = "Connection lost"; break; case QNetworkReply::SslHandshakeFailedError: errorText = "Security error"; //TODO need to handle sslErrors signal to get a better description here @@ -247,6 +279,7 @@ QString Core::NetworkAccess::getErrorText(QNetworkReply::NetworkError code) void Core::NetworkAccess::onDownloadFinished() { + qDebug() << "DEBUG: DOWNLOAD FINISHED"; QNetworkReply* rpl = static_cast(sender()); QString url = rpl->url().toString(); std::map::const_iterator itr = downloads.find(url); @@ -256,11 +289,14 @@ void Core::NetworkAccess::onDownloadFinished() Transfer* dwn = itr->second; if (dwn->success) { qDebug() << "download success for" << url; + QString err; QStringList hops = url.split("/"); QString fileName = hops.back(); QString jid; if (dwn->messages.size() > 0) { jid = dwn->messages.front().jid; + } else { + qDebug() << "An attempt to save the file but it doesn't seem to belong to any message, download is definately going to be broken"; } QString path = prepareDirectory(jid); if (path.size() > 0) { @@ -274,15 +310,16 @@ void Core::NetworkAccess::onDownloadFinished() qDebug() << "file" << path << "was successfully downloaded"; } else { qDebug() << "couldn't save file" << path; - path = QString(); + err = "Error opening file to write:" + file.errorString(); } + } else { + err = "Couldn't prepare a directory for file"; } if (path.size() > 0) { emit downloadFileComplete(dwn->messages, path); } else { - //TODO do I need to handle the failure here or it's already being handled in error? - //emit loadFileError(dwn->messages, path, false); + emit loadFileError(dwn->messages, "Error saving file " + url + "; " + err, false); } } @@ -298,6 +335,7 @@ void Core::NetworkAccess::startDownload(const std::list& ms QNetworkRequest req(url); dwn->reply = manager->get(req); connect(dwn->reply, &QNetworkReply::downloadProgress, this, &NetworkAccess::onDownloadProgress); + connect(dwn->reply, &QNetworkReply::sslErrors, this, &NetworkAccess::onDownloadSSLError); #if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0) connect(dwn->reply, qOverload(&QNetworkReply::errorOccurred), this, &NetworkAccess::onDownloadError); #else @@ -317,11 +355,11 @@ void Core::NetworkAccess::onUploadError(QNetworkReply::NetworkError code) qDebug() << "an error uploading" << url << ": the request is reporting an error but there is no record of it being uploading, ignoring"; } else { QString errorText = getErrorText(code); - if (errorText.size() > 0) { + //if (errorText.size() > 0) { itr->second->success = false; Transfer* upl = itr->second; emit loadFileError(upl->messages, errorText, true); - } + //} //TODO deletion? } @@ -360,11 +398,13 @@ void Core::NetworkAccess::onUploadProgress(qint64 bytesReceived, qint64 bytesTot qDebug() << "an error downloading" << url << ": the request had some progress but seems like no one is waiting for it, skipping"; } else { Transfer* upl = itr->second; - qreal received = bytesReceived; - qreal total = bytesTotal; - qreal progress = received/total; - upl->progress = progress; - emit loadFileProgress(upl->messages, progress, true); + if (upl->success) { + qreal received = bytesReceived; + qreal total = bytesTotal; + qreal progress = received/total; + upl->progress = progress; + emit loadFileProgress(upl->messages, progress, true); + } } } diff --git a/core/networkaccess.h b/core/networkaccess.h index 5b9eae2..75c189c 100644 --- a/core/networkaccess.h +++ b/core/networkaccess.h @@ -75,6 +75,7 @@ private: private slots: void onDownloadProgress(qint64 bytesReceived, qint64 bytesTotal); void onDownloadError(QNetworkReply::NetworkError code); + void onDownloadSSLError(const QList &errors); void onDownloadFinished(); void onUploadProgress(qint64 bytesReceived, qint64 bytesTotal); void onUploadError(QNetworkReply::NetworkError code); diff --git a/ui/models/messagefeed.cpp b/ui/models/messagefeed.cpp index d5fb3bc..c64d7ab 100644 --- a/ui/models/messagefeed.cpp +++ b/ui/models/messagefeed.cpp @@ -45,6 +45,8 @@ Models::MessageFeed::MessageFeed(const Element* ri, QObject* parent): syncState(incomplete), uploads(), downloads(), + failedDownloads(), + failedUploads(), unreadMessages(new std::set()), observersAmount(0) { @@ -142,6 +144,18 @@ void Models::MessageFeed::changeMessage(const QString& id, const QMap cr; for (MessageRoles role : changeRoles) { cr.push_back(role); @@ -421,6 +435,7 @@ bool Models::MessageFeed::sentByMe(const Shared::Message& msg) const Models::Attachment Models::MessageFeed::fillAttach(const Shared::Message& msg) const { ::Models::Attachment att; + QString id = msg.getId(); att.localPath = msg.getAttachPath(); att.remotePath = msg.getOutOfBandUrl(); @@ -429,22 +444,34 @@ Models::Attachment Models::MessageFeed::fillAttach(const Shared::Message& msg) c if (att.localPath.size() == 0) { att.state = none; } else { - Progress::const_iterator itr = uploads.find(msg.getId()); - if (itr == uploads.end()) { - att.state = local; + Err::const_iterator eitr = failedUploads.find(id); + if (eitr != failedUploads.end()) { + att.state = errorUpload; + att.error = eitr->second; } else { - att.state = uploading; - att.progress = itr->second; + Progress::const_iterator itr = uploads.find(id); + if (itr == uploads.end()) { + att.state = local; + } else { + att.state = uploading; + att.progress = itr->second; + } } } } else { if (att.localPath.size() == 0) { - Progress::const_iterator itr = downloads.find(msg.getId()); - if (itr == downloads.end()) { - att.state = remote; + Err::const_iterator eitr = failedDownloads.find(id); + if (eitr != failedDownloads.end()) { + att.state = errorDownload; + att.error = eitr->second; } else { - att.state = downloading; - att.progress = itr->second; + Progress::const_iterator itr = downloads.find(id); + if (itr == downloads.end()) { + att.state = remote; + } else { + att.state = downloading; + att.progress = itr->second; + } } } else { att.state = ready; @@ -456,12 +483,19 @@ Models::Attachment Models::MessageFeed::fillAttach(const Shared::Message& msg) c void Models::MessageFeed::downloadAttachment(const QString& messageId) { + bool notify = false; + Err::const_iterator eitr = failedDownloads.find(messageId); + if (eitr != failedDownloads.end()) { + failedDownloads.erase(eitr); + notify = true; + } + QModelIndex ind = modelIndexById(messageId); if (ind.isValid()) { std::pair progressPair = downloads.insert(std::make_pair(messageId, 0)); if (progressPair.second) { //Only to take action if we weren't already downloading it Shared::Message* msg = static_cast(ind.internalPointer()); - emit dataChanged(ind, ind, {MessageRoles::Attach}); + notify = true; emit fileDownloadRequest(msg->getOutOfBandUrl()); } else { qDebug() << "Attachment download for message with id" << messageId << "is already in progress, skipping"; @@ -469,32 +503,55 @@ void Models::MessageFeed::downloadAttachment(const QString& messageId) } else { qDebug() << "An attempt to download an attachment for the message that doesn't exist. ID:" << messageId; } -} - -void Models::MessageFeed::uploadAttachment(const QString& messageId) -{ - qDebug() << "request to upload attachment of the message" << messageId; + + if (notify) { + emit dataChanged(ind, ind, {MessageRoles::Attach}); + } } bool Models::MessageFeed::registerUpload(const QString& messageId) { - return uploads.insert(std::make_pair(messageId, 0)).second; + bool success = uploads.insert(std::make_pair(messageId, 0)).second; + + QVector roles({}); + Err::const_iterator eitr = failedUploads.find(messageId); + if (eitr != failedUploads.end()) { + failedUploads.erase(eitr); + roles.push_back(MessageRoles::Attach); + } else if (success) { + roles.push_back(MessageRoles::Attach); + } + + QModelIndex ind = modelIndexById(messageId); + emit dataChanged(ind, ind, roles); + + return success; } void Models::MessageFeed::fileProgress(const QString& messageId, qreal value, bool up) { Progress* pr = 0; + Err* err = 0; if (up) { pr = &uploads; + err = &failedUploads; } else { pr = &downloads; + err = &failedDownloads; + } + + QVector roles({}); + Err::const_iterator eitr = err->find(messageId); + if (eitr != err->end() && value != 1) { //like I want to clear this state when the download is started anew + err->erase(eitr); + roles.push_back(MessageRoles::Attach); } Progress::iterator itr = pr->find(messageId); if (itr != pr->end()) { itr->second = value; QModelIndex ind = modelIndexById(messageId); - emit dataChanged(ind, ind); //the type of the attach didn't change, so, there is no need to relayout, there is no role in event + emit dataChanged(ind, ind, roles); } } @@ -505,7 +562,29 @@ void Models::MessageFeed::fileComplete(const QString& messageId, bool up) void Models::MessageFeed::fileError(const QString& messageId, const QString& error, bool up) { - //TODO + Err* failed; + Progress* loads; + if (up) { + failed = &failedUploads; + loads = &uploads; + } else { + failed = &failedDownloads; + loads = &downloads; + } + + Progress::iterator pitr = loads->find(messageId); + if (pitr != loads->end()) { + loads->erase(pitr); + } + + std::pair pair = failed->insert(std::make_pair(messageId, error)); + if (!pair.second) { + pair.first->second = error; + } + QModelIndex ind = modelIndexById(messageId); + if (ind.isValid()) { + emit dataChanged(ind, ind, {MessageRoles::Attach}); + } } void Models::MessageFeed::incrementObservers() @@ -533,19 +612,18 @@ QModelIndex Models::MessageFeed::modelIndexById(const QString& id) const QModelIndex Models::MessageFeed::modelIndexByTime(const QString& id, const QDateTime& time) const { if (indexByTime.size() > 0) { - StorageByTime::const_iterator tItr = indexByTime.upper_bound(time); - StorageByTime::const_iterator tBeg = indexByTime.begin(); - StorageByTime::const_iterator tEnd = indexByTime.end(); + StorageByTime::const_iterator tItr = indexByTime.lower_bound(time); + StorageByTime::const_iterator tEnd = indexByTime.upper_bound(time); bool found = false; - while (tItr != tBeg) { - if (tItr != tEnd && id == (*tItr)->getId()) { + while (tItr != tEnd) { + if (id == (*tItr)->getId()) { found = true; break; } - --tItr; + ++tItr; } - if (found && tItr != tEnd && id == (*tItr)->getId()) { + if (found) { int position = indexByTime.rank(tItr); return createIndex(position, 0, *tItr); } @@ -566,7 +644,7 @@ void Models::MessageFeed::reportLocalPathInvalid(const QString& messageId) emit localPathInvalid(msg->getAttachPath()); - //gonna change the message in current model right away, to prevent spam on each attemt to draw element + //gonna change the message in current model right away, to prevent spam on each attempt to draw element QModelIndex index = modelIndexByTime(messageId, msg->getTime()); msg->setAttachPath(""); diff --git a/ui/models/messagefeed.h b/ui/models/messagefeed.h index abf67ee..efb005a 100644 --- a/ui/models/messagefeed.h +++ b/ui/models/messagefeed.h @@ -65,7 +65,6 @@ public: void responseArchive(const std::list list, bool last); void downloadAttachment(const QString& messageId); - void uploadAttachment(const QString& messageId); bool registerUpload(const QString& messageId); void reportLocalPathInvalid(const QString& messageId); @@ -148,12 +147,16 @@ private: SyncState syncState; typedef std::map Progress; + typedef std::map Err; Progress uploads; Progress downloads; + Err failedDownloads; + Err failedUploads; std::set* unreadMessages; uint16_t observersAmount; + static const QHash roles; }; @@ -173,6 +176,7 @@ struct Attachment { qreal progress; QString localPath; QString remotePath; + QString error; }; struct FeedItem { diff --git a/ui/utils/feedview.cpp b/ui/utils/feedview.cpp index 22ef4c4..5f515aa 100644 --- a/ui/utils/feedview.cpp +++ b/ui/utils/feedview.cpp @@ -394,16 +394,11 @@ void FeedView::setModel(QAbstractItemModel* p_model) } } -void FeedView::onMessageButtonPushed(const QString& messageId, bool download) +void FeedView::onMessageButtonPushed(const QString& messageId) { if (specialModel) { Models::MessageFeed* feed = static_cast(model()); - - if (download) { - feed->downloadAttachment(messageId); - } else { - feed->uploadAttachment(messageId); - } + feed->downloadAttachment(messageId); } } diff --git a/ui/utils/feedview.h b/ui/utils/feedview.h index 0b7e7d9..f5509fd 100644 --- a/ui/utils/feedview.h +++ b/ui/utils/feedview.h @@ -58,7 +58,7 @@ protected slots: void rowsInserted(const QModelIndex & parent, int start, int end) override; void verticalScrollbarValueChanged(int value) override; void dataChanged(const QModelIndex & topLeft, const QModelIndex & bottomRight, const QVector & roles) override; - void onMessageButtonPushed(const QString& messageId, bool download); + void onMessageButtonPushed(const QString& messageId); void onMessageInvalidPath(const QString& messageId); void onModelSyncStateChange(Models::MessageFeed::SyncState state); diff --git a/ui/utils/messagedelegate.cpp b/ui/utils/messagedelegate.cpp index 6b459f2..0381ae3 100644 --- a/ui/utils/messagedelegate.cpp +++ b/ui/utils/messagedelegate.cpp @@ -137,19 +137,39 @@ void MessageDelegate::paint(QPainter* painter, const QStyleOptionViewItem& optio clearHelperWidget(data); //i can't imagine the situation where it's gonna be needed break; //but it's a possible performance problem case Models::uploading: + paintPreview(data, painter, opt); case Models::downloading: paintBar(getBar(data), painter, data.sentByMe, opt); break; case Models::remote: - case Models::local: paintButton(getButton(data), painter, data.sentByMe, opt); break; case Models::ready: + case Models::local: clearHelperWidget(data); paintPreview(data, painter, opt); break; - case Models::errorDownload: - case Models::errorUpload: + case Models::errorDownload: { + paintButton(getButton(data), painter, data.sentByMe, opt); + painter->setFont(dateFont); + QColor q = painter->pen().color(); + q.setAlpha(180); + painter->setPen(q); + painter->drawText(opt.rect, opt.displayAlignment, data.attach.error, &rect); + opt.rect.adjust(0, rect.height() + textMargin, 0, 0); + } + + break; + case Models::errorUpload:{ + clearHelperWidget(data); + paintPreview(data, painter, opt); + painter->setFont(dateFont); + QColor q = painter->pen().color(); + q.setAlpha(180); + painter->setPen(q); + painter->drawText(opt.rect, opt.displayAlignment, data.attach.error, &rect); + opt.rect.adjust(0, rect.height() + textMargin, 0, 0); + } break; } painter->restore(); @@ -212,18 +232,24 @@ QSize MessageDelegate::sizeHint(const QStyleOptionViewItem& option, const QModel case Models::none: break; case Models::uploading: + messageSize.rheight() += calculateAttachSize(attach.localPath, messageRect).height() + textMargin; case Models::downloading: messageSize.rheight() += barHeight + textMargin; break; case Models::remote: - case Models::local: messageSize.rheight() += buttonHeight + textMargin; break; case Models::ready: + case Models::local: messageSize.rheight() += calculateAttachSize(attach.localPath, messageRect).height() + textMargin; break; case Models::errorDownload: + messageSize.rheight() += buttonHeight + textMargin; + messageSize.rheight() += dateMetrics.boundingRect(messageRect, Qt::TextWordWrap, attach.error).size().height() + textMargin; + break; case Models::errorUpload: + messageSize.rheight() += calculateAttachSize(attach.localPath, messageRect).height() + textMargin; + messageSize.rheight() += dateMetrics.boundingRect(messageRect, Qt::TextWordWrap, attach.error).size().height() + textMargin; break; } @@ -356,15 +382,7 @@ QPushButton * MessageDelegate::getButton(const Models::FeedItem& data) const std::map::const_iterator itr = buttons->find(data.id); FeedButton* result = 0; if (itr != buttons->end()) { - if ( - (data.attach.state == Models::remote && itr->second->download) || - (data.attach.state == Models::local && !itr->second->download) - ) { - result = itr->second; - } else { - delete itr->second; - buttons->erase(itr); - } + result = itr->second; } else { std::map::const_iterator barItr = bars->find(data.id); if (barItr != bars->end()) { @@ -376,13 +394,7 @@ QPushButton * MessageDelegate::getButton(const Models::FeedItem& data) const if (result == 0) { result = new FeedButton(); result->messageId = data.id; - if (data.attach.state == Models::remote) { - result->setText(QCoreApplication::translate("MessageLine", "Download")); - result->download = true; - } else { - result->setText(QCoreApplication::translate("MessageLine", "Upload")); - result->download = false; - } + result->setText(QCoreApplication::translate("MessageLine", "Download")); buttons->insert(std::make_pair(data.id, result)); connect(result, &QPushButton::clicked, this, &MessageDelegate::onButtonPushed); } @@ -529,7 +541,7 @@ void MessageDelegate::endClearWidgets() void MessageDelegate::onButtonPushed() const { FeedButton* btn = static_cast(sender()); - emit buttonPushed(btn->messageId, btn->download); + emit buttonPushed(btn->messageId); } void MessageDelegate::clearHelperWidget(const Models::FeedItem& data) const diff --git a/ui/utils/messagedelegate.h b/ui/utils/messagedelegate.h index 6a257b7..3af80e1 100644 --- a/ui/utils/messagedelegate.h +++ b/ui/utils/messagedelegate.h @@ -55,7 +55,7 @@ public: void beginClearWidgets(); signals: - void buttonPushed(const QString& messageId, bool download) const; + void buttonPushed(const QString& messageId) const; void invalidPath(const QString& messageId) const; protected: @@ -77,7 +77,6 @@ private: class FeedButton : public QPushButton { public: QString messageId; - bool download; }; QFont bodyFont;