diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index 579289df5..d2f880112 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -898,7 +898,7 @@ qint64 SyncJournalDb::getPHash(const QByteArray &file) return h; } -bool SyncJournalDb::setFileRecord(const SyncJournalFileRecord &_record) +Result SyncJournalDb::setFileRecord(const SyncJournalFileRecord &_record) { SyncJournalFileRecord record = _record; QMutexLocker locker(&_mutex); @@ -940,7 +940,7 @@ bool SyncJournalDb::setFileRecord(const SyncJournalFileRecord &_record) "INSERT OR REPLACE INTO metadata " "(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId, e2eMangledName, isE2eEncrypted) " "VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7, ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18);"), _db)) { - return false; + return _setFileRecordQuery.error(); } _setFileRecordQuery.bindValue(1, phash); @@ -963,16 +963,16 @@ bool SyncJournalDb::setFileRecord(const SyncJournalFileRecord &_record) _setFileRecordQuery.bindValue(18, record._isE2eEncrypted); if (!_setFileRecordQuery.exec()) { - return false; + return _setFileRecordQuery.error(); } // Can't be true anymore. _metadataTableIsEmpty = false; - return true; + return {}; } else { qCWarning(lcDb) << "Failed to connect database."; - return false; // checkConnect failed. + return tr("Failed to connect database."); // checkConnect failed. } } diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h index cbfa8d387..aa949752a 100644 --- a/src/common/syncjournaldb.h +++ b/src/common/syncjournaldb.h @@ -65,7 +65,7 @@ public: bool getFileRecordsByFileId(const QByteArray &fileId, const std::function &rowCallback); bool getFilesBelowPath(const QByteArray &path, const std::function &rowCallback); bool listFilesInPath(const QByteArray &path, const std::function &rowCallback); - bool setFileRecord(const SyncJournalFileRecord &record); + Result setFileRecord(const SyncJournalFileRecord &record); void keyValueStoreSet(const QString &key, QVariant value); qint64 keyValueStoreGetInt(const QString &key, qint64 defaultValue); diff --git a/src/common/vfs.h b/src/common/vfs.h index ba2c5c7af..1435f2633 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -104,6 +104,13 @@ public: XAttr, }; Q_ENUM(Mode) + enum class ConvertToPlaceholderResult { + Error, + Ok, + Locked + }; + Q_ENUM(ConvertToPlaceholderResult) + static QString modeToString(Mode mode); static Optional modeFromString(const QString &str); @@ -197,7 +204,7 @@ public: * new placeholder shall supersede, for rename-replace actions with new downloads, * for example. */ - virtual Q_REQUIRED_RESULT Result convertToPlaceholder( + virtual Q_REQUIRED_RESULT Result convertToPlaceholder( const QString &filename, const SyncFileItem &item, const QString &replacesFile = QString()) = 0; @@ -303,7 +310,7 @@ public: Result updateMetadata(const QString &, time_t, qint64, const QByteArray &) override { return {}; } Result createPlaceholder(const SyncFileItem &) override { return {}; } Result dehydratePlaceholder(const SyncFileItem &) override { return {}; } - Result convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override { return {}; } + Result convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override { return ConvertToPlaceholderResult::Ok; } bool needsMetadataUpdate(const SyncFileItem &) override { return false; } bool isDehydratedPlaceholder(const QString &) override { return false; } diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index dcc5dc7d6..7d8250d0a 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -756,19 +756,27 @@ QString OwncloudPropagator::adjustRenamedPath(const QString &original) const return OCC::adjustRenamedPath(_renamedDirectories, original); } -bool OwncloudPropagator::updateMetadata(const SyncFileItem &item, const QString &localFolderPath, SyncJournalDb &journal, Vfs &vfs) +Result OwncloudPropagator::updateMetadata(const SyncFileItem &item) { - QString fsPath = localFolderPath + item.destination(); - if (!vfs.convertToPlaceholder(fsPath, item)) { - return false; - } - auto record = item.toSyncJournalFileRecordWithInode(fsPath); - return journal.setFileRecord(record); + return OwncloudPropagator::staticUpdateMetadata(item, _localDir, syncOptions()._vfs.data(), _journal); } -bool OwncloudPropagator::updateMetadata(const SyncFileItem &item) +Result OwncloudPropagator::staticUpdateMetadata(const SyncFileItem &item, const QString localDir, + Vfs *vfs, SyncJournalDb *const journal) { - return updateMetadata(item, _localDir, *_journal, *syncOptions()._vfs); + const QString fsPath = localDir + item.destination(); + const auto result = vfs->convertToPlaceholder(fsPath, item); + if (!result) { + return result.error(); + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + return Vfs::ConvertToPlaceholderResult::Locked; + } + auto record = item.toSyncJournalFileRecordWithInode(fsPath); + const auto dBresult = journal->setFileRecord(record); + if (!dBresult) { + return dBresult.error(); + } + return Vfs::ConvertToPlaceholderResult::Ok; } // ================================================================================ @@ -1011,10 +1019,14 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) if (_item->_instruction == CSYNC_INSTRUCTION_RENAME || _item->_instruction == CSYNC_INSTRUCTION_NEW || _item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA) { - if (!propagator()->updateMetadata(*_item)) { + const auto result = propagator()->updateMetadata(*_item); + if (!result) { status = _item->_status = SyncFileItem::FatalError; - _item->_errorString = tr("Error writing metadata to the database"); - qCWarning(lcDirectory) << "Error writing to the database for file" << _item->_file; + _item->_errorString = tr("Error updating metadata: %1").arg(result.error()); + qCWarning(lcDirectory) << "Error writing to the database for file" << _item->_file << "with" << result.error(); + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + _item->_status = SyncFileItem::SoftError; + _item->_errorString = tr("File is currently in use"); } } } @@ -1141,7 +1153,7 @@ void CleanupPollsJob::slotPollFinished() } else if (job->_item->_status != SyncFileItem::Success) { qCWarning(lcCleanupPolls) << "There was an error with file " << job->_item->_file << job->_item->_errorString; } else { - if (!OwncloudPropagator::updateMetadata(*job->_item, _localPath, *_journal, *_vfs)) { + if (!OwncloudPropagator::staticUpdateMetadata(*job->_item, _localPath, _vfs.data(), _journal)) { qCWarning(lcCleanupPolls) << "database error"; job->_item->_status = SyncFileItem::FatalError; job->_item->_errorString = tr("Error writing metadata to the database"); diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index cbb4ec2ae..c2df7749f 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -560,8 +560,17 @@ public: * * Will also trigger a Vfs::convertToPlaceholder. */ - static bool updateMetadata(const SyncFileItem &item, const QString &localFolderPath, SyncJournalDb &journal, Vfs &vfs); - bool updateMetadata(const SyncFileItem &item); // convenience for the above + Result updateMetadata(const SyncFileItem &item); + + /** Update the database for an item. + * + * Typically after a sync operation succeeded. Updates the inode from + * the filesystem. + * + * Will also trigger a Vfs::convertToPlaceholder. + */ + static Result staticUpdateMetadata(const SyncFileItem &item, const QString localDir, + Vfs *vfs, SyncJournalDb * const journal); private slots: @@ -626,8 +635,8 @@ class CleanupPollsJob : public QObject QSharedPointer _vfs; public: - explicit CleanupPollsJob(const QVector &pollInfos, AccountPtr account, - SyncJournalDb *journal, const QString &localPath, const QSharedPointer &vfs, QObject *parent = nullptr) + explicit CleanupPollsJob(const QVector &pollInfos, AccountPtr account, SyncJournalDb *journal, const QString &localPath, + const QSharedPointer &vfs, QObject *parent = nullptr) : QObject(parent) , _pollInfos(pollInfos) , _account(account) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 54f01b5f5..8fb05024b 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -1135,10 +1135,13 @@ void PropagateDownloadFile::downloadFinished() void PropagateDownloadFile::updateMetadata(bool isConflict) { - QString fn = propagator()->fullLocalPath(_item->_file); - - if (!propagator()->updateMetadata(*_item)) { - done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); + const QString fn = propagator()->fullLocalPath(_item->_file); + const auto result = propagator()->updateMetadata(*_item); + if (!result) { + done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error())); + return; + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + done(SyncFileItem::SoftError, tr("The file %1 is currently in use").arg(_item->_file)); return; } diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp index baf36fd72..0809617dc 100644 --- a/src/libsync/propagateremotemkdir.cpp +++ b/src/libsync/propagateremotemkdir.cpp @@ -259,8 +259,12 @@ void PropagateRemoteMkdir::success() itemCopy._etag.clear(); // save the file id already so we can detect rename or remove - if (!propagator()->updateMetadata(itemCopy)) { - done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); + const auto result = propagator()->updateMetadata(itemCopy); + if (!result) { + done(SyncFileItem::FatalError, tr("Error writing metadata to the database: %1").arg(result.error())); + return; + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + done(SyncFileItem::FatalError, tr("The file %1 is currently in use").arg(_item->_file)); return; } diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 7514123e0..e65a5d4aa 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -252,8 +252,12 @@ void PropagateRemoteMove::finalize() newItem._size = oldRecord._fileSize; } } - if (!propagator()->updateMetadata(newItem)) { - done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); + const auto result = propagator()->updateMetadata(newItem); + if (!result) { + done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error())); + return; + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + done(SyncFileItem::SoftError, tr("The file %1 is currently in use").arg(newItem._file)); return; } if (pinState && *pinState != PinState::Inherited diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 780bab012..dc7fc766c 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -765,8 +765,12 @@ void PropagateUploadFileCommon::finalize() quotaIt.value() -= _fileToUpload._size; // Update the database entry - if (!propagator()->updateMetadata(*_item)) { - done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); + const auto result = propagator()->updateMetadata(*_item); + if (!result) { + done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error())); + return; + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + done(SyncFileItem::SoftError, tr("The file %1 is currently in use").arg(_item->_file)); return; } diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 3a6416fa0..b651aec81 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -189,8 +189,12 @@ void PropagateLocalMkdir::startLocalMkdir() // before the correct etag is stored. SyncFileItem newItem(*_item); newItem._etag = "_invalid_"; - if (!propagator()->updateMetadata(newItem)) { - done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); + const auto result = propagator()->updateMetadata(newItem); + if (!result) { + done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error())); + return; + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + done(SyncFileItem::SoftError, tr("The file %1 is currently in use").arg(newItem._file)); return; } propagator()->_journal->commit("localMkdir"); @@ -253,14 +257,18 @@ void PropagateLocalRename::start() if (oldRecord.isValid()) { newItem._checksumHeader = oldRecord._checksumHeader; } - if (!propagator()->updateMetadata(newItem)) { - done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); + const auto result = propagator()->updateMetadata(newItem); + if (!result) { + done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error())); + return; + } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) { + done(SyncFileItem::SoftError, tr("The file %1 is currently in use").arg(newItem._file)); return; } } else { propagator()->_renamedDirectories.insert(oldFile, _item->_renameTarget); if (!PropagateRemoteMove::adjustSelectiveSync(propagator()->_journal, oldFile, _item->_renameTarget)) { - done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); + done(SyncFileItem::FatalError, tr("Failed to rename file")); return; } } diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp index 81f4c3400..189ef1c6e 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp +++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp @@ -622,14 +622,14 @@ OCC::CfApiWrapper::PlaceHolderInfo OCC::CfApiWrapper::findPlaceholderInfo(const } } -OCC::Result OCC::CfApiWrapper::setPinState(const FileHandle &handle, OCC::PinStateEnums::PinState state, SetPinRecurseMode mode) +OCC::Result OCC::CfApiWrapper::setPinState(const FileHandle &handle, OCC::PinStateEnums::PinState state, SetPinRecurseMode mode) { const auto cfState = pinStateToCfPinState(state); const auto flags = pinRecurseModeToCfSetPinFlags(mode); const qint64 result = CfSetPinState(handle.get(), cfState, flags, nullptr); if (result == S_OK) { - return {}; + return OCC::Vfs::ConvertToPlaceholderResult::Ok; } else { qCWarning(lcCfApiWrapper) << "Couldn't set pin state" << state << "for" << pathForHandle(handle) << "with recurse mode" << mode << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage()); return { "Couldn't set pin state" }; @@ -682,7 +682,7 @@ OCC::Result OCC::CfApiWrapper::createPlaceholderInfo(const QStrin return {}; } -OCC::Result OCC::CfApiWrapper::updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath) +OCC::Result OCC::CfApiWrapper::updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath) { Q_ASSERT(handle); @@ -717,10 +717,10 @@ OCC::Result OCC::CfApiWrapper::updatePlaceholderInfo(const FileHa return { "Couldn't restore pin state" }; } - return {}; + return OCC::Vfs::ConvertToPlaceholderResult::Ok; } -OCC::Result OCC::CfApiWrapper::convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath) +OCC::Result OCC::CfApiWrapper::convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath) { Q_UNUSED(modtime); Q_UNUSED(size); diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.h b/src/libsync/vfs/cfapi/cfapiwrapper.h index 2e8810b45..d81dfe936 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.h +++ b/src/libsync/vfs/cfapi/cfapiwrapper.h @@ -18,6 +18,7 @@ #include "owncloudlib.h" #include "common/pinstate.h" #include "common/result.h" +#include "common/vfs.h" struct CF_PLACEHOLDER_BASIC_INFO; @@ -89,10 +90,10 @@ enum SetPinRecurseMode { ChildrenOnly }; -OWNCLOUDSYNC_EXPORT Result setPinState(const FileHandle &handle, PinState state, SetPinRecurseMode mode); +OWNCLOUDSYNC_EXPORT Result setPinState(const FileHandle &handle, PinState state, SetPinRecurseMode mode); OWNCLOUDSYNC_EXPORT Result createPlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId); -OWNCLOUDSYNC_EXPORT Result updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString()); -OWNCLOUDSYNC_EXPORT Result convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath); +OWNCLOUDSYNC_EXPORT Result updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString()); +OWNCLOUDSYNC_EXPORT Result convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath); } diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 5dcbc89c5..e97fa7031 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp +++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp @@ -111,10 +111,15 @@ Result VfsCfApi::updateMetadata(const QString &filePath, time_t m const auto localPath = QDir::toNativeSeparators(filePath); const auto handle = cfapi::handleForPath(localPath); if (handle) { - return cfapi::updatePlaceholderInfo(handle, modtime, size, fileId); + auto result = cfapi::updatePlaceholderInfo(handle, modtime, size, fileId); + if (result) { + return {}; + } else { + return result.error(); + } } else { qCWarning(lcCfApi) << "Couldn't update metadata for non existing file" << localPath; - return "Couldn't update metadata"; + return QStringLiteral("Couldn't update metadata"); } } @@ -150,7 +155,7 @@ Result VfsCfApi::dehydratePlaceholder(const SyncFileItem &item) return {}; } -Result VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) +Result VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) { const auto localPath = QDir::toNativeSeparators(filename); const auto replacesPath = QDir::toNativeSeparators(replacesFile); diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.h b/src/libsync/vfs/cfapi/vfs_cfapi.h index 1ec6826d1..3c41a6223 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.h +++ b/src/libsync/vfs/cfapi/vfs_cfapi.h @@ -43,7 +43,7 @@ public: Result createPlaceholder(const SyncFileItem &item) override; Result dehydratePlaceholder(const SyncFileItem &item) override; - Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override; + Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override; bool needsMetadataUpdate(const SyncFileItem &) override; bool isDehydratedPlaceholder(const QString &filePath) override; diff --git a/src/libsync/vfs/suffix/vfs_suffix.cpp b/src/libsync/vfs/suffix/vfs_suffix.cpp index f3858e8a2..547492eac 100644 --- a/src/libsync/vfs/suffix/vfs_suffix.cpp +++ b/src/libsync/vfs/suffix/vfs_suffix.cpp @@ -122,10 +122,10 @@ Result VfsSuffix::dehydratePlaceholder(const SyncFileItem &item) return {}; } -Result VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) +Result VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) { // Nothing necessary - return {}; + return Vfs::ConvertToPlaceholderResult::Ok; } bool VfsSuffix::isDehydratedPlaceholder(const QString &filePath) diff --git a/src/libsync/vfs/suffix/vfs_suffix.h b/src/libsync/vfs/suffix/vfs_suffix.h index 4aa513c03..ec608f620 100644 --- a/src/libsync/vfs/suffix/vfs_suffix.h +++ b/src/libsync/vfs/suffix/vfs_suffix.h @@ -41,7 +41,7 @@ public: Result createPlaceholder(const SyncFileItem &item) override; Result dehydratePlaceholder(const SyncFileItem &item) override; - Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &) override; + Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &) override; bool needsMetadataUpdate(const SyncFileItem &) override { return false; } bool isDehydratedPlaceholder(const QString &filePath) override; diff --git a/src/libsync/vfs/xattr/vfs_xattr.cpp b/src/libsync/vfs/xattr/vfs_xattr.cpp index 61029696e..dddfe3f74 100644 --- a/src/libsync/vfs/xattr/vfs_xattr.cpp +++ b/src/libsync/vfs/xattr/vfs_xattr.cpp @@ -112,10 +112,10 @@ Result VfsXAttr::dehydratePlaceholder(const SyncFileItem &item) return {}; } -Result VfsXAttr::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) +Result VfsXAttr::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) { // Nothing necessary - return {}; + return {ConvertToPlaceholderResult::Ok}; } bool VfsXAttr::needsMetadataUpdate(const SyncFileItem &) diff --git a/src/libsync/vfs/xattr/vfs_xattr.h b/src/libsync/vfs/xattr/vfs_xattr.h index 4b642c7a9..238f97e30 100644 --- a/src/libsync/vfs/xattr/vfs_xattr.h +++ b/src/libsync/vfs/xattr/vfs_xattr.h @@ -41,7 +41,7 @@ public: Result createPlaceholder(const SyncFileItem &item) override; Result dehydratePlaceholder(const SyncFileItem &item) override; - Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override; + Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override; bool needsMetadataUpdate(const SyncFileItem &item) override; bool isDehydratedPlaceholder(const QString &filePath) override;