From 4aa12351f3924e7ec0a100fe325400557708badb Mon Sep 17 00:00:00 2001 From: Jonathan Reams Date: Mon, 27 Mar 2023 16:34:11 -0400 Subject: [PATCH] Check result of deleting realm when immediately running sync file actions (#6237) --- CHANGELOG.md | 3 ++- src/realm/object-store/sync/sync_manager.cpp | 3 +-- test/object-store/sync/sync_manager.cpp | 16 ++++++++++++++++ test/object-store/util/test_utils.cpp | 7 +++++-- test/object-store/util/test_utils.hpp | 3 ++- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2853e909f32..5e5cf171e49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,8 @@ ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) * Changing the log level on the fly would not affect the core level log output ([#6440](https://github.com/realm/realm-core/issues/6440), since 13.7.0) -* None. +* `SyncManager::immediately_run_file_actions()` no longer ignores the result of trying to remove a realm. This could have resulted in a client reset action being reported as successful when it actually failed on windows if the `Realm` was still open ([#6050](https://github.com/realm/realm-core/issues/6050)). + ### Breaking changes * None. diff --git a/src/realm/object-store/sync/sync_manager.cpp b/src/realm/object-store/sync/sync_manager.cpp index cf5c24ec6eb..565264130c1 100644 --- a/src/realm/object-store/sync/sync_manager.cpp +++ b/src/realm/object-store/sync/sync_manager.cpp @@ -178,8 +178,7 @@ bool SyncManager::run_file_action(SyncFileActionMetadata& md) switch (md.action()) { case SyncFileActionMetadata::Action::DeleteRealm: // Delete all the files for the given Realm. - m_file_manager->remove_realm(md.original_name()); - return true; + return m_file_manager->remove_realm(md.original_name()); case SyncFileActionMetadata::Action::BackUpThenDeleteRealm: // Copy the primary Realm file to the recovery dir, and then delete the Realm. auto new_name = md.new_name(); diff --git a/test/object-store/sync/sync_manager.cpp b/test/object-store/sync/sync_manager.cpp index 480733dbaca..48c61fcd050 100644 --- a/test/object-store/sync/sync_manager.cpp +++ b/test/object-store/sync/sync_manager.cpp @@ -492,7 +492,23 @@ TEST_CASE("sync_manager: file actions", "[sync]") { const std::string realm_path_3 = file_manager.realm_file_path(uuid_3, local_uuid_3, realm_url, partition); const std::string realm_path_4 = file_manager.realm_file_path(uuid_4, local_uuid_4, realm_url, partition); + // On windows you can't delete a realm if the file is open elsewhere. +#ifdef _WIN32 + SECTION("Action::DeleteRealm - fails if locked") { + SharedRealm locked_realm; + create_dummy_realm(realm_path_1, &locked_realm); + + REQUIRE(locked_realm); + + TestSyncManager tsm(config); + manager.make_file_action_metadata(realm_path_1, realm_url, "user1", Action::DeleteRealm); + + REQUIRE_FALSE(tsm.app()->sync_manager()->immediately_run_file_actions(realm_path_1)); + } +#endif + SECTION("Action::DeleteRealm") { + // Create some file actions manager.make_file_action_metadata(realm_path_1, realm_url, "user1", Action::DeleteRealm); manager.make_file_action_metadata(realm_path_2, realm_url, "user2", Action::DeleteRealm); diff --git a/test/object-store/util/test_utils.cpp b/test/object-store/util/test_utils.cpp index 2a22b534ebf..4cd18d5769a 100644 --- a/test/object-store/util/test_utils.cpp +++ b/test/object-store/util/test_utils.cpp @@ -76,13 +76,16 @@ std::ostream& operator<<(std::ostream& os, const Exception& e) return os; } -bool create_dummy_realm(std::string path) +bool create_dummy_realm(std::string path, std::shared_ptr* out) { Realm::Config config; config.path = path; try { - _impl::RealmCoordinator::get_coordinator(path)->get_realm(config, none); + auto realm = _impl::RealmCoordinator::get_coordinator(path)->get_realm(config, none); REQUIRE_REALM_EXISTS(path); + if (out) { + *out = std::move(realm); + } return true; } catch (std::exception&) { diff --git a/test/object-store/util/test_utils.hpp b/test/object-store/util/test_utils.hpp index 3d22ccbc6a1..38f3e8f1ef3 100644 --- a/test/object-store/util/test_utils.hpp +++ b/test/object-store/util/test_utils.hpp @@ -127,8 +127,9 @@ inline ExceptionMatcher make_exception_matcher(ErrorCodes::Error code, std std::ostream& operator<<(std::ostream&, const Exception&); +class Realm; /// Open a Realm at a given path, creating its files. -bool create_dummy_realm(std::string path); +bool create_dummy_realm(std::string path, std::shared_ptr* out = nullptr); void reset_test_directory(const std::string& base_path); std::vector make_test_encryption_key(const char start = 0); void catch2_ensure_section_run_workaround(bool did_run_a_section, std::string section_name,