Random improvements.

Author: dconeybe

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java

@@ -50,7 +50,6 @@
 import java.util.Map;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicReference;
-
 import org.junit.After;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -1092,11 +1091,11 @@ public void resumingAQueryShouldUseBloomFilterToAvoidFullRequery() throws Except
       // bloom filter, and it was used to avert a full requery.
       AtomicReference<QuerySnapshot> snapshot2Ref = new AtomicReference<>();
       ArrayList<ExistenceFilterMismatchInfo> existenceFilterMismatches =
-              captureExistenceFilterMismatches(
-                      () -> {
-                        QuerySnapshot querySnapshot = waitFor(collection.get());
-                        snapshot2Ref.set(querySnapshot);
-                      });
+          captureExistenceFilterMismatches(
+              () -> {
+                QuerySnapshot querySnapshot = waitFor(collection.get());
+                snapshot2Ref.set(querySnapshot);
+              });
       QuerySnapshot snapshot2 = snapshot2Ref.get();
 
       // Verify that the snapshot from the resumed query contains the expected documents; that is,
@@ -1132,8 +1131,8 @@ public void resumingAQueryShouldUseBloomFilterToAvoidFullRequery() throws Except
       // Verify that Watch sent an existence filter with the correct counts when the query was
       // resumed.
       assertWithMessage("Watch should have sent exactly 1 existence filter")
-              .that(existenceFilterMismatches)
-              .hasSize(1);
+          .that(existenceFilterMismatches)
+          .hasSize(1);
       ExistenceFilterMismatchInfo existenceFilterMismatchInfo = existenceFilterMismatches.get(0);
       assertWithMessage("localCacheCount")
           .that(existenceFilterMismatchInfo.localCacheCount())

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/TestingHooksUtil.java

@@ -16,7 +16,6 @@
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
-
 import com.google.firebase.firestore.ListenerRegistration;
 import java.util.ArrayList;
 

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/BloomFilter.java

@@ -17,42 +17,82 @@
 import android.util.Base64;
 import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
