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

@@ -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._previousSize) << L;
-    _out << QString::number(item.log._other_modtime) << L;
+    _out << QString::number(item._previousModtime) << L;
-    _out << item.log._other_etag << L;
+    _out /* << other etag (removed) */ << L;
-    _out << item.log._other_fileId << L;
+    _out /* << other fileId (removed) */ << L;
-    _out << instructionToStr(item.log._other_instruction) << L;
+    _out /* << other instruction (removed) */ << L;
 
     _out << endl;
 }

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->_size == _item->_previousSize
-        && _item->_modtime == _item->log._other_modtime
+        && _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 qint64 expectedSize = _item->_previousSize;
-        const time_t expectedMtime = _item->log._other_modtime;
+        const time_t expectedMtime = _item->_previousModtime;
         if (!FileSystem::verifyFileUnchanged(fn, expectedSize, expectedMtime)) {
             propagator()->_anotherSyncNeeded = true;
             done(SyncFileItem::SoftError, tr("File has changed since discovery"));

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->_previousModtime = other->modtime;
-        item->log._other_fileId = other->file_id;
+        item->_previousSize = other->size;
-        item->log._other_instruction = other->instruction;
-        item->log._other_modtime = other->modtime;
-        item->log._other_size = 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)
+                // Take the things to write to the db from the "other" node (i.e: info from server).
-                (*it)->_modtime = (*it)->log._other_modtime;
+                // Do a lookup into the csync remote tree to get the metadata we need to restore.
-                (*it)->_size = (*it)->log._other_size;
+                ASSERT(_csync_ctx->status != CSYNC_STATUS_INIT);
-                (*it)->_fileId = (*it)->log._other_fileId;
+                auto csyncIt = _csync_ctx->remote.files.find((*it)->_file.toUtf8());
-                (*it)->_etag = (*it)->log._other_etag;
+                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;
             }

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<SyncFileItem> SyncFileItemPtr;

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(a1->_previousSize, quint64(4));
-            QCOMPARE(Utility::qDateTimeFromTime_t(a1->log._other_modtime), initialMtime);
+            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(b1->_previousSize, quint64(16));
-            QCOMPARE(Utility::qDateTimeFromTime_t(b1->log._other_modtime), initialMtime);
+            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(c1->_previousSize, quint64(26));
-            QCOMPARE(Utility::qDateTimeFromTime_t(c1->log._other_modtime), changedMtime);
+            QCOMPARE(Utility::qDateTimeFromTime_t(c1->_previousModtime), changedMtime);
         });
 
         QVERIFY(fakeFolder.syncOnce());