Merge remote-tracking branch 'origin/master' into m1-mac-fixes

Author: tom-andersen

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java

@@ -49,6 +49,7 @@
 import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 class CrashlyticsController {
@@ -241,6 +242,8 @@ public Task<Void> then(@Nullable Settings settings) throws Exception {
     try {
       // TODO(mrober): Don't block the main thread ever for on-demand fatals.
       Utils.awaitEvenIfOnMainThread(handleUncaughtExceptionTask);
+    } catch (TimeoutException e) {
+      Logger.getLogger().e("Cannot send reports. Timed out while fetching settings.");
     } catch (Exception e) {
       Logger.getLogger().e("Error handling uncaught exception", e);
       // Nothing to do in this case.

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/Utils.java

@@ -16,7 +16,6 @@
 
 import static java.util.Objects.requireNonNull;
 
-import android.os.Looper;
 import androidx.annotation.NonNull;
 import com.google.android.gms.tasks.Continuation;
 import com.google.android.gms.tasks.Task;
@@ -122,11 +121,7 @@ public static <T> T awaitEvenIfOnMainThread(Task<T> task)
           return null;
         });
 
-    if (Looper.getMainLooper() == Looper.myLooper()) {
-      latch.await(CrashlyticsCore.DEFAULT_MAIN_HANDLER_TIMEOUT_SEC, TimeUnit.SECONDS);
-    } else {
-      latch.await();
-    }
+    latch.await(CrashlyticsCore.DEFAULT_MAIN_HANDLER_TIMEOUT_SEC, TimeUnit.SECONDS);
 
     if (task.isSuccessful()) {
       return task.getResult();

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

@@ -41,7 +41,7 @@
  * the result set is equivalent across all implementations.
  *
  * <p>The Query engine will use indexed-based execution if a user has configured any index that can
- * be used to execute query (via {@link FirebaseFirestore#setIndexConfiguation}). Otherwise, the
+ * be used to execute query (via {@link FirebaseFirestore#setIndexConfiguration}). Otherwise, the
  * engine will try to optimize the query by re-using a previously persisted query result. If that is
  * not possible, the query will be executed via a full collection scan.
  *
@@ -86,8 +86,7 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
       ImmutableSortedSet<DocumentKey> remoteKeys) {
     hardAssert(initialized, "initialize() not called");
 
-    ImmutableSortedMap<DocumentKey, Document> result =
-        performQueryUsingIndex(query, query.toTarget());
+    ImmutableSortedMap<DocumentKey, Document> result = performQueryUsingIndex(query);
     if (result != null) {
       return result;
     }
@@ -104,13 +103,13 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
    * Performs an indexed query that evaluates the query based on a collection's persisted index
    * values. Returns {@code null} if an index is not available.
    */
-  private @Nullable ImmutableSortedMap<DocumentKey, Document> performQueryUsingIndex(
-      Query query, Target target) {
+  private @Nullable ImmutableSortedMap<DocumentKey, Document> performQueryUsingIndex(Query query) {
     if (query.matchesAllDocuments()) {
       // Don't use index queries that can be executed by scanning the collection.
       return null;
     }
 
+    Target target = query.toTarget();
     IndexType indexType = indexManager.getIndexType(target);
 
     if (indexType.equals(IndexType.NONE)) {

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

@@ -23,7 +23,6 @@
 import static java.lang.Math.max;
 
 import android.text.TextUtils;
-import android.util.Pair;
 import androidx.annotation.Nullable;
 import com.google.firebase.Timestamp;
 import com.google.firebase.database.collection.ImmutableSortedMap;
@@ -315,13 +314,12 @@ public IndexOffset getMinOffset(String collectionGroup) {
   @Override
   public IndexType getIndexType(Target target) {
     for (Target subTarget : getSubTargets(target)) {
-      Pair<FieldIndex, Integer> indexAndSegmentCount = getFieldIndexAndSegmentCount(subTarget);
-      if (indexAndSegmentCount == null) {
+      FieldIndex index = getFieldIndex(subTarget);
+      if (index == null) {
         return IndexType.NONE;
       }
 
-      Integer indexSegmentCount = indexAndSegmentCount.second;
-      if (indexSegmentCount < subTarget.getSegmentCount()) {
+      if (index.getSegments().size() < subTarget.getSegmentCount()) {
         return IndexType.PARTIAL;
       }
     }
@@ -332,9 +330,9 @@ public IndexType getIndexType(Target target) {
   public IndexOffset getMinOffset(Target target) {
     List<FieldIndex> fieldIndexes = new ArrayList<>();
     for (Target subTarget : getSubTargets(target)) {
-      Pair<FieldIndex, Integer> indexAndSegmentCount = getFieldIndexAndSegmentCount(subTarget);
-      if (indexAndSegmentCount != null) {
-        fieldIndexes.add(indexAndSegmentCount.first);
+      FieldIndex index = getFieldIndex(subTarget);
+      if (index != null) {
+        fieldIndexes.add(index);
       }
     }
     return getMinOffset(fieldIndexes);
@@ -468,12 +466,11 @@ public List<DocumentKey> getDocumentsMatchingTarget(Target target) {
     List<Object> bindings = new ArrayList<>();
 
     for (Target subTarget : getSubTargets(target)) {
-      Pair<FieldIndex, Integer> indexAndSegmentCount = getFieldIndexAndSegmentCount(subTarget);
-      if (indexAndSegmentCount == null) {
+      FieldIndex fieldIndex = getFieldIndex(subTarget);
+      if (fieldIndex == null) {
         return null;
       }
 
-      FieldIndex fieldIndex = indexAndSegmentCount.first;
       @Nullable List<Value> arrayValues = subTarget.getArrayValues(fieldIndex);
       @Nullable Collection<Value> notInValues = subTarget.getNotInValues(fieldIndex);
       @Nullable Bound lowerBound = subTarget.getLowerBound(fieldIndex);
@@ -627,12 +624,11 @@ private Object[] fillBounds(
   }
 
   /**
-   * Returns an index that can be used to serve the provided target, as well as the number of index
-   * segments that matched the target filters and orderBys. Returns {@code null} if no index is
-   * configured.
+   * Returns an index that can be used to serve the provided target. Returns {@code null} if no
+   * index is configured.
    */
   @Nullable
-  public Pair<FieldIndex, Integer> getFieldIndexAndSegmentCount(Target target) {
+  private FieldIndex getFieldIndex(Target target) {
     hardAssert(started, "IndexManager not started");
 
     TargetIndexMatcher targetIndexMatcher = new TargetIndexMatcher(target);
@@ -647,19 +643,16 @@ public Pair<FieldIndex, Integer> getFieldIndexAndSegmentCount(Target target) {
     }
 
     // Return the index with the most number of segments.
-    int maxNumMatchingSegments = -1;
     FieldIndex matchingIndex = null;
     for (FieldIndex fieldIndex : collectionIndexes) {
-      int numMatchingSegments = targetIndexMatcher.servedByIndex(fieldIndex);
-      if (numMatchingSegments > maxNumMatchingSegments) {
-        maxNumMatchingSegments = numMatchingSegments;
+      boolean matches = targetIndexMatcher.servedByIndex(fieldIndex);
+      if (matches
+          && (matchingIndex == null
+              || fieldIndex.getSegments().size() > matchingIndex.getSegments().size())) {
         matchingIndex = fieldIndex;
       }
     }
-    if (maxNumMatchingSegments == -1) {
-      return null;
-    }
-    return new Pair<>(matchingIndex, maxNumMatchingSegments);
+    return matchingIndex;
   }
 
   /**

firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java

@@ -111,8 +111,7 @@ public TargetIndexMatcher(Target target) {
   }
 
   /**
-   * Returns the number of index segments that match the TargetIndexMatcher's target. Returns -1 if
-   * the index cannot be used to serve the target.
+   * Returns whether the index can be used to serve the TargetIndexMatcher's target.
    *
    * <p>An index is considered capable of serving the target when:
    *
@@ -132,26 +131,21 @@ public TargetIndexMatcher(Target target) {
    *
    * @throws AssertionError if the index is for a different collection
    */
-  public int servedByIndex(FieldIndex index) {
+  public boolean servedByIndex(FieldIndex index) {
     hardAssert(index.getCollectionGroup().equals(collectionId), "Collection IDs do not match");
-    int numMatchingSegments = 0;
 
     // If there is an array element, find a matching filter.
     FieldIndex.Segment arraySegment = index.getArraySegment();
-    if (arraySegment != null) {
-      if (hasMatchingEqualityFilter(arraySegment)) {
-        ++numMatchingSegments;
-      } else {
-        return -1;
-      }
+    if (arraySegment != null && !hasMatchingEqualityFilter(arraySegment)) {
+      return false;
     }
 
     Iterator<OrderBy> orderBys = this.orderBys.iterator();
     List<FieldIndex.Segment> segments = index.getDirectionalSegments();
     int segmentIndex = 0;
 
     // Process all equalities first. Equalities can appear out of order.
-    for (; segmentIndex < segments.size(); ++segmentIndex, ++numMatchingSegments) {
+    for (; segmentIndex < segments.size(); ++segmentIndex) {
       // We attempt to greedily match all segments to equality filters. If a filter matches an
       // index segment, we can mark the segment as used. Since it is not possible to use the same
       // field path in both an equality and inequality/oderBy clause, we do not have to consider the
@@ -168,29 +162,28 @@ public int servedByIndex(FieldIndex index) {
     // filters and we do not need to map any segments to the target's inequality and orderBy
     // clauses.
     if (segmentIndex == segments.size()) {
-      return numMatchingSegments;
+      return true;
     }
 
     // If there is an inequality filter, the next segment must match both the filter and the first
     // orderBy clause.
     if (inequalityFilter != null) {
       FieldIndex.Segment segment = segments.get(segmentIndex);
       if (!matchesFilter(inequalityFilter, segment) || !matchesOrderBy(orderBys.next(), segment)) {
-        return -1;
+        return false;
       }
       ++segmentIndex;
-      ++numMatchingSegments;
     }
 
     // All remaining segments need to represent the prefix of the target's orderBy.
-    for (; segmentIndex < segments.size(); ++segmentIndex, ++numMatchingSegments) {
+    for (; segmentIndex < segments.size(); ++segmentIndex) {
       FieldIndex.Segment segment = segments.get(segmentIndex);
       if (!orderBys.hasNext() || !matchesOrderBy(orderBys.next(), segment)) {
-        return -1;
+        return false;
       }
     }
 
-    return numMatchingSegments;
+    return true;
   }
 
   private boolean hasMatchingEqualityFilter(FieldIndex.Segment segment) {

firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java

@@ -21,7 +21,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.orderBy;
 import static com.google.firebase.firestore.testutil.TestUtil.path;
 import static com.google.firebase.firestore.testutil.TestUtil.query;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import com.google.firebase.firestore.core.Query;
@@ -569,13 +569,13 @@ private void validateServesTarget(
       Query query, String field, FieldIndex.Segment.Kind kind, Object... fieldsAndKind) {
     FieldIndex expectedIndex = fieldIndex("collId", field, kind, fieldsAndKind);
     TargetIndexMatcher targetIndexMatcher = new TargetIndexMatcher(query.toTarget());
-    assertTrue(targetIndexMatcher.servedByIndex(expectedIndex) > 0);
+    assertTrue(targetIndexMatcher.servedByIndex(expectedIndex));
   }
 
   private void validateDoesNotServeTarget(
       Query query, String field, FieldIndex.Segment.Kind kind, Object... fieldsAndKind) {
     FieldIndex expectedIndex = fieldIndex("collId", field, kind, fieldsAndKind);
     TargetIndexMatcher targetIndexMatcher = new TargetIndexMatcher(query.toTarget());
-    assertEquals(targetIndexMatcher.servedByIndex(expectedIndex), -1);
+    assertFalse(targetIndexMatcher.servedByIndex(expectedIndex));
   }
 }