From 6e21845631f7ac32db636c77dbaacf623c38c763 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 13 Apr 2022 18:41:50 -0400 Subject: [PATCH 1/3] leveldb_snappy_test.cc: bring closer to parity with the test in the iOS repository --- .../integration_test_internal/CMakeLists.txt | 7 + .../src/leveldb_snappy_test.cc | 144 ++++++++++++------ 2 files changed, 106 insertions(+), 45 deletions(-) diff --git a/firestore/integration_test_internal/CMakeLists.txt b/firestore/integration_test_internal/CMakeLists.txt index d9d7d20849..96c76bd824 100644 --- a/firestore/integration_test_internal/CMakeLists.txt +++ b/firestore/integration_test_internal/CMakeLists.txt @@ -333,6 +333,13 @@ else() ${FIREBASE_INTEGRATION_TEST_SRCS} ) + # Set a preprocessor define so that tests can distinguish between having been + # built by Xcode vs. cmake. + target_compile_definitions(${integration_test_target_name} + PRIVATE + FIREBASE_TESTS_BUILT_BY_CMAKE + ) + if(APPLE) set(ADDITIONAL_LIBS gssapi_krb5 diff --git a/firestore/integration_test_internal/src/leveldb_snappy_test.cc b/firestore/integration_test_internal/src/leveldb_snappy_test.cc index 33f2247cd8..9ba163fbfb 100644 --- a/firestore/integration_test_internal/src/leveldb_snappy_test.cc +++ b/firestore/integration_test_internal/src/leveldb_snappy_test.cc @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -30,7 +31,6 @@ #include "Firestore/core/src/util/filesystem.h" #include "Firestore/core/src/util/path.h" -#include "firebase_test_framework.h" #include "gtest/gtest.h" #include "leveldb/db.h" @@ -49,36 +49,109 @@ using ::firebase::firestore::util::Path; // with reason "corruption". Path CreateLevelDbDatabaseThatUsesSnappyCompression(); -// This test ensures that we don't accidentally regress having added in Snappy -// compression support (https://github.com/firebase/firebase-ios-sdk/pull/9596). -TEST(LevelDbSnappy, LevelDbHasSnappySupportCompiledIn) { - // Do not run this test on iOS because LevelDb in iOS does not support Snappy. - SKIP_TEST_ON_IOS; +// Creates and opens a LevelDb database that contains at least one block that +// is compressed with Snappy compression, then iterates over it, invoking the +// given callback with the status at each point in the iteration. Once the +// callback is invoked with a `status` where `status.ok()` is not true, then +// iteration will stop and the callback will not be invoked again. +void IterateOverLevelDbDatabaseThatUsesSnappyCompression( + std::function); - Path leveldb_path = CreateLevelDbDatabaseThatUsesSnappyCompression(); - if (HasFatalFailure()) return; +#if FIREBASE_TESTS_BUILT_BY_CMAKE - leveldb::Options options; - options.create_if_missing = false; +// Ensure that LevelDb is compiled with Snappy compression support. +// See https://github.com/firebase/firebase-ios-sdk/pull/9596 for details. +TEST(LevelDbSnappy, LevelDbSupportsSnappy) { + IterateOverLevelDbDatabaseThatUsesSnappyCompression( + [](const leveldb::Status& status) { + ASSERT_TRUE(status.ok()) << ConvertStatus(status); + }); +} + +#else // FIREBASE_TESTS_BUILT_BY_CMAKE + +// Ensure that LevelDb is NOT compiled with Snappy compression support. +TEST(LevelDbSnappy, LevelDbDoesNotSupportSnappy) { + bool got_failed_status = false; + IterateOverLevelDbDatabaseThatUsesSnappyCompression( + [&](const leveldb::Status& status) { + if (!status.ok()) { + got_failed_status = true; + ASSERT_TRUE(status.IsCorruption()) << ConvertStatus(status); + } + }); + if (!HasFailure()) { + ASSERT_TRUE(got_failed_status) + << "Reading a Snappy-compressed LevelDb database was successful; " + "however, it should NOT have been successful " + "since Snappy support is expected to NOT be available."; + } +} + +#endif // FIREBASE_TESTS_BUILT_BY_CMAKE + +void IterateOverLevelDbDatabaseThatUsesSnappyCompression( + std::function callback) { std::unique_ptr db; { + Path leveldb_path = CreateLevelDbDatabaseThatUsesSnappyCompression(); + if (leveldb_path.empty()) { + return; + } + + leveldb::Options options; + options.create_if_missing = false; + leveldb::DB* db_ptr; leveldb::Status status = leveldb::DB::Open(options, leveldb_path.ToUtf8String(), &db_ptr); - ASSERT_TRUE(status.ok()); + + ASSERT_TRUE(status.ok()) + << "Opening LevelDb database " << leveldb_path.ToUtf8String() + << " failed: " << ConvertStatus(status); + db.reset(db_ptr); } - // One of the assertions below will fail when LevelDb attempts to read a block - // that is compressed with Snappy and Snappy compression support is not - // compiled in. std::unique_ptr it( db->NewIterator(leveldb::ReadOptions())); for (it->SeekToFirst(); it->Valid(); it->Next()) { - ASSERT_TRUE(it->status().ok()) << ConvertStatus(it->status()); + callback(it->status()); + if (!it->status().ok()) { + return; + } + } + + // Invoke the callback on the final status. + callback(it->status()); +} + +template +void WriteFile(const Path& dir, + const std::string& file_name, + const T& data_array) { + Filesystem* fs = Filesystem::Default(); + { + auto status = fs->RecursivelyCreateDir(dir); + if (!status.ok()) { + FAIL() << "Creating directory failed: " << dir.ToUtf8String() << " (" + << status.error_message() << ")"; + } + } + + Path file = dir.AppendUtf8(file_name); + std::ofstream out_file(file.native_value(), std::ios::binary); + if (!out_file) { + FAIL() << "Unable to open file for writing: " << file.ToUtf8String(); + } + + out_file.write(reinterpret_cast(data_array.data()), + data_array.size()); + out_file.close(); + if (!out_file) { + FAIL() << "Writing to file failed: " << file.ToUtf8String(); } - ASSERT_TRUE(it->status().ok()) << ConvertStatus(it->status()); } const std::array LevelDbSnappyFile_000005_ldb{ @@ -196,47 +269,27 @@ const std::array LevelDbSnappyFile_MANIFEST_000084{ 0x04, 0x0D, }; -template -void WriteFile(const Path& dir, - const std::string& file_name, - const T& data_array) { - Filesystem* fs = Filesystem::Default(); - { - auto status = fs->RecursivelyCreateDir(dir); - if (!status.ok()) { - FAIL() << "Creating directory failed: " << dir.ToUtf8String() << " (" - << status.error_message() << ")"; - } - } - - Path file = dir.AppendUtf8(file_name); - std::ofstream out_file(file.native_value(), std::ios::binary); - if (!out_file) { - FAIL() << "Unable to open file for writing: " << file.ToUtf8String(); - } - - out_file.write(reinterpret_cast(data_array.data()), - data_array.size()); - out_file.close(); - if (!out_file) { - FAIL() << "Writing to file failed: " << file.ToUtf8String(); - } -} - Path LevelDbDir() { Filesystem* fs = Filesystem::Default(); - Path dir = fs->TempDir().AppendUtf8("PersistenceTesting"); + Path dir = fs->TempDir().AppendUtf8("LevelDbSnappyTest"); // Delete the directory first to ensure isolation between runs. auto status = fs->RecursivelyRemove(dir); - EXPECT_TRUE(status.ok()) << "Failed to clean up leveldb in dir " + EXPECT_TRUE(status.ok()) << "Failed to clean up leveldb in directory " << dir.ToUtf8String() << ": " << status.ToString(); + if (! status.ok()) { + return {}; + } return dir; } Path CreateLevelDbDatabaseThatUsesSnappyCompression() { Path leveldb_dir = LevelDbDir(); + if (leveldb_dir.empty()){ + return {}; + } + WriteFile(leveldb_dir, "000005.ldb", LevelDbSnappyFile_000005_ldb); WriteFile(leveldb_dir, "000017.ldb", LevelDbSnappyFile_000017_ldb); WriteFile(leveldb_dir, "000085.ldb", LevelDbSnappyFile_000085_ldb); @@ -244,6 +297,7 @@ Path CreateLevelDbDatabaseThatUsesSnappyCompression() { WriteFile(leveldb_dir, "LOG.old", LevelDbSnappyFile_LOG_old); WriteFile(leveldb_dir, "LOG", LevelDbSnappyFile_LOG); WriteFile(leveldb_dir, "MANIFEST-000084", LevelDbSnappyFile_MANIFEST_000084); + return leveldb_dir; } From 207d95bb81b8a8c82d3b46001e2f30692d30d128 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 13 Apr 2022 18:49:43 -0400 Subject: [PATCH 2/3] leveldb_snappy_test.cc: format code --- .../integration_test_internal/src/leveldb_snappy_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/firestore/integration_test_internal/src/leveldb_snappy_test.cc b/firestore/integration_test_internal/src/leveldb_snappy_test.cc index 9ba163fbfb..4d82878691 100644 --- a/firestore/integration_test_internal/src/leveldb_snappy_test.cc +++ b/firestore/integration_test_internal/src/leveldb_snappy_test.cc @@ -277,7 +277,7 @@ Path LevelDbDir() { auto status = fs->RecursivelyRemove(dir); EXPECT_TRUE(status.ok()) << "Failed to clean up leveldb in directory " << dir.ToUtf8String() << ": " << status.ToString(); - if (! status.ok()) { + if (!status.ok()) { return {}; } @@ -286,7 +286,7 @@ Path LevelDbDir() { Path CreateLevelDbDatabaseThatUsesSnappyCompression() { Path leveldb_dir = LevelDbDir(); - if (leveldb_dir.empty()){ + if (leveldb_dir.empty()) { return {}; } From 73425df5343d786f5bc33fe44726cba6105c6e50 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Fri, 22 Apr 2022 12:32:35 -0400 Subject: [PATCH 3/3] Rename FIREBASE_TESTS_BUILT_BY_CMAKE to FIREBASE_TESTS_TARGET_DESKTOP --- firestore/integration_test_internal/CMakeLists.txt | 7 ++++--- .../integration_test_internal/src/leveldb_snappy_test.cc | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/firestore/integration_test_internal/CMakeLists.txt b/firestore/integration_test_internal/CMakeLists.txt index 96c76bd824..4e03212053 100644 --- a/firestore/integration_test_internal/CMakeLists.txt +++ b/firestore/integration_test_internal/CMakeLists.txt @@ -333,11 +333,12 @@ else() ${FIREBASE_INTEGRATION_TEST_SRCS} ) - # Set a preprocessor define so that tests can distinguish between having been - # built by Xcode vs. cmake. + # Set a preprocessor define so that tests can distinguish between tests for + # the desktop platforms (e.g. Windows, macOS, or Linux) and mobile platforms + # (e.g. Android, iOS). target_compile_definitions(${integration_test_target_name} PRIVATE - FIREBASE_TESTS_BUILT_BY_CMAKE + FIREBASE_TESTS_TARGET_DESKTOP ) if(APPLE) diff --git a/firestore/integration_test_internal/src/leveldb_snappy_test.cc b/firestore/integration_test_internal/src/leveldb_snappy_test.cc index 4d82878691..f17f04ba14 100644 --- a/firestore/integration_test_internal/src/leveldb_snappy_test.cc +++ b/firestore/integration_test_internal/src/leveldb_snappy_test.cc @@ -57,7 +57,7 @@ Path CreateLevelDbDatabaseThatUsesSnappyCompression(); void IterateOverLevelDbDatabaseThatUsesSnappyCompression( std::function); -#if FIREBASE_TESTS_BUILT_BY_CMAKE +#if FIREBASE_TESTS_TARGET_DESKTOP // Ensure that LevelDb is compiled with Snappy compression support. // See https://github.com/firebase/firebase-ios-sdk/pull/9596 for details. @@ -68,7 +68,7 @@ TEST(LevelDbSnappy, LevelDbSupportsSnappy) { }); } -#else // FIREBASE_TESTS_BUILT_BY_CMAKE +#else // FIREBASE_TESTS_TARGET_DESKTOP // Ensure that LevelDb is NOT compiled with Snappy compression support. TEST(LevelDbSnappy, LevelDbDoesNotSupportSnappy) { @@ -89,7 +89,7 @@ TEST(LevelDbSnappy, LevelDbDoesNotSupportSnappy) { } } -#endif // FIREBASE_TESTS_BUILT_BY_CMAKE +#endif // FIREBASE_TESTS_TARGET_DESKTOP void IterateOverLevelDbDatabaseThatUsesSnappyCompression( std::function callback) {