Skip to content

Commit 5647871

Browse files
cherylEnkiducherylEnkidu
authored
Merge b6c3465 into f2a0fa3 
2 parents f2a0fa3 + b6c3465 commit 5647871

File tree

7 files changed

+198
-27
lines changed

7 files changed

+198
-27
lines changed

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,4 +1419,77 @@ public void testCannotGetDocumentWithMemoryEagerGcEnabled() {
14191419
assertTrue(e instanceof FirebaseFirestoreException);
14201420
assertEquals(((FirebaseFirestoreException) e).getCode(), Code.UNAVAILABLE);
14211421
}
1422+
1423+
@Test
1424+
public void testGetPersistentCacheIndexManager() {
1425+
// Use persistent disk cache (explicit)
1426+
FirebaseFirestore db1 = testFirestore();
1427+
FirebaseFirestoreSettings settings1 =
1428+
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
1429+
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
1430+
.build();
1431+
db1.setFirestoreSettings(settings1);
1432+
assertNotNull(db1.getPersistentCacheIndexManager());
1433+
1434+
// Use persistent disk cache (default)
1435+
FirebaseFirestore db2 = testFirestore();
1436+
assertNotNull(db2.getPersistentCacheIndexManager());
1437+
1438+
// Disable persistent disk cache
1439+
FirebaseFirestore db3 = testFirestore();
1440+
FirebaseFirestoreSettings settings3 =
1441+
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
1442+
.setLocalCacheSettings(MemoryCacheSettings.newBuilder().build())
1443+
.build();
1444+
db3.setFirestoreSettings(settings3);
1445+
assertNull(db3.getPersistentCacheIndexManager());
1446+
1447+
// Disable persistent disk cache (deprecated)
1448+
FirebaseFirestore db4 = testFirestore();
1449+
FirebaseFirestoreSettings settings4 =
1450+
new FirebaseFirestoreSettings.Builder(db4.getFirestoreSettings())
1451+
.setPersistenceEnabled(false)
1452+
.build();
1453+
db4.setFirestoreSettings(settings4);
1454+
assertNull(db4.getPersistentCacheIndexManager());
1455+
}
1456+
1457+
@Test
1458+
public void testCanGetSameOrDifferentPersistentCacheIndexManager() {
1459+
// Use persistent disk cache (explicit)
1460+
FirebaseFirestore db1 = testFirestore();
1461+
FirebaseFirestoreSettings settings1 =
1462+
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
1463+
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
1464+
.build();
1465+
db1.setFirestoreSettings(settings1);
1466+
PersistentCacheIndexManager indexManager1 = db1.getPersistentCacheIndexManager();
1467+
PersistentCacheIndexManager indexManager2 = db1.getPersistentCacheIndexManager();
1468+
assertSame(indexManager1, indexManager2);
1469+
1470+
// Use persistent disk cache (default)
1471+
FirebaseFirestore db2 = testFirestore();
1472+
PersistentCacheIndexManager indexManager3 = db2.getPersistentCacheIndexManager();
1473+
PersistentCacheIndexManager indexManager4 = db2.getPersistentCacheIndexManager();
1474+
assertSame(indexManager3, indexManager4);
1475+
1476+
assertNotSame(indexManager1, indexManager3);
1477+
1478+
FirebaseFirestore db3 = testFirestore();
1479+
FirebaseFirestoreSettings settings3 =
1480+
new FirebaseFirestoreSettings.Builder(db3.getFirestoreSettings())
1481+
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
1482+
.build();
1483+
db3.setFirestoreSettings(settings3);
1484+
PersistentCacheIndexManager indexManager5 = db3.getPersistentCacheIndexManager();
1485+
assertNotSame(indexManager1, indexManager5);
1486+
assertNotSame(indexManager3, indexManager5);
1487+
1488+
// Use persistent disk cache (default)
1489+
FirebaseFirestore db4 = testFirestore();
1490+
PersistentCacheIndexManager indexManager6 = db4.getPersistentCacheIndexManager();
1491+
assertNotSame(indexManager1, indexManager6);
1492+
assertNotSame(indexManager3, indexManager6);
1493+
assertNotSame(indexManager5, indexManager6);
1494+
}
14221495
}

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,13 @@ public void testBadIndexDoesNotCrashClient() {
108108
+ "}"));
109109
}
110110

