Skip to content

Commit 70c8e89

Browse files
ehsannasehsannas
authored
Improve the integration test coverage for online vs offline comparisons. (#6841)
This PR improves the way `checkOnlineAndOfflineResultsMatch` test util function was written. It does so by first pre-populating the cache with the entire collection, then performing the query from cache (this results in a full collection scan), then performing the query from the server, and then performing the query from cache again (this results in using `performQueryUsingRemoteKeys`. It then ensures that all of these 3 results are the same and equal to the expected results. #no-changelog
1 parent 3b7e1e4 commit 70c8e89

File tree

6 files changed

+91
-42
lines changed

6 files changed

+91
-42
lines changed

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/CompositeIndexQueryTest.java

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,24 +91,27 @@ public void testOrQueriesWithCompositeIndexes() {
9191

9292
Query query = collection.where(or(greaterThan("a", 2), equalTo("b", 1)));
9393
// with one inequality: a>2 || b==1.
94-
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc5", "doc2", "doc3");
94+
testHelper.assertOnlineAndOfflineResultsMatch(
95+
collection, testHelper.query(query), "doc5", "doc2", "doc3");
9596

9697
// Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2
9798
query = collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limit(2);
98-
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc1", "doc2");
99+
testHelper.assertOnlineAndOfflineResultsMatch(
100+
collection, testHelper.query(query), "doc1", "doc2");
99101

100102
// Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2
101103
// Note: The public query API does not allow implicit ordering when limitToLast is used.
102104
query = collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limitToLast(2).orderBy("b");
103-
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc3", "doc4");
105+
testHelper.assertOnlineAndOfflineResultsMatch(
106+
collection, testHelper.query(query), "doc3", "doc4");
104107

105108
// Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1
106109
query = collection.where(or(equalTo("a", 2), equalTo("b", 1))).limit(1).orderBy("a");
107-
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc5");
110+
testHelper.assertOnlineAndOfflineResultsMatch(collection, testHelper.query(query), "doc5");
108111

109112
// Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1
110113
query = collection.where(or(equalTo("a", 2), equalTo("b", 1))).limitToLast(1).orderBy("a");
111-
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc2");
114+
testHelper.assertOnlineAndOfflineResultsMatch(collection, testHelper.query(query), "doc2");
112115
}
113116

114117
@Test
@@ -771,17 +774,17 @@ public void testMultipleInequalityFromCacheAndFromServer() {
771774

772775
// implicit AND: a != 1 && b < 2
773776
Query query1 = testHelper.query(collection).whereNotEqualTo("a", 1).whereLessThan("b", 2);
774-
testHelper.assertOnlineAndOfflineResultsMatch(query1, "doc2");
777+
testHelper.assertOnlineAndOfflineResultsMatch(collection, query1, "doc2");
775778

776779
// explicit AND: a != 1 && b < 2
777780
Query query2 = testHelper.query(collection).where(and(notEqualTo("a", 1), lessThan("b", 2)));
778-
testHelper.assertOnlineAndOfflineResultsMatch(query2, "doc2");
781+
testHelper.assertOnlineAndOfflineResultsMatch(collection, query2, "doc2");
779782

780783
// explicit AND: a < 3 && b not-in [2, 3]
781784
// Implicitly ordered by: a asc, b asc, __name__ asc
782785
Query query3 =
783786
testHelper.query(collection).where(and(lessThan("a", 3), notInArray("b", asList(2, 3))));
784-
testHelper.assertOnlineAndOfflineResultsMatch(query3, "doc1", "doc5", "doc2");
787+
testHelper.assertOnlineAndOfflineResultsMatch(collection, query3, "doc1", "doc5", "doc2");
785788

786789
// a <3 && b != 0, ordered by: b desc, a desc, __name__ desc
787790
Query query4 =
@@ -791,11 +794,11 @@ public void testMultipleInequalityFromCacheAndFromServer() {
791794
.whereNotEqualTo("b", 0)
792795
.orderBy("b", Direction.DESCENDING)
793796
.limit(2);
794-
testHelper.assertOnlineAndOfflineResultsMatch(query4, "doc4", "doc2");
797+
testHelper.assertOnlineAndOfflineResultsMatch(collection, query4, "doc4", "doc2");
795798

796799
// explicit OR: a>2 || b<1.
797800
Query query5 = testHelper.query(collection).where(or(greaterThan("a", 2), lessThan("b", 1)));
798-
testHelper.assertOnlineAndOfflineResultsMatch(query5, "doc1", "doc3");
801+
testHelper.assertOnlineAndOfflineResultsMatch(collection, query5, "doc1", "doc3");
799802
}
800803

801804
@Test

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,7 +1651,7 @@ public void sdkOrdersQueryByDocumentIdTheSameWayOnlineAndOffline() {
16511651
"a");
16521652

16531653
// Run query with snapshot listener
1654-
checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
1654+
checkOnlineAndOfflineResultsMatch(colRef, orderedQuery, expectedDocIds.toArray(new String[0]));
16551655
}
16561656

16571657
@Test
@@ -1708,7 +1708,7 @@ public void snapshotListenerSortsUnicodeStringsAsServer() {
17081708
assertTrue(getSnapshotDocIds.equals(expectedDocIds));
17091709
assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
17101710

1711-
checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
1711+
checkOnlineAndOfflineResultsMatch(colRef, orderedQuery, expectedDocIds.toArray(new String[0]));
17121712
}
17131713

17141714
@Test
@@ -1765,7 +1765,7 @@ public void snapshotListenerSortsUnicodeStringsInArrayAsServer() {
17651765
assertTrue(getSnapshotDocIds.equals(expectedDocIds));
17661766
assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
17671767

1768-
checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
1768+
checkOnlineAndOfflineResultsMatch(colRef, orderedQuery, expectedDocIds.toArray(new String[0]));
17691769
}
17701770

17711771
@Test
@@ -1822,7 +1822,7 @@ public void snapshotListenerSortsUnicodeStringsInMapAsServer() {
18221822
assertTrue(getSnapshotDocIds.equals(expectedDocIds));
18231823
assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
18241824

1825-
checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
1825+
checkOnlineAndOfflineResultsMatch(colRef, orderedQuery, expectedDocIds.toArray(new String[0]));
18261826
}
18271827

18281828
@Test
@@ -1879,7 +1879,7 @@ public void snapshotListenerSortsUnicodeStringsInMapKeyAsServer() {
18791879
assertTrue(getSnapshotDocIds.equals(expectedDocIds));
18801880
assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
18811881

1882-
checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
1882+
checkOnlineAndOfflineResultsMatch(colRef, orderedQuery, expectedDocIds.toArray(new String[0]));
18831883
}
18841884

18851885
@Test
@@ -1937,7 +1937,7 @@ public void snapshotListenerSortsUnicodeStringsInDocumentKeyAsServer() {
19371937
assertTrue(getSnapshotDocIds.equals(expectedDocIds));
19381938
assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
19391939

1940-
checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
1940+
checkOnlineAndOfflineResultsMatch(colRef, orderedQuery, expectedDocIds.toArray(new String[0]));
19411941
}
19421942

19431943
@Test
@@ -1986,6 +1986,6 @@ public void snapshotListenerSortsInvalidUnicodeStringsAsServer() {
19861986
assertTrue(getSnapshotDocIds.equals(expectedDocIds));
19871987
assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
19881988

1989-
checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
1989+
checkOnlineAndOfflineResultsMatch(colRef, orderedQuery, expectedDocIds.toArray(new String[0]));
19901990
}
19911991
}

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

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,30 +1470,38 @@ public void testOrQueries() {
14701470

14711471
// Two equalities: a==1 || b==1.
14721472
checkOnlineAndOfflineResultsMatch(
1473-
collection.where(or(equalTo("a", 1), equalTo("b", 1))), "doc1", "doc2", "doc4", "doc5");
1473+
collection,
1474+
collection.where(or(equalTo("a", 1), equalTo("b", 1))),
1475+
"doc1",
1476+
"doc2",
1477+
"doc4",
1478+
"doc5");
14741479

14751480
// (a==1 && b==0) || (a==3 && b==2)
14761481
checkOnlineAndOfflineResultsMatch(
1482+
collection,
14771483
collection.where(
14781484
or(and(equalTo("a", 1), equalTo("b", 0)), and(equalTo("a", 3), equalTo("b", 2)))),
14791485
"doc1",
14801486
"doc3");
14811487

14821488
// a==1 && (b==0 || b==3).
14831489
checkOnlineAndOfflineResultsMatch(
1490+
collection,
14841491
collection.where(and(equalTo("a", 1), or(equalTo("b", 0), equalTo("b", 3)))),
14851492
"doc1",
14861493
"doc4");
14871494

14881495
// (a==2 || b==2) && (a==3 || b==3)
14891496
checkOnlineAndOfflineResultsMatch(
1497+
collection,
14901498
collection.where(
14911499
and(or(equalTo("a", 2), equalTo("b", 2)), or(equalTo("a", 3), equalTo("b", 3)))),
14921500
"doc3");
14931501

14941502
// Test with limits without orderBy (the __name__ ordering is the tie breaker).
14951503
checkOnlineAndOfflineResultsMatch(
1496-
collection.where(or(equalTo("a", 2), equalTo("b", 1))).limit(1), "doc2");
1504+
collection, collection.where(or(equalTo("a", 2), equalTo("b", 1))).limit(1), "doc2");
14971505
}
14981506

