Skip to content

Add GetOptions for controlling offline get behaviour #655

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 12 commits into from
Jan 19, 2018
Merged

Add GetOptions for controlling offline get behaviour #655

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
80a40bd
GetOptions for controlling offline behaviour.
rsgowman Jan 10, 2018
6d29de2
GetOptions for controlling offline behaviour (pt2)
rsgowman Jan 11, 2018
30a513f
Address (easy) review feedback.
rsgowman Jan 16, 2018
c2395cc
Remove mistaken changes to unittests.
rsgowman Jan 12, 2018
76d0ae8
Prevent clang-format from breaking file
rsgowman Jan 16, 2018
2a32091
Run style.sh
rsgowman Jan 16, 2018
ec55882
Ensure swift bindings are correct
rsgowman Jan 16, 2018
61876ad
Alter getDocumentsFromLocalCache to be more like FSTSyncEngine.listen…
rsgowman Jan 17, 2018
f5024fb
Fix error messages.
rsgowman Jan 18, 2018
3abd449
Fix doc comments.
rsgowman Jan 18, 2018
d7b9e60
FIRSource -> FIRGetSource
rsgowman Jan 18, 2018
893e56a
Review feedback
rsgowman Jan 18, 2018
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
12 changes: 6 additions & 6 deletions Firestore/Example/SwiftBuildTest/main.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,11 @@ func readDocument(at docRef: DocumentReference) {
func readDocumentWithOptions(at docRef: DocumentReference) {
docRef.getDocument(options:GetOptions.defaultOptions()) { document, error in
}
docRef.getDocument(options:GetOptions.init(source:Source.default)) { document, error in
docRef.getDocument(options:GetOptions.init(source:GetSource.default)) { document, error in
Copy link
Contributor

Choose a reason for hiding this comment

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

Try making one of these use the unqualified form:

docRef.getDocument(options:GetOptions(source:.default)) { // ...

Also, explicitly calling init is not required. Using a constructor-style invocation calls the init function.

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.

}
docRef.getDocument(options:GetOptions.init(source:Source.server)) { document, error in
docRef.getDocument(options:GetOptions.init(source:GetSource.server)) { document, error in
}
docRef.getDocument(options:GetOptions.init(source:Source.cache)) { document, error in
docRef.getDocument(options:GetOptions.init(source:GetSource.cache)) { document, error in
}
}

Expand All @@ -249,11 +249,11 @@ func readDocuments(matching query: Query) {
func readDocumentsWithOptions(matching query: Query) {
query.getDocuments(options:GetOptions.defaultOptions()) { querySnapshot, error in
}
query.getDocuments(options:GetOptions.init(source:Source.default)) { querySnapshot, error in
query.getDocuments(options:GetOptions.init(source:GetSource.default)) { querySnapshot, error in
}
query.getDocuments(options:GetOptions.init(source:Source.server)) { querySnapshot, error in
query.getDocuments(options:GetOptions.init(source:GetSource.server)) { querySnapshot, error in
}
query.getDocuments(options:GetOptions.init(source:Source.cache)) { querySnapshot, error in
query.getDocuments(options:GetOptions.init(source:GetSource.cache)) { querySnapshot, error in
}
}

Expand Down
51 changes: 28 additions & 23 deletions Firestore/Example/Tests/Integration/API/FIRGetOptionsTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ - (void)testGetDocumentWhileOnlineCacheOnly {
// get doc and ensure that it exists, *is* from the cache, and matches
// the initialData.
FIRDocumentSnapshot *result =
[self readDocumentForRef:doc options:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]];
[self readDocumentForRef:doc
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.exists);
XCTAssertTrue(result.metadata.fromCache);
XCTAssertFalse(result.metadata.hasPendingWrites);
Expand All @@ -167,7 +168,7 @@ - (void)testGetCollectionWhileOnlineCacheOnly {
// initialDocs.
FIRQuerySnapshot *result =
[self readDocumentSetForRef:col
options:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]];
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.metadata.fromCache);
XCTAssertFalse(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
Expand Down Expand Up @@ -204,7 +205,8 @@ - (void)testGetDocumentWhileOfflineCacheOnly {
// get doc and ensure it exists, *is* from the cache, and matches the
// newData.
FIRDocumentSnapshot *result =
[self readDocumentForRef:doc options:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]];
[self readDocumentForRef:doc
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.exists);
XCTAssertTrue(result.metadata.fromCache);
XCTAssertTrue(result.metadata.hasPendingWrites);
Expand Down Expand Up @@ -236,7 +238,7 @@ - (void)testGetCollectionWhileOfflineCacheOnly {
// data.
FIRQuerySnapshot *result =
[self readDocumentSetForRef:col
options:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]];
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.metadata.fromCache);
XCTAssertTrue(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
Expand All @@ -262,7 +264,8 @@ - (void)testGetDocumentWhileOnlineServerOnly {
// get doc and ensure that it exists, is *not* from the cache, and matches
// the initialData.
FIRDocumentSnapshot *result =
[self readDocumentForRef:doc options:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]];
[self readDocumentForRef:doc
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]];
XCTAssertTrue(result.exists);
XCTAssertFalse(result.metadata.fromCache);
XCTAssertFalse(result.metadata.hasPendingWrites);
Expand All @@ -284,7 +287,7 @@ - (void)testGetCollectionWhileOnlineServerOnly {
// initialData.
FIRQuerySnapshot *result =
[self readDocumentSetForRef:col
options:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]];
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]];
XCTAssertFalse(result.metadata.fromCache);
XCTAssertFalse(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
Expand All @@ -311,7 +314,7 @@ - (void)testGetDocumentWhileOfflineServerOnly {

// attempt to get doc and ensure it cannot be retreived
XCTestExpectation *failedGetDocCompletion = [self expectationWithDescription:@"failedGetDoc"];
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand All @@ -337,7 +340,7 @@ - (void)testGetCollectionWhileOfflineServerOnly {

// attempt to get docs and ensure they cannot be retreived
XCTestExpectation *failedGetDocsCompletion = [self expectationWithDescription:@"failedGetDocs"];
[col getDocumentsWithOptions:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]
[col getDocumentsWithOptions:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]
completion:^(FIRQuerySnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand Down Expand Up @@ -377,23 +380,24 @@ - (void)testGetDocumentWhileOfflineWithDifferentGetOptions {
// get doc (from cache) and ensure it exists, *is* from the cache, and
// matches the newData.
FIRDocumentSnapshot *result =
[self readDocumentForRef:doc options:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]];
[self readDocumentForRef:doc
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.exists);
XCTAssertTrue(result.metadata.fromCache);
XCTAssertTrue(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(result.data, newData);

// attempt to get doc (with default get options)
result =
[self readDocumentForRef:doc options:[[FIRGetOptions alloc] initWithSource:FIRSourceDefault]];
result = [self readDocumentForRef:doc
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceDefault]];
XCTAssertTrue(result.exists);
XCTAssertTrue(result.metadata.fromCache);
XCTAssertTrue(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(result.data, newData);

// attempt to get doc (from the server) and ensure it cannot be retreived
XCTestExpectation *failedGetDocCompletion = [self expectationWithDescription:@"failedGetDoc"];
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand Down Expand Up @@ -437,7 +441,7 @@ - (void)testGetCollectionWhileOfflineWithDifferentGetOptions {
// matches the updated data.
FIRQuerySnapshot *result =
[self readDocumentSetForRef:col
options:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]];
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.metadata.fromCache);
XCTAssertTrue(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
Expand All @@ -454,7 +458,7 @@ - (void)testGetCollectionWhileOfflineWithDifferentGetOptions {

// attempt to get docs (with default get options)
result = [self readDocumentSetForRef:col
options:[[FIRGetOptions alloc] initWithSource:FIRSourceDefault]];
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceDefault]];
XCTAssertTrue(result.metadata.fromCache);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
@{@"key1" : @"value1"}, @{@"key2" : @"value2", @"key2b" : @"value2b"},
Expand All @@ -470,7 +474,7 @@ - (void)testGetCollectionWhileOfflineWithDifferentGetOptions {

// attempt to get docs (from the server) and ensure they cannot be retreived
XCTestExpectation *failedGetDocsCompletion = [self expectationWithDescription:@"failedGetDocs"];
[col getDocumentsWithOptions:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]
[col getDocumentsWithOptions:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]
completion:^(FIRQuerySnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand Down Expand Up @@ -543,7 +547,7 @@ - (void)testGetNonExistingDocWhileOnlineCacheOnly {
// certain documents *don't* exist.
XCTestExpectation *getNonExistingDocCompletion =
[self expectationWithDescription:@"getNonExistingDoc"];
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand All @@ -559,7 +563,7 @@ - (void)testGetNonExistingCollectionWhileOnlineCacheOnly {
// get collection and ensure it's empty and that it *is* from the cache.
FIRQuerySnapshot *snapshot =
[self readDocumentSetForRef:col
options:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]];
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]];
XCTAssertEqual(snapshot.count, 0);
XCTAssertEqual(snapshot.documentChanges.count, 0);
XCTAssertTrue(snapshot.metadata.fromCache);
Expand All @@ -577,7 +581,7 @@ - (void)testGetNonExistingDocWhileOfflineCacheOnly {
// certain documents *don't* exist.
XCTestExpectation *getNonExistingDocCompletion =
[self expectationWithDescription:@"getNonExistingDoc"];
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand All @@ -596,7 +600,7 @@ - (void)testGetNonExistingCollectionWhileOfflineCacheOnly {
// get collection and ensure it's empty and that it *is* from the cache.
FIRQuerySnapshot *snapshot =
[self readDocumentSetForRef:col
options:[[FIRGetOptions alloc] initWithSource:FIRSourceCache]];
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceCache]];
XCTAssertEqual(snapshot.count, 0);
XCTAssertEqual(snapshot.documentChanges.count, 0);
XCTAssertTrue(snapshot.metadata.fromCache);
Expand All @@ -608,7 +612,8 @@ - (void)testGetNonExistingDocWhileOnlineServerOnly {

// get doc and ensure that it does not exist and is *not* from the cache.
FIRDocumentSnapshot *snapshot =
[self readDocumentForRef:doc options:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]];
[self readDocumentForRef:doc
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]];
XCTAssertFalse(snapshot.exists);
XCTAssertFalse(snapshot.metadata.fromCache);
XCTAssertFalse(snapshot.metadata.hasPendingWrites);
Expand All @@ -620,7 +625,7 @@ - (void)testGetNonExistingCollectionWhileOnlineServerOnly {
// get collection and ensure that it's empty and that it's *not* from the cache.
FIRQuerySnapshot *snapshot =
[self readDocumentSetForRef:col
options:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]];
options:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]];
XCTAssertEqual(snapshot.count, 0);
XCTAssertEqual(snapshot.documentChanges.count, 0);
XCTAssertFalse(snapshot.metadata.fromCache);
Expand All @@ -638,7 +643,7 @@ - (void)testGetNonExistingDocWhileOfflineServerOnly {
// certain documents *don't* exist.
XCTestExpectation *getNonExistingDocCompletion =
[self expectationWithDescription:@"getNonExistingDoc"];
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]
[doc getDocumentWithOptions:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand All @@ -656,7 +661,7 @@ - (void)testGetNonExistingCollectionWhileOfflineServerOnly {

// attempt to get collection and ensure that it cannot be retreived
XCTestExpectation *failedGetDocsCompletion = [self expectationWithDescription:@"failedGetDocs"];
[col getDocumentsWithOptions:[[FIRGetOptions alloc] initWithSource:FIRSourceServer]
[col getDocumentsWithOptions:[[FIRGetOptions alloc] initWithSource:FIRGetSourceServer]
completion:^(FIRQuerySnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand Down
24 changes: 12 additions & 12 deletions Firestore/Source/API/FIRDocumentReference.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ - (void)getDocumentWithCompletion:(void (^)(FIRDocumentSnapshot *_Nullable docum
- (void)getDocumentWithOptions:(FIRGetOptions *)options
completion:(void (^)(FIRDocumentSnapshot *_Nullable document,
NSError *_Nullable error))completion {
if (options.source == FIRSourceCache) {
if (options.source == FIRGetSourceCache) {
[self.firestore.client getDocumentFromLocalCache:self completion:completion];
return;
}
Expand Down Expand Up @@ -256,17 +256,17 @@ - (void)getDocumentWithOptions:(FIRGetOptions *)options
@"Failed to get document because the client is offline.",
}]);
} else if (snapshot.exists && snapshot.metadata.fromCache &&
options.source == FIRSourceServer) {
completion(nil,
[NSError errorWithDomain:FIRFirestoreErrorDomain
code:FIRFirestoreErrorCodeUnavailable
userInfo:@{
NSLocalizedDescriptionKey :
@"Failed to get document from server. (However, this "
@"document does exist in the local cache. Run again "
@"without setting FIRSourceServer in the FIRGetOptions to "
@"retrieve the cached document.)"
}]);
options.source == FIRGetSourceServer) {
completion(
nil, [NSError errorWithDomain:FIRFirestoreErrorDomain
code:FIRFirestoreErrorCodeUnavailable
userInfo:@{
NSLocalizedDescriptionKey :
@"Failed to get document from server. (However, this "
@"document does exist in the local cache. Run again "
@"without setting FIRGetSourceServer in the FIRGetOptions to "
@"retrieve the cached document.)"
}]);
} else {
completion(snapshot, nil);
}
Expand Down
2 changes: 1 addition & 1 deletion Firestore/Source/API/FIRGetOptions+Internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface FIRGetOptions ()

/** Where getDocument[s] calls should get their data from. */
@property(nonatomic, readonly, getter=source) FIRSource source;
@property(nonatomic, readonly, getter=source) FIRGetSource source;

@end

Expand Down
4 changes: 2 additions & 2 deletions Firestore/Source/API/FIRGetOptions.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
@implementation FIRGetOptions

+ (FIRGetOptions *)defaultOptions {
return [[FIRGetOptions alloc] initWithSource:FIRSourceDefault];
return [[FIRGetOptions alloc] initWithSource:FIRGetSourceDefault];
}

- (instancetype)initWithSource:(FIRSource)source {
- (instancetype)initWithSource:(FIRGetSource)source {
if (self = [super init]) {
_source = source;
}
Expand Down
24 changes: 12 additions & 12 deletions Firestore/Source/API/FIRQuery.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ - (void)getDocumentsWithCompletion:(void (^)(FIRQuerySnapshot *_Nullable snapsho
- (void)getDocumentsWithOptions:(FIRGetOptions *)options
completion:(void (^)(FIRQuerySnapshot *_Nullable snapshot,
NSError *_Nullable error))completion {
if (options.source == FIRSourceCache) {
if (options.source == FIRGetSourceCache) {
[self.firestore.client getDocumentsFromLocalCache:self completion:completion];
return;
}
Expand All @@ -161,17 +161,17 @@ - (void)getDocumentsWithOptions:(FIRGetOptions *)options
dispatch_semaphore_wait(registered, DISPATCH_TIME_FOREVER);
[listenerRegistration remove];

if (snapshot.metadata.fromCache && options.source == FIRSourceServer) {
completion(nil,
[NSError errorWithDomain:FIRFirestoreErrorDomain
code:FIRFirestoreErrorCodeUnavailable
userInfo:@{
NSLocalizedDescriptionKey :
@"Failed to get documents from server. (However, these "
@"documents may exist in the local cache. Run again "
@"without setting FIRSourceServer in the FIRGetOptions to "
@"retrieve the cached documents.)"
}]);
if (snapshot.metadata.fromCache && options.source == FIRGetSourceServer) {
completion(
nil, [NSError errorWithDomain:FIRFirestoreErrorDomain
code:FIRFirestoreErrorCodeUnavailable
userInfo:@{
NSLocalizedDescriptionKey :
@"Failed to get documents from server. (However, these "
@"documents may exist in the local cache. Run again "
@"without setting FIRGetSourceServer in the FIRGetOptions to "
@"retrieve the cached documents.)"
}]);
} else {
completion(snapshot, nil);
}
Expand Down
Loading