From 0e3f78bf35f50efef20310b691e0bdd50fd144d9 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Thu, 7 Sep 2023 16:05:50 -0400 Subject: [PATCH] Firestore: FIRQueryTests.mm: Add test testBloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters (#11679) --- .../Tests/Integration/API/FIRQueryTests.mm | 140 +++++++++++++++++ .../Example/Tests/Util/FSTTestingHooks.h | 3 + .../Example/Tests/Util/FSTTestingHooks.mm | 45 +++++- Firestore/core/src/remote/remote_event.cc | 58 ++++--- Firestore/core/src/remote/remote_event.h | 11 +- Firestore/core/src/util/testing_hooks.h | 20 +++ .../core/test/unit/util/testing_hooks_test.cc | 142 ++++++++++++++---- 7 files changed, 361 insertions(+), 58 deletions(-) diff --git a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm index 3d4d3d70222..22f9ecf472a 100644 --- a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm @@ -1285,4 +1285,144 @@ - (void)testResumingAQueryShouldUseBloomFilterToAvoidFullRequery { } } +- (void)testBloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters { + // TODO(b/291365820): Stop skipping this test when running against the Firestore emulator once + // the emulator is improved to include a bloom filter in the existence filter messages that it + // sends. + XCTSkipIf([FSTIntegrationTestCase isRunningAgainstEmulator], + "Skip this test when running against the Firestore emulator because the emulator does " + "not include a bloom filter when it sends existence filter messages, making it " + "impossible for this test to verify the correctness of the bloom filter."); + + // Set this test to stop when the first failure occurs because some test assertion failures make + // the rest of the test not applicable or will even crash. + [self setContinueAfterFailure:NO]; + + // Define a comparator that compares `NSString` objects in a way that orders canonically- + // equivalent, but distinct, strings in a consistent manner by using `NSForcedOrderingSearch`. + // Otherwise, the bare `[NSString compare:]` method considers canonically-equivalent, but + // distinct, strings as "equal" and orders them indeterminately. + NSComparator sortComparator = ^(NSString *string1, NSString *string2) { + return [string1 compare:string2 options:NSForcedOrderingSearch]; + }; + + // Firestore does not do any Unicode normalization on the document IDs. Therefore, two document + // IDs that are canonically-equivalent (i.e. they visually appear identical) but are represented + // by a different sequence of Unicode code points are treated as distinct document IDs. + NSArray *testDocIds; + { + NSMutableArray *testDocIdsAccumulator = [[NSMutableArray alloc] init]; + [testDocIdsAccumulator addObject:@"DocumentToDelete"]; + // The next two strings both end with "e" with an accent: the first uses the dedicated Unicode + // code point for this character, while the second uses the standard lowercase "e" followed by + // the accent combining character. + [testDocIdsAccumulator addObject:@"LowercaseEWithAcuteAccent_\u00E9"]; + [testDocIdsAccumulator addObject:@"LowercaseEWithAcuteAccent_\u0065\u0301"]; + // The next two strings both end with an "e" with two different accents applied via the + // following two combining characters. The combining characters are specified in a different + // order and Firestore treats these document IDs as unique, despite the order of the combining + // characters being irrelevant. + [testDocIdsAccumulator addObject:@"LowercaseEWithMultipleAccents_\u0065\u0301\u0327"]; + [testDocIdsAccumulator addObject:@"LowercaseEWithMultipleAccents_\u0065\u0327\u0301"]; + // The next string contains a character outside the BMP (the "basic multilingual plane"); that + // is, its code point is greater than 0xFFFF. Since NSString stores text in sequences of 16-bit + // code units, using the UTF-16 encoding (according to + // https://www.objc.io/issues/9-strings/unicode) it is stored as a surrogate pair, two 16-bit + // code units U+D83D and U+DE00, to represent this character. Make sure that its presence is + // correctly tested in the bloom filter, which uses UTF-8 encoding. + [testDocIdsAccumulator addObject:@"Smiley_\U0001F600"]; + + testDocIds = [NSArray arrayWithArray:testDocIdsAccumulator]; + } + + // Verify assumptions about the equivalence of strings in `testDocIds`. + XCTAssertEqualObjects(testDocIds[1].decomposedStringWithCanonicalMapping, + testDocIds[2].decomposedStringWithCanonicalMapping); + XCTAssertEqualObjects(testDocIds[3].decomposedStringWithCanonicalMapping, + testDocIds[4].decomposedStringWithCanonicalMapping); + XCTAssertEqual([testDocIds[5] characterAtIndex:7], 0xD83D); + XCTAssertEqual([testDocIds[5] characterAtIndex:8], 0xDE00); + + // Create the mapping from document ID to document data for the document IDs specified in + // `testDocIds`. + NSMutableDictionary *> *testDocs = + [[NSMutableDictionary alloc] init]; + for (NSString *testDocId in testDocIds) { + [testDocs setValue:@{@"foo" : @42} forKey:testDocId]; + } + + // Create the documents whose names contain complex Unicode characters in a new collection. + FIRCollectionReference *collRef = [self collectionRefWithDocuments:testDocs]; + + // Run a query to populate the local cache with documents that have names with complex Unicode + // characters. + { + FIRQuerySnapshot *querySnapshot1 = [self readDocumentSetForRef:collRef + source:FIRFirestoreSourceDefault]; + XCTAssertEqualObjects( + [FIRQuerySnapshotGetIDs(querySnapshot1) sortedArrayUsingComparator:sortComparator], + [testDocIds sortedArrayUsingComparator:sortComparator], + @"querySnapshot1 has the wrong documents"); + } + + // Delete one of the documents so that the next call to collection.get() will experience an + // existence filter mismatch. Use a different Firestore instance to avoid affecting the local + // cache. + FIRDocumentReference *documentToDelete = [collRef documentWithPath:@"DocumentToDelete"]; + { + FIRFirestore *db2 = [self firestore]; + [self deleteDocumentRef:[db2 documentWithPath:documentToDelete.path]]; + } + + // Wait for 10 seconds, during which Watch will stop tracking the query and will send an + // existence filter rather than "delete" events when the query is resumed. + [NSThread sleepForTimeInterval:10.0f]; + + // Resume the query and save the resulting snapshot for verification. Use some internal testing + // hooks to "capture" the existence filter mismatches. + __block FIRQuerySnapshot *querySnapshot2; + NSArray *existenceFilterMismatches = + [FSTTestingHooks captureExistenceFilterMismatchesDuringBlock:^{ + querySnapshot2 = [self readDocumentSetForRef:collRef source:FIRFirestoreSourceDefault]; + }]; + + // Verify that the snapshot from the resumed query contains the expected documents; that is, that + // it contains the documents whose names contain complex Unicode characters and _not_ the document + // that was deleted. + { + NSMutableArray *querySnapshot2ExpectedDocumentIds = + [NSMutableArray arrayWithArray:testDocIds]; + [querySnapshot2ExpectedDocumentIds removeObject:documentToDelete.documentID]; + XCTAssertEqualObjects( + [FIRQuerySnapshotGetIDs(querySnapshot2) sortedArrayUsingComparator:sortComparator], + [querySnapshot2ExpectedDocumentIds sortedArrayUsingComparator:sortComparator], + @"querySnapshot2 has the wrong documents"); + } + + // Verify that Watch sent an existence filter with the correct counts. + XCTAssertEqual(existenceFilterMismatches.count, 1u, + @"Watch should have sent exactly 1 existence filter"); + FSTTestingHooksExistenceFilterMismatchInfo *existenceFilterMismatchInfo = + existenceFilterMismatches[0]; + XCTAssertEqual(existenceFilterMismatchInfo.localCacheCount, (int)testDocIds.count); + XCTAssertEqual(existenceFilterMismatchInfo.existenceFilterCount, (int)testDocIds.count - 1); + + // Verify that Watch sent a valid bloom filter. + FSTTestingHooksBloomFilter *bloomFilter = existenceFilterMismatchInfo.bloomFilter; + XCTAssertNotNil(bloomFilter, "Watch should have included a bloom filter in the existence filter"); + + // The bloom filter application should statistically be successful almost every time; the _only_ + // time when it would _not_ be successful is if there is a false positive when testing for + // 'DocumentToDelete' in the bloom filter. So verify that the bloom filter application is + // successful, unless there was a false positive. + BOOL isFalsePositive = [bloomFilter mightContain:documentToDelete]; + XCTAssertEqual(bloomFilter.applied, !isFalsePositive); + + // Verify that the bloom filter contains the document paths with complex Unicode characters. + for (FIRDocumentSnapshot *documentSnapshot in querySnapshot2.documents) { + XCTAssertTrue([bloomFilter mightContain:documentSnapshot.reference], + @"The bloom filter should contain %@", documentSnapshot.documentID); + } +} + @end diff --git a/Firestore/Example/Tests/Util/FSTTestingHooks.h b/Firestore/Example/Tests/Util/FSTTestingHooks.h index d6d6cbcbec9..5605111ded2 100644 --- a/Firestore/Example/Tests/Util/FSTTestingHooks.h +++ b/Firestore/Example/Tests/Util/FSTTestingHooks.h @@ -47,6 +47,9 @@ NS_ASSUME_NONNULL_BEGIN /** The number of bits of padding in the last byte of the bloom filter. */ @property(nonatomic, readonly) int padding; +/** Returns whether the bloom filter contains the given document. */ +- (BOOL)mightContain:(FIRDocumentReference*)documentRef; + @end // @interface FSTTestingHooksBloomFilter #pragma mark - FSTTestingHooksExistenceFilterMismatchInfo diff --git a/Firestore/Example/Tests/Util/FSTTestingHooks.mm b/Firestore/Example/Tests/Util/FSTTestingHooks.mm index 536fe79fd0b..0c5f5941915 100644 --- a/Firestore/Example/Tests/Util/FSTTestingHooks.mm +++ b/Firestore/Example/Tests/Util/FSTTestingHooks.mm @@ -47,34 +47,63 @@ @interface FSTTestingHooksBloomFilter () - (instancetype)initWithApplied:(BOOL)applied hashCount:(int)hashCount bitmapLength:(int)bitmapLength - padding:(int)padding NS_DESIGNATED_INITIALIZER; + padding:(int)padding + projectId:(std::string)projectId + databaseId:(std::string)databaseId + bloomFilter:(absl::optional)bloomFilter NS_DESIGNATED_INITIALIZER; -- (instancetype)initWithBloomFilterInfo:(const TestingHooks::BloomFilterInfo&)bloomFilterInfo; +- (instancetype)initWithBloomFilterInfo:(const TestingHooks::BloomFilterInfo&)bloomFilterInfo + projectId:(std::string)projectId + databaseId:(std::string)databaseId; @end #pragma mark - FSTTestingHooksBloomFilter implementation -@implementation FSTTestingHooksBloomFilter +@implementation FSTTestingHooksBloomFilter { + std::string _projectId; + std::string _databaseId; + absl::optional _bloomFilter; +} - (instancetype)initWithApplied:(BOOL)applied hashCount:(int)hashCount bitmapLength:(int)bitmapLength - padding:(int)padding { + padding:(int)padding + projectId:(std::string)projectId + databaseId:(std::string)databaseId + bloomFilter:(absl::optional)bloomFilter { if (self = [super init]) { _applied = applied; _hashCount = hashCount; _bitmapLength = bitmapLength; _padding = padding; + _projectId = std::move(projectId); + _databaseId = std::move(databaseId); + _bloomFilter = std::move(bloomFilter); } return self; } -- (instancetype)initWithBloomFilterInfo:(const TestingHooks::BloomFilterInfo&)bloomFilterInfo { +- (instancetype)initWithBloomFilterInfo:(const TestingHooks::BloomFilterInfo&)bloomFilterInfo + projectId:(std::string)projectId + databaseId:(std::string)databaseId { return [self initWithApplied:bloomFilterInfo.applied hashCount:bloomFilterInfo.hash_count bitmapLength:bloomFilterInfo.bitmap_length - padding:bloomFilterInfo.padding]; + padding:bloomFilterInfo.padding + projectId:std::move(projectId) + databaseId:std::move(databaseId) + bloomFilter:bloomFilterInfo.bloom_filter]; +} + +- (BOOL)mightContain:(FIRDocumentReference*)documentRef { + if (!_bloomFilter.has_value()) { + return NO; + } + std::string document_path = StringFormat("projects/%s/databases/%s/documents/%s", _projectId, + _databaseId, MakeStringView(documentRef.path)); + return _bloomFilter->MightContain(document_path); } @end @@ -114,7 +143,9 @@ - (instancetype)initWithExistenceFilterMismatchInfo: FSTTestingHooksBloomFilter* bloomFilter; if (existenceFilterMismatchInfo.bloom_filter.has_value()) { bloomFilter = [[FSTTestingHooksBloomFilter alloc] - initWithBloomFilterInfo:existenceFilterMismatchInfo.bloom_filter.value()]; + initWithBloomFilterInfo:existenceFilterMismatchInfo.bloom_filter.value() + projectId:existenceFilterMismatchInfo.project_id + databaseId:existenceFilterMismatchInfo.database_id]; } else { bloomFilter = nil; } diff --git a/Firestore/core/src/remote/remote_event.cc b/Firestore/core/src/remote/remote_event.cc index df9ee5848cb..52e83cbfbaf 100644 --- a/Firestore/core/src/remote/remote_event.cc +++ b/Firestore/core/src/remote/remote_event.cc @@ -216,21 +216,25 @@ namespace { TestingHooks::ExistenceFilterMismatchInfo create_existence_filter_mismatch_info_for_testing_hooks( - BloomFilterApplicationStatus status, int local_cache_count, - const ExistenceFilterWatchChange& existence_filter) { - absl::optional bloom_filter; + const ExistenceFilterWatchChange& existence_filter, + const DatabaseId& database_id, + absl::optional bloom_filter, + BloomFilterApplicationStatus status) { + absl::optional bloom_filter_info; if (existence_filter.filter().bloom_filter_parameters().has_value()) { const BloomFilterParameters& bloom_filter_parameters = existence_filter.filter().bloom_filter_parameters().value(); - bloom_filter = {status == BloomFilterApplicationStatus::kSuccess, - bloom_filter_parameters.hash_count, - static_cast(bloom_filter_parameters.bitmap.size()), - bloom_filter_parameters.padding}; + bloom_filter_info = { + status == BloomFilterApplicationStatus::kSuccess, + bloom_filter_parameters.hash_count, + static_cast(bloom_filter_parameters.bitmap.size()), + bloom_filter_parameters.padding, std::move(bloom_filter)}; } return {local_cache_count, existence_filter.filter().count(), - std::move(bloom_filter)}; + database_id.project_id(), database_id.database_id(), + std::move(bloom_filter_info)}; } } // namespace @@ -264,8 +268,13 @@ void WatchChangeAggregator::HandleExistenceFilter( int current_size = GetCurrentDocumentCountForTarget(target_id); if (current_size != expected_count) { // Apply bloom filter to identify and mark removed documents. + absl::optional bloom_filter = + ParseBloomFilter(existence_filter); BloomFilterApplicationStatus status = - ApplyBloomFilter(existence_filter, current_size); + bloom_filter.has_value() + ? ApplyBloomFilter(bloom_filter.value(), existence_filter, + current_size) + : BloomFilterApplicationStatus::kSkipped; if (status != BloomFilterApplicationStatus::kSuccess) { // If bloom filter application fails, we reset the mapping and // trigger re-run of the query. @@ -279,18 +288,20 @@ void WatchChangeAggregator::HandleExistenceFilter( TestingHooks::GetInstance().NotifyOnExistenceFilterMismatch( create_existence_filter_mismatch_info_for_testing_hooks( - status, current_size, existence_filter)); + current_size, existence_filter, + target_metadata_provider_->GetDatabaseId(), + std::move(bloom_filter), status)); } } } } -BloomFilterApplicationStatus WatchChangeAggregator::ApplyBloomFilter( - const ExistenceFilterWatchChange& existence_filter, int current_count) { +absl::optional WatchChangeAggregator::ParseBloomFilter( + const ExistenceFilterWatchChange& existence_filter) { const absl::optional& bloom_filter_parameters = existence_filter.filter().bloom_filter_parameters(); if (!bloom_filter_parameters.has_value()) { - return BloomFilterApplicationStatus::kSkipped; + return absl::nullopt; } util::StatusOr maybe_bloom_filter = @@ -300,23 +311,30 @@ BloomFilterApplicationStatus WatchChangeAggregator::ApplyBloomFilter( if (!maybe_bloom_filter.ok()) { LOG_WARN("Creating BloomFilter failed: %s", maybe_bloom_filter.status().error_message()); - return BloomFilterApplicationStatus::kSkipped; + return absl::nullopt; } BloomFilter bloom_filter = std::move(maybe_bloom_filter).ValueOrDie(); if (bloom_filter.bit_count() == 0) { - return BloomFilterApplicationStatus::kSkipped; + return absl::nullopt; } + return bloom_filter; +} + +BloomFilterApplicationStatus WatchChangeAggregator::ApplyBloomFilter( + const BloomFilter& bloom_filter, + const ExistenceFilterWatchChange& existence_filter, + int current_count) { + int expected_count = existence_filter.filter().count(); + int removed_document_count = FilterRemovedDocuments(bloom_filter, existence_filter.target_id()); - int expected_count = existence_filter.filter().count(); - if (expected_count != (current_count - removed_document_count)) { - return BloomFilterApplicationStatus::kFalsePositive; - } - return BloomFilterApplicationStatus::kSuccess; + return (expected_count == (current_count - removed_document_count)) + ? BloomFilterApplicationStatus::kSuccess + : BloomFilterApplicationStatus::kFalsePositive; } int WatchChangeAggregator::FilterRemovedDocuments( diff --git a/Firestore/core/src/remote/remote_event.h b/Firestore/core/src/remote/remote_event.h index 0d5331a0d7c..5e156bcf2ce 100644 --- a/Firestore/core/src/remote/remote_event.h +++ b/Firestore/core/src/remote/remote_event.h @@ -417,12 +417,21 @@ class WatchChangeAggregator { bool TargetContainsDocument(model::TargetId target_id, const model::DocumentKey& key); + /** + * Parse the bloom filter from the "unchanged_names" field of an existence + * filter. + */ + static absl::optional ParseBloomFilter( + const ExistenceFilterWatchChange& existence_filter); + /** * Apply bloom filter to remove the deleted documents, and return the * application status. */ BloomFilterApplicationStatus ApplyBloomFilter( - const ExistenceFilterWatchChange& existence_filter, int current_count); + const BloomFilter& bloom_filter, + const ExistenceFilterWatchChange& existence_filter, + int current_count); /** * Filter out removed documents based on bloom filter membership result and diff --git a/Firestore/core/src/util/testing_hooks.h b/Firestore/core/src/util/testing_hooks.h index 60dde001a9b..f9d05b96d7e 100644 --- a/Firestore/core/src/util/testing_hooks.h +++ b/Firestore/core/src/util/testing_hooks.h @@ -20,9 +20,11 @@ #include #include #include // NOLINT(build/c++11) +#include #include #include "Firestore/core/src/api/listener_registration.h" +#include "Firestore/core/src/remote/bloom_filter.h" #include "Firestore/core/src/util/no_destructor.h" #include "absl/types/optional.h" @@ -60,6 +62,12 @@ class TestingHooks final { /** The number of bits of padding in the last byte of the bloom filter. */ int padding = -1; + + /** + * The BloomFilter created from the existence filter; may be empty if + * creating it failed. + */ + absl::optional bloom_filter; }; /** @@ -76,6 +84,18 @@ class TestingHooks final { */ int existence_filter_count = -1; + /** + * The projectId used when checking documents for membership in the bloom + * filter. + */ + std::string project_id; + + /** + * The databaseId used when checking documents for membership in the bloom + * filter. + */ + std::string database_id; + /** * Information about the bloom filter provided by Watch in the * ExistenceFilter message's `unchangedNames` field. If empty, then that diff --git a/Firestore/core/test/unit/util/testing_hooks_test.cc b/Firestore/core/test/unit/util/testing_hooks_test.cc index 0032e82fc15..b7f27bc6e3d 100644 --- a/Firestore/core/test/unit/util/testing_hooks_test.cc +++ b/Firestore/core/test/unit/util/testing_hooks_test.cc @@ -19,13 +19,17 @@ #include // NOLINT(build/c++11) #include // NOLINT(build/c++11) #include +#include #include // NOLINT(build/c++11) -#include // NOLINT(build/c++11) #include "Firestore/core/src/api/listener_registration.h" +#include "Firestore/core/src/nanopb/byte_string.h" +#include "Firestore/core/src/remote/bloom_filter.h" #include "Firestore/core/src/util/defer.h" #include "Firestore/core/test/unit/testutil/async_testing.h" +#include "absl/types/optional.h" + #include "gtest/gtest.h" namespace { @@ -33,6 +37,8 @@ namespace { using namespace std::chrono_literals; // NOLINT(build/namespaces) using firebase::firestore::api::ListenerRegistration; +using firebase::firestore::nanopb::ByteString; +using firebase::firestore::remote::BloomFilter; using firebase::firestore::testutil::AsyncTest; using firebase::firestore::util::Defer; using firebase::firestore::util::TestingHooks; @@ -41,18 +47,58 @@ using ExistenceFilterMismatchInfoAccumulator = firebase::firestore::testutil::AsyncAccumulator< TestingHooks::ExistenceFilterMismatchInfo>; +/** + * Creates and returns a new `ExistenceFilterMismatchInfo` object populated + * with sample values. + * + * Each invocation of this function with the same argument will return an + * object populated with the same values as the previous invocation. + * + * @param seed The value to incorporate into the sample values populated in the + * returned object; a different seed will produce different sample values. + * + * @return A new `ExistenceFilterMismatchInfo` object populated with sample + * values based on the given `seed`. The object's `bloom_filter` member will + * contain a `BloomFilterInfo` whose `bloom_filter` member will contain a + * `BloomFilter`. + */ +TestingHooks::ExistenceFilterMismatchInfo SampleExistenceFilterMismatchInfo( + int seed = 0) { + int local_cache_count = 123 + seed; + int existence_filter_count = 456 + seed; + std::string project_id = "test_project_id" + std::to_string(seed); + std::string database_id = "test_database_id" + std::to_string(seed); + + std::string bloom_filter_bytes = "sample_bytes" + std::to_string(seed); + bool bloom_filter_applied = (seed % 2 == 0); + int bloom_filter_hash_count = 42 + seed; + int bloom_filter_bitmap_length = static_cast(bloom_filter_bytes.size()); + int bloom_filter_padding = (seed % 8); + + return {local_cache_count, existence_filter_count, project_id, database_id, + TestingHooks::BloomFilterInfo{ + bloom_filter_applied, bloom_filter_hash_count, + bloom_filter_bitmap_length, bloom_filter_padding, + BloomFilter(ByteString(bloom_filter_bytes), bloom_filter_padding, + bloom_filter_hash_count)}}; +} + class TestingHooksTest : public ::testing::Test, public AsyncTest { public: void AssertAccumulatedObject( const std::shared_ptr& accumulator, - TestingHooks::ExistenceFilterMismatchInfo expected) { + const TestingHooks::ExistenceFilterMismatchInfo& expected) { Await(accumulator->WaitForObject()); ASSERT_FALSE(accumulator->IsEmpty()); + TestingHooks::ExistenceFilterMismatchInfo info = accumulator->Shift(); EXPECT_EQ(info.local_cache_count, expected.local_cache_count); EXPECT_EQ(info.existence_filter_count, expected.existence_filter_count); + EXPECT_EQ(info.project_id, expected.project_id); + EXPECT_EQ(info.database_id, expected.database_id); EXPECT_EQ(info.bloom_filter.has_value(), expected.bloom_filter.has_value()); + if (info.bloom_filter.has_value() && expected.bloom_filter.has_value()) { const TestingHooks::BloomFilterInfo& info_bloom_filter = info.bloom_filter.value(); @@ -63,11 +109,18 @@ class TestingHooksTest : public ::testing::Test, public AsyncTest { EXPECT_EQ(info_bloom_filter.bitmap_length, expected_bloom_filter.bitmap_length); EXPECT_EQ(info_bloom_filter.padding, expected_bloom_filter.padding); + EXPECT_EQ(info_bloom_filter.bloom_filter.has_value(), + expected_bloom_filter.bloom_filter.has_value()); + if (info_bloom_filter.bloom_filter.has_value() && + expected_bloom_filter.bloom_filter.has_value()) { + EXPECT_EQ(info_bloom_filter.bloom_filter.value(), + expected_bloom_filter.bloom_filter.value()); + } } } std::future NotifyOnExistenceFilterMismatchAsync( - TestingHooks::ExistenceFilterMismatchInfo info) { + const TestingHooks::ExistenceFilterMismatchInfo& info) { return Async([info]() { TestingHooks::GetInstance().NotifyOnExistenceFilterMismatch(info); }); @@ -86,11 +139,46 @@ TEST_F(TestingHooksTest, OnExistenceFilterMismatchCallbackShouldGetNotified) { TestingHooks::GetInstance().OnExistenceFilterMismatch( accumulator->AsCallback()); Defer unregister_listener([=] { listener_registration->Remove(); }); - TestingHooks::BloomFilterInfo bloom_filter_info{true, 10, 11, 12}; - NotifyOnExistenceFilterMismatchAsync({123, 456, bloom_filter_info}); + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo()); + + AssertAccumulatedObject(accumulator, SampleExistenceFilterMismatchInfo()); +} + +TEST_F( + TestingHooksTest, + // NOLINTNEXTLINE(whitespace/line_length) + OnExistenceFilterMismatchCallbackShouldGetNotifiedWithAbsentExistenceFilterInfo) { + auto accumulator = ExistenceFilterMismatchInfoAccumulator::NewInstance(); + std::shared_ptr listener_registration = + TestingHooks::GetInstance().OnExistenceFilterMismatch( + accumulator->AsCallback()); + Defer unregister_listener([=] { listener_registration->Remove(); }); + TestingHooks::ExistenceFilterMismatchInfo existence_filter_mismatch_info = + SampleExistenceFilterMismatchInfo(); + existence_filter_mismatch_info.bloom_filter = absl::nullopt; + + NotifyOnExistenceFilterMismatchAsync(existence_filter_mismatch_info); + + AssertAccumulatedObject(accumulator, existence_filter_mismatch_info); +} + +TEST_F( + TestingHooksTest, + // NOLINTNEXTLINE(whitespace/line_length) + OnExistenceFilterMismatchCallbackShouldGetNotifiedWithAbsentExistenceFilter) { + auto accumulator = ExistenceFilterMismatchInfoAccumulator::NewInstance(); + std::shared_ptr listener_registration = + TestingHooks::GetInstance().OnExistenceFilterMismatch( + accumulator->AsCallback()); + Defer unregister_listener([=] { listener_registration->Remove(); }); + TestingHooks::ExistenceFilterMismatchInfo existence_filter_mismatch_info = + SampleExistenceFilterMismatchInfo(); + existence_filter_mismatch_info.bloom_filter->bloom_filter = absl::nullopt; + + NotifyOnExistenceFilterMismatchAsync(existence_filter_mismatch_info); - AssertAccumulatedObject(accumulator, {123, 456, bloom_filter_info}); + AssertAccumulatedObject(accumulator, existence_filter_mismatch_info); } TEST_F(TestingHooksTest, @@ -100,14 +188,13 @@ TEST_F(TestingHooksTest, TestingHooks::GetInstance().OnExistenceFilterMismatch( accumulator->AsCallback()); Defer unregister_listener([=] { listener_registration->Remove(); }); - TestingHooks::BloomFilterInfo bloom_filter_info{true, 10, 11, 12}; - - NotifyOnExistenceFilterMismatchAsync({111, 222, bloom_filter_info}); - AssertAccumulatedObject(accumulator, {111, 222, bloom_filter_info}); - NotifyOnExistenceFilterMismatchAsync({333, 444, bloom_filter_info}); - AssertAccumulatedObject(accumulator, {333, 444, bloom_filter_info}); - NotifyOnExistenceFilterMismatchAsync({555, 666, bloom_filter_info}); - AssertAccumulatedObject(accumulator, {555, 666, bloom_filter_info}); + + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo(0)); + AssertAccumulatedObject(accumulator, SampleExistenceFilterMismatchInfo(0)); + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo(1)); + AssertAccumulatedObject(accumulator, SampleExistenceFilterMismatchInfo(1)); + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo(2)); + AssertAccumulatedObject(accumulator, SampleExistenceFilterMismatchInfo(2)); } TEST_F(TestingHooksTest, @@ -122,12 +209,11 @@ TEST_F(TestingHooksTest, TestingHooks::GetInstance().OnExistenceFilterMismatch( accumulator2->AsCallback()); Defer unregister_listener2([=] { listener_registration2->Remove(); }); - TestingHooks::BloomFilterInfo bloom_filter_info{true, 10, 11, 12}; - NotifyOnExistenceFilterMismatchAsync({123, 456, bloom_filter_info}); + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo()); - AssertAccumulatedObject(accumulator1, {123, 456, bloom_filter_info}); - AssertAccumulatedObject(accumulator2, {123, 456, bloom_filter_info}); + AssertAccumulatedObject(accumulator1, SampleExistenceFilterMismatchInfo()); + AssertAccumulatedObject(accumulator2, SampleExistenceFilterMismatchInfo()); } TEST_F(TestingHooksTest, @@ -141,12 +227,11 @@ TEST_F(TestingHooksTest, TestingHooks::GetInstance().OnExistenceFilterMismatch( accumulator->AsCallback()); Defer unregister_listener2([=] { listener_registration1->Remove(); }); - TestingHooks::BloomFilterInfo bloom_filter_info{true, 10, 11, 12}; - NotifyOnExistenceFilterMismatchAsync({123, 456, bloom_filter_info}); + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo()); - AssertAccumulatedObject(accumulator, {123, 456, bloom_filter_info}); - AssertAccumulatedObject(accumulator, {123, 456, bloom_filter_info}); + AssertAccumulatedObject(accumulator, SampleExistenceFilterMismatchInfo()); + AssertAccumulatedObject(accumulator, SampleExistenceFilterMismatchInfo()); std::this_thread::sleep_for(250ms); EXPECT_TRUE(accumulator->IsEmpty()); } @@ -158,9 +243,8 @@ TEST_F(TestingHooksTest, TestingHooks::GetInstance().OnExistenceFilterMismatch( accumulator->AsCallback()); registration->Remove(); - TestingHooks::BloomFilterInfo bloom_filter_info{true, 10, 11, 12}; - NotifyOnExistenceFilterMismatchAsync({123, 456, bloom_filter_info}); + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo()); std::this_thread::sleep_for(250ms); EXPECT_TRUE(accumulator->IsEmpty()); @@ -182,14 +266,13 @@ TEST_F(TestingHooksTest, OnExistenceFilterMismatchRemoveShouldOnlyRemoveOne) { TestingHooks::GetInstance().OnExistenceFilterMismatch( accumulator3->AsCallback()); Defer unregister_listener3([=] { listener_registration3->Remove(); }); - TestingHooks::BloomFilterInfo bloom_filter_info{true, 10, 11, 12}; listener_registration2->Remove(); - NotifyOnExistenceFilterMismatchAsync({123, 456, bloom_filter_info}); + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo()); - AssertAccumulatedObject(accumulator1, {123, 456, bloom_filter_info}); - AssertAccumulatedObject(accumulator3, {123, 456, bloom_filter_info}); + AssertAccumulatedObject(accumulator1, SampleExistenceFilterMismatchInfo()); + AssertAccumulatedObject(accumulator3, SampleExistenceFilterMismatchInfo()); std::this_thread::sleep_for(250ms); EXPECT_TRUE(accumulator2->IsEmpty()); } @@ -203,9 +286,8 @@ TEST_F(TestingHooksTest, OnExistenceFilterMismatchMultipleRemovesHaveNoEffect) { listener_registration->Remove(); listener_registration->Remove(); listener_registration->Remove(); - TestingHooks::BloomFilterInfo bloom_filter_info{true, 10, 11, 12}; - NotifyOnExistenceFilterMismatchAsync({123, 456, bloom_filter_info}); + NotifyOnExistenceFilterMismatchAsync(SampleExistenceFilterMismatchInfo()); std::this_thread::sleep_for(250ms); EXPECT_TRUE(accumulator->IsEmpty());