111+
/**
112+
* After Auto Index Creation is enabled, through public API there is no way to state of indexes
113+
* sitting inside SDK. So this test only checks the API of auto index creation.
114+
*/
111115
@Test
112116
public void testAutoIndexCreationSetSuccessfully() {
113-
// Use persistent disk cache (default)
117+
// Use persistent disk cache (explicit)
114118
FirebaseFirestore db = testFirestore();
115119
FirebaseFirestoreSettings settings =
116120
new FirebaseFirestoreSettings.Builder(db.getFirestoreSettings())
@@ -128,19 +132,22 @@ public void testAutoIndexCreationSetSuccessfully() {
128132
assertEquals(1, results.size());
129133

130134
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation());
131-
132-
results = waitFor(collection.whereEqualTo("match", true).get());
135+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
133136
assertEquals(1, results.size());
134137

135138
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation());
136-
137-
results = waitFor(collection.whereEqualTo("match", true).get());
139+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
138140
assertEquals(1, results.size());
139141

140142
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().deleteAllIndexes());
143+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
141144
assertEquals(1, results.size());
142145
}
143146

147+
/**
148+
* After Auto Index Creation is enabled, through public API there is no way to state of indexes
149+
* sitting inside SDK. So this test only checks the API of auto index creation.
150+
*/
144151
@Test
145152
public void testAutoIndexCreationSetSuccessfullyUsingDefault() {
146153
// Use persistent disk cache (default)
@@ -156,16 +163,15 @@ public void testAutoIndexCreationSetSuccessfullyUsingDefault() {
156163
assertEquals(1, results.size());
157164

158165
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation());
159-
160-
results = waitFor(collection.whereEqualTo("match", true).get());
166+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
161167
assertEquals(1, results.size());
162168

163169
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation());
164-
165-
results = waitFor(collection.whereEqualTo("match", true).get());
170+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
166171
assertEquals(1, results.size());
167172

168173
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().deleteAllIndexes());
174+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
169175
assertEquals(1, results.size());
170176
}
171177

