Skip to content

Commit cde1c7d

Browse files
committed
address feedback 1
1 parent 13e2ed8 commit cde1c7d

File tree

7 files changed

+44
-35
lines changed

7 files changed

+44
-35
lines changed

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,13 @@ Document getDocument(DocumentKey key) {
103103
* for that key in the resulting set.
104104
*/
105105
ImmutableSortedMap<DocumentKey, Document> getDocuments(
106-
Iterable<DocumentKey> keys, AutoIndexing counter) {
106+
Iterable<DocumentKey> keys, QueryContext counter) {
107107
Map<DocumentKey, MutableDocument> docs = remoteDocumentCache.getAll(keys, counter);
108108
return getLocalViewOfDocuments(docs, new HashSet<>());
109109
}
110110

111111
ImmutableSortedMap<DocumentKey, Document> getDocuments(Iterable<DocumentKey> keys) {
112-
return getDocuments(keys, new AutoIndexing());
112+
return getDocuments(keys, new QueryContext());
113113
}
114114

115115
/**
@@ -265,7 +265,7 @@ void recalculateAndSaveOverlays(Set<DocumentKey> documentKeys) {
265265
* @param offset Read time and key to start scanning by (exclusive).
266266
*/
267267
ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
268-
Query query, IndexOffset offset, AutoIndexing counter) {
268+
Query query, IndexOffset offset, QueryContext counter) {
269269
ResourcePath path = query.getPath();
270270
if (query.isDocumentQuery()) {
271271
return getDocumentsMatchingDocumentQuery(path, counter);
@@ -278,12 +278,12 @@ ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
278278

279279
ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
280280
Query query, IndexOffset offset) {
281-
return getDocumentsMatchingQuery(query, offset, new AutoIndexing());
281+
return getDocumentsMatchingQuery(query, offset, new QueryContext());
282282
}
283283

284284
/** Performs a simple document lookup for the given path. */
285285
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingDocumentQuery(
286-
ResourcePath path, AutoIndexing counter) {
286+
ResourcePath path, QueryContext counter) {
287287
counter.fullScanCount++;
288288
ImmutableSortedMap<DocumentKey, Document> result = emptyDocumentMap();
289289
// Just do a simple document lookup.
@@ -295,7 +295,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingDocumentQu
295295
}
296296

297297
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionGroupQuery(
298-
Query query, IndexOffset offset, AutoIndexing counter) {
298+
Query query, IndexOffset offset, QueryContext counter) {
299299
hardAssert(
300300
query.getPath().isEmpty(),
301301
"Currently we only support collection group queries at the root.");
@@ -373,7 +373,7 @@ private void populateOverlays(Map<DocumentKey, Overlay> overlays, Set<DocumentKe
373373
}
374374

375375
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionQuery(
376-
Query query, IndexOffset offset, AutoIndexing counter) {
376+
Query query, IndexOffset offset, QueryContext counter) {
377377
Map<DocumentKey, Overlay> overlays =
378378
documentOverlayCache.getOverlays(query.getPath(), offset.getLargestBatchId());
379379
Map<DocumentKey, MutableDocument> remoteDocuments =

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public MutableDocument get(DocumentKey key) {
8181

8282
@Override
8383
public Map<DocumentKey, MutableDocument> getAll(
84-
Iterable<DocumentKey> keys, AutoIndexing counter) {
84+
Iterable<DocumentKey> keys, QueryContext counter) {
8585
Map<DocumentKey, MutableDocument> result = new HashMap<>();
8686
for (DocumentKey key : keys) {
8787
result.put(key, get(key));
@@ -91,7 +91,7 @@ public Map<DocumentKey, MutableDocument> getAll(
9191

9292
@Override
9393
public Map<DocumentKey, MutableDocument> getAll(Iterable<DocumentKey> keys) {
94-
return getAll(keys, new AutoIndexing());
94+
return getAll(keys, new QueryContext());
9595
}
9696

9797
@Override
@@ -106,7 +106,7 @@ public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
106106
Query query,
107107
IndexOffset offset,
108108
@Nonnull Set<DocumentKey> mutatedKeys,
109-
AutoIndexing counter) {
109+
QueryContext counter) {
110110
Map<DocumentKey, MutableDocument> result = new HashMap<>();
111111

112112
// Documents are ordered by key, so we can use a prefix scan to narrow down the documents
@@ -147,7 +147,7 @@ public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
147147
@Override
148148
public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
149149
Query query, IndexOffset offset, @Nonnull Set<DocumentKey> mutatedKeys) {
150-
return getDocumentsMatchingQuery(query, offset, mutatedKeys, new AutoIndexing());
150+
return getDocumentsMatchingQuery(query, offset, mutatedKeys, new QueryContext());
151151
}
152152

153153
Iterable<Document> getDocuments() {
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package com.google.firebase.firestore.local;
22

3-
public class AutoIndexing {
4-
AutoIndexing() {}
3+
public class QueryContext {
4+
QueryContext() {}
55

66
int fullScanCount = 0;
77
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
8282
return result;
8383
}
8484

85-
AutoIndexing counter = new AutoIndexing();
85+
QueryContext counter = new QueryContext();
8686
result = performQueryUsingRemoteKeys(query, remoteKeys, lastLimboFreeSnapshotVersion, counter);
8787
if (result != null) {
8888
return result;
8989
}
90-
counter = new AutoIndexing();
90+
counter = new QueryContext();
9191
return executeFullCollectionScan(query, counter);
9292
}
9393

@@ -146,7 +146,7 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
146146
Query query,
147147
ImmutableSortedSet<DocumentKey> remoteKeys,
148148
SnapshotVersion lastLimboFreeSnapshotVersion,
149-
AutoIndexing counter) {
149+
QueryContext counter) {
150150
if (query.matchesAllDocuments()) {
151151
// Don't use indexes for queries that can be executed by scanning the collection.
152152
return null;
@@ -245,7 +245,7 @@ private boolean needsRefill(
245245
}
246246

247247
private ImmutableSortedMap<DocumentKey, Document> executeFullCollectionScan(
248-
Query query, AutoIndexing counter) {
248+
Query query, QueryContext counter) {
249249
if (Logger.isDebugEnabled()) {
250250
Logger.debug(LOG_TAG, "Using full collection scan to execute query: %s", query.toString());
251251
}
@@ -257,7 +257,7 @@ private ImmutableSortedMap<DocumentKey, Document> executeFullCollectionScan(
257257
* been indexed.
258258
*/
259259
private ImmutableSortedMap<DocumentKey, Document> appendRemainingResults(
260-
Iterable<Document> indexedResults, Query query, IndexOffset offset, AutoIndexing counter) {
260+
Iterable<Document> indexedResults, Query query, IndexOffset offset, QueryContext counter) {
261261
// Retrieve all results for documents that were updated since the offset.
262262
ImmutableSortedMap<DocumentKey, Document> remainingResults =
263263
localDocumentsView.getDocumentsMatchingQuery(query, offset, counter);
@@ -269,6 +269,6 @@ private ImmutableSortedMap<DocumentKey, Document> appendRemainingResults(
269269

270270
private ImmutableSortedMap<DocumentKey, Document> appendRemainingResults(
271271
Iterable<Document> indexedResults, Query query, IndexOffset offset) {
272-
return appendRemainingResults(indexedResults, query, offset, new AutoIndexing());
272+
return appendRemainingResults(indexedResults, query, offset, new QueryContext());
273273
}
274274
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ interface RemoteDocumentCache {
6868
Map<DocumentKey, MutableDocument> getAll(Iterable<DocumentKey> documentKeys);
6969

7070
Map<DocumentKey, MutableDocument> getAll(
71-
Iterable<DocumentKey> documentKeys, AutoIndexing counter);
71+
Iterable<DocumentKey> documentKeys, QueryContext counter);
7272

7373
/**
7474
* Looks up the next {@code limit} documents for a collection group based on the provided offset.
@@ -94,5 +94,5 @@ Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
9494
Query query, IndexOffset offset, @Nonnull Set<DocumentKey> mutatedKeys);
9595

9696
Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
97-
Query query, IndexOffset offset, @Nonnull Set<DocumentKey> mutatedKeys, AutoIndexing counter);
97+
Query query, IndexOffset offset, @Nonnull Set<DocumentKey> mutatedKeys, QueryContext counter);
9898
}

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public MutableDocument get(DocumentKey documentKey) {
118118

119119
@Override
120120
public Map<DocumentKey, MutableDocument> getAll(
121-
Iterable<DocumentKey> documentKeys, AutoIndexing counter) {
121+
Iterable<DocumentKey> documentKeys, QueryContext counter) {
122122
Map<DocumentKey, MutableDocument> results = new HashMap<>();
123123
List<Object> bindVars = new ArrayList<>();
124124
for (DocumentKey key : documentKeys) {
@@ -142,16 +142,20 @@ public Map<DocumentKey, MutableDocument> getAll(
142142
longQuery
143143
.performNextSubquery()
144144
.forEach(
145-
row ->
146-
processRowInBackground(backgroundQueue, results, row, /*filter*/ null, counter));
145+
row -> {
146+
QueryContext singleCounter = new QueryContext();
147+
processRowInBackground(
148+
backgroundQueue, results, row, /*filter*/ null, singleCounter);
149+
counter.fullScanCount += singleCounter.fullScanCount;
150+
});
147151
}
148152
backgroundQueue.drain();
149153
return results;
150154
}
151155

152156
@Override
153157
public Map<DocumentKey, MutableDocument> getAll(Iterable<DocumentKey> documentKeys) {
154-
return getAll(documentKeys, new AutoIndexing());
158+
return getAll(documentKeys, new QueryContext());
155159
}
156160

157161
@Override
@@ -191,7 +195,7 @@ private Map<DocumentKey, MutableDocument> getAll(
191195
IndexOffset offset,
192196
int count,
193197
@Nullable Function<MutableDocument, Boolean> filter,
194-
AutoIndexing counter) {
198+
QueryContext counter) {
195199
Timestamp readTime = offset.getReadTime().getTimestamp();
196200
DocumentKey documentKey = offset.getDocumentKey();
197201

@@ -227,7 +231,12 @@ private Map<DocumentKey, MutableDocument> getAll(
227231
Map<DocumentKey, MutableDocument> results = new HashMap<>();
228232
db.query(sql.toString())
229233
.binding(bindVars)
230-
.forEach(row -> processRowInBackground(backgroundQueue, results, row, filter, counter));
234+
.forEach(
235+
row -> {
236+
QueryContext singleCounter = new QueryContext();
237+
processRowInBackground(backgroundQueue, results, row, filter, singleCounter);
238+
counter.fullScanCount += singleCounter.fullScanCount;
239+
});
231240
backgroundQueue.drain();
232241
return results;
233242
}
@@ -237,15 +246,15 @@ private Map<DocumentKey, MutableDocument> getAll(
237246
IndexOffset offset,
238247
int count,
239248
@Nullable Function<MutableDocument, Boolean> filter) {
240-
return getAll(collections, offset, count, filter, new AutoIndexing());
249+
return getAll(collections, offset, count, filter, new QueryContext());
241250
}
242251

243252
private void processRowInBackground(
244253
BackgroundQueue backgroundQueue,
245254
Map<DocumentKey, MutableDocument> results,
246255
Cursor row,
247256
@Nullable Function<MutableDocument, Boolean> filter,
248-
AutoIndexing counter) {
257+
QueryContext counter) {
249258
byte[] rawDocument = row.getBlob(0);
250259
int readTimeSeconds = row.getInt(1);
251260
int readTimeNanos = row.getInt(2);
@@ -269,15 +278,15 @@ private void processRowInBackground(
269278
@Override
270279
public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
271280
Query query, IndexOffset offset, @Nonnull Set<DocumentKey> mutatedKeys) {
272-
return getDocumentsMatchingQuery(query, offset, mutatedKeys, new AutoIndexing());
281+
return getDocumentsMatchingQuery(query, offset, mutatedKeys, new QueryContext());
273282
}
274283

275284
@Override
276285
public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
277286
Query query,
278287
IndexOffset offset,
279288
@Nonnull Set<DocumentKey> mutatedKeys,
280-
AutoIndexing counter) {
289+
QueryContext counter) {
281290
return getAll(
282291
Collections.singletonList(query.getPath()),
283292
offset,

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,12 @@ public MutableDocument get(DocumentKey documentKey) {
153153

154154
@Override
155155
public Map<DocumentKey, MutableDocument> getAll(Iterable<DocumentKey> documentKeys) {
156-
return getAll(documentKeys, new AutoIndexing());
156+
return getAll(documentKeys, new QueryContext());
157157
}
158158

159159
@Override
160160
public Map<DocumentKey, MutableDocument> getAll(
161-
Iterable<DocumentKey> documentKeys, AutoIndexing counter) {
161+
Iterable<DocumentKey> documentKeys, QueryContext counter) {
162162
Map<DocumentKey, MutableDocument> result = subject.getAll(documentKeys);
163163
for (MutableDocument document : result.values()) {
164164
documentsReadByKey[0] += document.isValidDocument() ? 1 : 0;
@@ -177,15 +177,15 @@ public Map<DocumentKey, MutableDocument> getAll(
177177
@Override
178178
public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
179179
Query query, IndexOffset offset, Set<DocumentKey> mutatedKeys) {
180-
return getDocumentsMatchingQuery(query, offset, mutatedKeys, new AutoIndexing());
180+
return getDocumentsMatchingQuery(query, offset, mutatedKeys, new QueryContext());
181181
}
182182

183183
@Override
184184
public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
185185
Query query,
186186
IndexOffset offset,
187187
@NonNull Set<DocumentKey> mutatedKeys,
188-
AutoIndexing counter) {
188+
QueryContext counter) {
189189
Map<DocumentKey, MutableDocument> result =
190190
subject.getDocumentsMatchingQuery(query, offset, mutatedKeys);
191191
documentsReadByCollection[0] += result.size();

0 commit comments

Comments
 (0)