Skip to content

Commit 0b7b93b

Browse files
authored
Forbid queries endAt an uncommitted server timestamp. (#1529)
* Forbid queries endAt an uncommitted server timestamp. Without this, it still fails, but: a) not until serializing the query, and b) the error is an internal error, and c) the error message is quite cryptic and has nothing to do with the problem. Port of firebase/firebase-android-sdk#138
1 parent 10bf7c1 commit 0b7b93b

File tree

2 files changed

+69
-3
lines changed

2 files changed

+69
-3
lines changed

packages/firestore/src/api/database.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ import {
4646
FieldValue,
4747
FieldValueOptions,
4848
ObjectValue,
49-
RefValue
49+
RefValue,
50+
ServerTimestampValue
5051
} from '../model/field_value';
5152
import { DeleteMutation, Mutation, Precondition } from '../model/mutation';
5253
import { FieldPath, ResourcePath } from '../model/path';
@@ -1557,7 +1558,8 @@ export class Query implements firestore.Query {
15571558
* position.
15581559
*
15591560
* Will throw if the document does not contain all fields of the order by
1560-
* of the query.
1561+
* of the query or if any of the fields in the order by are an uncommitted
1562+
* server timestamp.
15611563
*/
15621564
private boundFromDocument(
15631565
methodName: string,
@@ -1578,7 +1580,16 @@ export class Query implements firestore.Query {
15781580
components.push(new RefValue(this.firestore._databaseId, doc.key));
15791581
} else {
15801582
const value = doc.field(orderBy.field);
1581-
if (value !== undefined) {
1583+
if (value instanceof ServerTimestampValue) {
1584+
throw new FirestoreError(
1585+
Code.INVALID_ARGUMENT,
1586+
'Invalid query. You are trying to start or end a query using a ' +
1587+
'document for which the field "' +
1588+
orderBy.field +
1589+
'" is an uncommitted server timestamp. (Since the value of ' +
1590+
'this field is unknown, you cannot start/end a query with it.)'
1591+
);
1592+
} else if (value !== undefined) {
15821593
components.push(value);
15831594
} else {
15841595
const field = orderBy.field.canonicalString();

packages/firestore/test/integration/api/validation.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import * as firestore from '@firebase/firestore-types';
1919
import { expect } from 'chai';
2020

2121
import { CACHE_SIZE_UNLIMITED } from '../../../src/api/database';
22+
import { Deferred } from '../../util/promise';
2223
import firebase from '../util/firebase_export';
2324
import {
2425
ALT_PROJECT_ID,
@@ -795,6 +796,60 @@ apiDescribe('Validation:', persistence => {
795796
});
796797
});
797798

799+
validationIt(
800+
persistence,
801+
'cannot be sorted by an uncommitted server timestamp',
802+
db => {
803+
return withTestCollection(
804+
persistence,
805+
/*docs=*/ {},
806+
async (collection: firestore.CollectionReference) => {
807+
await db.disableNetwork();
808+
809+
const offlineDeferred = new Deferred<Void>();
810+
const onlineDeferred = new Deferred<Void>();
811+
812+
const unsubscribe = collection.onSnapshot(snapshot => {
813+
// Skip the initial empty snapshot.
814+
if (snapshot.empty) return;
815+
816+
expect(snapshot.docs).to.have.lengthOf(1);
817+
const docSnap: firestore.DocumentSnapshot = snapshot.docs[0];
818+
819+
if (snapshot.metadata.hasPendingWrites) {
820+
// Offline snapshot. Since the server timestamp is uncommitted,
821+
// we shouldn't be able to query by it.
822+
expect(() =>
823+
collection
824+
.orderBy('timestamp')
825+
.endAt(docSnap)
826+
.onSnapshot(() => {})
827+
).to.throw('uncommitted server timestamp');
828+
offlineDeferred.resolve();
829+
} else {
830+
// Online snapshot. Since the server timestamp is committed, we
831+
// should be able to query by it.
832+
collection
833+
.orderBy('timestamp')
834+
.endAt(docSnap)
835+
.onSnapshot(() => {});
836+
onlineDeferred.resolve();
837+
}
838+
});
839+
840+
const doc: firestore.DocumentReference = collection.doc();
841+
doc.set({ timestamp: FieldValue.serverTimestamp() });
842+
await offlineDeferred.promise;
843+
844+
await db.enableNetwork();
845+
await onlineDeferred.promise;
846+
847+
unsubscribe();
848+
}
849+
);
850+
}
851+
);
852+
798853
validationIt(
799854
persistence,
800855
'must not have more components than order by.',

0 commit comments

Comments
 (0)