From 568f4f8279822032d60cf6d640272e1d4a250298 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Tue, 23 Jan 2018 11:11:48 -0500 Subject: [PATCH 1/9] GetOptions for controlling offline behaviour. Add option to allow the user to control where DocumentReference.get() and Query.get() fetches from. By default, it fetches from the server (if possible) and falls back to the local cache. It's now possible to alternatively fetch from the local cache only, or to fetch from the server only (though in the server only case, latency compensation is still enabled). --- packages/firestore-types/index.d.ts | 42 +- packages/firestore/src/api/database.ts | 27 +- .../firestore/src/core/firestore_client.ts | 48 +- .../test/integration/api/database.test.ts | 2 +- .../test/integration/api/get_options.test.ts | 595 ++++++++++++++++++ 5 files changed, 702 insertions(+), 12 deletions(-) create mode 100644 packages/firestore/test/integration/api/get_options.test.ts diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index 05ef04c4bc3..59dc405b51f 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -394,6 +394,35 @@ export interface SetOptions { readonly merge?: boolean; } +/** + * An options object that configures the behavior of `get()` calls in + * `DocumentReference` and `Query`. By providing a `GetOptions` object, these + * methods can be configured to fetch results only from the server, only from + * the local cache or attempt the server and fall back to the cache (which is + * the default). + */ +export interface GetOptions { + /** + * Describes whether we should get from server or cache. + * + * Setting to 'default' (or not setting at all), if online, causes Firestore + * to try to give a consistent (server-retrieved) snapshot, or else revert to + * the cache to provide a value. + * + * Setting to 'server' causes Firestore to avoid the cache (generating an + * error if a value cannot be retrieved from the server). The cache will be + * updated if the RPC succeeds. Latency compensation still occurs (implying + * that if the cache is more up to date, then it's values will be merged into + * the results). + * + * Setting to 'cache' causes Firestore to immediately return a value from the + * cache, ignoring the server completely (implying that the returned value + * may be stale with respect to the value on the server.) For a single + * document, the get will fail if the document doesn't exist. + */ + readonly source?: 'default' | 'server' | 'cache'; +} + /** * A `DocumentReference` refers to a document location in a Firestore database * and can be used to write, read, or listen to the location. The document at @@ -495,14 +524,16 @@ export class DocumentReference { /** * Reads the document referred to by this `DocumentReference`. * - * Note: get() attempts to provide up-to-date data when possible by waiting - * for data from the server, but it may return cached data or fail if you - * are offline and the server cannot be reached. + * Note: By default, get() attempts to provide up-to-date data when possible + * by waiting for data from the server, but it may return cached data or fail + * if you are offline and the server cannot be reached. This behavior can be + * altered via the `GetOptions` parameter. * + * @param options An object to configure the get behavior. * @return A Promise resolved with a DocumentSnapshot containing the * current document contents. */ - get(): Promise; + get(options?: GetOptions): Promise; /** * Attaches a listener for DocumentSnapshot events. You may either pass @@ -876,9 +907,10 @@ export class Query { /** * Executes the query and returns the results as a QuerySnapshot. * + * @param options An object to configure the get behavior. * @return A Promise that will be resolved with the results of the Query. */ - get(): Promise; + get(options?: GetOptions): Promise; /** * Attaches a listener for QuerySnapshot events. You may either pass diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 6deb8401dc9..41bc1c4254e 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -956,10 +956,20 @@ export class DocumentReference implements firestore.DocumentReference { }; } - get(): Promise { - validateExactNumberOfArgs('DocumentReference.get', arguments, 0); + get( + options?: firestore.GetOptions + ): Promise { + validateBetweenNumberOfArgs('DocumentReference.get', arguments, 0, 1); return new Promise( (resolve: Resolver, reject: Rejecter) => { + if (options && options.source === "cache") { + this._firestoreClient.getDocumentFromLocalCache(this, { + next: (snap: firestore.DocumentSnapshot) => resolve(snap), + error: reject + }); + return; + } + const unlisten = this.onSnapshotInternal( { includeQueryMetadataChanges: true, @@ -1566,10 +1576,19 @@ export class Query implements firestore.Query { }; } - get(): Promise { - validateExactNumberOfArgs('Query.get', arguments, 0); + get( + options?: firestore.GetOptions + ): Promise { + validateBetweenNumberOfArgs('Query.get', arguments, 0, 1); return new Promise( (resolve: Resolver, reject: Rejecter) => { + if (options && options.source === 'cache') { + this.firestore.ensureClientConfigured().getDocumentsFromLocalCache(this, { + next: (snap: firestore.QuerySnapshot) => resolve(snap), + error: reject + }); + return; + } const unlisten = this.onSnapshotInternal( { includeDocumentMetadataChanges: false, diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index bb588eeec0b..ab647d71436 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -15,6 +15,7 @@ */ import { CredentialsProvider } from '../api/credentials'; +import { DocumentReference, DocumentSnapshot, Query, QuerySnapshot } from '../api/database'; import { User } from '../auth/user'; import { EventManager, @@ -23,6 +24,7 @@ import { QueryListener } from './event_manager'; import { SyncEngine } from './sync_engine'; +import { View, ViewChange, ViewDocumentChanges } from './view'; import { EagerGarbageCollector } from '../local/eager_garbage_collector'; import { GarbageCollector } from '../local/garbage_collector'; import { IndexedDbPersistence } from '../local/indexeddb_persistence'; @@ -30,18 +32,21 @@ import { LocalStore } from '../local/local_store'; import { MemoryPersistence } from '../local/memory_persistence'; import { NoOpGarbageCollector } from '../local/no_op_garbage_collector'; import { Persistence } from '../local/persistence'; +import { DocumentKeySet, documentKeySet } from '../model/collections'; +import { Document, MaybeDocument } from '../model/document'; import { Mutation } from '../model/mutation'; import { Platform } from '../platform/platform'; import { Datastore } from '../remote/datastore'; import { RemoteStore } from '../remote/remote_store'; import { JsonProtoSerializer } from '../remote/serializer'; +import { assert } from '../util/assert'; import { AsyncQueue } from '../util/async_queue'; import { Code, FirestoreError } from '../util/error'; import { debug } from '../util/log'; import { Deferred } from '../util/promise'; import { DatabaseId, DatabaseInfo } from './database_info'; -import { Query } from './query'; +import { Query as InternalQuery } from './query'; import { Transaction } from './transaction'; import { OnlineState } from './types'; import { ViewSnapshot } from './view_snapshot'; @@ -340,7 +345,7 @@ export class FirestoreClient { } listen( - query: Query, + query: InternalQuery, observer: Observer, options: ListenOptions ): QueryListener { @@ -357,6 +362,45 @@ export class FirestoreClient { }); } + getDocumentFromLocalCache( + doc: DocumentReference, + observer: Observer + ): void { + this.asyncQueue.schedule(() => { + return this.localStore.readDocument(doc._key).then(maybeDoc => { + if (maybeDoc instanceof Document) { + observer.next(new DocumentSnapshot(doc.firestore, doc._key, maybeDoc, true)); + } else { + observer.error(new Error( + "Failed to get document from cache. (However, this document may " + + "exist on the server. Run again without setting 'source' in " + + "the GetOptions to attempt to retrieve the document from the " + + "server.)")); + } + }); + }); + } + + getDocumentsFromLocalCache( + query: Query, + observer: Observer + ): void { + this.asyncQueue.schedule(() => { + return this.localStore.executeQuery(query._query).then(docs => { + const remoteKeys:DocumentKeySet = documentKeySet(); + + const view = new View(query._query, remoteKeys); + const viewDocChanges:ViewDocumentChanges = view.computeDocChanges(docs); + const viewChange:ViewChange = view.applyChanges(viewDocChanges); + assert(viewChange.limboChanges.length === 0, "View returned limbo documents during local-only query execution."); + + const snapshot:ViewSnapshot = viewChange.snapshot; + + observer.next(new QuerySnapshot(query.firestore, query._query, snapshot)); + }); + }); + } + write(mutations: Mutation[]): Promise { const deferred = new Deferred(); this.asyncQueue.schedule(() => this.syncEngine.write(mutations, deferred)); diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index afe103b49ec..13badc8cf92 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -482,7 +482,7 @@ apiDescribe('Database', persistence => { expect(err.message).to.exist; } ) - .then(queryForRejection.get) + .then(() => queryForRejection.get()) .then( () => { throw new Error('Promise resolved even though error was expected.'); diff --git a/packages/firestore/test/integration/api/get_options.test.ts b/packages/firestore/test/integration/api/get_options.test.ts new file mode 100644 index 00000000000..dc48333729f --- /dev/null +++ b/packages/firestore/test/integration/api/get_options.test.ts @@ -0,0 +1,595 @@ +/** + * Copyright 2018 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; +import { + apiDescribe, + withTestDoc, + withTestCollection +} from '../util/helpers'; + + +apiDescribe('GetOptions', persistence => { + + it('get document while online with default get options', () => { + const initialData = { "key" : "value" }; + return withTestDoc(persistence, docRef => { + return docRef.set(initialData) + .then(() => docRef.get()) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.false; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); + }); + }); + }); + + it('get collection while online with default get options', () => { + const initialDocs = { + "doc1" : {"key1" : "value1"}, + "doc2" : {"key2" : "value2"}, + "doc3" : {"key3" : "value3"} + }; + return withTestCollection(persistence, initialDocs, colRef => { + return colRef.get() + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.false; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + expect(qrySnap.docChanges.length).to.equal(3); + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data() + }); + expect(docsData).to.deep.equal(initialDocs); + }); + }); + }); + + it('get document while offline with default get options', () => { + const initialData = { "key" : "value" }; + const newData = { "key2" : "value2" }; + return withTestDoc(persistence, docRef => { + return docRef.set(initialData) + .then(() => docRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promise won't complete + docRef.set(newData); + return docRef.get(); + }).then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.true; + expect(doc.data()).to.deep.equal(newData); + }); + }); + }); + + it('get collection while offline with default get options', () => { + const initialDocs = { + "doc1" : {"key1" : "value1"}, + "doc2" : {"key2" : "value2"}, + "doc3" : {"key3" : "value3"} + }; + return withTestCollection(persistence, initialDocs, colRef => { + // force local cache of these + return colRef.get() + // now go offine. Note that if persistence is disabled, this will cause + // the initialDocs to be garbage collected. + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promises won't complete + colRef.doc("doc2").set({"key2b":"value2b"}, {merge:true}); + colRef.doc("doc3").set({"key3b":"value3b"}); + colRef.doc("doc4").set({"key4":"value4"}); + return colRef.get(); + }).then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data() + }); + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + "doc1" : { "key1": "value1" }, + "doc2" : { "key2": "value2", "key2b": "value2b" }, + "doc3" : { "key3b" : "value3b"}, + "doc4": {"key4":"value4"} + }); + } else { + expect(qrySnap.docChanges.length).to.equal(3); + expect(docsData).to.deep.equal({ + "doc2" : { "key2b": "value2b" }, + "doc3" : { "key3b" : "value3b"}, + "doc4": {"key4":"value4"} + }); + } + }); + }); + }); + + it('get document while online cache only', () => { + const initialData = { 'key': 'value'}; + return withTestDoc(persistence, docRef => { + return docRef.set(initialData) + .then(() => docRef.get({source: 'cache'})) + .then(doc => { + if (persistence) { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); + } else { + expect.fail(); + } + }) + .catch(err => { + // we expect an error when persistence has been disabled (since the + // document won't be in the local cache.) + expect(persistence).to.be.false; + }); + }); + }); + + it('get collection while online cache only', () => { + const initialDocs = { + "doc1" : {"key1" : "value1"}, + "doc2" : {"key2" : "value2"}, + "doc3" : {"key3" : "value3"} + }; + return withTestCollection(persistence, initialDocs, colRef => { + // force local cache of these + return colRef.get() + .then(ignored => colRef.get({source: 'cache'})) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(3); + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data() + }); + expect(docsData).to.deep.equal(initialDocs); + } else { + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.empty).to.be.true; + } + }); + }); + }); + + it('get document while offline cache only', () => { + const initialData = { 'key': 'value'}; + const newData = { "key2" : "value2" }; + return withTestDoc(persistence, docRef => { + return docRef.set(initialData) + .then(() => docRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promise won't complete + docRef.set(newData); + return docRef.get({source: 'cache'}); + }).then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.true; + expect(doc.data()).to.deep.equal(newData); + }); + }); + }); + + it('get collection while offline cache only', () => { + const initialDocs = { + "doc1" : {"key1" : "value1"}, + "doc2" : {"key2" : "value2"}, + "doc3" : {"key3" : "value3"} + }; + return withTestCollection(persistence, initialDocs, colRef => { + // force local cache of these + return colRef.get() + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promises won't complete + colRef.doc("doc2").set({"key2b":"value2b"}, {merge:true}); + colRef.doc("doc3").set({"key3b":"value3b"}); + colRef.doc("doc4").set({"key4":"value4"}); + return colRef.get({source: 'cache'}); + }).then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data() + }); + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + "doc1" : { "key1": "value1" }, + "doc2" : { "key2": "value2", "key2b": "value2b" }, + "doc3" : { "key3b" : "value3b"}, + "doc4": {"key4":"value4"} + }); + } else { + expect(qrySnap.docChanges.length).to.equal(3); + expect(docsData).to.deep.equal({ + "doc2" : { "key2b": "value2b" }, + "doc3" : { "key3b" : "value3b"}, + "doc4": {"key4":"value4"} + }); + } + }); + }); + }); + + it('get document while online server only', () => { + const initialData = { "key" : "value" }; + return withTestDoc(persistence, docRef => { + return docRef.set(initialData) + .then(() => docRef.get({source: 'server'})) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.false; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); + }); + }); + }); + + it('get collection while online server only', () => { + const initialDocs = { + "doc1" : {"key1" : "value1"}, + "doc2" : {"key2" : "value2"}, + "doc3" : {"key3" : "value3"} + }; + return withTestCollection(persistence, initialDocs, colRef => { + return colRef.get({source: 'server'}) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.false; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + expect(qrySnap.docChanges.length).to.equal(3); + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data() + }); + expect(docsData).to.deep.equal(initialDocs); + }); + }); + }); + + it('get document while offline server only', () => { + const initialData = { 'key': 'value'}; + return withTestDoc(persistence, docRef => { + return docRef.set(initialData) + .then(() => docRef.firestore.disableNetwork()) + .then(() => docRef.get({source: 'server'})) + .then(doc => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); + + it('get collection while offline server only', () => { + const initialDocs = { + "doc1" : {"key1" : "value1"}, + "doc2" : {"key2" : "value2"}, + "doc3" : {"key3" : "value3"} + }; + return withTestCollection(persistence, initialDocs, colRef => { + // force local cache of these + return colRef.get() + // now go offine. Note that if persistence is disabled, this will cause + // the initialDocs to be garbage collected. + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => colRef.get({source: 'server'})) + .then(qrySnap => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); + + it('get document while offline with different get options', () => { + const initialData = { 'key': 'value'}; + const newData = { "key2" : "value2" }; + return withTestDoc(persistence, docRef => { + return docRef.set(initialData) + .then(() => docRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promise won't complete + docRef.set(newData); + + // Create an initial listener for this query (to attempt to disrupt the + // gets below) and wait for the listener to deliver its initial + // snapshot before continuing. + return new Promise((resolve, reject) => { + docRef.onSnapshot( docSnap => { + resolve(); + }, error => { + reject(); + }); + }); + }) + .then(() => docRef.get({source: 'cache'})) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.true; + expect(doc.data()).to.deep.equal(newData); + return Promise.resolve(); + }) + .then(() => docRef.get()) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.true; + expect(doc.data()).to.deep.equal(newData); + return Promise.resolve(); + }) + .then(() => docRef.get({source: 'server'})) + .then(doc => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); + + it('get collection while offline with different get options', () => { + const initialDocs = { + "doc1" : {"key1" : "value1"}, + "doc2" : {"key2" : "value2"}, + "doc3" : {"key3" : "value3"} + }; + return withTestCollection(persistence, initialDocs, colRef => { + // force local cache of these + return colRef.get() + // now go offine. Note that if persistence is disabled, this will cause + // the initialDocs to be garbage collected. + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promises won't complete + colRef.doc("doc2").set({"key2b":"value2b"}, {merge:true}); + colRef.doc("doc3").set({"key3b":"value3b"}); + colRef.doc("doc4").set({"key4":"value4"}); + + // Create an initial listener for this query (to attempt to disrupt the + // gets below) and wait for the listener to deliver its initial + // snapshot before continuing. + return new Promise((resolve, reject) => { + colRef.onSnapshot( qrySnap => { + resolve(); + }, error => { + reject(); + }); + }); + }) + .then(() => colRef.get({source: 'cache'})) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data() + }); + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + "doc1" : { "key1": "value1" }, + "doc2" : { "key2": "value2", "key2b": "value2b" }, + "doc3" : { "key3b" : "value3b"}, + "doc4": {"key4":"value4"} + }); + } else { + expect(qrySnap.docChanges.length).to.equal(3); + expect(docsData).to.deep.equal({ + "doc2" : { "key2b": "value2b" }, + "doc3" : { "key3b" : "value3b"}, + "doc4": {"key4":"value4"} + }); + } + }) + .then(() => colRef.get()) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data() + }); + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + "doc1" : { "key1": "value1" }, + "doc2" : { "key2": "value2", "key2b": "value2b" }, + "doc3" : { "key3b" : "value3b"}, + "doc4": {"key4":"value4"} + }); + } else { + expect(qrySnap.docChanges.length).to.equal(3); + expect(docsData).to.deep.equal({ + "doc2" : { "key2b": "value2b" }, + "doc3" : { "key3b" : "value3b"}, + "doc4": {"key4":"value4"} + }); + } + }) + .then(() => colRef.get({source: 'server'})) + .then(qrySnap => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); + + it('get non existing doc while online with default get options', () => { + return withTestDoc(persistence, docRef => { + return docRef.get() + .then(doc => { + expect(doc.exists).to.be.false; + expect(doc.metadata.fromCache).to.be.false; + expect(doc.metadata.hasPendingWrites).to.be.false + }); + }); + }); + + it('get non existing collection while online with default get options', () => { + return withTestCollection(persistence, {}, colRef => { + return colRef.get() + .then(qrySnap => { + //expect(qrySnap.count).to.equal(0); + expect(qrySnap.empty).to.be.true; + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.metadata.fromCache).to.be.false; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + }); + }); + }); + + it('get non existing doc while offline with default get options', () => { + return withTestDoc(persistence, docRef => { + return docRef.firestore.disableNetwork() + .then(() => docRef.get()) + .then(doc => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); + + it('get non existing collection while offline with default get options', () => { + return withTestCollection(persistence, {}, colRef => { + return colRef.firestore.disableNetwork() + .then(() => colRef.get()) + .then(qrySnap => { + expect(qrySnap.empty).to.be.true; + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + }); + }); + }); + + it('get non existing doc while online cache only', () => { + return withTestDoc(persistence, docRef => { + // attempt to get doc. Currently, this is expected to fail. In the + // future, we might consider adding support for negative cache hits so + // that we know certain documents *don't* exist. + return docRef.get({source: 'cache'}) + .then(doc => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); + + it('get non existing collection while online cache only', () => { + return withTestCollection(persistence, {}, colRef => { + return colRef.get({source: 'cache'}) + .then(qrySnap => { + expect(qrySnap.empty).to.be.true; + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + }); + }); + }); + + it('get non existing doc while offline cache only', () => { + return withTestDoc(persistence, docRef => { + return docRef.firestore.disableNetwork() + // attempt to get doc. Currently, this is expected to fail. In the + // future, we might consider adding support for negative cache hits so + // that we know certain documents *don't* exist. + .then(() => docRef.get({source: 'cache'})) + .then(doc => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); + + it('get non existing collection while offline cache only', () => { + return withTestCollection(persistence, {}, colRef => { + return colRef.firestore.disableNetwork() + .then(() => colRef.get({source: 'cache'})) + .then(qrySnap => { + expect(qrySnap.empty).to.be.true; + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + }); + }); + }); + + it('get non existing doc while online server only', () => { + return withTestDoc(persistence, docRef => { + return docRef.get({source: 'server'}) + .then(doc => { + expect(doc.exists).to.be.false; + expect(doc.metadata.fromCache).to.be.false; + expect(doc.metadata.hasPendingWrites).to.be.false; + }); + }); + }); + + it('get non existing collection while online server only', () => { + return withTestCollection(persistence, {}, colRef => { + return colRef.get({source: 'server'}) + .then(qrySnap => { + expect(qrySnap.empty).to.be.true; + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.metadata.fromCache).to.be.false; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + }); + }); + }); + + it('get non existing doc while offline server only', () => { + return withTestDoc(persistence, docRef => { + return docRef.firestore.disableNetwork() + // attempt to get doc. Currently, this is expected to fail. In the + // future, we might consider adding support for negative cache hits so + // that we know certain documents *don't* exist. + .then(() => docRef.get({source: 'server'})) + .then(doc => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); + + it('get non existing collection while offline server only', () => { + return withTestCollection(persistence, {}, colRef => { + return colRef.firestore.disableNetwork() + .then(() => colRef.get({source: 'server'})) + .then(qrySnap => { + expect.fail(); + }) + .catch(expected => { + }); + }); + }); +}); From 56b4b9122bd9423b33599bfd7888a974bb71f0de Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Tue, 23 Jan 2018 11:47:50 -0500 Subject: [PATCH 2/9] [AUTOMATED]: Prettier Code Styling --- packages/firestore/src/api/database.ts | 20 +- .../firestore/src/core/firestore_client.ts | 43 +- .../test/integration/api/get_options.test.ts | 771 +++++++++--------- 3 files changed, 433 insertions(+), 401 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 41bc1c4254e..c2fc167615c 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -956,13 +956,11 @@ export class DocumentReference implements firestore.DocumentReference { }; } - get( - options?: firestore.GetOptions - ): Promise { + get(options?: firestore.GetOptions): Promise { validateBetweenNumberOfArgs('DocumentReference.get', arguments, 0, 1); return new Promise( (resolve: Resolver, reject: Rejecter) => { - if (options && options.source === "cache") { + if (options && options.source === 'cache') { this._firestoreClient.getDocumentFromLocalCache(this, { next: (snap: firestore.DocumentSnapshot) => resolve(snap), error: reject @@ -1576,17 +1574,17 @@ export class Query implements firestore.Query { }; } - get( - options?: firestore.GetOptions - ): Promise { + get(options?: firestore.GetOptions): Promise { validateBetweenNumberOfArgs('Query.get', arguments, 0, 1); return new Promise( (resolve: Resolver, reject: Rejecter) => { if (options && options.source === 'cache') { - this.firestore.ensureClientConfigured().getDocumentsFromLocalCache(this, { - next: (snap: firestore.QuerySnapshot) => resolve(snap), - error: reject - }); + this.firestore + .ensureClientConfigured() + .getDocumentsFromLocalCache(this, { + next: (snap: firestore.QuerySnapshot) => resolve(snap), + error: reject + }); return; } const unlisten = this.onSnapshotInternal( diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index ab647d71436..5e5abfca68e 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -15,7 +15,12 @@ */ import { CredentialsProvider } from '../api/credentials'; -import { DocumentReference, DocumentSnapshot, Query, QuerySnapshot } from '../api/database'; +import { + DocumentReference, + DocumentSnapshot, + Query, + QuerySnapshot +} from '../api/database'; import { User } from '../auth/user'; import { EventManager, @@ -369,13 +374,18 @@ export class FirestoreClient { this.asyncQueue.schedule(() => { return this.localStore.readDocument(doc._key).then(maybeDoc => { if (maybeDoc instanceof Document) { - observer.next(new DocumentSnapshot(doc.firestore, doc._key, maybeDoc, true)); + observer.next( + new DocumentSnapshot(doc.firestore, doc._key, maybeDoc, true) + ); } else { - observer.error(new Error( - "Failed to get document from cache. (However, this document may " - + "exist on the server. Run again without setting 'source' in " - + "the GetOptions to attempt to retrieve the document from the " - + "server.)")); + observer.error( + new Error( + 'Failed to get document from cache. (However, this document may ' + + "exist on the server. Run again without setting 'source' in " + + 'the GetOptions to attempt to retrieve the document from the ' + + 'server.)' + ) + ); } }); }); @@ -387,16 +397,23 @@ export class FirestoreClient { ): void { this.asyncQueue.schedule(() => { return this.localStore.executeQuery(query._query).then(docs => { - const remoteKeys:DocumentKeySet = documentKeySet(); + const remoteKeys: DocumentKeySet = documentKeySet(); const view = new View(query._query, remoteKeys); - const viewDocChanges:ViewDocumentChanges = view.computeDocChanges(docs); - const viewChange:ViewChange = view.applyChanges(viewDocChanges); - assert(viewChange.limboChanges.length === 0, "View returned limbo documents during local-only query execution."); + const viewDocChanges: ViewDocumentChanges = view.computeDocChanges( + docs + ); + const viewChange: ViewChange = view.applyChanges(viewDocChanges); + assert( + viewChange.limboChanges.length === 0, + 'View returned limbo documents during local-only query execution.' + ); - const snapshot:ViewSnapshot = viewChange.snapshot; + const snapshot: ViewSnapshot = viewChange.snapshot; - observer.next(new QuerySnapshot(query.firestore, query._query, snapshot)); + observer.next( + new QuerySnapshot(query.firestore, query._query, snapshot) + ); }); }); } diff --git a/packages/firestore/test/integration/api/get_options.test.ts b/packages/firestore/test/integration/api/get_options.test.ts index dc48333729f..75ebea15adf 100644 --- a/packages/firestore/test/integration/api/get_options.test.ts +++ b/packages/firestore/test/integration/api/get_options.test.ts @@ -15,44 +15,38 @@ */ import { expect } from 'chai'; -import { - apiDescribe, - withTestDoc, - withTestCollection -} from '../util/helpers'; - +import { apiDescribe, withTestDoc, withTestCollection } from '../util/helpers'; apiDescribe('GetOptions', persistence => { - it('get document while online with default get options', () => { - const initialData = { "key" : "value" }; + const initialData = { key: 'value' }; return withTestDoc(persistence, docRef => { - return docRef.set(initialData) - .then(() => docRef.get()) - .then(doc => { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.false; - expect(doc.metadata.hasPendingWrites).to.be.false; - expect(doc.data()).to.deep.equal(initialData); - }); + return docRef + .set(initialData) + .then(() => docRef.get()) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.false; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); + }); }); }); it('get collection while online with default get options', () => { const initialDocs = { - "doc1" : {"key1" : "value1"}, - "doc2" : {"key2" : "value2"}, - "doc3" : {"key3" : "value3"} + doc1: { key1: 'value1' }, + doc2: { key2: 'value2' }, + doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { - return colRef.get() - .then(qrySnap => { + return colRef.get().then(qrySnap => { expect(qrySnap.metadata.fromCache).to.be.false; expect(qrySnap.metadata.hasPendingWrites).to.be.false; expect(qrySnap.docChanges.length).to.equal(3); let docsData = {}; qrySnap.forEach(doc => { - docsData[doc.id] = doc.data() + docsData[doc.id] = doc.data(); }); expect(docsData).to.deep.equal(initialDocs); }); @@ -60,211 +54,223 @@ apiDescribe('GetOptions', persistence => { }); it('get document while offline with default get options', () => { - const initialData = { "key" : "value" }; - const newData = { "key2" : "value2" }; + const initialData = { key: 'value' }; + const newData = { key2: 'value2' }; return withTestDoc(persistence, docRef => { - return docRef.set(initialData) - .then(() => docRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promise won't complete - docRef.set(newData); - return docRef.get(); - }).then(doc => { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.true; - expect(doc.data()).to.deep.equal(newData); - }); + return docRef + .set(initialData) + .then(() => docRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promise won't complete + docRef.set(newData); + return docRef.get(); + }) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.true; + expect(doc.data()).to.deep.equal(newData); + }); }); }); it('get collection while offline with default get options', () => { const initialDocs = { - "doc1" : {"key1" : "value1"}, - "doc2" : {"key2" : "value2"}, - "doc3" : {"key3" : "value3"} + doc1: { key1: 'value1' }, + doc2: { key2: 'value2' }, + doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { // force local cache of these - return colRef.get() - // now go offine. Note that if persistence is disabled, this will cause - // the initialDocs to be garbage collected. - .then(ignored => colRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promises won't complete - colRef.doc("doc2").set({"key2b":"value2b"}, {merge:true}); - colRef.doc("doc3").set({"key3b":"value3b"}); - colRef.doc("doc4").set({"key4":"value4"}); - return colRef.get(); - }).then(qrySnap => { - expect(qrySnap.metadata.fromCache).to.be.true; - expect(qrySnap.metadata.hasPendingWrites).to.be.true; - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data() - }); - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(4); - expect(docsData).to.deep.equal({ - "doc1" : { "key1": "value1" }, - "doc2" : { "key2": "value2", "key2b": "value2b" }, - "doc3" : { "key3b" : "value3b"}, - "doc4": {"key4":"value4"} - }); - } else { - expect(qrySnap.docChanges.length).to.equal(3); - expect(docsData).to.deep.equal({ - "doc2" : { "key2b": "value2b" }, - "doc3" : { "key3b" : "value3b"}, - "doc4": {"key4":"value4"} - }); - } - }); + return ( + colRef + .get() + // now go offine. Note that if persistence is disabled, this will cause + // the initialDocs to be garbage collected. + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promises won't complete + colRef.doc('doc2').set({ key2b: 'value2b' }, { merge: true }); + colRef.doc('doc3').set({ key3b: 'value3b' }); + colRef.doc('doc4').set({ key4: 'value4' }); + return colRef.get(); + }) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data(); + }); + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + doc1: { key1: 'value1' }, + doc2: { key2: 'value2', key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + } else { + expect(qrySnap.docChanges.length).to.equal(3); + expect(docsData).to.deep.equal({ + doc2: { key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + } + }) + ); }); }); it('get document while online cache only', () => { - const initialData = { 'key': 'value'}; + const initialData = { key: 'value' }; return withTestDoc(persistence, docRef => { - return docRef.set(initialData) - .then(() => docRef.get({source: 'cache'})) - .then(doc => { - if (persistence) { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.false; - expect(doc.data()).to.deep.equal(initialData); - } else { - expect.fail(); - } - }) - .catch(err => { - // we expect an error when persistence has been disabled (since the - // document won't be in the local cache.) - expect(persistence).to.be.false; - }); + return docRef + .set(initialData) + .then(() => docRef.get({ source: 'cache' })) + .then(doc => { + if (persistence) { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); + } else { + expect.fail(); + } + }) + .catch(err => { + // we expect an error when persistence has been disabled (since the + // document won't be in the local cache.) + expect(persistence).to.be.false; + }); }); }); it('get collection while online cache only', () => { const initialDocs = { - "doc1" : {"key1" : "value1"}, - "doc2" : {"key2" : "value2"}, - "doc3" : {"key3" : "value3"} + doc1: { key1: 'value1' }, + doc2: { key2: 'value2' }, + doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { // force local cache of these - return colRef.get() - .then(ignored => colRef.get({source: 'cache'})) - .then(qrySnap => { - expect(qrySnap.metadata.fromCache).to.be.true; - expect(qrySnap.metadata.hasPendingWrites).to.be.false; - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(3); - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data() - }); - expect(docsData).to.deep.equal(initialDocs); - } else { - expect(qrySnap.docChanges.length).to.equal(0); - expect(qrySnap.empty).to.be.true; - } - }); + return colRef + .get() + .then(ignored => colRef.get({ source: 'cache' })) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(3); + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data(); + }); + expect(docsData).to.deep.equal(initialDocs); + } else { + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.empty).to.be.true; + } + }); }); }); it('get document while offline cache only', () => { - const initialData = { 'key': 'value'}; - const newData = { "key2" : "value2" }; + const initialData = { key: 'value' }; + const newData = { key2: 'value2' }; return withTestDoc(persistence, docRef => { - return docRef.set(initialData) - .then(() => docRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promise won't complete - docRef.set(newData); - return docRef.get({source: 'cache'}); - }).then(doc => { + return docRef + .set(initialData) + .then(() => docRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promise won't complete + docRef.set(newData); + return docRef.get({ source: 'cache' }); + }) + .then(doc => { expect(doc.exists).to.be.true; expect(doc.metadata.fromCache).to.be.true; expect(doc.metadata.hasPendingWrites).to.be.true; expect(doc.data()).to.deep.equal(newData); - }); + }); }); }); it('get collection while offline cache only', () => { const initialDocs = { - "doc1" : {"key1" : "value1"}, - "doc2" : {"key2" : "value2"}, - "doc3" : {"key3" : "value3"} + doc1: { key1: 'value1' }, + doc2: { key2: 'value2' }, + doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { // force local cache of these - return colRef.get() - .then(ignored => colRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promises won't complete - colRef.doc("doc2").set({"key2b":"value2b"}, {merge:true}); - colRef.doc("doc3").set({"key3b":"value3b"}); - colRef.doc("doc4").set({"key4":"value4"}); - return colRef.get({source: 'cache'}); - }).then(qrySnap => { - expect(qrySnap.metadata.fromCache).to.be.true; - expect(qrySnap.metadata.hasPendingWrites).to.be.true; - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data() - }); - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(4); - expect(docsData).to.deep.equal({ - "doc1" : { "key1": "value1" }, - "doc2" : { "key2": "value2", "key2b": "value2b" }, - "doc3" : { "key3b" : "value3b"}, - "doc4": {"key4":"value4"} - }); - } else { - expect(qrySnap.docChanges.length).to.equal(3); - expect(docsData).to.deep.equal({ - "doc2" : { "key2b": "value2b" }, - "doc3" : { "key3b" : "value3b"}, - "doc4": {"key4":"value4"} + return colRef + .get() + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promises won't complete + colRef.doc('doc2').set({ key2b: 'value2b' }, { merge: true }); + colRef.doc('doc3').set({ key3b: 'value3b' }); + colRef.doc('doc4').set({ key4: 'value4' }); + return colRef.get({ source: 'cache' }); + }) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data(); }); - } - }); + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + doc1: { key1: 'value1' }, + doc2: { key2: 'value2', key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + } else { + expect(qrySnap.docChanges.length).to.equal(3); + expect(docsData).to.deep.equal({ + doc2: { key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + } + }); }); }); it('get document while online server only', () => { - const initialData = { "key" : "value" }; + const initialData = { key: 'value' }; return withTestDoc(persistence, docRef => { - return docRef.set(initialData) - .then(() => docRef.get({source: 'server'})) - .then(doc => { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.false; - expect(doc.metadata.hasPendingWrites).to.be.false; - expect(doc.data()).to.deep.equal(initialData); - }); + return docRef + .set(initialData) + .then(() => docRef.get({ source: 'server' })) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.false; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); + }); }); }); it('get collection while online server only', () => { const initialDocs = { - "doc1" : {"key1" : "value1"}, - "doc2" : {"key2" : "value2"}, - "doc3" : {"key3" : "value3"} + doc1: { key1: 'value1' }, + doc2: { key2: 'value2' }, + doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { - return colRef.get({source: 'server'}) - .then(qrySnap => { + return colRef.get({ source: 'server' }).then(qrySnap => { expect(qrySnap.metadata.fromCache).to.be.false; expect(qrySnap.metadata.hasPendingWrites).to.be.false; expect(qrySnap.docChanges.length).to.equal(3); let docsData = {}; qrySnap.forEach(doc => { - docsData[doc.id] = doc.data() + docsData[doc.id] = doc.data(); }); expect(docsData).to.deep.equal(initialDocs); }); @@ -272,189 +278,197 @@ apiDescribe('GetOptions', persistence => { }); it('get document while offline server only', () => { - const initialData = { 'key': 'value'}; + const initialData = { key: 'value' }; return withTestDoc(persistence, docRef => { - return docRef.set(initialData) - .then(() => docRef.firestore.disableNetwork()) - .then(() => docRef.get({source: 'server'})) - .then(doc => { - expect.fail(); - }) - .catch(expected => { - }); + return docRef + .set(initialData) + .then(() => docRef.firestore.disableNetwork()) + .then(() => docRef.get({ source: 'server' })) + .then(doc => { + expect.fail(); + }) + .catch(expected => {}); }); }); it('get collection while offline server only', () => { const initialDocs = { - "doc1" : {"key1" : "value1"}, - "doc2" : {"key2" : "value2"}, - "doc3" : {"key3" : "value3"} + doc1: { key1: 'value1' }, + doc2: { key2: 'value2' }, + doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { // force local cache of these - return colRef.get() - // now go offine. Note that if persistence is disabled, this will cause - // the initialDocs to be garbage collected. - .then(ignored => colRef.firestore.disableNetwork()) - .then(() => colRef.get({source: 'server'})) - .then(qrySnap => { - expect.fail(); - }) - .catch(expected => { - }); + return ( + colRef + .get() + // now go offine. Note that if persistence is disabled, this will cause + // the initialDocs to be garbage collected. + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => colRef.get({ source: 'server' })) + .then(qrySnap => { + expect.fail(); + }) + .catch(expected => {}) + ); }); }); it('get document while offline with different get options', () => { - const initialData = { 'key': 'value'}; - const newData = { "key2" : "value2" }; + const initialData = { key: 'value' }; + const newData = { key2: 'value2' }; return withTestDoc(persistence, docRef => { - return docRef.set(initialData) - .then(() => docRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promise won't complete - docRef.set(newData); + return docRef + .set(initialData) + .then(() => docRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promise won't complete + docRef.set(newData); - // Create an initial listener for this query (to attempt to disrupt the - // gets below) and wait for the listener to deliver its initial - // snapshot before continuing. - return new Promise((resolve, reject) => { - docRef.onSnapshot( docSnap => { - resolve(); - }, error => { - reject(); + // Create an initial listener for this query (to attempt to disrupt the + // gets below) and wait for the listener to deliver its initial + // snapshot before continuing. + return new Promise((resolve, reject) => { + docRef.onSnapshot( + docSnap => { + resolve(); + }, + error => { + reject(); + } + ); }); - }); - }) - .then(() => docRef.get({source: 'cache'})) - .then(doc => { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.true; - expect(doc.data()).to.deep.equal(newData); - return Promise.resolve(); - }) - .then(() => docRef.get()) - .then(doc => { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.true; - expect(doc.data()).to.deep.equal(newData); - return Promise.resolve(); - }) - .then(() => docRef.get({source: 'server'})) - .then(doc => { - expect.fail(); - }) - .catch(expected => { - }); + }) + .then(() => docRef.get({ source: 'cache' })) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.true; + expect(doc.data()).to.deep.equal(newData); + return Promise.resolve(); + }) + .then(() => docRef.get()) + .then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.true; + expect(doc.data()).to.deep.equal(newData); + return Promise.resolve(); + }) + .then(() => docRef.get({ source: 'server' })) + .then(doc => { + expect.fail(); + }) + .catch(expected => {}); }); }); it('get collection while offline with different get options', () => { const initialDocs = { - "doc1" : {"key1" : "value1"}, - "doc2" : {"key2" : "value2"}, - "doc3" : {"key3" : "value3"} + doc1: { key1: 'value1' }, + doc2: { key2: 'value2' }, + doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { // force local cache of these - return colRef.get() - // now go offine. Note that if persistence is disabled, this will cause - // the initialDocs to be garbage collected. - .then(ignored => colRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promises won't complete - colRef.doc("doc2").set({"key2b":"value2b"}, {merge:true}); - colRef.doc("doc3").set({"key3b":"value3b"}); - colRef.doc("doc4").set({"key4":"value4"}); + return ( + colRef + .get() + // now go offine. Note that if persistence is disabled, this will cause + // the initialDocs to be garbage collected. + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promises won't complete + colRef.doc('doc2').set({ key2b: 'value2b' }, { merge: true }); + colRef.doc('doc3').set({ key3b: 'value3b' }); + colRef.doc('doc4').set({ key4: 'value4' }); - // Create an initial listener for this query (to attempt to disrupt the - // gets below) and wait for the listener to deliver its initial - // snapshot before continuing. - return new Promise((resolve, reject) => { - colRef.onSnapshot( qrySnap => { - resolve(); - }, error => { - reject(); - }); - }); - }) - .then(() => colRef.get({source: 'cache'})) - .then(qrySnap => { - expect(qrySnap.metadata.fromCache).to.be.true; - expect(qrySnap.metadata.hasPendingWrites).to.be.true; - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data() - }); - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(4); - expect(docsData).to.deep.equal({ - "doc1" : { "key1": "value1" }, - "doc2" : { "key2": "value2", "key2b": "value2b" }, - "doc3" : { "key3b" : "value3b"}, - "doc4": {"key4":"value4"} - }); - } else { - expect(qrySnap.docChanges.length).to.equal(3); - expect(docsData).to.deep.equal({ - "doc2" : { "key2b": "value2b" }, - "doc3" : { "key3b" : "value3b"}, - "doc4": {"key4":"value4"} - }); - } - }) - .then(() => colRef.get()) - .then(qrySnap => { - expect(qrySnap.metadata.fromCache).to.be.true; - expect(qrySnap.metadata.hasPendingWrites).to.be.true; - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data() - }); - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(4); - expect(docsData).to.deep.equal({ - "doc1" : { "key1": "value1" }, - "doc2" : { "key2": "value2", "key2b": "value2b" }, - "doc3" : { "key3b" : "value3b"}, - "doc4": {"key4":"value4"} - }); - } else { - expect(qrySnap.docChanges.length).to.equal(3); - expect(docsData).to.deep.equal({ - "doc2" : { "key2b": "value2b" }, - "doc3" : { "key3b" : "value3b"}, - "doc4": {"key4":"value4"} - }); - } - }) - .then(() => colRef.get({source: 'server'})) - .then(qrySnap => { - expect.fail(); - }) - .catch(expected => { - }); + // Create an initial listener for this query (to attempt to disrupt the + // gets below) and wait for the listener to deliver its initial + // snapshot before continuing. + return new Promise((resolve, reject) => { + colRef.onSnapshot( + qrySnap => { + resolve(); + }, + error => { + reject(); + } + ); + }); + }) + .then(() => colRef.get({ source: 'cache' })) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data(); + }); + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + doc1: { key1: 'value1' }, + doc2: { key2: 'value2', key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + } else { + expect(qrySnap.docChanges.length).to.equal(3); + expect(docsData).to.deep.equal({ + doc2: { key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + } + }) + .then(() => colRef.get()) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + let docsData = {}; + qrySnap.forEach(doc => { + docsData[doc.id] = doc.data(); + }); + if (persistence) { + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + doc1: { key1: 'value1' }, + doc2: { key2: 'value2', key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + } else { + expect(qrySnap.docChanges.length).to.equal(3); + expect(docsData).to.deep.equal({ + doc2: { key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + } + }) + .then(() => colRef.get({ source: 'server' })) + .then(qrySnap => { + expect.fail(); + }) + .catch(expected => {}) + ); }); }); it('get non existing doc while online with default get options', () => { return withTestDoc(persistence, docRef => { - return docRef.get() - .then(doc => { + return docRef.get().then(doc => { expect(doc.exists).to.be.false; expect(doc.metadata.fromCache).to.be.false; - expect(doc.metadata.hasPendingWrites).to.be.false + expect(doc.metadata.hasPendingWrites).to.be.false; }); }); }); it('get non existing collection while online with default get options', () => { return withTestCollection(persistence, {}, colRef => { - return colRef.get() - .then(qrySnap => { + return colRef.get().then(qrySnap => { //expect(qrySnap.count).to.equal(0); expect(qrySnap.empty).to.be.true; expect(qrySnap.docChanges.length).to.equal(0); @@ -466,26 +480,27 @@ apiDescribe('GetOptions', persistence => { it('get non existing doc while offline with default get options', () => { return withTestDoc(persistence, docRef => { - return docRef.firestore.disableNetwork() - .then(() => docRef.get()) - .then(doc => { - expect.fail(); - }) - .catch(expected => { - }); + return docRef.firestore + .disableNetwork() + .then(() => docRef.get()) + .then(doc => { + expect.fail(); + }) + .catch(expected => {}); }); }); it('get non existing collection while offline with default get options', () => { return withTestCollection(persistence, {}, colRef => { - return colRef.firestore.disableNetwork() - .then(() => colRef.get()) - .then(qrySnap => { - expect(qrySnap.empty).to.be.true; - expect(qrySnap.docChanges.length).to.equal(0); - expect(qrySnap.metadata.fromCache).to.be.true; - expect(qrySnap.metadata.hasPendingWrites).to.be.false; - }); + return colRef.firestore + .disableNetwork() + .then(() => colRef.get()) + .then(qrySnap => { + expect(qrySnap.empty).to.be.true; + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + }); }); }); @@ -494,19 +509,18 @@ apiDescribe('GetOptions', persistence => { // attempt to get doc. Currently, this is expected to fail. In the // future, we might consider adding support for negative cache hits so // that we know certain documents *don't* exist. - return docRef.get({source: 'cache'}) - .then(doc => { - expect.fail(); - }) - .catch(expected => { - }); + return docRef + .get({ source: 'cache' }) + .then(doc => { + expect.fail(); + }) + .catch(expected => {}); }); }); it('get non existing collection while online cache only', () => { return withTestCollection(persistence, {}, colRef => { - return colRef.get({source: 'cache'}) - .then(qrySnap => { + return colRef.get({ source: 'cache' }).then(qrySnap => { expect(qrySnap.empty).to.be.true; expect(qrySnap.docChanges.length).to.equal(0); expect(qrySnap.metadata.fromCache).to.be.true; @@ -517,36 +531,38 @@ apiDescribe('GetOptions', persistence => { it('get non existing doc while offline cache only', () => { return withTestDoc(persistence, docRef => { - return docRef.firestore.disableNetwork() - // attempt to get doc. Currently, this is expected to fail. In the - // future, we might consider adding support for negative cache hits so - // that we know certain documents *don't* exist. - .then(() => docRef.get({source: 'cache'})) - .then(doc => { - expect.fail(); - }) - .catch(expected => { - }); + return ( + docRef.firestore + .disableNetwork() + // attempt to get doc. Currently, this is expected to fail. In the + // future, we might consider adding support for negative cache hits so + // that we know certain documents *don't* exist. + .then(() => docRef.get({ source: 'cache' })) + .then(doc => { + expect.fail(); + }) + .catch(expected => {}) + ); }); }); it('get non existing collection while offline cache only', () => { return withTestCollection(persistence, {}, colRef => { - return colRef.firestore.disableNetwork() - .then(() => colRef.get({source: 'cache'})) - .then(qrySnap => { - expect(qrySnap.empty).to.be.true; - expect(qrySnap.docChanges.length).to.equal(0); - expect(qrySnap.metadata.fromCache).to.be.true; - expect(qrySnap.metadata.hasPendingWrites).to.be.false; - }); + return colRef.firestore + .disableNetwork() + .then(() => colRef.get({ source: 'cache' })) + .then(qrySnap => { + expect(qrySnap.empty).to.be.true; + expect(qrySnap.docChanges.length).to.equal(0); + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.false; + }); }); }); it('get non existing doc while online server only', () => { return withTestDoc(persistence, docRef => { - return docRef.get({source: 'server'}) - .then(doc => { + return docRef.get({ source: 'server' }).then(doc => { expect(doc.exists).to.be.false; expect(doc.metadata.fromCache).to.be.false; expect(doc.metadata.hasPendingWrites).to.be.false; @@ -556,8 +572,7 @@ apiDescribe('GetOptions', persistence => { it('get non existing collection while online server only', () => { return withTestCollection(persistence, {}, colRef => { - return colRef.get({source: 'server'}) - .then(qrySnap => { + return colRef.get({ source: 'server' }).then(qrySnap => { expect(qrySnap.empty).to.be.true; expect(qrySnap.docChanges.length).to.equal(0); expect(qrySnap.metadata.fromCache).to.be.false; @@ -568,28 +583,30 @@ apiDescribe('GetOptions', persistence => { it('get non existing doc while offline server only', () => { return withTestDoc(persistence, docRef => { - return docRef.firestore.disableNetwork() - // attempt to get doc. Currently, this is expected to fail. In the - // future, we might consider adding support for negative cache hits so - // that we know certain documents *don't* exist. - .then(() => docRef.get({source: 'server'})) - .then(doc => { - expect.fail(); - }) - .catch(expected => { - }); + return ( + docRef.firestore + .disableNetwork() + // attempt to get doc. Currently, this is expected to fail. In the + // future, we might consider adding support for negative cache hits so + // that we know certain documents *don't* exist. + .then(() => docRef.get({ source: 'server' })) + .then(doc => { + expect.fail(); + }) + .catch(expected => {}) + ); }); }); it('get non existing collection while offline server only', () => { return withTestCollection(persistence, {}, colRef => { - return colRef.firestore.disableNetwork() - .then(() => colRef.get({source: 'server'})) - .then(qrySnap => { - expect.fail(); - }) - .catch(expected => { - }); + return colRef.firestore + .disableNetwork() + .then(() => colRef.get({ source: 'server' })) + .then(qrySnap => { + expect.fail(); + }) + .catch(expected => {}); }); }); }); From 3cb151d8742c519b7d0103bbf2c754baa0e55765 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 5 Apr 2018 13:51:08 -0400 Subject: [PATCH 3/9] review feedback --- packages/firestore-types/index.d.ts | 33 +- packages/firestore/src/api/database.ts | 182 ++++--- .../firestore/src/core/firestore_client.ts | 70 ++- packages/firestore/src/util/async_queue.ts | 3 +- .../test/integration/api/get_options.test.ts | 448 ++++++++---------- .../test/integration/util/helpers.ts | 32 ++ 6 files changed, 401 insertions(+), 367 deletions(-) diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index 59dc405b51f..d52c28b5a15 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -395,30 +395,32 @@ export interface SetOptions { } /** - * An options object that configures the behavior of `get()` calls in + * An options object that configures the behavior of `get()` calls on * `DocumentReference` and `Query`. By providing a `GetOptions` object, these * methods can be configured to fetch results only from the server, only from - * the local cache or attempt the server and fall back to the cache (which is - * the default). + * the local cache or attempt to fetch results from the server and fall back to + * the cache (which is the default). */ export interface GetOptions { /** * Describes whether we should get from server or cache. * - * Setting to 'default' (or not setting at all), if online, causes Firestore - * to try to give a consistent (server-retrieved) snapshot, or else revert to - * the cache to provide a value. + * Setting to 'default' (or not setting at all), causes Firestore to try to + * retrieve an up-to-date (server-retrieved) snapshot, but fall back to + * returning cached data if the server can't be reached. * - * Setting to 'server' causes Firestore to avoid the cache (generating an - * error if a value cannot be retrieved from the server). The cache will be - * updated if the RPC succeeds. Latency compensation still occurs (implying - * that if the cache is more up to date, then it's values will be merged into - * the results). + * Setting to 'server' causes Firestore to avoid the cache, generating an + * error if the server cannot be reached. Note that the cache will still be + * updated if the server request succeeds. Also note that latency-compensation + * still takes effect, so any pending write operations will be visible in the + * returned data (merged into the server-provided data). * * Setting to 'cache' causes Firestore to immediately return a value from the * cache, ignoring the server completely (implying that the returned value - * may be stale with respect to the value on the server.) For a single - * document, the get will fail if the document doesn't exist. + * may be stale with respect to the value on the server.) If there is no data + * in the cache to satisfy the `get()` call, `DocumentReference.get()` will + * return an error and `QuerySnapshot.get()` will return an empty + * `QuerySnapshot` with no documents. */ readonly source?: 'default' | 'server' | 'cache'; } @@ -907,6 +909,11 @@ export class Query { /** * Executes the query and returns the results as a QuerySnapshot. * + * Note: By default, get() attempts to provide up-to-date data when possible + * by waiting for data from the server, but it may return cached data or fail + * if you are offline and the server cannot be reached. This behavior can be + * altered via the `GetOptions` parameter. + * * @param options An object to configure the get behavior. * @return A Promise that will be resolved with the results of the Query. */ diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index c2fc167615c..04a802be948 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -957,50 +957,90 @@ export class DocumentReference implements firestore.DocumentReference { } get(options?: firestore.GetOptions): Promise { - validateBetweenNumberOfArgs('DocumentReference.get', arguments, 0, 1); + validateOptionNames('DocumentReference.get', options, ['source']); + if (options) { + validateNamedOptionalPropertyEquals( + 'DocumentReference.get', + 'options', + 'source', + options.source, + ['default', 'server', 'cache'] + ); + } return new Promise( (resolve: Resolver, reject: Rejecter) => { if (options && options.source === 'cache') { - this._firestoreClient.getDocumentFromLocalCache(this, { - next: (snap: firestore.DocumentSnapshot) => resolve(snap), - error: reject - }); - return; + this.firestore + .ensureClientConfigured() + .getDocumentFromLocalCache(this._key) + .then((doc: Document) => { + resolve( + new DocumentSnapshot( + this.firestore, + this._key, + doc, + /*fromCache=*/ true + ) + ); + }, reject); + } else { + this.getViaSnapshotListener_(resolve, reject, options); } + } + ); + } - const unlisten = this.onSnapshotInternal( - { - includeQueryMetadataChanges: true, - includeDocumentMetadataChanges: true, - waitForSyncWhenOnline: true - }, - { - next: (snap: firestore.DocumentSnapshot) => { - // Remove query first before passing event to user to avoid - // user actions affecting the now stale query. - unlisten(); - - if (!snap.exists && snap.metadata.fromCache) { - // TODO(dimond): If we're online and the document doesn't - // exist then we resolve with a doc.exists set to false. If - // we're offline however, we reject the Promise in this - // case. Two options: 1) Cache the negative response from - // the server so we can deliver that even when you're - // offline 2) Actually reject the Promise in the online case - // if the document doesn't exist. - reject( - new FirestoreError( - Code.ABORTED, - 'Failed to get document because the client is ' + 'offline.' - ) - ); - } else { - resolve(snap); - } - }, - error: reject + private getViaSnapshotListener_( + resolve: Resolver, + reject: Rejecter, + options?: firestore.GetOptions + ): void { + const unlisten = this.onSnapshotInternal( + { + includeQueryMetadataChanges: true, + includeDocumentMetadataChanges: true, + waitForSyncWhenOnline: true + }, + { + next: (snap: firestore.DocumentSnapshot) => { + // Remove query first before passing event to user to avoid + // user actions affecting the now stale query. + unlisten(); + + if (!snap.exists && snap.metadata.fromCache) { + // TODO(dimond): If we're online and the document doesn't + // exist then we resolve with a doc.exists set to false. If + // we're offline however, we reject the Promise in this + // case. Two options: 1) Cache the negative response from + // the server so we can deliver that even when you're + // offline 2) Actually reject the Promise in the online case + // if the document doesn't exist. + reject( + new FirestoreError( + Code.ABORTED, + 'Failed to get document because the client is ' + 'offline.' + ) + ); + } else if ( + snap.exists && + snap.metadata.fromCache && + options && + options.source === 'server' + ) { + reject( + new FirestoreError( + Code.UNAVAILABLE, + 'Failed to get document from server. (However, this ' + + 'document does exist in the local cache. Run again ' + + 'without setting source to FIRGetSourceServer to ' + + 'retrieve the cached document.)' + ) + ); + } else { + resolve(snap); } - ); + }, + error: reject } ); } @@ -1581,29 +1621,53 @@ export class Query implements firestore.Query { if (options && options.source === 'cache') { this.firestore .ensureClientConfigured() - .getDocumentsFromLocalCache(this, { - next: (snap: firestore.QuerySnapshot) => resolve(snap), - error: reject - }); - return; + .getDocumentsFromLocalCache(this._query) + .then((viewSnap: ViewSnapshot) => { + resolve(new QuerySnapshot(this.firestore, this._query, viewSnap)); + }, reject); + } else { + this.getViaSnapshotListener_(resolve, reject, options); } - const unlisten = this.onSnapshotInternal( - { - includeDocumentMetadataChanges: false, - includeQueryMetadataChanges: true, - waitForSyncWhenOnline: true - }, - { - next: (result: firestore.QuerySnapshot) => { - // Remove query first before passing event to user to avoid - // user actions affecting the now stale query. - unlisten(); - - resolve(result); - }, - error: reject + } + ); + } + + private getViaSnapshotListener_( + resolve: Resolver, + reject: Rejecter, + options?: firestore.GetOptions + ): void { + const unlisten = this.onSnapshotInternal( + { + includeDocumentMetadataChanges: false, + includeQueryMetadataChanges: true, + waitForSyncWhenOnline: true + }, + { + next: (result: firestore.QuerySnapshot) => { + // Remove query first before passing event to user to avoid + // user actions affecting the now stale query. + unlisten(); + + if ( + result.metadata.fromCache && + options && + options.source === 'server' + ) { + reject( + new FirestoreError( + Code.UNAVAILABLE, + 'Failed to get documents from server. (However, these ' + + 'documents may exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached documents.)' + ) + ); + } else { + resolve(result); } - ); + }, + error: reject } ); } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 5e5abfca68e..59623555c65 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -15,12 +15,6 @@ */ import { CredentialsProvider } from '../api/credentials'; -import { - DocumentReference, - DocumentSnapshot, - Query, - QuerySnapshot -} from '../api/database'; import { User } from '../auth/user'; import { EventManager, @@ -37,8 +31,13 @@ import { LocalStore } from '../local/local_store'; import { MemoryPersistence } from '../local/memory_persistence'; import { NoOpGarbageCollector } from '../local/no_op_garbage_collector'; import { Persistence } from '../local/persistence'; -import { DocumentKeySet, documentKeySet } from '../model/collections'; +import { + DocumentKeySet, + documentKeySet, + DocumentMap +} from '../model/collections'; import { Document, MaybeDocument } from '../model/document'; +import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; import { Platform } from '../platform/platform'; import { Datastore } from '../remote/datastore'; @@ -367,55 +366,40 @@ export class FirestoreClient { }); } - getDocumentFromLocalCache( - doc: DocumentReference, - observer: Observer - ): void { - this.asyncQueue.schedule(() => { - return this.localStore.readDocument(doc._key).then(maybeDoc => { + getDocumentFromLocalCache(docKey: DocumentKey): Promise { + return this.asyncQueue + .schedule(() => { + return this.localStore.readDocument(docKey); + }) + .then((maybeDoc: MaybeDocument | null) => { if (maybeDoc instanceof Document) { - observer.next( - new DocumentSnapshot(doc.firestore, doc._key, maybeDoc, true) - ); + return maybeDoc; } else { - observer.error( - new Error( - 'Failed to get document from cache. (However, this document may ' + - "exist on the server. Run again without setting 'source' in " + - 'the GetOptions to attempt to retrieve the document from the ' + - 'server.)' - ) + throw new FirestoreError( + Code.UNAVAILABLE, + 'Failed to get document from cache. (However, this document may ' + + "exist on the server. Run again without setting 'source' in " + + 'the GetOptions to attempt to retrieve the document from the ' + + 'server.)' ); } }); - }); } - getDocumentsFromLocalCache( - query: Query, - observer: Observer - ): void { - this.asyncQueue.schedule(() => { - return this.localStore.executeQuery(query._query).then(docs => { + getDocumentsFromLocalCache(query: InternalQuery): Promise { + return this.asyncQueue + .schedule(() => { + return this.localStore.executeQuery(query); + }) + .then((docs: DocumentMap) => { const remoteKeys: DocumentKeySet = documentKeySet(); - const view = new View(query._query, remoteKeys); + const view = new View(query, remoteKeys); const viewDocChanges: ViewDocumentChanges = view.computeDocChanges( docs ); - const viewChange: ViewChange = view.applyChanges(viewDocChanges); - assert( - viewChange.limboChanges.length === 0, - 'View returned limbo documents during local-only query execution.' - ); - - const snapshot: ViewSnapshot = viewChange.snapshot; - - observer.next( - new QuerySnapshot(query.firestore, query._query, snapshot) - ); + return view.applyChanges(viewDocChanges).snapshot; }); - }); } write(mutations: Mutation[]): Promise { diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 4dce61ed40f..c603b5b84c9 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -118,8 +118,9 @@ export class AsyncQueue { // and return the rejected Promise. throw error; }) - .then(() => { + .then(result => { this.operationInProgress = false; + return result; }); }); return this.tail as AnyDuringMigration; diff --git a/packages/firestore/test/integration/api/get_options.test.ts b/packages/firestore/test/integration/api/get_options.test.ts index 75ebea15adf..7836882548d 100644 --- a/packages/firestore/test/integration/api/get_options.test.ts +++ b/packages/firestore/test/integration/api/get_options.test.ts @@ -15,21 +15,23 @@ */ import { expect } from 'chai'; -import { apiDescribe, withTestDoc, withTestCollection } from '../util/helpers'; +import { + apiDescribe, + toDataMap, + withTestDocAndInitialData, + withTestCollection +} from '../util/helpers'; apiDescribe('GetOptions', persistence => { it('get document while online with default get options', () => { const initialData = { key: 'value' }; - return withTestDoc(persistence, docRef => { - return docRef - .set(initialData) - .then(() => docRef.get()) - .then(doc => { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.false; - expect(doc.metadata.hasPendingWrites).to.be.false; - expect(doc.data()).to.deep.equal(initialData); - }); + return withTestDocAndInitialData(persistence, initialData, docRef => { + return docRef.get().then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.false; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); + }); }); }); @@ -44,32 +46,26 @@ apiDescribe('GetOptions', persistence => { expect(qrySnap.metadata.fromCache).to.be.false; expect(qrySnap.metadata.hasPendingWrites).to.be.false; expect(qrySnap.docChanges.length).to.equal(3); - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data(); - }); - expect(docsData).to.deep.equal(initialDocs); + expect(toDataMap(qrySnap)).to.deep.equal(initialDocs); }); }); }); it('get document while offline with default get options', () => { const initialData = { key: 'value' }; - const newData = { key2: 'value2' }; - return withTestDoc(persistence, docRef => { + return withTestDocAndInitialData(persistence, initialData, docRef => { + // Register a snapshot to force the data to stay in the cache and not be + // garbage collected. + docRef.onSnapshot(() => {}); return docRef - .set(initialData) - .then(() => docRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promise won't complete - docRef.set(newData); - return docRef.get(); - }) + .get() + .then(ignored => docRef.firestore.disableNetwork()) + .then(() => docRef.get()) .then(doc => { expect(doc.exists).to.be.true; expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.true; - expect(doc.data()).to.deep.equal(newData); + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); }); }); }); @@ -81,130 +77,104 @@ apiDescribe('GetOptions', persistence => { doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { - // force local cache of these - return ( - colRef - .get() - // now go offine. Note that if persistence is disabled, this will cause - // the initialDocs to be garbage collected. - .then(ignored => colRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promises won't complete - colRef.doc('doc2').set({ key2b: 'value2b' }, { merge: true }); - colRef.doc('doc3').set({ key3b: 'value3b' }); - colRef.doc('doc4').set({ key4: 'value4' }); - return colRef.get(); - }) - .then(qrySnap => { - expect(qrySnap.metadata.fromCache).to.be.true; - expect(qrySnap.metadata.hasPendingWrites).to.be.true; - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data(); - }); - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(4); - expect(docsData).to.deep.equal({ - doc1: { key1: 'value1' }, - doc2: { key2: 'value2', key2b: 'value2b' }, - doc3: { key3b: 'value3b' }, - doc4: { key4: 'value4' } - }); - } else { - expect(qrySnap.docChanges.length).to.equal(3); - expect(docsData).to.deep.equal({ - doc2: { key2b: 'value2b' }, - doc3: { key3b: 'value3b' }, - doc4: { key4: 'value4' } - }); - } - }) - ); + // Register a snapshot to force the data to stay in the cache and not be + // garbage collected. + colRef.onSnapshot(() => {}); + return colRef + .get() + .then(ignored => colRef.firestore.disableNetwork()) + .then(() => { + // NB: since we're offline, the returned promises won't complete + colRef.doc('doc2').set({ key2b: 'value2b' }, { merge: true }); + colRef.doc('doc3').set({ key3b: 'value3b' }); + colRef.doc('doc4').set({ key4: 'value4' }); + return colRef.get(); + }) + .then(qrySnap => { + expect(qrySnap.metadata.fromCache).to.be.true; + expect(qrySnap.metadata.hasPendingWrites).to.be.true; + const docsData = toDataMap(qrySnap); + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + doc1: { key1: 'value1' }, + doc2: { key2: 'value2', key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } + }); + }); }); }); - it('get document while online cache only', () => { + it('get document while online with source=cache', () => { const initialData = { key: 'value' }; - return withTestDoc(persistence, docRef => { + return withTestDocAndInitialData(persistence, initialData, docRef => { + // Register a snapshot to force the data to stay in the cache and not be + // garbage collected. + docRef.onSnapshot(() => {}); return docRef - .set(initialData) - .then(() => docRef.get({ source: 'cache' })) + .get() + .then(ignored => docRef.get({ source: 'cache' })) .then(doc => { - if (persistence) { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.false; - expect(doc.data()).to.deep.equal(initialData); - } else { - expect.fail(); - } - }) - .catch(err => { - // we expect an error when persistence has been disabled (since the - // document won't be in the local cache.) - expect(persistence).to.be.false; + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.true; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); }); }); }); - it('get collection while online cache only', () => { + it('get collection while online with source=cache', () => { const initialDocs = { doc1: { key1: 'value1' }, doc2: { key2: 'value2' }, doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { - // force local cache of these + // Register a snapshot to force the data to stay in the cache and not be + // garbage collected. + colRef.onSnapshot(() => {}); return colRef .get() .then(ignored => colRef.get({ source: 'cache' })) .then(qrySnap => { expect(qrySnap.metadata.fromCache).to.be.true; expect(qrySnap.metadata.hasPendingWrites).to.be.false; - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(3); - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data(); - }); - expect(docsData).to.deep.equal(initialDocs); - } else { - expect(qrySnap.docChanges.length).to.equal(0); - expect(qrySnap.empty).to.be.true; - } + expect(qrySnap.docChanges.length).to.equal(3); + expect(toDataMap(qrySnap)).to.deep.equal(initialDocs); }); }); }); - it('get document while offline cache only', () => { + it('get document while offline with source=cache', () => { const initialData = { key: 'value' }; - const newData = { key2: 'value2' }; - return withTestDoc(persistence, docRef => { + + return withTestDocAndInitialData(persistence, initialData, docRef => { + // Register a snapshot to force the data to stay in the cache and not be + // garbage collected. + docRef.onSnapshot(() => {}); return docRef - .set(initialData) - .then(() => docRef.firestore.disableNetwork()) - .then(() => { - // NB: since we're offline, the returned promise won't complete - docRef.set(newData); - return docRef.get({ source: 'cache' }); - }) + .get() + .then(ignored => docRef.firestore.disableNetwork()) + .then(() => docRef.get({ source: 'cache' })) .then(doc => { expect(doc.exists).to.be.true; expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.true; - expect(doc.data()).to.deep.equal(newData); + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); }); }); }); - it('get collection while offline cache only', () => { + it('get collection while offline with source=cache', () => { const initialDocs = { doc1: { key1: 'value1' }, doc2: { key2: 'value2' }, doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { - // force local cache of these + // Register a snapshot to force the data to stay in the cache and not be + // garbage collected. + colRef.onSnapshot(() => {}); return colRef .get() .then(ignored => colRef.firestore.disableNetwork()) @@ -218,46 +188,31 @@ apiDescribe('GetOptions', persistence => { .then(qrySnap => { expect(qrySnap.metadata.fromCache).to.be.true; expect(qrySnap.metadata.hasPendingWrites).to.be.true; - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data(); + const docsData = toDataMap(qrySnap); + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + doc1: { key1: 'value1' }, + doc2: { key2: 'value2', key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } }); - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(4); - expect(docsData).to.deep.equal({ - doc1: { key1: 'value1' }, - doc2: { key2: 'value2', key2b: 'value2b' }, - doc3: { key3b: 'value3b' }, - doc4: { key4: 'value4' } - }); - } else { - expect(qrySnap.docChanges.length).to.equal(3); - expect(docsData).to.deep.equal({ - doc2: { key2b: 'value2b' }, - doc3: { key3b: 'value3b' }, - doc4: { key4: 'value4' } - }); - } }); }); }); - it('get document while online server only', () => { + it('get document while online with source=server', () => { const initialData = { key: 'value' }; - return withTestDoc(persistence, docRef => { - return docRef - .set(initialData) - .then(() => docRef.get({ source: 'server' })) - .then(doc => { - expect(doc.exists).to.be.true; - expect(doc.metadata.fromCache).to.be.false; - expect(doc.metadata.hasPendingWrites).to.be.false; - expect(doc.data()).to.deep.equal(initialData); - }); + return withTestDocAndInitialData(persistence, initialData, docRef => { + return docRef.get({ source: 'server' }).then(doc => { + expect(doc.exists).to.be.true; + expect(doc.metadata.fromCache).to.be.false; + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); + }); }); }); - it('get collection while online server only', () => { + it('get collection while online with source=server', () => { const initialDocs = { doc1: { key1: 'value1' }, doc2: { key2: 'value2' }, @@ -268,30 +223,29 @@ apiDescribe('GetOptions', persistence => { expect(qrySnap.metadata.fromCache).to.be.false; expect(qrySnap.metadata.hasPendingWrites).to.be.false; expect(qrySnap.docChanges.length).to.equal(3); - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data(); - }); - expect(docsData).to.deep.equal(initialDocs); + expect(toDataMap(qrySnap)).to.deep.equal(initialDocs); }); }); }); - it('get document while offline server only', () => { + it('get document while offline with source=server', () => { const initialData = { key: 'value' }; - return withTestDoc(persistence, docRef => { + return withTestDocAndInitialData(persistence, initialData, docRef => { return docRef - .set(initialData) + .get({ source: 'server' }) + .then(ignored => {}) .then(() => docRef.firestore.disableNetwork()) .then(() => docRef.get({ source: 'server' })) - .then(doc => { - expect.fail(); - }) - .catch(expected => {}); + .then( + doc => { + expect.fail(); + }, + expected => {} + ); }); }); - it('get collection while offline server only', () => { + it('get collection while offline with source=server', () => { const initialDocs = { doc1: { key1: 'value1' }, doc2: { key2: 'value2' }, @@ -306,25 +260,27 @@ apiDescribe('GetOptions', persistence => { // the initialDocs to be garbage collected. .then(ignored => colRef.firestore.disableNetwork()) .then(() => colRef.get({ source: 'server' })) - .then(qrySnap => { - expect.fail(); - }) - .catch(expected => {}) + .then( + qrySnap => { + expect.fail(); + }, + expected => {} + ) ); }); }); it('get document while offline with different get options', () => { const initialData = { key: 'value' }; - const newData = { key2: 'value2' }; - return withTestDoc(persistence, docRef => { + + return withTestDocAndInitialData(persistence, initialData, docRef => { + // Register a snapshot to force the data to stay in the cache and not be + // garbage collected. + docRef.onSnapshot(() => {}); return docRef - .set(initialData) - .then(() => docRef.firestore.disableNetwork()) + .get() + .then(ignored => docRef.firestore.disableNetwork()) .then(() => { - // NB: since we're offline, the returned promise won't complete - docRef.set(newData); - // Create an initial listener for this query (to attempt to disrupt the // gets below) and wait for the listener to deliver its initial // snapshot before continuing. @@ -343,23 +299,25 @@ apiDescribe('GetOptions', persistence => { .then(doc => { expect(doc.exists).to.be.true; expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.true; - expect(doc.data()).to.deep.equal(newData); + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); return Promise.resolve(); }) .then(() => docRef.get()) .then(doc => { expect(doc.exists).to.be.true; expect(doc.metadata.fromCache).to.be.true; - expect(doc.metadata.hasPendingWrites).to.be.true; - expect(doc.data()).to.deep.equal(newData); + expect(doc.metadata.hasPendingWrites).to.be.false; + expect(doc.data()).to.deep.equal(initialData); return Promise.resolve(); }) .then(() => docRef.get({ source: 'server' })) - .then(doc => { - expect.fail(); - }) - .catch(expected => {}); + .then( + doc => { + expect.fail(); + }, + expected => {} + ); }); }); @@ -370,7 +328,9 @@ apiDescribe('GetOptions', persistence => { doc3: { key3: 'value3' } }; return withTestCollection(persistence, initialDocs, colRef => { - // force local cache of these + // Register a snapshot to force the data to stay in the cache and not be + // garbage collected. + colRef.onSnapshot(() => {}); return ( colRef .get() @@ -401,63 +361,41 @@ apiDescribe('GetOptions', persistence => { .then(qrySnap => { expect(qrySnap.metadata.fromCache).to.be.true; expect(qrySnap.metadata.hasPendingWrites).to.be.true; - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data(); + const docsData = toDataMap(qrySnap); + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + doc1: { key1: 'value1' }, + doc2: { key2: 'value2', key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } }); - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(4); - expect(docsData).to.deep.equal({ - doc1: { key1: 'value1' }, - doc2: { key2: 'value2', key2b: 'value2b' }, - doc3: { key3b: 'value3b' }, - doc4: { key4: 'value4' } - }); - } else { - expect(qrySnap.docChanges.length).to.equal(3); - expect(docsData).to.deep.equal({ - doc2: { key2b: 'value2b' }, - doc3: { key3b: 'value3b' }, - doc4: { key4: 'value4' } - }); - } }) .then(() => colRef.get()) .then(qrySnap => { expect(qrySnap.metadata.fromCache).to.be.true; expect(qrySnap.metadata.hasPendingWrites).to.be.true; - let docsData = {}; - qrySnap.forEach(doc => { - docsData[doc.id] = doc.data(); + const docsData = toDataMap(qrySnap); + expect(qrySnap.docChanges.length).to.equal(4); + expect(docsData).to.deep.equal({ + doc1: { key1: 'value1' }, + doc2: { key2: 'value2', key2b: 'value2b' }, + doc3: { key3b: 'value3b' }, + doc4: { key4: 'value4' } }); - if (persistence) { - expect(qrySnap.docChanges.length).to.equal(4); - expect(docsData).to.deep.equal({ - doc1: { key1: 'value1' }, - doc2: { key2: 'value2', key2b: 'value2b' }, - doc3: { key3b: 'value3b' }, - doc4: { key4: 'value4' } - }); - } else { - expect(qrySnap.docChanges.length).to.equal(3); - expect(docsData).to.deep.equal({ - doc2: { key2b: 'value2b' }, - doc3: { key3b: 'value3b' }, - doc4: { key4: 'value4' } - }); - } }) .then(() => colRef.get({ source: 'server' })) - .then(qrySnap => { - expect.fail(); - }) - .catch(expected => {}) + .then( + qrySnap => { + expect.fail(); + }, + expected => {} + ) ); }); }); it('get non existing doc while online with default get options', () => { - return withTestDoc(persistence, docRef => { + return withTestDocAndInitialData(persistence, null, docRef => { return docRef.get().then(doc => { expect(doc.exists).to.be.false; expect(doc.metadata.fromCache).to.be.false; @@ -479,14 +417,16 @@ apiDescribe('GetOptions', persistence => { }); it('get non existing doc while offline with default get options', () => { - return withTestDoc(persistence, docRef => { + return withTestDocAndInitialData(persistence, null, docRef => { return docRef.firestore .disableNetwork() .then(() => docRef.get()) - .then(doc => { - expect.fail(); - }) - .catch(expected => {}); + .then( + doc => { + expect.fail(); + }, + expected => {} + ); }); }); @@ -504,21 +444,21 @@ apiDescribe('GetOptions', persistence => { }); }); - it('get non existing doc while online cache only', () => { - return withTestDoc(persistence, docRef => { + it('get non existing doc while online with source=cache', () => { + return withTestDocAndInitialData(persistence, null, docRef => { // attempt to get doc. Currently, this is expected to fail. In the // future, we might consider adding support for negative cache hits so // that we know certain documents *don't* exist. - return docRef - .get({ source: 'cache' }) - .then(doc => { + return docRef.get({ source: 'cache' }).then( + doc => { expect.fail(); - }) - .catch(expected => {}); + }, + expected => {} + ); }); }); - it('get non existing collection while online cache only', () => { + it('get non existing collection while online with source=cache', () => { return withTestCollection(persistence, {}, colRef => { return colRef.get({ source: 'cache' }).then(qrySnap => { expect(qrySnap.empty).to.be.true; @@ -529,8 +469,8 @@ apiDescribe('GetOptions', persistence => { }); }); - it('get non existing doc while offline cache only', () => { - return withTestDoc(persistence, docRef => { + it('get non existing doc while offline with source=cache', () => { + return withTestDocAndInitialData(persistence, null, docRef => { return ( docRef.firestore .disableNetwork() @@ -538,15 +478,17 @@ apiDescribe('GetOptions', persistence => { // future, we might consider adding support for negative cache hits so // that we know certain documents *don't* exist. .then(() => docRef.get({ source: 'cache' })) - .then(doc => { - expect.fail(); - }) - .catch(expected => {}) + .then( + doc => { + expect.fail(); + }, + expected => {} + ) ); }); }); - it('get non existing collection while offline cache only', () => { + it('get non existing collection while offline with source=cache', () => { return withTestCollection(persistence, {}, colRef => { return colRef.firestore .disableNetwork() @@ -560,8 +502,8 @@ apiDescribe('GetOptions', persistence => { }); }); - it('get non existing doc while online server only', () => { - return withTestDoc(persistence, docRef => { + it('get non existing doc while online with source=server', () => { + return withTestDocAndInitialData(persistence, null, docRef => { return docRef.get({ source: 'server' }).then(doc => { expect(doc.exists).to.be.false; expect(doc.metadata.fromCache).to.be.false; @@ -570,7 +512,7 @@ apiDescribe('GetOptions', persistence => { }); }); - it('get non existing collection while online server only', () => { + it('get non existing collection while online with source=server', () => { return withTestCollection(persistence, {}, colRef => { return colRef.get({ source: 'server' }).then(qrySnap => { expect(qrySnap.empty).to.be.true; @@ -581,8 +523,8 @@ apiDescribe('GetOptions', persistence => { }); }); - it('get non existing doc while offline server only', () => { - return withTestDoc(persistence, docRef => { + it('get non existing doc while offline with source=server', () => { + return withTestDocAndInitialData(persistence, null, docRef => { return ( docRef.firestore .disableNetwork() @@ -590,23 +532,27 @@ apiDescribe('GetOptions', persistence => { // future, we might consider adding support for negative cache hits so // that we know certain documents *don't* exist. .then(() => docRef.get({ source: 'server' })) - .then(doc => { - expect.fail(); - }) - .catch(expected => {}) + .then( + doc => { + expect.fail(); + }, + expected => {} + ) ); }); }); - it('get non existing collection while offline server only', () => { + it('get non existing collection while offline with source=server', () => { return withTestCollection(persistence, {}, colRef => { return colRef.firestore .disableNetwork() .then(() => colRef.get({ source: 'server' })) - .then(qrySnap => { - expect.fail(); - }) - .catch(expected => {}); + .then( + qrySnap => { + expect.fail(); + }, + expected => {} + ); }); }); }); diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 3ab1b914214..689e1eb9934 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -86,6 +86,16 @@ export function toDataArray( return docSet.docs.map(d => d.data()); } +export function toDataMap( + docSet: firestore.QuerySnapshot +): { [field: string]: firestore.DocumentData } { + let docsData = {}; + docSet.forEach(doc => { + docsData[doc.id] = doc.data(); + }); + return docsData; +} + export function withTestDb( persistence: boolean, fn: (db: firestore.FirebaseFirestore) => Promise @@ -191,6 +201,28 @@ export function withTestDoc( }); } +// TODO(rsgowman): Modify withTestDoc to take in (an optional) initialData and +// fix existing usages of it. Then delete this function. This makes withTestDoc +// more analogous to withTestCollection and eliminates the pattern of +// `withTestDoc(..., docRef => { docRef.set(initialData) ...});` that otherwise is +// quite common. +export function withTestDocAndInitialData( + persistence: boolean, + initialData: firestore.DocumentData | null, + fn: (doc: firestore.DocumentReference) => Promise +): Promise { + return withTestDb(persistence, db => { + const docRef: firestore.DocumentReference = db + .collection('test-collection') + .doc(); + if (initialData) { + return docRef.set(initialData).then(() => fn(docRef)); + } else { + return fn(docRef); + } + }); +} + export function withTestCollection( persistence: boolean, docs: { [key: string]: firestore.DocumentData }, From 2c2b4d8e9cf042f7d446f54657abb581df19a924 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Mon, 9 Apr 2018 18:08:10 -0400 Subject: [PATCH 4/9] Fixes resulting from merging master. --- packages/firestore/src/core/firestore_client.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index dc8e496c86b..e5c8bbe4174 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -23,7 +23,7 @@ import { QueryListener } from './event_manager'; import { SyncEngine } from './sync_engine'; -import { View, ViewChange, ViewDocumentChanges } from './view'; +import { View, ViewDocumentChanges } from './view'; import { EagerGarbageCollector } from '../local/eager_garbage_collector'; import { GarbageCollector } from '../local/garbage_collector'; import { IndexedDbPersistence } from '../local/indexeddb_persistence'; @@ -43,7 +43,6 @@ import { Platform } from '../platform/platform'; import { Datastore } from '../remote/datastore'; import { RemoteStore } from '../remote/remote_store'; import { JsonProtoSerializer } from '../remote/serializer'; -import { assert } from '../util/assert'; import { AsyncQueue } from '../util/async_queue'; import { Code, FirestoreError } from '../util/error'; import { debug } from '../util/log'; @@ -368,7 +367,7 @@ export class FirestoreClient { getDocumentFromLocalCache(docKey: DocumentKey): Promise { return this.asyncQueue - .schedule(() => { + .enqueue(() => { return this.localStore.readDocument(docKey); }) .then((maybeDoc: MaybeDocument | null) => { @@ -388,7 +387,7 @@ export class FirestoreClient { getDocumentsFromLocalCache(query: InternalQuery): Promise { return this.asyncQueue - .schedule(() => { + .enqueue(() => { return this.localStore.executeQuery(query); }) .then((docs: DocumentMap) => { From a5f73fbbceff84838273900e40bb007750f25bbd Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 11 Apr 2018 17:46:30 -0400 Subject: [PATCH 5/9] review feedback --- packages/firebase/index.d.ts | 49 +++++++++++++++++-- packages/firestore/src/api/database.ts | 12 ++--- .../firestore/src/core/firestore_client.ts | 6 +-- .../test/integration/util/helpers.ts | 2 +- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 7f04594d4d2..91fbf87149a 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -1112,6 +1112,37 @@ declare namespace firebase.firestore { readonly merge?: boolean; } + /** + * An options object that configures the behavior of `get()` calls on + * `DocumentReference` and `Query`. By providing a `GetOptions` object, these + * methods can be configured to fetch results only from the server, only from + * the local cache or attempt to fetch results from the server and fall back to + * the cache (which is the default). + */ + export interface GetOptions { + /** + * Describes whether we should get from server or cache. + * + * Setting to 'default' (or not setting at all), causes Firestore to try to + * retrieve an up-to-date (server-retrieved) snapshot, but fall back to + * returning cached data if the server can't be reached. + * + * Setting to 'server' causes Firestore to avoid the cache, generating an + * error if the server cannot be reached. Note that the cache will still be + * updated if the server request succeeds. Also note that latency-compensation + * still takes effect, so any pending write operations will be visible in the + * returned data (merged into the server-provided data). + * + * Setting to 'cache' causes Firestore to immediately return a value from the + * cache, ignoring the server completely (implying that the returned value + * may be stale with respect to the value on the server.) If there is no data + * in the cache to satisfy the `get()` call, `DocumentReference.get()` will + * return an error and `QuerySnapshot.get()` will return an empty + * `QuerySnapshot` with no documents. + */ + readonly source?: 'default' | 'server' | 'cache'; + } + /** * A `DocumentReference` refers to a document location in a Firestore database * and can be used to write, read, or listen to the location. The document at @@ -1213,14 +1244,16 @@ declare namespace firebase.firestore { /** * Reads the document referred to by this `DocumentReference`. * - * Note: get() attempts to provide up-to-date data when possible by waiting - * for data from the server, but it may return cached data or fail if you - * are offline and the server cannot be reached. + * Note: By default, get() attempts to provide up-to-date data when possible + * by waiting for data from the server, but it may return cached data or fail + * if you are offline and the server cannot be reached. This behavior can be + * altered via the `GetOptions` parameter. * + * @param options An object to configure the get behavior. * @return A Promise resolved with a DocumentSnapshot containing the * current document contents. */ - get(): Promise; + get(options?: GetOptions): Promise; /** * Attaches a listener for DocumentSnapshot events. You may either pass @@ -1598,9 +1631,15 @@ declare namespace firebase.firestore { /** * Executes the query and returns the results as a QuerySnapshot. * + * Note: By default, get() attempts to provide up-to-date data when possible + * by waiting for data from the server, but it may return cached data or fail + * if you are offline and the server cannot be reached. This behavior can be + * altered via the `GetOptions` parameter. + * + * @param options An object to configure the get behavior. * @return A Promise that will be resolved with the results of the Query. */ - get(): Promise; + get(options?: GetOptions): Promise; /** * Attaches a listener for QuerySnapshot events. You may either pass diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index ab307fac810..2add2d58a5f 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -1028,13 +1028,13 @@ export class DocumentReference implements firestore.DocumentReference { ); }, reject); } else { - this.getViaSnapshotListener_(resolve, reject, options); + this.getViaSnapshotListener(resolve, reject, options); } } ); } - private getViaSnapshotListener_( + private getViaSnapshotListener( resolve: Resolver, reject: Rejecter, options?: firestore.GetOptions @@ -1061,7 +1061,7 @@ export class DocumentReference implements firestore.DocumentReference { // if the document doesn't exist. reject( new FirestoreError( - Code.ABORTED, + Code.UNAVAILABLE, 'Failed to get document because the client is ' + 'offline.' ) ); @@ -1076,7 +1076,7 @@ export class DocumentReference implements firestore.DocumentReference { Code.UNAVAILABLE, 'Failed to get document from server. (However, this ' + 'document does exist in the local cache. Run again ' + - 'without setting source to FIRGetSourceServer to ' + + 'without setting source to "server" to ' + 'retrieve the cached document.)' ) ); @@ -1679,13 +1679,13 @@ export class Query implements firestore.Query { resolve(new QuerySnapshot(this.firestore, this._query, viewSnap)); }, reject); } else { - this.getViaSnapshotListener_(resolve, reject, options); + this.getViaSnapshotListener(resolve, reject, options); } } ); } - private getViaSnapshotListener_( + private getViaSnapshotListener( resolve: Resolver, reject: Rejecter, options?: firestore.GetOptions diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index e5c8bbe4174..640b200b904 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -49,7 +49,7 @@ import { debug } from '../util/log'; import { Deferred } from '../util/promise'; import { DatabaseId, DatabaseInfo } from './database_info'; -import { Query as InternalQuery } from './query'; +import { Query } from './query'; import { Transaction } from './transaction'; import { OnlineState } from './types'; import { ViewSnapshot } from './view_snapshot'; @@ -348,7 +348,7 @@ export class FirestoreClient { } listen( - query: InternalQuery, + query: Query, observer: Observer, options: ListenOptions ): QueryListener { @@ -385,7 +385,7 @@ export class FirestoreClient { }); } - getDocumentsFromLocalCache(query: InternalQuery): Promise { + getDocumentsFromLocalCache(query: Query): Promise { return this.asyncQueue .enqueue(() => { return this.localStore.executeQuery(query); diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 4d055e7a814..604131c5b38 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -90,7 +90,7 @@ export function toDataArray( export function toDataMap( docSet: firestore.QuerySnapshot ): { [field: string]: firestore.DocumentData } { - let docsData = {}; + const docsData = {}; docSet.forEach(doc => { docsData[doc.id] = doc.data(); }); From e32c840b79eb097a928725d54d482e68c947c4cc Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 12 Apr 2018 16:46:05 -0400 Subject: [PATCH 6/9] Update changelog --- packages/firestore/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/firestore/CHANGELOG.md b/packages/firestore/CHANGELOG.md index a7d8d618d34..3fe2ae01f8f 100644 --- a/packages/firestore/CHANGELOG.md +++ b/packages/firestore/CHANGELOG.md @@ -10,6 +10,10 @@ `FirestoreSettings` to `true`. Note that the current behavior (`DocumentSnapshot`s returning JS Date objects) will be removed in a future release. `Timestamp` supports higher precision than JS Date. +- [feature] Added ability to control whether DocumentReference.get() and + Query.get() should fetch from server only, cache only, or attempt server and + fall back to the cache (which was the only option previously, and is now the + default.) # 0.3.6 - [fixed] Fixed a regression in the Firebase JS release 4.11.0 that could From 5a4576f926f23c6d68209c075fb2b2b53e69ce2f Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 12 Apr 2018 16:56:43 -0400 Subject: [PATCH 7/9] review feedback --- packages/firestore/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/firestore/CHANGELOG.md b/packages/firestore/CHANGELOG.md index 3fe2ae01f8f..36e0b6722f3 100644 --- a/packages/firestore/CHANGELOG.md +++ b/packages/firestore/CHANGELOG.md @@ -11,9 +11,9 @@ (`DocumentSnapshot`s returning JS Date objects) will be removed in a future release. `Timestamp` supports higher precision than JS Date. - [feature] Added ability to control whether DocumentReference.get() and - Query.get() should fetch from server only, cache only, or attempt server and - fall back to the cache (which was the only option previously, and is now the - default.) + Query.get() should fetch from server only, (by passing { source: 'server' }, + cache only (by passing { source: 'cache' }), or attempt server and fall back + to the cache (which was the only option previously, and is now the default.) # 0.3.6 - [fixed] Fixed a regression in the Firebase JS release 4.11.0 that could From a1eb31c9030100da7c35cad0038285fe94f76870 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 12 Apr 2018 16:58:12 -0400 Subject: [PATCH 8/9] period after bracket --- packages/firestore/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/CHANGELOG.md b/packages/firestore/CHANGELOG.md index 36e0b6722f3..eb61dad9b31 100644 --- a/packages/firestore/CHANGELOG.md +++ b/packages/firestore/CHANGELOG.md @@ -13,7 +13,7 @@ - [feature] Added ability to control whether DocumentReference.get() and Query.get() should fetch from server only, (by passing { source: 'server' }, cache only (by passing { source: 'cache' }), or attempt server and fall back - to the cache (which was the only option previously, and is now the default.) + to the cache (which was the only option previously, and is now the default). # 0.3.6 - [fixed] Fixed a regression in the Firebase JS release 4.11.0 that could From e807f0044700c20be9fed20ed8bd1fa5853b563f Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 12 Apr 2018 17:19:05 -0400 Subject: [PATCH 9/9] close all the parens! --- packages/firestore/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/CHANGELOG.md b/packages/firestore/CHANGELOG.md index eb61dad9b31..22176674d24 100644 --- a/packages/firestore/CHANGELOG.md +++ b/packages/firestore/CHANGELOG.md @@ -11,7 +11,7 @@ (`DocumentSnapshot`s returning JS Date objects) will be removed in a future release. `Timestamp` supports higher precision than JS Date. - [feature] Added ability to control whether DocumentReference.get() and - Query.get() should fetch from server only, (by passing { source: 'server' }, + Query.get() should fetch from server only, (by passing { source: 'server' }), cache only (by passing { source: 'cache' }), or attempt server and fall back to the cache (which was the only option previously, and is now the default).