Skip to content

Commit 5c93eb2

Browse files
author
Brian Chen
committed
resolve comments and add spec tests
1 parent 60bcfa6 commit 5c93eb2

File tree

7 files changed

+324
-270
lines changed

7 files changed

+324
-270
lines changed

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

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -504,12 +504,30 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
504504
* @return An unsubscribe function that can be called to cancel the
505505
* snapshot listener.
506506
*/
507-
_onSnapshotsInSync(onSync: () => void): Unsubscribe {
507+
_onSnapshotsInSync(observer: PartialObserver<void>): Unsubscribe;
508+
_onSnapshotsInSync(onSync: () => void): Unsubscribe;
509+
_onSnapshotsInSync(arg: unknown): Unsubscribe {
508510
this.ensureClientConfigured();
509511

510-
const observer: PartialObserver<void> = {
511-
next: onSync
512-
};
512+
if (isPartialObserver(arg)) {
513+
return this.onSnapshotsInSyncInternal(arg as PartialObserver<void>);
514+
} else {
515+
validateArgType(
516+
'DocumentReference.onSnapshotsInSync',
517+
'function',
518+
0,
519+
arg
520+
);
521+
const observer: PartialObserver<void> = {
522+
next: arg as () => void
523+
};
524+
return this.onSnapshotsInSyncInternal(observer);
525+
}
526+
}
527+
528+
private onSnapshotsInSyncInternal(
529+
observer: PartialObserver<void>
530+
): Unsubscribe {
513531
const errHandler = (err: Error): void => {
514532
throw fail('Uncaught Error in onSnapshotsInSync');
515533
};
@@ -521,7 +539,6 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
521539
},
522540
error: errHandler
523541
});
524-
525542
this._firestoreClient!.addSnapshotsInSyncListener(asyncObserver);
526543
return () => {
527544
asyncObserver.mute();

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,8 @@ export class EventManager implements SyncEngineListener {
163163
}
164164

165165
addSnapshotsInSyncListener(observer: Observer<void>): void {
166-
// If there are no active query listeners, run the callback immediately.
167-
if (this.queries.isEmpty()) {
168-
observer.next();
169-
}
170166
this.snapshotsInSyncListeners.add(observer);
167+
observer.next();
171168
}
172169

173170
removeSnapshotsInSyncListener(observer: Observer<void>): void {

‎packages/firestore/test/integration/api/database.test.ts

Lines changed: 19 additions & 213 deletions
Original file line numberDiff line numberDiff line change
@@ -545,37 +545,23 @@ apiDescribe('Database', (persistence: boolean) => {
545545

546546
apiDescribe('onSnapshotsInSync listener', () => {
547547
function createDocumentListener(
548-
deferred: Deferred<void>,
549548
target: firestore.DocumentReference,
550549
callback: (snapshot: firestore.DocumentSnapshot) => void
551550
): void {
552551
target.onSnapshot(snap => {
553552
callback(snap);
554-
deferred.resolve();
555553
});
556554
}
557555

558556
function createQueryListener(
559-
deferred: Deferred<void>,
560557
target: firestore.Query,
561558
callback: (snapshot: firestore.QuerySnapshot) => void
562559
): void {
563560
target.onSnapshot(snap => {
564561
callback(snap);
565-
deferred.resolve();
566562
});
567563
}
568564

569-
it('fires even if there are no local listeners', () => {
570-
const d1 = new Deferred<void>();
571-
return withTestDb(persistence, async db => {
572-
onSnapshotsInSync(db, () => {
573-
d1.resolve();
574-
});
575-
await d1.promise;
576-
});
577-
});
578-
579565
it('fires after local listeners fire', () => {
580566
const testDocs = {
581567
a: { foo: 1 }
@@ -588,22 +574,25 @@ apiDescribe('Database', (persistence: boolean) => {
588574
const d4 = new Deferred<void>();
589575
const doc = coll.doc('a');
590576

591-
createDocumentListener(d1, doc, snapshot => {
577+
createDocumentListener(doc, snapshot => {
592578
firedValues.push(1);
579+
d1.resolve();
593580
});
594-
createDocumentListener(d2, doc, snapshot => {
581+
createDocumentListener(doc, snapshot => {
595582
firedValues.push(2);
583+
d2.resolve();
596584
});
597-
createDocumentListener(d3, doc, snapshot => {
585+
createDocumentListener(doc, snapshot => {
598586
firedValues.push(3);
587+
d3.resolve();
599588
});
600589
onSnapshotsInSync(doc.firestore, () => {
601590
firedValues.push(4);
602591
d4.resolve();
603592
});
604593

605594
await Promise.all([d1.promise, d2.promise, d3.promise, d4.promise]);
606-
expect(firedValues).to.deep.equal([1, 2, 3, 4]);
595+
expect(firedValues).to.deep.equal([4, 1, 2, 3, 4]);
607596
});
608597
});
609598

@@ -623,9 +612,10 @@ apiDescribe('Database', (persistence: boolean) => {
623612
const query2 = coll.where('foo', '>', 0);
624613
const query3 = coll.where('foo', '>', 0);
625614

626-
createQueryListener(d1, query1, snapshot => {
615+
createQueryListener(query1, snapshot => {
627616
firedValues.push(1);
628617
pending.set(query1, snapshot);
618+
d1.resolve();
629619
});
630620
onSnapshotsInSync(coll.doc().firestore, async () => {
631621
if (pending.has(query2) && pending.has(query3)) {
@@ -645,167 +635,22 @@ apiDescribe('Database', (persistence: boolean) => {
645635

646636
// Add duplicate queries and ensure that the onSnapshotsInSync callback
647637
// was fired for each duplicate.
648-
createQueryListener(d2, query2, snapshot => {
638+
createQueryListener(query2, snapshot => {
649639
firedValues.push(2);
650640
pending.set(query2, snapshot);
641+
d2.resolve();
651642
});
652-
createQueryListener(d3, query3, snapshot => {
643+
createQueryListener(query3, snapshot => {
653644
firedValues.push(3);
654645
pending.set(query3, snapshot);
646+
d3.resolve();
655647
});
656648

657649
await Promise.all([d2.promise, d3.promise, onSyncDeferred2.promise]);
658650
expect(firedValues).to.deep.equal([2, 5, 3, 4]);
659651
});
660652
});
661653

662-
it('queues and fires multiple global listeners', () => {
663-
const testDocs = {
664-
a: { foo: 1 }
665-
};
666-
return withTestCollection(persistence, testDocs, async coll => {
667-
let firedValues: number[] = [];
668-
const d1 = new Deferred<void>();
669-
let d2 = new Deferred<void>();
670-
let d3 = new Deferred<void>();
671-
const doc = coll.doc('a');
672-
673-
createDocumentListener(d1, doc, snapshot => {
674-
firedValues.push(1);
675-
});
676-
onSnapshotsInSync(doc.firestore, () => {
677-
firedValues.push(2);
678-
d2.resolve();
679-
});
680-
onSnapshotsInSync(doc.firestore, () => {
681-
firedValues.push(3);
682-
d3.resolve();
683-
});
684-
685-
await Promise.all([d1.promise, d2.promise, d3.promise]);
686-
expect(firedValues).to.deep.equal([1, 2, 3]);
687-
firedValues = [];
688-
689-
// Set the doc and verify the global listeners fired last.
690-
d2 = new Deferred<void>();
691-
d3 = new Deferred<void>();
692-
await doc.set({ foo: 3 });
693-
await Promise.all([d2.promise, d3.promise]);
694-
expect(firedValues).to.deep.equal([1, 2, 3]);
695-
});
696-
});
697-
698-
it('fires global snapshot events for document snapshots', () => {
699-
const testDocs = {
700-
a: { foo: 1 },
701-
b: { foo: 2 }
702-
};
703-
return withTestCollection(persistence, testDocs, async coll => {
704-
let pending: Map<
705-
firestore.DocumentReference,
706-
firestore.DocumentSnapshot
707-
> = new Map();
708-
let firedValues: number[] = [];
709-
let onSyncDeferred = new Deferred<void>();
710-
const d1 = new Deferred<void>();
711-
const d2 = new Deferred<void>();
712-
const docA = coll.doc('a');
713-
const docB = coll.doc('b');
714-
715-
createDocumentListener(d1, docA, snapshot => {
716-
firedValues.push(1);
717-
pending.set(docA, snapshot);
718-
});
719-
createDocumentListener(d2, docB, snapshot => {
720-
firedValues.push(2);
721-
pending.set(docB, snapshot);
722-
});
723-
onSnapshotsInSync(docA.firestore, async () => {
724-
// Ensure that all other listeners have fired first.
725-
if (pending.has(docA)) {
726-
firedValues.push(3);
727-
} else if (pending.has(docB)) {
728-
firedValues.push(4);
729-
}
730-
onSyncDeferred.resolve();
731-
});
732-
733-
// Wait for listeners to fire and reset values before adding onSnapshotsInSync.
734-
await Promise.all([d1.promise, d2.promise, onSyncDeferred]);
735-
firedValues = [];
736-
pending = new Map();
737-
onSyncDeferred = new Deferred<void>();
738-
739-
// Set docA and check for the global listener.
740-
await docA.set({ foo: 3 });
741-
await onSyncDeferred.promise;
742-
expect(firedValues).to.deep.equal([1, 3]);
743-
pending = new Map();
744-
firedValues = [];
745-
onSyncDeferred = new Deferred<void>();
746-
747-
// Set docB and check for the global listener.
748-
await docB.set({ foo: 3 });
749-
await onSyncDeferred.promise;
750-
expect(firedValues).to.deep.equal([2, 4]);
751-
});
752-
});
753-
754-
it('fires global snapshot events for query snapshots', () => {
755-
const testDocs = {
756-
a: { foo: 1 },
757-
b: { foo: 2 }
758-
};
759-
return withTestCollection(persistence, testDocs, async coll => {
760-
let pending: Map<firestore.Query, firestore.QuerySnapshot> = new Map();
761-
let firedValues: number[] = [];
762-
let onSyncDeferred = new Deferred<void>();
763-
const d1 = new Deferred<void>();
764-
const d2 = new Deferred<void>();
765-
const query1 = coll.where('foo', '>', 0);
766-
const query2 = coll.where('foo', '==', 3);
767-
768-
createQueryListener(d1, query1, snapshot => {
769-
firedValues.push(1);
770-
pending.set(query1, snapshot);
771-
});
772-
createQueryListener(d2, query2, snapshot => {
773-
firedValues.push(2);
774-
pending.set(query2, snapshot);
775-
});
776-
onSnapshotsInSync(coll.doc().firestore, async () => {
777-
// Ensure that all other listeners have fired first.
778-
if (pending.has(query1) && pending.has(query2)) {
779-
firedValues.push(3);
780-
} else if (pending.has(query1)) {
781-
firedValues.push(4);
782-
} else if (pending.has(query2)) {
783-
firedValues.push(5);
784-
}
785-
onSyncDeferred.resolve();
786-
});
787-
788-
// Wait for listeners to fire and reset values.
789-
await Promise.all([d1.promise, d2.promise, onSyncDeferred]);
790-
firedValues = [];
791-
pending = new Map();
792-
onSyncDeferred = new Deferred<void>();
793-
794-
// Verify that query2 should not receive a snapshot.
795-
await coll.doc('a').set({ foo: 2 });
796-
await onSyncDeferred.promise;
797-
expect(firedValues).to.deep.equal([1, 4]);
798-
pending = new Map();
799-
firedValues = [];
800-
onSyncDeferred = new Deferred<void>();
801-
802-
// Verify that query1 and query2 each receive a snapshot.
803-
await coll.doc('b').set({ foo: 3 });
804-
await onSyncDeferred.promise;
805-
expect(firedValues).to.deep.equal([1, 2, 3]);
806-
});
807-
});
808-
809654
it('fires global snapshot events for combinbations of queries and docs', () => {
810655
const testDocs = {
811656
a: { foo: 1 }
@@ -825,17 +670,20 @@ apiDescribe('Database', (persistence: boolean) => {
825670
const query1 = coll.where('foo', '>', 0);
826671
const query2 = coll.where('foo', '>', 1);
827672

828-
createDocumentListener(d1, doc, snapshot => {
673+
createDocumentListener(doc, snapshot => {
829674
firedValues.push(1);
830675
pending.set(doc, snapshot);
676+
d1.resolve();
831677
});
832-
createQueryListener(d2, query1, snapshot => {
678+
createQueryListener(query1, snapshot => {
833679
firedValues.push(2);
834680
pending.set(query1, snapshot);
681+
d2.resolve();
835682
});
836-
createQueryListener(d3, query2, snapshot => {
683+
createQueryListener(query2, snapshot => {
837684
firedValues.push(3);
838685
pending.set(query2, snapshot);
686+
d3.resolve();
839687
});
840688

841689
// Wait for listeners to fire and reset values before adding onSnapshotsInSync.
@@ -857,48 +705,6 @@ apiDescribe('Database', (persistence: boolean) => {
857705
expect(firedValues[3]).to.equal(4);
858706
});
859707
});
860-
861-
it('does not fire if the user unregisters a listener', () => {
862-
const testDocs = {
863-
a: { foo: 1 }
864-
};
865-
return withTestCollection(persistence, testDocs, async coll => {
866-
let pending: Map<
867-
firestore.DocumentReference,
868-
firestore.DocumentSnapshot
869-
> = new Map();
870-
let firedValues: number[] = [];
871-
const d1 = new Deferred<void>();
872-
const d2 = new Deferred<void>();
873-
let d3 = new Deferred<void>();
874-
const doc = coll.doc('a');
875-
876-
createDocumentListener(d1, doc, snapshot => {
877-
firedValues.push(1);
878-
pending.set(doc, snapshot);
879-
});
880-
const unsubscribeFn = onSnapshotsInSync(doc.firestore, () => {
881-
firedValues.push(2);
882-
d2.resolve();
883-
});
884-
onSnapshotsInSync(doc.firestore, () => {
885-
firedValues.push(3);
886-
d3.resolve();
887-
});
888-
889-
// Wait for listeners to fire and reset values.
890-
await Promise.all([d1.promise, d2.promise, d3.promise]);
891-
firedValues = [];
892-
pending = new Map();
893-
d3 = new Deferred<void>();
894-
895-
// Remove a listener and make sure it does not fire.
896-
unsubscribeFn();
897-
await doc.set({ foo: 3 });
898-
await d3.promise;
899-
expect(firedValues).to.deep.equal([1, 3]);
900-
});
901-
});
902708
});
903709

904710
apiDescribe('Queries are validated client-side', (persistence: boolean) => {

0 commit comments

Comments
 (0)