Address feedbacks other than adding tests and comments

Author: cherylEnkidu

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

@@ -15,7 +15,7 @@
 package com.google.firebase.firestore;
 
 import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testFirestore;
-import static org.junit.Assert.fail;
+import static com.google.firebase.firestore.testutil.TestUtil.assertDoesNotThrow;
 
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import com.google.android.gms.tasks.Task;
@@ -125,12 +125,4 @@ public void testAutomaticIndexingSetSuccessfullyUseDefault() {
     assertDoesNotThrow(
         () -> db.getPersistentCacheIndexManager().setAutomaticIndexingEnabled(false));
   }
-
-  public void assertDoesNotThrow(Runnable block) {
-    try {
-      block.run();
-    } catch (Exception e) {
-      fail("Should not have thrown " + e);
-    }
-  }
 }

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

@@ -260,14 +260,14 @@ void recalculateAndSaveOverlays(Set<DocumentKey> documentKeys) {
    * @param offset Read time and key to start scanning by (exclusive).
    */
   ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
-      Query query, IndexOffset offset, @Nullable QueryContext counter) {
+      Query query, IndexOffset offset, @Nullable QueryContext context) {
     ResourcePath path = query.getPath();
     if (query.isDocumentQuery()) {
       return getDocumentsMatchingDocumentQuery(path);
     } else if (query.isCollectionGroupQuery()) {
-      return getDocumentsMatchingCollectionGroupQuery(query, offset, counter);
+      return getDocumentsMatchingCollectionGroupQuery(query, offset, context);
     } else {
-      return getDocumentsMatchingCollectionQuery(query, offset, counter);
+      return getDocumentsMatchingCollectionQuery(query, offset, context);
     }
   }
 