+import com.google.protobuf.ByteString;
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 
-public class BloomFilter {
+public final class BloomFilter {
   private final int bitCount;
-  private final byte[] bitmap;
+  private final ByteString bitmap;
   private final int hashCount;
   private final MessageDigest md5HashMessageDigest;
 
-  public BloomFilter(@NonNull byte[] bitmap, int padding, int hashCount) {
-    if (bitmap == null) {
-      throw new NullPointerException("Bitmap cannot be null.");
-    }
+  /**
+   * Creates a new {@link BloomFilter} with the given parameters.
+   *
+   * @param bitmap the bitmap of the bloom filter; must not be null.
+   * @param padding the padding, in bits, of the last byte of the bloom filter; must be between 0
+   *     (zero) and 7, inclusive; must be 0 (zero) if {@code bitmap.length==0}.
+   * @param hashCount The number of hash functions to use; must be strictly greater than zero; may
+   *     be 0 (zero) if and only if {@code bitmap.length==0}.
+   */
+  public BloomFilter(@NonNull ByteString bitmap, int padding, int hashCount) {
     if (padding < 0 || padding >= 8) {
-      throw new BloomFilterException("Invalid padding: " + padding);
+      throw new IllegalArgumentException("Invalid padding: " + padding);
     }
     if (hashCount < 0) {
-      throw new BloomFilterException("Invalid hash count: " + hashCount);
+      throw new IllegalArgumentException("Invalid hash count: " + hashCount);
     }
-    if (bitmap.length > 0 && hashCount == 0) {
+    if (bitmap.size() > 0 && hashCount == 0) {
       // Only empty bloom filter can have 0 hash count.
-      throw new BloomFilterException("Invalid hash count: " + hashCount);
+      throw new IllegalArgumentException("Invalid hash count: " + hashCount);
     }
-    if (bitmap.length == 0 && padding != 0) {
+    if (bitmap.size() == 0 && padding != 0) {
       // Empty bloom filter should have 0 padding.
-      throw new BloomFilterException(
+      throw new IllegalArgumentException(
           "Expected padding of 0 when bitmap length is 0, but got " + padding);
     }
 
     this.bitmap = bitmap;
     this.hashCount = hashCount;
-    this.bitCount = bitmap.length * 8 - padding;
+    this.bitCount = bitmap.size() * 8 - padding;
     this.md5HashMessageDigest = createMd5HashMessageDigest();
   }
 
+  /**
+   * Creates an instance of {@link BloomFilter} with the given arguments, throwing a well-defined
+   * exception if the given arguments do not satisfy the requirements documented in the {@link
+   * BloomFilter} constructor.
+   */
+  public static BloomFilter create(@NonNull ByteString bitmap, int padding, int hashCount)
+      throws BloomFilterCreateException {
+    if (padding < 0 || padding >= 8) {
+      throw new BloomFilterCreateException("Invalid padding: " + padding);
+    }
+    if (hashCount < 0) {
+      throw new BloomFilterCreateException("Invalid hash count: " + hashCount);
+    }
+    if (bitmap.size() > 0 && hashCount == 0) {
+      // Only empty bloom filter can have 0 hash count.
+      throw new BloomFilterCreateException("Invalid hash count: " + hashCount);
+    }
+    if (bitmap.size() == 0 && padding != 0) {
+      // Empty bloom filter should have 0 padding.
+      throw new BloomFilterCreateException(
+          "Expected padding of 0 when bitmap length is 0, but got " + padding);
+    }
+
+    return new BloomFilter(bitmap, padding, hashCount);
+  }
+
+  /** Exception thrown by {@link #create} if the given arguments are not valid. */
+  public static final class BloomFilterCreateException extends Exception {
+    public BloomFilterCreateException(String message) {
+      super(message);
+    }
+  }
+
   @VisibleForTesting
   int getBitCount() {
     return this.bitCount;
@@ -146,7 +186,7 @@ private static long unsignedRemainder(long dividend, long divisor) {
   /** Return whether the bit at the given index in the bitmap is set to 1. */
   private boolean isBitSet(int index) {
     // To retrieve bit n, calculate: (bitmap[n / 8] & (0x01 << (n % 8))).
-    byte byteAtIndex = this.bitmap[index / 8];
+    byte byteAtIndex = this.bitmap.byteAt(index / 8);
     int offset = index % 8;
     return (byteAtIndex & (0x01 << offset)) != 0;
   }
@@ -159,7 +199,7 @@ public String toString() {
         + ", size="
         + bitCount
         + ", bitmap=\""
-        + Base64.encodeToString(bitmap, Base64.NO_WRAP)
+        + Base64.encodeToString(bitmap.toByteArray(), Base64.NO_WRAP)
         + "\"}";
   }
 }

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/BloomFilterException.java

@@ -31,6 +31,7 @@
 import com.google.firebase.firestore.remote.WatchChange.ExistenceFilterWatchChange;
 import com.google.firebase.firestore.remote.WatchChange.WatchTargetChange;
 import com.google.firebase.firestore.util.Logger;
+import com.google.protobuf.ByteString;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -252,14 +253,14 @@ private BloomFilterApplicationStatus applyBloomFilter(
       return BloomFilterApplicationStatus.SKIPPED;
     }
 
-    byte[] bitmap = unchangedNames.getBits().getBitmap().toByteArray();
+    ByteString bitmap = unchangedNames.getBits().getBitmap();
     BloomFilter bloomFilter;
 
     try {
       bloomFilter =
-          new BloomFilter(
+          BloomFilter.create(
               bitmap, unchangedNames.getBits().getPadding(), unchangedNames.getHashCount());
-    } catch (BloomFilterException e) {
+    } catch (BloomFilter.BloomFilterCreateException e) {
       Logger.warn(
           LOG_TAG,
           "Applying bloom filter failed: ("