Skip to content

Commit 3dd9c6a

Browse files
committed
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 520ea85 commit 3dd9c6a

File tree

2 files changed

+62
-2
lines changed

2 files changed

+62
-2
lines changed

Firestore/Example/Tests/Integration/API/FIRValidationTests.mm

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,55 @@ - (void)testQueryCannotBeCreatedFromDocumentsMissingSortValues {
420420
FSTAssertThrows([query queryEndingAtDocument:snapshot], reason);
421421
}
422422

423+
- (void)testQueriesCannotBeSortedByAnUncommittedServerTimestamp {
424+
__weak FIRCollectionReference *collection = [self collectionRef];
425+
FIRFirestore *db = [self firestore];
426+
427+
[db disableNetworkWithCompletion:[self completionForExpectationWithName:@"Disable network"]];
428+
[self awaitExpectations];
429+
430+
XCTestExpectation *offlineCallbackDone =
431+
[self expectationWithDescription:@"offline callback done"];
432+
XCTestExpectation *onlineCallbackDone = [self expectationWithDescription:@"online callback done"];
433+
434+
[collection addSnapshotListener:^(FIRQuerySnapshot *snapshot, NSError *error) {
435+
XCTAssertNil(error);
436+
437+
// Skip the initial empty snapshot.
438+
if (snapshot.empty) return;
439+
440+
XCTAssertEqual(snapshot.count, 1);
441+
FIRQueryDocumentSnapshot *docSnap = [snapshot documents][0];
442+
443+
if ([snapshot metadata].pendingWrites) {
444+
// Offline snapshot. Since the server timestamp is uncommitted, we
445+
// shouldn't be able to query by it.
446+
NSString *reason =
447+
@"Invalid query. Your are trying to start or end a query using a document for which the "
448+
@"field 'timestamp' is an uncommitted server timestamp. (Since the value of this field "
449+
@"is unknown, you cannot start/end a query with it.)";
450+
FSTAssertThrows([[[collection queryOrderedByField:@"timestamp"] queryEndingAtDocument:docSnap]
451+
addSnapshotListener:^(FIRQuerySnapshot *, NSError *){
452+
}],
453+
reason);
454+
[offlineCallbackDone fulfill];
455+
} else {
456+
// Online snapshot. Since the server timestamp is committed, we should be able to query by it.
457+
[[[collection queryOrderedByField:@"timestamp"] queryEndingAtDocument:docSnap]
458+
addSnapshotListener:^(FIRQuerySnapshot *, NSError *){
459+
}];
460+
[onlineCallbackDone fulfill];
461+
}
462+
}];
463+
464+
FIRDocumentReference *document = [collection documentWithAutoID];
465+
[document setData:@{@"timestamp" : [FIRFieldValue fieldValueForServerTimestamp]}];
466+
[self awaitExpectations];
467+
468+
[db enableNetworkWithCompletion:[self completionForExpectationWithName:@"Enable network"]];
469+
[self awaitExpectations];
470+
}
471+
423472
- (void)testQueryBoundMustNotHaveMoreComponentsThanSortOrders {
424473
FIRCollectionReference *testCollection = [self collectionRef];
425474
FIRQuery *query = [testCollection queryOrderedByField:@"foo"];

Firestore/Source/API/FIRQuery.mm

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#import "Firestore/Source/API/FIRDocumentReference+Internal.h"
2323
#import "Firestore/Source/API/FIRDocumentSnapshot+Internal.h"
2424
#import "Firestore/Source/API/FIRFieldPath+Internal.h"
25+
#import "Firestore/Source/API/FIRFieldValue+Internal.h"
2526
#import "Firestore/Source/API/FIRFirestore+Internal.h"
2627
#import "Firestore/Source/API/FIRListenerRegistration+Internal.h"
2728
#import "Firestore/Source/API/FIRQuery+Internal.h"
@@ -550,7 +551,9 @@ - (void)validateOrderByField:(const FieldPath &)orderByField
550551
* Note that the FSTBound will always include the key of the document and the position will be
551552
* unambiguous.
552553
*
553-
* Will throw if the document does not contain all fields of the order by of the query.
554+
* Will throw if the document does not contain all fields of the order by of
555+
* the query or if any of the fields in the order by are an uncommitted server
556+
* timestamp.
554557
*/
555558
- (FSTBound *)boundFromSnapshot:(FIRDocumentSnapshot *)snapshot isBefore:(BOOL)isBefore {
556559
if (![snapshot exists]) {
@@ -572,7 +575,15 @@ - (FSTBound *)boundFromSnapshot:(FIRDocumentSnapshot *)snapshot isBefore:(BOOL)i
572575
databaseID:self.firestore.databaseID]];
573576
} else {
574577
FSTFieldValue *value = [document fieldForPath:sortOrder.field];
575-
if (value != nil) {
578+
579+
if ([value isKindOfClass:[FSTServerTimestampValue class]]) {
580+
FSTThrowInvalidUsage(@"InvalidQueryException",
581+
@"Invalid query. Your are trying to start or end a query using a "
582+
"document for which the field '%s' is an uncommitted server "
583+
"timestamp. (Since the value of this field is unknown, you cannot "
584+
"start/end a query with it.)",
585+
sortOrder.field.CanonicalString().c_str());
586+
} else if (value != nil) {
576587
[components addObject:value];
577588
} else {
578589
FSTThrowInvalidUsage(@"InvalidQueryException",

0 commit comments

Comments
 (0)