diff --git a/firebase-firestore/CHANGELOG.md b/firebase-firestore/CHANGELOG.md index 0ed23c6a3f7..6a31280f361 100644 --- a/firebase-firestore/CHANGELOG.md +++ b/firebase-firestore/CHANGELOG.md @@ -3,8 +3,9 @@ by opting into a release at [go/firebase-android-release](http:go/firebase-android-release) (Googlers only). # 24.1.0 -- [changed] Performance optimization for offline usage. -- [changed] Improved performance for queries with collections that contain +- [changed] Improved performance for databases that contain many document + updates that have not yet been synced with the backend. +- [changed] Improved performance for queries against collections that contain subcollections. # 24.0.0 diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexBackfiller.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexBackfiller.java index ff3caf18390..58fff876637 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexBackfiller.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexBackfiller.java @@ -19,19 +19,15 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.firebase.database.collection.ImmutableSortedMap; -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.FieldIndex; import com.google.firebase.firestore.model.FieldIndex.IndexOffset; -import com.google.firebase.firestore.model.ResourcePath; import com.google.firebase.firestore.util.AsyncQueue; import com.google.firebase.firestore.util.Logger; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; -import java.util.List; +import java.util.Iterator; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -138,22 +134,18 @@ private int writeIndexEntries(LocalDocumentsView localDocumentsView) { /** Writes entries for the fetched field indexes. */ private int writeEntriesForCollectionGroup( LocalDocumentsView localDocumentsView, String collectionGroup, int entriesRemainingUnderCap) { - Query query = new Query(ResourcePath.EMPTY, collectionGroup); + // TODO(indexing): Support mutation batch Ids when sorting and writing indexes. // Use the earliest offset of all field indexes to query the local cache. IndexOffset existingOffset = getExistingOffset(indexManager.getFieldIndexes(collectionGroup)); - - // TODO(indexing): Use limit queries to only fetch the required number of entries. - // TODO(indexing): Support mutation batch Ids when sorting and writing indexes. ImmutableSortedMap documents = - localDocumentsView.getDocumentsMatchingQuery(query, existingOffset); - - List oldestDocuments = getOldestDocuments(documents, entriesRemainingUnderCap); - indexManager.updateIndexEntries(oldestDocuments); + localDocumentsView.getDocuments(collectionGroup, existingOffset, entriesRemainingUnderCap); + indexManager.updateIndexEntries(documents); - IndexOffset newOffset = getNewOffset(oldestDocuments, existingOffset); + IndexOffset newOffset = getNewOffset(documents, existingOffset); indexManager.updateCollectionGroup(collectionGroup, newOffset); - return oldestDocuments.size(); + + return documents.size(); } /** Returns the lowest offset for the provided index group. */ @@ -168,37 +160,22 @@ private IndexOffset getExistingOffset(Collection fieldIndexes) { return lowestOffset == null ? IndexOffset.NONE : lowestOffset; } - /** - * Returns the offset for the index based on the newly indexed documents. - * - * @param documents a list of documents sorted by read time and key (ascending) - * @param currentOffset the current offset of the index group - */ - private IndexOffset getNewOffset(List documents, IndexOffset currentOffset) { - IndexOffset latestOffset = - documents.isEmpty() - ? IndexOffset.create(remoteDocumentCache.getLatestReadTime()) - : IndexOffset.create( - documents.get(documents.size() - 1).getReadTime(), - documents.get(documents.size() - 1).getKey()); - // Make sure the index does not go back in time - latestOffset = latestOffset.compareTo(currentOffset) > 0 ? latestOffset : currentOffset; - return latestOffset; - } - - /** Returns up to {@code count} documents sorted by read time and key. */ - private List getOldestDocuments( - ImmutableSortedMap documents, int count) { - List oldestDocuments = new ArrayList<>(); - for (Map.Entry entry : documents) { - oldestDocuments.add(entry.getValue()); + /** Returns the offset for the index based on the newly indexed documents. */ + private IndexOffset getNewOffset( + ImmutableSortedMap documents, IndexOffset currentOffset) { + if (documents.isEmpty()) { + return IndexOffset.create(remoteDocumentCache.getLatestReadTime()); + } else { + IndexOffset latestOffset = currentOffset; + Iterator> it = documents.iterator(); + while (it.hasNext()) { + IndexOffset newOffset = IndexOffset.fromDocument(it.next().getValue()); + if (newOffset.compareTo(latestOffset) > 0) { + latestOffset = newOffset; + } + } + return latestOffset; } - Collections.sort( - oldestDocuments, - (l, r) -> - IndexOffset.create(l.getReadTime(), l.getKey()) - .compareTo(IndexOffset.create(r.getReadTime(), r.getKey()))); - return oldestDocuments.subList(0, Math.min(count, oldestDocuments.size())); } @VisibleForTesting diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexManager.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexManager.java index 7868134d1b6..0307779523e 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexManager.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexManager.java @@ -15,6 +15,7 @@ package com.google.firebase.firestore.local; import androidx.annotation.Nullable; +import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.firestore.core.Target; import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.DocumentKey; @@ -97,5 +98,5 @@ public interface IndexManager { void updateCollectionGroup(String collectionGroup, FieldIndex.IndexOffset offset); /** Updates the index entries for the provided documents. */ - void updateIndexEntries(Collection documents); + void updateIndexEntries(ImmutableSortedMap documents); } 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 c73d999afd4..f94cacb753b 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 @@ -238,10 +238,13 @@ void recalculateAndSaveOverlays(Set documentKeys) { recalculateAndSaveOverlays(docs); } - // TODO: The Querying implementation here should move 100% to the query engines. - // Instead, we should just provide a getCollectionDocuments() method here that return all the - // documents in a given collection so that query engine can do that and then filter in - // memory. + /** Gets the local view of the next {@code count} documents based on their read time. */ + ImmutableSortedMap getDocuments( + String collectionGroup, IndexOffset offset, int count) { + Map docs = + remoteDocumentCache.getAll(collectionGroup, offset, count); + return getLocalViewOfDocuments(docs, new HashSet<>()); + } /** * Performs a query against the local view of all documents. diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryIndexManager.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryIndexManager.java index d235c650130..c1758262fd6 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryIndexManager.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryIndexManager.java @@ -16,6 +16,7 @@ import static com.google.firebase.firestore.util.Assert.hardAssert; import androidx.annotation.Nullable; +import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.firestore.core.Target; import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.DocumentKey; @@ -98,7 +99,7 @@ public Collection getFieldIndexes() { } @Override - public void updateIndexEntries(Collection documents) { + public void updateIndexEntries(ImmutableSortedMap documents) { // Field indices are not supported with memory persistence. } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryRemoteDocumentCache.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryRemoteDocumentCache.java index a8a7e29cdfb..acd84633979 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryRemoteDocumentCache.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryRemoteDocumentCache.java @@ -81,6 +81,13 @@ public Map getAll(Iterable keys) { return result; } + @Override + public Map getAll( + String collectionGroup, IndexOffset offset, int limit) { + // This method should only be called from the IndexBackfiller if SQLite is enabled. + throw new UnsupportedOperationException("getAll(String, IndexOffset, int) is not supported."); + } + @Override public ImmutableSortedMap getAllDocumentsMatchingQuery( Query query, IndexOffset offset) { @@ -108,7 +115,7 @@ public ImmutableSortedMap getAllDocumentsMatchingQ continue; } - if (IndexOffset.create(doc.getReadTime(), doc.getKey()).compareTo(offset) <= 0) { + if (IndexOffset.fromDocument(doc).compareTo(offset) <= 0) { continue; } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/RemoteDocumentCache.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/RemoteDocumentCache.java index 701ddda9d9b..87283229ee2 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/RemoteDocumentCache.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/RemoteDocumentCache.java @@ -17,7 +17,7 @@ import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.firestore.core.Query; import com.google.firebase.firestore.model.DocumentKey; -import com.google.firebase.firestore.model.FieldIndex; +import com.google.firebase.firestore.model.FieldIndex.IndexOffset; import com.google.firebase.firestore.model.MutableDocument; import com.google.firebase.firestore.model.SnapshotVersion; import java.util.Map; @@ -65,6 +65,17 @@ interface RemoteDocumentCache { */ Map getAll(Iterable documentKeys); + /** + * Looks up the next {@code limit} documents for a collection group based on the provided offset. + * The ordering is based on the document's read time and key. + * + * @param collectionGroup The collection group to scan. + * @param offset The offset to start the scan at. + * @param limit The maximum number of results to return. + * @return A map with next set of documents. + */ + Map getAll(String collectionGroup, IndexOffset offset, int limit); + /** * Executes a query against the cached Document entries * @@ -78,7 +89,7 @@ interface RemoteDocumentCache { * @return The set of matching documents. */ ImmutableSortedMap getAllDocumentsMatchingQuery( - Query query, FieldIndex.IndexOffset offset); + Query query, IndexOffset offset); /** Returns the latest read time of any document in the cache. */ SnapshotVersion getLatestReadTime(); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteIndexManager.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteIndexManager.java index 5aad19168b3..ced9950e402 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteIndexManager.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteIndexManager.java @@ -23,6 +23,7 @@ import androidx.annotation.Nullable; import com.google.firebase.Timestamp; +import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.firestore.auth.User; import com.google.firebase.firestore.core.Bound; import com.google.firebase.firestore.core.FieldFilter; @@ -222,16 +223,15 @@ public void deleteFieldIndex(FieldIndex index) { } @Override - public void updateIndexEntries(Collection documents) { + public void updateIndexEntries(ImmutableSortedMap documents) { hardAssert(started, "IndexManager not started"); - for (Document document : documents) { - Collection fieldIndexes = getFieldIndexes(document.getKey().getCollectionGroup()); + for (Map.Entry entry : documents) { + Collection fieldIndexes = getFieldIndexes(entry.getKey().getCollectionGroup()); for (FieldIndex fieldIndex : fieldIndexes) { - SortedSet existingEntries = - getExistingIndexEntries(document.getKey(), fieldIndex); - SortedSet newEntries = computeIndexEntries(document, fieldIndex); + SortedSet existingEntries = getExistingIndexEntries(entry.getKey(), fieldIndex); + SortedSet newEntries = computeIndexEntries(entry.getValue(), fieldIndex); if (!existingEntries.equals(newEntries)) { - updateEntries(document, existingEntries, newEntries); + updateEntries(entry.getValue(), existingEntries, newEntries); } } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLitePersistence.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLitePersistence.java index f15d14f4ec4..1977ed066bd 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLitePersistence.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLitePersistence.java @@ -54,6 +54,10 @@ * helper routines that make dealing with SQLite much more pleasant. */ public final class SQLitePersistence extends Persistence { + /** + * The maximum number of bind args for a single statement. Set to 900 instead of 999 for safety. + */ + public static final int MAX_ARGS = 900; /** * Creates the database name that is used to identify the database to be used with a Firestore diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java index 85e3af99239..fb45c355c46 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java @@ -16,26 +16,33 @@ import static com.google.firebase.firestore.util.Assert.fail; import static com.google.firebase.firestore.util.Assert.hardAssert; +import static com.google.firebase.firestore.util.Util.firstNEntries; +import static com.google.firebase.firestore.util.Util.repeatSequence; +import androidx.annotation.VisibleForTesting; import com.google.firebase.Timestamp; import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.firestore.core.Query; import com.google.firebase.firestore.model.DocumentCollections; import com.google.firebase.firestore.model.DocumentKey; -import com.google.firebase.firestore.model.FieldIndex; +import com.google.firebase.firestore.model.FieldIndex.IndexOffset; import com.google.firebase.firestore.model.MutableDocument; +import com.google.firebase.firestore.model.ResourcePath; import com.google.firebase.firestore.model.SnapshotVersion; import com.google.firebase.firestore.util.BackgroundQueue; import com.google.firebase.firestore.util.Executors; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.MessageLite; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.Executor; final class SQLiteRemoteDocumentCache implements RemoteDocumentCache { + /** The number of bind args per collection group in {@link #getAll(String, IndexOffset, int)} */ + @VisibleForTesting static final int BINDS_PER_STATEMENT = 9; private final SQLitePersistence db; private final LocalSerializer serializer; @@ -129,47 +136,70 @@ public Map getAll(Iterable documentKe } @Override - public ImmutableSortedMap getAllDocumentsMatchingQuery( - final Query query, FieldIndex.IndexOffset offset) { - hardAssert( - !query.isCollectionGroupQuery(), - "CollectionGroup queries should be handled in LocalDocumentsView"); - - StringBuilder sql = - new StringBuilder( - "SELECT contents, read_time_seconds, read_time_nanos " - + "FROM remote_documents WHERE path >= ? AND path < ? AND path_length = ?"); + public Map getAll( + String collectionGroup, IndexOffset offset, int limit) { + List collectionParents = indexManager.getCollectionParents(collectionGroup); + List collections = new ArrayList<>(collectionParents.size()); + for (ResourcePath collectionParent : collectionParents) { + collections.add(collectionParent.append(collectionGroup)); + } - boolean hasOffset = !FieldIndex.IndexOffset.NONE.equals(offset); - Object[] bindVars = new Object[3 + (hasOffset ? 6 : 0)]; + if (collections.isEmpty()) { + return Collections.emptyMap(); + } else if (BINDS_PER_STATEMENT * collections.size() < SQLitePersistence.MAX_ARGS) { + return getAll(collections, offset, limit); + } else { + // We need to fan out our collection scan since SQLite only supports 999 binds per statement. + Map results = new HashMap<>(); + int pageSize = SQLitePersistence.MAX_ARGS / BINDS_PER_STATEMENT; + for (int i = 0; i < collections.size(); i += pageSize) { + results.putAll( + getAll( + collections.subList(i, Math.min(collections.size(), i + pageSize)), offset, limit)); + } + return firstNEntries(results, limit, IndexOffset.DOCUMENT_COMPARATOR); + } + } - String prefix = EncodedPath.encode(query.getPath()); + /** + * Returns the next {@code count} documents from the provided collections, ordered by read time. + */ + private Map getAll( + List collections, IndexOffset offset, int count) { + Timestamp readTime = offset.getReadTime().getTimestamp(); + DocumentKey documentKey = offset.getDocumentKey(); + StringBuilder sql = + repeatSequence( + "SELECT contents, read_time_seconds, read_time_nanos, path " + + "FROM remote_documents " + + "WHERE path >= ? AND path < ? AND path_length = ? " + + "AND (read_time_seconds > ? OR ( " + + "read_time_seconds = ? AND read_time_nanos > ?) OR ( " + + "read_time_seconds = ? AND read_time_nanos = ? and path > ?)) ", + collections.size(), + " UNION "); + sql.append("ORDER BY read_time_seconds, read_time_nanos, path LIMIT ?"); + + Object[] bindVars = new Object[BINDS_PER_STATEMENT * collections.size() + 1]; int i = 0; - bindVars[i++] = prefix; - bindVars[i++] = EncodedPath.prefixSuccessor(prefix); - bindVars[i++] = query.getPath().length() + 1; - - if (hasOffset) { - Timestamp readTime = offset.getReadTime().getTimestamp(); - DocumentKey documentKey = offset.getDocumentKey(); - - sql.append( - " AND (read_time_seconds > ? OR (" - + "read_time_seconds = ? AND read_time_nanos > ?) OR (" - + "read_time_seconds = ? AND read_time_nanos = ? and path > ?))"); + for (ResourcePath collection : collections) { + String prefixPath = EncodedPath.encode(collection); + bindVars[i++] = prefixPath; + bindVars[i++] = EncodedPath.prefixSuccessor(prefixPath); + bindVars[i++] = collection.length() + 1; bindVars[i++] = readTime.getSeconds(); bindVars[i++] = readTime.getSeconds(); bindVars[i++] = readTime.getNanoseconds(); bindVars[i++] = readTime.getSeconds(); bindVars[i++] = readTime.getNanoseconds(); - bindVars[i] = EncodedPath.encode(documentKey.getPath()); + bindVars[i++] = EncodedPath.encode(documentKey.getPath()); } + bindVars[i] = count; - ImmutableSortedMap[] results = - (ImmutableSortedMap[]) - new ImmutableSortedMap[] {DocumentCollections.emptyMutableDocumentMap()}; BackgroundQueue backgroundQueue = new BackgroundQueue(); + Map[] results = + (HashMap[]) (new HashMap[] {new HashMap()}); db.query(sql.toString()) .binding(bindVars) @@ -188,10 +218,8 @@ public ImmutableSortedMap getAllDocumentsMatchingQ () -> { MutableDocument document = decodeMaybeDocument(rawDocument, readTimeSeconds[0], readTimeNanos[0]); - if (document.isFoundDocument() && query.matches(document)) { - synchronized (SQLiteRemoteDocumentCache.this) { - results[0] = results[0].insert(document.getKey(), document); - } + synchronized (SQLiteRemoteDocumentCache.this) { + results[0].put(document.getKey(), document); } }); }); @@ -205,6 +233,26 @@ public ImmutableSortedMap getAllDocumentsMatchingQ return results[0]; } + @Override + public ImmutableSortedMap getAllDocumentsMatchingQuery( + final Query query, IndexOffset offset) { + hardAssert( + !query.isCollectionGroupQuery(), + "CollectionGroup queries should be handled in LocalDocumentsView"); + + Map allDocuments = + getAll(Collections.singletonList(query.getPath()), offset, Integer.MAX_VALUE); + + ImmutableSortedMap matchingDocuments = + DocumentCollections.emptyMutableDocumentMap(); + for (MutableDocument document : allDocuments.values()) { + if (document.isFoundDocument() && query.matches(document)) { + matchingDocuments = matchingDocuments.insert(document.getKey(), document); + } + } + return matchingDocuments; + } + @Override public SnapshotVersion getLatestReadTime() { SnapshotVersion latestReadTime = diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java index 5bdeee337ac..8bf3ae6d6be 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java @@ -49,7 +49,7 @@ class SQLiteSchema { * The version of the schema. Increase this by one for each migration added to runMigrations * below. */ - static final int VERSION = 14; + static final int VERSION = 15; // TODO(indexing): Remove this constant and increment VERSION to enable indexing support static final int INDEXING_SUPPORT_VERSION = VERSION + 1; @@ -181,6 +181,10 @@ void runSchemaUpgrades(int fromVersion, int toVersion) { addPendingDataMigration(Persistence.DATA_MIGRATION_BUILD_OVERLAYS); } + if (fromVersion < 15 && toVersion >= 15) { + ensureReadTime(); + } + /* * Adding a new schema upgrade? READ THIS FIRST! * @@ -654,6 +658,12 @@ private void ensurePathLength() { } while (resultsRemaining[0]); } + /** Initialize the remote_document's read_time column with 0 values if they are not set. */ + private void ensureReadTime() { + db.execSQL( + "UPDATE remote_documents SET read_time_seconds = 0, read_time_nanos = 0 WHERE read_time_seconds IS NULL"); + } + private void createBundleCache() { ifTablesDontExist( new String[] {"bundles", "named_queries"}, diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/FieldIndex.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/FieldIndex.java index b2fe8e806cf..40e465a327b 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/FieldIndex.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/FieldIndex.java @@ -116,6 +116,9 @@ public static IndexState create( /** Stores the latest read time and document that were processed for an index. */ @AutoValue public abstract static class IndexOffset implements Comparable { + public static final Comparator DOCUMENT_COMPARATOR = + (l, r) -> IndexOffset.fromDocument(l).compareTo(IndexOffset.fromDocument(r)); + public static final IndexOffset NONE = create(SnapshotVersion.NONE, DocumentKey.empty()); /** @@ -145,6 +148,11 @@ public static IndexOffset create(SnapshotVersion readTime) { return new AutoValue_FieldIndex_IndexOffset(successor, DocumentKey.empty()); } + /** Creates a new offset based on the provided document. */ + public static IndexOffset fromDocument(Document document) { + return new AutoValue_FieldIndex_IndexOffset(document.getReadTime(), document.getKey()); + } + /** * Returns the latest read time version that has been indexed by Firestore for this field index. */ diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java index 289e6656840..8819511f19c 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java @@ -31,8 +31,10 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Random; import java.util.SortedSet; @@ -366,4 +368,19 @@ private static void diffCollections( private static T advanceIterator(Iterator it) { return it.hasNext() ? it.next() : null; } + + /** Returns a map with the first {#code n} elements of {#code data} when sorted by comp. */ + public static Map firstNEntries(Map data, int n, Comparator comp) { + if (data.size() <= n) { + return data; + } else { + List> sortedVlaues = new ArrayList<>(data.entrySet()); + Collections.sort(sortedVlaues, (l, r) -> comp.compare(l.getValue(), r.getValue())); + Map result = new HashMap<>(); + for (int i = 0; i < n; ++i) { + result.put(sortedVlaues.get(i).getKey(), sortedVlaues.get(i).getValue()); + } + return result; + } + } } diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/CountingQueryEngine.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/CountingQueryEngine.java index babe9bb5d24..a434d2af4ca 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/CountingQueryEngine.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/CountingQueryEngine.java @@ -149,6 +149,14 @@ public Map getAll(Iterable documentKe return result; } + @Override + public Map getAll( + String collectionGroup, IndexOffset offset, int limit) { + Map result = subject.getAll(collectionGroup, offset, limit); + documentsReadByQuery[0] += result.size(); + return result; + } + @Override public ImmutableSortedMap getAllDocumentsMatchingQuery( Query query, IndexOffset offset) { diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexBackfillerTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexBackfillerTest.java index 7a268674c98..983017002e9 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexBackfillerTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexBackfillerTest.java @@ -27,6 +27,7 @@ import com.google.firebase.firestore.core.Target; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.FieldIndex; +import com.google.firebase.firestore.model.FieldIndex.IndexOffset; import com.google.firebase.firestore.model.MutableDocument; import com.google.firebase.firestore.model.SnapshotVersion; import com.google.firebase.firestore.testutil.TestUtil; @@ -123,6 +124,7 @@ public void testBackfillWritesLatestReadTimeToFieldIndexOnCompletion() { @Test public void testBackfillFetchesDocumentsAfterEarliestReadTime() { + addDoc("latest/doc", "foo", version(10)); addFieldIndex("coll1", "foo", version(10)); // Documents before earliest read time should not be fetched. @@ -130,9 +132,9 @@ public void testBackfillFetchesDocumentsAfterEarliestReadTime() { int documentsProcessed = backfiller.backfill(); assertEquals(0, documentsProcessed); - // Read time of index should not change. + // Read time should be the highest read time from the cache. Iterator it = indexManager.getFieldIndexes("coll1").iterator(); - assertEquals(version(10), it.next().getIndexState().getOffset().getReadTime()); + assertEquals(IndexOffset.create(version(10)), it.next().getIndexState().getOffset()); // Documents that are after the earliest read time but before field index read time are fetched. addDoc("coll1/docB", "boo", version(19)); 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 f84e94345a6..95650107423 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 @@ -15,6 +15,7 @@ package com.google.firebase.firestore.local; import static com.google.firebase.firestore.testutil.TestUtil.doc; +import static com.google.firebase.firestore.testutil.TestUtil.docMap; 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.query; @@ -27,7 +28,6 @@ import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MutableDocument; import com.google.firebase.firestore.model.SnapshotVersion; -import java.util.Collections; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -80,10 +80,10 @@ public void combinesIndexedWithNonIndexedResults() { MutableDocument doc3 = doc("coll/c", 3, map("foo", true)); remoteDocuments.add(doc1, doc1.getVersion()); - indexManager.updateIndexEntries(Collections.singletonList(doc1)); + indexManager.updateIndexEntries(docMap(doc1)); remoteDocuments.add(doc2, doc2.getVersion()); - indexManager.updateIndexEntries(Collections.singletonList(doc2)); + indexManager.updateIndexEntries(docMap(doc2)); remoteDocuments.add(doc3, doc3.getVersion()); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/RemoteDocumentCacheTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/RemoteDocumentCacheTestCase.java index b8a72a1b981..18eda812756 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/RemoteDocumentCacheTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/RemoteDocumentCacheTestCase.java @@ -57,7 +57,7 @@ abstract class RemoteDocumentCacheTestCase { private final Map DOC_DATA = map("data", 2); private Persistence persistence; - private RemoteDocumentCache remoteDocumentCache; + protected RemoteDocumentCache remoteDocumentCache; @Before public void setUp() { @@ -264,11 +264,11 @@ public void testLatestReadTime() { assertEquals(version(3), latestReadTime); } - private MutableDocument addTestDocumentAtPath(String path) { + protected MutableDocument addTestDocumentAtPath(String path) { return addTestDocumentAtPath(path, 42, 42); } - private MutableDocument addTestDocumentAtPath(String path, int updateTime, int readTime) { + protected MutableDocument addTestDocumentAtPath(String path, int updateTime, int readTime) { MutableDocument doc = doc(path, updateTime, map("data", 2)); add(doc, version(readTime)); return doc; diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java index 035751b73ca..ab692c0a82c 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java @@ -20,6 +20,7 @@ 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; +import static com.google.firebase.firestore.testutil.TestUtil.docMap; import static com.google.firebase.firestore.testutil.TestUtil.fieldIndex; import static com.google.firebase.firestore.testutil.TestUtil.filter; import static com.google.firebase.firestore.testutil.TestUtil.key; @@ -707,7 +708,7 @@ private void verifySequenceNumber( } private void addDocs(Document... docs) { - indexManager.updateIndexEntries(Arrays.asList(docs)); + indexManager.updateIndexEntries(docMap(docs)); } private void addDoc(String key, Map data) { diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCacheTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCacheTest.java index fa6c473d444..a5f234e0114 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCacheTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCacheTest.java @@ -14,6 +14,15 @@ package com.google.firebase.firestore.local; +import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.firestore.testutil.TestUtil.key; +import static com.google.firebase.firestore.testutil.TestUtil.version; + +import com.google.firebase.firestore.model.DocumentKey; +import com.google.firebase.firestore.model.FieldIndex; +import com.google.firebase.firestore.model.MutableDocument; +import java.util.Map; +import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; @@ -26,4 +35,55 @@ public final class SQLiteRemoteDocumentCacheTest extends RemoteDocumentCacheTest Persistence getPersistence() { return PersistenceTestHelpers.createSQLitePersistence(); } + + @Test + public void testNextDocumentsFromCollectionGroup() { + addTestDocumentAtPath("a/1"); + addTestDocumentAtPath("a/2"); + addTestDocumentAtPath("b/3"); + + Map results = + remoteDocumentCache.getAll("a", FieldIndex.IndexOffset.NONE, Integer.MAX_VALUE); + assertThat(results.keySet()).containsExactly(key("a/1"), key("a/2")); + } + + @Test + public void testNextDocumentsFromCollectionGroupWithLimit() { + addTestDocumentAtPath("a/1", /* updateTime= */ 1, /* readTime= */ 11); + addTestDocumentAtPath("b/2/a/2", /* updateTime= */ 1, /* readTime= */ 12); + addTestDocumentAtPath("a/3", /* updateTime= */ 1, /* readTime= */ 13); + + Map results = + remoteDocumentCache.getAll("a", FieldIndex.IndexOffset.NONE, 2); + assertThat(results.keySet()).containsExactly(key("a/1"), key("b/2/a/2")); + } + + @Test + public void testNextDocumentsFromCollectionGroupWithOffset() { + addTestDocumentAtPath("a/1", /* updateTime= */ 1, /* readTime= */ 11); + addTestDocumentAtPath("b/2/a/2", /* updateTime= */ 2, /* readTime= = */ 12); + addTestDocumentAtPath("a/3", /* updateTime= */ 3, /* readTime= = */ 13); + + Map results = + remoteDocumentCache.getAll("a", FieldIndex.IndexOffset.create(version(11)), 2); + assertThat(results.keySet()).containsExactly(key("b/2/a/2"), key("a/3")); + } + + @Test + public void testNextDocumentsFromNonExistingCollectionGroup() { + Map results = + remoteDocumentCache.getAll("a", FieldIndex.IndexOffset.create(version(11)), 2); + assertThat(results).isEmpty(); + } + + @Test + public void testNextDocumentsForLargeCollectionGroup() { + int size = 999 / SQLiteRemoteDocumentCache.BINDS_PER_STATEMENT + 1; + for (int i = 0; i < size; ++i) { + addTestDocumentAtPath("a/" + i + "/b/doc"); + } + Map results = + remoteDocumentCache.getAll("b", FieldIndex.IndexOffset.NONE, size); + assertThat(results).hasSize(size); + } } diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java index dad8a3eb684..5ca1e2e897f 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java @@ -426,7 +426,7 @@ public void existingDocumentsRemainReadableAfterIndexFreeMigration() { new Object[] {encode(path("coll/existing")), createDummyDocument("coll/existing")}); // Run the index-free migration. - schema.runSchemaUpgrades(8, 10); + schema.runSchemaUpgrades(8, 14); db.execSQL( "INSERT INTO remote_documents (path, read_time_seconds, read_time_nanos, contents) VALUES (?, ?, ?, ?)", new Object[] {encode(path("coll/old")), 0, 1000, createDummyDocument("coll/old")}); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/util/UtilTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/util/UtilTest.java index a28a61ed5b8..6ff424ef994 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/util/UtilTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/util/UtilTest.java @@ -15,6 +15,7 @@ package com.google.firebase.firestore.util; import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.firestore.util.Util.firstNEntries; import static org.junit.Assert.assertEquals; import com.google.firebase.firestore.testutil.TestUtil; @@ -22,7 +23,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -69,6 +72,16 @@ public void testDiffCollectionsWithEmptyLists() { validateDiffCollection(Collections.emptyList(), Collections.emptyList()); } + @Test + public void testFirstNEntries() { + Map data = new HashMap<>(); + data.put(1, 1); + data.put(3, 3); + data.put(2, 2); + data = firstNEntries(data, 2, Integer::compare); + assertThat(data).containsExactly(1, 1, 2, 2); + } + private void validateDiffCollection(List before, List after) { List result = new ArrayList<>(before); Util.diffCollections(before, after, String::compareTo, result::add, result::remove); 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 d50cd65d09b..3a78f06e079 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 @@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.firebase.firestore.model.DocumentCollections.emptyDocumentMap; -import static com.google.firebase.firestore.model.DocumentCollections.emptyMutableDocumentMap; import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -223,10 +222,10 @@ public static MutableDocument unknownDoc(String key, long version) { return MutableDocument.newUnknownDocument(key(key), version(version)); } - public static ImmutableSortedMap docMap( - MutableDocument[] documents) { - ImmutableSortedMap map = emptyMutableDocumentMap(); - for (MutableDocument maybeDocument : documents) { + public static ImmutableSortedMap docMap(T... documents) { + ImmutableSortedMap map = + (ImmutableSortedMap) emptyDocumentMap(); + for (T maybeDocument : documents) { map = map.insert(maybeDocument.getKey(), maybeDocument); } return map;