From b2e9b9380fb0bf6f6a438180931d170837d984ce Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Fri, 17 Feb 2023 14:46:35 -0800
Subject: [PATCH 01/17] add cpp file

---
 Firestore/core/src/remote/bloom_filter.cpp | 85 +++++++++++++++++++++
 Firestore/core/src/remote/bloom_filter.h   | 88 ++++++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 Firestore/core/src/remote/bloom_filter.cpp
 create mode 100644 Firestore/core/src/remote/bloom_filter.h

diff --git a/Firestore/core/src/remote/bloom_filter.cpp b/Firestore/core/src/remote/bloom_filter.cpp
new file mode 100644
index 00000000000..45adeb92dfa
--- /dev/null
+++ b/Firestore/core/src/remote/bloom_filter.cpp
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "bloom_filter.h"
+#include <CommonCrypto/CommonDigest.h>
+
+namespace firebase {
+namespace firestore {
+namespace remote {
+
+// Helper function to hash the given key string to Hash structure using the
+// MD5 hash function.
+BloomFilter::Hash StringToHash(const absl::string_view key) {
+  unsigned char md5_digest[CC_MD5_DIGEST_LENGTH];
+  static_assert(sizeof(md5_digest) == sizeof(BloomFilter::Hash), "");
+
+  CC_MD5_CTX context;
+  CC_MD5_Init(&context);
+  CC_MD5_Update(&context, key.data(), key.size());
+  CC_MD5_Final(md5_digest, &context);
+
+  uint64_t* hash128 = reinterpret_cast<uint64_t*>(md5_digest);
+  // We are assuming little endian based on
+  // http://g3doc/devtools/x86free/g3doc/porting.md#endianness-detection.
+  return BloomFilter::Hash{hash128[0], hash128[1]};
+}
+
+// Helper function to calculate the ith hash value of the given `Hash` struct,
+// and calculate its corresponding bit index in the bitmap to be set or
+// checked. The caller must ensure that the `bit_count` parameter passed
+// in is greater than zero.
+int32_t CalculateBitIndex(const BloomFilter::Hash& hash,
+                          int32_t i,
+                          int32_t bit_count) {
+  //  CHECK_GT(bit_count, 0);  // Crash ok.
+  uint64_t val = hash.h1 + i * hash.h2;
+  return val % bit_count;
+}
+
+// Return whether the bit on the given index in the bitmap is set to 1.
+bool IsBitSet(const std::vector<uint8_t>& bitmap, int32_t index) {
+  uint8_t byteAtIndex = bitmap[index / 8];
+  int offset = index % 8;
+  return (byteAtIndex & (0x01 << offset)) != 0;
+}
+
+BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
+                         int32_t padding,
+                         int32_t hash_count) {
+  // do validation
+  bitmap_ = bitmap;
+  hash_count_ = hash_count;
+  bit_count_ = bitmap.size() * 8 - padding;
+}
+
+bool BloomFilter::MightContain(const absl::string_view value) const {
+  if (value.empty() || bit_count_ == 0) return false;
+  Hash hash = StringToHash(value);
+  // The `hash_count_` and `bit_count_` fields are guaranteed to be
+  // non-negative when the `BloomFilter` object is constructed.
+  for (int32_t i = 0; i < hash_count_; i++) {
+    int32_t index = CalculateBitIndex(hash, i, bit_count_);
+    if (!IsBitSet(bitmap_, index)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+}  // namespace remote
+}  // namespace firestore
+}  // namespace firebase
diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
new file mode 100644
index 00000000000..c0317c85fcb
--- /dev/null
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -0,0 +1,88 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef FIREBASE_CORE_SRC_REMOTE_BLOOM_FILTER_H
+#define FIREBASE_CORE_SRC_REMOTE_BLOOM_FILTER_H
+
+#include <string>
+#include <vector>
+#include "absl/strings/string_view.h"
+
+namespace firebase {
+namespace firestore {
+namespace remote {
+
+class BloomFilter final {
+ public:
+  BloomFilter(std::vector<uint8_t> bitmap, int32_t padding, int32_t hash_count);
+
+  // Copyable & movable.
+  BloomFilter(const BloomFilter&) = default;
+  BloomFilter(BloomFilter&&) = default;
+  BloomFilter& operator=(const BloomFilter&) = default;
+  BloomFilter& operator=(BloomFilter&&) = default;
+
+  /**
+   * Check whether the given string is a possible member of the bloom filter. It
+   * might return false positive result, ie, the given string is not a member of
+   * the bloom filter, but the method returned true.
+   *
+   * @param value the string to be tested for membership.
+   * @return true if the given string might be contained in the bloom filter, or
+   * false if the given string is definitely not contained in the bloom filter.
+   */
+  bool MightContain(absl::string_view value) const;
+
+  // Get the `bit_count_` field. Used for testing purpose.
+  int32_t bit_count() const {
+    return bit_count_;
+  }
+
+  // When inserting a key into bitmap, the first step is to generate k hashes
+  // out of it, where we need its Hash struct representation. See
+  // `InsertToBitmap()` for details.
+  //
+  // The steps to convert a key string into a Hash struct:
+  //   - Hash the key with MD5 to obtain a 128-bit hash.
+  //   - Treat the resulting 128-bit hash as 2 distinct 64-bit hash values,
+  //     named `h1` and `h2`, interpreted as unsigned integers using 2's
+  //     complement encoding.
+  // See `BloomFilterBuilder::AddKey()` for the corresponding code and more
+  // details.
+  struct Hash {
+    uint64_t h1;
+    uint64_t h2;
+  };
+
+ private:
+  // The number of bits in the bloom filter. Guaranteed to be non-negative, and
+  // less than the max number of bits `bitmap_` can represent, i.e.,
+  // bitmap_.size() * 8.
+  int32_t bit_count_ = 0;
+
+  // The number of hash functions used to construct the filter. Guaranteed to be
+  // non-negative.
+  int32_t hash_count_ = 0;
+
+  // Bloom filter's bitmap.
+  std::vector<uint8_t> bitmap_;
+};
+
+}  // namespace remote
+}  // namespace firestore
+}  // namespace firebase
+
+#endif  // FIREBASE_CORE_SRC_REMOTE_BLOOM_FILTER_H

From 2f14bc709a311765e2d30f024a497a8379b614f1 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Fri, 17 Feb 2023 15:15:39 -0800
Subject: [PATCH 02/17] Update bloom_filter.h

---
 Firestore/core/src/remote/bloom_filter.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index c0317c85fcb..612feeae557 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-#ifndef FIREBASE_CORE_SRC_REMOTE_BLOOM_FILTER_H
-#define FIREBASE_CORE_SRC_REMOTE_BLOOM_FILTER_H
+#ifndef FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_H_
+#define FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_H_
 
 #include <string>
 #include <vector>
@@ -85,4 +85,4 @@ class BloomFilter final {
 }  // namespace firestore
 }  // namespace firebase
 
-#endif  // FIREBASE_CORE_SRC_REMOTE_BLOOM_FILTER_H
+#endif  // FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_H_

From 33c65fa6335eccb259cca3c41bd7497efa5c0312 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Fri, 17 Feb 2023 17:45:30 -0800
Subject: [PATCH 03/17] implement BloomFilter class

---
 Firestore/core/src/remote/bloom_filter.cc     | 114 ++++++++++++++++++
 Firestore/core/src/remote/bloom_filter.cpp    |  85 -------------
 Firestore/core/src/remote/bloom_filter.h      |   9 ++
 .../test/unit/remote/bloom_filter_test.cc     |  46 +++++++
 4 files changed, 169 insertions(+), 85 deletions(-)
 create mode 100644 Firestore/core/src/remote/bloom_filter.cc
 delete mode 100644 Firestore/core/src/remote/bloom_filter.cpp
 create mode 100644 Firestore/core/test/unit/remote/bloom_filter_test.cc

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
new file mode 100644
index 00000000000..b1b165f8cf2
--- /dev/null
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -0,0 +1,114 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "bloom_filter.h"
+// #include <CommonCrypto/CommonDigest.h>
+#include <openssl/md5.h>
+
+namespace firebase {
+namespace firestore {
+namespace remote {
+
+/** Helper function to hash a string using md5 hashing algorithm, and return an
+ * array of 16 bytes. */
+BloomFilter::Hash BloomFilter::Md5HashDigest(
+    const absl::string_view key) const {
+  unsigned char md5_digest[MD5_DIGEST_LENGTH];
+  static_assert(sizeof(md5_digest) == sizeof(BloomFilter::Hash), "");
+
+  MD5_CTX context;
+  MD5_Init(&context);
+  MD5_Update(&context, key.data(), key.size());
+  MD5_Final(md5_digest, &context);
+
+  uint64_t* hash128 = reinterpret_cast<uint64_t*>(md5_digest);
+  return BloomFilter::Hash{hash128[0], hash128[1]};
+}
+
+/**
+ * Calculate the ith hash value based on the hashed 64 bit unsigned integers,
+ * and calculate its corresponding bit index in the bitmap to be checked.
+ */
+int32_t BloomFilter::GetBitIndex(const BloomFilter::Hash& hash,
+                                 int32_t i,
+                                 int32_t bit_count) const {
+  //  CHECK_GT(bit_count, 0);  // Crash ok.
+  uint64_t val = hash.h1 + i * hash.h2;
+  return val % bit_count;
+}
+
+/** Return whether the bit at the given index in the bitmap is set to 1. */
+bool BloomFilter::IsBitSet(const std::vector<uint8_t>& bitmap,
+                           int32_t index) const {
+  uint8_t byteAtIndex = bitmap[index / 8];
+  int offset = index % 8;
+  return (byteAtIndex & (0x01 << offset)) != 0;
+}
+
+BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
+                         int32_t padding,
+                         int32_t hash_count) {
+
+  if (padding < 0 || padding >= 8) {
+    throw std::invalid_argument(&"Invalid padding: "[padding]);
+  }
+  if (hash_count < 0) {
+    throw std::invalid_argument(&"Invalid hash count: "[hash_count]);
+  }
+  if (bitmap.size() > 0 && hash_count == 0) {
+    // Only empty bloom filter can have 0 hash count.
+    throw std::invalid_argument(&"Invalid hash count: "[hash_count]);
+  }
+  if (bitmap.size() == 0) {
+    // Empty bloom filter should have 0 padding.
+    if (padding != 0) {
+      throw std::invalid_argument(
+          &"Expected padding of 0 when bitmap length is 0, but got "[padding]);
+    }
+  }
+
+  bitmap_ = bitmap;
+  hash_count_ = hash_count;
+  bit_count_ = bitmap.size() * 8 - padding;
+}
+
+/**
+ * Check whether the given string is a possible member of the bloom filter. It
+ * might return false positive result, ie, the given string is not a member of
+ * the bloom filter, but the method returned true.
+ *
+ * @param value the string to be tested for membership.
+ * @return true if the given string might be contained in the bloom filter, or
+ * false if the given string is definitely not contained in the bloom filter.
+ */
+bool BloomFilter::MightContain(const absl::string_view value) const {
+  // Empty bitmap should return false on membership check.
+  if (bit_count_ == 0) return false;
+  Hash hash = Md5HashDigest(value);
+  // The `hash_count_` and `bit_count_` fields are guaranteed to be
+  // non-negative when the `BloomFilter` object is constructed.
+  for (int32_t i = 0; i < hash_count_; i++) {
+    int32_t index = GetBitIndex(hash, i, bit_count_);
+    if (!IsBitSet(bitmap_, index)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+}  // namespace remote
+}  // namespace firestore
+}  // namespace firebase
diff --git a/Firestore/core/src/remote/bloom_filter.cpp b/Firestore/core/src/remote/bloom_filter.cpp
deleted file mode 100644
index 45adeb92dfa..00000000000
--- a/Firestore/core/src/remote/bloom_filter.cpp
+++ /dev/null
@@ -1,85 +0,0 @@
-/*
- * Copyright 2023 Google LLC
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "bloom_filter.h"
-#include <CommonCrypto/CommonDigest.h>
-
-namespace firebase {
-namespace firestore {
-namespace remote {
-
-// Helper function to hash the given key string to Hash structure using the
-// MD5 hash function.
-BloomFilter::Hash StringToHash(const absl::string_view key) {
-  unsigned char md5_digest[CC_MD5_DIGEST_LENGTH];
-  static_assert(sizeof(md5_digest) == sizeof(BloomFilter::Hash), "");
-
-  CC_MD5_CTX context;
-  CC_MD5_Init(&context);
-  CC_MD5_Update(&context, key.data(), key.size());
-  CC_MD5_Final(md5_digest, &context);
-
-  uint64_t* hash128 = reinterpret_cast<uint64_t*>(md5_digest);
-  // We are assuming little endian based on
-  // http://g3doc/devtools/x86free/g3doc/porting.md#endianness-detection.
-  return BloomFilter::Hash{hash128[0], hash128[1]};
-}
-
-// Helper function to calculate the ith hash value of the given `Hash` struct,
-// and calculate its corresponding bit index in the bitmap to be set or
-// checked. The caller must ensure that the `bit_count` parameter passed
-// in is greater than zero.
-int32_t CalculateBitIndex(const BloomFilter::Hash& hash,
-                          int32_t i,
-                          int32_t bit_count) {
-  //  CHECK_GT(bit_count, 0);  // Crash ok.
-  uint64_t val = hash.h1 + i * hash.h2;
-  return val % bit_count;
-}
-
-// Return whether the bit on the given index in the bitmap is set to 1.
-bool IsBitSet(const std::vector<uint8_t>& bitmap, int32_t index) {
-  uint8_t byteAtIndex = bitmap[index / 8];
-  int offset = index % 8;
-  return (byteAtIndex & (0x01 << offset)) != 0;
-}
-
-BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
-                         int32_t padding,
-                         int32_t hash_count) {
-  // do validation
-  bitmap_ = bitmap;
-  hash_count_ = hash_count;
-  bit_count_ = bitmap.size() * 8 - padding;
-}
-
-bool BloomFilter::MightContain(const absl::string_view value) const {
-  if (value.empty() || bit_count_ == 0) return false;
-  Hash hash = StringToHash(value);
-  // The `hash_count_` and `bit_count_` fields are guaranteed to be
-  // non-negative when the `BloomFilter` object is constructed.
-  for (int32_t i = 0; i < hash_count_; i++) {
-    int32_t index = CalculateBitIndex(hash, i, bit_count_);
-    if (!IsBitSet(bitmap_, index)) {
-      return false;
-    }
-  }
-  return true;
-}
-
-}  // namespace remote
-}  // namespace firestore
-}  // namespace firebase
diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index 612feeae557..7e39a269b3e 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -79,6 +79,15 @@ class BloomFilter final {
 
   // Bloom filter's bitmap.
   std::vector<uint8_t> bitmap_;
+
+  // Do we want these here? if not, I can remove the MakeString:: in .cc
+  BloomFilter::Hash Md5HashDigest(const absl::string_view key) const;
+
+  int32_t GetBitIndex(const BloomFilter::Hash& hash,
+                      int32_t i,
+                      int32_t bit_count) const;
+
+  bool IsBitSet(const std::vector<uint8_t>& bitmap, int32_t index) const;
 };
 
 }  // namespace remote
diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc
new file mode 100644
index 00000000000..f20c3fc4e03
--- /dev/null
+++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "Firestore/core/src/remote/bloom_filter.h"
+#include <vector>
+#include "gtest/gtest.h"
+
+namespace firebase {
+namespace firestore {
+namespace remote {
+
+class BloomFilterTest : public ::testing::Test {};
+
+TEST_F(BloomFilterTest, CanInstantiateEmptyBloomFilter) {
+  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
+  EXPECT_EQ(bloomFilter.bit_count(), 0);
+}
+
+TEST_F(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
+  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 1, 1);
+  EXPECT_EQ(bloomFilter.bit_count(), 7);
+}
+
+TEST_F(BloomFilterTest, CanRunMightContain) {
+  // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑"
+  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);
+  EXPECT_TRUE(bloomFilter.MightContain("ÀÒ∑"));
+  EXPECT_FALSE(bloomFilter.MightContain("Ò∑À"));
+}
+
+}  // namespace remote
+}  //  namespace firestore
+}  //  namespace firebase

From 9338fe2a1cb01203ff281e9271a95464d8e3eb1b Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Fri, 17 Feb 2023 18:06:37 -0800
Subject: [PATCH 04/17] add validation test case

---
 Firestore/core/src/remote/bloom_filter.cc     |  2 -
 .../test/unit/remote/bloom_filter_test.cc     | 50 +++++++++++++++++--
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index b1b165f8cf2..ea3185c06eb 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -27,7 +27,6 @@ namespace remote {
 BloomFilter::Hash BloomFilter::Md5HashDigest(
     const absl::string_view key) const {
   unsigned char md5_digest[MD5_DIGEST_LENGTH];
-  static_assert(sizeof(md5_digest) == sizeof(BloomFilter::Hash), "");
 
   MD5_CTX context;
   MD5_Init(&context);
@@ -61,7 +60,6 @@ bool BloomFilter::IsBitSet(const std::vector<uint8_t>& bitmap,
 BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
                          int32_t padding,
                          int32_t hash_count) {
-
   if (padding < 0 || padding >= 8) {
     throw std::invalid_argument(&"Invalid padding: "[padding]);
   }
diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc
index f20c3fc4e03..826472a15cc 100644
--- a/Firestore/core/test/unit/remote/bloom_filter_test.cc
+++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc
@@ -30,17 +30,61 @@ TEST_F(BloomFilterTest, CanInstantiateEmptyBloomFilter) {
 }
 
 TEST_F(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
-  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 1, 1);
-  EXPECT_EQ(bloomFilter.bit_count(), 7);
+  {
+    BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{1}, 0, 1);
+    EXPECT_EQ(bloomFilter1.bit_count(), 8);
+  }
+  {
+    BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, 7, 1);
+    EXPECT_EQ(bloomFilter2.bit_count(), 1);
+  }
 }
 
-TEST_F(BloomFilterTest, CanRunMightContain) {
+/** handle exception */
+//TEST_F(BloomFilterTest,
+//       ConstructorShouldThrowIAEOnNonEmptyBloomFilterWithZeroHashCount) {
+//  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 1, 0);
+//}
+//
+//TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativePadding) {
+//  { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, -1, 1); }
+//  { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, -1, 1); }
+//}
+//
+//TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativeHashCount) {
+//  { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, 0, -1); }
+//  { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, 1, -1); }
+//}
+//
+//TEST_F(BloomFilterTest, ConstructorShouldThrowIAEIfPaddingIsTooLarge) {
+//  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 8, 1);
+//}
+
+TEST_F(BloomFilterTest, MightContainCanProcessNonStandardCharacters) {
   // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑"
   BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);
   EXPECT_TRUE(bloomFilter.MightContain("ÀÒ∑"));
   EXPECT_FALSE(bloomFilter.MightContain("Ò∑À"));
 }
 
+TEST_F(BloomFilterTest, MightContainOnEmptyBloomFilterShouldReturnFalse) {
+  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
+  EXPECT_FALSE(bloomFilter.MightContain(""));
+  EXPECT_FALSE(bloomFilter.MightContain("a"));
+}
+
+TEST_F(BloomFilterTest,
+       MightContainWithEmptyStringMightReturnFalsePositiveResult) {
+  {
+    BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{1}, 1, 1);
+    EXPECT_FALSE(bloomFilter1.MightContain(""));
+  }
+  {
+    BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{255}, 0, 16);
+    EXPECT_TRUE(bloomFilter2.MightContain(""));
+  }
+}
+
 }  // namespace remote
 }  //  namespace firestore
 }  //  namespace firebase

