diff --git a/firebase-database-collection/src/main/java/com/google/firebase/database/collection/ImmutableSortedMap.java b/firebase-database-collection/src/main/java/com/google/firebase/database/collection/ImmutableSortedMap.java index f62c973aa1d..d7a648b9684 100644 --- a/firebase-database-collection/src/main/java/com/google/firebase/database/collection/ImmutableSortedMap.java +++ b/firebase-database-collection/src/main/java/com/google/firebase/database/collection/ImmutableSortedMap.java @@ -56,6 +56,14 @@ public abstract class ImmutableSortedMap implements Iterable getComparator(); + public ImmutableSortedMap insertAll(ImmutableSortedMap source) { + ImmutableSortedMap result = this; + for (Map.Entry entry : source) { + result = result.insert(entry.getKey(), entry.getValue()); + } + return result; + } + @Override @SuppressWarnings("unchecked") public boolean equals(Object o) { diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/RemoteStoreTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/RemoteStoreTest.java index eb703a1af99..da03bc2253d 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/RemoteStoreTest.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/RemoteStoreTest.java @@ -24,6 +24,7 @@ import com.google.firebase.firestore.local.LocalStore; import com.google.firebase.firestore.local.MemoryPersistence; import com.google.firebase.firestore.local.Persistence; +import com.google.firebase.firestore.local.SimpleQueryEngine; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.mutation.MutationBatchResult; import com.google.firebase.firestore.testutil.IntegrationTestUtil; @@ -72,9 +73,10 @@ public ImmutableSortedSet getRemoteKeysForTarget(int targetId) { }; FakeConnectivityMonitor connectivityMonitor = new FakeConnectivityMonitor(); + SimpleQueryEngine queryEngine = new SimpleQueryEngine(); Persistence persistence = MemoryPersistence.createEagerGcMemoryPersistence(); persistence.start(); - LocalStore localStore = new LocalStore(persistence, User.UNAUTHENTICATED); + LocalStore localStore = new LocalStore(persistence, queryEngine, User.UNAUTHENTICATED); RemoteStore remoteStore = new RemoteStore(callback, localStore, datastore, testQueue, connectivityMonitor); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java index fb8b23c7948..84aa3aa76b1 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java @@ -37,7 +37,9 @@ import com.google.firebase.firestore.local.LruGarbageCollector; import com.google.firebase.firestore.local.MemoryPersistence; import com.google.firebase.firestore.local.Persistence; +import com.google.firebase.firestore.local.QueryEngine; import com.google.firebase.firestore.local.SQLitePersistence; +import com.google.firebase.firestore.local.SimpleQueryEngine; import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MaybeDocument; @@ -263,7 +265,9 @@ private void initialize(Context context, User user, boolean usePersistence, long } persistence.start(); - localStore = new LocalStore(persistence, user); + // TODO(index-free): Use IndexFreeQueryEngine/IndexedQueryEngine as appropriate. + QueryEngine queryEngine = new SimpleQueryEngine(); + localStore = new LocalStore(persistence, queryEngine, user); if (gc != null) { lruScheduler = gc.newScheduler(asyncQueue, localStore); lruScheduler.start(); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Query.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Query.java index 107bab94089..b4206485503 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Query.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Query.java @@ -117,6 +117,18 @@ public boolean isCollectionGroupQuery() { return collectionGroup != null; } + /** + * Returns true if this query does not specify any query constraints that could remove results. + */ + public boolean matchesAllDocuments() { + return filters.isEmpty() + && limit == NO_LIMIT + && startAt == null + && endAt == null + && (getExplicitOrderBy().isEmpty() + || (getExplicitOrderBy().size() == 1 && getFirstOrderByField().isKeyField())); + } + /** The filters on the documents returned by the query. */ public List getFilters() { return filters; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java index e03694637e4..6fc55434886 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java @@ -193,9 +193,10 @@ public int listen(Query query) { private ViewSnapshot initializeViewAndComputeSnapshot(QueryData queryData) { Query query = queryData.getQuery(); - ImmutableSortedMap docs = localStore.executeQuery(query); ImmutableSortedSet remoteKeys = localStore.getRemoteDocumentKeys(queryData.getTargetId()); + ImmutableSortedMap docs = + localStore.executeQuery(query, queryData, remoteKeys); View view = new View(query, remoteKeys); View.DocumentChanges viewDocChanges = view.computeDocChanges(docs); @@ -557,7 +558,8 @@ private void emitNewSnapsAndNotifyLocalStore( // against the local store to make sure we didn't lose any good docs that had been past the // limit. ImmutableSortedMap docs = - localStore.executeQuery(queryView.getQuery()); + localStore.executeQuery( + queryView.getQuery(), /* queryData= */ null, DocumentKey.emptyKeySet()); viewDocChanges = view.computeDocChanges(docs, viewDocChanges); } TargetChange targetChange = diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexFreeQueryEngine.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexFreeQueryEngine.java new file mode 100644 index 00000000000..a6b10572f35 --- /dev/null +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexFreeQueryEngine.java @@ -0,0 +1,150 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.firestore.local; + +import static com.google.firebase.firestore.util.Assert.hardAssert; + +import androidx.annotation.Nullable; +import com.google.firebase.database.collection.ImmutableSortedMap; +import com.google.firebase.database.collection.ImmutableSortedSet; +import com.google.firebase.firestore.core.Query; +import com.google.firebase.firestore.model.Document; +import com.google.firebase.firestore.model.DocumentCollections; +import com.google.firebase.firestore.model.DocumentKey; +import com.google.firebase.firestore.model.MaybeDocument; +import com.google.firebase.firestore.model.SnapshotVersion; +import java.util.Map; + +/** + * A query engine that takes advantage of the target document mapping in the QueryCache. The + * IndexFreeQueryEngine optimizes query execution by only reading the documents previously matched a + * query plus any documents that were edited after the query was last listened to. + * + *

There are some cases where Index-Free queries are not guaranteed to produce to the same + * results as full collection scans. In these case, the IndexFreeQueryEngine falls back to a full + * query processing. These cases are: + * + *

    + *
  1. Limit queries where a document that matched the query previously no longer matches the + * query. In this case, we have to scan all local documents since a document that was sent to + * us as part of a different query result may now fall into the limit. + *
  2. Limit queries that include edits that occurred after the last remote snapshot (both + * latency-compensated or committed). Even if an edited document continues to match the query, + * an edit may cause a document to sort below another document that is in the local cache. + *
  3. Queries where the last snapshot contained Limbo documents. Even though a Limbo document is + * not part of the backend result set, we need to include Limbo documents in local views to + * ensure consistency between different Query views. If there exists a previous query snapshot + * that contained no limbo documents, we can instead use the older snapshot version for + * Index-Free processing. + *
+ */ +public class IndexFreeQueryEngine implements QueryEngine { + private LocalDocumentsView localDocumentsView; + + @Override + public void setLocalDocumentsView(LocalDocumentsView localDocuments) { + this.localDocumentsView = localDocuments; + } + + @Override + public ImmutableSortedMap getDocumentsMatchingQuery( + Query query, @Nullable QueryData queryData, ImmutableSortedSet remoteKeys) { + hardAssert(localDocumentsView != null, "setLocalDocumentsView() not called"); + + // Queries that match all document don't benefit from using IndexFreeQueries. It is more + // efficient to scan all documents in a collection, rather than to perform individual lookups. + if (query.matchesAllDocuments()) { + return executeFullCollectionScan(query); + } + + // Queries that have never seen a snapshot without limbo free documents should also be run as a + // full collection scan. + if (queryData == null + || queryData.getLastLimboFreeSnapshotVersion().equals(SnapshotVersion.NONE)) { + return executeFullCollectionScan(query); + } + + ImmutableSortedMap result = + executeIndexFreeQuery(query, queryData, remoteKeys); + + return result != null ? result : executeFullCollectionScan(query); + } + + /** + * Attempts index-free query execution. Returns the set of query results on success, otherwise + * returns null. + */ + private @Nullable ImmutableSortedMap executeIndexFreeQuery( + Query query, QueryData queryData, ImmutableSortedSet remoteKeys) { + // Fetch the documents that matched the query at the last snapshot. + ImmutableSortedMap previousResults = + localDocumentsView.getDocuments(remoteKeys); + + // Limit queries are not eligible for index-free query execution if any part of the result was + // modified after we received the last query snapshot. This makes sure that we re-populate the + // view with older documents that may sort before the modified document. + if (query.hasLimit() + && containsUpdatesSinceSnapshotVersion(previousResults, queryData.getSnapshotVersion())) { + return null; + } + + ImmutableSortedMap results = DocumentCollections.emptyDocumentMap(); + + // Re-apply the query filter since previously matching documents do not necessarily still + // match the query. + for (Map.Entry entry : previousResults) { + MaybeDocument maybeDoc = entry.getValue(); + if (maybeDoc instanceof Document && query.matches((Document) maybeDoc)) { + Document doc = (Document) maybeDoc; + results = results.insert(entry.getKey(), doc); + } else if (query.hasLimit()) { + // Limit queries with documents that no longer match need to be re-filled from cache. + return null; + } + } + + // Retrieve all results for documents that were updated since the last limbo-document free + // remote snapshot. + ImmutableSortedMap updatedResults = + localDocumentsView.getDocumentsMatchingQuery( + query, queryData.getLastLimboFreeSnapshotVersion()); + + results = results.insertAll(updatedResults); + + return results; + } + + @Override + public void handleDocumentChange(MaybeDocument oldDocument, MaybeDocument newDocument) { + // No indexes to update. + } + + private boolean containsUpdatesSinceSnapshotVersion( + ImmutableSortedMap previousResults, + SnapshotVersion sinceSnapshotVersion) { + for (Map.Entry doc : previousResults) { + if (doc.getValue().hasPendingWrites() + || doc.getValue().getVersion().compareTo(sinceSnapshotVersion) > 0) { + return true; + } + } + + return false; + } + + private ImmutableSortedMap executeFullCollectionScan(Query query) { + return localDocumentsView.getDocumentsMatchingQuery(query, SnapshotVersion.NONE); + } +} diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexedQueryEngine.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexedQueryEngine.java index 2cb85b05461..fd54260f486 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexedQueryEngine.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexedQueryEngine.java @@ -19,6 +19,7 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.firebase.database.collection.ImmutableSortedMap; +import com.google.firebase.database.collection.ImmutableSortedSet; import com.google.firebase.firestore.core.FieldFilter; import com.google.firebase.firestore.core.Filter; import com.google.firebase.firestore.core.Filter.Operator; @@ -88,17 +89,23 @@ public class IndexedQueryEngine implements QueryEngine { private static final List lowCardinalityTypes = Arrays.asList(BooleanValue.class, ArrayValue.class, ObjectValue.class); - private final LocalDocumentsView localDocuments; private final SQLiteCollectionIndex collectionIndex; + private LocalDocumentsView localDocuments; - public IndexedQueryEngine( - LocalDocumentsView localDocuments, SQLiteCollectionIndex collectionIndex) { - this.localDocuments = localDocuments; + public IndexedQueryEngine(SQLiteCollectionIndex collectionIndex) { this.collectionIndex = collectionIndex; } @Override - public ImmutableSortedMap getDocumentsMatchingQuery(Query query) { + public void setLocalDocumentsView(LocalDocumentsView localDocuments) { + this.localDocuments = localDocuments; + } + + @Override + public ImmutableSortedMap getDocumentsMatchingQuery( + Query query, @Nullable QueryData queryData, ImmutableSortedSet remoteKeys) { + hardAssert(localDocuments != null, "setLocalDocumentsView() not called"); + return query.isDocumentQuery() ? localDocuments.getDocumentsMatchingQuery(query, SnapshotVersion.NONE) : performCollectionQuery(query); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java index 670b55cd874..99858b8d51b 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java @@ -40,7 +40,7 @@ * mutations in the MutationQueue to the RemoteDocumentCache. */ // TODO: Turn this into the UnifiedDocumentCache / whatever. -final class LocalDocumentsView { +class LocalDocumentsView { private final RemoteDocumentCache remoteDocumentCache; private final MutationQueue mutationQueue; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java index 65c59483a1a..ecb44399a24 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java @@ -19,6 +19,7 @@ import android.util.SparseArray; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import com.google.firebase.Timestamp; import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.database.collection.ImmutableSortedSet; @@ -125,7 +126,7 @@ public final class LocalStore { /** Used to generate targetIds for queries tracked locally. */ private final TargetIdGenerator targetIdGenerator; - public LocalStore(Persistence persistence, User initialUser) { + public LocalStore(Persistence persistence, QueryEngine queryEngine, User initialUser) { hardAssert( persistence.isStarted(), "LocalStore was passed an unstarted persistence implementation"); this.persistence = persistence; @@ -135,8 +136,9 @@ public LocalStore(Persistence persistence, User initialUser) { remoteDocuments = persistence.getRemoteDocumentCache(); localDocuments = new LocalDocumentsView(remoteDocuments, mutationQueue, persistence.getIndexManager()); - // TODO: Use IndexedQueryEngine as appropriate. - queryEngine = new SimpleQueryEngine(localDocuments); + + this.queryEngine = queryEngine; + queryEngine.setLocalDocumentsView(localDocuments); localViewReferences = new ReferenceSet(); persistence.getReferenceDelegate().setInMemoryPins(localViewReferences); @@ -170,8 +172,7 @@ public ImmutableSortedMap handleUserChange(User user // Recreate our LocalDocumentsView using the new MutationQueue. localDocuments = new LocalDocumentsView(remoteDocuments, mutationQueue, persistence.getIndexManager()); - // TODO: Use IndexedQueryEngine as appropriate. - queryEngine = new SimpleQueryEngine(localDocuments); + queryEngine.setLocalDocumentsView(localDocuments); // Union the old/new changed keys. ImmutableSortedSet changedKeys = DocumentKey.emptyKeySet(); @@ -561,6 +562,21 @@ public QueryData allocateQuery(Query query) { return cached; } + /** + * Returns the QueryData as seen by the LocalStore, including updates that may have not yet been + * persisted to the QueryCache. + */ + @VisibleForTesting + @Nullable + QueryData getQueryData(Query query) { + QueryData queryData = queryCache.getQueryData(query); + if (queryData == null) { + return null; + } + QueryData updatedQueryData = targetIds.get(queryData.getTargetId()); + return updatedQueryData != null ? updatedQueryData : queryData; + } + /** Mutable state for the transaction in allocateQuery. */ private static class AllocateQueryHolder { QueryData cached; @@ -611,7 +627,23 @@ public void releaseQuery(Query query) { /** Runs the given query against all the documents in the local store and returns the results. */ public ImmutableSortedMap executeQuery(Query query) { - return queryEngine.getDocumentsMatchingQuery(query); + QueryData queryData = getQueryData(query); + if (queryData != null) { + ImmutableSortedSet remoteKeys = + this.queryCache.getMatchingKeysForTargetId(queryData.getTargetId()); + return executeQuery(query, queryData, remoteKeys); + } else { + return executeQuery(query, null, DocumentKey.emptyKeySet()); + } + } + + /** + * Runs the given query against the local store and returns the results, potentially taking + * advantage of the provided query data and the set of remote document keys. + */ + public ImmutableSortedMap executeQuery( + Query query, @Nullable QueryData queryData, ImmutableSortedSet remoteKeys) { + return queryEngine.getDocumentsMatchingQuery(query, queryData, remoteKeys); } /** diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java index 45e7afc7888..d071316718b 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java @@ -14,17 +14,26 @@ package com.google.firebase.firestore.local; +import androidx.annotation.Nullable; import com.google.firebase.database.collection.ImmutableSortedMap; +import com.google.firebase.database.collection.ImmutableSortedSet; import com.google.firebase.firestore.core.Query; import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MaybeDocument; -/** Represents a query engine capable of performing queries over the local document cache. */ -interface QueryEngine { +/** + * Represents a query engine capable of performing queries over the local document cache. You must + * call setQueryDataProvider() and setLocalDocumentsView() before using. + */ +public interface QueryEngine { + + /** Sets the document view to query against. */ + void setLocalDocumentsView(LocalDocumentsView localDocuments); /** Returns all local documents matching the specified query. */ - ImmutableSortedMap getDocumentsMatchingQuery(Query query); + ImmutableSortedMap getDocumentsMatchingQuery( + Query query, @Nullable QueryData queryData, ImmutableSortedSet remoteKeys); /** * Notifies the query engine of a document change in case it would like to update indexes and the diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SimpleQueryEngine.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SimpleQueryEngine.java index 13c524771aa..998aba3bb3f 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SimpleQueryEngine.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SimpleQueryEngine.java @@ -14,7 +14,11 @@ package com.google.firebase.firestore.local; +import static com.google.firebase.firestore.util.Assert.hardAssert; + +import androidx.annotation.Nullable; import com.google.firebase.database.collection.ImmutableSortedMap; +import com.google.firebase.database.collection.ImmutableSortedSet; import com.google.firebase.firestore.core.Query; import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.DocumentKey; @@ -27,14 +31,18 @@ */ public class SimpleQueryEngine implements QueryEngine { - private final LocalDocumentsView localDocumentsView; + private LocalDocumentsView localDocumentsView; - public SimpleQueryEngine(LocalDocumentsView localDocumentsView) { - this.localDocumentsView = localDocumentsView; + @Override + public void setLocalDocumentsView(LocalDocumentsView localDocuments) { + this.localDocumentsView = localDocuments; } @Override - public ImmutableSortedMap getDocumentsMatchingQuery(Query query) { + public ImmutableSortedMap getDocumentsMatchingQuery( + Query query, @Nullable QueryData queryData, ImmutableSortedSet remoteKeys) { + hardAssert(localDocumentsView != null, "setLocalDocumentsView() not called"); + // TODO: Once LocalDocumentsView provides a getCollectionDocuments() method, we // should call that here and then filter the results. return localDocumentsView.getDocumentsMatchingQuery(query, SnapshotVersion.NONE); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/core/QueryTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/core/QueryTest.java index e72ce1c12cc..1665bf72c20 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/core/QueryTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/core/QueryTest.java @@ -31,6 +31,7 @@ import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.ResourcePath; import com.google.firebase.firestore.testutil.ComparatorTester; +import java.util.Collections; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -528,4 +529,28 @@ public void testImplicitOrderBy() { asList(orderBy("foo", "desc"), orderBy("bar", "asc"), orderBy(KEY_FIELD_NAME, "asc")), baseQuery.orderBy(orderBy("foo", "desc")).orderBy(orderBy("bar", "asc")).getOrderBy()); } + + @Test + public void testMatchesAllDocuments() { + Query baseQuery = Query.atPath(ResourcePath.fromString("collection")); + assertTrue(baseQuery.matchesAllDocuments()); + + Query query = baseQuery.orderBy(orderBy("__name__")); + assertTrue(query.matchesAllDocuments()); + + query = baseQuery.orderBy(orderBy("foo")); + assertFalse(query.matchesAllDocuments()); + + query = baseQuery.filter(filter("foo", "==", "bar")); + assertFalse(query.matchesAllDocuments()); + + query = baseQuery.limit(1); + assertFalse(query.matchesAllDocuments()); + + query = baseQuery.startAt(new Bound(Collections.emptyList(), true)); + assertFalse(query.matchesAllDocuments()); + + query = baseQuery.endAt(new Bound(Collections.emptyList(), true)); + assertFalse(query.matchesAllDocuments()); + } } diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexFreeQueryEngineTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexFreeQueryEngineTest.java new file mode 100644 index 00000000000..29e73f89a26 --- /dev/null +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexFreeQueryEngineTest.java @@ -0,0 +1,290 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.firestore.local; + +import static com.google.firebase.firestore.testutil.TestUtil.doc; +import static com.google.firebase.firestore.testutil.TestUtil.docSet; +import static com.google.firebase.firestore.testutil.TestUtil.filter; +import static com.google.firebase.firestore.testutil.TestUtil.map; +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.version; +import static org.junit.Assert.assertEquals; + +import com.google.android.gms.common.internal.Preconditions; +import com.google.firebase.database.collection.ImmutableSortedMap; +import com.google.firebase.database.collection.ImmutableSortedSet; +import com.google.firebase.firestore.auth.User; +import com.google.firebase.firestore.core.Query; +import com.google.firebase.firestore.core.View; +import com.google.firebase.firestore.model.Document; +import com.google.firebase.firestore.model.DocumentKey; +import com.google.firebase.firestore.model.DocumentSet; +import com.google.firebase.firestore.model.SnapshotVersion; +import com.google.protobuf.ByteString; +import java.util.Collections; +import java.util.concurrent.Callable; +import javax.annotation.Nullable; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class IndexFreeQueryEngineTest { + + private static final int TEST_TARGET_ID = 1; + + private static final Document MATCHING_DOC_A = + doc("coll/a", 1, map("matches", true, "order", 1), Document.DocumentState.SYNCED); + private static final Document NON_MATCHING_DOC_A = + doc("coll/a", 1, map("matches", false, "order", 1), Document.DocumentState.SYNCED); + private static final Document PENDING_MATCHING_DOC_A = + doc("coll/a", 1, map("matches", true, "order", 1), Document.DocumentState.LOCAL_MUTATIONS); + private static final Document PENDING_NON_MATCHING_DOC_A = + doc("coll/a", 1, map("matches", false, "order", 1), Document.DocumentState.LOCAL_MUTATIONS); + private static final Document UDPATED_DOC_A = + doc("coll/a", 11, map("matches", true, "order", 1), Document.DocumentState.SYNCED); + private static final Document MATCHING_DOC_B = + doc("coll/b", 1, map("matches", true, "order", 2), Document.DocumentState.SYNCED); + private static final Document NON_MATCHING_DOC_B = + doc("coll/b", 1, map("matches", false, "order", 2), Document.DocumentState.SYNCED); + private static final Document UPDATED_MATCHING_DOC_B = + doc("coll/b", 11, map("matches", true, "order", 2), Document.DocumentState.SYNCED); + + private MemoryPersistence persistence; + private MemoryRemoteDocumentCache remoteDocumentCache; + private QueryCache queryCache; + private QueryEngine queryEngine; + + private @Nullable Boolean expectIndexFreeExecution; + + @Before + public void setUp() { + expectIndexFreeExecution = null; + + persistence = MemoryPersistence.createEagerGcMemoryPersistence(); + queryCache = new MemoryQueryCache(persistence); + queryEngine = new IndexFreeQueryEngine(); + + remoteDocumentCache = persistence.getRemoteDocumentCache(); + + LocalDocumentsView localDocuments = + new LocalDocumentsView( + remoteDocumentCache, + persistence.getMutationQueue(User.UNAUTHENTICATED), + new MemoryIndexManager()) { + @Override + public ImmutableSortedMap getDocumentsMatchingQuery( + Query query, SnapshotVersion sinceReadTime) { + assertEquals( + "Observed query execution mode did not match expectation", + expectIndexFreeExecution, + !SnapshotVersion.NONE.equals(sinceReadTime)); + return super.getDocumentsMatchingQuery(query, sinceReadTime); + } + }; + queryEngine.setLocalDocumentsView(localDocuments); + } + + /** Adds the provided documents to the query target mapping. */ + private void persistQueryMapping(Document... docs) { + persistence.runTransaction( + "persistQueryMapping", + () -> { + ImmutableSortedSet remoteKeys = DocumentKey.emptyKeySet(); + for (Document doc : docs) { + remoteKeys = remoteKeys.insert(doc.getKey()); + } + queryCache.addMatchingKeys(remoteKeys, TEST_TARGET_ID); + }); + } + + /** Adds the provided documents to the remote document cache. */ + private void addDocument(Document... docs) { + persistence.runTransaction( + "addDocument", + () -> { + for (Document doc : docs) { + remoteDocumentCache.add(doc, doc.getVersion()); + } + }); + } + + private T expectIndexFreeQuery(Callable c) throws Exception { + try { + expectIndexFreeExecution = true; + return c.call(); + } finally { + expectIndexFreeExecution = null; + } + } + + private T expectFullCollectionQuery(Callable c) throws Exception { + try { + expectIndexFreeExecution = false; + return c.call(); + } finally { + expectIndexFreeExecution = null; + } + } + + private DocumentSet runQuery(Query query, QueryData queryData) { + Preconditions.checkNotNull( + expectIndexFreeExecution, + "Encountered runQuery() call not wrapped in expectIndexFreeQuery()/expectFullCollectionQuery()"); + ImmutableSortedMap docs = + queryEngine.getDocumentsMatchingQuery( + query, queryData, queryCache.getMatchingKeysForTargetId(TEST_TARGET_ID)); + View view = + new View(query, new ImmutableSortedSet<>(Collections.emptyList(), DocumentKey::compareTo)); + View.DocumentChanges viewDocChanges = view.computeDocChanges(docs); + return view.applyChanges(viewDocChanges).getSnapshot().getDocuments(); + } + + @Test + public void usesTargetMappingForInitialView() throws Exception { + Query query = query("coll").filter(filter("matches", "==", true)); + QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true); + + addDocument(MATCHING_DOC_A, MATCHING_DOC_B); + persistQueryMapping(MATCHING_DOC_A, MATCHING_DOC_B); + + DocumentSet docs = expectIndexFreeQuery(() -> runQuery(query, queryData)); + assertEquals(docSet(query.comparator(), MATCHING_DOC_A, MATCHING_DOC_B), docs); + } + + @Test + public void filtersNonMatchingInitialResults() throws Exception { + Query query = query("coll").filter(filter("matches", "==", true)); + QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true); + + addDocument(MATCHING_DOC_A, MATCHING_DOC_B); + persistQueryMapping(MATCHING_DOC_A, MATCHING_DOC_B); + + // Add a mutated document that is not yet part of query's set of remote keys. + addDocument(PENDING_NON_MATCHING_DOC_A); + + DocumentSet docs = expectIndexFreeQuery(() -> runQuery(query, queryData)); + assertEquals(docSet(query.comparator(), MATCHING_DOC_B), docs); + } + + @Test + public void includesChangesSinceInitialResults() throws Exception { + Query query = query("coll").filter(filter("matches", "==", true)); + QueryData originalQueryData = queryData(query, /* hasLimboFreeSnapshot= */ true); + + addDocument(MATCHING_DOC_A, MATCHING_DOC_B); + persistQueryMapping(MATCHING_DOC_A, MATCHING_DOC_B); + + DocumentSet docs = expectIndexFreeQuery(() -> runQuery(query, originalQueryData)); + assertEquals(docSet(query.comparator(), MATCHING_DOC_A, MATCHING_DOC_B), docs); + + addDocument(UPDATED_MATCHING_DOC_B); + + docs = expectIndexFreeQuery(() -> runQuery(query, originalQueryData)); + assertEquals(docSet(query.comparator(), MATCHING_DOC_A, UPDATED_MATCHING_DOC_B), docs); + } + + @Test + public void doesNotUseInitialResultsWithoutLimboFreeSnapshotVersion() throws Exception { + Query query = query("coll").filter(filter("matches", "==", true)); + QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ false); + + DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData)); + assertEquals(docSet(query.comparator()), docs); + } + + @Test + public void doesNotUseInitialResultsForUnfilteredCollectionQuery() throws Exception { + Query query = query("coll"); + QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true); + + DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData)); + assertEquals(docSet(query.comparator()), docs); + } + + @Test + public void doesNotUseInitialResultsForLimitQueryWithDocumentRemoval() throws Exception { + Query query = query("coll").filter(filter("matches", "==", true)).limit(1); + + // While the backend would never add DocA to the set of remote keys, this allows us to easily + // simulate what would happen when a document no longer matches due to an out-of-band update. + addDocument(NON_MATCHING_DOC_A); + persistQueryMapping(NON_MATCHING_DOC_A); + + addDocument(MATCHING_DOC_B); + + QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true); + DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData)); + assertEquals(docSet(query.comparator(), MATCHING_DOC_B), docs); + } + + @Test + public void doesNotUseInitialResultsForLimitQueryWithPendingWrite() throws Exception { + Query query = + query("coll") + .filter(filter("matches", "==", true)) + .orderBy(orderBy("order", "desc")) + .limit(1); + + // Add a query mapping for a document that matches, but that sorts below another document due to + // a pending write. + addDocument(PENDING_MATCHING_DOC_A); + persistQueryMapping(PENDING_MATCHING_DOC_A); + + QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true); + + addDocument(MATCHING_DOC_B); + + DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData)); + assertEquals(docSet(query.comparator(), MATCHING_DOC_B), docs); + } + + @Test + public void doesNotUseInitialResultsForLimitQueryWithDocumentThatHasBeenUpdatedOutOfBand() + throws Exception { + Query query = + query("coll") + .filter(filter("matches", "==", true)) + .orderBy(orderBy("order", "desc")) + .limit(1); + + // Add a query mapping for a document that matches, but that sorts below another document based + // due to an update that the SDK received after the query's snapshot was persisted. + addDocument(UDPATED_DOC_A); + persistQueryMapping(UDPATED_DOC_A); + + QueryData queryData = queryData(query, /* hasLimboFreeSnapshot= */ true); + + addDocument(MATCHING_DOC_B); + + DocumentSet docs = expectFullCollectionQuery(() -> runQuery(query, queryData)); + assertEquals(docSet(query.comparator(), MATCHING_DOC_B), docs); + } + + private QueryData queryData(Query query, boolean hasLimboFreeSnapshot) { + return new QueryData( + query, + TEST_TARGET_ID, + 1, + QueryPurpose.LISTEN, + version(10), + hasLimboFreeSnapshot ? version(10) : SnapshotVersion.NONE, + ByteString.EMPTY); + } +} diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexedQueryEngineTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexedQueryEngineTest.java index 48ab21b2e11..b18efe0d9aa 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexedQueryEngineTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexedQueryEngineTest.java @@ -69,7 +69,7 @@ public void setUp() { remoteDocuments = persistence.getRemoteDocumentCache(); LocalDocumentsView localDocuments = new LocalDocumentsView(remoteDocuments, mutationQueue, persistence.getIndexManager()); - queryEngine = new IndexedQueryEngine(localDocuments, index); + queryEngine = new IndexedQueryEngine(index); } private void addDocument(Document newDoc) { @@ -214,7 +214,8 @@ public void addDocumentQuery() { Query query = query("coll").filter(filter("a", "==", "a")); ImmutableSortedMap results = - queryEngine.getDocumentsMatchingQuery(query); + queryEngine.getDocumentsMatchingQuery( + query, /* queryData= */ null, DocumentKey.emptyKeySet()); assertThat(results).doesNotContain(IGNORED_DOC.getKey()); assertThat(results).contains(MATCHING_DOC.getKey()); @@ -229,7 +230,8 @@ public void updateDocumentQuery() { Query query = query("coll").filter(filter("a", "==", "a")); ImmutableSortedMap results = - queryEngine.getDocumentsMatchingQuery(query); + queryEngine.getDocumentsMatchingQuery( + query, /* queryData= */ null, DocumentKey.emptyKeySet()); assertThat(results).doesNotContain(IGNORED_DOC.getKey()); assertThat(results).contains(MATCHING_DOC.getKey()); @@ -244,7 +246,8 @@ public void removeDocumentQuery() { Query query = query("coll").filter(filter("a", "==", "a")); ImmutableSortedMap results = - queryEngine.getDocumentsMatchingQuery(query); + queryEngine.getDocumentsMatchingQuery( + query, /* queryData= */ null, DocumentKey.emptyKeySet()); assertThat(results).doesNotContain(IGNORED_DOC.getKey()); assertThat(results).doesNotContain(MATCHING_DOC.getKey()); @@ -262,7 +265,8 @@ public void nestedQuery() { Query query = query("coll").filter(filter("a.a", "==", "a")); ImmutableSortedMap results = - queryEngine.getDocumentsMatchingQuery(query); + queryEngine.getDocumentsMatchingQuery( + query, /* queryData= */ null, DocumentKey.emptyKeySet()); assertThat(results).doesNotContain(ignoredDoc.getKey()); assertThat(results).contains(matchingDoc.getKey()); @@ -276,7 +280,8 @@ public void orderByQuery() { Query query = query("coll").orderBy(TestUtil.orderBy("a")); ImmutableSortedMap results = - queryEngine.getDocumentsMatchingQuery(query); + queryEngine.getDocumentsMatchingQuery( + query, /* queryData= */ null, DocumentKey.emptyKeySet()); assertThat(results).doesNotContain(IGNORED_DOC.getKey()); assertThat(results).contains(MATCHING_DOC.getKey()); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java index 4004759bf6f..cdb16bcc2ba 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java @@ -19,6 +19,7 @@ import static com.google.firebase.firestore.testutil.TestUtil.deleteMutation; import static com.google.firebase.firestore.testutil.TestUtil.deletedDoc; import static com.google.firebase.firestore.testutil.TestUtil.doc; +import static com.google.firebase.firestore.testutil.TestUtil.filter; import static com.google.firebase.firestore.testutil.TestUtil.key; import static com.google.firebase.firestore.testutil.TestUtil.map; import static com.google.firebase.firestore.testutil.TestUtil.noChangeEvent; @@ -40,6 +41,8 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; +import static org.junit.Assume.assumeTrue; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -72,6 +75,7 @@ import java.util.Set; import javax.annotation.Nullable; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -86,15 +90,19 @@ * */ public abstract class LocalStoreTestCase { + private QueryEngine queryEngine; private Persistence localStorePersistence; private LocalStore localStore; private List batches; private @Nullable ImmutableSortedMap lastChanges; + private ImmutableSortedMap lastQueryResult; private int lastTargetId; AccumulatingStatsCollector statsCollector; + abstract QueryEngine getQueryEngine(); + abstract Persistence getPersistence(); abstract boolean garbageCollectorIsEager(); @@ -104,10 +112,12 @@ public void setUp() { statsCollector = new AccumulatingStatsCollector(); batches = new ArrayList<>(); lastChanges = null; + lastQueryResult = null; lastTargetId = 0; localStorePersistence = getPersistence(); - localStore = new LocalStore(localStorePersistence, User.UNAUTHENTICATED); + queryEngine = getQueryEngine(); + localStore = new LocalStore(localStorePersistence, queryEngine, User.UNAUTHENTICATED); localStore.start(); } @@ -136,6 +146,10 @@ private void notifyLocalViewChanges(LocalViewChanges changes) { localStore.notifyLocalViewChanges(asList(changes)); } + private void udpateViews(int targetId, boolean synced) { + notifyLocalViewChanges(viewChanges(targetId, synced, asList(), asList())); + } + private void acknowledgeMutation(long documentVersion, @Nullable Object transformResult) { MutationBatch batch = batches.remove(0); SnapshotVersion version = version(documentVersion); @@ -165,6 +179,11 @@ private int allocateQuery(Query query) { return queryData.getTargetId(); } + private void executeQuery(Query query) { + resetPersistenceStats(); + lastQueryResult = localStore.executeQuery(query); + } + private void releaseQuery(Query query) { localStore.releaseQuery(query); } @@ -212,6 +231,14 @@ private void assertNotContains(String keyPathString) { assertNull(actual); } + private void assertQueryReturned(String... keys) { + assertNotNull(lastQueryResult); + for (String key : keys) { + assertTrue("Expected query to return: " + key, lastQueryResult.containsKey(key(key))); + } + assertEquals(lastQueryResult.size(), keys.length); + } + /** * Asserts the expected numbers of mutation rows read by the MutationQueue since the last call to * `resetPersistenceStats()`. @@ -284,7 +311,7 @@ public void testHandlesSetMutationThenAckThenRelease() { allocateQuery(query); writeMutation(setMutation("foo/bar", map("foo", "bar"))); - notifyLocalViewChanges(viewChanges(2, asList("foo/bar"), emptyList())); + notifyLocalViewChanges(viewChanges(2, /* synced= */ false, asList("foo/bar"), emptyList())); assertChanged(doc("foo/bar", 0, map("foo", "bar"), Document.DocumentState.LOCAL_MUTATIONS)); assertContains(doc("foo/bar", 0, map("foo", "bar"), Document.DocumentState.LOCAL_MUTATIONS)); @@ -804,14 +831,16 @@ public void testPinsDocumentsInTheLocalView() { assertContains(doc("foo/bar", 1, map("foo", "bar"))); assertContains(doc("foo/baz", 0, map("foo", "baz"), Document.DocumentState.LOCAL_MUTATIONS)); - notifyLocalViewChanges(viewChanges(2, asList("foo/bar", "foo/baz"), emptyList())); + notifyLocalViewChanges( + viewChanges(2, /* synced= */ false, asList("foo/bar", "foo/baz"), emptyList())); applyRemoteEvent(updateRemoteEvent(doc("foo/bar", 1, map("foo", "bar")), none, two)); applyRemoteEvent(updateRemoteEvent(doc("foo/baz", 2, map("foo", "baz")), two, none)); acknowledgeMutation(2); assertContains(doc("foo/bar", 1, map("foo", "bar"))); assertContains(doc("foo/baz", 2, map("foo", "baz"))); - notifyLocalViewChanges(viewChanges(2, emptyList(), asList("foo/bar", "foo/baz"))); + notifyLocalViewChanges( + viewChanges(2, /* synced= */ false, emptyList(), asList("foo/bar", "foo/baz"))); releaseQuery(query); assertNotContains("foo/bar"); @@ -901,10 +930,7 @@ public void testReadsAllDocumentsForCollectionQueries() { @Test public void testPersistsResumeTokens() { - // This test only works in the absence of the EagerGarbageCollector. - if (garbageCollectorIsEager()) { - return; - } + assumeFalse(garbageCollectorIsEager()); Query query = query("foo/bar"); int targetId = allocateQuery(query); @@ -921,10 +947,7 @@ public void testPersistsResumeTokens() { @Test public void testDoesNotReplaceResumeTokenWithEmptyByteString() { - // This test only works in the absence of the EagerGarbageCollector. - if (garbageCollectorIsEager()) { - return; - } + assumeFalse(garbageCollectorIsEager()); Query query = query("foo/bar"); int targetId = allocateQuery(query); @@ -979,12 +1002,10 @@ public void testHandlesSetMutationThenTransformMutationThenTransformMutation() { @Test public void testHandlesSetMutationThenAckThenTransformMutationThenAckThenTransformMutation() { - if (garbageCollectorIsEager()) { - // Since this test doesn't start a listen, Eager GC removes the documents from the cache as - // soon as the mutation is applied. This creates a lot of special casing in this unit test but - // does not expand its test coverage. - return; - } + // Since this test doesn't start a listen, Eager GC removes the documents from the cache as + // soon as the mutation is applied. This creates a lot of special casing in this unit test but + // does not expand its test coverage. + assumeFalse(garbageCollectorIsEager()); writeMutation(setMutation("foo/bar", map("sum", 0))); assertContains(doc("foo/bar", 0, map("sum", 0), Document.DocumentState.LOCAL_MUTATIONS)); @@ -1007,6 +1028,208 @@ public void testHandlesSetMutationThenAckThenTransformMutationThenAckThenTransfo assertChanged(doc("foo/bar", 2, map("sum", 3), Document.DocumentState.LOCAL_MUTATIONS)); } + @Test + public void testUsesTargetMappingToExecuteQueries() { + assumeFalse(garbageCollectorIsEager()); + assumeTrue(queryEngine instanceof IndexFreeQueryEngine); + + // This test verifies that once a target mapping has been written, only documents that match + // the query are read from the RemoteDocumentCache. + + Query query = + Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true)); + int targetId = allocateQuery(query); + + writeMutation(setMutation("foo/a", map("matches", true))); + writeMutation(setMutation("foo/b", map("matches", true))); + writeMutation(setMutation("foo/ignored", map("matches", false))); + acknowledgeMutation(10); + acknowledgeMutation(10); + acknowledgeMutation(10); + + // Execute the query, but note that we read all existing documents from the RemoteDocumentCache + // since we do not yet have target mapping. + executeQuery(query); + assertRemoteDocumentsRead(3); + + // Issue a RemoteEvent to persist the target mapping. + applyRemoteEvent( + addedRemoteEvent( + asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))), + asList(targetId), + emptyList())); + applyRemoteEvent(noChangeEvent(targetId, 10)); + udpateViews(targetId, /* synced= */ true); + + // Execute the query again, this time verifying that we only read the two documents that match + // the query. + executeQuery(query); + assertRemoteDocumentsRead(2); + assertQueryReturned("foo/a", "foo/b"); + } + + @Test + public void testLastLimboFreeSnapshotIsAdvancedDuringViewProcessing() { + assumeFalse(garbageCollectorIsEager()); + assumeTrue(queryEngine instanceof IndexFreeQueryEngine); + + // This test verifies that the `lastLimboFreeSnapshot` version for QueryData is advanced when + // we compute a limbo-free free view and that the mapping is persisted when we release a query. + + writeMutation(setMutation("foo/ignored", map("matches", false))); + acknowledgeMutation(10); + + Query query = + Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true)); + int targetId = allocateQuery(query); + + // Mark the query as current. + applyRemoteEvent( + addedRemoteEvent( + asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))), + asList(targetId), + emptyList())); + applyRemoteEvent(noChangeEvent(targetId, 10)); + + // At this point, we have not yet confirmed that the query is limbo free. + Assert.assertEquals( + SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion()); + + // Update the view, but don't mark the view synced. + Assert.assertEquals( + SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion()); + udpateViews(targetId, /* synced=*/ true); + + // The query is marked limbo-free only when we mark the view synced. + udpateViews(targetId, /* synced=*/ true); + Assert.assertNotEquals( + SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion()); + + // The last limbo free snapshot version is persisted even if we release the query. + releaseQuery(query); + allocateQuery(query); + + // Verify that we only read the two documents that match the query. + executeQuery(query); + assertRemoteDocumentsRead(2); + assertQueryReturned("foo/a", "foo/b"); + } + + @Test + public void testQueriesIncludeLocallyModifiedDocuments() { + assumeFalse(garbageCollectorIsEager()); + + // This test verifies that queries that have a persisted TargetMapping include documents that + // were modified by local edits after the target mapping was written. + Query query = + Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true)); + int targetId = allocateQuery(query); + + applyRemoteEvent( + addedRemoteEvent( + asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))), + asList(targetId), + emptyList())); + applyRemoteEvent(noChangeEvent(targetId, 10)); + udpateViews(targetId, /* synced= */ true); + + // Execute the query based on the RemoteEvent. + executeQuery(query); + assertQueryReturned("foo/a", "foo/b"); + + // Write a document. + writeMutation(setMutation("foo/c", map("matches", true))); + + // Execute the query and make sure that the pending mutation is included in the result. + executeQuery(query); + assertQueryReturned("foo/a", "foo/b", "foo/c"); + + acknowledgeMutation(11); + + // Execute the query and make sure that the acknowledged mutation is included in the result. + executeQuery(query); + assertQueryReturned("foo/a", "foo/b", "foo/c"); + } + + @Test + public void testQueriesIncludeDocumentsFromOtherQueries() { + assumeFalse(garbageCollectorIsEager()); + + // This test verifies that queries that have a persisted TargetMapping include documents that + // were modified by other queries after the target mapping was written. + + Query filteredQuery = + Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true)); + int targetId = allocateQuery(filteredQuery); + + applyRemoteEvent( + addedRemoteEvent( + asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))), + asList(targetId), + emptyList())); + applyRemoteEvent(noChangeEvent(targetId, 10)); + udpateViews(targetId, /* synced=*/ true); + releaseQuery(filteredQuery); + + // Start another query and add more matching documents to the collection. + Query fullQuery = + Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true)); + targetId = allocateQuery(filteredQuery); + applyRemoteEvent( + addedRemoteEvent( + asList( + doc("foo/a", 10, map("matches", true)), + doc("foo/b", 10, map("matches", true)), + doc("foo/c", 20, map("matches", true))), + asList(targetId), + emptyList())); + releaseQuery(fullQuery); + + // Run the original query again and ensure that both the original matches as well as all new + // matches are included in the result set. + allocateQuery(filteredQuery); + executeQuery(filteredQuery); + assertQueryReturned("foo/a", "foo/b", "foo/c"); + } + + @Test + public void testQueriesFilterDocumentsThatNoLongerMatch() { + assumeFalse(garbageCollectorIsEager()); + + // This test verifies that documents that once matched a query are post-filtered if they no + // longer match the query filter. + + // Add two document results for a simple filter query + Query filteredQuery = + Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true)); + int targetId = allocateQuery(filteredQuery); + + applyRemoteEvent( + addedRemoteEvent( + asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))), + asList(targetId), + emptyList())); + applyRemoteEvent(noChangeEvent(targetId, 10)); + udpateViews(targetId, /* synced=*/ true); + releaseQuery(filteredQuery); + + // Modify one of the documents to no longer match while the filtered query is inactive. + Query fullQuery = + Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true)); + targetId = allocateQuery(filteredQuery); + applyRemoteEvent( + addedRemoteEvent( + asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 20, map("matches", false))), + asList(targetId), + emptyList())); + releaseQuery(fullQuery); + + // Re-run the filtered query and verify that the modified document is no longer returned. + allocateQuery(filteredQuery); + executeQuery(filteredQuery); + assertQueryReturned("foo/a"); + } + @Test public void testHandlesSetMutationThenTransformMutationThenRemoteEventThenTransformMutation() { Query query = Query.atPath(ResourcePath.fromString("foo")); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLocalStoreTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLocalStoreTest.java index 5793626fe2f..4b9b05664b5 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLocalStoreTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLocalStoreTest.java @@ -14,19 +14,38 @@ package com.google.firebase.firestore.local; +import java.util.Arrays; +import java.util.Collection; import org.junit.runner.RunWith; -import org.robolectric.RobolectricTestRunner; +import org.robolectric.ParameterizedRobolectricTestRunner; import org.robolectric.annotation.Config; -@RunWith(RobolectricTestRunner.class) +@RunWith(ParameterizedRobolectricTestRunner.class) @Config(manifest = Config.NONE) public class MemoryLocalStoreTest extends LocalStoreTestCase { + private QueryEngine queryEngine; + + @ParameterizedRobolectricTestRunner.Parameters(name = "QueryEngine = {0}") + public static Collection data() { + return Arrays.asList( + new Object[] {new SimpleQueryEngine()}, new Object[] {new IndexFreeQueryEngine()}); + } + + public MemoryLocalStoreTest(QueryEngine queryEngine) { + this.queryEngine = queryEngine; + } + @Override Persistence getPersistence() { return PersistenceTestHelpers.createEagerGCMemoryPersistence(statsCollector); } + @Override + QueryEngine getQueryEngine() { + return this.queryEngine; + } + @Override boolean garbageCollectorIsEager() { return true; diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java index 5fc83ea808a..9f66aa72679 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java @@ -14,14 +14,33 @@ package com.google.firebase.firestore.local; +import java.util.Arrays; +import java.util.Collection; import org.junit.runner.RunWith; -import org.robolectric.RobolectricTestRunner; +import org.robolectric.ParameterizedRobolectricTestRunner; import org.robolectric.annotation.Config; -@RunWith(RobolectricTestRunner.class) +@RunWith(ParameterizedRobolectricTestRunner.class) @Config(manifest = Config.NONE) public class SQLiteLocalStoreTest extends LocalStoreTestCase { + private QueryEngine queryEngine; + + @ParameterizedRobolectricTestRunner.Parameters(name = "QueryEngine = {0}") + public static Collection data() { + return Arrays.asList( + new Object[] {new SimpleQueryEngine()}, new Object[] {new IndexFreeQueryEngine()}); + } + + public SQLiteLocalStoreTest(QueryEngine queryEngine) { + this.queryEngine = queryEngine; + } + + @Override + QueryEngine getQueryEngine() { + return this.queryEngine; + } + @Override Persistence getPersistence() { return PersistenceTestHelpers.createSQLitePersistence(statsCollector); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/QueryEvent.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/QueryEvent.java index 4f528afe78a..ddb63712e2a 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/QueryEvent.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/QueryEvent.java @@ -14,10 +14,10 @@ package com.google.firebase.firestore.spec; +import androidx.annotation.Nullable; import com.google.firebase.firestore.FirebaseFirestoreException; import com.google.firebase.firestore.core.Query; import com.google.firebase.firestore.core.ViewSnapshot; -import javax.annotation.Nullable; /** Object that contains exactly one of either a view snapshot or an error for the given query. */ public class QueryEvent { diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java index e6c5f6f966a..b4816ba1adb 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java @@ -48,7 +48,9 @@ import com.google.firebase.firestore.local.LocalStore; import com.google.firebase.firestore.local.Persistence; import com.google.firebase.firestore.local.QueryData; +import com.google.firebase.firestore.local.QueryEngine; import com.google.firebase.firestore.local.QueryPurpose; +import com.google.firebase.firestore.local.SimpleQueryEngine; import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MaybeDocument; @@ -262,7 +264,9 @@ protected void specTearDown() throws Exception { */ private void initClient() { localPersistence = getPersistence(garbageCollectionEnabled); - LocalStore localStore = new LocalStore(localPersistence, currentUser); + // TODO(index-free): Update to index-free query engine when it becomes default. + QueryEngine queryEngine = new SimpleQueryEngine(); + LocalStore localStore = new LocalStore(localPersistence, queryEngine, currentUser); queue = new AsyncQueue(); diff --git a/firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java b/firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java index 476e6d05f15..42a35a5c71e 100644 --- a/firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java +++ b/firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java @@ -23,6 +23,7 @@ import androidx.annotation.NonNull; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.android.gms.common.internal.Preconditions; import com.google.common.base.Charsets; import com.google.common.collect.Lists; import com.google.common.collect.Sets; @@ -395,8 +396,16 @@ public static RemoteEvent noChangeEvent(int targetId, int version, ByteString re public static RemoteEvent addedRemoteEvent( MaybeDocument doc, List updatedInTargets, List removedFromTargets) { - DocumentChange change = - new DocumentChange(updatedInTargets, removedFromTargets, doc.getKey(), doc); + return addedRemoteEvent(Collections.singletonList(doc), updatedInTargets, removedFromTargets); + } + + public static RemoteEvent addedRemoteEvent( + List docs, List updatedInTargets, List removedFromTargets) { + Preconditions.checkArgument(!docs.isEmpty(), "Cannot pass empty docs array"); + + ResourcePath collectionPath = docs.get(0).getKey().getPath().popLast(); + SnapshotVersion version = docs.get(0).getVersion(); + WatchChangeAggregator aggregator = new WatchChangeAggregator( new WatchChangeAggregator.TargetMetadataProvider() { @@ -407,11 +416,16 @@ public ImmutableSortedSet getRemoteKeysForTarget(int targetId) { @Override public QueryData getQueryDataForTarget(int targetId) { - return queryData(targetId, QueryPurpose.LISTEN, doc.getKey().toString()); + return queryData(targetId, QueryPurpose.LISTEN, collectionPath.toString()); } }); - aggregator.handleDocumentChange(change); - return aggregator.createRemoteEvent(doc.getVersion()); + + for (MaybeDocument doc : docs) { + DocumentChange change = + new DocumentChange(updatedInTargets, removedFromTargets, doc.getKey(), doc); + aggregator.handleDocumentChange(change); + } + return aggregator.createRemoteEvent(version); } public static RemoteEvent updateRemoteEvent( @@ -509,7 +523,7 @@ public static MutationResult mutationResult(long version) { } public static LocalViewChanges viewChanges( - int targetId, List addedKeys, List removedKeys) { + int targetId, boolean synced, List addedKeys, List removedKeys) { ImmutableSortedSet added = DocumentKey.emptyKeySet(); for (String keyPath : addedKeys) { added = added.insert(key(keyPath)); @@ -518,7 +532,7 @@ public static LocalViewChanges viewChanges( for (String keyPath : removedKeys) { removed = removed.insert(key(keyPath)); } - return new LocalViewChanges(targetId, false, added, removed); + return new LocalViewChanges(targetId, synced, added, removed); } /** Creates a resume token to match the given snapshot version. */