Skip to content

Convert FIRGetOptions to use instance methods (only) #741

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

Closed
wants to merge 2 commits into from
Closed

Convert FIRGetOptions to use instance methods (only) #741

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
aecdbab
Convert FIRGetOptions to use instance methods (only)
rsgowman Feb 1, 2018
9b02384
style.sh
rsgowman Feb 1, 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.source(GetSource.default)) { document, error in
docRef.getDocument(options:GetOptions().source(GetSource.default)) { document, error in
}
docRef.getDocument(options:GetOptions.source(.server)) { document, error in
docRef.getDocument(options:GetOptions().source(.server)) { document, error in
}
docRef.getDocument(options:GetOptions.source(GetSource.cache)) { document, error in
docRef.getDocument(options:GetOptions().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.source(GetSource.default)) { querySnapshot, error in
query.getDocuments(options:GetOptions().source(GetSource.default)) { querySnapshot, error in
}
query.getDocuments(options:GetOptions.source(.server)) { querySnapshot, error in
query.getDocuments(options:GetOptions().source(.server)) { querySnapshot, error in
}
query.getDocuments(options:GetOptions.source(GetSource.cache)) { querySnapshot, error in
query.getDocuments(options:GetOptions().source(GetSource.cache)) { querySnapshot, error in
}
}

Expand Down
76 changes: 45 additions & 31 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 optionsWithSource:FIRGetSourceCache]];
[self readDocumentForRef:doc
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.exists);
XCTAssertTrue(result.metadata.fromCache);
XCTAssertFalse(result.metadata.hasPendingWrites);
Expand All @@ -165,8 +166,9 @@ - (void)testGetCollectionWhileOnlineCacheOnly {

// get docs and ensure they *are* from the cache, and matches the
// initialDocs.
FIRQuerySnapshot *result =
[self readDocumentSetForRef:col options:[FIRGetOptions optionsWithSource:FIRGetSourceCache]];
FIRQuerySnapshot *result = [self
readDocumentSetForRef:col
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.metadata.fromCache);
XCTAssertFalse(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
Expand Down Expand Up @@ -203,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 optionsWithSource:FIRGetSourceCache]];
[self readDocumentForRef:doc
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.exists);
XCTAssertTrue(result.metadata.fromCache);
XCTAssertTrue(result.metadata.hasPendingWrites);
Expand Down Expand Up @@ -233,8 +236,9 @@ - (void)testGetCollectionWhileOfflineCacheOnly {

// get docs and ensure they *are* from the cache, and matches the updated
// data.
FIRQuerySnapshot *result =
[self readDocumentSetForRef:col options:[FIRGetOptions optionsWithSource:FIRGetSourceCache]];
FIRQuerySnapshot *result = [self
readDocumentSetForRef:col
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.metadata.fromCache);
XCTAssertTrue(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
Expand All @@ -260,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 optionsWithSource:FIRGetSourceServer]];
[self readDocumentForRef:doc
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]];
XCTAssertTrue(result.exists);
XCTAssertFalse(result.metadata.fromCache);
XCTAssertFalse(result.metadata.hasPendingWrites);
Expand All @@ -280,8 +285,9 @@ - (void)testGetCollectionWhileOnlineServerOnly {

// get docs and ensure they are *not* from the cache, and matches the
// initialData.
FIRQuerySnapshot *result =
[self readDocumentSetForRef:col options:[FIRGetOptions optionsWithSource:FIRGetSourceServer]];
FIRQuerySnapshot *result = [self
readDocumentSetForRef:col
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]];
XCTAssertFalse(result.metadata.fromCache);
XCTAssertFalse(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
Expand All @@ -308,7 +314,7 @@ - (void)testGetDocumentWhileOfflineServerOnly {

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

// attempt to get docs and ensure they cannot be retreived
XCTestExpectation *failedGetDocsCompletion = [self expectationWithDescription:@"failedGetDocs"];
[col getDocumentsWithOptions:[FIRGetOptions optionsWithSource:FIRGetSourceServer]
[col getDocumentsWithOptions:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]
completion:^(FIRQuerySnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand Down Expand Up @@ -374,23 +380,25 @@ - (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 optionsWithSource:FIRGetSourceCache]];
[self readDocumentForRef:doc
options:[[[FIRGetOptions alloc] init] optionsWithSource: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 optionsWithSource:FIRGetSourceDefault]];
result = [self
readDocumentForRef:doc
options:[[[FIRGetOptions alloc] init] optionsWithSource: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 optionsWithSource:FIRGetSourceServer]
[doc getDocumentWithOptions:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand Down Expand Up @@ -432,8 +440,9 @@ - (void)testGetCollectionWhileOfflineWithDifferentGetOptions {

// get docs (from cache) and ensure they *are* from the cache, and
// matches the updated data.
FIRQuerySnapshot *result =
[self readDocumentSetForRef:col options:[FIRGetOptions optionsWithSource:FIRGetSourceCache]];
FIRQuerySnapshot *result = [self
readDocumentSetForRef:col
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]];
XCTAssertTrue(result.metadata.fromCache);
XCTAssertTrue(result.metadata.hasPendingWrites);
XCTAssertEqualObjects(FIRQuerySnapshotGetData(result), (@[
Expand All @@ -449,8 +458,9 @@ - (void)testGetCollectionWhileOfflineWithDifferentGetOptions {
]));

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

// attempt to get docs (from the server) and ensure they cannot be retreived
XCTestExpectation *failedGetDocsCompletion = [self expectationWithDescription:@"failedGetDocs"];
[col getDocumentsWithOptions:[FIRGetOptions optionsWithSource:FIRGetSourceServer]
[col getDocumentsWithOptions:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]
completion:^(FIRQuerySnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand Down Expand Up @@ -539,7 +549,7 @@ - (void)testGetNonExistingDocWhileOnlineCacheOnly {
// certain documents *don't* exist.
XCTestExpectation *getNonExistingDocCompletion =
[self expectationWithDescription:@"getNonExistingDoc"];
[doc getDocumentWithOptions:[FIRGetOptions optionsWithSource:FIRGetSourceCache]
[doc getDocumentWithOptions:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand All @@ -553,8 +563,9 @@ - (void)testGetNonExistingCollectionWhileOnlineCacheOnly {
FIRCollectionReference *col = [self collectionRef];

// get collection and ensure it's empty and that it *is* from the cache.
FIRQuerySnapshot *snapshot =
[self readDocumentSetForRef:col options:[FIRGetOptions optionsWithSource:FIRGetSourceCache]];
FIRQuerySnapshot *snapshot = [self
readDocumentSetForRef:col
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]];
XCTAssertEqual(snapshot.count, 0);
XCTAssertEqual(snapshot.documentChanges.count, 0);
XCTAssertTrue(snapshot.metadata.fromCache);
Expand All @@ -572,7 +583,7 @@ - (void)testGetNonExistingDocWhileOfflineCacheOnly {
// certain documents *don't* exist.
XCTestExpectation *getNonExistingDocCompletion =
[self expectationWithDescription:@"getNonExistingDoc"];
[doc getDocumentWithOptions:[FIRGetOptions optionsWithSource:FIRGetSourceCache]
[doc getDocumentWithOptions:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand All @@ -589,8 +600,9 @@ - (void)testGetNonExistingCollectionWhileOfflineCacheOnly {
[self disableNetwork];

// get collection and ensure it's empty and that it *is* from the cache.
FIRQuerySnapshot *snapshot =
[self readDocumentSetForRef:col options:[FIRGetOptions optionsWithSource:FIRGetSourceCache]];
FIRQuerySnapshot *snapshot = [self
readDocumentSetForRef:col
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceCache]];
XCTAssertEqual(snapshot.count, 0);
XCTAssertEqual(snapshot.documentChanges.count, 0);
XCTAssertTrue(snapshot.metadata.fromCache);
Expand All @@ -602,7 +614,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 optionsWithSource:FIRGetSourceServer]];
[self readDocumentForRef:doc
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]];
XCTAssertFalse(snapshot.exists);
XCTAssertFalse(snapshot.metadata.fromCache);
XCTAssertFalse(snapshot.metadata.hasPendingWrites);
Expand All @@ -612,8 +625,9 @@ - (void)testGetNonExistingCollectionWhileOnlineServerOnly {
FIRCollectionReference *col = [self collectionRef];

// get collection and ensure that it's empty and that it's *not* from the cache.
FIRQuerySnapshot *snapshot =
[self readDocumentSetForRef:col options:[FIRGetOptions optionsWithSource:FIRGetSourceServer]];
FIRQuerySnapshot *snapshot = [self
readDocumentSetForRef:col
options:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]];
XCTAssertEqual(snapshot.count, 0);
XCTAssertEqual(snapshot.documentChanges.count, 0);
XCTAssertFalse(snapshot.metadata.fromCache);
Expand All @@ -631,7 +645,7 @@ - (void)testGetNonExistingDocWhileOfflineServerOnly {
// certain documents *don't* exist.
XCTestExpectation *getNonExistingDocCompletion =
[self expectationWithDescription:@"getNonExistingDoc"];
[doc getDocumentWithOptions:[FIRGetOptions optionsWithSource:FIRGetSourceServer]
[doc getDocumentWithOptions:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]
completion:^(FIRDocumentSnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand All @@ -649,7 +663,7 @@ - (void)testGetNonExistingCollectionWhileOfflineServerOnly {

// attempt to get collection and ensure that it cannot be retreived
XCTestExpectation *failedGetDocsCompletion = [self expectationWithDescription:@"failedGetDocs"];
[col getDocumentsWithOptions:[FIRGetOptions optionsWithSource:FIRGetSourceServer]
[col getDocumentsWithOptions:[[[FIRGetOptions alloc] init] optionsWithSource:FIRGetSourceServer]
completion:^(FIRQuerySnapshot *snapshot, NSError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
Expand Down
2 changes: 0 additions & 2 deletions Firestore/Source/API/FIRGetOptions+Internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ NS_ASSUME_NONNULL_BEGIN
/** Where getDocument[s] calls should get their data from. */
@property(nonatomic, readonly, getter=source) FIRGetSource source;

- (instancetype)initWithSource:(FIRGetSource)source;

@end

NS_ASSUME_NONNULL_END
11 changes: 6 additions & 5 deletions Firestore/Source/API/FIRGetOptions.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,19 @@
@implementation FIRGetOptions

+ (instancetype)defaultOptions {
return [[FIRGetOptions alloc] initWithSource:FIRGetSourceDefault];
return [[FIRGetOptions alloc] init];
}

- (instancetype)initWithSource:(FIRGetSource)source {
- (instancetype)init {
if (self = [super init]) {
_source = source;
_source = FIRGetSourceDefault;
}
return self;
}

+ (instancetype)optionsWithSource:(FIRGetSource)source {
return [[FIRGetOptions alloc] initWithSource:source];
- (instancetype)optionsWithSource:(FIRGetSource)source {
_source = source;
return self;
}

@end
Expand Down
5 changes: 1 addition & 4 deletions Firestore/Source/Public/FIRGetOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ NS_ASSUME_NONNULL_BEGIN
NS_SWIFT_NAME(GetOptions)
@interface FIRGetOptions : NSObject

/** */
- (instancetype)init __attribute((unavailable("FIRGetOptions cannot be created directly.")));

/**
* Returns the default options.
*
Expand Down Expand Up @@ -65,7 +62,7 @@ typedef NS_ENUM(NSUInteger, FIRGetSource) {
/**
* Initializes the get options with the specified source.
*/
+ (instancetype)optionsWithSource:(FIRGetSource)source NS_SWIFT_NAME(source(_:));
- (instancetype)optionsWithSource:(FIRGetSource)source NS_SWIFT_NAME(source(_:));
Copy link
Contributor

Choose a reason for hiding this comment

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

optionsWithSource suggests immutability. Perhaps this should be setSource?

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's an initializer, it should be

- (instancetype)initWithSource:(FIRGetSource)source

The Swift name will automatically be synthesized to init(source:).


@end

Expand Down