From 9ec449aeb7d1eed5c69ec13d1a38434c4650a1de Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 10 Oct 2017 09:41:25 +0200 Subject: [PATCH 1/6] Adding goOnline/goOffline to the Web SDK --- packages/firestore/package.json | 1 + packages/firestore/src/api/database.ts | 2 +- .../firestore/src/core/firestore_client.ts | 18 + packages/firestore/src/remote/remote_store.ts | 130 ++-- .../test/integration/api/database.test.ts | 49 +- .../test/integration/api/query.test.ts | 560 +++++++++--------- 6 files changed, 435 insertions(+), 325 deletions(-) diff --git a/packages/firestore/package.json b/packages/firestore/package.json index 8647a767000..cad1c803f90 100644 --- a/packages/firestore/package.json +++ b/packages/firestore/package.json @@ -6,6 +6,7 @@ "dev": "gulp dev", "test": "run-p test:browser test:node", "test:browser": "karma start --single-run", + "test:browser-debug" : "karma start --browsers=Chrome", "test:node": "mocha 'test/{,!(integration|browser)/**/}*.test.ts' --compilers ts:ts-node/register -r src/platform_node/node_init.ts --retries 5 --timeout 5000", "prepare": "gulp build" }, diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 17277f1f01f..7fb7c113aef 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -186,7 +186,7 @@ export class Firestore implements firestore.Firestore, FirebaseService { // // Operations on the _firestoreClient don't block on _firestoreReady. Those // are already set to synchronize on the async queue. - private _firestoreClient: FirestoreClient | undefined; + _firestoreClient: FirestoreClient | undefined; public _dataConverter: UserDataConverter; public get _databaseId(): DatabaseId { diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 54d2faf813a..dfb7ea75847 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -160,6 +160,15 @@ export class FirestoreClient { return persistenceResult.promise; } + /** Enables the network connection and requeues all pending operations. */ + public enableNetwork() : Promise { + return this.asyncQueue + .schedule(() => { + this.remoteStore.enableNetwork(); + return Promise.resolve(); + }); + } + /** * Initializes persistent storage, attempting to use IndexedDB if * usePersistence is true or memory-only if false. @@ -314,6 +323,15 @@ export class FirestoreClient { return this.syncEngine.handleUserChange(user); } + /** Disabled the network connection. Pending operations will not complete. */ + public disableNetwork() : Promise { + return this.asyncQueue + .schedule(() => { + this.remoteStore.disableNetwork(); + return Promise.resolve(); + }); + } + shutdown(): Promise { return this.asyncQueue .schedule(() => { diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 330f82cc551..38af4e9b64c 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -116,8 +116,8 @@ export class RemoteStore { private accumulatedWatchChanges: WatchChange[] = []; - private watchStream: PersistentListenStream; - private writeStream: PersistentWriteStream; + private watchStream: PersistentListenStream = null; + private writeStream: PersistentWriteStream = null; /** * The online state of the watch stream. The state is set to healthy if and @@ -149,10 +149,7 @@ export class RemoteStore { * LocalStore, etc. */ start(): Promise { - return this.setupStreams().then(() => { - // Resume any writes - return this.fillWritePipeline(); - }); + return this.enableNetwork(); } private setOnlineStateToHealthy(): void { @@ -192,7 +189,17 @@ export class RemoteStore { } } - private setupStreams(): Promise { + private isNetworkEnabled() : boolean { + assert((this.watchStream == null) == (this.writeStream == null), 'WatchStream and WriteStream should both be null or non-null'); + return this.watchStream != null; + } + + /** Re-enables the network. Only to be called as the counterpart to disableNetwork(). */ + enableNetwork() : Promise { + assert(this.watchStream == null, "enableNetwork() called with non-null watchStream."); + assert(this.writeStream == null, "enableNetwork() called with non-null writeStream."); + + // Create new streams (but note they're not started yet). this.watchStream = this.datastore.newPersistentWatchStream({ onOpen: this.onWatchStreamOpen.bind(this), onClose: this.onWatchStreamClose.bind(this), @@ -208,15 +215,37 @@ export class RemoteStore { // Load any saved stream token from persistent storage return this.localStore.getLastStreamToken().then(token => { this.writeStream.lastStreamToken = token; + + if (this.shouldStartWatchStream()) { + this.startWatchStream(); + } + + this.updateAndBroadcastOnlineState(OnlineState.Unknown); + + return this.fillWritePipeline(); // This may start the writeStream. }); } - shutdown(): Promise { - log.debug(LOG_TAG, 'RemoteStore shutting down.'); - this.cleanupWatchStreamState(); - this.writeStream.stop(); + + /** Temporarily disables the network. The network can be re-enabled using enableNetwork(). */ + disableNetwork() { + this.updateAndBroadcastOnlineState(OnlineState.Failed); + + // NOTE: We're guaranteed not to get any further events from these streams (not even a close + // event). this.watchStream.stop(); + this.writeStream.stop(); + + this.cleanUpWatchStreamState(); + this.cleanUpWriteStreamState(); + + this.writeStream = null; + this.watchStream = null; + } + shutdown(): Promise { + log.debug(LOG_TAG, 'RemoteStore shutting down.'); + this.disableNetwork(); return Promise.resolve(undefined); } @@ -228,11 +257,12 @@ export class RemoteStore { ); // Mark this as something the client is currently listening for. this.listenTargets[queryData.targetId] = queryData; - if (this.watchStream.isOpen()) { - this.sendWatchRequest(queryData); - } else if (!this.watchStream.isStarted()) { + + if (this.shouldStartWatchStream()) { // The listen will be sent in onWatchStreamOpen this.startWatchStream(); + } else if (this.isNetworkEnabled() && this.watchStream.isOpen()) { + this.sendWatchRequest(queryData); } } @@ -244,7 +274,7 @@ export class RemoteStore { ); const queryData = this.listenTargets[targetId]; delete this.listenTargets[targetId]; - if (this.watchStream.isOpen()) { + if (this.isNetworkEnabled() && this.watchStream.isOpen()) { this.sendUnwatchRequest(targetId); } } @@ -279,11 +309,7 @@ export class RemoteStore { } private startWatchStream(): void { - assert(!this.watchStream.isStarted(), "Can't restart started watch stream"); - assert( - this.shouldStartWatchStream(), - 'Tried to start watch stream even though it should not be started' - ); + assert(this.shouldStartWatchStream(), "startWriteStream() called when shouldStartWatchStream() is false."); this.watchStream.start(); } @@ -292,10 +318,10 @@ export class RemoteStore { * active targets trying to be listened too */ private shouldStartWatchStream(): boolean { - return !objUtils.isEmpty(this.listenTargets); + return this.isNetworkEnabled() && !this.watchStream.isStarted() && !objUtils.isEmpty(this.listenTargets); } - private cleanupWatchStreamState(): void { + private cleanUpWatchStreamState(): void { // If the connection is closed then we'll never get a snapshot version for // the accumulated changes and so we'll never be able to complete the batch. // When we start up again the server is going to resend these changes @@ -314,7 +340,10 @@ export class RemoteStore { } private onWatchStreamClose(error: FirestoreError | null): Promise { - this.cleanupWatchStreamState(); + assert(this.isNetworkEnabled(), + "onWatchStreamClose() should only be called when the network is enabled"); + + this.cleanUpWatchStreamState(); // If there was an error, retry the connection. if (this.shouldStartWatchStream()) { @@ -510,6 +539,11 @@ export class RemoteStore { return promiseChain; } + cleanUpWriteStreamState() { + this.lastBatchSeen = BATCHID_UNKNOWN; + this.pendingWrites = []; + } + /** * Notifies that there are new mutations to process in the queue. This is * typically called by SyncEngine after it has sent mutations to LocalStore. @@ -543,7 +577,7 @@ export class RemoteStore { * writes complete the backend will be able to accept more. */ canWriteMutations(): boolean { - return this.pendingWrites.length < MAX_PENDING_WRITES; + return this.isNetworkEnabled() && this.pendingWrites.length < MAX_PENDING_WRITES; } // For testing @@ -565,15 +599,19 @@ export class RemoteStore { this.pendingWrites.push(batch); - if (!this.writeStream.isStarted()) { + if (this.shouldStartWriteStream()) { this.startWriteStream(); - } else if (this.writeStream.handshakeComplete) { + } else if (this.isNetworkEnabled() && this.writeStream.handshakeComplete) { this.writeStream.writeMutations(batch.mutations); } } + private shouldStartWriteStream() : boolean { + return this.isNetworkEnabled() && !this.writeStream.isStarted() && this.pendingWrites.length > 0; + } + private startWriteStream(): void { - assert(!this.writeStream.isStarted(), "Can't restart started write stream"); + assert(this.shouldStartWriteStream(), "startWriteStream() called when shouldStartWriteStream() is false."); this.writeStream.start(); } @@ -632,6 +670,9 @@ export class RemoteStore { } private onWriteStreamClose(error?: FirestoreError): Promise { + assert(this.isNetworkEnabled(), + "onWriteStreamClose() should only be called when the network is enabled"); + // Ignore close if there are no pending writes. if (this.pendingWrites.length > 0) { assert( @@ -653,7 +694,7 @@ export class RemoteStore { return errorHandling.then(() => { // The write stream might have been started by refilling the write // pipeline for failed writes - if (this.pendingWrites.length > 0 && !this.writeStream.isStarted()) { + if (this.shouldStartWriteStream()) { this.startWriteStream(); } }); @@ -713,33 +754,10 @@ export class RemoteStore { handleUserChange(user: User): Promise { log.debug(LOG_TAG, 'RemoteStore changing users: uid=', user.uid); - // Clear pending writes because those are per-user. Watched targets - // persist across users so don't clear those. - this.lastBatchSeen = BATCHID_UNKNOWN; - this.pendingWrites = []; - - // Stop the streams. They promise not to call us back. - this.watchStream.stop(); - this.writeStream.stop(); - - this.cleanupWatchStreamState(); - - // Create new streams (but note they're not started yet). - return this.setupStreams() - .then(() => { - // If there are any watchedTargets, properly handle the stream - // restart now that RemoteStore is ready to handle them. - if (this.shouldStartWatchStream()) { - this.startWatchStream(); - } - - // Resume any writes - return this.fillWritePipeline(); - }) - .then(() => { - // User change moves us back to the unknown state because we might - // not want to re-open the stream - this.setOnlineStateToUnknown(); - }); + // Tear down and re-create our network streams. This will ensure we get a fresh auth token + // for the new user and re-fill the write pipeline with new mutations from the LocalStore + // (since mutations are per-user). + this.disableNetwork(); + return this.enableNetwork(); } } diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index b4f1eb099ef..1d6118b7998 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -18,9 +18,10 @@ import { expect } from 'chai'; import * as firestore from 'firestore'; import { Deferred } from '../../../src/util/promise'; -import { asyncIt } from '../../util/helpers'; +import { fasyncIt, asyncIt } from '../../util/helpers'; import firebase from '../util/firebase_export'; import { apiDescribe, withTestCollection, withTestDb } from '../util/helpers'; +import {Firestore} from '../../../src/api/database'; apiDescribe('Database', persistence => { asyncIt('can set a document', () => { @@ -514,4 +515,50 @@ apiDescribe('Database', persistence => { return Promise.resolve(); }); }); + + fasyncIt('can queue writes while offline', () => { + return withTestDb(persistence, db => { + const docRef = db.collection('rooms').doc(); + const firestoreClient = (docRef.firestore as Firestore)._firestoreClient; + + console.log(firestoreClient); + console.log(docRef.firestore); + + return firestoreClient.disableNetwork().then(() => { + return Promise.all([ + docRef.delete(), + firestoreClient.enableNetwork() + ]); + }).then(() => Promise.resolve()); + }); + }); + + fasyncIt('can get documents while offline', () => { + return withTestDb(persistence, db => { + const docRef = db.collection('rooms').doc(); + const firestoreClient = (docRef.firestore as Firestore)._firestoreClient; + + return firestoreClient.disableNetwork().then(() => { + const promises = []; + let done : () => void; + + promises.push(docRef.set({a:1})); + promises.push(new Promise(resolve => { + done = resolve; + })); + + docRef.get().then(snapshot => { + expect(snapshot.metadata.fromCache).to.be.true; + firestoreClient.enableNetwork().then(() => { + return docRef.get().then(snapshot => { + expect(snapshot.metadata.fromCache).to.be.false; + done(); + }); + }); + }); + + return Promise.all(promises); + }).then(() => Promise.resolve());; + }) + }); }); diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 47d55078f94..6f3be42ca5f 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -18,47 +18,48 @@ import { expect } from 'chai'; import * as firestore from 'firestore'; import { addEqualityMatcher } from '../../util/equality_matcher'; -import { asyncIt, EventsAccumulator, toDataArray } from '../../util/helpers'; +import { fasyncIt, asyncIt, EventsAccumulator, toDataArray } from '../../util/helpers'; import firebase from '../util/firebase_export'; import { apiDescribe, withTestCollection, withTestDbs } from '../util/helpers'; +import {Firestore} from '../../../src/api/database'; apiDescribe('Queries', persistence => { addEqualityMatcher(); asyncIt('can issue limit queries', () => { const testDocs = { - a: { k: 'a' }, - b: { k: 'b' }, - c: { k: 'c' } + a: {k: 'a'}, + b: {k: 'b'}, + c: {k: 'c'} }; return withTestCollection(persistence, testDocs, collection => { return collection - .limit(2) - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([{ k: 'a' }, { k: 'b' }]); - }); + .limit(2) + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([{k: 'a'}, {k: 'b'}]); + }); }); }); asyncIt('can issue limit queries using descending sort order', () => { const testDocs = { - a: { k: 'a', sort: 0 }, - b: { k: 'b', sort: 1 }, - c: { k: 'c', sort: 1 }, - d: { k: 'd', sort: 2 } + a: {k: 'a', sort: 0}, + b: {k: 'b', sort: 1}, + c: {k: 'c', sort: 1}, + d: {k: 'd', sort: 2} }; return withTestCollection(persistence, testDocs, collection => { return collection - .orderBy('sort', 'desc') - .limit(2) - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([ - { k: 'd', sort: 2 }, - { k: 'c', sort: 1 } - ]); - }); + .orderBy('sort', 'desc') + .limit(2) + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([ + {k: 'd', sort: 2}, + {k: 'c', sort: 1} + ]); + }); }); }); @@ -88,209 +89,209 @@ apiDescribe('Queries', persistence => { }; return withTestCollection(persistence, testDocs, coll => { return coll - .where('foo', '>', 21.0) - .orderBy('foo', 'desc') - .get() - .then(docs => { - expect(docs.docs.map(d => d.id)).to.deep.equal([ - 'g', - 'f', - 'c', - 'b', - 'a' - ]); - }); + .where('foo', '>', 21.0) + .orderBy('foo', 'desc') + .get() + .then(docs => { + expect(docs.docs.map(d => d.id)).to.deep.equal([ + 'g', + 'f', + 'c', + 'b', + 'a' + ]); + }); }); }); asyncIt('can use unary filters', () => { return withTestDbs(persistence, 2, ([writerDb, readerDb]) => { return Promise.all([ - writerDb - .collection('query_test') - .doc('a') - .set({ null: null, nan: NaN }), - writerDb - .collection('query_test') - .doc('b') - .set({ null: null, nan: 0 }), - writerDb - .collection('query_test') - .doc('c') - .set({ null: false, nan: NaN }) - ]) - .then(() => { - return readerDb - .collection('query_test') - .where('null', '==', null) - .where('nan', '==', NaN) - .get(); - }) - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([{ null: null, nan: NaN }]); - }); + writerDb + .collection('query_test') + .doc('a') + .set({null: null, nan: NaN}), + writerDb + .collection('query_test') + .doc('b') + .set({null: null, nan: 0}), + writerDb + .collection('query_test') + .doc('c') + .set({null: false, nan: NaN}) + ]) + .then(() => { + return readerDb + .collection('query_test') + .where('null', '==', null) + .where('nan', '==', NaN) + .get(); + }) + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([{null: null, nan: NaN}]); + }); }); }); asyncIt('can filter on infinity', () => { return withTestDbs(persistence, 2, ([writerDb, readerDb]) => { return Promise.all([ - writerDb - .collection('query_test') - .doc('a') - .set({ inf: Infinity }), - writerDb - .collection('query_test') - .doc('b') - .set({ inf: -Infinity }) - ]) - .then(() => { - return readerDb - .collection('query_test') - .where('inf', '==', Infinity) - .get(); - }) - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([{ inf: Infinity }]); - }); + writerDb + .collection('query_test') + .doc('a') + .set({inf: Infinity}), + writerDb + .collection('query_test') + .doc('b') + .set({inf: -Infinity}) + ]) + .then(() => { + return readerDb + .collection('query_test') + .where('inf', '==', Infinity) + .get(); + }) + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([{inf: Infinity}]); + }); }); }); asyncIt('will not get metadata only updates', () => { - const testDocs = { a: { v: 'a' }, b: { v: 'b' } }; + const testDocs = {a: {v: 'a'}, b: {v: 'b'}}; return withTestCollection(persistence, testDocs, coll => { const storeEvent = new EventsAccumulator(); let unlisten: (() => void) | null = null; return Promise.all([ - coll.doc('a').set({ v: 'a' }), - coll.doc('b').set({ v: 'b' }) - ]) - .then(() => { - unlisten = coll.onSnapshot(storeEvent.storeEvent); - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - expect(toDataArray(querySnap)).to.deep.equal([ - { v: 'a' }, - { v: 'b' } - ]); - return coll.doc('a').set({ v: 'a1' }); - }) - .then(() => { - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - expect(toDataArray(querySnap)).to.deep.equal([ - { v: 'a1' }, - { v: 'b' } - ]); - return storeEvent.assertNoAdditionalEvents(); - }) - .then(() => { - unlisten!(); - }); + coll.doc('a').set({v: 'a'}), + coll.doc('b').set({v: 'b'}) + ]) + .then(() => { + unlisten = coll.onSnapshot(storeEvent.storeEvent); + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + expect(toDataArray(querySnap)).to.deep.equal([ + {v: 'a'}, + {v: 'b'} + ]); + return coll.doc('a').set({v: 'a1'}); + }) + .then(() => { + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + expect(toDataArray(querySnap)).to.deep.equal([ + {v: 'a1'}, + {v: 'b'} + ]); + return storeEvent.assertNoAdditionalEvents(); + }) + .then(() => { + unlisten!(); + }); }); }); asyncIt('can listen for the same query with different options', () => { - const testDocs = { a: { v: 'a' }, b: { v: 'b' } }; + const testDocs = {a: {v: 'a'}, b: {v: 'b'}}; return withTestCollection(persistence, testDocs, coll => { const storeEvent = new EventsAccumulator(); const storeEventFull = new EventsAccumulator(); let unlisten1: (() => void) | null = null; let unlisten2: (() => void) | null = null; return Promise.all([ - coll.doc('a').set({ v: 'a' }), - coll.doc('b').set({ v: 'b' }) - ]) - .then(() => { - unlisten1 = coll.onSnapshot(storeEvent.storeEvent); - unlisten2 = coll.onSnapshot( - { includeDocumentMetadataChanges: true }, - storeEventFull.storeEvent - ); - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - expect(toDataArray(querySnap)).to.deep.equal([ - { v: 'a' }, - { v: 'b' } - ]); - return storeEventFull.awaitEvent(); - }) - .then(querySnap => { - expect(toDataArray(querySnap)).to.deep.equal([ - { v: 'a' }, - { v: 'b' } - ]); - return coll.doc('a').set({ v: 'a1' }); - }) - .then(() => { - return storeEventFull.awaitEvents(2); - }) - .then(events => { - // Expect two events for the write, once from latency compensation - // and once from the acknowledgment from the server. - expect(toDataArray(events[0])).to.deep.equal([ - { v: 'a1' }, - { v: 'b' } - ]); - expect(toDataArray(events[1])).to.deep.equal([ - { v: 'a1' }, - { v: 'b' } - ]); - const localResult = events[0].docs; - expect(localResult[0].metadata.hasPendingWrites).to.equal(true); - const syncedResults = events[1].docs; - expect(syncedResults[0].metadata.hasPendingWrites).to.equal(false); + coll.doc('a').set({v: 'a'}), + coll.doc('b').set({v: 'b'}) + ]) + .then(() => { + unlisten1 = coll.onSnapshot(storeEvent.storeEvent); + unlisten2 = coll.onSnapshot( + {includeDocumentMetadataChanges: true}, + storeEventFull.storeEvent + ); + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + expect(toDataArray(querySnap)).to.deep.equal([ + {v: 'a'}, + {v: 'b'} + ]); + return storeEventFull.awaitEvent(); + }) + .then(querySnap => { + expect(toDataArray(querySnap)).to.deep.equal([ + {v: 'a'}, + {v: 'b'} + ]); + return coll.doc('a').set({v: 'a1'}); + }) + .then(() => { + return storeEventFull.awaitEvents(2); + }) + .then(events => { + // Expect two events for the write, once from latency compensation + // and once from the acknowledgment from the server. + expect(toDataArray(events[0])).to.deep.equal([ + {v: 'a1'}, + {v: 'b'} + ]); + expect(toDataArray(events[1])).to.deep.equal([ + {v: 'a1'}, + {v: 'b'} + ]); + const localResult = events[0].docs; + expect(localResult[0].metadata.hasPendingWrites).to.equal(true); + const syncedResults = events[1].docs; + expect(syncedResults[0].metadata.hasPendingWrites).to.equal(false); - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - // Expect only one event for the write. - expect(toDataArray(querySnap)).to.deep.equal([ - { v: 'a1' }, - { v: 'b' } - ]); - return storeEvent.assertNoAdditionalEvents(); - }) - .then(() => { - return coll.doc('b').set({ v: 'b1' }); - }) - .then(() => { - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - // Expect only one event from the second write - expect(toDataArray(querySnap)).to.deep.equal([ - { v: 'a1' }, - { v: 'b1' } - ]); - return storeEventFull.awaitEvents(2); - }) - .then(events => { - // Expect 2 events from the second write. - expect(toDataArray(events[0])).to.deep.equal([ - { v: 'a1' }, - { v: 'b1' } - ]); - expect(toDataArray(events[1])).to.deep.equal([ - { v: 'a1' }, - { v: 'b1' } - ]); - const localResults = events[0].docs; - expect(localResults[1].metadata.hasPendingWrites).to.equal(true); - const syncedResults = events[1].docs; - expect(syncedResults[1].metadata.hasPendingWrites).to.equal(false); - return storeEvent.assertNoAdditionalEvents(); - }) - .then(() => { - return storeEventFull.assertNoAdditionalEvents(); - }) - .then(() => { - unlisten1!(); - unlisten2!(); - }); + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + // Expect only one event for the write. + expect(toDataArray(querySnap)).to.deep.equal([ + {v: 'a1'}, + {v: 'b'} + ]); + return storeEvent.assertNoAdditionalEvents(); + }) + .then(() => { + return coll.doc('b').set({v: 'b1'}); + }) + .then(() => { + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + // Expect only one event from the second write + expect(toDataArray(querySnap)).to.deep.equal([ + {v: 'a1'}, + {v: 'b1'} + ]); + return storeEventFull.awaitEvents(2); + }) + .then(events => { + // Expect 2 events from the second write. + expect(toDataArray(events[0])).to.deep.equal([ + {v: 'a1'}, + {v: 'b1'} + ]); + expect(toDataArray(events[1])).to.deep.equal([ + {v: 'a1'}, + {v: 'b1'} + ]); + const localResults = events[0].docs; + expect(localResults[1].metadata.hasPendingWrites).to.equal(true); + const syncedResults = events[1].docs; + expect(syncedResults[1].metadata.hasPendingWrites).to.equal(false); + return storeEvent.assertNoAdditionalEvents(); + }) + .then(() => { + return storeEventFull.assertNoAdditionalEvents(); + }) + .then(() => { + unlisten1!(); + unlisten2!(); + }); }); }); @@ -303,9 +304,9 @@ apiDescribe('Queries', persistence => { date3.setMilliseconds(2); const testDocs = { - '1': { id: '1', date: date1 }, - '2': { id: '2', date: date2 }, - '3': { id: '3', date: date3 } + '1': {id: '1', date: date1}, + '2': {id: '2', date: date2}, + '3': {id: '3', date: date3} }; return withTestCollection(persistence, testDocs, coll => { // Make sure to issue the queries in parallel @@ -317,20 +318,20 @@ apiDescribe('Queries', persistence => { const docs2 = results[1]; expect(toDataArray(docs1)).to.deep.equal([ - { id: '2', date: date2 }, - { id: '3', date: date3 } + {id: '2', date: date2}, + {id: '3', date: date3} ]); - expect(toDataArray(docs2)).to.deep.equal([{ id: '3', date: date3 }]); + expect(toDataArray(docs2)).to.deep.equal([{id: '3', date: date3}]); }); }); }); asyncIt('can listen for QueryMetadata changes', () => { const testDocs = { - '1': { sort: 1, filter: true, key: '1' }, - '2': { sort: 2, filter: true, key: '2' }, - '3': { sort: 2, filter: true, key: '3' }, - '4': { sort: 3, filter: false, key: '4' } + '1': {sort: 1, filter: true, key: '1'}, + '2': {sort: 2, filter: true, key: '2'}, + '3': {sort: 2, filter: true, key: '3'}, + '4': {sort: 3, filter: false, key: '4'} }; return withTestCollection(persistence, testDocs, coll => { const query = coll.where('key', '<', '4'); @@ -344,11 +345,11 @@ apiDescribe('Queries', persistence => { ]); const query2 = coll.where('filter', '==', true); unlisten2 = query2.onSnapshot( - { - includeQueryMetadataChanges: true, - includeDocumentMetadataChanges: false - }, - accum.storeEvent + { + includeQueryMetadataChanges: true, + includeDocumentMetadataChanges: false + }, + accum.storeEvent ); }); return accum.awaitEvents(2).then(events => { @@ -370,85 +371,110 @@ apiDescribe('Queries', persistence => { asyncIt('can explicitly sort by document ID', () => { const testDocs = { - a: { key: 'a' }, - b: { key: 'b' }, - c: { key: 'c' } + a: {key: 'a'}, + b: {key: 'b'}, + c: {key: 'c'} }; return withTestCollection(persistence, testDocs, coll => { // Ideally this would be descending to validate it's different than // the default, but that requires an extra index return coll - .orderBy(firebase.firestore.FieldPath.documentId()) - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([ - testDocs['a'], - testDocs['b'], - testDocs['c'] - ]); - }); + .orderBy(firebase.firestore.FieldPath.documentId()) + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([ + testDocs['a'], + testDocs['b'], + testDocs['c'] + ]); + }); }); }); asyncIt('can query by document ID', () => { const testDocs = { - aa: { key: 'aa' }, - ab: { key: 'ab' }, - ba: { key: 'ba' }, - bb: { key: 'bb' } + aa: {key: 'aa'}, + ab: {key: 'ab'}, + ba: {key: 'ba'}, + bb: {key: 'bb'} }; return withTestCollection(persistence, testDocs, coll => { return coll - .where(firebase.firestore.FieldPath.documentId(), '==', 'ab') - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([testDocs['ab']]); - return coll - .where(firebase.firestore.FieldPath.documentId(), '>', 'aa') - .where(firebase.firestore.FieldPath.documentId(), '<=', 'ba') - .get(); - }) - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([ - testDocs['ab'], - testDocs['ba'] - ]); - }); + .where(firebase.firestore.FieldPath.documentId(), '==', 'ab') + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([testDocs['ab']]); + return coll + .where(firebase.firestore.FieldPath.documentId(), '>', 'aa') + .where(firebase.firestore.FieldPath.documentId(), '<=', 'ba') + .get(); + }) + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([ + testDocs['ab'], + testDocs['ba'] + ]); + }); }); }); asyncIt('can query by document ID using refs', () => { const testDocs = { - aa: { key: 'aa' }, - ab: { key: 'ab' }, - ba: { key: 'ba' }, - bb: { key: 'bb' } + aa: {key: 'aa'}, + ab: {key: 'ab'}, + ba: {key: 'ba'}, + bb: {key: 'bb'} }; return withTestCollection(persistence, testDocs, coll => { return coll - .where(firebase.firestore.FieldPath.documentId(), '==', coll.doc('ab')) - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([testDocs['ab']]); - return coll - .where( - firebase.firestore.FieldPath.documentId(), - '>', - coll.doc('aa') - ) - .where( - firebase.firestore.FieldPath.documentId(), - '<=', - coll.doc('ba') - ) - .get(); - }) - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([ - testDocs['ab'], - testDocs['ba'] - ]); - }); + .where(firebase.firestore.FieldPath.documentId(), '==', coll.doc('ab')) + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([testDocs['ab']]); + return coll + .where( + firebase.firestore.FieldPath.documentId(), + '>', + coll.doc('aa') + ) + .where( + firebase.firestore.FieldPath.documentId(), + '<=', + coll.doc('ba') + ) + .get(); + }) + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([ + testDocs['ab'], + testDocs['ba'] + ]); + }); + }); + }); + + fasyncIt('can query while reconnecting to network', () => { + return withTestCollection(persistence, /* docs= */ {}, coll => { + const firestoreClient = (coll.firestore as Firestore)._firestoreClient; + + let done: () => void; + const promise = new Promise(resolve => { + done = resolve; + }); + + coll.onSnapshot({includeQueryMetadataChanges: true}, snapshot => { + if (!snapshot.empty && !snapshot.metadata.fromCache) { + done(); + } + }); + + firestoreClient.disableNetwork().then(() => { + coll.doc().set({a: 1}); + firestoreClient.enableNetwork(); + }); + + return promise; }); }); }); + From 96f26268f081567d6e68911ad2335b0aa193b3fa Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 10 Oct 2017 12:06:30 +0200 Subject: [PATCH 2/6] [AUTOMATED]: Prettier Code Styling --- .../firestore/src/core/firestore_client.ts | 22 +- packages/firestore/src/remote/remote_store.ts | 60 +- .../test/integration/api/database.test.ts | 62 +- .../test/integration/api/query.test.ts | 547 +++++++++--------- 4 files changed, 364 insertions(+), 327 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index dfb7ea75847..c0061f1c0d1 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -161,12 +161,11 @@ export class FirestoreClient { } /** Enables the network connection and requeues all pending operations. */ - public enableNetwork() : Promise { - return this.asyncQueue - .schedule(() => { - this.remoteStore.enableNetwork(); - return Promise.resolve(); - }); + public enableNetwork(): Promise { + return this.asyncQueue.schedule(() => { + this.remoteStore.enableNetwork(); + return Promise.resolve(); + }); } /** @@ -324,12 +323,11 @@ export class FirestoreClient { } /** Disabled the network connection. Pending operations will not complete. */ - public disableNetwork() : Promise { - return this.asyncQueue - .schedule(() => { - this.remoteStore.disableNetwork(); - return Promise.resolve(); - }); + public disableNetwork(): Promise { + return this.asyncQueue.schedule(() => { + this.remoteStore.disableNetwork(); + return Promise.resolve(); + }); } shutdown(): Promise { diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 38af4e9b64c..0d8c409c1f1 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -189,15 +189,24 @@ export class RemoteStore { } } - private isNetworkEnabled() : boolean { - assert((this.watchStream == null) == (this.writeStream == null), 'WatchStream and WriteStream should both be null or non-null'); + private isNetworkEnabled(): boolean { + assert( + (this.watchStream == null) == (this.writeStream == null), + 'WatchStream and WriteStream should both be null or non-null' + ); return this.watchStream != null; } /** Re-enables the network. Only to be called as the counterpart to disableNetwork(). */ - enableNetwork() : Promise { - assert(this.watchStream == null, "enableNetwork() called with non-null watchStream."); - assert(this.writeStream == null, "enableNetwork() called with non-null writeStream."); + enableNetwork(): Promise { + assert( + this.watchStream == null, + 'enableNetwork() called with non-null watchStream.' + ); + assert( + this.writeStream == null, + 'enableNetwork() called with non-null writeStream.' + ); // Create new streams (but note they're not started yet). this.watchStream = this.datastore.newPersistentWatchStream({ @@ -226,7 +235,6 @@ export class RemoteStore { }); } - /** Temporarily disables the network. The network can be re-enabled using enableNetwork(). */ disableNetwork() { this.updateAndBroadcastOnlineState(OnlineState.Failed); @@ -309,7 +317,10 @@ export class RemoteStore { } private startWatchStream(): void { - assert(this.shouldStartWatchStream(), "startWriteStream() called when shouldStartWatchStream() is false."); + assert( + this.shouldStartWatchStream(), + 'startWriteStream() called when shouldStartWatchStream() is false.' + ); this.watchStream.start(); } @@ -318,7 +329,11 @@ export class RemoteStore { * active targets trying to be listened too */ private shouldStartWatchStream(): boolean { - return this.isNetworkEnabled() && !this.watchStream.isStarted() && !objUtils.isEmpty(this.listenTargets); + return ( + this.isNetworkEnabled() && + !this.watchStream.isStarted() && + !objUtils.isEmpty(this.listenTargets) + ); } private cleanUpWatchStreamState(): void { @@ -340,8 +355,10 @@ export class RemoteStore { } private onWatchStreamClose(error: FirestoreError | null): Promise { - assert(this.isNetworkEnabled(), - "onWatchStreamClose() should only be called when the network is enabled"); + assert( + this.isNetworkEnabled(), + 'onWatchStreamClose() should only be called when the network is enabled' + ); this.cleanUpWatchStreamState(); @@ -577,7 +594,9 @@ export class RemoteStore { * writes complete the backend will be able to accept more. */ canWriteMutations(): boolean { - return this.isNetworkEnabled() && this.pendingWrites.length < MAX_PENDING_WRITES; + return ( + this.isNetworkEnabled() && this.pendingWrites.length < MAX_PENDING_WRITES + ); } // For testing @@ -606,12 +625,19 @@ export class RemoteStore { } } - private shouldStartWriteStream() : boolean { - return this.isNetworkEnabled() && !this.writeStream.isStarted() && this.pendingWrites.length > 0; + private shouldStartWriteStream(): boolean { + return ( + this.isNetworkEnabled() && + !this.writeStream.isStarted() && + this.pendingWrites.length > 0 + ); } private startWriteStream(): void { - assert(this.shouldStartWriteStream(), "startWriteStream() called when shouldStartWriteStream() is false."); + assert( + this.shouldStartWriteStream(), + 'startWriteStream() called when shouldStartWriteStream() is false.' + ); this.writeStream.start(); } @@ -670,8 +696,10 @@ export class RemoteStore { } private onWriteStreamClose(error?: FirestoreError): Promise { - assert(this.isNetworkEnabled(), - "onWriteStreamClose() should only be called when the network is enabled"); + assert( + this.isNetworkEnabled(), + 'onWriteStreamClose() should only be called when the network is enabled' + ); // Ignore close if there are no pending writes. if (this.pendingWrites.length > 0) { diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 1d6118b7998..43fa6c8ec48 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -18,10 +18,10 @@ import { expect } from 'chai'; import * as firestore from 'firestore'; import { Deferred } from '../../../src/util/promise'; -import { fasyncIt, asyncIt } from '../../util/helpers'; +import { asyncIt } from '../../util/helpers'; import firebase from '../util/firebase_export'; import { apiDescribe, withTestCollection, withTestDb } from '../util/helpers'; -import {Firestore} from '../../../src/api/database'; +import { Firestore } from '../../../src/api/database'; apiDescribe('Database', persistence => { asyncIt('can set a document', () => { @@ -516,7 +516,7 @@ apiDescribe('Database', persistence => { }); }); - fasyncIt('can queue writes while offline', () => { + asyncIt('can queue writes while offline', () => { return withTestDb(persistence, db => { const docRef = db.collection('rooms').doc(); const firestoreClient = (docRef.firestore as Firestore)._firestoreClient; @@ -524,41 +524,49 @@ apiDescribe('Database', persistence => { console.log(firestoreClient); console.log(docRef.firestore); - return firestoreClient.disableNetwork().then(() => { - return Promise.all([ + return firestoreClient + .disableNetwork() + .then(() => { + return Promise.all([ docRef.delete(), firestoreClient.enableNetwork() - ]); - }).then(() => Promise.resolve()); + ]); + }) + .then(() => Promise.resolve()); }); }); - fasyncIt('can get documents while offline', () => { + asyncIt('can get documents while offline', () => { return withTestDb(persistence, db => { const docRef = db.collection('rooms').doc(); const firestoreClient = (docRef.firestore as Firestore)._firestoreClient; - return firestoreClient.disableNetwork().then(() => { - const promises = []; - let done : () => void; - - promises.push(docRef.set({a:1})); - promises.push(new Promise(resolve => { - done = resolve; - })); - - docRef.get().then(snapshot => { - expect(snapshot.metadata.fromCache).to.be.true; - firestoreClient.enableNetwork().then(() => { - return docRef.get().then(snapshot => { - expect(snapshot.metadata.fromCache).to.be.false; - done(); + return firestoreClient + .disableNetwork() + .then(() => { + const promises = []; + let done: () => void; + + promises.push(docRef.set({ a: 1 })); + promises.push( + new Promise(resolve => { + done = resolve; + }) + ); + + docRef.get().then(snapshot => { + expect(snapshot.metadata.fromCache).to.be.true; + firestoreClient.enableNetwork().then(() => { + return docRef.get().then(snapshot => { + expect(snapshot.metadata.fromCache).to.be.false; + done(); + }); }); }); - }); - return Promise.all(promises); - }).then(() => Promise.resolve());; - }) + return Promise.all(promises); + }) + .then(() => Promise.resolve()); + }); }); }); diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 6f3be42ca5f..40244b0af3a 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -18,48 +18,52 @@ import { expect } from 'chai'; import * as firestore from 'firestore'; import { addEqualityMatcher } from '../../util/equality_matcher'; -import { fasyncIt, asyncIt, EventsAccumulator, toDataArray } from '../../util/helpers'; +import { + asyncIt, + EventsAccumulator, + toDataArray +} from '../../util/helpers'; import firebase from '../util/firebase_export'; import { apiDescribe, withTestCollection, withTestDbs } from '../util/helpers'; -import {Firestore} from '../../../src/api/database'; +import { Firestore } from '../../../src/api/database'; apiDescribe('Queries', persistence => { addEqualityMatcher(); asyncIt('can issue limit queries', () => { const testDocs = { - a: {k: 'a'}, - b: {k: 'b'}, - c: {k: 'c'} + a: { k: 'a' }, + b: { k: 'b' }, + c: { k: 'c' } }; return withTestCollection(persistence, testDocs, collection => { return collection - .limit(2) - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([{k: 'a'}, {k: 'b'}]); - }); + .limit(2) + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([{ k: 'a' }, { k: 'b' }]); + }); }); }); asyncIt('can issue limit queries using descending sort order', () => { const testDocs = { - a: {k: 'a', sort: 0}, - b: {k: 'b', sort: 1}, - c: {k: 'c', sort: 1}, - d: {k: 'd', sort: 2} + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 }, + c: { k: 'c', sort: 1 }, + d: { k: 'd', sort: 2 } }; return withTestCollection(persistence, testDocs, collection => { return collection - .orderBy('sort', 'desc') - .limit(2) - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([ - {k: 'd', sort: 2}, - {k: 'c', sort: 1} - ]); - }); + .orderBy('sort', 'desc') + .limit(2) + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([ + { k: 'd', sort: 2 }, + { k: 'c', sort: 1 } + ]); + }); }); }); @@ -89,209 +93,209 @@ apiDescribe('Queries', persistence => { }; return withTestCollection(persistence, testDocs, coll => { return coll - .where('foo', '>', 21.0) - .orderBy('foo', 'desc') - .get() - .then(docs => { - expect(docs.docs.map(d => d.id)).to.deep.equal([ - 'g', - 'f', - 'c', - 'b', - 'a' - ]); - }); + .where('foo', '>', 21.0) + .orderBy('foo', 'desc') + .get() + .then(docs => { + expect(docs.docs.map(d => d.id)).to.deep.equal([ + 'g', + 'f', + 'c', + 'b', + 'a' + ]); + }); }); }); asyncIt('can use unary filters', () => { return withTestDbs(persistence, 2, ([writerDb, readerDb]) => { return Promise.all([ - writerDb - .collection('query_test') - .doc('a') - .set({null: null, nan: NaN}), - writerDb - .collection('query_test') - .doc('b') - .set({null: null, nan: 0}), - writerDb - .collection('query_test') - .doc('c') - .set({null: false, nan: NaN}) - ]) - .then(() => { - return readerDb - .collection('query_test') - .where('null', '==', null) - .where('nan', '==', NaN) - .get(); - }) - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([{null: null, nan: NaN}]); - }); + writerDb + .collection('query_test') + .doc('a') + .set({ null: null, nan: NaN }), + writerDb + .collection('query_test') + .doc('b') + .set({ null: null, nan: 0 }), + writerDb + .collection('query_test') + .doc('c') + .set({ null: false, nan: NaN }) + ]) + .then(() => { + return readerDb + .collection('query_test') + .where('null', '==', null) + .where('nan', '==', NaN) + .get(); + }) + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([{ null: null, nan: NaN }]); + }); }); }); asyncIt('can filter on infinity', () => { return withTestDbs(persistence, 2, ([writerDb, readerDb]) => { return Promise.all([ - writerDb - .collection('query_test') - .doc('a') - .set({inf: Infinity}), - writerDb - .collection('query_test') - .doc('b') - .set({inf: -Infinity}) - ]) - .then(() => { - return readerDb - .collection('query_test') - .where('inf', '==', Infinity) - .get(); - }) - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([{inf: Infinity}]); - }); + writerDb + .collection('query_test') + .doc('a') + .set({ inf: Infinity }), + writerDb + .collection('query_test') + .doc('b') + .set({ inf: -Infinity }) + ]) + .then(() => { + return readerDb + .collection('query_test') + .where('inf', '==', Infinity) + .get(); + }) + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([{ inf: Infinity }]); + }); }); }); asyncIt('will not get metadata only updates', () => { - const testDocs = {a: {v: 'a'}, b: {v: 'b'}}; + const testDocs = { a: { v: 'a' }, b: { v: 'b' } }; return withTestCollection(persistence, testDocs, coll => { const storeEvent = new EventsAccumulator(); let unlisten: (() => void) | null = null; return Promise.all([ - coll.doc('a').set({v: 'a'}), - coll.doc('b').set({v: 'b'}) - ]) - .then(() => { - unlisten = coll.onSnapshot(storeEvent.storeEvent); - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - expect(toDataArray(querySnap)).to.deep.equal([ - {v: 'a'}, - {v: 'b'} - ]); - return coll.doc('a').set({v: 'a1'}); - }) - .then(() => { - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - expect(toDataArray(querySnap)).to.deep.equal([ - {v: 'a1'}, - {v: 'b'} - ]); - return storeEvent.assertNoAdditionalEvents(); - }) - .then(() => { - unlisten!(); - }); + coll.doc('a').set({ v: 'a' }), + coll.doc('b').set({ v: 'b' }) + ]) + .then(() => { + unlisten = coll.onSnapshot(storeEvent.storeEvent); + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + expect(toDataArray(querySnap)).to.deep.equal([ + { v: 'a' }, + { v: 'b' } + ]); + return coll.doc('a').set({ v: 'a1' }); + }) + .then(() => { + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + expect(toDataArray(querySnap)).to.deep.equal([ + { v: 'a1' }, + { v: 'b' } + ]); + return storeEvent.assertNoAdditionalEvents(); + }) + .then(() => { + unlisten!(); + }); }); }); asyncIt('can listen for the same query with different options', () => { - const testDocs = {a: {v: 'a'}, b: {v: 'b'}}; + const testDocs = { a: { v: 'a' }, b: { v: 'b' } }; return withTestCollection(persistence, testDocs, coll => { const storeEvent = new EventsAccumulator(); const storeEventFull = new EventsAccumulator(); let unlisten1: (() => void) | null = null; let unlisten2: (() => void) | null = null; return Promise.all([ - coll.doc('a').set({v: 'a'}), - coll.doc('b').set({v: 'b'}) - ]) - .then(() => { - unlisten1 = coll.onSnapshot(storeEvent.storeEvent); - unlisten2 = coll.onSnapshot( - {includeDocumentMetadataChanges: true}, - storeEventFull.storeEvent - ); - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - expect(toDataArray(querySnap)).to.deep.equal([ - {v: 'a'}, - {v: 'b'} - ]); - return storeEventFull.awaitEvent(); - }) - .then(querySnap => { - expect(toDataArray(querySnap)).to.deep.equal([ - {v: 'a'}, - {v: 'b'} - ]); - return coll.doc('a').set({v: 'a1'}); - }) - .then(() => { - return storeEventFull.awaitEvents(2); - }) - .then(events => { - // Expect two events for the write, once from latency compensation - // and once from the acknowledgment from the server. - expect(toDataArray(events[0])).to.deep.equal([ - {v: 'a1'}, - {v: 'b'} - ]); - expect(toDataArray(events[1])).to.deep.equal([ - {v: 'a1'}, - {v: 'b'} - ]); - const localResult = events[0].docs; - expect(localResult[0].metadata.hasPendingWrites).to.equal(true); - const syncedResults = events[1].docs; - expect(syncedResults[0].metadata.hasPendingWrites).to.equal(false); + coll.doc('a').set({ v: 'a' }), + coll.doc('b').set({ v: 'b' }) + ]) + .then(() => { + unlisten1 = coll.onSnapshot(storeEvent.storeEvent); + unlisten2 = coll.onSnapshot( + { includeDocumentMetadataChanges: true }, + storeEventFull.storeEvent + ); + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + expect(toDataArray(querySnap)).to.deep.equal([ + { v: 'a' }, + { v: 'b' } + ]); + return storeEventFull.awaitEvent(); + }) + .then(querySnap => { + expect(toDataArray(querySnap)).to.deep.equal([ + { v: 'a' }, + { v: 'b' } + ]); + return coll.doc('a').set({ v: 'a1' }); + }) + .then(() => { + return storeEventFull.awaitEvents(2); + }) + .then(events => { + // Expect two events for the write, once from latency compensation + // and once from the acknowledgment from the server. + expect(toDataArray(events[0])).to.deep.equal([ + { v: 'a1' }, + { v: 'b' } + ]); + expect(toDataArray(events[1])).to.deep.equal([ + { v: 'a1' }, + { v: 'b' } + ]); + const localResult = events[0].docs; + expect(localResult[0].metadata.hasPendingWrites).to.equal(true); + const syncedResults = events[1].docs; + expect(syncedResults[0].metadata.hasPendingWrites).to.equal(false); - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - // Expect only one event for the write. - expect(toDataArray(querySnap)).to.deep.equal([ - {v: 'a1'}, - {v: 'b'} - ]); - return storeEvent.assertNoAdditionalEvents(); - }) - .then(() => { - return coll.doc('b').set({v: 'b1'}); - }) - .then(() => { - return storeEvent.awaitEvent(); - }) - .then(querySnap => { - // Expect only one event from the second write - expect(toDataArray(querySnap)).to.deep.equal([ - {v: 'a1'}, - {v: 'b1'} - ]); - return storeEventFull.awaitEvents(2); - }) - .then(events => { - // Expect 2 events from the second write. - expect(toDataArray(events[0])).to.deep.equal([ - {v: 'a1'}, - {v: 'b1'} - ]); - expect(toDataArray(events[1])).to.deep.equal([ - {v: 'a1'}, - {v: 'b1'} - ]); - const localResults = events[0].docs; - expect(localResults[1].metadata.hasPendingWrites).to.equal(true); - const syncedResults = events[1].docs; - expect(syncedResults[1].metadata.hasPendingWrites).to.equal(false); - return storeEvent.assertNoAdditionalEvents(); - }) - .then(() => { - return storeEventFull.assertNoAdditionalEvents(); - }) - .then(() => { - unlisten1!(); - unlisten2!(); - }); + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + // Expect only one event for the write. + expect(toDataArray(querySnap)).to.deep.equal([ + { v: 'a1' }, + { v: 'b' } + ]); + return storeEvent.assertNoAdditionalEvents(); + }) + .then(() => { + return coll.doc('b').set({ v: 'b1' }); + }) + .then(() => { + return storeEvent.awaitEvent(); + }) + .then(querySnap => { + // Expect only one event from the second write + expect(toDataArray(querySnap)).to.deep.equal([ + { v: 'a1' }, + { v: 'b1' } + ]); + return storeEventFull.awaitEvents(2); + }) + .then(events => { + // Expect 2 events from the second write. + expect(toDataArray(events[0])).to.deep.equal([ + { v: 'a1' }, + { v: 'b1' } + ]); + expect(toDataArray(events[1])).to.deep.equal([ + { v: 'a1' }, + { v: 'b1' } + ]); + const localResults = events[0].docs; + expect(localResults[1].metadata.hasPendingWrites).to.equal(true); + const syncedResults = events[1].docs; + expect(syncedResults[1].metadata.hasPendingWrites).to.equal(false); + return storeEvent.assertNoAdditionalEvents(); + }) + .then(() => { + return storeEventFull.assertNoAdditionalEvents(); + }) + .then(() => { + unlisten1!(); + unlisten2!(); + }); }); }); @@ -304,9 +308,9 @@ apiDescribe('Queries', persistence => { date3.setMilliseconds(2); const testDocs = { - '1': {id: '1', date: date1}, - '2': {id: '2', date: date2}, - '3': {id: '3', date: date3} + '1': { id: '1', date: date1 }, + '2': { id: '2', date: date2 }, + '3': { id: '3', date: date3 } }; return withTestCollection(persistence, testDocs, coll => { // Make sure to issue the queries in parallel @@ -318,20 +322,20 @@ apiDescribe('Queries', persistence => { const docs2 = results[1]; expect(toDataArray(docs1)).to.deep.equal([ - {id: '2', date: date2}, - {id: '3', date: date3} + { id: '2', date: date2 }, + { id: '3', date: date3 } ]); - expect(toDataArray(docs2)).to.deep.equal([{id: '3', date: date3}]); + expect(toDataArray(docs2)).to.deep.equal([{ id: '3', date: date3 }]); }); }); }); asyncIt('can listen for QueryMetadata changes', () => { const testDocs = { - '1': {sort: 1, filter: true, key: '1'}, - '2': {sort: 2, filter: true, key: '2'}, - '3': {sort: 2, filter: true, key: '3'}, - '4': {sort: 3, filter: false, key: '4'} + '1': { sort: 1, filter: true, key: '1' }, + '2': { sort: 2, filter: true, key: '2' }, + '3': { sort: 2, filter: true, key: '3' }, + '4': { sort: 3, filter: false, key: '4' } }; return withTestCollection(persistence, testDocs, coll => { const query = coll.where('key', '<', '4'); @@ -345,11 +349,11 @@ apiDescribe('Queries', persistence => { ]); const query2 = coll.where('filter', '==', true); unlisten2 = query2.onSnapshot( - { - includeQueryMetadataChanges: true, - includeDocumentMetadataChanges: false - }, - accum.storeEvent + { + includeQueryMetadataChanges: true, + includeDocumentMetadataChanges: false + }, + accum.storeEvent ); }); return accum.awaitEvents(2).then(events => { @@ -371,89 +375,89 @@ apiDescribe('Queries', persistence => { asyncIt('can explicitly sort by document ID', () => { const testDocs = { - a: {key: 'a'}, - b: {key: 'b'}, - c: {key: 'c'} + a: { key: 'a' }, + b: { key: 'b' }, + c: { key: 'c' } }; return withTestCollection(persistence, testDocs, coll => { // Ideally this would be descending to validate it's different than // the default, but that requires an extra index return coll - .orderBy(firebase.firestore.FieldPath.documentId()) - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([ - testDocs['a'], - testDocs['b'], - testDocs['c'] - ]); - }); + .orderBy(firebase.firestore.FieldPath.documentId()) + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([ + testDocs['a'], + testDocs['b'], + testDocs['c'] + ]); + }); }); }); asyncIt('can query by document ID', () => { const testDocs = { - aa: {key: 'aa'}, - ab: {key: 'ab'}, - ba: {key: 'ba'}, - bb: {key: 'bb'} + aa: { key: 'aa' }, + ab: { key: 'ab' }, + ba: { key: 'ba' }, + bb: { key: 'bb' } }; return withTestCollection(persistence, testDocs, coll => { return coll - .where(firebase.firestore.FieldPath.documentId(), '==', 'ab') - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([testDocs['ab']]); - return coll - .where(firebase.firestore.FieldPath.documentId(), '>', 'aa') - .where(firebase.firestore.FieldPath.documentId(), '<=', 'ba') - .get(); - }) - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([ - testDocs['ab'], - testDocs['ba'] - ]); - }); + .where(firebase.firestore.FieldPath.documentId(), '==', 'ab') + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([testDocs['ab']]); + return coll + .where(firebase.firestore.FieldPath.documentId(), '>', 'aa') + .where(firebase.firestore.FieldPath.documentId(), '<=', 'ba') + .get(); + }) + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([ + testDocs['ab'], + testDocs['ba'] + ]); + }); }); }); asyncIt('can query by document ID using refs', () => { const testDocs = { - aa: {key: 'aa'}, - ab: {key: 'ab'}, - ba: {key: 'ba'}, - bb: {key: 'bb'} + aa: { key: 'aa' }, + ab: { key: 'ab' }, + ba: { key: 'ba' }, + bb: { key: 'bb' } }; return withTestCollection(persistence, testDocs, coll => { return coll - .where(firebase.firestore.FieldPath.documentId(), '==', coll.doc('ab')) - .get() - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([testDocs['ab']]); - return coll - .where( - firebase.firestore.FieldPath.documentId(), - '>', - coll.doc('aa') - ) - .where( - firebase.firestore.FieldPath.documentId(), - '<=', - coll.doc('ba') - ) - .get(); - }) - .then(docs => { - expect(toDataArray(docs)).to.deep.equal([ - testDocs['ab'], - testDocs['ba'] - ]); - }); + .where(firebase.firestore.FieldPath.documentId(), '==', coll.doc('ab')) + .get() + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([testDocs['ab']]); + return coll + .where( + firebase.firestore.FieldPath.documentId(), + '>', + coll.doc('aa') + ) + .where( + firebase.firestore.FieldPath.documentId(), + '<=', + coll.doc('ba') + ) + .get(); + }) + .then(docs => { + expect(toDataArray(docs)).to.deep.equal([ + testDocs['ab'], + testDocs['ba'] + ]); + }); }); }); - fasyncIt('can query while reconnecting to network', () => { + asyncIt('can query while reconnecting to network', () => { return withTestCollection(persistence, /* docs= */ {}, coll => { const firestoreClient = (coll.firestore as Firestore)._firestoreClient; @@ -462,14 +466,14 @@ apiDescribe('Queries', persistence => { done = resolve; }); - coll.onSnapshot({includeQueryMetadataChanges: true}, snapshot => { + coll.onSnapshot({ includeQueryMetadataChanges: true }, snapshot => { if (!snapshot.empty && !snapshot.metadata.fromCache) { done(); } }); firestoreClient.disableNetwork().then(() => { - coll.doc().set({a: 1}); + coll.doc().set({ a: 1 }); firestoreClient.enableNetwork(); }); @@ -477,4 +481,3 @@ apiDescribe('Queries', persistence => { }); }); }); - From 9375faa1b01258efe14f0f2599b07a8235bc6930 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 10 Oct 2017 12:10:41 +0200 Subject: [PATCH 3/6] [AUTOMATED]: Prettier Code Styling --- packages/firestore/test/integration/api/database.test.ts | 3 --- packages/firestore/test/integration/api/query.test.ts | 6 +----- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 43fa6c8ec48..97b0be3a21c 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -521,9 +521,6 @@ apiDescribe('Database', persistence => { const docRef = db.collection('rooms').doc(); const firestoreClient = (docRef.firestore as Firestore)._firestoreClient; - console.log(firestoreClient); - console.log(docRef.firestore); - return firestoreClient .disableNetwork() .then(() => { diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 40244b0af3a..7cbd57506e1 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -18,11 +18,7 @@ import { expect } from 'chai'; import * as firestore from 'firestore'; import { addEqualityMatcher } from '../../util/equality_matcher'; -import { - asyncIt, - EventsAccumulator, - toDataArray -} from '../../util/helpers'; +import { asyncIt, EventsAccumulator, toDataArray } from '../../util/helpers'; import firebase from '../util/firebase_export'; import { apiDescribe, withTestCollection, withTestDbs } from '../util/helpers'; import { Firestore } from '../../../src/api/database'; From 8331953102e78b432d26c44e80da8eaa23911fa1 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 11 Oct 2017 04:55:12 +0200 Subject: [PATCH 4/6] Adding --auto-watch to Karma invocation --- packages/firestore/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/package.json b/packages/firestore/package.json index cad1c803f90..2dceb6c68bb 100644 --- a/packages/firestore/package.json +++ b/packages/firestore/package.json @@ -6,7 +6,7 @@ "dev": "gulp dev", "test": "run-p test:browser test:node", "test:browser": "karma start --single-run", - "test:browser-debug" : "karma start --browsers=Chrome", + "test:browser:debug" : "karma start --browsers=Chrome --auto-watch", "test:node": "mocha 'test/{,!(integration|browser)/**/}*.test.ts' --compilers ts:ts-node/register -r src/platform_node/node_init.ts --retries 5 --timeout 5000", "prepare": "gulp build" }, From 1ad42227281d06fe19e00785f7f5232da947ca41 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 11 Oct 2017 05:46:26 +0200 Subject: [PATCH 5/6] Addressing Mike's comment --- packages/firestore/src/api/database.ts | 4 ++- .../firestore/src/core/firestore_client.ts | 8 ++--- packages/firestore/src/remote/remote_store.ts | 4 ++- .../test/integration/api/database.test.ts | 36 ++++++++----------- .../test/integration/api/query.test.ts | 12 +++---- 5 files changed, 29 insertions(+), 35 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 11a8f0a2701..5bcb2547ea2 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -187,7 +187,9 @@ export class Firestore implements firestore.Firestore, FirebaseService { // // Operations on the _firestoreClient don't block on _firestoreReady. Those // are already set to synchronize on the async queue. - _firestoreClient: FirestoreClient | undefined; + // + // This is public for testing. + public _firestoreClient: FirestoreClient | undefined; public _dataConverter: UserDataConverter; constructor(databaseIdOrApp: FirestoreDatabase | FirebaseApp) { diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index c0061f1c0d1..08a7e4648e6 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -163,8 +163,7 @@ export class FirestoreClient { /** Enables the network connection and requeues all pending operations. */ public enableNetwork(): Promise { return this.asyncQueue.schedule(() => { - this.remoteStore.enableNetwork(); - return Promise.resolve(); + return this.remoteStore.enableNetwork(); }); } @@ -322,11 +321,10 @@ export class FirestoreClient { return this.syncEngine.handleUserChange(user); } - /** Disabled the network connection. Pending operations will not complete. */ + /** Disables the network connection. Pending operations will not complete. */ public disableNetwork(): Promise { return this.asyncQueue.schedule(() => { - this.remoteStore.disableNetwork(); - return Promise.resolve(); + return this.remoteStore.disableNetwork(); }); } diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 0d8c409c1f1..d9a3cdf143c 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -236,7 +236,7 @@ export class RemoteStore { } /** Temporarily disables the network. The network can be re-enabled using enableNetwork(). */ - disableNetwork() { + disableNetwork() : Promise { this.updateAndBroadcastOnlineState(OnlineState.Failed); // NOTE: We're guaranteed not to get any further events from these streams (not even a close @@ -249,6 +249,8 @@ export class RemoteStore { this.writeStream = null; this.watchStream = null; + + return Promise.resolve(); } shutdown(): Promise { diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 97b0be3a21c..d56d9b4b823 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -525,11 +525,14 @@ apiDescribe('Database', persistence => { .disableNetwork() .then(() => { return Promise.all([ - docRef.delete(), + docRef.set({ foo: 'bar' }), firestoreClient.enableNetwork() ]); }) - .then(() => Promise.resolve()); + .then(() => docRef.get()) + .then(doc =>{ + expect(doc.data()).to.deep.equal({ foo: 'bar' }); + }); }); }); @@ -541,29 +544,20 @@ apiDescribe('Database', persistence => { return firestoreClient .disableNetwork() .then(() => { - const promises = []; - let done: () => void; - - promises.push(docRef.set({ a: 1 })); - promises.push( - new Promise(resolve => { - done = resolve; - }) - ); + const writePromise = docRef.set({foo: 'bar'}); - docRef.get().then(snapshot => { + return docRef.get().then(snapshot => { expect(snapshot.metadata.fromCache).to.be.true; - firestoreClient.enableNetwork().then(() => { - return docRef.get().then(snapshot => { - expect(snapshot.metadata.fromCache).to.be.false; - done(); + return firestoreClient.enableNetwork().then(() => { + return writePromise.then(() => { + docRef.get().then(doc => { + expect(snapshot.metadata.fromCache).to.be.false; + expect(doc.data()).to.deep.equal({foo: 'bar'}); + }); }); }); - }); - - return Promise.all(promises); - }) - .then(() => Promise.resolve()); + }) + }); }); }); }); diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 7cbd57506e1..d0c7f9ee065 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -22,6 +22,7 @@ import { asyncIt, EventsAccumulator, toDataArray } from '../../util/helpers'; import firebase from '../util/firebase_export'; import { apiDescribe, withTestCollection, withTestDbs } from '../util/helpers'; import { Firestore } from '../../../src/api/database'; +import { Deferred } from '../../../src/util/promise'; apiDescribe('Queries', persistence => { addEqualityMatcher(); @@ -457,14 +458,11 @@ apiDescribe('Queries', persistence => { return withTestCollection(persistence, /* docs= */ {}, coll => { const firestoreClient = (coll.firestore as Firestore)._firestoreClient; - let done: () => void; - const promise = new Promise(resolve => { - done = resolve; - }); + const deferred = new Deferred(); - coll.onSnapshot({ includeQueryMetadataChanges: true }, snapshot => { + const unregister = coll.onSnapshot({ includeQueryMetadataChanges: true }, snapshot => { if (!snapshot.empty && !snapshot.metadata.fromCache) { - done(); + deferred.resolve(); } }); @@ -473,7 +471,7 @@ apiDescribe('Queries', persistence => { firestoreClient.enableNetwork(); }); - return promise; + return deferred.promise.then(unregister); }); }); }); From 323422d95536e1b3a4d183b0c9b228eb46fe323a Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 11 Oct 2017 05:47:36 +0200 Subject: [PATCH 6/6] [AUTOMATED]: Prettier Code Styling --- packages/firestore/src/remote/remote_store.ts | 2 +- .../test/integration/api/database.test.ts | 34 +++++++++---------- .../test/integration/api/query.test.ts | 11 +++--- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index d9a3cdf143c..cdb753e814e 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -236,7 +236,7 @@ export class RemoteStore { } /** Temporarily disables the network. The network can be re-enabled using enableNetwork(). */ - disableNetwork() : Promise { + disableNetwork(): Promise { this.updateAndBroadcastOnlineState(OnlineState.Failed); // NOTE: We're guaranteed not to get any further events from these streams (not even a close diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index d56d9b4b823..5068d44c8f7 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -529,10 +529,10 @@ apiDescribe('Database', persistence => { firestoreClient.enableNetwork() ]); }) - .then(() => docRef.get()) - .then(doc =>{ - expect(doc.data()).to.deep.equal({ foo: 'bar' }); - }); + .then(() => docRef.get()) + .then(doc => { + expect(doc.data()).to.deep.equal({ foo: 'bar' }); + }); }); }); @@ -541,23 +541,21 @@ apiDescribe('Database', persistence => { const docRef = db.collection('rooms').doc(); const firestoreClient = (docRef.firestore as Firestore)._firestoreClient; - return firestoreClient - .disableNetwork() - .then(() => { - const writePromise = docRef.set({foo: 'bar'}); - - return docRef.get().then(snapshot => { - expect(snapshot.metadata.fromCache).to.be.true; - return firestoreClient.enableNetwork().then(() => { - return writePromise.then(() => { - docRef.get().then(doc => { - expect(snapshot.metadata.fromCache).to.be.false; - expect(doc.data()).to.deep.equal({foo: 'bar'}); - }); + return firestoreClient.disableNetwork().then(() => { + const writePromise = docRef.set({ foo: 'bar' }); + + return docRef.get().then(snapshot => { + expect(snapshot.metadata.fromCache).to.be.true; + return firestoreClient.enableNetwork().then(() => { + return writePromise.then(() => { + docRef.get().then(doc => { + expect(snapshot.metadata.fromCache).to.be.false; + expect(doc.data()).to.deep.equal({ foo: 'bar' }); }); }); - }) + }); }); + }); }); }); }); diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index d0c7f9ee065..0e5f04d6b2a 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -460,11 +460,14 @@ apiDescribe('Queries', persistence => { const deferred = new Deferred(); - const unregister = coll.onSnapshot({ includeQueryMetadataChanges: true }, snapshot => { - if (!snapshot.empty && !snapshot.metadata.fromCache) { - deferred.resolve(); + const unregister = coll.onSnapshot( + { includeQueryMetadataChanges: true }, + snapshot => { + if (!snapshot.empty && !snapshot.metadata.fromCache) { + deferred.resolve(); + } } - }); + ); firestoreClient.disableNetwork().then(() => { coll.doc().set({ a: 1 });