From 50ffc7373fd4f0cd645343622754ff55f4350a49 Mon Sep 17 00:00:00 2001 From: alex-z Date: Tue, 20 Dec 2022 19:49:35 +0100 Subject: [PATCH] Update file's metadata in the local database when the etag changes while file remains unchanged. Fix subsequent conflict when locking and unlocking. Signed-off-by: alex-z --- src/libsync/propagatedownload.cpp | 32 ++++++++++++++++++++++++--- src/libsync/propagatedownload.h | 4 ++++ test/testsyncconflict.cpp | 36 +++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index b390cee55..2a0981410 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -1106,13 +1106,39 @@ void PropagateDownloadFile::contentChecksumComputed(const QByteArray &checksumTy { _item->_checksumHeader = makeChecksumHeader(checksumType, checksum); + const auto localFilePath = propagator()->fullLocalPath(_item->_file); + SyncJournalFileRecord record; + if (_item->_instruction != CSYNC_INSTRUCTION_CONFLICT && FileSystem::fileExists(localFilePath) + && (propagator()->_journal->getFileRecord(_item->_file, &record) && record.isValid()) + && (record._modtime == _item->_modtime && record._etag != _item->_etag)) { + const auto computeChecksum = new ComputeChecksum(this); + computeChecksum->setChecksumType(checksumType); + connect(computeChecksum, &ComputeChecksum::done, this, &PropagateDownloadFile::localFileContentChecksumComputed); + computeChecksum->start(localFilePath); + return; + } + + finalizeDownload(); +} + +void PropagateDownloadFile::localFileContentChecksumComputed(const QByteArray &checksumType, const QByteArray &checksum) +{ + if (_item->_checksumHeader == makeChecksumHeader(checksumType, checksum)) { + FileSystem::remove(_tmpFile.fileName()); + updateMetadata(false); + return; + } + finalizeDownload(); +} + +void PropagateDownloadFile::finalizeDownload() +{ if (_isEncrypted) { if (_downloadEncryptedHelper->decryptFile(_tmpFile)) { - downloadFinished(); + downloadFinished(); } else { - done(SyncFileItem::NormalError, _downloadEncryptedHelper->errorString()); + done(SyncFileItem::NormalError, _downloadEncryptedHelper->errorString()); } - } else { downloadFinished(); } diff --git a/src/libsync/propagatedownload.h b/src/libsync/propagatedownload.h index 47d8a5c9e..cb9304d3e 100644 --- a/src/libsync/propagatedownload.h +++ b/src/libsync/propagatedownload.h @@ -230,6 +230,10 @@ private slots: void transmissionChecksumValidated(const QByteArray &checksumType, const QByteArray &checksum); /// Called when the download's checksum computation is done void contentChecksumComputed(const QByteArray &checksumType, const QByteArray &checksum); + /// Called when the local file's checksum computation is done + void localFileContentChecksumComputed(const QByteArray &checksumType, const QByteArray &checksum); + + void finalizeDownload(); void downloadFinished(); /// Called when it's time to update the db metadata void updateMetadata(bool isConflict); diff --git a/test/testsyncconflict.cpp b/test/testsyncconflict.cpp index a12b47645..8a00d76fe 100644 --- a/test/testsyncconflict.cpp +++ b/test/testsyncconflict.cpp @@ -599,6 +599,42 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + void testEtagChangeFileNotChangedGeneratesNoConflicts() + { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + ItemCompletedSpy completeSpy(fakeFolder); + + fakeFolder.remoteModifier().insert("A/fake_conflict", 'W'); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!itemConflict(completeSpy, "A/fake_conflict")); + + completeSpy.clear(); + + fakeFolder.remoteModifier().setContents("A/fake_conflict", 'W'); + fakeFolder.localModifier().setContents("A/fake_conflict", 'W'); + + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!itemConflict(completeSpy, "A/fake_conflict")); + } + + void testEtagChangeFileChangedGeneratesConflicts() + { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + ItemCompletedSpy completeSpy(fakeFolder); + + fakeFolder.remoteModifier().insert("A/real_conflict", 'W'); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!itemConflict(completeSpy, "A/real_conflict")); + + completeSpy.clear(); + + fakeFolder.remoteModifier().setContents("A/real_conflict", 'W'); + fakeFolder.localModifier().setContents("A/real_conflict", 'L'); + + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(itemConflict(completeSpy, "A/real_conflict")); + } + // Test what happens if we remove entries both on the server, and locally void testRemoveRemove() {