Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Increase test coverage for persistent cache indexing #5213

Merged
merged 4 commits into from
Aug 17, 2023
Merged

Increase test coverage for persistent cache indexing #5213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6112ff8
Improve readability
cherylEnkidu Jul 28, 2023
cfb1bec
optimize deleteAllIndexes tests
cherylEnkidu Aug 12, 2023
768f9b3
backport tests from web
cherylEnkidu Aug 15, 2023
b6c3465
Address Feedback2
cherylEnkidu Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1419,4 +1419,77 @@ public void testCannotGetDocumentWithMemoryEagerGcEnabled() {
assertTrue(e instanceof FirebaseFirestoreException);
assertEquals(((FirebaseFirestoreException) e).getCode(), Code.UNAVAILABLE);
}

@Test
public void testGetPersistentCacheIndexManager() {
// Use persistent disk cache (explicit)
FirebaseFirestore db1 = testFirestore();
FirebaseFirestoreSettings settings1 =
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
.build();
db1.setFirestoreSettings(settings1);
assertNotNull(db1.getPersistentCacheIndexManager());

// Use persistent disk cache (default)
FirebaseFirestore db2 = testFirestore();
assertNotNull(db2.getPersistentCacheIndexManager());

// Disable persistent disk cache
FirebaseFirestore db3 = testFirestore();
FirebaseFirestoreSettings settings3 =
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
.setLocalCacheSettings(MemoryCacheSettings.newBuilder().build())
.build();
db3.setFirestoreSettings(settings3);
assertNull(db3.getPersistentCacheIndexManager());

// Disable persistent disk cache (deprecated)
FirebaseFirestore db4 = testFirestore();
FirebaseFirestoreSettings settings4 =
new FirebaseFirestoreSettings.Builder(db4.getFirestoreSettings())
.setPersistenceEnabled(false)
.build();
db4.setFirestoreSettings(settings4);
assertNull(db4.getPersistentCacheIndexManager());
}

@Test
public void testCanGetSameOrDifferentPersistentCacheIndexManager() {
// Use persistent disk cache (explicit)
FirebaseFirestore db1 = testFirestore();
FirebaseFirestoreSettings settings1 =
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
.build();
db1.setFirestoreSettings(settings1);
PersistentCacheIndexManager indexManager1 = db1.getPersistentCacheIndexManager();
PersistentCacheIndexManager indexManager2 = db1.getPersistentCacheIndexManager();
assertSame(indexManager1, indexManager2);

// Use persistent disk cache (default)
FirebaseFirestore db2 = testFirestore();
PersistentCacheIndexManager indexManager3 = db2.getPersistentCacheIndexManager();
PersistentCacheIndexManager indexManager4 = db2.getPersistentCacheIndexManager();
assertSame(indexManager3, indexManager4);

assertNotSame(indexManager1, indexManager3);

FirebaseFirestore db3 = testFirestore();
FirebaseFirestoreSettings settings3 =
new FirebaseFirestoreSettings.Builder(db3.getFirestoreSettings())
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
.build();
db3.setFirestoreSettings(settings3);
PersistentCacheIndexManager indexManager5 = db3.getPersistentCacheIndexManager();
assertNotSame(indexManager1, indexManager5);
assertNotSame(indexManager3, indexManager5);

// Use persistent disk cache (default)
FirebaseFirestore db4 = testFirestore();
PersistentCacheIndexManager indexManager6 = db4.getPersistentCacheIndexManager();
assertNotSame(indexManager1, indexManager6);
assertNotSame(indexManager3, indexManager6);
assertNotSame(indexManager5, indexManager6);
}
}
26 changes: 17 additions & 9 deletions firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,13 @@ public void testBadIndexDoesNotCrashClient() {
+ "}"));
}

/**
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
* After Auto Index Creation is enabled, through public API there is no way to state of indexes
* sitting inside SDK. So this test only checks the API of auto index creation.
*/
@Test
public void testAutoIndexCreationSetSuccessfully() {
// Use persistent disk cache (default)
// Use persistent disk cache (explicit)
FirebaseFirestore db = testFirestore();
FirebaseFirestoreSettings settings =
new FirebaseFirestoreSettings.Builder(db.getFirestoreSettings())
Expand All @@ -128,19 +132,22 @@ public void testAutoIndexCreationSetSuccessfully() {
assertEquals(1, results.size());

assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation());

results = waitFor(collection.whereEqualTo("match", true).get());
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
assertEquals(1, results.size());

assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation());