From 4bb92a17d0075190498d2e31191db0b97ba94c0a Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Tue, 21 Feb 2023 14:24:43 -0800
Subject: [PATCH 05/17] use CC_MD5

---
 Firestore/core/src/remote/bloom_filter.cc     | 32 ++++++++-----
 .../core/src/remote/bloom_filter_exception.h  | 47 +++++++++++++++++++
 .../test/unit/remote/bloom_filter_test.cc     | 35 +++++++-------
 3 files changed, 86 insertions(+), 28 deletions(-)
 create mode 100644 Firestore/core/src/remote/bloom_filter_exception.h

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index ea3185c06eb..9a863c588b9 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -13,10 +13,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
+#include <CommonCrypto/CommonDigest.h>
 
 #include "bloom_filter.h"
-// #include <CommonCrypto/CommonDigest.h>
-#include <openssl/md5.h>
+#include "bloom_filter_exception.h"
 
 namespace firebase {
 namespace firestore {
@@ -26,12 +29,12 @@ namespace remote {
  * array of 16 bytes. */
 BloomFilter::Hash BloomFilter::Md5HashDigest(
     const absl::string_view key) const {
-  unsigned char md5_digest[MD5_DIGEST_LENGTH];
+  unsigned char md5_digest[CC_MD5_DIGEST_LENGTH];
 
-  MD5_CTX context;
-  MD5_Init(&context);
-  MD5_Update(&context, key.data(), key.size());
-  MD5_Final(md5_digest, &context);
+  CC_MD5_CTX context;
+  CC_MD5_Init(&context);
+  CC_MD5_Update(&context, key.data(), key.size());
+  CC_MD5_Final(md5_digest, &context);
 
   uint64_t* hash128 = reinterpret_cast<uint64_t*>(md5_digest);
   return BloomFilter::Hash{hash128[0], hash128[1]};
@@ -61,20 +64,23 @@ BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
                          int32_t padding,
                          int32_t hash_count) {
   if (padding < 0 || padding >= 8) {
-    throw std::invalid_argument(&"Invalid padding: "[padding]);
+    throw BloomFilterException("Invalid padding: " + std::to_string(padding));
   }
   if (hash_count < 0) {
-    throw std::invalid_argument(&"Invalid hash count: "[hash_count]);
+    throw BloomFilterException("Invalid hash count: " +
+                               std::to_string(hash_count));
   }
   if (bitmap.size() > 0 && hash_count == 0) {
     // Only empty bloom filter can have 0 hash count.
-    throw std::invalid_argument(&"Invalid hash count: "[hash_count]);
+    throw BloomFilterException("Invalid hash count: " +
+                               std::to_string(hash_count));
   }
   if (bitmap.size() == 0) {
     // Empty bloom filter should have 0 padding.
     if (padding != 0) {
-      throw std::invalid_argument(
-          &"Expected padding of 0 when bitmap length is 0, but got "[padding]);
+      throw BloomFilterException(
+          "Expected padding of 0 when bitmap length is 0, but got " +
+          std::to_string(padding));
     }
   }
 
@@ -107,6 +113,8 @@ bool BloomFilter::MightContain(const absl::string_view value) const {
   return true;
 }
 
+#pragma clang diagnostic pop
+
 }  // namespace remote
 }  // namespace firestore
 }  // namespace firebase
diff --git a/Firestore/core/src/remote/bloom_filter_exception.h b/Firestore/core/src/remote/bloom_filter_exception.h
new file mode 100644
index 00000000000..b679f97d60f
--- /dev/null
+++ b/Firestore/core/src/remote/bloom_filter_exception.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_ERROR_H_
+#define FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_ERROR_H_
+
+#include <exception>
+#include <string>
+
+namespace firebase {
+namespace firestore {
+namespace remote {
+
+/**
+ * An exception thrown if BloomFilter encounters an error.
+ */
+class BloomFilterException : public std::exception {
+ public:
+  BloomFilterException(const std::string& message) : message_(message) {
+  }
+
+  const char* what() const noexcept override {
+    return message_.c_str();
+  }
+
+ private:
+  std::string message_;
+};
+
+}  // namespace remote
+}  // namespace firestore
+}  // namespace firebase
+
+#endif  // FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_ERROR_H_
diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc
index 826472a15cc..0483e70098d 100644
--- a/Firestore/core/test/unit/remote/bloom_filter_test.cc
+++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc
@@ -15,6 +15,8 @@
  */
 
 #include "Firestore/core/src/remote/bloom_filter.h"
+#include "Firestore/core/src/remote/bloom_filter_exception.h"
+
 #include <vector>
 #include "gtest/gtest.h"
 
@@ -41,24 +43,25 @@ TEST_F(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
 }
 
 /** handle exception */
-//TEST_F(BloomFilterTest,
-//       ConstructorShouldThrowIAEOnNonEmptyBloomFilterWithZeroHashCount) {
-//  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 1, 0);
-//}
-//
-//TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativePadding) {
-//  { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, -1, 1); }
-//  { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, -1, 1); }
-//}
+TEST_F(BloomFilterTest,
+       ConstructorShouldThrowIAEOnNonEmptyBloomFilterWithZeroHashCount) {
+  EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, 1, 0),
+               BloomFilterException);
+}
+
+// TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativePadding) {
+//   { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, -1, 1); }
+//   { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, -1, 1); }
+// }
 //
