Skip to content

GetOptions for controlling offline behaviour #463

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.

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 12 commits into from
Apr 12, 2018
49 changes: 44 additions & 5 deletions packages/firebase/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<DocumentSnapshot>;
get(options?: GetOptions): Promise<DocumentSnapshot>;

/**
* Attaches a listener for DocumentSnapshot events. You may either pass
Expand Down Expand Up @@ -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<QuerySnapshot>;
get(options?: GetOptions): Promise<QuerySnapshot>;

/**
* Attaches a listener for QuerySnapshot events. You may either pass
Expand Down
49 changes: 44 additions & 5 deletions packages/firestore-types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,37 @@ export interface SetOptions {
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
Expand Down Expand Up @@ -595,14 +626,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<DocumentSnapshot>;
get(options?: GetOptions): Promise<DocumentSnapshot>;

/**
* Attaches a listener for DocumentSnapshot events. You may either pass
Expand Down Expand Up @@ -976,9 +1009,15 @@ export class Query {
/**
* Executes the query and returns the results as a QuerySnapshot.
*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the same "Note: By default, ..." comment here? (it's a good comment! :-))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* 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<QuerySnapshot>;
get(options?: GetOptions): Promise<QuerySnapshot>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great news! As it turns out our type definitions are duplicated in 3 places so you will also want to update them in

declare namespace firebase.firestore {
and
declare namespace firebase.firestore {

cc/ @jshcrowthe :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Actually one of those may have just gone away (yay): c198f9a

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's horrifying. :(

(Done.)


/**
* Attaches a listener for QuerySnapshot events. You may either pass
Expand Down
4 changes: 4 additions & 0 deletions packages/firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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, (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
Expand Down
185 changes: 133 additions & 52 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1000,43 +1000,91 @@ export class DocumentReference implements firestore.DocumentReference {
};
}

get(): Promise<firestore.DocumentSnapshot> {
validateExactNumberOfArgs('DocumentReference.get', arguments, 0);
get(options?: firestore.GetOptions): Promise<firestore.DocumentSnapshot> {
validateOptionNames('DocumentReference.get', options, ['source']);
if (options) {
validateNamedOptionalPropertyEquals(
'DocumentReference.get',
'options',
'source',
options.source,
['default', 'server', 'cache']
);
}
return new Promise(
(resolve: Resolver<firestore.DocumentSnapshot>, reject: Rejecter) => {
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
if (options && options.source === 'cache') {
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);
}
}
);
}

private getViaSnapshotListener(
resolve: Resolver<firestore.DocumentSnapshot>,
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.UNAVAILABLE,
'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 "server" to ' +
'retrieve the cached document.)'
)
);
} else {
resolve(snap);
}
);
},
error: reject
}
);
}
Expand Down Expand Up @@ -1619,27 +1667,60 @@ export class Query implements firestore.Query {
};
}

get(): Promise<firestore.QuerySnapshot> {
validateExactNumberOfArgs('Query.get', arguments, 0);
get(options?: firestore.GetOptions): Promise<firestore.QuerySnapshot> {
validateBetweenNumberOfArgs('Query.get', arguments, 0, 1);
return new Promise(
(resolve: Resolver<firestore.QuerySnapshot>, reject: Rejecter) => {
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
if (options && options.source === 'cache') {
this.firestore
.ensureClientConfigured()
.getDocumentsFromLocalCache(this._query)
.then((viewSnap: ViewSnapshot) => {
resolve(new QuerySnapshot(this.firestore, this._query, viewSnap));
}, reject);
} else {
this.getViaSnapshotListener(resolve, reject, options);
}
}
);
}

private getViaSnapshotListener(
resolve: Resolver<firestore.QuerySnapshot>,
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
}
);
}
Expand Down
Loading