From 9909c1f1ecedaa78cad3e620c52b4827d3d030b5 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 17 Feb 2023 14:11:11 -0800 Subject: [PATCH 1/5] add integration test for bloom filter --- .../google/firebase/firestore/QueryTest.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) 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 c2eef0989c2..f9757ac2462 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 @@ -21,8 +21,10 @@ import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testCollectionWithDocs; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testFirestore; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.waitFor; +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.writeAllDocs; import static com.google.firebase.firestore.testutil.TestUtil.expectError; import static com.google.firebase.firestore.testutil.TestUtil.map; +import static com.google.firebase.firestore.util.Util.autoId; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.junit.Assert.assertEquals; @@ -1029,6 +1031,46 @@ public void testMultipleUpdatesWhileOffline() { assertEquals(asList(map("foo", "zzyzx", "bar", "2")), querySnapshotToValues(snapshot2)); } + @Test + public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter() { + Map> testDocs = new LinkedHashMap<>(); + for (int i = 1; i <= 100; i++) { + testDocs.put("doc" + i, map("key", i)); + } + + // Setup firestore with disabled persistence and populate a collection with testDocs. + FirebaseFirestore firestore = testFirestore(); + firestore.setFirestoreSettings( + new FirebaseFirestoreSettings.Builder().setPersistenceEnabled(false).build()); + CollectionReference collection = firestore.collection(autoId()); + writeAllDocs(collection, testDocs); + + QuerySnapshot snapshot1 = waitFor(collection.get()); + assertEquals(snapshot1.size(), 100); + + // Delete 50 docs in transaction so that it doesn't affect local cache. + waitFor( + firestore.runTransaction( + transaction -> { + for (int i = 1; i <= 50; i++) { + DocumentReference docRef = collection.document("doc" + i); + transaction.delete(docRef); + } + return null; + })); + + // Wait 10 seconds, during which Watch will stop tracking the query + // and will send an existence filter rather than "delete" events. + try { + Thread.sleep(10000); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } + + QuerySnapshot snapshot2 = waitFor(collection.get()); + assertEquals(snapshot2.size(), 50); + } + // TODO(orquery): Enable this test when prod supports OR queries. @Ignore @Test From 23e109f8307367eb8c23147f4cbd28dcd2d2b688 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 23 Feb 2023 11:04:38 -0800 Subject: [PATCH 2/5] resolve comments --- .../google/firebase/firestore/QueryTest.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) 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 4fc5f4df398..9053dcedb63 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 @@ -39,6 +39,7 @@ import com.google.firebase.firestore.testutil.EventAccumulator; import com.google.firebase.firestore.testutil.IntegrationTestUtil; import java.util.ArrayList; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -1032,10 +1033,11 @@ public void testMultipleUpdatesWhileOffline() { } @Test - public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter() { - Map> testDocs = new LinkedHashMap<>(); + public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter() + throws InterruptedException { + Map> testData = new HashMap<>(); for (int i = 1; i <= 100; i++) { - testDocs.put("doc" + i, map("key", i)); + testData.put("doc" + i, map("key", i)); } // Setup firestore with disabled persistence and populate a collection with testDocs. @@ -1043,17 +1045,19 @@ public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter( firestore.setFirestoreSettings( new FirebaseFirestoreSettings.Builder().setPersistenceEnabled(false).build()); CollectionReference collection = firestore.collection(autoId()); - writeAllDocs(collection, testDocs); + writeAllDocs(collection, testData); + // Populate the cache and save the resume token. QuerySnapshot snapshot1 = waitFor(collection.get()); assertEquals(snapshot1.size(), 100); + List documents = snapshot1.getDocuments(); // Delete 50 docs in transaction so that it doesn't affect local cache. waitFor( firestore.runTransaction( transaction -> { for (int i = 1; i <= 50; i++) { - DocumentReference docRef = collection.document("doc" + i); + DocumentReference docRef = documents.get(i).getReference(); transaction.delete(docRef); } return null; @@ -1061,11 +1065,7 @@ public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter( // Wait 10 seconds, during which Watch will stop tracking the query // and will send an existence filter rather than "delete" events. - try { - Thread.sleep(10000); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } + Thread.sleep(10000); QuerySnapshot snapshot2 = waitFor(collection.get()); assertEquals(snapshot2.size(), 50); From 1fe5fe99fd1ab0e2639a208c5ced807f265c9940 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 24 Feb 2023 13:29:42 -0800 Subject: [PATCH 3/5] skip the test while using emulator --- .../google/firebase/firestore/QueryTest.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) 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 9053dcedb63..fce238407cb 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 @@ -14,6 +14,7 @@ package com.google.firebase.firestore; +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.isRunningAgainstEmulator; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.nullList; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.querySnapshotToIds; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.querySnapshotToValues; @@ -21,16 +22,15 @@ import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testCollectionWithDocs; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testFirestore; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.waitFor; -import static com.google.firebase.firestore.testutil.IntegrationTestUtil.writeAllDocs; import static com.google.firebase.firestore.testutil.TestUtil.expectError; import static com.google.firebase.firestore.testutil.TestUtil.map; -import static com.google.firebase.firestore.util.Util.autoId; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.gms.tasks.Task; @@ -1035,17 +1035,19 @@ public void testMultipleUpdatesWhileOffline() { @Test public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter() throws InterruptedException { + // TODO(Mila):Remove this condition once the bug is resolved. + assumeFalse( + "Skip this test when running against the Firestore emulator as there is a bug related to " + + "sending existence filter in response: b/270731363.", + isRunningAgainstEmulator()); + Map> testData = new HashMap<>(); for (int i = 1; i <= 100; i++) { testData.put("doc" + i, map("key", i)); } - // Setup firestore with disabled persistence and populate a collection with testDocs. - FirebaseFirestore firestore = testFirestore(); - firestore.setFirestoreSettings( - new FirebaseFirestoreSettings.Builder().setPersistenceEnabled(false).build()); - CollectionReference collection = firestore.collection(autoId()); - writeAllDocs(collection, testData); + CollectionReference collection = testCollectionWithDocs(testData); + FirebaseFirestore firestore = collection.getFirestore(); // Populate the cache and save the resume token. QuerySnapshot snapshot1 = waitFor(collection.get()); From 9f7ebaf6a00cb19a2e546de162e2d9642d8eaa61 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 24 Feb 2023 13:34:30 -0800 Subject: [PATCH 4/5] format --- .../google/firebase/firestore/QueryTest.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) 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 fce238407cb..e2ebee68473 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 @@ -1045,9 +1045,7 @@ public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter( for (int i = 1; i <= 100; i++) { testData.put("doc" + i, map("key", i)); } - CollectionReference collection = testCollectionWithDocs(testData); - FirebaseFirestore firestore = collection.getFirestore(); // Populate the cache and save the resume token. QuerySnapshot snapshot1 = waitFor(collection.get()); @@ -1056,14 +1054,16 @@ public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter( // Delete 50 docs in transaction so that it doesn't affect local cache. waitFor( - firestore.runTransaction( - transaction -> { - for (int i = 1; i <= 50; i++) { - DocumentReference docRef = documents.get(i).getReference(); - transaction.delete(docRef); - } - return null; - })); + collection + .getFirestore() + .runTransaction( + transaction -> { + for (int i = 1; i <= 50; i++) { + DocumentReference docRef = documents.get(i).getReference(); + transaction.delete(docRef); + } + return null; + })); // Wait 10 seconds, during which Watch will stop tracking the query // and will send an existence filter rather than "delete" events. From a6947ca0fc3e79fbf8ccb72f37d497d86723a430 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 27 Feb 2023 09:07:34 -0800 Subject: [PATCH 5/5] remove TODO comment --- .../java/com/google/firebase/firestore/QueryTest.java | 1 - 1 file changed, 1 deletion(-) 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 e2ebee68473..9202fe0376e 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 @@ -1035,7 +1035,6 @@ public void testMultipleUpdatesWhileOffline() { @Test public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter() throws InterruptedException { - // TODO(Mila):Remove this condition once the bug is resolved. assumeFalse( "Skip this test when running against the Firestore emulator as there is a bug related to " + "sending existence filter in response: b/270731363.",