-//TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativeHashCount) {
-//  { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, 0, -1); }
-//  { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, 1, -1); }
-//}
+// TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativeHashCount) {
+//   { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, 0, -1); }
+//   { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, 1, -1); }
+// }
 //
-//TEST_F(BloomFilterTest, ConstructorShouldThrowIAEIfPaddingIsTooLarge) {
-//  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 8, 1);
-//}
+// TEST_F(BloomFilterTest, ConstructorShouldThrowIAEIfPaddingIsTooLarge) {
+//   BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 8, 1);
+// }
 
 TEST_F(BloomFilterTest, MightContainCanProcessNonStandardCharacters) {
   // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑"

From ac6557e3c32e9062b11f562518af7cc9d0915051 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Tue, 21 Feb 2023 15:08:13 -0800
Subject: [PATCH 06/17] update bloom filter unit test

---
 .../test/unit/remote/bloom_filter_test.cc     | 44 ++++++++++++-------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc
index 0483e70098d..8a8cb12981d 100644
--- a/Firestore/core/test/unit/remote/bloom_filter_test.cc
+++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc
@@ -42,27 +42,41 @@ TEST_F(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
   }
 }
 
-/** handle exception */
+/** Handle exception in creating bloomFilter instance*/
 TEST_F(BloomFilterTest,
-       ConstructorShouldThrowIAEOnNonEmptyBloomFilterWithZeroHashCount) {
+       ConstructorShouldThrowExceptionOnNonEmptyBloomFilterWithZeroHashCount) {
   EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, 1, 0),
                BloomFilterException);
 }
 
-// TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativePadding) {
-//   { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, -1, 1); }
-//   { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, -1, 1); }
-// }
-//
-// TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativeHashCount) {
-//   { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, 0, -1); }
-//   { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, 1, -1); }
-// }
-//
-// TEST_F(BloomFilterTest, ConstructorShouldThrowIAEIfPaddingIsTooLarge) {
-//   BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 8, 1);
-// }
+TEST_F(BloomFilterTest, ConstructorShouldThrowExceptionOnNegativePadding) {
+  {
+    EXPECT_THROW(BloomFilter(std::vector<uint8_t>{0}, -1, 1),
+                 BloomFilterException);
+  }
+  {
+    EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, -1, 1),
+                 BloomFilterException);
+  }
+}
+
+TEST_F(BloomFilterTest, ConstructorShouldThrowExceptionOnNegativeHashCount) {
+  {
+    EXPECT_THROW(BloomFilter(std::vector<uint8_t>{0}, 0, -1),
+                 BloomFilterException);
+  }
+  {
+    EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, 1, -1),
+                 BloomFilterException);
+  }
+}
+
+TEST_F(BloomFilterTest, ConstructorShouldThrowExceptionIfPaddingIsTooLarge) {
+  EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, 8, 1),
+               BloomFilterException);
+}
 
+/* Test mightContain result */
 TEST_F(BloomFilterTest, MightContainCanProcessNonStandardCharacters) {
   // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑"
   BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);

From be543a8581a06762c12b34b394be7a3f434baa93 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Tue, 21 Feb 2023 16:58:03 -0800
Subject: [PATCH 07/17] make Hash struct private

---
 Firestore/core/src/remote/bloom_filter.h | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index 4676e56c780..2d02ebf44d3 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -51,22 +51,6 @@ class BloomFilter final {
     return bit_count_;
   }
 
-  // When inserting a key into bitmap, the first step is to generate k hashes
-  // out of it, where we need its Hash struct representation. See
-  // `InsertToBitmap()` for details.
-  //
-  // The steps to convert a key string into a Hash struct:
-  //   - Hash the key with MD5 to obtain a 128-bit hash.
-  //   - Treat the resulting 128-bit hash as 2 distinct 64-bit hash values,
-  //     named `h1` and `h2`, interpreted as unsigned integers using 2's
-  //     complement encoding.
-  // See `BloomFilterBuilder::AddKey()` for the corresponding code and more
-  // details.
-  struct Hash {
-    uint64_t h1;
-    uint64_t h2;
-  };
-
  private:
   // The number of bits in the bloom filter. Guaranteed to be non-negative, and
   // less than the max number of bits `bitmap_` can represent, i.e.,
@@ -87,6 +71,14 @@ class BloomFilter final {
                       int32_t bit_count) const;
 
   bool IsBitSet(const std::vector<uint8_t>& bitmap, int32_t index) const;
+
+  // When checking membership of a key in bitmap, the first step is to generate
+  // a 128-bit hash, and treat it as 2 distinct 64-bit hash values, named `h1`
+  // and `h2`, interpreted as unsigned integers using 2's  complement encoding.
+  struct Hash {
+    uint64_t h1;
+    uint64_t h2;
+  };
 };
 
 }  // namespace remote

From 61c7e8229d08b379207221786d0f88a4849d1244 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Tue, 21 Feb 2023 18:17:13 -0800
Subject: [PATCH 08/17] Update bloom_filter.h

---
 Firestore/core/src/remote/bloom_filter.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index 2d02ebf44d3..459fbc235cd 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -51,6 +51,14 @@ class BloomFilter final {
     return bit_count_;
   }
 
+  // When checking membership of a key in bitmap, the first step is to generate
+  // a 128-bit hash, and treat it as 2 distinct 64-bit hash values, named `h1`
+  // and `h2`, interpreted as unsigned integers using 2's  complement encoding.
+  struct Hash {
+    uint64_t h1;
+    uint64_t h2;
+  };
+
  private:
   // The number of bits in the bloom filter. Guaranteed to be non-negative, and
   // less than the max number of bits `bitmap_` can represent, i.e.,
@@ -71,14 +79,6 @@ class BloomFilter final {
                       int32_t bit_count) const;
 
   bool IsBitSet(const std::vector<uint8_t>& bitmap, int32_t index) const;
-
-  // When checking membership of a key in bitmap, the first step is to generate
-  // a 128-bit hash, and treat it as 2 distinct 64-bit hash values, named `h1`
-  // and `h2`, interpreted as unsigned integers using 2's  complement encoding.
-  struct Hash {
-    uint64_t h1;
-    uint64_t h2;
-  };
 };
 
 }  // namespace remote

From b33f0d14e71fa90710c0200dc43e1de0741a513b Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Tue, 21 Feb 2023 20:58:17 -0800
Subject: [PATCH 09/17] remove bloom filter exception

---
 Firestore/core/src/remote/bloom_filter.cc     | 60 ++++++++++++++-----
 Firestore/core/src/remote/bloom_filter.h      |  4 ++
 .../core/src/remote/bloom_filter_exception.h  | 47 ---------------
 .../test/unit/remote/bloom_filter_test.cc     | 39 ++++--------
 4 files changed, 60 insertions(+), 90 deletions(-)
 delete mode 100644 Firestore/core/src/remote/bloom_filter_exception.h

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index 9a863c588b9..7b51637d7f7 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -16,15 +16,19 @@
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
 
-#include <CommonCrypto/CommonDigest.h>
-
 #include "bloom_filter.h"
-#include "bloom_filter_exception.h"
+#include <CommonCrypto/CommonDigest.h>
+#include "Firestore/core/src/util/hard_assert.h"
+#include "Firestore/core/src/util/log.h"
+#include "Firestore/core/src/util/statusor.h"
 
 namespace firebase {
 namespace firestore {
 namespace remote {
 
+using util::Status;
+using util::StatusOr;
+
 /** Helper function to hash a string using md5 hashing algorithm, and return an
  * array of 16 bytes. */
 BloomFilter::Hash BloomFilter::Md5HashDigest(
@@ -47,7 +51,6 @@ BloomFilter::Hash BloomFilter::Md5HashDigest(
 int32_t BloomFilter::GetBitIndex(const BloomFilter::Hash& hash,
                                  int32_t i,
                                  int32_t bit_count) const {
-  //  CHECK_GT(bit_count, 0);  // Crash ok.
   uint64_t val = hash.h1 + i * hash.h2;
   return val % bit_count;
 }
@@ -63,30 +66,57 @@ bool BloomFilter::IsBitSet(const std::vector<uint8_t>& bitmap,
 BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
                          int32_t padding,
                          int32_t hash_count) {
+  //  HARD_ASSERT(padding >= 0 && padding < 8, "Invalid padding: %s",
+  //              std::to_string(padding));
+  //  HARD_ASSERT(hash_count >= 0, "Invalid hash count: %s",
+  //              std::to_string(hash_count));
+  //  // Only empty bloom filter can have 0 hash count.
+  //  HARD_ASSERT(bitmap.size() == 0 || hash_count != 0, "Invalid hash count:
+  //  %s",
+  //              std::to_string(hash_count));
+  //  // Empty bloom filter should have 0 padding.
+  //  HARD_ASSERT(bitmap.size() != 0 || padding == 0,
+  //              "Expected padding of 0 when bitmap length is 0, but got %s",
+  //              std::to_string(padding));
+
+  StatusOr<BloomFilter> maybe_bloom_filter =
+      Create(bitmap, padding, hash_count);
+
+  if (!maybe_bloom_filter.ok()) {
+    LOG_WARN("Creating bloom filter failed: %s",
+             maybe_bloom_filter.status().error_message());
+    return;
+  }
+}
+
+StatusOr<BloomFilter> BloomFilter::Create(std::vector<uint8_t>& bitmap,
+                                          int32_t& padding,
+                                          int32_t& hash_count) {
   if (padding < 0 || padding >= 8) {
-    throw BloomFilterException("Invalid padding: " + std::to_string(padding));
+    return Status(firestore::Error::kErrorInvalidArgument,
+                  "Invalid padding: " + std::to_string(padding));
   }
   if (hash_count < 0) {
-    throw BloomFilterException("Invalid hash count: " +
-                               std::to_string(hash_count));
+    return Status(firestore::Error::kErrorInvalidArgument,
+                  "Invalid hash count: " + std::to_string(hash_count));
   }
   if (bitmap.size() > 0 && hash_count == 0) {
     // Only empty bloom filter can have 0 hash count.
-    throw BloomFilterException("Invalid hash count: " +
-                               std::to_string(hash_count));
+    return Status(firestore::Error::kErrorInvalidArgument,
+                  "Invalid hash count: " + std::to_string(hash_count));
   }
-  if (bitmap.size() == 0) {
+  if (bitmap.size() == 0 && padding != 0) {
     // Empty bloom filter should have 0 padding.
-    if (padding != 0) {
-      throw BloomFilterException(
-          "Expected padding of 0 when bitmap length is 0, but got " +
-          std::to_string(padding));
-    }
+    return Status(firestore::Error::kErrorInvalidArgument,
+                  "Expected padding of 0 when bitmap length is 0, but got " +
+                      std::to_string(padding));
   }
 
   bitmap_ = bitmap;
   hash_count_ = hash_count;
   bit_count_ = bitmap.size() * 8 - padding;
+
+  return *this;
 }
 
 /**
diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index 459fbc235cd..5d0d91e6750 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -19,6 +19,7 @@
 
 #include <string>
 #include <vector>
+#include "Firestore/core/src/util/statusor.h"
 #include "absl/strings/string_view.h"
 
 namespace firebase {
@@ -79,6 +80,9 @@ class BloomFilter final {
                       int32_t bit_count) const;
 
   bool IsBitSet(const std::vector<uint8_t>& bitmap, int32_t index) const;
+  util::StatusOr<BloomFilter> Create(std::vector<uint8_t>& bitmap,
+                                     int32_t& padding,
+                                     int32_t& hash_count);
 };
 
 }  // namespace remote
diff --git a/Firestore/core/src/remote/bloom_filter_exception.h b/Firestore/core/src/remote/bloom_filter_exception.h
deleted file mode 100644
index b679f97d60f..00000000000
--- a/Firestore/core/src/remote/bloom_filter_exception.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * Copyright 2023 Google LLC
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_ERROR_H_
-#define FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_ERROR_H_
-
-#include <exception>
-#include <string>
-
-namespace firebase {
-namespace firestore {
-namespace remote {
-
-/**
- * An exception thrown if BloomFilter encounters an error.
- */
-class BloomFilterException : public std::exception {
- public:
-  BloomFilterException(const std::string& message) : message_(message) {
-  }
-
-  const char* what() const noexcept override {
-    return message_.c_str();
-  }
-
- private:
-  std::string message_;
-};
-
-}  // namespace remote
-}  // namespace firestore
-}  // namespace firebase
-
-#endif  // FIRESTORE_CORE_SRC_REMOTE_BLOOM_FILTER_ERROR_H_
diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc
index 8a8cb12981d..e91516bc216 100644
--- a/Firestore/core/test/unit/remote/bloom_filter_test.cc
+++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc
@@ -15,8 +15,6 @@
  */
 
 #include "Firestore/core/src/remote/bloom_filter.h"
-#include "Firestore/core/src/remote/bloom_filter_exception.h"
-
 #include <vector>
 #include "gtest/gtest.h"
 
@@ -42,41 +40,26 @@ TEST_F(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
   }
 }
 
-/** Handle exception in creating bloomFilter instance*/
+/** handle exception silently*/
 TEST_F(BloomFilterTest,
-       ConstructorShouldThrowExceptionOnNonEmptyBloomFilterWithZeroHashCount) {
-  EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, 1, 0),
-               BloomFilterException);
+       ConstructorShouldThrowIAEOnNonEmptyBloomFilterWithZeroHashCount) {
+  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 1, 0);
 }
 
