Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/use file id to track renamed items #7334

Merged
merged 3 commits into from
Oct 17, 2024
Merged

Bugfix/use file id to track renamed items #7334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7301ad3
in automated tests, update modtime when modifying the content of a file
mgallien Oct 16, 2024
f0908db
handle NEW/NEW conflicts by being real conflicts
mgallien Oct 16, 2024
05b6db5
remove the enforcement of identical etag for a server side item MOVE
mgallien Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/libsync/discovery.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/discovery.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/discovery.cpp 

File src/libsync/discovery.cpp does not conform to Custom style guidelines. (lines 1597)
* Copyright (C) by Olivier Goffart <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -912,9 +912,8 @@
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;
}
Expand Down Expand Up @@ -1595,7 +1594,7 @@
// 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!
//
Expand Down
1 change: 1 addition & 0 deletions test/syncenginetestutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 4 additions & 18 deletions test/testchunkingng.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand All @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion test/testsyncengine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
*/

#include <QtTest>

Check failure on line 8 in test/testsyncengine.cpp

View workflow job for this annotation

GitHub Actions / build

test/testsyncengine.cpp:8:10 [clang-diagnostic-error] 

'QtTest' file not found
#include <QTextCodec>

#include "syncenginetestutils.h"
Expand Down Expand Up @@ -432,7 +432,7 @@

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")
Expand Down
32 changes: 0 additions & 32 deletions test/testsyncmove.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
Loading