diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java index feb29d93e18..745ac0be2e0 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java +++ b/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 snapshot2Ref = new AtomicReference<>(); ArrayList 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()) diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/TestingHooksUtil.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/TestingHooksUtil.java index e2c408f44c3..2e8faa4dc4c 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/TestingHooksUtil.java +++ b/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; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/BloomFilter.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/BloomFilter.java index e51a89906c7..4ba5a84cc5c 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/BloomFilter.java +++ b/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) + "\"}"; } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/BloomFilterException.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/BloomFilterException.java deleted file mode 100644 index 429b501ed47..00000000000 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/BloomFilterException.java +++ /dev/null @@ -1,23 +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. - -package com.google.firebase.firestore.remote; - -import androidx.annotation.NonNull; - -public class BloomFilterException extends RuntimeException { - public BloomFilterException(@NonNull String detailMessage) { - super(detailMessage); - } -} diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.java index b8056377e44..922f8bc2f55 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.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: (" diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/remote/BloomFilterTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/remote/BloomFilterTest.java index c2ba39c4d7f..0c82dcf5c46 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/remote/BloomFilterTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/remote/BloomFilterTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import com.google.protobuf.ByteString; import java.io.BufferedReader; import java.io.FileInputStream; import java.io.InputStreamReader; @@ -42,98 +43,123 @@ public class BloomFilterTest { @Test public void instantiateEmptyBloomFilter() { - BloomFilter bloomFilter = new BloomFilter(new byte[0], 0, 0); + BloomFilter bloomFilter = new BloomFilter(ByteString.empty(), 0, 0); assertEquals(bloomFilter.getBitCount(), 0); } @Test public void instantiateNonEmptyBloomFilter() { { - BloomFilter bloomFilter1 = new BloomFilter(new byte[] {1}, 0, 1); - assertEquals(bloomFilter1.getBitCount(), 8); + BloomFilter bloomFilter = new BloomFilter(ByteString.copyFrom(new byte[] {1}), 0, 1); + assertEquals(bloomFilter.getBitCount(), 8); } { - BloomFilter bloomFilter2 = new BloomFilter(new byte[] {1}, 7, 1); - assertEquals(bloomFilter2.getBitCount(), 1); + BloomFilter bloomFilter = new BloomFilter(ByteString.copyFrom(new byte[] {1}), 7, 1); + assertEquals(bloomFilter.getBitCount(), 1); } } @Test public void constructorShouldThrowNPEOnNullBitmap() { + assertThrows(NullPointerException.class, () -> new BloomFilter(null, 0, 0)); + assertThrows(NullPointerException.class, () -> new BloomFilter(null, 1, 1)); + } + + @Test + public void createShouldCreateAnEmptyBloomFilter() throws Exception { + BloomFilter bloomFilter = BloomFilter.create(ByteString.empty(), 0, 0); + assertEquals(bloomFilter.getBitCount(), 0); + } + + @Test + public void createShouldCreatenNonEmptyBloomFilter() throws Exception { { - NullPointerException emptyBloomFilterException = - assertThrows(NullPointerException.class, () -> new BloomFilter(null, 0, 0)); - assertThat(emptyBloomFilterException).hasMessageThat().contains("Bitmap cannot be null."); + BloomFilter bloomFilter = BloomFilter.create(ByteString.copyFrom(new byte[] {1}), 0, 1); + assertEquals(bloomFilter.getBitCount(), 8); } { - NullPointerException nonEmptyBloomFilterException = - assertThrows(NullPointerException.class, () -> new BloomFilter(null, 1, 1)); - assertThat(nonEmptyBloomFilterException).hasMessageThat().contains("Bitmap cannot be null."); + BloomFilter bloomFilter = BloomFilter.create(ByteString.copyFrom(new byte[] {1}), 7, 1); + assertEquals(bloomFilter.getBitCount(), 1); } } @Test - public void constructorShouldThrowBFEOnEmptyBloomFilterWithNonZeroPadding() { - BloomFilterException exception = - assertThrows(BloomFilterException.class, () -> new BloomFilter(new byte[0], 1, 0)); - assertThat(exception) - .hasMessageThat() - .contains("Expected padding of 0 when bitmap length is 0, but got 1"); + public void createShouldThrowBFEOnEmptyBloomFilterWithNonZeroPadding() { + BloomFilter.BloomFilterCreateException exception = + assertThrows( + BloomFilter.BloomFilterCreateException.class, + () -> BloomFilter.create(ByteString.empty(), 1, 0)); + assertThat(exception).hasMessageThat().ignoringCase().contains("padding of 0"); + assertThat(exception).hasMessageThat().ignoringCase().contains("bitmap length is 0"); + assertThat(exception).hasMessageThat().ignoringCase().contains("got 1"); } @Test - public void constructorShouldThrowBFEOnNonEmptyBloomFilterWithZeroHashCount() { - BloomFilterException zeroHashCountException = - assertThrows(BloomFilterException.class, () -> new BloomFilter(new byte[] {1}, 1, 0)); - assertThat(zeroHashCountException).hasMessageThat().contains("Invalid hash count: 0"); + public void createShouldThrowOnNonEmptyBloomFilterWithZeroHashCount() { + BloomFilter.BloomFilterCreateException exception = + assertThrows( + BloomFilter.BloomFilterCreateException.class, + () -> BloomFilter.create(ByteString.copyFrom(new byte[] {1}), 1, 0)); + assertThat(exception).hasMessageThat().ignoringCase().contains("hash count: 0"); } @Test - public void constructorShouldThrowBFEOnNegativePadding() { + public void createShouldThrowOnNegativePadding() { { - BloomFilterException emptyBloomFilterException = - assertThrows(BloomFilterException.class, () -> new BloomFilter(new byte[0], -1, 0)); - assertThat(emptyBloomFilterException).hasMessageThat().contains("Invalid padding: -1"); + BloomFilter.BloomFilterCreateException exception = + assertThrows( + BloomFilter.BloomFilterCreateException.class, + () -> BloomFilter.create(ByteString.empty(), -1, 0)); + assertThat(exception).hasMessageThat().ignoringCase().contains("padding: -1"); } { - BloomFilterException nonEmptyBloomFilterException = - assertThrows(BloomFilterException.class, () -> new BloomFilter(new byte[] {1}, -1, 1)); - assertThat(nonEmptyBloomFilterException).hasMessageThat().contains("Invalid padding: -1"); + BloomFilter.BloomFilterCreateException exception = + assertThrows( + BloomFilter.BloomFilterCreateException.class, + () -> BloomFilter.create(ByteString.copyFrom(new byte[] {1}), -1, 1)); + assertThat(exception).hasMessageThat().ignoringCase().contains("padding: -1"); } } @Test - public void constructorShouldThrowBFEOnNegativeHashCount() { + public void createShouldThrowOnNegativeHashCount() { { - BloomFilterException emptyBloomFilterException = - assertThrows(BloomFilterException.class, () -> new BloomFilter(new byte[0], 0, -1)); - assertThat(emptyBloomFilterException).hasMessageThat().contains("Invalid hash count: -1"); + BloomFilter.BloomFilterCreateException exception = + assertThrows( + BloomFilter.BloomFilterCreateException.class, + () -> BloomFilter.create(ByteString.empty(), 0, -1)); + assertThat(exception).hasMessageThat().ignoringCase().contains("hash count: -1"); } { - BloomFilterException nonEmptyBloomFilterException = - assertThrows(BloomFilterException.class, () -> new BloomFilter(new byte[] {1}, 1, -1)); - assertThat(nonEmptyBloomFilterException).hasMessageThat().contains("Invalid hash count: -1"); + BloomFilter.BloomFilterCreateException exception = + assertThrows( + BloomFilter.BloomFilterCreateException.class, + () -> BloomFilter.create(ByteString.copyFrom(new byte[] {1}), 1, -1)); + assertThat(exception).hasMessageThat().ignoringCase().contains("hash count: -1"); } } @Test - public void constructorShouldThrowBFEIfPaddingIsTooLarge() { - BloomFilterException exception = - assertThrows(BloomFilterException.class, () -> new BloomFilter(new byte[] {1}, 8, 1)); - assertThat(exception).hasMessageThat().contains("Invalid padding: 8"); + public void createShouldThrowIfPaddingIsTooLarge() { + BloomFilter.BloomFilterCreateException exception = + assertThrows( + BloomFilter.BloomFilterCreateException.class, + () -> BloomFilter.create(ByteString.copyFrom(new byte[] {1}), 8, 1)); + assertThat(exception).hasMessageThat().ignoringCase().contains("padding: 8"); } @Test public void mightContainCanProcessNonStandardCharacters() { // A non-empty BloomFilter object with 1 insertion : "ÀÒ∑" - BloomFilter bloomFilter = new BloomFilter(new byte[] {(byte) 237, 5}, 5, 8); + BloomFilter bloomFilter = + new BloomFilter(ByteString.copyFrom(new byte[] {(byte) 237, 5}), 5, 8); assertTrue(bloomFilter.mightContain("ÀÒ∑")); assertFalse(bloomFilter.mightContain("Ò∑À")); } @Test public void mightContainOnEmptyBloomFilterShouldReturnFalse() { - BloomFilter bloomFilter = new BloomFilter(new byte[0], 0, 0); + BloomFilter bloomFilter = new BloomFilter(ByteString.empty(), 0, 0); assertFalse(bloomFilter.mightContain("")); assertFalse(bloomFilter.mightContain("a")); } @@ -141,26 +167,36 @@ public void mightContainOnEmptyBloomFilterShouldReturnFalse() { @Test public void mightContainWithEmptyStringMightReturnFalsePositiveResult() { { - BloomFilter bloomFilter1 = new BloomFilter(new byte[] {1}, 1, 1); - assertFalse(bloomFilter1.mightContain("")); + BloomFilter bloomFilter = new BloomFilter(ByteString.copyFrom(new byte[] {1}), 1, 1); + assertFalse(bloomFilter.mightContain("")); } { - BloomFilter bloomFilter2 = new BloomFilter(new byte[] {(byte) 255}, 0, 16); - assertTrue(bloomFilter2.mightContain("")); + BloomFilter bloomFilter = + new BloomFilter(ByteString.copyFrom(new byte[] {(byte) 255}), 0, 16); + assertTrue(bloomFilter.mightContain("")); } } @Test - public void bloomFilterToString() { - { - BloomFilter emptyBloomFilter = new BloomFilter(new byte[0], 0, 0); - assertEquals(emptyBloomFilter.toString(), "BloomFilter{hashCount=0, size=0, bitmap=\"\"}"); - } - { - BloomFilter nonEmptyBloomFilter = new BloomFilter(new byte[] {1}, 1, 1); - assertEquals( - nonEmptyBloomFilter.toString(), "BloomFilter{hashCount=1, size=7, bitmap=\"AQ==\"}"); - } + public void toStringOnEmptyBitmap() { + String toStringResult = new BloomFilter(ByteString.empty(), 0, 0).toString(); + assertThat(toStringResult).startsWith("BloomFilter{"); + assertThat(toStringResult).endsWith("}"); + assertThat(toStringResult).contains("hashCount=0"); + assertThat(toStringResult).contains("size=0"); + assertThat(toStringResult).contains("bitmap=\"\""); + } + + @Test + public void toStringOnNonEmptyBitmap() { + String toStringResult = + new BloomFilter(ByteString.copyFrom(new byte[] {0x01, (byte) 0xAE, (byte) 0xFF}), 3, 7) + .toString(); + assertThat(toStringResult).startsWith("BloomFilter{"); + assertThat(toStringResult).endsWith("}"); + assertThat(toStringResult).contains("hashCount=7"); + assertThat(toStringResult).contains("size=21"); + assertThat(toStringResult).contains("bitmap=\"Aa7/\""); } /** @@ -177,7 +213,7 @@ public void bloomFilterToString() { private static void runGoldenTest(String testFile) throws Exception { String resultFile = testFile.replace("bloom_filter_proto", "membership_test_result"); if (resultFile.equals(testFile)) { - throw new BloomFilterException("Cannot find corresponding result file for " + testFile); + throw new IllegalArgumentException("Cannot find corresponding result file for " + testFile); } JSONObject testJson = readJsonFile(testFile); @@ -188,7 +224,8 @@ private static void runGoldenTest(String testFile) throws Exception { int padding = bits.getInt("padding"); int hashCount = testJson.getInt("hashCount"); BloomFilter bloomFilter = - new BloomFilter(Base64.getDecoder().decode(bitmap), padding, hashCount); + BloomFilter.create( + ByteString.copyFrom(Base64.getDecoder().decode(bitmap)), padding, hashCount); String membershipTestResults = resultJSON.getString("membershipTestResults");