-TEST_F(BloomFilterTest, ConstructorShouldThrowExceptionOnNegativePadding) {
-  {
-    EXPECT_THROW(BloomFilter(std::vector<uint8_t>{0}, -1, 1),
-                 BloomFilterException);
-  }
-  {
-    EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, -1, 1),
-                 BloomFilterException);
-  }
+TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativePadding) {
+  { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, -1, 1); }
+  { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, -1, 1); }
 }
 
-TEST_F(BloomFilterTest, ConstructorShouldThrowExceptionOnNegativeHashCount) {
-  {
-    EXPECT_THROW(BloomFilter(std::vector<uint8_t>{0}, 0, -1),
-                 BloomFilterException);
-  }
-  {
-    EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, 1, -1),
-                 BloomFilterException);
-  }
+TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativeHashCount) {
+  { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, 0, -1); }
+  { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, 1, -1); }
 }
 
-TEST_F(BloomFilterTest, ConstructorShouldThrowExceptionIfPaddingIsTooLarge) {
-  EXPECT_THROW(BloomFilter(std::vector<uint8_t>{1}, 8, 1),
-               BloomFilterException);
+TEST_F(BloomFilterTest, ConstructorShouldThrowIAEIfPaddingIsTooLarge) {
+  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 8, 1);
 }
 
-/* Test mightContain result */
 TEST_F(BloomFilterTest, MightContainCanProcessNonStandardCharacters) {
   // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑"
   BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);

From d6f7a6489a71dae5626290fca92dfa0d72137fb6 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Wed, 22 Feb 2023 10:46:09 -0800
Subject: [PATCH 10/17] update the Create method and test cases

---
 .../Firestore.xcodeproj/project.pbxproj       | 14 +++
 Firestore/core/src/remote/bloom_filter.cc     | 46 ++++-----
 Firestore/core/src/remote/bloom_filter.h      | 13 ++-
 .../test/unit/remote/bloom_filter_test.cc     | 99 +++++++++++++------
 4 files changed, 113 insertions(+), 59 deletions(-)

diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj
index 601a3e602e7..5f76ac57f1e 100644
--- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj
+++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj
@@ -32,6 +32,7 @@
 		048A55EED3241ABC28752F86 /* memory_mutation_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 74FBEFA4FE4B12C435011763 /* memory_mutation_queue_test.cc */; };
 		04D7D9DB95E66FECF2C0A412 /* bundle_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = F7FC06E0A47D393DE1759AE1 /* bundle_cache_test.cc */; };
 		0500A324CEC854C5B0CF364C /* FIRCollectionReferenceTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E045202154AA00B64F25 /* FIRCollectionReferenceTests.mm */; };
+		0500F75D28C112E4F5EE90ED /* bloom_filter_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ECBF36AA2F0AE1B2C1403D44 /* bloom_filter_test.cc */; };
 		050FB0783F462CEDD44BEFFD /* document_overlay_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = FFCA39825D9678A03D1845D0 /* document_overlay_cache_test.cc */; };
 		0535C1B65DADAE1CE47FA3CA /* string_format_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = 9CFD366B783AE27B9E79EE7A /* string_format_apple_test.mm */; };
 		053C11420E49AE1A77E21C20 /* memory_document_overlay_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 29D9C76922DAC6F710BC1EF4 /* memory_document_overlay_cache_test.cc */; };
@@ -677,6 +678,7 @@
 		6ABB82D43C0728EB095947AF /* geo_point_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB7BAB332012B519001E0872 /* geo_point_test.cc */; };
 		6AED40FF444F0ACFE3AE96E3 /* target_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B5C37696557C81A6C2B7271A /* target_cache_test.cc */; };
 		6AF739DDA9D33DF756DE7CDE /* autoid_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54740A521FC913E500713A1A /* autoid_test.cc */; };
+		6B78203C409594CD98CDF3CC /* bloom_filter_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ECBF36AA2F0AE1B2C1403D44 /* bloom_filter_test.cc */; };
 		6B94E0AE1002C5C9EA0F5582 /* log_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54C2294E1FECABAE007D065B /* log_test.cc */; };
 		6BA8753F49951D7AEAD70199 /* watch_change_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 2D7472BC70C024D736FF74D9 /* watch_change_test.cc */; };
 		6BFB7A4D37F1B7EB5A7461B0 /* memory_query_engine_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 8EF6A33BC2D84233C355F1D0 /* memory_query_engine_test.cc */; };
@@ -1077,6 +1079,7 @@
 		BC5AC8890974E0821431267E /* limit_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 54DA129F1F315EE100DD57A1 /* limit_spec_test.json */; };
 		BC8DFBCB023DBD914E27AA7D /* query_listener_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 7C3F995E040E9E9C5E8514BB /* query_listener_test.cc */; };
 		BCA720A0F54D23654F806323 /* ConditionalConformanceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E3228F51DCDC2E90D5C58F97 /* ConditionalConformanceTests.swift */; };
+		BD1FF9AD3746627A220E3720 /* bloom_filter_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ECBF36AA2F0AE1B2C1403D44 /* bloom_filter_test.cc */; };
 		BD6CC8614970A3D7D2CF0D49 /* exponential_backoff_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B6D1B68420E2AB1A00B35856 /* exponential_backoff_test.cc */; };
 		BDD2D1812BAD962E3C81A53F /* hashing_test_apple.mm in Sources */ = {isa = PBXBuildFile; fileRef = B69CF3F02227386500B281C8 /* hashing_test_apple.mm */; };
 		BDDAE67000DBF10E9EA7FED0 /* nanopb_util_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 6F5B6C1399F92FD60F2C582B /* nanopb_util_test.cc */; };
@@ -1136,6 +1139,7 @@
 		C901A1BFD553B6DD70BB7CC7 /* bundle_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = F7FC06E0A47D393DE1759AE1 /* bundle_cache_test.cc */; };
 		C961FA581F87000DF674BBC8 /* field_transform_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 7515B47C92ABEEC66864B55C /* field_transform_test.cc */; };
 		C9F96C511F45851D38EC449C /* status.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 618BBE9920B89AAC00B5BCE7 /* status.pb.cc */; };
+		CA077AA69CF12D5170BB05C2 /* bloom_filter_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ECBF36AA2F0AE1B2C1403D44 /* bloom_filter_test.cc */; };
 		CA989C0E6020C372A62B7062 /* testutil.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54A0352820A3B3BD003E0143 /* testutil.cc */; };
 		CAFB1E0ED514FEF4641E3605 /* log_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54C2294E1FECABAE007D065B /* log_test.cc */; };
 		CB2C731116D6C9464220626F /* FIRQueryUnitTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = FF73B39D04D1760190E6B84A /* FIRQueryUnitTests.mm */; };
@@ -1247,6 +1251,7 @@
 		E21D819A06D9691A4B313440 /* remote_store_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 3B843E4A1F3930A400548890 /* remote_store_spec_test.json */; };
 		E25DCFEF318E003B8B7B9DC8 /* index_backfiller_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1F50E872B3F117A674DA8E94 /* index_backfiller_test.cc */; };
 		E27C0996AF6EC6D08D91B253 /* document.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 544129D821C2DDC800EFB9CC /* document.pb.cc */; };
+		E288C093C725954581B69325 /* bloom_filter_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ECBF36AA2F0AE1B2C1403D44 /* bloom_filter_test.cc */; };
 		E2AE851F9DC4C037CCD05E36 /* remote_document_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 7EB299CF85034F09CFD6F3FD /* remote_document_cache_test.cc */; };
 		E2B15548A3B6796CE5A01975 /* FIRListenerRegistrationTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E06B202154D500B64F25 /* FIRListenerRegistrationTests.mm */; };
 		E2B7AEDCAAC5AD74C12E85C1 /* datastore_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 3167BD972EFF8EC636530E59 /* datastore_test.cc */; };
@@ -1282,6 +1287,7 @@
 		E99D5467483B746D4AA44F74 /* fields_array_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = BA4CBA48204C9E25B56993BC /* fields_array_test.cc */; };
 		EA38690795FBAA182A9AA63E /* FIRDatabaseTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E06C202154D500B64F25 /* FIRDatabaseTests.mm */; };
 		EA46611779C3EEF12822508C /* annotations.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 618BBE9520B89AAC00B5BCE7 /* annotations.pb.cc */; };
+		EA8C203393B17C238A776133 /* bloom_filter_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ECBF36AA2F0AE1B2C1403D44 /* bloom_filter_test.cc */; };
 		EAA1962BFBA0EBFBA53B343F /* bundle_builder.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4F5B96F3ABCD2CA901DB1CD4 /* bundle_builder.cc */; };
 		EAC0914B6DCC53008483AEE3 /* leveldb_snappy_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D9D94300B9C02F7069523C00 /* leveldb_snappy_test.cc */; };
 		EADD28A7859FBB9BE4D913B0 /* memory_remote_document_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1CA9800A53669EFBFFB824E3 /* memory_remote_document_cache_test.cc */; };
