Skip to content

Commit b4f0e23

Browse files
schmidt-sebastianschmidt-sebastian
committed
Remove Held Write Acks
1 parent 229832f commit b4f0e23

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+1504
-951
lines changed

‎packages/firestore/src/api/database.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
RelationOp
3333
} from '../core/query';
3434
import { Transaction as InternalTransaction } from '../core/transaction';
35-
import { ChangeType, ViewSnapshot } from '../core/view_snapshot';
35+
import { ChangeType, SyncState, ViewSnapshot } from '../core/view_snapshot';
3636
import { Document, MaybeDocument, NoDocument } from '../model/document';
3737
import { DocumentKey } from '../model/document_key';
3838
import {
@@ -1225,8 +1225,10 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
12251225
}
12261226

12271227
get metadata(): firestore.SnapshotMetadata {
1228+
const syncState = this._fromCache ? SyncState.Local : SyncState.Synced;
1229+
12281230
return new SnapshotMetadata(
1229-
this._document !== null && this._document.hasLocalMutations,
1231+
this._document !== null && this._document.hasPendingWrites(syncState),
12301232
this._fromCache
12311233
);
12321234
}

‎packages/firestore/src/core/firestore_client.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import {
6161
import { AutoId } from '../util/misc';
6262
import { PersistenceSettings } from '../api/database';
6363
import { assert } from '../util/assert';
64+
import { SnapshotVersion } from './snapshot_version';
6465

6566
const LOG_TAG = 'FirestoreClient';
6667

@@ -293,17 +294,15 @@ export class FirestoreClient {
293294
// TODO(http://b/33384523): For now we just disable garbage collection
294295
// when persistence is enabled.
295296
this.garbageCollector = new NoOpGarbageCollector();
296-
const storagePrefix = IndexedDbPersistence.buildStoragePrefix(
297-
this.databaseInfo
298-
);
297+
299298
// Opt to use proto3 JSON in case the platform doesn't support Uint8Array.
300299
const serializer = new JsonProtoSerializer(this.databaseInfo.databaseId, {
301300
useProto3Json: true
302301
});
303302

304303
return Promise.resolve().then(() => {
305304
const persistence: IndexedDbPersistence = new IndexedDbPersistence(
306-
storagePrefix,
305+
this.databaseInfo,
307306
this.clientId,
308307
this.platform,
309308
this.asyncQueue,
@@ -322,6 +321,9 @@ export class FirestoreClient {
322321
);
323322
}
324323

324+
const storagePrefix = IndexedDbPersistence.buildStoragePrefix(
325+
this.databaseInfo
326+
);
325327
this.sharedClientState = settings.experimentalTabSynchronization
326328
? new WebStorageSharedClientState(
327329
this.asyncQueue,
@@ -360,8 +362,7 @@ export class FirestoreClient {
360362
this.localStore = new LocalStore(
361363
this.persistence,
362364
user,
363-
this.garbageCollector,
364-
this.sharedClientState
365+
this.garbageCollector
365366
);
366367
const serializer = this.platform.newSerializer(
367368
this.databaseInfo.databaseId
@@ -503,7 +504,8 @@ export class FirestoreClient {
503504

504505
const view = new View(query, remoteKeys);
505506
const viewDocChanges: ViewDocumentChanges = view.computeDocChanges(
506-
docs
507+
docs,
508+
SnapshotVersion.MIN
507509
);
508510
return view.applyChanges(
509511
viewDocChanges,

‎packages/firestore/src/core/sync_engine.ts

+62-12
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,10 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
245245
.remoteDocumentKeys(queryData.targetId)
246246
.then(remoteKeys => {
247247
const view = new View(query, remoteKeys);
248-
const viewDocChanges = view.computeDocChanges(docs);
248+
const viewDocChanges = view.computeDocChanges(
249+
docs,
250+
SnapshotVersion.MIN
251+
);
249252
const synthesizedTargetChange = TargetChange.createSynthesizedTargetChangeForCurrentChange(
250253
queryData.targetId,
251254
current && this.onlineState !== OnlineState.Offline
@@ -349,7 +352,10 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
349352
.then(result => {
350353
this.sharedClientState.addPendingMutation(result.batchId);
351354
this.addMutationCallback(result.batchId, userCallback);
352-
return this.emitNewSnapsAndNotifyLocalStore(result.changes);
355+
return this.emitNewSnapsAndNotifyLocalStore(
356+
result.changes,
357+
SnapshotVersion.MIN
358+
);
353359
})
354360
.then(() => {
355361
return this.remoteStore.fillWritePipeline();
@@ -459,7 +465,11 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
459465
}
460466
}
461467
);
462-
return this.emitNewSnapsAndNotifyLocalStore(changes, remoteEvent);
468+
return this.emitNewSnapsAndNotifyLocalStore(
469+
changes,
470+
remoteEvent.snapshotVersion,
471+
remoteEvent
472+
);
463473
})
464474
.catch(err => this.ignoreIfPrimaryLeaseLoss(err));
465475
}
@@ -505,7 +515,12 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
505515
this.assertSubscribed('rejectListens()');
506516

507517
// PORTING NOTE: Multi-tab only.
508-
this.sharedClientState.updateQueryState(targetId, 'rejected', err);
518+
this.sharedClientState.updateQueryState(
519+
targetId,
520+
SnapshotVersion.MIN,
521+
'rejected',
522+
err
523+
);
509524

510525
const limboResolution = this.limboResolutionsByTarget[targetId];
511526
const limboKey = limboResolution && limboResolution.key;
@@ -527,7 +542,11 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
527542
);
528543
documentUpdates = documentUpdates.insert(
529544
limboKey,
530-
new NoDocument(limboKey, SnapshotVersion.forDeletedDoc())
545+
new NoDocument(
546+
limboKey,
547+
SnapshotVersion.forDeletedDoc(),
548+
SnapshotVersion.MIN
549+
)
531550
);
532551
const resolvedLimboDocuments = documentKeySet().add(limboKey);
533552
const event = new RemoteEvent(
@@ -552,6 +571,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
552571
// PORTING NOTE: Multi-tab only
553572
async applyBatchState(
554573
batchId: BatchId,
574+
snapshotVersion: SnapshotVersion,
555575
batchState: MutationBatchState,
556576
error?: FirestoreError
557577
): Promise<void> {
@@ -585,7 +605,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
585605
fail(`Unknown batchState: ${batchState}`);
586606
}
587607

588-
await this.emitNewSnapsAndNotifyLocalStore(documents);
608+
await this.emitNewSnapsAndNotifyLocalStore(documents, snapshotVersion);
589609
}
590610

591611
applySuccessfulWrite(
@@ -604,7 +624,15 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
604624
return this.localStore
605625
.acknowledgeBatch(mutationBatchResult)
606626
.then(changes => {
607-
return this.emitNewSnapsAndNotifyLocalStore(changes);
627+
this.sharedClientState.updateMutationState(
628+
batchId,
629+
mutationBatchResult.commitVersion,
630+
'acknowledged'
631+
);
632+
return this.emitNewSnapsAndNotifyLocalStore(
633+
changes,
634+
mutationBatchResult.commitVersion
635+
);
608636
})
609637
.catch(err => this.ignoreIfPrimaryLeaseLoss(err));
610638
}
@@ -621,8 +649,16 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
621649
return this.localStore
622650
.rejectBatch(batchId)
623651
.then(changes => {
624-
this.sharedClientState.updateMutationState(batchId, 'rejected', error);
625-
return this.emitNewSnapsAndNotifyLocalStore(changes);
652+
this.sharedClientState.updateMutationState(
653+
batchId,
654+
SnapshotVersion.MIN,
655+
'rejected',
656+
error
657+
);
658+
return this.emitNewSnapsAndNotifyLocalStore(
659+
changes,
660+
SnapshotVersion.MIN
661+
);
626662
})
627663
.catch(err => this.ignoreIfPrimaryLeaseLoss(err));
628664
}
@@ -759,6 +795,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
759795

760796
private async emitNewSnapsAndNotifyLocalStore(
761797
changes: MaybeDocumentMap,
798+
snapshotVersion: SnapshotVersion,
762799
remoteEvent?: RemoteEvent
763800
): Promise<void> {
764801
const newSnaps: ViewSnapshot[] = [];
@@ -769,15 +806,22 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
769806
queriesProcessed.push(
770807
Promise.resolve()
771808
.then(() => {
772-
const viewDocChanges = queryView.view.computeDocChanges(changes);
809+
const viewDocChanges = queryView.view.computeDocChanges(
810+
changes,
811+
snapshotVersion
812+
);
773813
if (!viewDocChanges.needsRefill) {
774814
return viewDocChanges;
775815
}
776816
// The query has a limit and some docs were removed, so we need
777817
// to re-run the query against the local store to make sure we
778818
// didn't lose any good docs that had been past the limit.
779819
return this.localStore.executeQuery(queryView.query).then(docs => {
780-
return queryView.view.computeDocChanges(docs, viewDocChanges);
820+
return queryView.view.computeDocChanges(
821+
docs,
822+
snapshotVersion,
823+
viewDocChanges
824+
);
781825
});
782826
})
783827
.then((viewDocChanges: ViewDocumentChanges) => {
@@ -796,6 +840,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
796840
if (this.isPrimary) {
797841
this.sharedClientState.updateQueryState(
798842
queryView.targetId,
843+
snapshotVersion,
799844
viewChange.snapshot.fromCache ? 'not-current' : 'current'
800845
);
801846
}
@@ -860,7 +905,10 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
860905
result.removedBatchIds,
861906
result.addedBatchIds
862907
);
863-
await this.emitNewSnapsAndNotifyLocalStore(result.affectedDocuments);
908+
await this.emitNewSnapsAndNotifyLocalStore(
909+
result.affectedDocuments,
910+
SnapshotVersion.MIN
911+
);
864912
}
865913

866914
await this.remoteStore.handleCredentialChange();
@@ -973,6 +1021,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
9731021
// PORTING NOTE: Multi-tab only
9741022
async applyTargetState(
9751023
targetId: TargetId,
1024+
snapshotVersion: SnapshotVersion,
9761025
state: QueryTargetState,
9771026
error?: FirestoreError
9781027
): Promise<void> {
@@ -994,6 +1043,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
9941043
);
9951044
return this.emitNewSnapsAndNotifyLocalStore(
9961045
changes,
1046+
snapshotVersion,
9971047
synthesizedRemoteEvent
9981048
);
9991049
}

‎packages/firestore/src/core/transaction.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export class Transaction {
3737
constructor(private datastore: Datastore) {}
3838

3939
private recordVersion(doc: MaybeDocument): void {
40-
let docVersion = doc.version;
40+
let docVersion = doc.remoteVersion;
4141
if (doc instanceof NoDocument) {
4242
// For deleted docs, we must use baseVersion 0 when we overwrite them.
4343
docVersion = SnapshotVersion.forDeletedDoc();

0 commit comments

Comments
 (0)