Reverse the dependency between SyncJournalFileRecord and SyncFileItem

This will allow us to also use the SyncJournalDB in csync.

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

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");

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

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;

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;

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

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;

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.

src/libsync/syncfileitem.cpp

@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) by Klaas Freitag <freitag@owncloud.com>
+ *
+ * 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 <QLoggingCategory>
+#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<SyncFileItem::Type>(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;
+}
+
+}

src/libsync/syncfileitem.h

@@ -25,6 +25,10 @@
 
 namespace OCC {
 
+class SyncFileItem;
+class SyncJournalFileRecord;
+typedef QSharedPointer<SyncFileItem> 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<SyncFileItem> SyncFileItemPtr;
 inline bool operator<(const SyncFileItemPtr &item1, const SyncFileItemPtr &item2)
 {
     return *item1 < *item2;

src/libsync/syncjournalfilerecord.cpp

@@ -13,23 +13,10 @@
  */
 
 #include "syncjournalfilerecord.h"
-#include "syncfileitem.h"
 #include "common/utility.h"
-#include "filesystem.h"
-
-#include <QLoggingCategory>
-#include <qfileinfo.h>
-
-#ifdef Q_OS_WIN
-#include <windows.h>
-#else
-#include <sys/stat.h>
-#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<SyncFileItem::Type>(_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()

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;