Skip to content

Refactor some FirestoreClient methods #3507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 30, 2020
Merged

Refactor some FirestoreClient methods #3507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bb0930
Refactor some FirestoreClient methods
schmidt-sebastian Jul 29, 2020
b6b38db
Fix bug
schmidt-sebastian Jul 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions packages/firestore/exp/src/api/reference.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ import { cast } from '../../../lite/src/api/util';
import { DocumentSnapshot, QuerySnapshot } from './snapshot';
import {
applyFirestoreDataConverter,
getDocsViaSnapshotListener,
getDocViaSnapshotListener,
SnapshotMetadata,
validateHasExplicitOrderByForLimitToLast
} from '../../../src/api/database';
Expand Down Expand Up @@ -65,8 +63,7 @@ export function getDoc<T>(
const ref = cast<DocumentReference<T>>(reference, DocumentReference);
const firestore = cast<Firestore>(ref.firestore, Firestore);
return getFirestoreClient(firestore).then(async firestoreClient => {
const viewSnapshot = await getDocViaSnapshotListener(
firestoreClient,
const viewSnapshot = await firestoreClient.getDocumentViaSnapshotListener(
ref._key
);
return convertToDocSnapshot(firestore, ref, viewSnapshot);
Expand Down Expand Up @@ -101,8 +98,7 @@ export function getDocFromServer<T>(
const ref = cast<DocumentReference<T>>(reference, DocumentReference);
const firestore = cast<Firestore>(ref.firestore, Firestore);
return getFirestoreClient(firestore).then(async firestoreClient => {
const viewSnapshot = await getDocViaSnapshotListener(
firestoreClient,
const viewSnapshot = await firestoreClient.getDocumentViaSnapshotListener(
ref._key,
{ source: 'server' }
);
Expand All @@ -118,8 +114,7 @@ export function getDocs<T>(

validateHasExplicitOrderByForLimitToLast(internalQuery._query);
return getFirestoreClient(firestore).then(async firestoreClient => {
const snapshot = await getDocsViaSnapshotListener(
firestoreClient,
const snapshot = await firestoreClient.getDocumentsViaSnapshotListener(
internalQuery._query
);
return new QuerySnapshot(firestore, internalQuery, snapshot);
Expand All @@ -145,8 +140,7 @@ export function getDocsFromServer<T>(
const internalQuery = cast<Query<T>>(query, Query);
const firestore = cast<Firestore>(query.firestore, Firestore);
return getFirestoreClient(firestore).then(async firestoreClient => {
const snapshot = await getDocsViaSnapshotListener(
firestoreClient,
const snapshot = await firestoreClient.getDocumentsViaSnapshotListener(
internalQuery._query,
{ source: 'server' }
);
Expand Down
119 changes: 6 additions & 113 deletions packages/firestore/src/api/database.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ import { getLogLevel, logError, LogLevel, setLogLevel } from '../util/log';
import { AutoId } from '../util/misc';
import { Deferred } from '../util/promise';
import { FieldPath as ExternalFieldPath } from './field_path';

import {
CredentialsProvider,
CredentialsSettings,
Expand Down Expand Up @@ -1234,9 +1233,9 @@ export class DocumentReference<T = firestore.DocumentData>
validateBetweenNumberOfArgs('DocumentReference.get', arguments, 0, 1);
validateGetOptions('DocumentReference.get', options);

const firestoreClient = this.firestore.ensureClientConfigured();
if (options && options.source === 'cache') {
return this.firestore
.ensureClientConfigured()
return firestoreClient
.getDocumentFromLocalCache(this._key)
.then(
doc =>
Expand All @@ -1250,11 +1249,9 @@ export class DocumentReference<T = firestore.DocumentData>
)
);
} else {
return getDocViaSnapshotListener(
this._firestoreClient,
this._key,
options
).then(snapshot => this._convertToDocSnapshot(snapshot));
return firestoreClient
.getDocumentViaSnapshotListener(this._key, options)
.then(snapshot => this._convertToDocSnapshot(snapshot));
}
}

Expand Down Expand Up @@ -1286,68 +1283,6 @@ export class DocumentReference<T = firestore.DocumentData>
}
}

/**
* Retrieves a latency-compensated document from the backend via a
* SnapshotListener.
*/
export function getDocViaSnapshotListener(
firestoreClient: FirestoreClient,
key: DocumentKey,
options?: firestore.GetOptions
): Promise<ViewSnapshot> {
const result = new Deferred<ViewSnapshot>();
const unlisten = firestoreClient.listen(
newQueryForPath(key.path),
{
includeMetadataChanges: true,
waitForSyncWhenOnline: true
},
{
next: (snap: ViewSnapshot) => {
// Remove query first before passing event to user to avoid
// user actions affecting the now stale query.
unlisten();

const exists = snap.docs.has(key);
if (!exists && snap.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.
result.reject(
new FirestoreError(
Code.UNAVAILABLE,
'Failed to get document because the client is ' + 'offline.'
)
);
} else if (
exists &&
snap.fromCache &&
options &&
options.source === 'server'
) {
result.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 "server" to ' +
'retrieve the cached document.)'
)
);
} else {
result.resolve(snap);
}
},
error: e => result.reject(e)
}
);
return result.promise;
}

export class SnapshotMetadata implements firestore.SnapshotMetadata {
constructor(
readonly hasPendingWrites: boolean,
Expand Down Expand Up @@ -2170,56 +2105,14 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
const firestoreClient = this.firestore.ensureClientConfigured();
return (options && options.source === 'cache'
? firestoreClient.getDocumentsFromLocalCache(this._query)
: getDocsViaSnapshotListener(firestoreClient, this._query, options)
: firestoreClient.getDocumentsViaSnapshotListener(this._query, options)
).then(
snap =>
new QuerySnapshot(this.firestore, this._query, snap, this._converter)
);
}
}

/**
* Retrieves a latency-compensated query snapshot from the backend via a
* SnapshotListener.
*/
export function getDocsViaSnapshotListener(
firestore: FirestoreClient,
query: InternalQuery,
options?: firestore.GetOptions
): Promise<ViewSnapshot> {
const result = new Deferred<ViewSnapshot>();
const unlisten = firestore.listen(
query,
{
includeMetadataChanges: true,
waitForSyncWhenOnline: true
},
{
next: snapshot => {
// Remove query first before passing event to user to avoid
// user actions affecting the now stale query.
unlisten();

if (snapshot.fromCache && options && options.source === 'server') {
result.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 {
result.resolve(snapshot);
}
},
error: e => result.reject(e)
}
);
return result.promise;
}

export class QuerySnapshot<T = firestore.DocumentData>
implements firestore.QuerySnapshot<T> {
private _cachedChanges: Array<firestore.DocumentChange<T>> | null = null;
Expand Down
Loading