@@ -186,4 +192,6 @@ public void testAutoIndexCreationAfterFailsTermination() {
186192
() -> db.getPersistentCacheIndexManager().deleteAllIndexes(),
187193
"The client has already been terminated");
188194
}
195+
196+
// TODO(b/296100693) Add testing hooks to verify indexes are created as expected.
189197
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
120120
* Decides whether SDK should create a full matched field index for this query based on query
121121
* context and query result size.
122122
*/
123-
// TODO(csi): Auto experiment data.
124123
private void createCacheIndexes(Query query, QueryContext context, int resultSize) {
125124
if (context.getDocumentReadCount() < indexAutoCreationMinCollectionSize) {
126125
Logger.debug(

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,9 @@ public FieldIndex buildTargetIndex() {
219219
}
220220
}
221221

222+
// Note: We do not explicitly check `inequalityFilter` but rather rely on the target defining an
223+
// appropriate `orderBys` to ensure that the required index segment is added. The query engine
224+
// would reject a query with an inequality filter that lacks the required order-by clause.
222225
for (OrderBy orderBy : orderBys) {
223226
// Stop adding more segments if we see a order-by on key. Typically this is the default
224227
// implicit order-by which is covered in the index_entry table as a separate column.

‎firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.truth.Truth.assertWithMessage;
1818
import static com.google.firebase.firestore.model.FieldIndex.IndexState;
1919
import static com.google.firebase.firestore.model.FieldIndex.Segment.Kind;
20+
import static com.google.firebase.firestore.testutil.TestUtil.andFilters;
2021
import static com.google.firebase.firestore.testutil.TestUtil.bound;
2122
import static com.google.firebase.firestore.testutil.TestUtil.deletedDoc;
2223
import static com.google.firebase.firestore.testutil.TestUtil.doc;
@@ -1189,6 +1190,49 @@ public void testIndexTypeForOrQueries() throws Exception {
11891190
validateIndexType(query11, IndexManager.IndexType.PARTIAL);
11901191
}
11911192

1193+
@Test
1194+
public void TestCreateTargetIndexesCreatesFullIndexesForEachSubTarget() {
1195+
Query query =
1196+
query("coll")
1197+
.filter(orFilters(filter("a", "==", 1), filter("b", "==", 2), filter("c", "==", 3)));
1198+
1199+
Query subQuery1 = query("coll").filter(filter("a", "==", 1));
1200+
Query subQuery2 = query("coll").filter(filter("b", "==", 2));
1201+
Query subQuery3 = query("coll").filter(filter("c", "==", 3));
1202+
1203+
validateIndexType(query, IndexManager.IndexType.NONE);
1204+
validateIndexType(subQuery1, IndexManager.IndexType.NONE);
1205+
validateIndexType(subQuery2, IndexManager.IndexType.NONE);
1206+
validateIndexType(subQuery3, IndexManager.IndexType.NONE);
1207+
1208+
indexManager.createTargetIndexes(query.toTarget());
1209+
1210+
validateIndexType(query, IndexManager.IndexType.FULL);
1211+
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
1212+
validateIndexType(subQuery2, IndexManager.IndexType.FULL);
1213+
validateIndexType(subQuery3, IndexManager.IndexType.FULL);
1214+
}
1215+
1216+
@Test
1217+
public void TestCreateTargetIndexesUpgradesPartialIndexToFullIndex() {
1218+
Query query = query("coll").filter(andFilters(filter("a", "==", 1), filter("b", "==", 2)));
1219+
1220+
Query subQuery1 = query("coll").filter(filter("a", "==", 1));
1221+
Query subQuery2 = query("coll").filter(filter("b", "==", 2));
1222+
1223+
indexManager.createTargetIndexes(subQuery1.toTarget());
1224+
1225+
validateIndexType(query, IndexManager.IndexType.PARTIAL);
1226+
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
1227+
validateIndexType(subQuery2, IndexManager.IndexType.NONE);
1228+
1229+
indexManager.createTargetIndexes(query.toTarget());
1230+
1231+
validateIndexType(query, IndexManager.IndexType.FULL);
1232+
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
1233+
validateIndexType(subQuery2, IndexManager.IndexType.NONE);
1234+
}
1235+
11921236
private void validateIndexType(Query query, IndexManager.IndexType expected) {
11931237
IndexManager.IndexType indexType = indexManager.getIndexType(query.toTarget());
11941238
assertEquals(indexType, expected);

‎firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java

Lines changed: 60 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static com.google.firebase.firestore.testutil.TestUtil.key;
2525
import static com.google.firebase.firestore.testutil.TestUtil.keyMap;
2626
import static com.google.firebase.firestore.testutil.TestUtil.map;
27+
import static com.google.firebase.firestore.testutil.TestUtil.orFilters;
2728
import static com.google.firebase.firestore.testutil.TestUtil.orderBy;
2829
import static com.google.firebase.firestore.testutil.TestUtil.query;
2930
import static com.google.firebase.firestore.testutil.TestUtil.setMutation;
@@ -398,19 +399,50 @@ public void testCanAutoCreateIndexes() {
398399
assertQueryReturned("coll/a", "coll/e", "coll/f");
399400
}
400401

402+
@Test
403+
public void testCanAutoCreateIndexesWorksWithOrQuery() {
404+
Query query = query("coll").filter(orFilters(filter("a", "==", 3), filter("b", "==", true)));
405+
int targetId = allocateQuery(query);
406+
407+
setIndexAutoCreationEnabled(true);
408+
setMinCollectionSizeToAutoCreateIndex(0);
409+
setRelativeIndexReadCostPerDocument(2);
410+
411+
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("b", true)), targetId));
412+
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("b", false)), targetId));
413+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("a", 5, "b", false)), targetId));
414+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("a", true)), targetId));
415+
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("a", 3, "b", true)), targetId));
416+
417+
// First time query runs without indexes.
418+
// Based on current heuristic, collection document counts (5) > 2 * resultSize (2).
419+
// Full matched index should be created.
420+
executeQuery(query);
421+
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
422+
assertQueryReturned("coll/e", "coll/a");
423+
424+
backfillIndexes();
425+
426+
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("a", 3, "b", false)), targetId));
427+
428+
executeQuery(query);
429+
assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 1);
430+
assertQueryReturned("coll/f", "coll/e", "coll/a");
431+
}
432+
401433
@Test
402434
public void testDoesNotAutoCreateIndexesForSmallCollections() {
403-
Query query = query("coll").filter(filter("count", ">=", 3));
435+
Query query = query("coll").filter(filter("foo", "==", 9)).filter(filter("count", ">=", 3));
404436
int targetId = allocateQuery(query);
405437

406438
setIndexAutoCreationEnabled(true);
407439
setRelativeIndexReadCostPerDocument(2);
408440

409-
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("count", 5)), targetId));
410-
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("count", 1)), targetId));
411-
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("count", 0)), targetId));
412-
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("count", 1)), targetId));
413-
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("count", 3)), targetId));
441+
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("foo", 9, "count", 5)), targetId));
442+
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("foo", 8, "count", 6)), targetId));
443+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("foo", 9, "count", 0)), targetId));
444+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("count", 4)), targetId));
445+
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("foo", 9, "count", 3)), targetId));
414446