14991507
@Test
@@ -1510,7 +1518,11 @@ public void testOrQueriesWithIn() {
15101518

15111519
// a==2 || b in [2,3]
15121520
checkOnlineAndOfflineResultsMatch(
1513-
collection.where(or(equalTo("a", 2), inArray("b", asList(2, 3)))), "doc3", "doc4", "doc6");
1521+
collection,
1522+
collection.where(or(equalTo("a", 2), inArray("b", asList(2, 3)))),
1523+
"doc3",
1524+
"doc4",
1525+
"doc6");
15141526
}
15151527

15161528
@Test
@@ -1527,10 +1539,15 @@ public void testOrQueriesWithArrayMembership() {
15271539

15281540
// a==2 || b array-contains 7
15291541
checkOnlineAndOfflineResultsMatch(
1530-
collection.where(or(equalTo("a", 2), arrayContains("b", 7))), "doc3", "doc4", "doc6");
1542+
collection,
1543+
collection.where(or(equalTo("a", 2), arrayContains("b", 7))),
1544+
"doc3",
1545+
"doc4",
1546+
"doc6");
15311547

15321548
// a==2 || b array-contains-any [0, 3]
15331549
checkOnlineAndOfflineResultsMatch(
1550+
collection,
15341551
collection.where(or(equalTo("a", 2), arrayContainsAny("b", asList(0, 3)))),
15351552
"doc1",
15361553
"doc4",
@@ -1551,12 +1568,12 @@ public void testMultipleInOps() {
15511568

15521569
// Two IN operations on different fields with disjunction.
15531570
Query query1 = collection.where(or(inArray("a", asList(2, 3)), inArray("b", asList(0, 2))));
1554-
checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc3", "doc6");
1571+
checkOnlineAndOfflineResultsMatch(collection, query1, "doc1", "doc3", "doc6");
15551572

15561573
// Two IN operations on the same field with disjunction.
15571574
// a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]).
15581575
Query query2 = collection.where(or(inArray("a", asList(0, 3)), inArray("a", asList(0, 2))));
1559-
checkOnlineAndOfflineResultsMatch(query2, "doc3", "doc6");
1576+
checkOnlineAndOfflineResultsMatch(collection, query2, "doc3", "doc6");
15601577
}
15611578

15621579
@Test
@@ -1573,14 +1590,14 @@ public void testUsingInWithArrayContainsAny() {
15731590

15741591
Query query1 =
15751592
collection.where(or(inArray("a", asList(2, 3)), arrayContainsAny("b", asList(0, 7))));
1576-
checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc3", "doc4", "doc6");
1593+
checkOnlineAndOfflineResultsMatch(collection, query1, "doc1", "doc3", "doc4", "doc6");
15771594

15781595
Query query2 =
15791596
collection.where(
15801597
or(
15811598
and(inArray("a", asList(2, 3)), equalTo("c", 10)),
15821599
arrayContainsAny("b", asList(0, 7))));
1583-
checkOnlineAndOfflineResultsMatch(query2, "doc1", "doc3", "doc4");
1600+
checkOnlineAndOfflineResultsMatch(collection, query2, "doc1", "doc3", "doc4");
15841601
}
15851602

15861603
@Test
@@ -1596,20 +1613,20 @@ public void testUsingInWithArrayContains() {
15961613
CollectionReference collection = testCollectionWithDocs(testDocs);
15971614

15981615
Query query1 = collection.where(or(inArray("a", asList(2, 3)), arrayContains("b", 3)));
1599-
checkOnlineAndOfflineResultsMatch(query1, "doc3", "doc4", "doc6");
1616+
checkOnlineAndOfflineResultsMatch(collection, query1, "doc3", "doc4", "doc6");
16001617

16011618
Query query2 = collection.where(and(inArray("a", asList(2, 3)), arrayContains("b", 7)));
1602-
checkOnlineAndOfflineResultsMatch(query2, "doc3");
1619+
checkOnlineAndOfflineResultsMatch(collection, query2, "doc3");
16031620

16041621
Query query3 =
16051622
collection.where(
16061623
or(inArray("a", asList(2, 3)), and(arrayContains("b", 3), equalTo("a", 1))));
1607-
checkOnlineAndOfflineResultsMatch(query3, "doc3", "doc4", "doc6");
1624+
checkOnlineAndOfflineResultsMatch(collection, query3, "doc3", "doc4", "doc6");
16081625

16091626
Query query4 =
16101627
collection.where(
16111628
and(inArray("a", asList(2, 3)), or(arrayContains("b", 7), equalTo("a", 1))));
1612-
checkOnlineAndOfflineResultsMatch(query4, "doc3");
1629+
checkOnlineAndOfflineResultsMatch(collection, query4, "doc3");
16131630
}
16141631

16151632
@Test
@@ -1625,9 +1642,9 @@ public void testOrderByEquality() {
16251642
CollectionReference collection = testCollectionWithDocs(testDocs);
16261643

16271644
Query query1 = collection.where(equalTo("a", 1)).orderBy("a");
1628-
checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc4", "doc5");
1645+
checkOnlineAndOfflineResultsMatch(collection, query1, "doc1", "doc4", "doc5");
16291646

16301647
Query query2 = collection.where(inArray("a", asList(2, 3))).orderBy("a");
1631-
checkOnlineAndOfflineResultsMatch(query2, "doc6", "doc3");
1648+
checkOnlineAndOfflineResultsMatch(collection, query2, "doc6", "doc3");
16321649
}
16331650
}

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/VectorTest.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,8 @@ public void vectorFieldOrderOnlineAndOffline() throws Exception {
323323
Query orderedQuery = randomColl.orderBy("embedding");
324324

325325
// Run query with snapshot listener
326-
checkOnlineAndOfflineResultsMatch(orderedQuery, docIds.stream().toArray(String[]::new));
326+
checkOnlineAndOfflineResultsMatch(
327+
randomColl, orderedQuery, docIds.stream().toArray(String[]::new));
327328
}
328329

