From 85a93efe510fdc22403eb17f07ecf352c19f1690 Mon Sep 17 00:00:00 2001 From: Jocelyn Turcotte Date: Thu, 24 Aug 2017 17:07:22 +0200 Subject: [PATCH] Remove SyncFileItem::log This remove the remaining "other" fields of the sync log to save a bit of memory. other_etag and other_fileId don't give much information to the users and other_instruction will always be INST_NONE anyway. other_modtime and other_size are kept since they are sometimes used. They were renamed to have a bit more meaningful name. SyncEngine::checkPermissions will now fetch its information from the csync trees since they are now preserved until right after this point. Fixes #3213 --- src/gui/syncrunfilelog.cpp | 10 +++++----- src/libsync/propagatedownload.cpp | 8 ++++---- src/libsync/syncengine.cpp | 22 ++++++++++++---------- src/libsync/syncfileitem.h | 15 ++++++--------- test/testsyncengine.cpp | 12 ++++++------ 5 files changed, 33 insertions(+), 34 deletions(-) diff --git a/src/gui/syncrunfilelog.cpp b/src/gui/syncrunfilelog.cpp index 79099cd66..51396cbb0 100644 --- a/src/gui/syncrunfilelog.cpp +++ b/src/gui/syncrunfilelog.cpp @@ -161,11 +161,11 @@ void SyncRunFileLog::logItem(const SyncFileItem &item) _out << item._status << L; _out << item._errorString << L; _out << QString::number(item._httpErrorCode) << L; - _out << QString::number(item.log._other_size) << L; - _out << QString::number(item.log._other_modtime) << L; - _out << item.log._other_etag << L; - _out << item.log._other_fileId << L; - _out << instructionToStr(item.log._other_instruction) << L; + _out << QString::number(item._previousSize) << L; + _out << QString::number(item._previousModtime) << L; + _out /* << other etag (removed) */ << L; + _out /* << other fileId (removed) */ << L; + _out /* << other instruction (removed) */ << L; _out << endl; } diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index e98278e0c..0168713f4 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -350,8 +350,8 @@ void PropagateDownloadFile::start() // compare the remote checksum to the local one. // Maybe it's not a real conflict and no download is necessary! if (_item->_instruction == CSYNC_INSTRUCTION_CONFLICT - && _item->_size == _item->log._other_size - && _item->_modtime == _item->log._other_modtime + && _item->_size == _item->_previousSize + && _item->_modtime == _item->_previousModtime && !_item->_checksumHeader.isEmpty()) { qCDebug(lcPropagateDownload) << _item->_file << "may not need download, computing checksum"; auto computeChecksum = new ComputeChecksum(this); @@ -813,8 +813,8 @@ void PropagateDownloadFile::downloadFinished() // phase by comparing size and mtime to the previous values. This // is necessary to avoid overwriting user changes that happened between // the discovery phase and now. - const qint64 expectedSize = _item->log._other_size; - const time_t expectedMtime = _item->log._other_modtime; + const qint64 expectedSize = _item->_previousSize; + const time_t expectedMtime = _item->_previousModtime; if (!FileSystem::verifyFileUnchanged(fn, expectedSize, expectedMtime)) { propagator()->_anotherSyncNeeded = true; done(SyncFileItem::SoftError, tr("File has changed since discovery")); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index a2475a243..38353f9eb 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -667,11 +667,8 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other, _needsUpdate = true; if (other) { - item->log._other_etag = other->etag; - item->log._other_fileId = other->file_id; - item->log._other_instruction = other->instruction; - item->log._other_modtime = other->modtime; - item->log._other_size = other->size; + item->_previousModtime = other->modtime; + item->_previousSize = other->size; } _syncItemMap.insert(key, item); @@ -1293,11 +1290,16 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) (*it)->_instruction = CSYNC_INSTRUCTION_CONFLICT; (*it)->_direction = SyncFileItem::Down; (*it)->_isRestoration = true; - // take the things to write to the db from the "other" node (i.e: info from server) - (*it)->_modtime = (*it)->log._other_modtime; - (*it)->_size = (*it)->log._other_size; - (*it)->_fileId = (*it)->log._other_fileId; - (*it)->_etag = (*it)->log._other_etag; + // Take the things to write to the db from the "other" node (i.e: info from server). + // Do a lookup into the csync remote tree to get the metadata we need to restore. + ASSERT(_csync_ctx->status != CSYNC_STATUS_INIT); + auto csyncIt = _csync_ctx->remote.files.find((*it)->_file.toUtf8()); + if (csyncIt != _csync_ctx->remote.files.end()) { + (*it)->_modtime = csyncIt->second->modtime; + (*it)->_size = csyncIt->second->size; + (*it)->_fileId = csyncIt->second->file_id; + (*it)->_etag = csyncIt->second->etag; + } (*it)->_errorString = tr("Not allowed to upload this file because it is read-only on the server, restoring"); continue; } diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index 45660f83f..9ea4e4d45 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -101,6 +101,8 @@ public: , _modtime(0) , _size(0) , _inode(0) + , _previousSize(0) + , _previousModtime(0) { } @@ -217,17 +219,12 @@ public: // - for conflicts (remote checksum) (what about eval_rename/new reconcile?) QByteArray _checksumHeader; + // The size and modtime of the file getting overwritten (on the disk for downloads, on the server for uploads). + quint64 _previousSize; + time_t _previousModtime; + QString _directDownloadUrl; QString _directDownloadCookies; - - struct - { - quint64 _other_size; - time_t _other_modtime; - QByteArray _other_etag; - QByteArray _other_fileId; - enum csync_instructions_e _other_instruction BITFIELD(16); - } log; }; typedef QSharedPointer SyncFileItemPtr; diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index 716bc5329..4baa43a13 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -441,8 +441,8 @@ private slots: QCOMPARE(a1->_size, quint64(5)); QCOMPARE(Utility::qDateTimeFromTime_t(a1->_modtime), changedMtime); - QCOMPARE(a1->log._other_size, quint64(4)); - QCOMPARE(Utility::qDateTimeFromTime_t(a1->log._other_modtime), initialMtime); + QCOMPARE(a1->_previousSize, quint64(4)); + QCOMPARE(Utility::qDateTimeFromTime_t(a1->_previousModtime), initialMtime); // b2: should have remote size and modtime QVERIFY(b1); @@ -450,8 +450,8 @@ private slots: QCOMPARE(b1->_direction, SyncFileItem::Down); QCOMPARE(b1->_size, quint64(17)); QCOMPARE(Utility::qDateTimeFromTime_t(b1->_modtime), changedMtime); - QCOMPARE(b1->log._other_size, quint64(16)); - QCOMPARE(Utility::qDateTimeFromTime_t(b1->log._other_modtime), initialMtime); + QCOMPARE(b1->_previousSize, quint64(16)); + QCOMPARE(Utility::qDateTimeFromTime_t(b1->_previousModtime), initialMtime); // c1: conflicts are downloads, so remote size and modtime QVERIFY(c1); @@ -459,8 +459,8 @@ private slots: QCOMPARE(c1->_direction, SyncFileItem::None); QCOMPARE(c1->_size, quint64(25)); QCOMPARE(Utility::qDateTimeFromTime_t(c1->_modtime), changedMtime2); - QCOMPARE(c1->log._other_size, quint64(26)); - QCOMPARE(Utility::qDateTimeFromTime_t(c1->log._other_modtime), changedMtime); + QCOMPARE(c1->_previousSize, quint64(26)); + QCOMPARE(Utility::qDateTimeFromTime_t(c1->_previousModtime), changedMtime); }); QVERIFY(fakeFolder.syncOnce()); -- 2.30.2