results = waitFor(collection.whereEqualTo("match", true).get());
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
assertEquals(1, results.size());

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

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

assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation());

results = waitFor(collection.whereEqualTo("match", true).get());
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
assertEquals(1, results.size());

assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation());

results = waitFor(collection.whereEqualTo("match", true).get());
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
assertEquals(1, results.size());

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

Expand All @@ -186,4 +192,6 @@ public void testAutoIndexCreationAfterFailsTermination() {
() -> db.getPersistentCacheIndexManager().deleteAllIndexes(),
"The client has already been terminated");
}

// TODO(b/296100693) Add testing hooks to verify indexes are created as expected.
}
1 change: 0 additions & 1 deletion firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
* Decides whether SDK should create a full matched field index for this query based on query
* context and query result size.
*/
// TODO(csi): Auto experiment data.
private void createCacheIndexes(Query query, QueryContext context, int resultSize) {
if (context.getDocumentReadCount() < indexAutoCreationMinCollectionSize) {
Logger.debug(
Expand Down
3 changes: 3 additions & 0 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ public FieldIndex buildTargetIndex() {
}
}

// Note: We do not explicitly check `inequalityFilter` but rather rely on the target defining an
// appropriate `orderBys` to ensure that the required index segment is added. The query engine
// would reject a query with an inequality filter that lacks the required order-by clause.
for (OrderBy orderBy : orderBys) {
// Stop adding more segments if we see a order-by on key. Typically this is the default
// implicit order-by which is covered in the index_entry table as a separate column.
Expand Down
44 changes: 44 additions & 0 deletions ...e-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.firebase.firestore.model.FieldIndex.IndexState;
import static com.google.firebase.firestore.model.FieldIndex.Segment.Kind;
import static com.google.firebase.firestore.testutil.TestUtil.andFilters;
import static com.google.firebase.firestore.testutil.TestUtil.bound;
import static com.google.firebase.firestore.testutil.TestUtil.deletedDoc;
import static com.google.firebase.firestore.testutil.TestUtil.doc;
Expand Down Expand Up @@ -1189,6 +1190,49 @@ public void testIndexTypeForOrQueries() throws Exception {
validateIndexType(query11, IndexManager.IndexType.PARTIAL);
}

@Test
public void TestCreateTargetIndexesCreatesFullIndexesForEachSubTarget() {
Query query =
query("coll")
.filter(orFilters(filter("a", "==", 1), filter("b", "==", 2), filter("c", "==", 3)));

Query subQuery1 = query("coll").filter(filter("a", "==", 1));
Query subQuery2 = query("coll").filter(filter("b", "==", 2));
Query subQuery3 = query("coll").filter(filter("c", "==", 3));

validateIndexType(query, IndexManager.IndexType.NONE);
validateIndexType(subQuery1, IndexManager.IndexType.NONE);
validateIndexType(subQuery2, IndexManager.IndexType.NONE);
validateIndexType(subQuery3, IndexManager.IndexType.NONE);

indexManager.createTargetIndexes(query.toTarget());

validateIndexType(query, IndexManager.IndexType.FULL);
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
validateIndexType(subQuery2, IndexManager.IndexType.FULL);
validateIndexType(subQuery3, IndexManager.IndexType.FULL);
}

@Test
public void TestCreateTargetIndexesUpgradesPartialIndexToFullIndex() {
Query query = query("coll").filter(andFilters(filter("a", "==", 1), filter("b", "==", 2)));

Query subQuery1 = query("coll").filter(filter("a", "==", 1));
Query subQuery2 = query("coll").filter(filter("b", "==", 2));

indexManager.createTargetIndexes(subQuery1.toTarget());

validateIndexType(query, IndexManager.IndexType.PARTIAL);
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
validateIndexType(subQuery2, IndexManager.IndexType.NONE);

indexManager.createTargetIndexes(query.toTarget());

validateIndexType(query, IndexManager.IndexType.FULL);
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
validateIndexType(subQuery2, IndexManager.IndexType.NONE);
}

private void validateIndexType(Query query, IndexManager.IndexType expected) {
IndexManager.IndexType indexType = indexManager.getIndexType(query.toTarget());
assertEquals(indexType, expected);
Expand Down
76 changes: 60 additions & 16 deletions ...ase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static com.google.firebase.firestore.testutil.TestUtil.key;
import static com.google.firebase.firestore.testutil.TestUtil.keyMap;
import static com.google.firebase.firestore.testutil.TestUtil.map;
import static com.google.firebase.firestore.testutil.TestUtil.orFilters;
import static com.google.firebase.firestore.testutil.TestUtil.orderBy;
import static com.google.firebase.firestore.testutil.TestUtil.query;
import static com.google.firebase.firestore.testutil.TestUtil.setMutation;
Expand Down Expand Up @@ -398,19 +399,50 @@ public void testCanAutoCreateIndexes() {
assertQueryReturned("coll/a", "coll/e", "coll/f");
}

@Test
public void testCanAutoCreateIndexesWorksWithOrQuery() {
Query query = query("coll").filter(orFilters(filter("a", "==", 3), filter("b", "==", true)));
int targetId = allocateQuery(query);

setIndexAutoCreationEnabled(true);
setMinCollectionSizeToAutoCreateIndex(0);
setRelativeIndexReadCostPerDocument(2);

applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("b", true)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("b", false)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("a", 5, "b", false)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("a", true)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("a", 3, "b", true)), targetId));

// First time query runs without indexes.
// Based on current heuristic, collection document counts (5) > 2 * resultSize (2).
// Full matched index should be created.
executeQuery(query);
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
assertQueryReturned("coll/e", "coll/a");

backfillIndexes();

applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("a", 3, "b", false)), targetId));