415447
// SDK will not create indexes since collection size is too small.
416448
executeQuery(query);
@@ -419,7 +451,7 @@ public void testDoesNotAutoCreateIndexesForSmallCollections() {
419451

420452
backfillIndexes();
421453

422-
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("count", 4)), targetId));
454+
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("foo", 9, "count", 4)), targetId));
423455

424456
executeQuery(query);
425457
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 3);
@@ -460,18 +492,22 @@ public void testDoesNotAutoCreateIndexesWhenIndexLookUpIsExpensive() {
460492

461493
@Test
462494
public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
463-
Query query = query("coll").filter(filter("matches", "==", "foo"));
495+
Query query =
496+
query("coll").filter(filter("matches", "==", "foo")).filter(filter("count", ">", 10));
464497
int targetId = allocateQuery(query);
465498

466499
setIndexAutoCreationEnabled(true);
467500
setMinCollectionSizeToAutoCreateIndex(0);
468501
setRelativeIndexReadCostPerDocument(2);
469502

470-
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("matches", "foo")), targetId));
471-
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("matches", "")), targetId));
472-
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "bar")), targetId));
473-
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7)), targetId));
474-
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("matches", "foo")), targetId));
503+
applyRemoteEvent(
504+
addedRemoteEvent(doc("coll/a", 10, map("matches", "foo", "count", 11)), targetId));
505+
applyRemoteEvent(
506+
addedRemoteEvent(doc("coll/b", 10, map("matches", "foo", "count", 9)), targetId));
507+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "foo")), targetId));
508+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7, "count", 11)), targetId));
509+
applyRemoteEvent(
510+
addedRemoteEvent(doc("coll/e", 10, map("matches", "foo", "count", 21)), targetId));
475511

476512
// First time query is running without indexes.
477513
// Based on current heuristic, collection document counts (5) > 2 * resultSize (2).
@@ -483,7 +519,8 @@ public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
483519
setBackfillerMaxDocumentsToProcess(2);
484520
backfillIndexes();
485521

486-
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("matches", "foo")), targetId));
522+
applyRemoteEvent(
523+
addedRemoteEvent(doc("coll/f", 20, map("matches", "foo", "count", 15)), targetId));
487524

488525
executeQuery(query);
489526
assertRemoteDocumentsRead(/* byKey= */ 1, /* byCollection= */ 2);
@@ -564,12 +601,14 @@ public void testDisableIndexAutoCreationWorks() {
564601

565602
executeQuery(query2);
566603
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
604+
assertQueryReturned("foo/a", "foo/e");
567605

568606
backfillIndexes();
569607

570608
// Run the query in second time, test index won't be created
571609
executeQuery(query2);
572610
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
611+
assertQueryReturned("foo/a", "foo/e");
573612
}
574613

575614
@Test
@@ -594,8 +633,6 @@ public void testDeleteAllIndexesWorksWithIndexAutoCreation() {
594633
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
595634
assertQueryReturned("coll/a", "coll/e");
596635

597-
setIndexAutoCreationEnabled(false);
598-
599636
backfillIndexes();
600637

601638
executeQuery(query);
@@ -607,6 +644,13 @@ public void testDeleteAllIndexesWorksWithIndexAutoCreation() {
607644
executeQuery(query);
608645
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
609646
assertQueryReturned("coll/a", "coll/e");
647+
648+
// Field index is created again.
649+
backfillIndexes();
650+
651+
executeQuery(query);
652+
assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 0);
653+
assertQueryReturned("coll/a", "coll/e");
610654
}
611655

612656
@Test

‎firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ public void testBuildTargetIndexWithQueriesWithArrayContains() {
664664
}
665665

666666
@Test
667-
public void testBuildTargetIndexWithQueriesWithOrderBy() {
667+
public void testBuildTargetIndexWithQueriesWithOrderBys() {
668668
for (Query query : queriesWithOrderBys) {
669669
validateBuildTargetIndexCreateFullMatchIndex(query);
670670
}

0 commit comments

Comments
 (0)