Review

Author: schmidt-sebastian

firebase-crashlytics-ndk/src/third_party/crashpad

@@ -177,7 +177,7 @@ private IndexOffset getExistingOffset(Collection<FieldIndex> fieldIndexes) {
   private IndexOffset getNewOffset(List<Document> documents, IndexOffset currentOffset) {
     IndexOffset latestOffset =
         documents.isEmpty()
-            ? IndexOffset.NONE
+            ? IndexOffset.create(remoteDocumentCache.getLatestReadTime())
             : IndexOffset.create(
                 documents.get(documents.size() - 1).getReadTime(),
                 documents.get(documents.size() - 1).getKey());

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

@@ -130,6 +130,11 @@ public static IndexOffset create(SnapshotVersion readTime, DocumentKey documentK
      * Creates an offset that matches all documents with a read time higher than {@code readTime}.
      */
     public static IndexOffset create(SnapshotVersion readTime) {
+      // We want to create an offset that matches all documents with a read time greater than
+      // the provided read time. To do so, we technically need to create an offset for
+      // `(readTime, MAX_DOCUMENT_KEY)`. While we could use Unicode codepoints to generate
+      // MAX_DOCUMENT_KEY, it is much easier to use `(readTime + 1, DocumentKey.empty())` since
+      // `> DocumentKey.empty()` matches all valid document IDs.
       long successorSeconds = readTime.getTimestamp().getSeconds();
       int successorNanos = readTime.getTimestamp().getNanoseconds() + 1;
       SnapshotVersion successor =

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

@@ -188,13 +188,13 @@ public void testBackfillUsesDocumentKeyOffsetForLargeSnapshots() {
     addDoc("coll1/docB", "foo", version(1));
     addDoc("coll1/docC", "foo", version(1));
 
-    IndexBackfiller.Results results = backfiller.backfill();
-    assertEquals(2, results.getDocumentsProcessed());
+    int documentsProcessed = backfiller.backfill();
+    assertEquals(2, documentsProcessed);
 
     verifyQueryResults("coll1", "coll1/docA", "coll1/docB");
 
-    results = backfiller.backfill();
-    assertEquals(1, results.getDocumentsProcessed());
+    documentsProcessed = backfiller.backfill();
+    assertEquals(1, documentsProcessed);
 
     verifyQueryResults("coll1", "coll1/docA", "coll1/docB", "coll1/docC");
   }
@@ -261,6 +261,21 @@ public void testBackfillWritesUntilCap() {
     verifyQueryResults("coll2", "coll2/docA");
   }
 
+  @Test
+  public void testBackfillUsesLatestReadTimeForEmptyCollections() {
+    addFieldIndex("coll", "foo", version(1));
+    addDoc("readtime/doc", "foo", version(2));
+
+    int documentsProcessed = backfiller.backfill();
+    assertEquals(0, documentsProcessed);
+
+    addDoc("coll/ignored", "foo", version(2));
+    addDoc("coll/added", "foo", version(3));
+
+    documentsProcessed = backfiller.backfill();
+    assertEquals(1, documentsProcessed);
+  }
+
   private void addFieldIndex(String collectionGroup, String fieldName) {
     FieldIndex fieldIndex =
         fieldIndex(collectionGroup, fieldName, FieldIndex.Segment.Kind.ASCENDING);

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

@@ -17,9 +17,11 @@
 import static com.google.firebase.firestore.model.FieldIndex.IndexState;
 import static com.google.firebase.firestore.model.FieldIndex.SEMANTIC_COMPARATOR;
 import static com.google.firebase.firestore.testutil.TestUtil.fieldIndex;
+import static com.google.firebase.firestore.testutil.TestUtil.key;
 import static com.google.firebase.firestore.testutil.TestUtil.version;
 import static org.junit.Assert.assertEquals;
 
+import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
@@ -89,4 +91,26 @@ public void comparatorIncludesSegmentsLength() {
     assertEquals(0, SEMANTIC_COMPARATOR.compare(indexOriginal, indexSame));
     assertEquals(-1, SEMANTIC_COMPARATOR.compare(indexOriginal, indexDifferent));
   }
+
+  @Test
+  public void indexOffsetComparator() {
+    IndexOffset docAOffset = IndexOffset.create(version(1), key("foo/a"));
+    IndexOffset docBOffset = IndexOffset.create(version(1), key("foo/b"));
+    IndexOffset version1Offset = IndexOffset.create(version(1));
+    IndexOffset docCOffset = IndexOffset.create(version(2), key("foo/c"));
+    IndexOffset version2Offset = IndexOffset.create(version(2));
+
+    assertEquals(-1, docAOffset.compareTo(docBOffset));
+    assertEquals(-1, docAOffset.compareTo(version1Offset));
+    assertEquals(-1, version1Offset.compareTo(docCOffset));
+    assertEquals(-1, version1Offset.compareTo(version2Offset));
+    assertEquals(-1, docCOffset.compareTo(version2Offset));
+  }
+
+  @Test
+  public void indexOffsetAdvancesSeconds() {
+    IndexOffset actualSuccessor = IndexOffset.create(version(1, (int) 1e9 - 1));
+    IndexOffset expectedSuccessor = IndexOffset.create(version(2, 0), DocumentKey.empty());
+    assertEquals(expectedSuccessor, actualSuccessor);
+  }
 }