@@ -289,7 +289,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingDocumentQu
   }
 
   private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionGroupQuery(
-      Query query, IndexOffset offset, @Nullable QueryContext counter) {
+      Query query, IndexOffset offset, @Nullable QueryContext context) {
     hardAssert(
         query.getPath().isEmpty(),
         "Currently we only support collection group queries at the root.");
@@ -302,7 +302,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
     for (ResourcePath parent : parents) {
       Query collectionQuery = query.asCollectionQueryAtPath(parent.append(collectionId));
       ImmutableSortedMap<DocumentKey, Document> collectionResults =
-          getDocumentsMatchingCollectionQuery(collectionQuery, offset, counter);
+          getDocumentsMatchingCollectionQuery(collectionQuery, offset, context);
       for (Map.Entry<DocumentKey, Document> docEntry : collectionResults) {
         results = results.insert(docEntry.getKey(), docEntry.getValue());
       }
@@ -367,11 +367,11 @@ private void populateOverlays(Map<DocumentKey, Overlay> overlays, Set<DocumentKe
   }
 
   private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionQuery(
-      Query query, IndexOffset offset, @Nullable QueryContext counter) {
+      Query query, IndexOffset offset, @Nullable QueryContext context) {
     Map<DocumentKey, Overlay> overlays =
         documentOverlayCache.getOverlays(query.getPath(), offset.getLargestBatchId());
     Map<DocumentKey, MutableDocument> remoteDocuments =
-        remoteDocumentCache.getDocumentsMatchingQuery(query, offset, overlays.keySet(), counter);
+        remoteDocumentCache.getDocumentsMatchingQuery(query, offset, overlays.keySet(), context);
 
     // As documents might match the query because of their overlay we need to include documents
     // for all overlays in the initial document set.

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

@@ -101,7 +101,7 @@ public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
       Query query,
       IndexOffset offset,
       @Nonnull Set<DocumentKey> mutatedKeys,
-      @Nullable QueryContext counter) {
+      @Nullable QueryContext context) {
     Map<DocumentKey, MutableDocument> result = new HashMap<>();
 
     // Documents are ordered by key, so we can use a prefix scan to narrow down the documents

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

@@ -17,13 +17,13 @@
 public class QueryContext {
   public QueryContext() {}
 
-  private int documentCount = 0;
+  private int documentReadCount = 0;
 
-  public int getDocumentCount() {
-    return documentCount;
+  public int getDocumentReadCount() {
+    return documentReadCount;
   }
 
   public void increaseDocumentCount() {
-    documentCount++;
+    documentReadCount++;
   }
 }

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

@@ -93,18 +93,33 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
       return result;
     }
 
-    QueryContext counter = new QueryContext();
-    result = executeFullCollectionScan(query, counter);
+    QueryContext context = new QueryContext();
+    result = executeFullCollectionScan(query, context);
     if (result != null && automaticIndexingEnabled) {
-      CreateCacheIndices(query, counter, result.size());
+      CreateCacheIndexes(query, context, result.size());
     }
     return result;
   }
 
   // TODO(csi): Auto experiment data.
-  private void CreateCacheIndices(Query query, QueryContext counter, int resultSize) {
-    if (counter.getDocumentCount() > 2 * resultSize) {
+  private void CreateCacheIndexes(Query query, QueryContext context, int resultSize) {
+    String decisionStr = "";
+    if (context.getDocumentReadCount() > 2 * resultSize) {
       indexManager.createTargetIndices(query.toTarget());
+    } else {
+      decisionStr = " not";
+    }
+
+    if (Logger.isDebugEnabled()) {
+      Logger.debug(
+          LOG_TAG,
+          "Query ran locally using a full collection scan, walking through %s documents in total "
+              + "and returning %s documents. The SDK has decided%s to create cache indexes "
+              + "for this query, as using cache indexes may%s help improve performance.",
+          context.getDocumentReadCount(),
+          resultSize,
+          decisionStr,
+          decisionStr);
     }
   }
 
@@ -260,11 +275,11 @@ private boolean needsRefill(
   }
 
   private ImmutableSortedMap<DocumentKey, Document> executeFullCollectionScan(
-      Query query, QueryContext counter) {
+      Query query, QueryContext context) {
     if (Logger.isDebugEnabled()) {
       Logger.debug(LOG_TAG, "Using full collection scan to execute query: %s", query.toString());
     }
-    return localDocumentsView.getDocumentsMatchingQuery(query, IndexOffset.NONE, counter);
+    return localDocumentsView.getDocumentsMatchingQuery(query, IndexOffset.NONE, context);
   }
 
   /**

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

@@ -95,5 +95,5 @@ Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
       Query query,
       IndexOffset offset,
       @Nonnull Set<DocumentKey> mutatedKeys,
-      @Nullable QueryContext counter);
+      @Nullable QueryContext context);
 }

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

@@ -186,7 +186,7 @@ private Map<DocumentKey, MutableDocument> getAll(
       IndexOffset offset,
       int count,
       @Nullable Function<MutableDocument, Boolean> filter,
-      @Nullable QueryContext counter) {
+      @Nullable QueryContext context) {
     Timestamp readTime = offset.getReadTime().getTimestamp();
     DocumentKey documentKey = offset.getDocumentKey();
 
@@ -225,8 +225,8 @@ private Map<DocumentKey, MutableDocument> getAll(
         .forEach(
             row -> {
               processRowInBackground(backgroundQueue, results, row, filter);
-              if (counter != null) {
-                counter.increaseDocumentCount();
+              if (context != null) {
+                context.increaseDocumentCount();
               }
             });
     backgroundQueue.drain();
@@ -276,13 +276,13 @@ public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
       Query query,
       IndexOffset offset,
       @Nonnull Set<DocumentKey> mutatedKeys,
-      @Nullable QueryContext counter) {
+      @Nullable QueryContext context) {
     return getAll(
         Collections.singletonList(query.getPath()),
         offset,
         Integer.MAX_VALUE,
         (MutableDocument doc) -> query.matches(doc) || mutatedKeys.contains(doc.getKey()),
-        counter);
+        context);
   }
 
   private MutableDocument decodeMaybeDocument(

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

@@ -179,7 +179,7 @@ public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
           Query query,
           IndexOffset offset,
           @NonNull Set<DocumentKey> mutatedKeys,
-          @Nullable QueryContext counter) {
+          @Nullable QueryContext context) {
         Map<DocumentKey, MutableDocument> result =
             subject.getDocumentsMatchingQuery(query, offset, mutatedKeys);
         documentsReadByCollection[0] += result.size();