From 5fbed0d1cd85f5bca4c865af543bda12af47226c Mon Sep 17 00:00:00 2001 From: Jocelyn Turcotte Date: Wed, 30 Aug 2017 11:17:23 +0200 Subject: [PATCH] Reverse the dependency between SyncJournalFileRecord and SyncFileItem This will allow us to also use the SyncJournalDB in csync. --- src/libsync/CMakeLists.txt | 1 + src/libsync/owncloudpropagator.cpp | 14 ++-- src/libsync/propagatedownload.cpp | 2 +- src/libsync/propagateremotemkdir.cpp | 2 +- src/libsync/propagateremotemove.cpp | 2 +- src/libsync/propagateupload.cpp | 2 +- src/libsync/propagatorjobs.cpp | 4 +- src/libsync/syncengine.cpp | 2 +- src/libsync/syncfileitem.cpp | 71 +++++++++++++++++++ src/libsync/syncfileitem.h | 15 +++- src/libsync/syncjournalfilerecord.cpp | 98 --------------------------- src/libsync/syncjournalfilerecord.h | 13 ---- 12 files changed, 103 insertions(+), 123 deletions(-) create mode 100644 src/libsync/syncfileitem.cpp diff --git a/src/libsync/CMakeLists.txt b/src/libsync/CMakeLists.txt index 3015d7fe3..c7ac8ef08 100644 --- a/src/libsync/CMakeLists.txt +++ b/src/libsync/CMakeLists.txt @@ -51,6 +51,7 @@ set(libsync_SRCS propagateremotemove.cpp propagateremotemkdir.cpp syncengine.cpp + syncfileitem.cpp syncfilestatus.cpp syncfilestatustracker.cpp syncjournaldb.cpp diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 7af77926c..c8bd46cd4 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -133,7 +133,13 @@ static time_t getMaxBlacklistTime() static SyncJournalErrorBlacklistRecord createBlacklistEntry( const SyncJournalErrorBlacklistRecord &old, const SyncFileItem &item) { - auto entry = SyncJournalErrorBlacklistRecord::fromSyncFileItem(item); + SyncJournalErrorBlacklistRecord entry; + entry._file = item._file; + entry._errorString = item._errorString; + entry._lastTryModtime = item._modtime; + entry._lastTryEtag = item._etag; + entry._lastTryTime = Utility::qDateTimeToTime_t(QDateTime::currentDateTime()); + entry._renameTarget = item._renameTarget; entry._retryCount = old._retryCount + 1; static time_t minBlacklistTime(getMinBlacklistTime()); @@ -929,7 +935,7 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) _item->_fileId = mkdir->_item->_fileId; } } - SyncJournalFileRecord record(*_item, propagator()->_localDir + _item->_file); + SyncJournalFileRecord record = _item->toSyncJournalFileRecordWithInode(propagator()->_localDir + _item->_file); bool ok = propagator()->_journal->setFileRecordMetadata(record); if (!ok) { status = _item->_status = SyncFileItem::FatalError; @@ -959,8 +965,8 @@ void CleanupPollsJob::start() auto info = _pollInfos.first(); _pollInfos.pop_front(); SyncJournalFileRecord record = _journal->getFileRecord(info._file); - SyncFileItemPtr item(new SyncFileItem(record.toSyncFileItem())); if (record.isValid()) { + SyncFileItemPtr item = SyncFileItem::fromSyncJournalFileRecord(record); PollJob *job = new PollJob(_account, info._url, item, _journal, _localPath, this); connect(job, SIGNAL(finishedSignal()), SLOT(slotPollFinished())); job->start(); @@ -978,7 +984,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 (!_journal->setFileRecord(SyncJournalFileRecord(*job->_item, _localPath + job->_item->_file))) { + if (!_journal->setFileRecord(job->_item->toSyncJournalFileRecordWithInode(_localPath + job->_item->_file))) { qCWarning(lcCleanupPolls) << "database error"; job->_item->_status = SyncFileItem::FatalError; job->_item->_errorString = tr("Error writing metadata to the database"); diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 0168713f4..21af4d45c 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -873,7 +873,7 @@ void PropagateDownloadFile::updateMetadata(bool isConflict) { QString fn = propagator()->getFilePath(_item->_file); - if (!propagator()->_journal->setFileRecord(SyncJournalFileRecord(*_item, fn))) { + if (!propagator()->_journal->setFileRecord(_item->toSyncJournalFileRecordWithInode(fn))) { done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); return; } diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp index f293fa0a9..92ba2b9a5 100644 --- a/src/libsync/propagateremotemkdir.cpp +++ b/src/libsync/propagateremotemkdir.cpp @@ -142,7 +142,7 @@ void PropagateRemoteMkdir::propfindError() void PropagateRemoteMkdir::success() { // save the file id already so we can detect rename or remove - SyncJournalFileRecord record(*_item, propagator()->_localDir + _item->destination()); + SyncJournalFileRecord record = _item->toSyncJournalFileRecordWithInode(propagator()->_localDir + _item->destination()); if (!propagator()->_journal->setFileRecord(record)) { done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); return; diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index a45736564..7f0d8785b 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -170,7 +170,7 @@ void PropagateRemoteMove::finalize() // to the new record. It is not a problem to skip it here. propagator()->_journal->deleteFileRecord(_item->_originalFile); - SyncJournalFileRecord record(*_item, propagator()->getFilePath(_item->_renameTarget)); + SyncJournalFileRecord record = _item->toSyncJournalFileRecordWithInode(propagator()->getFilePath(_item->_renameTarget)); record._path = _item->_renameTarget; if (oldRecord.isValid()) { record._checksumHeader = oldRecord._checksumHeader; diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index f72d36ca7..c49be0785 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -614,7 +614,7 @@ void PropagateUploadFileCommon::finalize() quotaIt.value() -= _item->_size; // Update the database entry - if (!propagator()->_journal->setFileRecord(SyncJournalFileRecord(*_item, propagator()->getFilePath(_item->_file)))) { + if (!propagator()->_journal->setFileRecord(_item->toSyncJournalFileRecordWithInode(propagator()->getFilePath(_item->_file)))) { done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); return; } diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index de9339996..1d841abb4 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -178,7 +178,7 @@ void PropagateLocalMkdir::start() // Adding an entry with a dummy etag to the database still makes sense here // so the database is aware that this folder exists even if the sync is aborted // before the correct etag is stored. - SyncJournalFileRecord record(*_item, newDirStr); + SyncJournalFileRecord record = _item->toSyncJournalFileRecordWithInode(newDirStr); record._etag = "_invalid_"; if (!propagator()->_journal->setFileRecord(record)) { done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); @@ -239,7 +239,7 @@ void PropagateLocalRename::start() const auto oldFile = _item->_file; _item->_file = _item->_renameTarget; - SyncJournalFileRecord record(*_item, targetFile); + SyncJournalFileRecord record = _item->toSyncJournalFileRecordWithInode(targetFile); record._path = _item->_renameTarget; if (oldRecord.isValid()) { record._checksumHeader = oldRecord._checksumHeader; diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index a2afbedde..5a0a56cf9 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -590,7 +590,7 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other, FileSystem::setFileReadOnlyWeak(filePath, isReadOnly); } - _journal->setFileRecordMetadata(SyncJournalFileRecord(*item, filePath)); + _journal->setFileRecordMetadata(item->toSyncJournalFileRecordWithInode(filePath)); } else { // The local tree is walked first and doesn't have all the info from the server. // Update only outdated data from the disk. diff --git a/src/libsync/syncfileitem.cpp b/src/libsync/syncfileitem.cpp new file mode 100644 index 000000000..681d703c9 --- /dev/null +++ b/src/libsync/syncfileitem.cpp @@ -0,0 +1,71 @@ +/* + * Copyright (C) by Klaas Freitag + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "syncfileitem.h" +#include "syncjournalfilerecord.h" +#include "common/utility.h" + +#include +#include "csync/vio/csync_vio_local.h" + +namespace OCC { + +Q_LOGGING_CATEGORY(lcFileItem, "sync.fileitem", QtInfoMsg) + +SyncJournalFileRecord SyncFileItem::toSyncJournalFileRecordWithInode(const QString &localFileName) +{ + SyncJournalFileRecord rec; + rec._path = _file; + rec._modtime = Utility::qDateTimeFromTime_t(_modtime); + rec._type = _type; + rec._etag = _etag; + rec._fileId = _fileId; + rec._fileSize = _size; + rec._remotePerm = _remotePerm; + rec._serverHasIgnoredFiles = _serverHasIgnoredFiles; + rec._checksumHeader = _checksumHeader; + + // Go through csync vio just to get the inode. + csync_file_stat_t fs; + if (csync_vio_local_stat(localFileName.toUtf8().constData(), &fs) == 0) { + rec._inode = fs.inode; + qCDebug(lcFileItem) << localFileName << "Retrieved inode " << _inode << "(previous item inode: " << _inode << ")"; + } else { + // use the "old" inode coming with the item for the case where the + // filesystem stat fails. That can happen if the the file was removed + // or renamed meanwhile. For the rename case we still need the inode to + // detect the rename though. + rec._inode = _inode; + qCWarning(lcFileItem) << "Failed to query the 'inode' for file " << localFileName; + } + return rec; +} + +SyncFileItemPtr SyncFileItem::fromSyncJournalFileRecord(const SyncJournalFileRecord &rec) +{ + SyncFileItemPtr item(new SyncFileItem); + item->_file = rec._path; + item->_inode = rec._inode; + item->_modtime = Utility::qDateTimeToTime_t(rec._modtime); + item->_type = static_cast(rec._type); + item->_etag = rec._etag; + item->_fileId = rec._fileId; + item->_size = rec._fileSize; + item->_remotePerm = rec._remotePerm; + item->_serverHasIgnoredFiles = rec._serverHasIgnoredFiles; + item->_checksumHeader = rec._checksumHeader; + return item; +} + +} diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index 8c546038a..ab9526a93 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -25,6 +25,10 @@ namespace OCC { +class SyncFileItem; +class SyncJournalFileRecord; +typedef QSharedPointer SyncFileItemPtr; + /** * @brief The SyncFileItem class * @ingroup libsync @@ -86,6 +90,16 @@ public: BlacklistedError }; + SyncJournalFileRecord toSyncJournalFileRecordWithInode(const QString &localFileName); + + /** Creates a basic SyncFileItem from a DB record + * + * This is intended in particular for read-update-write cycles that need + * to go through a a SyncFileItem, like PollJob. + */ + static SyncFileItemPtr fromSyncJournalFileRecord(const SyncJournalFileRecord &rec); + + SyncFileItem() : _type(UnknownType) , _direction(None) @@ -232,7 +246,6 @@ public: QString _directDownloadCookies; }; -typedef QSharedPointer SyncFileItemPtr; inline bool operator<(const SyncFileItemPtr &item1, const SyncFileItemPtr &item2) { return *item1 < *item2; diff --git a/src/libsync/syncjournalfilerecord.cpp b/src/libsync/syncjournalfilerecord.cpp index 9c68e646c..5936f0f50 100644 --- a/src/libsync/syncjournalfilerecord.cpp +++ b/src/libsync/syncjournalfilerecord.cpp @@ -13,23 +13,10 @@ */ #include "syncjournalfilerecord.h" -#include "syncfileitem.h" #include "common/utility.h" -#include "filesystem.h" - -#include -#include - -#ifdef Q_OS_WIN -#include -#else -#include -#endif namespace OCC { -Q_LOGGING_CATEGORY(lcFileRecord, "sync.database.filerecord", QtInfoMsg) - SyncJournalFileRecord::SyncJournalFileRecord() : _inode(0) , _type(0) @@ -38,77 +25,6 @@ SyncJournalFileRecord::SyncJournalFileRecord() { } -SyncJournalFileRecord::SyncJournalFileRecord(const SyncFileItem &item, const QString &localFileName) - : _path(item._file) - , _modtime(Utility::qDateTimeFromTime_t(item._modtime)) - , _type(item._type) - , _etag(item._etag) - , _fileId(item._fileId) - , _fileSize(item._size) - , _remotePerm(item._remotePerm) - , _serverHasIgnoredFiles(item._serverHasIgnoredFiles) - , _checksumHeader(item._checksumHeader) -{ - // use the "old" inode coming with the item for the case where the - // filesystem stat fails. That can happen if the the file was removed - // or renamed meanwhile. For the rename case we still need the inode to - // detect the rename though. - _inode = item._inode; - -#ifdef Q_OS_WIN - /* Query the inode: - based on code from csync_vio_local.c (csync_vio_local_stat) - Get the Windows file id as an inode replacement. */ - - HANDLE h = CreateFileW((wchar_t *)FileSystem::longWinPath(localFileName).utf16(), 0, FILE_SHARE_READ, NULL, OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL + FILE_FLAG_BACKUP_SEMANTICS, NULL); - - if (h == INVALID_HANDLE_VALUE) { - qCWarning(lcFileRecord) << "Failed to query the 'inode' because CreateFileW failed for file " << localFileName; - } else { - BY_HANDLE_FILE_INFORMATION fileInfo; - - if (GetFileInformationByHandle(h, &fileInfo)) { - ULARGE_INTEGER FileIndex; - FileIndex.HighPart = fileInfo.nFileIndexHigh; - FileIndex.LowPart = fileInfo.nFileIndexLow; - FileIndex.QuadPart &= 0x0000FFFFFFFFFFFF; - - /* printf("Index: %I64i\n", FileIndex.QuadPart); */ - - _inode = FileIndex.QuadPart; - } else { - qCWarning(lcFileRecord) << "Failed to query the 'inode' for file " << localFileName; - } - CloseHandle(h); - } -#else - struct stat sb; - if (stat(QFile::encodeName(localFileName).constData(), &sb) < 0) { - qCWarning(lcFileRecord) << "Failed to query the 'inode' for file " << localFileName; - } else { - _inode = sb.st_ino; - } -#endif - qCDebug(lcFileRecord) << localFileName << "Retrieved inode " << _inode << "(previous item inode: " << item._inode << ")"; -} - -SyncFileItem SyncJournalFileRecord::toSyncFileItem() -{ - SyncFileItem item; - item._file = _path; - item._inode = _inode; - item._modtime = Utility::qDateTimeToTime_t(_modtime); - item._type = static_cast(_type); - item._etag = _etag; - item._fileId = _fileId; - item._size = _fileSize; - item._remotePerm = _remotePerm; - item._serverHasIgnoredFiles = _serverHasIgnoredFiles; - item._checksumHeader = _checksumHeader; - return item; -} - QByteArray SyncJournalFileRecord::numericFileId() const { // Use the id up until the first non-numeric character @@ -120,20 +36,6 @@ QByteArray SyncJournalFileRecord::numericFileId() const return _fileId; } -SyncJournalErrorBlacklistRecord SyncJournalErrorBlacklistRecord::fromSyncFileItem( - const SyncFileItem &item) -{ - SyncJournalErrorBlacklistRecord record; - record._file = item._file; - record._errorString = item._errorString; - record._lastTryModtime = item._modtime; - record._lastTryEtag = item._etag; - record._lastTryTime = Utility::qDateTimeToTime_t(QDateTime::currentDateTime()); - record._renameTarget = item._renameTarget; - record._retryCount = 1; - return record; -} - bool SyncJournalErrorBlacklistRecord::isValid() const { return !_file.isEmpty() diff --git a/src/libsync/syncjournalfilerecord.h b/src/libsync/syncjournalfilerecord.h index 6ef0ba169..86d214908 100644 --- a/src/libsync/syncjournalfilerecord.h +++ b/src/libsync/syncjournalfilerecord.h @@ -33,16 +33,6 @@ class OWNCLOUDSYNC_EXPORT SyncJournalFileRecord public: SyncJournalFileRecord(); - /// Creates a record from an existing item while updating the inode - SyncJournalFileRecord(const SyncFileItem &, const QString &localFileName); - - /** Creates a basic SyncFileItem from the record - * - * This is intended in particular for read-update-write cycles that need - * to go through a a SyncFileItem, like PollJob. - */ - SyncFileItem toSyncFileItem(); - bool isValid() { return !_path.isEmpty(); @@ -91,9 +81,6 @@ public: { } - /// Create a record based on an item. - static SyncJournalErrorBlacklistRecord fromSyncFileItem(const SyncFileItem &item); - /// The number of times the operation was unsuccessful so far. int _retryCount;