diff --git a/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm b/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm index 4d51d93577e..cbccbfc6860 100644 --- a/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm +++ b/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm @@ -342,7 +342,7 @@ - (SnapshotVersion)parseVersion:(NSNumber *_Nullable)version { NSString *bitmapEncoded = bitsData[@"bitmap"]; std::string bitmapDecoded; absl::Base64Unescape([bitmapEncoded cStringUsingEncoding:NSASCIIStringEncoding], &bitmapDecoded); - std::vector bitmap(bitmapDecoded.begin(), bitmapDecoded.end()); + ByteString bitmap(bitmapDecoded); // If not specified in proto, default padding and hashCount to 0. int32_t padding = [bitsData[@"padding"] intValue]; diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc index 7ca39e1b00d..ddf84b866e9 100644 --- a/Firestore/core/src/remote/bloom_filter.cc +++ b/Firestore/core/src/remote/bloom_filter.cc @@ -26,6 +26,7 @@ namespace firebase { namespace firestore { namespace remote { +using nanopb::ByteString; using util::Status; using util::StatusOr; @@ -38,9 +39,9 @@ bool HasSameBits(const BloomFilter& lhs, const BloomFilter& rhs) { return true; } - const std::vector& bitmap1 = lhs.bitmap(); - const std::vector& bitmap2 = rhs.bitmap(); - const auto byte_count = static_cast(bitmap1.size()); + const auto byte_count = static_cast(lhs.bitmap().size()); + const uint8_t* bitmap1 = lhs.bitmap().data(); + const uint8_t* bitmap2 = rhs.bitmap().data(); // Compare all bytes from the bitmap, except for the last byte. for (int32_t i = 0; i < byte_count - 1; ++i) { @@ -81,14 +82,12 @@ int32_t BloomFilter::GetBitIndex(const Hash& hash, int32_t hash_index) const { } bool BloomFilter::IsBitSet(int32_t index) const { - uint8_t byte_at_index = bitmap_[index / 8]; + uint8_t byte_at_index = bitmap_.data()[index / 8]; int32_t offset = index % 8; return (byte_at_index & (static_cast(0x01) << offset)) != 0; } -BloomFilter::BloomFilter(std::vector bitmap, - int32_t padding, - int32_t hash_count) +BloomFilter::BloomFilter(ByteString bitmap, int32_t padding, int32_t hash_count) : bit_count_(static_cast(bitmap.size()) * 8 - padding), hash_count_(hash_count), bitmap_(std::move(bitmap)) { @@ -101,7 +100,7 @@ BloomFilter::BloomFilter(std::vector bitmap, HARD_ASSERT(bit_count_ >= 0); } -StatusOr BloomFilter::Create(std::vector bitmap, +StatusOr BloomFilter::Create(ByteString bitmap, int32_t padding, int32_t hash_count) { if (padding < 0 || padding >= 8) { diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h index 6c0facaad10..ce4e6a58296 100644 --- a/Firestore/core/src/remote/bloom_filter.h +++ b/Firestore/core/src/remote/bloom_filter.h @@ -18,7 +18,7 @@ #define FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_H_ #include -#include +#include "Firestore/core/src/nanopb/byte_string.h" #include "Firestore/core/src/util/statusor.h" #include "absl/strings/string_view.h" @@ -28,7 +28,7 @@ namespace remote { class BloomFilter final { public: - BloomFilter(std::vector bitmap, int32_t padding, int32_t hash_count); + BloomFilter(nanopb::ByteString bitmap, int32_t padding, int32_t hash_count); // Copyable & movable. BloomFilter(const BloomFilter&) = default; @@ -42,7 +42,7 @@ class BloomFilter final { * @return a new BloomFilter if the inputs are valid, otherwise returns a not * `ok()` status. */ - static util::StatusOr Create(std::vector bitmap, + static util::StatusOr Create(nanopb::ByteString bitmap, int32_t padding, int32_t hash_count); @@ -75,7 +75,7 @@ class BloomFilter final { } /** Bloom filter's bitmap. */ - const std::vector& bitmap() const { + const nanopb::ByteString& bitmap() const { return bitmap_; } @@ -108,7 +108,7 @@ class BloomFilter final { int32_t hash_count_ = 0; - std::vector bitmap_; + nanopb::ByteString bitmap_; }; bool operator==(const BloomFilter& lhs, const BloomFilter& rhs); diff --git a/Firestore/core/src/remote/existence_filter.h b/Firestore/core/src/remote/existence_filter.h index ed0b2298242..b7b1ada9742 100644 --- a/Firestore/core/src/remote/existence_filter.h +++ b/Firestore/core/src/remote/existence_filter.h @@ -18,8 +18,8 @@ #define FIRESTORE_CORE_SRC_REMOTE_EXISTENCE_FILTER_H_ #include -#include +#include "Firestore/core/src/nanopb/byte_string.h" #include "Firestore/core/src/remote/bloom_filter.h" namespace firebase { @@ -27,7 +27,7 @@ namespace firestore { namespace remote { struct BloomFilterParameters { - std::vector bitmap; + nanopb::ByteString bitmap; int32_t padding; int32_t hash_count; }; diff --git a/Firestore/core/src/remote/serializer.cc b/Firestore/core/src/remote/serializer.cc index e64aefe0f06..f301a65e37f 100644 --- a/Firestore/core/src/remote/serializer.cc +++ b/Firestore/core/src/remote/serializer.cc @@ -1456,15 +1456,13 @@ ExistenceFilter Serializer::DecodeExistenceFilter( int32_t hash_count = filter.unchanged_names.hash_count; int32_t padding = 0; - std::vector bitmap; + ByteString bitmap; if (filter.unchanged_names.has_bits) { padding = filter.unchanged_names.bits.padding; - - pb_bytes_array_t* bitmap_ptr = filter.unchanged_names.bits.bitmap; - if (bitmap_ptr != nullptr) { - bitmap = std::vector(bitmap_ptr->bytes, - bitmap_ptr->bytes + bitmap_ptr->size); - } + // TODO(b/274668697) Steal the bytes using ByteString::Take() instead of + // copying them. To do this, the `filter` argument will need to be + // non-const, which will affect the caller(s), and their caller(s), etc. + bitmap = ByteString(filter.unchanged_names.bits.bitmap); } return {filter.count, BloomFilterParameters{std::move(bitmap), padding, hash_count}}; diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc index 43015e3981c..aaf57b98064 100644 --- a/Firestore/core/test/unit/remote/bloom_filter_test.cc +++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc @@ -18,7 +18,6 @@ #include #include -#include #include "Firestore/core/src/util/hard_assert.h" #include "Firestore/core/src/util/json_reader.h" @@ -31,6 +30,7 @@ namespace firestore { namespace remote { namespace { +using nanopb::ByteString; using nlohmann::json; using util::JsonReader; using util::Path; @@ -38,24 +38,24 @@ using util::Status; using util::StatusOr; TEST(BloomFilterUnitTest, CanInstantiateEmptyBloomFilter) { - BloomFilter bloom_filter(std::vector{}, 0, 0); + BloomFilter bloom_filter(ByteString{}, 0, 0); EXPECT_EQ(bloom_filter.bit_count(), 0); } TEST(BloomFilterUnitTest, CanInstantiateNonEmptyBloomFilter) { { - BloomFilter bloom_filter(std::vector{1}, 0, 1); + BloomFilter bloom_filter(ByteString{1}, 0, 1); EXPECT_EQ(bloom_filter.bit_count(), 8); } { - BloomFilter bloom_filter(std::vector{1}, 7, 1); + BloomFilter bloom_filter(ByteString{1}, 7, 1); EXPECT_EQ(bloom_filter.bit_count(), 1); } } TEST(BloomFilterUnitTest, CreateShouldReturnBloomFilterOnValidInputs) { StatusOr maybe_bloom_filter = - BloomFilter::Create(std::vector{1}, 1, 1); + BloomFilter::Create(ByteString{1}, 1, 1); ASSERT_TRUE(maybe_bloom_filter.ok()); BloomFilter bloom_filter = maybe_bloom_filter.ValueOrDie(); EXPECT_EQ(bloom_filter.bit_count(), 7); @@ -63,7 +63,7 @@ TEST(BloomFilterUnitTest, CreateShouldReturnBloomFilterOnValidInputs) { TEST(BloomFilterUnitTest, CreateShouldBeAbleToCreatEmptyBloomFilter) { StatusOr maybe_bloom_filter = - BloomFilter::Create(std::vector{}, 0, 0); + BloomFilter::Create(ByteString{}, 0, 0); ASSERT_TRUE(maybe_bloom_filter.ok()); BloomFilter bloom_filter = maybe_bloom_filter.ValueOrDie(); EXPECT_EQ(bloom_filter.bit_count(), 0); @@ -72,14 +72,14 @@ TEST(BloomFilterUnitTest, CreateShouldBeAbleToCreatEmptyBloomFilter) { TEST(BloomFilterUnitTest, CreateShouldReturnNotOKStatusOnNegativePadding) { { StatusOr maybe_bloom_filter = - BloomFilter::Create(std::vector{}, -1, 0); + BloomFilter::Create(ByteString{}, -1, 0); ASSERT_FALSE(maybe_bloom_filter.ok()); EXPECT_EQ(maybe_bloom_filter.status().error_message(), "Invalid padding: -1"); } { StatusOr maybe_bloom_filter = - BloomFilter::Create(std::vector{1}, -1, 1); + BloomFilter::Create(ByteString{1}, -1, 1); ASSERT_FALSE(maybe_bloom_filter.ok()); EXPECT_EQ(maybe_bloom_filter.status().error_message(), "Invalid padding: -1"); @@ -89,14 +89,14 @@ TEST(BloomFilterUnitTest, CreateShouldReturnNotOKStatusOnNegativePadding) { TEST(BloomFilterUnitTest, CreateShouldReturnNotOKStatusOnNegativeHashCount) { { StatusOr maybe_bloom_filter = - BloomFilter::Create(std::vector{}, 0, -1); + BloomFilter::Create(ByteString{}, 0, -1); ASSERT_FALSE(maybe_bloom_filter.ok()); EXPECT_EQ(maybe_bloom_filter.status().error_message(), "Invalid hash count: -1"); } { StatusOr maybe_bloom_filter = - BloomFilter::Create(std::vector{1}, 1, -1); + BloomFilter::Create(ByteString{1}, 1, -1); ASSERT_FALSE(maybe_bloom_filter.ok()); EXPECT_EQ(maybe_bloom_filter.status().error_message(), "Invalid hash count: -1"); @@ -105,7 +105,7 @@ TEST(BloomFilterUnitTest, CreateShouldReturnNotOKStatusOnNegativeHashCount) { TEST(BloomFilterUnitTest, CreateShouldReturnNotOKStatusOnZeroHashCount) { StatusOr maybe_bloom_filter = - BloomFilter::Create(std::vector{1}, 1, 0); + BloomFilter::Create(ByteString{1}, 1, 0); ASSERT_FALSE(maybe_bloom_filter.ok()); EXPECT_EQ(maybe_bloom_filter.status().error_message(), "Invalid hash count: 0"); @@ -113,43 +113,43 @@ TEST(BloomFilterUnitTest, CreateShouldReturnNotOKStatusOnZeroHashCount) { TEST(BloomFilterUnitTest, CreateShouldReturnNotOKStatusIfPaddingIsTooLarge) { StatusOr maybe_bloom_filter = - BloomFilter::Create(std::vector{1}, 8, 1); + BloomFilter::Create(ByteString{1}, 8, 1); ASSERT_FALSE(maybe_bloom_filter.ok()); EXPECT_EQ(maybe_bloom_filter.status().error_message(), "Invalid padding: 8"); } TEST(BloomFilterTest, CheckBloomFiltersEqualityWithSameInput) { - BloomFilter bloom_filter1(std::vector{1}, 1, 1); - BloomFilter bloom_filter2(std::vector{1}, 1, 1); + BloomFilter bloom_filter1(ByteString{1}, 1, 1); + BloomFilter bloom_filter2(ByteString{1}, 1, 1); EXPECT_TRUE(bloom_filter1 == bloom_filter2); EXPECT_FALSE(bloom_filter1 != bloom_filter2); } TEST(BloomFilterTest, CheckBloomFiltersEqualityWithDifferentBitmap) { { - BloomFilter bloom_filter1(std::vector{1}, 1, 1); - BloomFilter bloom_filter2(std::vector{2}, 1, 1); + BloomFilter bloom_filter1(ByteString{1}, 1, 1); + BloomFilter bloom_filter2(ByteString{2}, 1, 1); EXPECT_FALSE(bloom_filter1 == bloom_filter2); EXPECT_TRUE(bloom_filter1 != bloom_filter2); } { - BloomFilter bloom_filter1(std::vector{1}, 1, 1); - BloomFilter bloom_filter2(std::vector{1, 1}, 1, 1); + BloomFilter bloom_filter1(ByteString{1}, 1, 1); + BloomFilter bloom_filter2(ByteString{1, 1}, 1, 1); EXPECT_FALSE(bloom_filter1 == bloom_filter2); EXPECT_TRUE(bloom_filter1 != bloom_filter2); } } TEST(BloomFilterTest, CheckBloomFiltersEqualityWithDifferentPadding) { - BloomFilter bloom_filter1(std::vector{1}, 1, 1); - BloomFilter bloom_filter2(std::vector{1}, 2, 1); + BloomFilter bloom_filter1(ByteString{1}, 1, 1); + BloomFilter bloom_filter2(ByteString{1}, 2, 1); EXPECT_FALSE(bloom_filter1 == bloom_filter2); EXPECT_TRUE(bloom_filter1 != bloom_filter2); } TEST(BloomFilterTest, CheckBloomFiltersEqualityWithDifferentHashCount) { - BloomFilter bloom_filter1(std::vector{1}, 1, 1); - BloomFilter bloom_filter2(std::vector{1}, 1, 2); + BloomFilter bloom_filter1(ByteString{1}, 1, 1); + BloomFilter bloom_filter2(ByteString{1}, 1, 2); EXPECT_FALSE(bloom_filter1 == bloom_filter2); EXPECT_TRUE(bloom_filter1 != bloom_filter2); } @@ -159,17 +159,17 @@ TEST(BloomFilterTest, // In BloomFilter bitmap, padding is guaranteed to be less than 8, and // starts counting from the leftmost indexes of the last byte. { - BloomFilter bloom_filter1(std::vector{255, 127}, 1, + BloomFilter bloom_filter1(ByteString{255, 127}, 1, 1); // bitmap -> 11111111 01111111 - BloomFilter bloom_filter2(std::vector{255, 255}, 1, + BloomFilter bloom_filter2(ByteString{255, 255}, 1, 1); // bitmap -> 11111111 11111111 EXPECT_TRUE(bloom_filter1 == bloom_filter2); EXPECT_FALSE(bloom_filter1 != bloom_filter2); } { - BloomFilter bloom_filter1(std::vector{255, 207}, 4, + BloomFilter bloom_filter1(ByteString{255, 207}, 4, 1); // bitmap -> 11111111 11001111 - BloomFilter bloom_filter2(std::vector{255, 255}, 4, + BloomFilter bloom_filter2(ByteString{255, 255}, 4, 1); // bitmap -> 11111111 11111111 EXPECT_TRUE(bloom_filter1 == bloom_filter2); EXPECT_FALSE(bloom_filter1 != bloom_filter2); @@ -178,13 +178,13 @@ TEST(BloomFilterTest, TEST(BloomFilterTest, MightContainCanProcessNonStandardCharacters) { // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑" - BloomFilter bloom_filter(std::vector{237, 5}, 5, 8); + BloomFilter bloom_filter(ByteString{237, 5}, 5, 8); EXPECT_TRUE(bloom_filter.MightContain("ÀÒ∑")); EXPECT_FALSE(bloom_filter.MightContain("Ò∑À")); } TEST(BloomFilterUnitTest, MightContainOnEmptyBloomFilterShouldReturnFalse) { - BloomFilter bloom_filter(std::vector{}, 0, 0); + BloomFilter bloom_filter(ByteString{}, 0, 0); EXPECT_FALSE(bloom_filter.MightContain("")); EXPECT_FALSE(bloom_filter.MightContain("a")); } @@ -192,11 +192,11 @@ TEST(BloomFilterUnitTest, MightContainOnEmptyBloomFilterShouldReturnFalse) { TEST(BloomFilterUnitTest, MightContainWithEmptyStringMightReturnFalsePositiveResult) { { - BloomFilter bloom_filter(std::vector{1}, 1, 1); + BloomFilter bloom_filter(ByteString{1}, 1, 1); EXPECT_FALSE(bloom_filter.MightContain("")); } { - BloomFilter bloom_filter(std::vector{255}, 0, 16); + BloomFilter bloom_filter(ByteString{255}, 0, 16); EXPECT_TRUE(bloom_filter.MightContain("")); } } @@ -241,7 +241,7 @@ class BloomFilterGoldenTest : public ::testing::Test { int hash_count = reader.OptionalInt("hashCount", test_file, 0); std::string decoded; absl::Base64Unescape(bitmap, &decoded); - std::vector decoded_map(decoded.begin(), decoded.end()); + ByteString decoded_map(decoded); StatusOr maybe_bloom_filter = BloomFilter::Create(std::move(decoded_map), padding, hash_count);