From 271aa31da2f230aea3435716788615df310b8fb3 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 15 Feb 2023 14:07:55 -0800 Subject: [PATCH 1/4] Make OR APIs public. --- .../src/main/java/com/google/firebase/firestore/Filter.java | 4 ---- .../src/main/java/com/google/firebase/firestore/Query.java | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/Filter.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/Filter.java index 534b093dc6a..46be54b5898 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/Filter.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/Filter.java @@ -16,14 +16,10 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.annotation.RestrictTo; import com.google.firebase.firestore.core.FieldFilter.Operator; import java.util.Arrays; import java.util.List; -// TODO(orquery): Remove the `hide` and scope annotations. -/** @hide */ -@RestrictTo(RestrictTo.Scope.LIBRARY) /** * A {@code Filter} represents a restriction on one or more field values and can be used to refine * the results of a {@code Query}. diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java index 89af9608f63..5817b8f4b66 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java @@ -388,14 +388,14 @@ public Query whereNotIn(@NonNull FieldPath fieldPath, @NonNull List Date: Thu, 16 Feb 2023 15:02:01 -0800 Subject: [PATCH 2/4] Enable tests and separate out tests that require composite indexes. --- .../google/firebase/firestore/QueryTest.java | 95 ++++++++++++------- 1 file changed, 61 insertions(+), 34 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 08ffc043d35..8fbfffabf6b 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; @@ -29,6 +30,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.gms.tasks.Task; @@ -42,7 +44,6 @@ import java.util.Map; import java.util.concurrent.Semaphore; import org.junit.After; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; @@ -1029,8 +1030,6 @@ public void testMultipleUpdatesWhileOffline() { assertEquals(asList(map("foo", "zzyzx", "bar", "2")), querySnapshotToValues(snapshot2)); } - // TODO(orquery): Enable this test when prod supports OR queries. - @Ignore @Test public void testOrQueries() { Map> testDocs = @@ -1050,13 +1049,6 @@ public void testOrQueries() { "doc4", "doc5"); - // with one inequality: a>2 || b==1. - checkOnlineAndOfflineResultsMatch( - collection.where(Filter.or(Filter.greaterThan("a", 2), Filter.equalTo("b", 1))), - "doc5", - "doc2", - "doc3"); - // (a==1 && b==0) || (a==3 && b==2) checkOnlineAndOfflineResultsMatch( collection.where( @@ -1082,6 +1074,35 @@ public void testOrQueries() { Filter.or(Filter.equalTo("a", 3), Filter.equalTo("b", 3)))), "doc3"); + // Test with limits without orderBy (the __name__ ordering is the tie breaker). + checkOnlineAndOfflineResultsMatch( + collection.where(Filter.or(Filter.equalTo("a", 2), Filter.equalTo("b", 1))).limit(1), + "doc2"); + } + + @Test + public void testOrQueriesWithCompositeIndexes() { + assumeTrue( + "Skip this test if running against production because it results in a " + + "'missing index' error. The Firestore Emulator, however, does serve these " + + " queries.", + isRunningAgainstEmulator()); + Map> testDocs = + map( + "doc1", map("a", 1, "b", 0), + "doc2", map("a", 2, "b", 1), + "doc3", map("a", 3, "b", 2), + "doc4", map("a", 1, "b", 3), + "doc5", map("a", 1, "b", 1)); + CollectionReference collection = testCollectionWithDocs(testDocs); + + // with one inequality: a>2 || b==1. + checkOnlineAndOfflineResultsMatch( + collection.where(Filter.or(Filter.greaterThan("a", 2), Filter.equalTo("b", 1))), + "doc5", + "doc2", + "doc3"); + // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 checkOnlineAndOfflineResultsMatch( collection.where(Filter.or(Filter.equalTo("a", 1), Filter.greaterThan("b", 0))).limit(2), @@ -1113,17 +1134,10 @@ public void testOrQueries() { .limitToLast(1) .orderBy("a"), "doc2"); - - // Test with limits without orderBy (the __name__ ordering is the tie breaker). - checkOnlineAndOfflineResultsMatch( - collection.where(Filter.or(Filter.equalTo("a", 2), Filter.equalTo("b", 1))).limit(1), - "doc2"); } - // TODO(orquery): Enable this test when prod supports OR queries. - @Ignore @Test - public void testOrQueriesWithInAndNotIn() { + public void testOrQueriesWithIn() { Map> testDocs = map( "doc1", map("a", 1, "b", 0), @@ -1140,6 +1154,24 @@ public void testOrQueriesWithInAndNotIn() { "doc3", "doc4", "doc6"); + } + + @Test + public void testOrQueriesWithNotIn() { + assumeTrue( + "Skip this test if running against production because it results in a " + + "'missing index' error. The Firestore Emulator, however, does serve these " + + " queries", + isRunningAgainstEmulator()); + Map> testDocs = + map( + "doc1", map("a", 1, "b", 0), + "doc2", map("b", 1), + "doc3", map("a", 3, "b", 2), + "doc4", map("a", 1, "b", 3), + "doc5", map("a", 1), + "doc6", map("a", 2)); + CollectionReference collection = testCollectionWithDocs(testDocs); // a==2 || b not-in [2,3] // Has implicit orderBy b. @@ -1149,8 +1181,6 @@ public void testOrQueriesWithInAndNotIn() { "doc2"); } - // TODO(orquery): Enable this test when prod supports OR queries. - @Ignore @Test public void testOrQueriesWithArrayMembership() { Map> testDocs = @@ -1179,7 +1209,6 @@ public void testOrQueriesWithArrayMembership() { "doc6"); } - @Ignore @Test public void testMultipleInOps() { Map> testDocs = @@ -1194,16 +1223,14 @@ public void testMultipleInOps() { // Two IN operations on different fields with disjunction. Query query1 = - collection - .where(Filter.or(Filter.inArray("a", asList(2, 3)), Filter.inArray("b", asList(0, 2)))) - .orderBy("a"); - checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc6", "doc3"); + collection.where( + Filter.or(Filter.inArray("a", asList(2, 3)), Filter.inArray("b", asList(0, 2)))); + checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc3", "doc6"); // Two IN operations on different fields with conjunction. Query query2 = - collection - .where(Filter.and(Filter.inArray("a", asList(2, 3)), Filter.inArray("b", asList(0, 2)))) - .orderBy("a"); + collection.where( + Filter.and(Filter.inArray("a", asList(2, 3)), Filter.inArray("b", asList(0, 2)))); checkOnlineAndOfflineResultsMatch(query2, "doc3"); // Two IN operations on the same field. @@ -1232,9 +1259,8 @@ public void testMultipleInOps() { Filter.inArray("a", asList(1, 3)), Filter.or( Filter.inArray("a", asList(0, 2)), - Filter.and( - Filter.greaterThanOrEqualTo("b", 1), Filter.inArray("a", asList(1, 3)))))); - checkOnlineAndOfflineResultsMatch(query6, "doc3", "doc4"); + Filter.and(Filter.equalTo("b", 2), Filter.inArray("a", asList(1, 3)))))); + checkOnlineAndOfflineResultsMatch(query6, "doc3"); // Nested composite filter on different fields. Query query7 = @@ -1248,7 +1274,6 @@ public void testMultipleInOps() { checkOnlineAndOfflineResultsMatch(query7, "doc4"); } - @Ignore @Test public void testUsingInWithArrayContainsAny() { Map> testDocs = @@ -1288,7 +1313,6 @@ public void testUsingInWithArrayContainsAny() { checkOnlineAndOfflineResultsMatch(query4, "doc3", "doc6"); } - @Ignore @Test public void testUsingInWithArrayContains() { Map> testDocs = @@ -1326,9 +1350,12 @@ public void testUsingInWithArrayContains() { checkOnlineAndOfflineResultsMatch(query4, "doc3"); } - @Ignore @Test public void testOrderByEquality() { + assumeTrue( + "Skip this test if running against production because order-by-equality is " + + "not supported yet.", + isRunningAgainstEmulator()); Map> testDocs = map( "doc1", map("a", 1, "b", asList(0)), From 12d5b538bee968a12c88ee89f81e0bb0500e208e Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Thu, 16 Feb 2023 15:34:08 -0800 Subject: [PATCH 3/4] Update api.txt. --- firebase-firestore/api.txt | 27 +++++++++ .../google/firebase/firestore/QueryTest.java | 60 ++----------------- 2 files changed, 31 insertions(+), 56 deletions(-) diff --git a/firebase-firestore/api.txt b/firebase-firestore/api.txt index edf6c6ed73b..6e11846bf99 100644 --- a/firebase-firestore/api.txt +++ b/firebase-firestore/api.txt @@ -145,6 +145,32 @@ package com.google.firebase.firestore { method @NonNull public static com.google.firebase.firestore.FieldValue serverTimestamp(); } + public class Filter { + ctor public Filter(); + method @NonNull public static com.google.firebase.firestore.Filter and(com.google.firebase.firestore.Filter...); + method @NonNull public static com.google.firebase.firestore.Filter arrayContains(@NonNull String, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter arrayContains(@NonNull com.google.firebase.firestore.FieldPath, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter arrayContainsAny(@NonNull String, @NonNull java.util.List); + method @NonNull public static com.google.firebase.firestore.Filter arrayContainsAny(@NonNull com.google.firebase.firestore.FieldPath, @NonNull java.util.List); + method @NonNull public static com.google.firebase.firestore.Filter equalTo(@NonNull String, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter equalTo(@NonNull com.google.firebase.firestore.FieldPath, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter greaterThan(@NonNull String, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter greaterThan(@NonNull com.google.firebase.firestore.FieldPath, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter greaterThanOrEqualTo(@NonNull String, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter greaterThanOrEqualTo(@NonNull com.google.firebase.firestore.FieldPath, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter inArray(@NonNull String, @NonNull java.util.List); + method @NonNull public static com.google.firebase.firestore.Filter inArray(@NonNull com.google.firebase.firestore.FieldPath, @NonNull java.util.List); + method @NonNull public static com.google.firebase.firestore.Filter lessThan(@NonNull String, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter lessThan(@NonNull com.google.firebase.firestore.FieldPath, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter lessThanOrEqualTo(@NonNull String, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter lessThanOrEqualTo(@NonNull com.google.firebase.firestore.FieldPath, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter notEqualTo(@NonNull String, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter notEqualTo(@NonNull com.google.firebase.firestore.FieldPath, @Nullable Object); + method @NonNull public static com.google.firebase.firestore.Filter notInArray(@NonNull String, @NonNull java.util.List); + method @NonNull public static com.google.firebase.firestore.Filter notInArray(@NonNull com.google.firebase.firestore.FieldPath, @NonNull java.util.List); + method @NonNull public static com.google.firebase.firestore.Filter or(com.google.firebase.firestore.Filter...); + } + public class FirebaseFirestore { method @NonNull public com.google.firebase.firestore.ListenerRegistration addSnapshotsInSyncListener(@NonNull Runnable); method @NonNull public com.google.firebase.firestore.ListenerRegistration addSnapshotsInSyncListener(@NonNull android.app.Activity, @NonNull Runnable); @@ -322,6 +348,7 @@ package com.google.firebase.firestore { method @NonNull public com.google.firebase.firestore.Query startAfter(java.lang.Object...); method @NonNull public com.google.firebase.firestore.Query startAt(@NonNull com.google.firebase.firestore.DocumentSnapshot); method @NonNull public com.google.firebase.firestore.Query startAt(java.lang.Object...); + method @NonNull public com.google.firebase.firestore.Query where(@NonNull com.google.firebase.firestore.Filter); method @NonNull public com.google.firebase.firestore.Query whereArrayContains(@NonNull String, @NonNull Object); method @NonNull public com.google.firebase.firestore.Query whereArrayContains(@NonNull com.google.firebase.firestore.FieldPath, @NonNull Object); method @NonNull public com.google.firebase.firestore.Query whereArrayContainsAny(@NonNull String, @NonNull java.util.List); 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 8fbfffabf6b..2e29d72a35f 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 @@ -1227,51 +1227,12 @@ public void testMultipleInOps() { Filter.or(Filter.inArray("a", asList(2, 3)), Filter.inArray("b", asList(0, 2)))); checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc3", "doc6"); - // Two IN operations on different fields with conjunction. - Query query2 = - collection.where( - Filter.and(Filter.inArray("a", asList(2, 3)), Filter.inArray("b", asList(0, 2)))); - checkOnlineAndOfflineResultsMatch(query2, "doc3"); - - // Two IN operations on the same field. - // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". - Query query3 = - collection.where( - Filter.and(Filter.inArray("a", asList(1, 2, 3)), Filter.inArray("a", asList(0, 1, 4)))); - checkOnlineAndOfflineResultsMatch(query3, "doc1", "doc4", "doc5"); - - // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an empty set. - Query query4 = - collection.where( - Filter.and(Filter.inArray("a", asList(2, 3)), Filter.inArray("a", asList(0, 1, 4)))); - checkOnlineAndOfflineResultsMatch(query4); - + // Two IN operations on the same field with disjunction. // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). - Query query5 = + Query query2 = collection.where( Filter.or(Filter.inArray("a", asList(0, 3)), Filter.inArray("a", asList(0, 2)))); - checkOnlineAndOfflineResultsMatch(query5, "doc3", "doc6"); - - // Nested composite filter on the same field. - Query query6 = - collection.where( - Filter.and( - Filter.inArray("a", asList(1, 3)), - Filter.or( - Filter.inArray("a", asList(0, 2)), - Filter.and(Filter.equalTo("b", 2), Filter.inArray("a", asList(1, 3)))))); - checkOnlineAndOfflineResultsMatch(query6, "doc3"); - - // Nested composite filter on different fields. - Query query7 = - collection.where( - Filter.and( - Filter.inArray("b", asList(0, 3)), - Filter.or( - Filter.inArray("b", asList(1)), - Filter.and( - Filter.inArray("b", asList(2, 3)), Filter.inArray("a", asList(1, 3)))))); - checkOnlineAndOfflineResultsMatch(query7, "doc4"); + checkOnlineAndOfflineResultsMatch(query2, "doc3", "doc6"); } @Test @@ -1293,24 +1254,11 @@ public void testUsingInWithArrayContainsAny() { checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc3", "doc4", "doc6"); Query query2 = - collection.where( - Filter.and( - Filter.inArray("a", asList(2, 3)), Filter.arrayContainsAny("b", asList(0, 7)))); - checkOnlineAndOfflineResultsMatch(query2, "doc3"); - - Query query3 = collection.where( Filter.or( Filter.and(Filter.inArray("a", asList(2, 3)), Filter.equalTo("c", 10)), Filter.arrayContainsAny("b", asList(0, 7)))); - checkOnlineAndOfflineResultsMatch(query3, "doc1", "doc3", "doc4"); - - Query query4 = - collection.where( - Filter.and( - Filter.inArray("a", asList(2, 3)), - Filter.or(Filter.arrayContainsAny("b", asList(0, 7)), Filter.equalTo("c", 20)))); - checkOnlineAndOfflineResultsMatch(query4, "doc3", "doc6"); + checkOnlineAndOfflineResultsMatch(query2, "doc1", "doc3", "doc4"); } @Test From 4becf1a148e45ef31116af96dbc97ab79abb0341 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Fri, 3 Mar 2023 14:33:10 -0800 Subject: [PATCH 4/4] Disable two more tests against production. --- firebase-firestore/CHANGELOG.md | 1 + .../java/com/google/firebase/firestore/QueryTest.java | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/firebase-firestore/CHANGELOG.md b/firebase-firestore/CHANGELOG.md index 301f1ba4102..e879c27c788 100644 --- a/firebase-firestore/CHANGELOG.md +++ b/firebase-firestore/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased * [fixed] Fix a potential high-memory usage issue. * [fixed] Fix an issue that stops some performance optimization being applied. +* [feature] Add support for disjunctions in queries (`OR` queries). # 24.4.1 * [fixed] Fix `FAILED_PRECONDITION` when writing to a deleted document in a 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 2e29d72a35f..0f3e03f8a7c 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 @@ -1211,6 +1211,10 @@ public void testOrQueriesWithArrayMembership() { @Test public void testMultipleInOps() { + // TODO(orquery): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because it's not yet supported.", + isRunningAgainstEmulator()); Map> testDocs = map( "doc1", map("a", 1, "b", 0), @@ -1237,6 +1241,10 @@ public void testMultipleInOps() { @Test public void testUsingInWithArrayContainsAny() { + // TODO(orquery): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because it's not yet supported.", + isRunningAgainstEmulator()); Map> testDocs = map( "doc1", map("a", 1, "b", asList(0)), @@ -1300,6 +1308,7 @@ public void testUsingInWithArrayContains() { @Test public void testOrderByEquality() { + // TODO(orquery): Enable this test against production when possible. assumeTrue( "Skip this test if running against production because order-by-equality is " + "not supported yet.",