@@ -1804,6 +1810,7 @@
 		E42355285B9EF55ABD785792 /* Pods_Firestore_Example_macOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Example_macOS.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		E592181BFD7C53C305123739 /* Pods-Firestore_Tests_iOS.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Tests_iOS.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Tests_iOS/Pods-Firestore_Tests_iOS.debug.xcconfig"; sourceTree = "<group>"; };
 		E76F0CDF28E5FA62D21DE648 /* leveldb_target_cache_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = leveldb_target_cache_test.cc; sourceTree = "<group>"; };
+		ECBF36AA2F0AE1B2C1403D44 /* bloom_filter_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = bloom_filter_test.cc; sourceTree = "<group>"; };
 		ECEBABC7E7B693BE808A1052 /* Pods_Firestore_IntegrationTests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_IntegrationTests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		EF79BDA33A25371CD72BCE94 /* bloom_filter.pb.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = bloom_filter.pb.cc; sourceTree = "<group>"; };
 		EF83ACD5E1E9F25845A9ACED /* leveldb_migrations_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = leveldb_migrations_test.cc; sourceTree = "<group>"; };
@@ -2025,6 +2032,7 @@
 		546854A720A3681B004BDBD5 /* remote */ = {
 			isa = PBXGroup;
 			children = (
+				ECBF36AA2F0AE1B2C1403D44 /* bloom_filter_test.cc */,
 				CF39535F2C41AB0006FA6C0E /* create_noop_connectivity_monitor.cc */,
 				9098A0C535096F2EE9C35DE0 /* create_noop_connectivity_monitor.h */,
 				3167BD972EFF8EC636530E59 /* datastore_test.cc */,
@@ -3649,6 +3657,7 @@
 				1733601ECCEA33E730DEAF45 /* autoid_test.cc in Sources */,
 				0DAA255C2FEB387895ADEE12 /* bits_test.cc in Sources */,
 				FBA8282F8E99C878E4B9E87F /* bloom_filter.pb.cc in Sources */,
+				0500F75D28C112E4F5EE90ED /* bloom_filter_test.cc in Sources */,
 				394259BB091E1DB5994B91A2 /* bundle.pb.cc in Sources */,
 				EBAC5E8D0E2ECD9FBEDB7DAE /* bundle_builder.cc in Sources */,
 				5150E9F256E6E82D6F3CB3F1 /* bundle_cache_test.cc in Sources */,
@@ -3857,6 +3866,7 @@
 				5D5E24E3FA1128145AA117D2 /* autoid_test.cc in Sources */,
 				B6FDE6F91D3F81D045E962A0 /* bits_test.cc in Sources */,
 				F8EC78289E8FBC73AC640435 /* bloom_filter.pb.cc in Sources */,
+				BD1FF9AD3746627A220E3720 /* bloom_filter_test.cc in Sources */,
 				4D1775B7916D4CDAD1BF1876 /* bundle.pb.cc in Sources */,
 				474DF520B9859479845C8A4D /* bundle_builder.cc in Sources */,
 				04D7D9DB95E66FECF2C0A412 /* bundle_cache_test.cc in Sources */,
@@ -4081,6 +4091,7 @@
 				B842780CF42361ACBBB381A9 /* autoid_test.cc in Sources */,
 				146C140B254F3837A4DD7AE8 /* bits_test.cc in Sources */,
 				B79DDA1869EE15B93C5231F6 /* bloom_filter.pb.cc in Sources */,
+				E288C093C725954581B69325 /* bloom_filter_test.cc in Sources */,
 				3DDC57212ADBA9AD498EAA4C /* bundle.pb.cc in Sources */,
 				F3DEF2DB11FADAABDAA4C8BB /* bundle_builder.cc in Sources */,
 				392966346DA5EB3165E16A22 /* bundle_cache_test.cc in Sources */,
@@ -4305,6 +4316,7 @@
 				6AF739DDA9D33DF756DE7CDE /* autoid_test.cc in Sources */,
 				C1B4621C0820EEB0AC9CCD22 /* bits_test.cc in Sources */,
 				CEA99E72C941969C54BE3248 /* bloom_filter.pb.cc in Sources */,
+				6B78203C409594CD98CDF3CC /* bloom_filter_test.cc in Sources */,
 				01C66732ECCB83AB1D896026 /* bundle.pb.cc in Sources */,
 				EAA1962BFBA0EBFBA53B343F /* bundle_builder.cc in Sources */,
 				C901A1BFD553B6DD70BB7CC7 /* bundle_cache_test.cc in Sources */,
@@ -4523,6 +4535,7 @@
 				54740A581FC914F000713A1A /* autoid_test.cc in Sources */,
 				AB380D02201BC69F00D97691 /* bits_test.cc in Sources */,
 				22FC2BEE59BEDE4CFF0FAA7E /* bloom_filter.pb.cc in Sources */,
+				CA077AA69CF12D5170BB05C2 /* bloom_filter_test.cc in Sources */,
 				784FCB02C76096DACCBA11F2 /* bundle.pb.cc in Sources */,
 				856A1EAAD674ADBDAAEDAC37 /* bundle_builder.cc in Sources */,
 				BB3F35B1510FE5449E50EC8A /* bundle_cache_test.cc in Sources */,
@@ -4766,6 +4779,7 @@
 				8F781F527ED72DC6C123689E /* autoid_test.cc in Sources */,
 				0B9BD73418289EFF91917934 /* bits_test.cc in Sources */,
 				35F32BDF43950B4E715A1BDB /* bloom_filter.pb.cc in Sources */,
+				EA8C203393B17C238A776133 /* bloom_filter_test.cc in Sources */,
 				F8126CD7308A4B8AEC0F30A8 /* bundle.pb.cc in Sources */,
 				5AFA1055E8F6B4E4B1CCE2C4 /* bundle_builder.cc in Sources */,
 				AE5E5E4A7BF12C2337AFA13B /* bundle_cache_test.cc in Sources */,
diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index 7b51637d7f7..b066208cfd7 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -66,32 +66,26 @@ bool BloomFilter::IsBitSet(const std::vector<uint8_t>& bitmap,
 BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
                          int32_t padding,
                          int32_t hash_count) {
-  //  HARD_ASSERT(padding >= 0 && padding < 8, "Invalid padding: %s",
-  //              std::to_string(padding));
-  //  HARD_ASSERT(hash_count >= 0, "Invalid hash count: %s",
-  //              std::to_string(hash_count));
-  //  // Only empty bloom filter can have 0 hash count.
-  //  HARD_ASSERT(bitmap.size() == 0 || hash_count != 0, "Invalid hash count:
-  //  %s",
-  //              std::to_string(hash_count));
-  //  // Empty bloom filter should have 0 padding.
-  //  HARD_ASSERT(bitmap.size() != 0 || padding == 0,
-  //              "Expected padding of 0 when bitmap length is 0, but got %s",
-  //              std::to_string(padding));
+  HARD_ASSERT(padding >= 0 && padding < 8, "Invalid padding: %s",
+              std::to_string(padding));
+  HARD_ASSERT(hash_count >= 0, "Invalid hash count: %s",
+              std::to_string(hash_count));
+  // Only empty bloom filter can have 0 hash count.
+  HARD_ASSERT(bitmap.size() == 0 || hash_count != 0, "Invalid hash count:%s",
+              std::to_string(hash_count));
+  // Empty bloom filter should have 0 padding.
+  HARD_ASSERT(bitmap.size() != 0 || padding == 0,
+              "Expected padding of 0 when bitmap length is 0, but got %s",
+              std::to_string(padding));
 
-  StatusOr<BloomFilter> maybe_bloom_filter =
-      Create(bitmap, padding, hash_count);
-
-  if (!maybe_bloom_filter.ok()) {
-    LOG_WARN("Creating bloom filter failed: %s",
-             maybe_bloom_filter.status().error_message());
-    return;
-  }
+  bitmap_ = bitmap;
+  hash_count_ = hash_count;
+  bit_count_ = bitmap.size() * 8 - padding;
 }
 
-StatusOr<BloomFilter> BloomFilter::Create(std::vector<uint8_t>& bitmap,
-                                          int32_t& padding,
-                                          int32_t& hash_count) {
+StatusOr<BloomFilter> BloomFilter::Create(std::vector<uint8_t> bitmap,
+                                          int32_t padding,
+                                          int32_t hash_count) {
   if (padding < 0 || padding >= 8) {
     return Status(firestore::Error::kErrorInvalidArgument,
                   "Invalid padding: " + std::to_string(padding));
@@ -112,11 +106,7 @@ StatusOr<BloomFilter> BloomFilter::Create(std::vector<uint8_t>& bitmap,
                       std::to_string(padding));
   }
 
-  bitmap_ = bitmap;
-  hash_count_ = hash_count;
-  bit_count_ = bitmap.size() * 8 - padding;
-
-  return *this;
+  return BloomFilter(bitmap, padding, hash_count);
 }
 
 /**
diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index 5d0d91e6750..806250c2a01 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -36,6 +36,16 @@ class BloomFilter final {
   BloomFilter& operator=(const BloomFilter&) = default;
   BloomFilter& operator=(BloomFilter&&) = default;
 
+  /**
+   * Creates a BloomFilter object or return a status.
+   *
+   * @return a new BloomFilter if the inputs are valid, otherwise returns a not
+   * `ok()` status.
+   */
+  static util::StatusOr<BloomFilter> Create(std::vector<uint8_t> bitmap,
+                                            int32_t padding,
+                                            int32_t hash_count);
+
   /**
    * Check whether the given string is a possible member of the bloom filter. It
    * might return false positive result, ie, the given string is not a member of
@@ -80,9 +90,6 @@ class BloomFilter final {
                       int32_t bit_count) const;
 
   bool IsBitSet(const std::vector<uint8_t>& bitmap, int32_t index) const;
-  util::StatusOr<BloomFilter> Create(std::vector<uint8_t>& bitmap,
-                                     int32_t& padding,
-                                     int32_t& hash_count);
 };
 
 }  // namespace remote
diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc
index e91516bc216..d7fbbc94e53 100644
--- a/Firestore/core/test/unit/remote/bloom_filter_test.cc
+++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc
@@ -22,66 +22,109 @@ namespace firebase {
 namespace firestore {
 namespace remote {
 
+using util::Status;
+using util::StatusOr;
+
 class BloomFilterTest : public ::testing::Test {};
 
 TEST_F(BloomFilterTest, CanInstantiateEmptyBloomFilter) {
-  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
-  EXPECT_EQ(bloomFilter.bit_count(), 0);
+  BloomFilter bloom_Filter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
+  EXPECT_EQ(bloom_Filter.bit_count(), 0);
 }
 
 TEST_F(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
   {
-    BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{1}, 0, 1);
-    EXPECT_EQ(bloomFilter1.bit_count(), 8);
+    BloomFilter bloom_Filter_1 = BloomFilter(std::vector<uint8_t>{1}, 0, 1);
+    EXPECT_EQ(bloom_Filter_1.bit_count(), 8);
   }
   {
-    BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, 7, 1);
-    EXPECT_EQ(bloomFilter2.bit_count(), 1);
+    BloomFilter bloom_Filter_2 = BloomFilter(std::vector<uint8_t>{1}, 7, 1);
+    EXPECT_EQ(bloom_Filter_2.bit_count(), 1);
   }
 }
 
-/** handle exception silently*/
-TEST_F(BloomFilterTest,
-       ConstructorShouldThrowIAEOnNonEmptyBloomFilterWithZeroHashCount) {
-  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 1, 0);
+/* Test Create factory method with valid and invalid inputs */
+TEST_F(BloomFilterTest, CreateMethodShouldReturnBloomFilterOnValidInputs) {
+  StatusOr<BloomFilter> maybe_bloom_filter =
+      BloomFilter::Create(std::vector<uint8_t>{1}, 1, 1);
+  EXPECT_TRUE(maybe_bloom_filter.ok());
+  BloomFilter bloom_filter = maybe_bloom_filter.ValueOrDie();
+  EXPECT_EQ(bloom_filter.bit_count(), 7);
+}
+
+TEST_F(BloomFilterTest, CreateMethodShouldReturnNotOKStatusOnNegativePadding) {
+  {
+    StatusOr<BloomFilter> maybe_bloom_filter_1 =
+        BloomFilter::Create(std::vector<uint8_t>{0}, -1, 1);
+    EXPECT_FALSE(maybe_bloom_filter_1.ok());
+    EXPECT_EQ(maybe_bloom_filter_1.status().error_message(),
+              "Invalid padding: -1");
+  }
+  {
+    StatusOr<BloomFilter> maybe_bloom_filter_2 =
+        BloomFilter::Create(std::vector<uint8_t>{1}, -1, 1);
+    EXPECT_FALSE(maybe_bloom_filter_2.ok());
+    EXPECT_EQ(maybe_bloom_filter_2.status().error_message(),
+              "Invalid padding: -1");
+  }
 }
 
-TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativePadding) {
-  { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, -1, 1); }
-  { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, -1, 1); }
+TEST_F(BloomFilterTest,
+       CreateMethodShouldReturnNotOKStatusOnNegativeHashCount) {
+  {
+    StatusOr<BloomFilter> maybe_bloom_filter_1 =
+        BloomFilter::Create(std::vector<uint8_t>{0}, 0, -1);
+    EXPECT_FALSE(maybe_bloom_filter_1.ok());
+    EXPECT_EQ(maybe_bloom_filter_1.status().error_message(),
+              "Invalid hash count: -1");
+  }
+  {
+    StatusOr<BloomFilter> maybe_bloom_filter_2 =
+        BloomFilter::Create(std::vector<uint8_t>{1}, 1, -1);
+    EXPECT_FALSE(maybe_bloom_filter_2.ok());
+    EXPECT_EQ(maybe_bloom_filter_2.status().error_message(),
+              "Invalid hash count: -1");
+  }
 }
 
-TEST_F(BloomFilterTest, ConstructorShouldThrowIAEOnNegativeHashCount) {
-  { BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{0}, 0, -1); }
-  { BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{1}, 1, -1); }
+TEST_F(BloomFilterTest, CreateMethodShouldReturnNotOKStatusOnZeroHashCount) {
+  StatusOr<BloomFilter> maybe_bloom_filter =
+      BloomFilter::Create(std::vector<uint8_t>{1}, 1, 0);
+  EXPECT_FALSE(maybe_bloom_filter.ok());
+  EXPECT_EQ(maybe_bloom_filter.status().error_message(),
+            "Invalid hash count: 0");
 }
 