329330
/** Verifies that the SDK filters vector fields the same way for online and offline queries*/
@@ -363,13 +364,15 @@ public void vectorFieldFilterOnlineAndOffline() throws Exception {
363364
.orderBy("embedding")
364365
.whereLessThan("embedding", FieldValue.vector(new double[] {1, 2, 100, 4, 4}));
365366
checkOnlineAndOfflineResultsMatch(
366-
orderedQueryLessThan, docIds.subList(2, 11).stream().toArray(String[]::new));
367+
randomColl, orderedQueryLessThan, docIds.subList(2, 11).stream().toArray(String[]::new));
367368

368369
Query orderedQueryGreaterThan =
369370
randomColl
370371
.orderBy("embedding")
371372
.whereGreaterThan("embedding", FieldValue.vector(new double[] {1, 2, 100, 4, 4}));
372373
checkOnlineAndOfflineResultsMatch(
373-
orderedQueryGreaterThan, docIds.subList(12, 13).stream().toArray(String[]::new));
374+
randomColl,
375+
orderedQueryGreaterThan,
376+
docIds.subList(12, 13).stream().toArray(String[]::new));
374377
}
375378
}

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/CompositeIndexTestHelper.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,10 @@ private Map<String, Map<String, Object>> prepareTestDocuments(
122122
// actual document IDs created by the test helper.
123123
@NonNull
124124
public void assertOnlineAndOfflineResultsMatch(
125-
@NonNull Query query, @NonNull String... expectedDocs) {
126-
checkOnlineAndOfflineResultsMatch(query, toHashedIds(expectedDocs));
125+
@NonNull CollectionReference collection,
126+
@NonNull Query query,
127+
@NonNull String... expectedDocs) {
128+
checkOnlineAndOfflineResultsMatch(collection, query, toHashedIds(expectedDocs));
127129
}
128130

129131
// Asserts that the IDs in the query snapshot matches the expected Ids. The expected document

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -524,17 +524,41 @@ public static List<Object> nullList() {
524524
* documents as running the query while offline. If `expectedDocs` is provided, it also checks
525525
* that both online and offline query result is equal to the expected documents.
526526
*
527+
* This function first performs a "get" for the entire COLLECTION from the server.
528+
* It then performs the QUERY from CACHE which, results in `executeFullCollectionScan()`
529+
* It then performs the QUERY from SERVER.
530+
* It then performs the QUERY from CACHE again, which results in `performQueryUsingRemoteKeys()`.
531+
* It then ensure that all the above QUERY results are the same.
532+
*
533+
* @param collection The collection on which the query is performed.
527534
* @param query The query to check
528535
* @param expectedDocs Ordered list of document keys that are expected to match the query
529536
*/
530-
public static void checkOnlineAndOfflineResultsMatch(Query query, String... expectedDocs) {
537+
public static void checkOnlineAndOfflineResultsMatch(
538+
CollectionReference collection, Query query, String... expectedDocs) {
539+
// Note: Order matters. The following has to be done in the specific order:
540+
541+
// 1- Pre-populate the cache with the entire collection.
542+
waitFor(collection.get(Source.SERVER));
543+
544+
// 2- This performs the query against the cache using full collection scan.
545+
QuerySnapshot docsFromCacheFullCollectionScan = waitFor(query.get(Source.CACHE));
546+
547+
// 3- This goes to the server (backend/emulator).
531548
QuerySnapshot docsFromServer = waitFor(query.get(Source.SERVER));
532-
QuerySnapshot docsFromCache = waitFor(query.get(Source.CACHE));
533549

534-
assertEquals(querySnapshotToIds(docsFromServer), querySnapshotToIds(docsFromCache));
535-
List<String> expected = asList(expectedDocs);
536-
if (!expected.isEmpty()) {
537-
assertEquals(expected, querySnapshotToIds(docsFromCache));
550+
// 4- This performs the query against the cache using remote keys.
551+
QuerySnapshot docsFromCacheUsingRemoteKeys = waitFor(query.get(Source.CACHE));
552+
553+
assertEquals(
554+
querySnapshotToIds(docsFromServer), querySnapshotToIds(docsFromCacheFullCollectionScan));
555+
assertEquals(
556+
querySnapshotToIds(docsFromServer), querySnapshotToIds(docsFromCacheUsingRemoteKeys));
557+
558+
// Expected document IDs.
559+
List<String> expectedDocIds = asList(expectedDocs);
560+
if (!expectedDocIds.isEmpty()) {
561+
assertEquals(expectedDocIds, querySnapshotToIds(docsFromServer));
538562
}
539563
}
540564
}

0 commit comments

Comments
 (0)