Merge pull request #5293 from nextcloud/bugfix/do-not-generate-conflict-when-locking-and-unlocking

Update file's metadata in the local database when the etag changes while file remains unchanged. Fix subsequent conflict when locking and unlocking.

Author: allexzander

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

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

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()
     {