-TEST_F(BloomFilterTest, ConstructorShouldThrowIAEIfPaddingIsTooLarge) {
-  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{1}, 8, 1);
+TEST_F(BloomFilterTest,
+       CreateMethodShouldReturnNotOKStatusIfPaddingIsTooLarge) {
+  StatusOr<BloomFilter> maybe_bloom_filter =
+      BloomFilter::Create(std::vector<uint8_t>{1}, 8, 1);
+  EXPECT_FALSE(maybe_bloom_filter.ok());
+  EXPECT_EQ(maybe_bloom_filter.status().error_message(), "Invalid padding: 8");
 }
 
 TEST_F(BloomFilterTest, MightContainCanProcessNonStandardCharacters) {
   // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑"
-  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);
-  EXPECT_TRUE(bloomFilter.MightContain("ÀÒ∑"));
-  EXPECT_FALSE(bloomFilter.MightContain("Ò∑À"));
+  BloomFilter bloom_Filter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);
+  EXPECT_TRUE(bloom_Filter.MightContain("ÀÒ∑"));
+  EXPECT_FALSE(bloom_Filter.MightContain("Ò∑À"));
 }
 
 TEST_F(BloomFilterTest, MightContainOnEmptyBloomFilterShouldReturnFalse) {
-  BloomFilter bloomFilter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
-  EXPECT_FALSE(bloomFilter.MightContain(""));
-  EXPECT_FALSE(bloomFilter.MightContain("a"));
+  BloomFilter bloom_Filter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
+  EXPECT_FALSE(bloom_Filter.MightContain(""));
+  EXPECT_FALSE(bloom_Filter.MightContain("a"));
 }
 
 TEST_F(BloomFilterTest,
        MightContainWithEmptyStringMightReturnFalsePositiveResult) {
   {
-    BloomFilter bloomFilter1 = BloomFilter(std::vector<uint8_t>{1}, 1, 1);
-    EXPECT_FALSE(bloomFilter1.MightContain(""));
+    BloomFilter bloom_Filter_1 = BloomFilter(std::vector<uint8_t>{1}, 1, 1);
+    EXPECT_FALSE(bloom_Filter_1.MightContain(""));
   }
   {
-    BloomFilter bloomFilter2 = BloomFilter(std::vector<uint8_t>{255}, 0, 16);
-    EXPECT_TRUE(bloomFilter2.MightContain(""));
+    BloomFilter bloom_Filter_2 = BloomFilter(std::vector<uint8_t>{255}, 0, 16);
+    EXPECT_TRUE(bloom_Filter_2.MightContain(""));
   }
 }
 

From 7ddaf723e66fb3116e0887e27452dd4eb809436f Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Wed, 22 Feb 2023 11:02:43 -0800
Subject: [PATCH 11/17] fromat

---
 Firestore/core/src/remote/bloom_filter.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index b066208cfd7..66c3f3461c0 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -16,8 +16,9 @@
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
 
-#include "bloom_filter.h"
-#include <CommonCrypto/CommonDigest.h>
+#include "Firestore/core/src/remote/bloom_filter.h"
+
+#include "CommonCrypto/CommonDigest.h"
 #include "Firestore/core/src/util/hard_assert.h"
 #include "Firestore/core/src/util/log.h"
 #include "Firestore/core/src/util/statusor.h"

From d105a3cd5be368c2b2bdf8ff8e376434e0df2605 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Wed, 22 Feb 2023 22:24:09 -0800
Subject: [PATCH 12/17] resolve comments

---
 Firestore/core/src/remote/bloom_filter.cc | 47 +++++++-------------
 Firestore/core/src/remote/bloom_filter.h  | 52 ++++++++++++++---------
 2 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index 66c3f3461c0..2005b8c116a 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -13,6 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+// TODO(Mila): Replace this CommonCrypto with platform based MD5 calculation.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
 
@@ -30,10 +32,8 @@ namespace remote {
 using util::Status;
 using util::StatusOr;
 
-/** Helper function to hash a string using md5 hashing algorithm, and return an
- * array of 16 bytes. */
 BloomFilter::Hash BloomFilter::Md5HashDigest(
-    const absl::string_view key) const {
+    const absl::string_view& key) const {
   unsigned char md5_digest[CC_MD5_DIGEST_LENGTH];
 
   CC_MD5_CTX context;
@@ -41,27 +41,21 @@ BloomFilter::Hash BloomFilter::Md5HashDigest(
   CC_MD5_Update(&context, key.data(), key.size());
   CC_MD5_Final(md5_digest, &context);
 
+  // TODO(Mila): Replace this casting with safer function.
   uint64_t* hash128 = reinterpret_cast<uint64_t*>(md5_digest);
   return BloomFilter::Hash{hash128[0], hash128[1]};
 }
 
-/**
- * Calculate the ith hash value based on the hashed 64 bit unsigned integers,
- * and calculate its corresponding bit index in the bitmap to be checked.
- */
 int32_t BloomFilter::GetBitIndex(const BloomFilter::Hash& hash,
-                                 int32_t i,
-                                 int32_t bit_count) const {
-  uint64_t val = hash.h1 + i * hash.h2;
-  return val % bit_count;
+                                 int32_t hash_index) const {
+  uint64_t val = hash.h1 + (hash_index * hash.h2);
+  return val % bit_count_;
 }
 
-/** Return whether the bit at the given index in the bitmap is set to 1. */
-bool BloomFilter::IsBitSet(const std::vector<uint8_t>& bitmap,
-                           int32_t index) const {
-  uint8_t byteAtIndex = bitmap[index / 8];
-  int offset = index % 8;
-  return (byteAtIndex & (0x01 << offset)) != 0;
+bool BloomFilter::IsBitSet(int32_t index) const {
+  uint8_t byte_at_index{bitmap_[index / 8]};
+  int offset{index % 8};
+  return (byte_at_index & (0x01 << offset)) != 0;
 }
 
 BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
@@ -79,9 +73,9 @@ BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
               "Expected padding of 0 when bitmap length is 0, but got %s",
               std::to_string(padding));
 
-  bitmap_ = bitmap;
+  bitmap_ = std::move(bitmap);
   hash_count_ = hash_count;
-  bit_count_ = bitmap.size() * 8 - padding;
+  bit_count_ = bitmap_.size() * 8 - padding;
 }
 
 StatusOr<BloomFilter> BloomFilter::Create(std::vector<uint8_t> bitmap,
@@ -107,18 +101,9 @@ StatusOr<BloomFilter> BloomFilter::Create(std::vector<uint8_t> bitmap,
                       std::to_string(padding));
   }
 
-  return BloomFilter(bitmap, padding, hash_count);
+  return BloomFilter(std::move(bitmap), padding, hash_count);
 }
 
