Simplify

Author: schmidt-sebastian

firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryRemoteDocumentCache.java

@@ -95,12 +95,15 @@ public Map<DocumentKey, MutableDocument> getAll(
       ResourcePath documentParent = collectionParent.append(collectionGroup);
       Iterator<Map.Entry<DocumentKey, MutableDocument>> iterator =
           docs.iteratorFrom(DocumentKey.fromPath(documentParent.append("")));
+
       while (iterator.hasNext()) {
         Map.Entry<DocumentKey, MutableDocument> entry = iterator.next();
         DocumentKey key = entry.getKey();
+
         if (!documentParent.isPrefixOf(key.getPath())) {
           break;
         }
+
         if (IndexOffset.fromDocument(entry.getValue()).compareTo(offset) > 0) {
           allDocuments.add(entry.getValue());
         }

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java

@@ -41,7 +41,7 @@
 
 final class SQLiteRemoteDocumentCache implements RemoteDocumentCache {
   /** The number of bind args per collection group in {@link #getAll(String, IndexOffset, int)} */
-  @VisibleForTesting static final int GET_ALL_BINDS_PER_STATEMENT = 8;
+  @VisibleForTesting static final int BINDS_PER_STATEMENT = 9;
 
   private final SQLitePersistence db;
   private final LocalSerializer serializer;
@@ -83,14 +83,12 @@ public void add(MutableDocument document, SnapshotVersion readTime) {
   @Override
   public void remove(DocumentKey documentKey) {
     String path = EncodedPath.encode(documentKey.getPath());
-
     db.execute("DELETE FROM remote_documents WHERE path = ?", path);
   }
 
   @Override
   public MutableDocument get(DocumentKey documentKey) {
     String path = EncodedPath.encode(documentKey.getPath());
-
     MutableDocument document =
         db.query(
                 "SELECT contents, read_time_seconds, read_time_nanos "
@@ -145,12 +143,12 @@ public Map<DocumentKey, MutableDocument> getAll(
 
     if (collections.isEmpty()) {
       return Collections.emptyMap();
-    } else if (GET_ALL_BINDS_PER_STATEMENT * collections.size() < SQLitePersistence.MAX_ARGS) {
+    } else if (BINDS_PER_STATEMENT * collections.size() < SQLitePersistence.MAX_ARGS) {
       return getAll(collections, offset, count);
     } else {
       // We need to fan out our collection scan since SQLite only supports 999 binds per statement.
       Map<DocumentKey, MutableDocument> results = new HashMap<>();
-      int pageSize = SQLitePersistence.MAX_ARGS / GET_ALL_BINDS_PER_STATEMENT;
+      int pageSize = SQLitePersistence.MAX_ARGS / BINDS_PER_STATEMENT;
       for (int i = 0; i < collections.size(); i += pageSize) {
         results.putAll(
             getAll(
@@ -168,26 +166,25 @@ private Map<DocumentKey, MutableDocument> getAll(
     Timestamp readTime = offset.getReadTime().getTimestamp();
     DocumentKey documentKey = offset.getDocumentKey();
 
-    // TODO(indexing): Add path_length
-
     StringBuilder sql =
         repeatSequence(
             "SELECT contents, read_time_seconds, read_time_nanos, path "
                 + "FROM remote_documents "
-                + "WHERE path >= ? AND path < ? "
+                + "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[GET_ALL_BINDS_PER_STATEMENT * collections.size() + 1];
+    Object[] bindVars = new Object[BINDS_PER_STATEMENT * collections.size() + 1];
     int i = 0;
-    for (ResourcePath collectionParent : collections) {
-      String prefixPath = EncodedPath.encode(collectionParent);
+    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();
@@ -236,73 +233,17 @@ public ImmutableSortedMap<DocumentKey, MutableDocument> getAllDocumentsMatchingQ
         !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 = ?");
-
-    Object[] bindVars = new Object[3 + (IndexOffset.NONE.equals(offset) ? 0 : 6)];
-
-    String prefix = EncodedPath.encode(query.getPath());
-
-    int i = 0;
-    bindVars[i++] = prefix;
-    bindVars[i++] = EncodedPath.prefixSuccessor(prefix);
-    bindVars[i++] = query.getPath().length() + 1;
-
-    if (!IndexOffset.NONE.equals(offset)) {
-      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 > ?))");
-      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());
-    }
-
-    ImmutableSortedMap<DocumentKey, MutableDocument>[] results =
-        (ImmutableSortedMap<DocumentKey, MutableDocument>[])
-            new ImmutableSortedMap[] {DocumentCollections.emptyMutableDocumentMap()};
-    BackgroundQueue backgroundQueue = new BackgroundQueue();
-
-    db.query(sql.toString())
-        .binding(bindVars)
-        .forEach(
-            row -> {
-              // Store row values in array entries to provide the correct context inside the
-              // executor.
-              final byte[] rawDocument = row.getBlob(0);
-              final int[] readTimeSeconds = {row.getInt(1)};
-              final int[] readTimeNanos = {row.getInt(2)};
-
-              // Since scheduling background tasks incurs overhead, we only dispatch to a
-              // background thread if there are still some documents remaining.
-              Executor executor = row.isLast() ? Executors.DIRECT_EXECUTOR : backgroundQueue;
-              executor.execute(
-                  () -> {
-                    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);
-                      }
-                    }
-                  });
-            });
-
-    try {
-      backgroundQueue.drain();
-    } catch (InterruptedException e) {
-      fail("Interrupted while deserializing documents", e);
+    Map<DocumentKey, MutableDocument> allDocuments =
+        getAll(Collections.singletonList(query.getPath()), offset, Integer.MAX_VALUE);
+    ImmutableSortedMap<DocumentKey, MutableDocument> matchingDocuments =
+        DocumentCollections.emptyMutableDocumentMap();
+    for (MutableDocument document : allDocuments.values()) {
+      if (document.isFoundDocument() && query.matches(document)) {
+        matchingDocuments = matchingDocuments.insert(document.getKey(), document);
+      }
     }
 
-    return results[0];
+    return matchingDocuments;
   }
 
   @Override

firebase-firestore/src/test/java/com/google/firebase/firestore/local/RemoteDocumentCacheTestCase.java

@@ -288,7 +288,7 @@ public void testNextDocumentsForNonExistingCollectionGroup() {
 
   @Test
   public void testNextDocumentsForLargeCollectionGroup() {
-    int size = 999 / SQLiteRemoteDocumentCache.GET_ALL_BINDS_PER_STATEMENT + 1;
+    int size = 999 / SQLiteRemoteDocumentCache.BINDS_PER_STATEMENT + 1;
     for (int i = 0; i < size; ++i) {
       addTestDocumentAtPath("a/" + i + "/b/doc");
     }