mirrors/nextcloud-desktop
1
0
Fork 0
mirror of https://github.com/nextcloud/desktop.git synced 2024-11-23 13:35:58 +03:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #3511 from nextcloud/bugfix/vfsFixes

Browse source 
Improve the error message returned by updateMetadata
This commit is contained in:
Matthieu Gallien 2021-07-06 16:15:26 +02:00 committed by GitHub
commit c1f0716d53
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 115 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/common/syncjournaldb.cpp
View file

@ -898,7 +898,7 @@ qint64 SyncJournalDb::getPHash(const QByteArray &file)
return h;
}
bool SyncJournalDb::setFileRecord(const SyncJournalFileRecord &_record)
Result<void, QString> 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.
}
}

2
src/common/syncjournaldb.h
View file

@ -65,7 +65,7 @@ public:
bool getFileRecordsByFileId(const QByteArray &fileId, const std::function<void(const SyncJournalFileRecord &)> &rowCallback);
bool getFilesBelowPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
bool listFilesInPath(const QByteArray &path, const std::function<void(const SyncJournalFileRecord&)> &rowCallback);
bool setFileRecord(const SyncJournalFileRecord &record);
Result<void, QString> setFileRecord(const SyncJournalFileRecord &record);
void keyValueStoreSet(const QString &key, QVariant value);
qint64 keyValueStoreGetInt(const QString &key, qint64 defaultValue);

11
src/common/vfs.h
View file

@ -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<Mode> 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<void, QString> convertToPlaceholder(
virtual Q_REQUIRED_RESULT Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(
const QString &filename,
const SyncFileItem &item,
const QString &replacesFile = QString()) = 0;
@ -303,7 +310,7 @@ public:
Result<void, QString> updateMetadata(const QString &, time_t, qint64, const QByteArray &) override { return {}; }
Result<void, QString> createPlaceholder(const SyncFileItem &) override { return {}; }
Result<void, QString> dehydratePlaceholder(const SyncFileItem &) override { return {}; }
Result<void, QString> convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override { return {}; }
Result<ConvertToPlaceholderResult, QString> 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; }

38
src/libsync/owncloudpropagator.cpp
View file

@ -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<Vfs::ConvertToPlaceholderResult, QString> 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<Vfs::ConvertToPlaceholderResult, QString> 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");

17
src/libsync/owncloudpropagator.h
View file

@ -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<Vfs::ConvertToPlaceholderResult, QString> 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<Vfs::ConvertToPlaceholderResult, QString> staticUpdateMetadata(const SyncFileItem &item, const QString localDir,
Vfs *vfs, SyncJournalDb * const journal);
private slots:
@ -626,8 +635,8 @@ class CleanupPollsJob : public QObject
QSharedPointer<Vfs> _vfs;
public:
explicit CleanupPollsJob(const QVector<SyncJournalDb::PollInfo> &pollInfos, AccountPtr account,
SyncJournalDb *journal, const QString &localPath, const QSharedPointer<Vfs> &vfs, QObject *parent = nullptr)
explicit CleanupPollsJob(const QVector<SyncJournalDb::PollInfo> &pollInfos, AccountPtr account, SyncJournalDb *journal, const QString &localPath,
const QSharedPointer<Vfs> &vfs, QObject *parent = nullptr)
: QObject(parent)
, _pollInfos(pollInfos)
, _account(account)

11
src/libsync/propagatedownload.cpp
View file

@ -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;
}

8
src/libsync/propagateremotemkdir.cpp
View file

@ -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;
}

8
src/libsync/propagateremotemove.cpp
View file

@ -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

8
src/libsync/propagateupload.cpp
View file

@ -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;
}

18
src/libsync/propagatorjobs.cpp
View file

@ -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;
}
}

10
src/libsync/vfs/cfapi/cfapiwrapper.cpp
View file

