From bb9ce8db215542d051a999bb86161d84ae2249ce Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 26 Nov 2020 15:27:03 +0100 Subject: [PATCH] Test that the sync behave well if there are errors while reading the database --- src/common/syncjournaldb.cpp | 22 +++++++--- src/common/syncjournaldb.h | 7 ++++ test/CMakeLists.txt | 1 + test/syncenginetestutils.h | 3 ++ test/testdatabaseerror.cpp | 80 ++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 test/testdatabaseerror.cpp diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index ed28a5e87..f90450859 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -279,6 +279,13 @@ bool SyncJournalDb::sqlFail(const QString &log, const SqlQuery &query) bool SyncJournalDb::checkConnect() { + if (autotestFailCounter >= 0) { + if (!autotestFailCounter--) { + qCInfo(lcDb) << "Error Simulated"; + return false; + } + } + if (_db.isOpen()) { // Unfortunately the sqlite isOpen check can return true even when the underlying storage // has become unavailable - and then some operations may cause crashes. See #6049 @@ -634,7 +641,7 @@ bool SyncJournalDb::updateMetadataTableStructure() bool re = true; // check if the file_id column is there and create it if not - if (!checkConnect()) { + if (columns.isEmpty()) { return false; } @@ -751,7 +758,10 @@ bool SyncJournalDb::updateMetadataTableStructure() commitInternal("update database structure: add isE2eEncrypted col"); } - if (!tableColumns("uploadinfo").contains("contentChecksum")) { + auto uploadInfoColumns = tableColumns("uploadinfo"); + if (uploadInfoColumns.isEmpty()) + return false; + if (!uploadInfoColumns.contains("contentChecksum")) { SqlQuery query(_db); query.prepare("ALTER TABLE uploadinfo ADD COLUMN contentChecksum TEXT;"); if (!query.exec()) { @@ -761,7 +771,10 @@ bool SyncJournalDb::updateMetadataTableStructure() commitInternal("update database structure: add contentChecksum col for uploadinfo"); } - if (!tableColumns("conflicts").contains("basePath")) { + auto conflictsColumns = tableColumns("conflicts"); + if (conflictsColumns.isEmpty()) + return false; + if (!conflictsColumns.contains("basePath")) { SqlQuery query(_db); query.prepare("ALTER TABLE conflicts ADD COLUMN basePath TEXT;"); if (!query.exec()) { @@ -788,8 +801,7 @@ bool SyncJournalDb::updateErrorBlacklistTableStructure() auto columns = tableColumns("blacklist"); bool re = true; - // check if the file_id column is there and create it if not - if (!checkConnect()) { + if (columns.isEmpty()) { return false; } diff --git a/src/common/syncjournaldb.h b/src/common/syncjournaldb.h index 8898eefe2..417f2b7e5 100644 --- a/src/common/syncjournaldb.h +++ b/src/common/syncjournaldb.h @@ -245,6 +245,13 @@ public: */ void markVirtualFileForDownloadRecursively(const QByteArray &path); + /** + * Only used for auto-test: + * when positive, will decrease the counter for every database operation. + * reaching 0 makes the operation fails + */ + int autotestFailCounter = -1; + private: int getFileRecordCount(); bool updateDatabaseStructure(); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8c719097e..031333721 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -58,6 +58,7 @@ nextcloud_add_test(LocalDiscovery "syncenginetestutils.h") nextcloud_add_test(RemoteDiscovery "syncenginetestutils.h") nextcloud_add_test(Permissions "syncenginetestutils.h") nextcloud_add_test(SelectiveSync "syncenginetestutils.h") +nextcloud_add_test(DatabaseError "syncenginetestutils.h") nextcloud_add_test(LockedFiles "syncenginetestutils.h;../src/gui/lockwatcher.cpp") nextcloud_add_test(FolderWatcher "${FolderWatcher_SRC}") diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index efa581d17..ecdcb48c4 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -12,6 +12,7 @@ #include "filesystem.h" #include "syncengine.h" #include "common/syncjournaldb.h" +#include "csync_exclude.h" #include #include @@ -920,6 +921,8 @@ public: _journalDb = std::make_unique(localPath() + "._sync_test.db"); _syncEngine = std::make_unique(_account, localPath(), "", _journalDb.get()); + // Ignore temporary files from the download. (This is in the default exclude list, but we don't load it) + _syncEngine->excludedFiles().addManualExclude("]*.~*"); // A new folder will update the local file state database on first sync. // To have a state matching what users will encounter, we have to a sync diff --git a/test/testdatabaseerror.cpp b/test/testdatabaseerror.cpp new file mode 100644 index 000000000..4c2ad5de2 --- /dev/null +++ b/test/testdatabaseerror.cpp @@ -0,0 +1,80 @@ +/* + * This software is in the public domain, furnished "as is", without technical + * support, and with no warranty, express or implied, as to its usefulness for + * any purpose. + * + */ + +#include +#include "syncenginetestutils.h" +#include + +using namespace OCC; + + +class TestDatabaseError : public QObject +{ + Q_OBJECT + +private slots: + void testDatabaseError() { + /* This test will make many iteration, at each iteration, the iᵗʰ database access will fail. + * The test ensure that if there is a failure, the next sync recovers. And if there was + * no error, then everything was sync'ed properly. + */ + + FileInfo finalState; + for (int count = 0; true; ++count) { + qInfo() << "Starting Iteration" << count; + + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + + // Do a couple of changes + fakeFolder.remoteModifier().insert("A/a0"); + fakeFolder.remoteModifier().appendByte("A/a1"); + fakeFolder.remoteModifier().remove("A/a2"); + fakeFolder.remoteModifier().rename("S/s1", "S/s1_renamed"); + fakeFolder.remoteModifier().mkdir("D"); + fakeFolder.remoteModifier().mkdir("D/subdir"); + fakeFolder.remoteModifier().insert("D/subdir/file"); + fakeFolder.localModifier().insert("B/b0"); + fakeFolder.localModifier().appendByte("B/b1"); + fakeFolder.remoteModifier().remove("B/b2"); + fakeFolder.localModifier().mkdir("NewDir"); + fakeFolder.localModifier().rename("C", "NewDir/C"); + + // Set the counter + fakeFolder.syncJournal().autotestFailCounter = count; + + // run the sync + bool result = fakeFolder.syncOnce(); + + qInfo() << "Result of iteration" << count << "was" << result; + + if (fakeFolder.syncJournal().autotestFailCounter >= 0) { + // No error was thrown, we are finished + QVERIFY(result); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QCOMPARE(fakeFolder.currentRemoteState(), finalState); + return; + } + + if (!result) { + fakeFolder.syncJournal().autotestFailCounter = -1; + // Try again + QVERIFY(fakeFolder.syncOnce()); + } + + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + if (count == 0) { + finalState = fakeFolder.currentRemoteState(); + } else { + // the final state should be the same for every iteration + QCOMPARE(fakeFolder.currentRemoteState(), finalState); + } + } + } +}; + +QTEST_GUILESS_MAIN(TestDatabaseError) +#include "testdatabaseerror.moc"