executeQuery(query);
assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 1);
assertQueryReturned("coll/f", "coll/e", "coll/a");
}

@Test
public void testDoesNotAutoCreateIndexesForSmallCollections() {
Query query = query("coll").filter(filter("count", ">=", 3));
Query query = query("coll").filter(filter("foo", "==", 9)).filter(filter("count", ">=", 3));
cherylEnkidu marked this conversation as resolved.
Show resolved Hide resolved
int targetId = allocateQuery(query);

setIndexAutoCreationEnabled(true);
setRelativeIndexReadCostPerDocument(2);

applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("count", 5)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("count", 1)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("count", 0)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("count", 1)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("count", 3)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("foo", 9, "count", 5)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("foo", 8, "count", 6)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("foo", 9, "count", 0)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("count", 4)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("foo", 9, "count", 3)), targetId));

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

backfillIndexes();

applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("count", 4)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("foo", 9, "count", 4)), targetId));

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

@Test
public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
Query query = query("coll").filter(filter("matches", "==", "foo"));
Query query =
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
query("coll").filter(filter("matches", "==", "foo")).filter(filter("count", ">", 10));
int targetId = allocateQuery(query);

setIndexAutoCreationEnabled(true);
setMinCollectionSizeToAutoCreateIndex(0);
setRelativeIndexReadCostPerDocument(2);

applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("matches", "foo")), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("matches", "")), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "bar")), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("matches", "foo")), targetId));
applyRemoteEvent(
addedRemoteEvent(doc("coll/a", 10, map("matches", "foo", "count", 11)), targetId));
applyRemoteEvent(
addedRemoteEvent(doc("coll/b", 10, map("matches", "foo", "count", 9)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "foo")), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7, "count", 11)), targetId));
applyRemoteEvent(
addedRemoteEvent(doc("coll/e", 10, map("matches", "foo", "count", 21)), targetId));

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

applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("matches", "foo")), targetId));
applyRemoteEvent(
addedRemoteEvent(doc("coll/f", 20, map("matches", "foo", "count", 15)), targetId));

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

executeQuery(query2);
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
assertQueryReturned("foo/a", "foo/e");

backfillIndexes();

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

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

setIndexAutoCreationEnabled(false);

backfillIndexes();

executeQuery(query);
Expand All @@ -607,6 +644,13 @@ public void testDeleteAllIndexesWorksWithIndexAutoCreation() {
executeQuery(query);
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
assertQueryReturned("coll/a", "coll/e");

// Field index is created again.
backfillIndexes();

executeQuery(query);
assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 0);
assertQueryReturned("coll/a", "coll/e");
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ...e-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ public void testBuildTargetIndexWithQueriesWithArrayContains() {
}

@Test
public void testBuildTargetIndexWithQueriesWithOrderBy() {
public void testBuildTargetIndexWithQueriesWithOrderBys() {
for (Query query : queriesWithOrderBys) {
validateBuildTargetIndexCreateFullMatchIndex(query);
}
Expand Down
Loading