-/**
- * Check whether the given string is a possible member of the bloom filter. It
- * might return false positive result, ie, the given string is not a member of
- * the bloom filter, but the method returned true.
- *
- * @param value the string to be tested for membership.
- * @return true if the given string might be contained in the bloom filter, or
- * false if the given string is definitely not contained in the bloom filter.
- */
 bool BloomFilter::MightContain(const absl::string_view value) const {
   // Empty bitmap should return false on membership check.
   if (bit_count_ == 0) return false;
@@ -126,8 +111,8 @@ bool BloomFilter::MightContain(const absl::string_view value) const {
   // The `hash_count_` and `bit_count_` fields are guaranteed to be
   // non-negative when the `BloomFilter` object is constructed.
   for (int32_t i = 0; i < hash_count_; i++) {
-    int32_t index = GetBitIndex(hash, i, bit_count_);
-    if (!IsBitSet(bitmap_, index)) {
+    int32_t index = GetBitIndex(hash, i);
+    if (!IsBitSet(index)) {
       return false;
     }
   }
diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index 806250c2a01..8e13c158950 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -57,39 +57,53 @@ class BloomFilter final {
    */
   bool MightContain(absl::string_view value) const;
 
-  // Get the `bit_count_` field. Used for testing purpose.
+  /** Get the `bit_count_` field. Used for testing purpose. */
   int32_t bit_count() const {
     return bit_count_;
   }
 
-  // When checking membership of a key in bitmap, the first step is to generate
-  // a 128-bit hash, and treat it as 2 distinct 64-bit hash values, named `h1`
-  // and `h2`, interpreted as unsigned integers using 2's  complement encoding.
+ private:
+  /**
+   * When checking membership of a key in bitmap, the first step is to generate
+   * a 128-bit hash, and treat it as 2 distinct 64-bit hash values, named `h1`
+   * and `h2`, interpreted as unsigned integers using 2's  complement encoding.
+   */
   struct Hash {
     uint64_t h1;
     uint64_t h2;
   };
 
- private:
-  // The number of bits in the bloom filter. Guaranteed to be non-negative, and
-  // less than the max number of bits `bitmap_` can represent, i.e.,
-  // bitmap_.size() * 8.
-  int32_t bit_count_ = 0;
+  /**
+   * Helper function to hash a string using md5 hashing algorithm, and return a
+   * Hash object.
+   */
+  Hash Md5HashDigest(const absl::string_view& key) const;
 
-  // The number of hash functions used to construct the filter. Guaranteed to be
-  // non-negative.
-  int32_t hash_count_ = 0;
+  /**
+   * Calculate the ith hash value based on the hashed 64 bit unsigned integers,
+   * and calculate its corresponding bit index in the bitmap to be checked.
+   */
+  int32_t GetBitIndex(const BloomFilter::Hash& hash, int32_t hash_index) const;
 
-  // Bloom filter's bitmap.
-  std::vector<uint8_t> bitmap_;
+  /** Return whether the bit at the given index in the bitmap is set to 1. */
 
-  BloomFilter::Hash Md5HashDigest(const absl::string_view key) const;
+  bool IsBitSet(int32_t index) const;
 
-  int32_t GetBitIndex(const BloomFilter::Hash& hash,
-                      int32_t i,
-                      int32_t bit_count) const;
+  /**
+   * The number of bits in the bloom filter. Guaranteed to be non-negative, and
+   * less than the max number of bits `bitmap_` can represent, i.e.,
+   * bitmap_.size() * 8.
+   */
+  int32_t bit_count_ = 0;
 
-  bool IsBitSet(const std::vector<uint8_t>& bitmap, int32_t index) const;
+  /**
+   * The number of hash functions used to construct the filter. Guaranteed to
+   * be non-negative.
+   */
+  int32_t hash_count_ = 0;
+
+  /** Bloom filter's bitmap. */
+  std::vector<uint8_t> bitmap_;
 };
 
 }  // namespace remote

From 60511bcb2ec5050d1448ce9caa914526de7f86a7 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Wed, 22 Feb 2023 22:29:03 -0800
Subject: [PATCH 13/17] fix typo

---
 Firestore/core/src/remote/bloom_filter.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index 8e13c158950..b3d40d162e8 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -37,7 +37,7 @@ class BloomFilter final {
   BloomFilter& operator=(BloomFilter&&) = default;
 
   /**
-   * Creates a BloomFilter object or return a status.
+   * Creates a BloomFilter object or returns a status.
    *
    * @return a new BloomFilter if the inputs are valid, otherwise returns a not
    * `ok()` status.
@@ -66,7 +66,7 @@ class BloomFilter final {
   /**
    * When checking membership of a key in bitmap, the first step is to generate
    * a 128-bit hash, and treat it as 2 distinct 64-bit hash values, named `h1`
-   * and `h2`, interpreted as unsigned integers using 2's  complement encoding.
+   * and `h2`, interpreted as unsigned integers using 2's complement encoding.
    */
   struct Hash {
     uint64_t h1;

From b6fcea73b54625a50af583c6969202cc07902b8b Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Thu, 23 Feb 2023 09:48:52 -0800
Subject: [PATCH 14/17] resolve comments

---
 Firestore/core/src/remote/bloom_filter.cc | 14 +++++++-------
 Firestore/core/src/remote/bloom_filter.h  |  4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index 2005b8c116a..4234a742640 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -20,6 +20,8 @@
 
 #include "Firestore/core/src/remote/bloom_filter.h"
 
+#include <utility>
+
 #include "CommonCrypto/CommonDigest.h"
 #include "Firestore/core/src/util/hard_assert.h"
 #include "Firestore/core/src/util/log.h"
@@ -32,8 +34,7 @@ namespace remote {
 using util::Status;
 using util::StatusOr;
 
-BloomFilter::Hash BloomFilter::Md5HashDigest(
-    const absl::string_view& key) const {
+BloomFilter::Hash BloomFilter::Md5HashDigest(absl::string_view key) const {
   unsigned char md5_digest[CC_MD5_DIGEST_LENGTH];
 
   CC_MD5_CTX context;
@@ -41,13 +42,12 @@ BloomFilter::Hash BloomFilter::Md5HashDigest(
   CC_MD5_Update(&context, key.data(), key.size());
   CC_MD5_Final(md5_digest, &context);
 
-  // TODO(Mila): Replace this casting with safer function.
+  // TODO(Mila): Replace this casting with safer function (b/270568625).
   uint64_t* hash128 = reinterpret_cast<uint64_t*>(md5_digest);
-  return BloomFilter::Hash{hash128[0], hash128[1]};
+  return Hash{hash128[0], hash128[1]};
 }
 
-int32_t BloomFilter::GetBitIndex(const BloomFilter::Hash& hash,
-                                 int32_t hash_index) const {
+int32_t BloomFilter::GetBitIndex(const Hash& hash, int32_t hash_index) const {
   uint64_t val = hash.h1 + (hash_index * hash.h2);
   return val % bit_count_;
 }
@@ -104,7 +104,7 @@ StatusOr<BloomFilter> BloomFilter::Create(std::vector<uint8_t> bitmap,
   return BloomFilter(std::move(bitmap), padding, hash_count);
 }
 
-bool BloomFilter::MightContain(const absl::string_view value) const {
+bool BloomFilter::MightContain(absl::string_view value) const {
   // Empty bitmap should return false on membership check.
   if (bit_count_ == 0) return false;
   Hash hash = Md5HashDigest(value);
diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index b3d40d162e8..0647ea7cc0d 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -77,13 +77,13 @@ class BloomFilter final {
    * Helper function to hash a string using md5 hashing algorithm, and return a
    * Hash object.
    */
-  Hash Md5HashDigest(const absl::string_view& key) const;
+  Hash Md5HashDigest(absl::string_view key) const;
 
   /**
    * Calculate the ith hash value based on the hashed 64 bit unsigned integers,
    * and calculate its corresponding bit index in the bitmap to be checked.
    */
-  int32_t GetBitIndex(const BloomFilter::Hash& hash, int32_t hash_index) const;
+  int32_t GetBitIndex(const Hash& hash, int32_t hash_index) const;
 
   /** Return whether the bit at the given index in the bitmap is set to 1. */
 

From c67c68b3d696f5a8f1a0e5ae2aebff24b34fb56c Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Fri, 24 Feb 2023 12:38:13 -0800
Subject: [PATCH 15/17] resolve comments

---
 Firestore/core/src/remote/bloom_filter.cc     |  53 ++++-----
 Firestore/core/src/remote/bloom_filter.h      |   4 +-
 .../test/unit/remote/bloom_filter_test.cc     | 102 +++++++++---------
 3 files changed, 82 insertions(+), 77 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index 4234a742640..00cc5a346a2 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -14,10 +14,6 @@
  * limitations under the License.
  */
 
-// TODO(Mila): Replace this CommonCrypto with platform based MD5 calculation.
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-
 #include "Firestore/core/src/remote/bloom_filter.h"
 
 #include <utility>
@@ -26,6 +22,7 @@
 #include "Firestore/core/src/util/hard_assert.h"
 #include "Firestore/core/src/util/log.h"
 #include "Firestore/core/src/util/statusor.h"
+#include "Firestore/core/src/util/warnings.h"
 
 namespace firebase {
 namespace firestore {
@@ -34,6 +31,10 @@ namespace remote {
 using util::Status;
 using util::StatusOr;
 
+// TODO(Mila): Replace CommonCrypto with platform based MD5 calculation and
+// remove suppress.
+SUPPRESS_DEPRECATED_DECLARATIONS_BEGIN();
+
 BloomFilter::Hash BloomFilter::Md5HashDigest(absl::string_view key) const {
   unsigned char md5_digest[CC_MD5_DIGEST_LENGTH];
 
@@ -46,36 +47,38 @@ BloomFilter::Hash BloomFilter::Md5HashDigest(absl::string_view key) const {
   uint64_t* hash128 = reinterpret_cast<uint64_t*>(md5_digest);
   return Hash{hash128[0], hash128[1]};
 }
+SUPPRESS_END();
 
 int32_t BloomFilter::GetBitIndex(const Hash& hash, int32_t hash_index) const {
-  uint64_t val = hash.h1 + (hash_index * hash.h2);
-  return val % bit_count_;
+  uint64_t hash_index_uint64 = static_cast<uint64_t>(hash_index);
+  uint64_t bit_count_uint64 = static_cast<uint64_t>(bit_count_);
+
+  uint64_t val = hash.h1 + (hash_index_uint64 * hash.h2);
+  uint64_t bit_index = val % bit_count_uint64;
+
+  HARD_ASSERT(bit_index <= INT32_MAX);
+  return bit_index;
 }
 
 bool BloomFilter::IsBitSet(int32_t index) const {
-  uint8_t byte_at_index{bitmap_[index / 8]};
-  int offset{index % 8};
-  return (byte_at_index & (0x01 << offset)) != 0;
+  uint8_t byte_at_index = bitmap_[index / 8];
+  int offset = index % 8;
+  return (byte_at_index & (static_cast<uint8_t>(0x01) << offset)) != 0;
 }
 
 BloomFilter::BloomFilter(std::vector<uint8_t> bitmap,
                          int32_t padding,
-                         int32_t hash_count) {
-  HARD_ASSERT(padding >= 0 && padding < 8, "Invalid padding: %s",
-              std::to_string(padding));
-  HARD_ASSERT(hash_count >= 0, "Invalid hash count: %s",
-              std::to_string(hash_count));
+                         int32_t hash_count)
+    : bit_count_(static_cast<int32_t>(bitmap.size()) * 8 - padding),
+      hash_count_(hash_count),
+      bitmap_(std::move(bitmap)) {
+  HARD_ASSERT(padding >= 0 && padding < 8);
+  HARD_ASSERT(hash_count_ >= 0);
   // Only empty bloom filter can have 0 hash count.
-  HARD_ASSERT(bitmap.size() == 0 || hash_count != 0, "Invalid hash count:%s",
-              std::to_string(hash_count));
+  HARD_ASSERT(bitmap_.size() == 0 || hash_count_ != 0);
   // Empty bloom filter should have 0 padding.
-  HARD_ASSERT(bitmap.size() != 0 || padding == 0,
-              "Expected padding of 0 when bitmap length is 0, but got %s",
-              std::to_string(padding));
-
-  bitmap_ = std::move(bitmap);
-  hash_count_ = hash_count;
-  bit_count_ = bitmap_.size() * 8 - padding;
+  HARD_ASSERT(bitmap_.size() != 0 || padding == 0);
+  HARD_ASSERT(bit_count_ >= 0);
 }
 
 StatusOr<BloomFilter> BloomFilter::Create(std::vector<uint8_t> bitmap,
@@ -110,7 +113,7 @@ bool BloomFilter::MightContain(absl::string_view value) const {
   Hash hash = Md5HashDigest(value);
   // The `hash_count_` and `bit_count_` fields are guaranteed to be
   // non-negative when the `BloomFilter` object is constructed.
-  for (int32_t i = 0; i < hash_count_; i++) {
+  for (int32_t i = 0; i < hash_count_; ++i) {
     int32_t index = GetBitIndex(hash, i);
     if (!IsBitSet(index)) {
       return false;
@@ -119,8 +122,6 @@ bool BloomFilter::MightContain(absl::string_view value) const {
   return true;
 }
 
-#pragma clang diagnostic pop
-
 }  // namespace remote
 }  // namespace firestore
 }  // namespace firebase
diff --git a/Firestore/core/src/remote/bloom_filter.h b/Firestore/core/src/remote/bloom_filter.h
index 0647ea7cc0d..e406611b1cc 100644
--- a/Firestore/core/src/remote/bloom_filter.h
+++ b/Firestore/core/src/remote/bloom_filter.h
@@ -74,8 +74,7 @@ class BloomFilter final {
   };
 
   /**
-   * Helper function to hash a string using md5 hashing algorithm, and return a
-   * Hash object.
+   * Calculate the MD5 digest of the given string, and return a Hash object.
    */
   Hash Md5HashDigest(absl::string_view key) const;
 
@@ -86,7 +85,6 @@ class BloomFilter final {
   int32_t GetBitIndex(const Hash& hash, int32_t hash_index) const;
 
   /** Return whether the bit at the given index in the bitmap is set to 1. */
-
   bool IsBitSet(int32_t index) const;
 
   /**
diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc
index d7fbbc94e53..c7395b7186d 100644
--- a/Firestore/core/test/unit/remote/bloom_filter_test.cc
+++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc
@@ -21,113 +21,119 @@
 namespace firebase {
 namespace firestore {
 namespace remote {
+namespace {
 
 using util::Status;
 using util::StatusOr;
 
-class BloomFilterTest : public ::testing::Test {};
-
-TEST_F(BloomFilterTest, CanInstantiateEmptyBloomFilter) {
-  BloomFilter bloom_Filter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
-  EXPECT_EQ(bloom_Filter.bit_count(), 0);
+TEST(BloomFilterTest, CanInstantiateEmptyBloomFilter) {
+  BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
+  EXPECT_EQ(bloom_filter.bit_count(), 0);
 }
 
-TEST_F(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
+TEST(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
   {
-    BloomFilter bloom_Filter_1 = BloomFilter(std::vector<uint8_t>{1}, 0, 1);
-    EXPECT_EQ(bloom_Filter_1.bit_count(), 8);
+    BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{1}, 0, 1);
+    EXPECT_EQ(bloom_filter.bit_count(), 8);
   }
   {
-    BloomFilter bloom_Filter_2 = BloomFilter(std::vector<uint8_t>{1}, 7, 1);
-    EXPECT_EQ(bloom_Filter_2.bit_count(), 1);
+    BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{1}, 7, 1);
+    EXPECT_EQ(bloom_filter.bit_count(), 1);
   }
 }
 
 /* Test Create factory method with valid and invalid inputs */
-TEST_F(BloomFilterTest, CreateMethodShouldReturnBloomFilterOnValidInputs) {
+TEST(BloomFilterTest, CreateShouldReturnBloomFilterOnValidInputs) {
   StatusOr<BloomFilter> maybe_bloom_filter =
       BloomFilter::Create(std::vector<uint8_t>{1}, 1, 1);
-  EXPECT_TRUE(maybe_bloom_filter.ok());
+  ASSERT_TRUE(maybe_bloom_filter.ok());
   BloomFilter bloom_filter = maybe_bloom_filter.ValueOrDie();
   EXPECT_EQ(bloom_filter.bit_count(), 7);
 }
 
-TEST_F(BloomFilterTest, CreateMethodShouldReturnNotOKStatusOnNegativePadding) {
+TEST(BloomFilterTest, CreateShouldBeAbleToCreatEmptyBloomFilter) {
+  StatusOr<BloomFilter> maybe_bloom_filter =
+      BloomFilter::Create(std::vector<uint8_t>{}, 0, 0);
+  ASSERT_TRUE(maybe_bloom_filter.ok());
+  BloomFilter bloom_filter = maybe_bloom_filter.ValueOrDie();
+  EXPECT_EQ(bloom_filter.bit_count(), 0);
+}
+
+TEST(BloomFilterTest, CreateShouldReturnNotOKStatusOnNegativePadding) {
   {
-    StatusOr<BloomFilter> maybe_bloom_filter_1 =
+    StatusOr<BloomFilter> maybe_bloom_filter =
         BloomFilter::Create(std::vector<uint8_t>{0}, -1, 1);
-    EXPECT_FALSE(maybe_bloom_filter_1.ok());
-    EXPECT_EQ(maybe_bloom_filter_1.status().error_message(),
+    ASSERT_FALSE(maybe_bloom_filter.ok());
+    EXPECT_EQ(maybe_bloom_filter.status().error_message(),
               "Invalid padding: -1");
   }
   {
-    StatusOr<BloomFilter> maybe_bloom_filter_2 =
+    StatusOr<BloomFilter> maybe_bloom_filter =
         BloomFilter::Create(std::vector<uint8_t>{1}, -1, 1);
-    EXPECT_FALSE(maybe_bloom_filter_2.ok());
-    EXPECT_EQ(maybe_bloom_filter_2.status().error_message(),
+    ASSERT_FALSE(maybe_bloom_filter.ok());
+    EXPECT_EQ(maybe_bloom_filter.status().error_message(),
               "Invalid padding: -1");
   }
 }
 
-TEST_F(BloomFilterTest,
-       CreateMethodShouldReturnNotOKStatusOnNegativeHashCount) {
+TEST(BloomFilterTest, CreateShouldReturnNotOKStatusOnNegativeHashCount) {
   {
-    StatusOr<BloomFilter> maybe_bloom_filter_1 =
+    StatusOr<BloomFilter> maybe_bloom_filter =
         BloomFilter::Create(std::vector<uint8_t>{0}, 0, -1);
-    EXPECT_FALSE(maybe_bloom_filter_1.ok());
-    EXPECT_EQ(maybe_bloom_filter_1.status().error_message(),
+    ASSERT_FALSE(maybe_bloom_filter.ok());
+    EXPECT_EQ(maybe_bloom_filter.status().error_message(),
               "Invalid hash count: -1");
   }
   {
-    StatusOr<BloomFilter> maybe_bloom_filter_2 =
+    StatusOr<BloomFilter> maybe_bloom_filter =
         BloomFilter::Create(std::vector<uint8_t>{1}, 1, -1);
-    EXPECT_FALSE(maybe_bloom_filter_2.ok());
-    EXPECT_EQ(maybe_bloom_filter_2.status().error_message(),
+    ASSERT_FALSE(maybe_bloom_filter.ok());
+    EXPECT_EQ(maybe_bloom_filter.status().error_message(),
               "Invalid hash count: -1");
   }
 }
 
-TEST_F(BloomFilterTest, CreateMethodShouldReturnNotOKStatusOnZeroHashCount) {
+TEST(BloomFilterTest, CreateShouldReturnNotOKStatusOnZeroHashCount) {
   StatusOr<BloomFilter> maybe_bloom_filter =
       BloomFilter::Create(std::vector<uint8_t>{1}, 1, 0);
-  EXPECT_FALSE(maybe_bloom_filter.ok());
+  ASSERT_FALSE(maybe_bloom_filter.ok());
   EXPECT_EQ(maybe_bloom_filter.status().error_message(),
             "Invalid hash count: 0");
 }
 
-TEST_F(BloomFilterTest,
-       CreateMethodShouldReturnNotOKStatusIfPaddingIsTooLarge) {
+TEST(BloomFilterTest, CreateShouldReturnNotOKStatusIfPaddingIsTooLarge) {
   StatusOr<BloomFilter> maybe_bloom_filter =
       BloomFilter::Create(std::vector<uint8_t>{1}, 8, 1);
-  EXPECT_FALSE(maybe_bloom_filter.ok());
+  ASSERT_FALSE(maybe_bloom_filter.ok());
   EXPECT_EQ(maybe_bloom_filter.status().error_message(), "Invalid padding: 8");
 }
 
-TEST_F(BloomFilterTest, MightContainCanProcessNonStandardCharacters) {
+TEST(BloomFilterTest, MightContainCanProcessNonStandardCharacters) {
   // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑"
-  BloomFilter bloom_Filter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);
-  EXPECT_TRUE(bloom_Filter.MightContain("ÀÒ∑"));
-  EXPECT_FALSE(bloom_Filter.MightContain("Ò∑À"));
+  BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);
+  EXPECT_TRUE(bloom_filter.MightContain("ÀÒ∑"));
+  EXPECT_FALSE(bloom_filter.MightContain("Ò∑À"));
 }
 
-TEST_F(BloomFilterTest, MightContainOnEmptyBloomFilterShouldReturnFalse) {
-  BloomFilter bloom_Filter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
-  EXPECT_FALSE(bloom_Filter.MightContain(""));
-  EXPECT_FALSE(bloom_Filter.MightContain("a"));
+TEST(BloomFilterTest, MightContainOnEmptyBloomFilterShouldReturnFalse) {
+  BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
+  EXPECT_FALSE(bloom_filter.MightContain(""));
+  EXPECT_FALSE(bloom_filter.MightContain("a"));
 }
 
-TEST_F(BloomFilterTest,
-       MightContainWithEmptyStringMightReturnFalsePositiveResult) {
+TEST(BloomFilterTest,
+     MightContainWithEmptyStringMightReturnFalsePositiveResult) {
   {
-    BloomFilter bloom_Filter_1 = BloomFilter(std::vector<uint8_t>{1}, 1, 1);
-    EXPECT_FALSE(bloom_Filter_1.MightContain(""));
+    BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{1}, 1, 1);
+    EXPECT_FALSE(bloom_filter.MightContain(""));
   }
   {
-    BloomFilter bloom_Filter_2 = BloomFilter(std::vector<uint8_t>{255}, 0, 16);
-    EXPECT_TRUE(bloom_Filter_2.MightContain(""));
+    BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{255}, 0, 16);
+    EXPECT_TRUE(bloom_filter.MightContain(""));
   }
 }
 
+}  // namespace
 }  // namespace remote
-}  //  namespace firestore
-}  //  namespace firebase
+}  // namespace firestore
+}  // namespace firebase

From 73af04a9015028ebdd55da6f9a9a7fcd7749f95d Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Fri, 24 Feb 2023 16:55:09 -0800
Subject: [PATCH 16/17] Update bloom_filter.cc

---
 Firestore/core/src/remote/bloom_filter.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index 00cc5a346a2..c866eed7ce0 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -53,8 +53,8 @@ int32_t BloomFilter::GetBitIndex(const Hash& hash, int32_t hash_index) const {
   uint64_t hash_index_uint64 = static_cast<uint64_t>(hash_index);
   uint64_t bit_count_uint64 = static_cast<uint64_t>(bit_count_);
 
-  uint64_t val = hash.h1 + (hash_index_uint64 * hash.h2);
-  uint64_t bit_index = val % bit_count_uint64;
+  uint64_t combined_hash = hash.h1 + (hash_index_uint64 * hash.h2);
+  uint64_t bit_index = combined_hash % bit_count_uint64;
 
   HARD_ASSERT(bit_index <= INT32_MAX);
   return bit_index;
@@ -62,7 +62,7 @@ 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];
-  int offset = index % 8;
+  int32_t offset = index % 8;
   return (byte_at_index & (static_cast<uint8_t>(0x01) << offset)) != 0;
 }
 

From 2a8989e2f5d7aa951a0a7dd8e3c254c3eb163621 Mon Sep 17 00:00:00 2001
From: milaGGL <107142260+milaGGL@users.noreply.github.com>
Date: Mon, 27 Feb 2023 10:09:21 -0800
Subject: [PATCH 17/17] resolve comments

---
 Firestore/core/src/remote/bloom_filter.cc     |  2 +-
 .../test/unit/remote/bloom_filter_test.cc     | 19 +++++++++----------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/Firestore/core/src/remote/bloom_filter.cc b/Firestore/core/src/remote/bloom_filter.cc
index c866eed7ce0..b42624a681c 100644
--- a/Firestore/core/src/remote/bloom_filter.cc
+++ b/Firestore/core/src/remote/bloom_filter.cc
@@ -20,7 +20,6 @@
 
 #include "CommonCrypto/CommonDigest.h"
 #include "Firestore/core/src/util/hard_assert.h"
-#include "Firestore/core/src/util/log.h"
 #include "Firestore/core/src/util/statusor.h"
 #include "Firestore/core/src/util/warnings.h"
 
@@ -50,6 +49,7 @@ BloomFilter::Hash BloomFilter::Md5HashDigest(absl::string_view key) const {
 SUPPRESS_END();
 
 int32_t BloomFilter::GetBitIndex(const Hash& hash, int32_t hash_index) const {
+  HARD_ASSERT(hash_index >= 0);
   uint64_t hash_index_uint64 = static_cast<uint64_t>(hash_index);
   uint64_t bit_count_uint64 = static_cast<uint64_t>(bit_count_);
 
diff --git a/Firestore/core/test/unit/remote/bloom_filter_test.cc b/Firestore/core/test/unit/remote/bloom_filter_test.cc
index c7395b7186d..953cbf93260 100644
--- a/Firestore/core/test/unit/remote/bloom_filter_test.cc
+++ b/Firestore/core/test/unit/remote/bloom_filter_test.cc
@@ -27,22 +27,21 @@ using util::Status;
 using util::StatusOr;
 
 TEST(BloomFilterTest, CanInstantiateEmptyBloomFilter) {
-  BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
+  BloomFilter bloom_filter(std::vector<uint8_t>{}, 0, 0);
   EXPECT_EQ(bloom_filter.bit_count(), 0);
 }
 
 TEST(BloomFilterTest, CanInstantiateNonEmptyBloomFilter) {
   {
-    BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{1}, 0, 1);
+    BloomFilter bloom_filter(std::vector<uint8_t>{1}, 0, 1);
     EXPECT_EQ(bloom_filter.bit_count(), 8);
   }
   {
-    BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{1}, 7, 1);
+    BloomFilter bloom_filter(std::vector<uint8_t>{1}, 7, 1);
     EXPECT_EQ(bloom_filter.bit_count(), 1);
   }
 }
 
-/* Test Create factory method with valid and invalid inputs */
 TEST(BloomFilterTest, CreateShouldReturnBloomFilterOnValidInputs) {
   StatusOr<BloomFilter> maybe_bloom_filter =
       BloomFilter::Create(std::vector<uint8_t>{1}, 1, 1);
@@ -62,7 +61,7 @@ TEST(BloomFilterTest, CreateShouldBeAbleToCreatEmptyBloomFilter) {
 TEST(BloomFilterTest, CreateShouldReturnNotOKStatusOnNegativePadding) {
   {
     StatusOr<BloomFilter> maybe_bloom_filter =
-        BloomFilter::Create(std::vector<uint8_t>{0}, -1, 1);
+        BloomFilter::Create(std::vector<uint8_t>{}, -1, 0);
     ASSERT_FALSE(maybe_bloom_filter.ok());
     EXPECT_EQ(maybe_bloom_filter.status().error_message(),
               "Invalid padding: -1");
@@ -79,7 +78,7 @@ TEST(BloomFilterTest, CreateShouldReturnNotOKStatusOnNegativePadding) {
 TEST(BloomFilterTest, CreateShouldReturnNotOKStatusOnNegativeHashCount) {
   {
     StatusOr<BloomFilter> maybe_bloom_filter =
-        BloomFilter::Create(std::vector<uint8_t>{0}, 0, -1);
+        BloomFilter::Create(std::vector<uint8_t>{}, 0, -1);
     ASSERT_FALSE(maybe_bloom_filter.ok());
     EXPECT_EQ(maybe_bloom_filter.status().error_message(),
               "Invalid hash count: -1");
@@ -110,13 +109,13 @@ TEST(BloomFilterTest, CreateShouldReturnNotOKStatusIfPaddingIsTooLarge) {
 
 TEST(BloomFilterTest, MightContainCanProcessNonStandardCharacters) {
   // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑"
-  BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{237, 5}, 5, 8);
+  BloomFilter bloom_filter(std::vector<uint8_t>{237, 5}, 5, 8);
   EXPECT_TRUE(bloom_filter.MightContain("ÀÒ∑"));
   EXPECT_FALSE(bloom_filter.MightContain("Ò∑À"));
 }
 
 TEST(BloomFilterTest, MightContainOnEmptyBloomFilterShouldReturnFalse) {
-  BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{}, 0, 0);
+  BloomFilter bloom_filter(std::vector<uint8_t>{}, 0, 0);
   EXPECT_FALSE(bloom_filter.MightContain(""));
   EXPECT_FALSE(bloom_filter.MightContain("a"));
 }
@@ -124,11 +123,11 @@ TEST(BloomFilterTest, MightContainOnEmptyBloomFilterShouldReturnFalse) {
 TEST(BloomFilterTest,
      MightContainWithEmptyStringMightReturnFalsePositiveResult) {
   {
-    BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{1}, 1, 1);
+    BloomFilter bloom_filter(std::vector<uint8_t>{1}, 1, 1);
     EXPECT_FALSE(bloom_filter.MightContain(""));
   }
   {
-    BloomFilter bloom_filter = BloomFilter(std::vector<uint8_t>{255}, 0, 16);
+    BloomFilter bloom_filter(std::vector<uint8_t>{255}, 0, 16);
     EXPECT_TRUE(bloom_filter.MightContain(""));
   }
 }