Skip to content

Commit 110b867

Browse files
wu-huiwu-hui
authored
Delete Overlay Flag (#3211)
* Delete Overlay Flag
1 parent e87b4c7 commit 110b867

File tree

6 files changed

+29
-191
lines changed

6 files changed

+29
-191
lines changed

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

Lines changed: 22 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -88,54 +88,13 @@ IndexManager getIndexManager() {
8888
* for it.
8989
*/
9090
Document getDocument(DocumentKey key) {
91-
if (Persistence.OVERLAY_SUPPORT_ENABLED) {
92-
Mutation overlay = documentOverlayCache.getOverlay(key);
93-
MutableDocument fromOverlay = remoteDocumentCache.get(key);
94-
if (overlay != null) {
95-
overlay.applyToLocalView(fromOverlay, null, Timestamp.now());
96-
}
97-
98-
// TODO(Overlay): Remove below and just return `fromOverlay`.
99-
List<MutationBatch> batches = mutationQueue.getAllMutationBatchesAffectingDocumentKey(key);
100-
Document fromMutationQueue = getDocument(key, batches);
101-
hardAssert(
102-
fromOverlay.equals(fromMutationQueue),
103-
"Document from overlay does not match mutation queue");
104-
105-
return fromOverlay;
91+
Mutation overlay = documentOverlayCache.getOverlay(key);
92+
MutableDocument fromOverlay = remoteDocumentCache.get(key);
93+
if (overlay != null) {
94+
overlay.applyToLocalView(fromOverlay, null, Timestamp.now());
10695
}
107-
List<MutationBatch> batches = mutationQueue.getAllMutationBatchesAffectingDocumentKey(key);
108-
return getDocument(key, batches);
109-
}
11096

111-
// Internal version of {@code getDocument} that allows reusing batches.
112-
private Document getDocument(DocumentKey key, List<MutationBatch> inBatches) {
113-
MutableDocument document = remoteDocumentCache.get(key);
114-
for (MutationBatch batch : inBatches) {
115-
batch.applyToLocalView(document);
116-
}
117-
return document;
118-
}
119-
120-
/**
121-
* Applies the given {@code batches} to the given {@code docs}. The docs are updated to reflect
122-
* the contents of the mutations.
123-
*
124-
* <p>Returns a {@link DocumentKey} to {@link FieldMask} map, representing the fields mutated for
125-
* each document. This is useful to build overlays.
126-
*/
127-
private Map<DocumentKey, FieldMask> applyLocalMutationsToDocuments(
128-
Map<DocumentKey, MutableDocument> docs, List<MutationBatch> batches) {
129-
Map<DocumentKey, FieldMask> changedMasks = new HashMap<>();
130-
for (Map.Entry<DocumentKey, MutableDocument> base : docs.entrySet()) {
131-
FieldMask mask = null;
132-
for (MutationBatch batch : batches) {
133-
mask = batch.applyToLocalView(base.getValue(), mask);
134-
}
135-
changedMasks.put(base.getKey(), mask);
136-
}
137-
138-
return changedMasks;
97+
return fromOverlay;
13998
}
14099

141100
/**
@@ -160,31 +119,25 @@ ImmutableSortedMap<DocumentKey, Document> getDocuments(Iterable<DocumentKey> key
160119
ImmutableSortedMap<DocumentKey, Document> getLocalViewOfDocuments(
161120
Map<DocumentKey, MutableDocument> docs, Set<DocumentKey> existenceStateChanged) {
162121
ImmutableSortedMap<DocumentKey, Document> results = emptyDocumentMap();
163-
if (Persistence.OVERLAY_SUPPORT_ENABLED) {
164-
Map<DocumentKey, MutableDocument> recalculateDocuments = new HashMap<>();
165-
for (Map.Entry<DocumentKey, MutableDocument> entry : docs.entrySet()) {
166-
Mutation overlay = documentOverlayCache.getOverlay(entry.getKey());
167-
// Recalculate an overlay if the document's existence state is changed due to a remote
168-
// event *and* the overlay is a PatchMutation. This is because document existence state
169-
// can change if some patch mutation's preconditions are met.
170-
// NOTE: we recalculate when `overlay` is null as well, because there might be a patch
171-
// mutation whose precondition does not match before the change (hence overlay==null),
172-
// but would now match.
173-
if (existenceStateChanged.contains(entry.getKey())
174-
&& (overlay == null || overlay instanceof PatchMutation)) {
175-
recalculateDocuments.put(entry.getKey(), docs.get(entry.getKey()));
176-
} else if (overlay != null) {
177-
overlay.applyToLocalView(entry.getValue(), null, Timestamp.now());
178-
}
122+
Map<DocumentKey, MutableDocument> recalculateDocuments = new HashMap<>();
123+
for (Map.Entry<DocumentKey, MutableDocument> entry : docs.entrySet()) {
124+
Mutation overlay = documentOverlayCache.getOverlay(entry.getKey());
125+
// Recalculate an overlay if the document's existence state is changed due to a remote
126+
// event *and* the overlay is a PatchMutation. This is because document existence state
127+
// can change if some patch mutation's preconditions are met.
128+
// NOTE: we recalculate when `overlay` is null as well, because there might be a patch
129+
// mutation whose precondition does not match before the change (hence overlay==null),
130+
// but would now match.
131+
if (existenceStateChanged.contains(entry.getKey())
132+
&& (overlay == null || overlay instanceof PatchMutation)) {
133+
recalculateDocuments.put(entry.getKey(), docs.get(entry.getKey()));
134+
} else if (overlay != null) {
135+
overlay.applyToLocalView(entry.getValue(), null, Timestamp.now());
179136
}
180-
181-
recalculateAndSaveOverlays(recalculateDocuments);
182-
} else {
183-
List<MutationBatch> batches =
184-
mutationQueue.getAllMutationBatchesAffectingDocumentKeys(docs.keySet());
185-
applyLocalMutationsToDocuments(docs, batches);
186137
}
187138

139+
recalculateAndSaveOverlays(recalculateDocuments);
140+
188141
for (Map.Entry<DocumentKey, MutableDocument> entry : docs.entrySet()) {
189142
results = results.insert(entry.getKey(), entry.getValue());
190143
}
@@ -295,31 +248,9 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
295248
return results;
296249
}
297250

251+
/** Queries the remote documents and overlays by doing a full collection scan. */
298252
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionQuery(
299253
Query query, IndexOffset offset) {
300-
if (Persistence.OVERLAY_SUPPORT_ENABLED) {
301-
// TODO(Overlay): Remove the assert and just return `fromOverlay`.
302-
ImmutableSortedMap<DocumentKey, Document> fromOverlay =
303-
getDocumentsMatchingCollectionQueryFromOverlayCache(query, offset);
304-
// TODO(Overlay): Delete below before merging. The code passes, but there are tests
305-
// looking at how many documents read from remote document, and this would double
306-
// the count.
307-
/*
308-
ImmutableSortedMap<DocumentKey, Document> fromMutationQueue =
309-
getDocumentsMatchingCollectionQueryFromMutationQueue(query, sinceReadTime);
310-
hardAssert(
311-
fromOverlay.equals(fromMutationQueue),
312-
"Documents from overlay do not match mutation queue version.");
313-
*/
314-
return fromOverlay;
315-
} else {
316-
return getDocumentsMatchingCollectionQueryFromMutationQueue(query, offset);
317-
}
318-
}
319-
320-
/** Queries the remote documents and overlays by doing a full collection scan. */
321-
private ImmutableSortedMap<DocumentKey, Document>
322-
getDocumentsMatchingCollectionQueryFromOverlayCache(Query query, IndexOffset offset) {
323254
ImmutableSortedMap<DocumentKey, MutableDocument> remoteDocuments =
324255
remoteDocumentCache.getAllDocumentsMatchingQuery(query, offset);
325256
Map<DocumentKey, Mutation> overlays = documentOverlayCache.getOverlays(query.getPath(), -1);
@@ -352,79 +283,4 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
352283

353284
return results;
354285
}
355-
356-
/** Queries the remote documents and mutation queue, by doing a full collection scan. */
357-
private ImmutableSortedMap<DocumentKey, Document>
358-
getDocumentsMatchingCollectionQueryFromMutationQueue(Query query, IndexOffset offset) {
359-
ImmutableSortedMap<DocumentKey, MutableDocument> remoteDocuments =
360-
remoteDocumentCache.getAllDocumentsMatchingQuery(query, offset);
361-
362-
// TODO(indexing): We should plumb sinceReadTime through to the mutation queue
363-
List<MutationBatch> matchingBatches = mutationQueue.getAllMutationBatchesAffectingQuery(query);
364-
365-
remoteDocuments = addMissingBaseDocuments(matchingBatches, remoteDocuments);
366-
367-
for (MutationBatch batch : matchingBatches) {
368-
for (Mutation mutation : batch.getMutations()) {
369-
// Only process documents belonging to the collection.
370-
if (!query.getPath().isImmediateParentOf(mutation.getKey().getPath())) {
371-
continue;
372-
}
373-
374-
DocumentKey key = mutation.getKey();
375-
MutableDocument document = remoteDocuments.get(key);
376-
if (document == null) {
377-
// Create invalid document to apply mutations on top of
378-
document = MutableDocument.newInvalidDocument(key);
379-
remoteDocuments = remoteDocuments.insert(key, document);
380-
}
381-
mutation.applyToLocalView(
382-
document, FieldMask.fromSet(new HashSet<>()), batch.getLocalWriteTime());
383-
if (!document.isFoundDocument()) {
384-
remoteDocuments = remoteDocuments.remove(key);
385-
}
386-
}
387-
}
388-
389-
ImmutableSortedMap<DocumentKey, Document> results = emptyDocumentMap();
390-
for (Map.Entry<DocumentKey, MutableDocument> docEntry : remoteDocuments) {
391-
// Finally, insert the documents that still match the query
392-
if (query.matches(docEntry.getValue())) {
393-
results = results.insert(docEntry.getKey(), docEntry.getValue());
394-
}
395-
}
396-
397-
return results;
398-
}
399-
400-
/**
401-
* It is possible that a {@code PatchMutation} can make a document match a query, even if the
402-
* version in the {@code RemoteDocumentCache} is not a match yet (waiting for server to ack). To
403-
* handle this, we find all document keys affected by the {@code PatchMutation}s that are not in
404-
* {@code existingDocs} yet, and back fill them via {@code remoteDocumentCache.getAll}, otherwise
405-
* those {@code PatchMutation}s will be ignored because no base document can be found, and lead to
406-
* missing results for the query.
407-
*/
408-
private ImmutableSortedMap<DocumentKey, MutableDocument> addMissingBaseDocuments(
409-
List<MutationBatch> matchingBatches,
410-
ImmutableSortedMap<DocumentKey, MutableDocument> existingDocs) {
411-
HashSet<DocumentKey> missingDocKeys = new HashSet<>();
412-
for (MutationBatch batch : matchingBatches) {
413-
for (Mutation mutation : batch.getMutations()) {
414-
if (mutation instanceof PatchMutation && !existingDocs.containsKey(mutation.getKey())) {
415-
missingDocKeys.add(mutation.getKey());
416-
}
417-
}
418-
}
419-
420-
ImmutableSortedMap<DocumentKey, MutableDocument> mergedDocs = existingDocs;
421-
Map<DocumentKey, MutableDocument> missingDocs = remoteDocumentCache.getAll(missingDocKeys);
422-
for (Map.Entry<DocumentKey, MutableDocument> entry : missingDocs.entrySet()) {
423-
if (entry.getValue().isFoundDocument()) {
424-
mergedDocs = mergedDocs.insert(entry.getKey(), entry.getValue());
425-
}
426-
}
427-
428-
return mergedDocs;
429-
}
430286
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -278,9 +278,7 @@ public LocalWriteResult writeLocally(List<Mutation> mutations) {
278278
MutationBatch batch =
279279
mutationQueue.addMutationBatch(localWriteTime, baseMutations, mutations);
280280
Map<DocumentKey, Mutation> overlays = batch.applyToLocalDocumentSet(documents);
281-
if (Persistence.OVERLAY_SUPPORT_ENABLED) {
282-
documentOverlayCache.saveOverlays(batch.getBatchId(), overlays);
283-
}
281+
documentOverlayCache.saveOverlays(batch.getBatchId(), overlays);
284282
return new LocalWriteResult(batch.getBatchId(), documents);
285283
});
286284
}
@@ -310,10 +308,8 @@ public ImmutableSortedMap<DocumentKey, Document> acknowledgeBatch(
310308
applyWriteToRemoteDocuments(batchResult);
311309
mutationQueue.performConsistencyCheck();
312310

313-
if (Persistence.OVERLAY_SUPPORT_ENABLED) {
314-
documentOverlayCache.removeOverlaysForBatchId(batchResult.getBatch().getBatchId());
315-
localDocuments.recalculateAndSaveOverlays(getKeysWithTransformResults(batchResult));
316-
}
311+
documentOverlayCache.removeOverlaysForBatchId(batchResult.getBatch().getBatchId());
312+
localDocuments.recalculateAndSaveOverlays(getKeysWithTransformResults(batchResult));
317313

318314
return localDocuments.getDocuments(batch.getKeys());
319315
});
@@ -350,10 +346,8 @@ public ImmutableSortedMap<DocumentKey, Document> rejectBatch(int batchId) {
350346
mutationQueue.removeMutationBatch(toReject);
351347
mutationQueue.performConsistencyCheck();
352348

353-
if (Persistence.OVERLAY_SUPPORT_ENABLED) {
354-
documentOverlayCache.removeOverlaysForBatchId(batchId);
355-
localDocuments.recalculateAndSaveOverlays(toReject.getKeys());
356-
}
349+
documentOverlayCache.removeOverlaysForBatchId(batchId);
350+
localDocuments.recalculateAndSaveOverlays(toReject.getKeys());
357351

358352
return localDocuments.getDocuments(toReject.getKeys());
359353
});

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/Persistence.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@
5050
public abstract class Persistence {
5151
static final String TAG = Persistence.class.getSimpleName();
5252

53-
/** Temporary setting for enabling document overlays. */
54-
// TODO(Overlay): Remove this.
55-
public static boolean OVERLAY_SUPPORT_ENABLED = true;
5653
/** Constant string to indicate a data migration is required to support overlays. */
5754
public static String DATA_MIGRATION_BUILD_OVERLAYS = "BUILD_OVERLAYS";
5855

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteOverlayMigrationManager.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,6 @@ public SQLiteOverlayMigrationManager(SQLitePersistence persistence) {
3939

4040
@Override
4141
public void run() {
42-
if (Persistence.OVERLAY_SUPPORT_ENABLED == false || !hasPendingOverlayMigration()) {
43-
return;
44-
}
4542
buildOverlays();
4643
}
4744

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,6 @@ void runSchemaUpgrades(int fromVersion, int toVersion) {
174174
}
175175

176176
if (fromVersion < 14 && toVersion >= 14) {
177-
Preconditions.checkState(
178-
Persistence.OVERLAY_SUPPORT_ENABLED || Persistence.INDEXING_SUPPORT_ENABLED);
179177
createOverlays();
180178
createDataMigrationTable();
181179
addPendingDataMigration(Persistence.DATA_MIGRATION_BUILD_OVERLAYS);

‎firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -978,12 +978,8 @@ public void testReadsAllDocumentsForInitialCollectionQueries() {
978978
localStore.executeQuery(query, /* usePreviousResults= */ true);
979979

980980
assertRemoteDocumentsRead(/* byKey= */ 0, /* byQuery= */ 2);
981-
if (Persistence.OVERLAY_SUPPORT_ENABLED) {
982-
// No mutations are read because only overlay is needed.
983-
assertMutationsRead(/* byKey= */ 0, /* byQuery= */ 0);
984-
} else {
985-
assertMutationsRead(/* byKey= */ 0, /* byQuery= */ 1);
986-
}
981+
// No mutations are read because only overlay is needed.
982+
assertMutationsRead(/* byKey= */ 0, /* byQuery= */ 0);
987983
}
988984

989985
@Test

0 commit comments

Comments
 (0)