From afddc1bca0f51e7ebb56c507b35ad2e9022af7e1 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Thu, 11 May 2023 20:11:07 +0800 Subject: [PATCH] Improve general readability of bulkpropagatorjob.cpp Signed-off-by: Claudio Cambra --- src/libsync/bulkpropagatorjob.cpp | 73 ++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 941c9c52b..dab9f9846 100644 --- a/src/libsync/bulkpropagatorjob.cpp +++ b/src/libsync/bulkpropagatorjob.cpp @@ -139,6 +139,7 @@ void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item, pi._errorCount = 0; pi._contentChecksum = item->_checksumHeader; pi._size = item->_size; + propagator()->_journal->setUploadInfo(item->_file, pi); propagator()->_journal->commit("Upload info"); @@ -150,13 +151,17 @@ void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item, const auto originalFilePathAbsolute = propagator()->fullLocalPath(item->_file); const auto newFilePathAbsolute = propagator()->fullLocalPath(item->_renameTarget); const auto renameSuccess = QFile::rename(originalFilePathAbsolute, newFilePathAbsolute); + if (!renameSuccess) { done(item, SyncFileItem::NormalError, "File contains trailing spaces and couldn't be renamed"); return; } + qCWarning(lcBulkPropagatorJob()) << item->_file << item->_renameTarget; + fileToUpload._file = item->_file = item->_renameTarget; fileToUpload._path = propagator()->fullLocalPath(fileToUpload._file); + item->_modtime = FileSystem::getModTime(newFilePathAbsolute); if (item->_modtime <= 0) { _pendingChecksumFiles.remove(item->_file); @@ -191,8 +196,11 @@ void BulkPropagatorJob::triggerUpload() int timeout = 0; for(auto &singleFile : _filesToUpload) { // job takes ownership of device via a QScopedPointer. Job deletes itself when finishing - auto device = std::make_unique( - singleFile._localPath, 0, singleFile._fileSize, &propagator()->_bandwidthManager); + auto device = std::make_unique(singleFile._localPath, + 0, + singleFile._fileSize, + &propagator()->_bandwidthManager); + if (!device->open(QIODevice::ReadOnly)) { qCWarning(lcBulkPropagatorJob) << "Could not prepare upload device: " << device->errorString(); @@ -207,6 +215,7 @@ void BulkPropagatorJob::triggerUpload() return; } + singleFile._headers["X-File-Path"] = singleFile._remotePath.toUtf8(); uploadParametersData.push_back({std::move(device), singleFile._headers}); timeout += singleFile._fileSize; @@ -276,8 +285,8 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item, item->_checksumHeader = transmissionChecksumHeader; - const QString fullFilePath = fileToUpload._path; - const QString originalFilePath = propagator()->fullLocalPath(item->_file); + const auto fullFilePath = fileToUpload._path; + const auto originalFilePath = propagator()->fullLocalPath(item->_file); if (!FileSystem::fileExists(fullFilePath)) { _pendingChecksumFiles.remove(item->_file); @@ -285,7 +294,8 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item, checkPropagationIsDone(); return; } - const time_t prevModtime = item->_modtime; // the _item value was set in PropagateUploadFile::start() + + const auto prevModtime = item->_modtime; // the _item value was set in PropagateUploadFile::start() // but a potential checksum calculation could have taken some time during which the file could // have been changed again, so better check again here. @@ -299,7 +309,11 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item, if (prevModtime != item->_modtime) { propagator()->_anotherSyncNeeded = true; _pendingChecksumFiles.remove(item->_file); - qDebug() << "trigger another sync after checking modified time of item" << item->_file << "prevModtime" << prevModtime << "Curr" << item->_modtime; + + qCDebug(lcBulkPropagatorJob) << "trigger another sync after checking modified time of item" << item->_file + << "prevModtime" << prevModtime + << "Curr" << item->_modtime; + slotOnErrorStartFolderUnlock(item, SyncFileItem::SoftError, tr("Local file changed during syncing. It will be resumed.")); checkPropagationIsDone(); return; @@ -334,7 +348,7 @@ void BulkPropagatorJob::slotPutFinishedOneFile(const BulkUploadItem &singleFile, PutMultiFileJob *job, const QJsonObject &fileReply) { - bool finished = false; + auto finished = false; qCInfo(lcBulkPropagatorJob()) << singleFile._item->_file << "file headers" << fileReply; @@ -396,7 +410,7 @@ void BulkPropagatorJob::slotPutFinishedOneFile(const BulkUploadItem &singleFile, void BulkPropagatorJob::slotPutFinished() { - auto *job = qobject_cast(sender()); + const auto job = qobject_cast(sender()); Q_ASSERT(job); slotJobDestroyed(job); // remove it from the _jobs list @@ -442,13 +456,13 @@ void BulkPropagatorJob::slotJobDestroyed(QObject *job) void BulkPropagatorJob::adjustLastJobTimeout(AbstractNetworkJob *job, qint64 fileSize) const { constexpr double threeMinutes = 3.0 * 60 * 1000; + const auto timeBound = qBound(job->timeoutMsec(), + // Calculate 3 minutes for each gigabyte of data + qRound64(threeMinutes * static_cast(fileSize) / 1e9), + // Maximum of 30 minutes + static_cast(30 * 60 * 1000)); - job->setTimeout(qBound( - job->timeoutMsec(), - // Calculate 3 minutes for each gigabyte of data - qRound64(threeMinutes * static_cast(fileSize) / 1e9), - // Maximum of 30 minutes - static_cast(30 * 60 * 1000))); + job->setTimeout(timeBound); } void BulkPropagatorJob::finalizeOneFile(const BulkUploadItem &oneFile) @@ -507,7 +521,11 @@ void BulkPropagatorJob::done(SyncFileItemPtr item, item->_status = status; item->_errorString = errorString; - qCInfo(lcBulkPropagatorJob) << "Item completed" << item->destination() << item->_status << item->_instruction << item->_errorString; + qCInfo(lcBulkPropagatorJob) << "Item completed" + << item->destination() + << item->_status + << item->_instruction + << item->_errorString; handleFileRestoration(item, errorString); @@ -533,6 +551,7 @@ QMap BulkPropagatorJob::headers(SyncFileItemPtr item) co QMap headers; headers[QByteArrayLiteral("Content-Type")] = QByteArrayLiteral("application/octet-stream"); headers[QByteArrayLiteral("X-File-Mtime")] = QByteArray::number(qint64(item->_modtime)); + if (qEnvironmentVariableIntValue("OWNCLOUD_LAZYOPS")) { headers[QByteArrayLiteral("OC-LazyOps")] = QByteArrayLiteral("true"); } @@ -589,16 +608,17 @@ void BulkPropagatorJob::checkResettingErrors(SyncFileItemPtr item) const { if (item->_httpErrorCode == 412 || propagator()->account()->capabilities().httpErrorCodesThatResetFailingChunkedUploads().contains(item->_httpErrorCode)) { + auto uploadInfo = propagator()->_journal->getUploadInfo(item->_file); uploadInfo._errorCount += 1; if (uploadInfo._errorCount > 3) { qCInfo(lcBulkPropagatorJob) << "Reset transfer of" << item->_file - << "due to repeated error" << item->_httpErrorCode; + << "due to repeated error" << item->_httpErrorCode; uploadInfo = SyncJournalDb::UploadInfo(); } else { qCInfo(lcBulkPropagatorJob) << "Error count for maybe-reset error" << item->_httpErrorCode - << "on file" << item->_file - << "is" << uploadInfo._errorCount; + << "on file" << item->_file + << "is" << uploadInfo._errorCount; } propagator()->_journal->setUploadInfo(item->_file, uploadInfo); propagator()->_journal->commit("Upload info"); @@ -610,7 +630,6 @@ void BulkPropagatorJob::commonErrorHandling(SyncFileItemPtr item, { // Ensure errors that should eventually reset the chunked upload are tracked. checkResettingErrors(item); - abortWithError(item, SyncFileItem::NormalError, errorMessage); } @@ -636,6 +655,7 @@ bool BulkPropagatorJob::checkFileChanged(SyncFileItemPtr item, { if (!FileSystem::verifyFileUnchanged(fullFilePath, item->_size, item->_modtime)) { propagator()->_anotherSyncNeeded = true; + if (!finished) { abortWithError(item, SyncFileItem::SoftError, tr("Local file changed during sync.")); // FIXME: the legacy code was retrying for a few seconds. @@ -669,10 +689,8 @@ void BulkPropagatorJob::handleFileRestoration(SyncFileItemPtr item, } else { item->_errorString += tr("Restoration failed: %1").arg(errorString); } - } else { - if (item->_errorString.isEmpty()) { - item->_errorString = errorString; - } + } else if (item->_errorString.isEmpty()) { + item->_errorString = errorString; } } @@ -685,9 +703,14 @@ void BulkPropagatorJob::handleJobDoneErrors(SyncFileItemPtr item, SyncFileItem::Status status) { if (item->hasErrorStatus()) { - qCWarning(lcPropagator) << "Could not complete propagation of" << item->destination() << "by" << this << "with status" << item->_status << "and error:" << item->_errorString; + qCWarning(lcPropagator) << "Could not complete propagation of" << item->destination() + << "by" << this + << "with status" << item->_status + << "and error:" << item->_errorString; } else { - qCInfo(lcPropagator) << "Completed propagation of" << item->destination() << "by" << this << "with status" << item->_status; + qCInfo(lcPropagator) << "Completed propagation of" << item->destination() + << "by" << this + << "with status" << item->_status; } if (item->_status == SyncFileItem::FatalError) { -- 2.30.2