Skip to content

Commit 28ac1cb

Browse files
committed
Reuse existing code for finding least recent index offset.
1 parent 893a014 commit 28ac1cb

File tree

5 files changed

+35
-34
lines changed

5 files changed

+35
-34
lines changed

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

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import com.google.firebase.firestore.util.Logger;
2727
import java.util.Collection;
2828
import java.util.HashSet;
29-
import java.util.Iterator;
3029
import java.util.Map;
3130
import java.util.Set;
3231
import java.util.concurrent.TimeUnit;
@@ -133,7 +132,8 @@ private int writeEntriesForCollectionGroup(
133132
String collectionGroup, int documentsRemainingUnderCap) {
134133
// Use the earliest offset of all field indexes to query the local cache.
135134
Collection<FieldIndex> fieldIndexes = indexManager.getFieldIndexes(collectionGroup);
136-
IndexOffset existingOffset = getExistingOffset(fieldIndexes);
135+
hardAssert(!fieldIndexes.isEmpty(), "Updating collection without indexes");
136+
IndexOffset existingOffset = indexManager.getLeastRecentIndexOffset(fieldIndexes);
137137

138138
LocalDocumentsResult nextBatch =
139139
localDocumentsView.getNextDocuments(
@@ -161,24 +161,6 @@ private IndexOffset getNewOffset(IndexOffset existingOffset, LocalDocumentsResul
161161
Math.max(lookupResult.getBatchId(), existingOffset.getLargestBatchId()));
162162
}
163163

164-
/** Returns the lowest offset for the provided index group. */
165-
private IndexOffset getExistingOffset(Collection<FieldIndex> fieldIndexes) {
166-
hardAssert(!fieldIndexes.isEmpty(), "Updating collection without indexes");
167-
168-
Iterator<FieldIndex> it = fieldIndexes.iterator();
169-
IndexOffset minOffset = it.next().getIndexState().getOffset();
170-
int minBatchId = minOffset.getLargestBatchId();
171-
while (it.hasNext()) {
172-
IndexOffset newOffset = it.next().getIndexState().getOffset();
173-
if (newOffset.compareTo(minOffset) < 0) {
174-
minOffset = newOffset;
175-
}
176-
minBatchId = Math.max(newOffset.getLargestBatchId(), minBatchId);
177-
}
178-
179-
return IndexOffset.create(minOffset.getReadTime(), minOffset.getDocumentKey(), minBatchId);
180-
}
181-
182164
@VisibleForTesting
183165
void setMaxDocumentsToProcess(int newMax) {
184166
maxDocumentsToProcess = newMax;

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,13 @@ public interface IndexManager {
7979

8080
/**
8181
* Iterates over all field indexes that are used to serve the given target, and returns the least
82-
* recent offset of them all.
82+
* recent offset of them all. Asserts that the target can be served from index.
8383
*/
8484
FieldIndex.IndexOffset getLeastRecentIndexOffset(Target target);
8585

86+
/** Returns the lowest offset for the provided index group. */
87+
FieldIndex.IndexOffset getLeastRecentIndexOffset(Collection<FieldIndex> fieldIndexes);
88+
8689
/**
8790
* Returns an index that can be used to serve the provided target. Returns {@code null} if no
8891
* index is configured.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.firebase.firestore.core.Target;
2424
import com.google.firebase.firestore.model.Document;
2525
import com.google.firebase.firestore.model.DocumentKey;
26+
import com.google.firebase.firestore.model.FieldIndex;
2627
import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
2728
import com.google.firebase.firestore.model.SnapshotVersion;
2829
import com.google.firebase.firestore.util.Logger;

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

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -296,22 +296,37 @@ public boolean canServeFromIndex(Target target) {
296296
return true;
297297
}
298298

299-
// TODO(orquery): This looks awfully like `getExistingOffset` in IndexBackfillter. Refactor.
299+
@Override
300+
public FieldIndex.IndexOffset getLeastRecentIndexOffset(Collection<FieldIndex> fieldIndexes) {
301+
hardAssert(
302+
!fieldIndexes.isEmpty(),
303+
"Found empty index group when looking for least recent index offset.");
304+
305+
Iterator<FieldIndex> it = fieldIndexes.iterator();
306+
FieldIndex.IndexOffset minOffset = it.next().getIndexState().getOffset();
307+
int minBatchId = minOffset.getLargestBatchId();
308+
while (it.hasNext()) {
309+
FieldIndex.IndexOffset newOffset = it.next().getIndexState().getOffset();
310+
if (newOffset.compareTo(minOffset) < 0) {
311+
minOffset = newOffset;
312+
}
313+
minBatchId = Math.max(newOffset.getLargestBatchId(), minBatchId);
314+
}
315+
316+
return FieldIndex.IndexOffset.create(
317+
minOffset.getReadTime(), minOffset.getDocumentKey(), minBatchId);
318+
}
319+
300320
@Override
301321
public FieldIndex.IndexOffset getLeastRecentIndexOffset(Target target) {
302-
FieldIndex.IndexOffset min = null;
322+
hardAssert(
323+
canServeFromIndex(target),
324+
"Cannot find least recent index offset if target cannot be served from index.");
325+
List<FieldIndex> fieldIndexes = new ArrayList<>();
303326
for (Target subTarget : getSubTargets(target)) {
304-
FieldIndex index = getFieldIndex(subTarget);
305-
if (index == null) {
306-
// If any of this target's sub-queries does not have a field index, return NONE.
307-
return FieldIndex.IndexOffset.NONE;
308-
}
309-
FieldIndex.IndexOffset candidate = index.getIndexState().getOffset();
310-
if (min == null || candidate.compareTo(min) < 0) {
311-
min = candidate;
312-
}
327+
fieldIndexes.add(getFieldIndex(subTarget));
313328
}
314-
return min == null ? FieldIndex.IndexOffset.NONE : min;
329+
return getLeastRecentIndexOffset(fieldIndexes);
315330
}
316331

317332
private List<Target> getSubTargets(Target target) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ private void verifyQueryResults(Query query, String... expectedKeys) {
527527
if (indexManager.canServeFromIndex(target)) {
528528
Set<DocumentKey> actualKeys = indexManager.getDocumentsMatchingTarget(target);
529529
assertThat(actualKeys)
530-
.containsExactlyElementsIn(Arrays.stream(expectedKeys).map(TestUtil::key).toArray());
530+
.containsExactlyElementsIn(Arrays.stream(expectedKeys).map(TestUtil::key).toArray());
531531
} else {
532532
assertEquals(0, expectedKeys.length);
533533
}

0 commit comments

Comments
 (0)