diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index cc61bfab54807..90f26719e0ad2 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -912,9 +912,8 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it done = true; return; } - if (!serverEntry.isDirectory && base._etag != serverEntry.etag) { - /* File with different etag, don't do a rename, but download the file again */ - qCInfo(lcDisco, "file etag different, not a rename"); + if (!serverEntry.isDirectory && (base._modtime != serverEntry.modtime || base._fileSize != serverEntry.size)) { + qCInfo(lcDisco, "file metadata different, forcing a download of the new file"); done = true; return; } @@ -1595,7 +1594,7 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce // If there's no content hash, use heuristics if (serverEntry.checksumHeader.isEmpty()) { // If the size or mtime is different, it's definitely a conflict. - bool isConflict = (serverEntry.size != localEntry.size) || (serverEntry.modtime != localEntry.modtime); + bool isConflict = (serverEntry.size != localEntry.size) || (serverEntry.modtime != localEntry.modtime) || (dbEntry.isValid() && dbEntry._modtime != localEntry.modtime && serverEntry.modtime == localEntry.modtime); // It could be a conflict even if size and mtime match! // diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp index 6c01bf29cab8a..7c8d4eabcddd3 100644 --- a/test/syncenginetestutils.cpp +++ b/test/syncenginetestutils.cpp @@ -178,6 +178,7 @@ void FileInfo::setContents(const QString &relativePath, char contentChar) FileInfo *file = findInvalidatingEtags(relativePath); Q_ASSERT(file); file->contentChar = contentChar; + file->lastModified = QDateTime::currentDateTimeUtc(); } void FileInfo::appendByte(const QString &relativePath) diff --git a/test/testchunkingng.cpp b/test/testchunkingng.cpp index 8ec79d34ad593..87612fab538a1 100644 --- a/test/testchunkingng.cpp +++ b/test/testchunkingng.cpp @@ -328,35 +328,22 @@ private slots: fakeFolder.localModifier().insert("A/a0", size); QVERIFY(!fakeFolder.syncOnce()); // error: abort! - // Now the next sync gets a NEW/NEW conflict and since there's no checksum - // it just becomes a UPDATE_METADATA - auto checkEtagUpdated = [&](SyncFileItemVector &items) { - QCOMPARE(items.size(), 1); - QCOMPARE(items[0]->_file, QLatin1String("A")); - SyncJournalFileRecord record; - QVERIFY(fakeFolder.syncJournal().getFileRecord(QByteArray("A/a0"), &record)); - QCOMPARE(record._etag, fakeFolder.remoteModifier().find("A/a0")->etag); - }; - auto connection = connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToPropagate, checkEtagUpdated); QVERIFY(fakeFolder.syncOnce()); - disconnect(connection); QCOMPARE(nGET, 0); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - // Test 2: modified file upload aborted + nGET = 0; fakeFolder.localModifier().appendByte("A/a0"); QVERIFY(!fakeFolder.syncOnce()); // error: abort! // An EVAL/EVAL conflict is also UPDATE_METADATA when there's no checksums - connection = connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToPropagate, checkEtagUpdated); QVERIFY(fakeFolder.syncOnce()); - disconnect(connection); - QCOMPARE(nGET, 0); + QCOMPARE(nGET, 1); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - // Test 3: modified file upload aborted, with good checksums + nGET = 0; fakeFolder.localModifier().appendByte("A/a0"); QVERIFY(!fakeFolder.syncOnce()); // error: abort! @@ -365,12 +352,11 @@ private slots: fakeFolder.remoteModifier().find("A/a0")->checksums = moveChecksumHeader; QVERIFY(fakeFolder.syncOnce()); - disconnect(connection); QCOMPARE(nGET, 0); // no new download, just a metadata update! QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - // Test 4: New file, that gets deleted locally before the next sync + nGET = 0; fakeFolder.localModifier().insert("A/a3", size); QVERIFY(!fakeFolder.syncOnce()); // error: abort! fakeFolder.localModifier().remove("A/a3"); diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index 120bc52d809e3..c46a9f7ab0143 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -432,7 +432,7 @@ private slots: QTest::newRow("Same mtime, but no server checksum -> ignored in reconcile") << true << QByteArray() - << 0; + << 1; QTest::newRow("Same mtime, weak server checksum differ -> downloaded") << true << QByteArray("Adler32:bad") diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp index 1266fdef4b0ee..1475013cf3226 100644 --- a/test/testsyncmove.cpp +++ b/test/testsyncmove.cpp @@ -741,38 +741,6 @@ private slots: } } - // https://github.com/owncloud/client/issues/6629#issuecomment-402450691 - // When a file is moved and the server mtime was not in sync, the local mtime should be kept - void testMoveAndMTimeChange() - { - FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; - OperationCounter counter; - fakeFolder.setServerOverride(counter.functor()); - - // Changing the mtime on the server (without invalidating the etag) - fakeFolder.remoteModifier().find("A/a1")->lastModified = QDateTime::currentDateTimeUtc().addSecs(-50000); - fakeFolder.remoteModifier().find("A/a2")->lastModified = QDateTime::currentDateTimeUtc().addSecs(-40000); - - // Move a few files - fakeFolder.remoteModifier().rename("A/a1", "A/a1_server_renamed"); - fakeFolder.localModifier().rename("A/a2", "A/a2_local_renamed"); - - QVERIFY(fakeFolder.syncOnce()); - QCOMPARE(counter.nGET, 0); - QCOMPARE(counter.nPUT, 0); - QCOMPARE(counter.nMOVE, 1); - QCOMPARE(counter.nDELETE, 0); - - // Another sync should do nothing - QVERIFY(fakeFolder.syncOnce()); - QCOMPARE(counter.nGET, 0); - QCOMPARE(counter.nPUT, 0); - QCOMPARE(counter.nMOVE, 1); - QCOMPARE(counter.nDELETE, 0); - - QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - } - // Test for https://github.com/owncloud/client/issues/6694 void testInvertFolderHierarchy() {