@ -622,14 +622,14 @@ OCC::CfApiWrapper::PlaceHolderInfo OCC::CfApiWrapper::findPlaceholderInfo(const
}
}
OCC::Result<void, QString> OCC::CfApiWrapper::setPinState(const FileHandle &handle, OCC::PinStateEnums::PinState state, SetPinRecurseMode mode)
OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> 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<void, QString> OCC::CfApiWrapper::createPlaceholderInfo(const QStrin
return {};
}
OCC::Result<void, QString> OCC::CfApiWrapper::updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath)
OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> 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<void, QString> OCC::CfApiWrapper::updatePlaceholderInfo(const FileHa
return { "Couldn't restore pin state" };
}
return {};
return OCC::Vfs::ConvertToPlaceholderResult::Ok;
}
OCC::Result<void, QString> OCC::CfApiWrapper::convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath)
OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath)
{
Q_UNUSED(modtime);
Q_UNUSED(size);

7
src/libsync/vfs/cfapi/cfapiwrapper.h
View file

@ -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<void, QString> setPinState(const FileHandle &handle, PinState state, SetPinRecurseMode mode);
OWNCLOUDSYNC_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> setPinState(const FileHandle &handle, PinState state, SetPinRecurseMode mode);
OWNCLOUDSYNC_EXPORT Result<void, QString> createPlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId);
OWNCLOUDSYNC_EXPORT Result<void, QString> updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString());
OWNCLOUDSYNC_EXPORT Result<void, QString> convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath);
OWNCLOUDSYNC_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString());
OWNCLOUDSYNC_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath);
}

11
src/libsync/vfs/cfapi/vfs_cfapi.cpp
View file

@ -111,10 +111,15 @@ Result<void, QString> 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<void, QString> VfsCfApi::dehydratePlaceholder(const SyncFileItem &item)
return {};
}
Result<void, QString> VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile)
Result<Vfs::ConvertToPlaceholderResult, QString> VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile)
{
const auto localPath = QDir::toNativeSeparators(filename);
const auto replacesPath = QDir::toNativeSeparators(replacesFile);

2
src/libsync/vfs/cfapi/vfs_cfapi.h
View file

@ -43,7 +43,7 @@ public:
Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
Result<void, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override;
Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override;
bool needsMetadataUpdate(const SyncFileItem &) override;
bool isDehydratedPlaceholder(const QString &filePath) override;

4
src/libsync/vfs/suffix/vfs_suffix.cpp
View file

@ -122,10 +122,10 @@ Result<void, QString> VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
return {};
}
Result<void, QString> VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)
Result<Vfs::ConvertToPlaceholderResult, QString> VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)
{
// Nothing necessary
return {};
return Vfs::ConvertToPlaceholderResult::Ok;
}
bool VfsSuffix::isDehydratedPlaceholder(const QString &filePath)

2
src/libsync/vfs/suffix/vfs_suffix.h
View file

@ -41,7 +41,7 @@ public:
Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
Result<void, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &) override;
Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &) override;
bool needsMetadataUpdate(const SyncFileItem &) override { return false; }
bool isDehydratedPlaceholder(const QString &filePath) override;

4
src/libsync/vfs/xattr/vfs_xattr.cpp
View file

@ -112,10 +112,10 @@ Result<void, QString> VfsXAttr::dehydratePlaceholder(const SyncFileItem &item)
return {};
}
Result<void, QString> VfsXAttr::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)
Result<Vfs::ConvertToPlaceholderResult, QString> VfsXAttr::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)
{
// Nothing necessary
return {};
return {ConvertToPlaceholderResult::Ok};
}
bool VfsXAttr::needsMetadataUpdate(const SyncFileItem &)

2
src/libsync/vfs/xattr/vfs_xattr.h
View file

@ -41,7 +41,7 @@ public:
Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
Result<void, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override;
Result<ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override;
bool needsMetadataUpdate(const SyncFileItem &item) override;
bool isDehydratedPlaceholder(const QString &filePath) override;