Skip to content
This repository was archived by the owner on Jan 16, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
17 changes: 17 additions & 0 deletions ParseTwitterUtils/PF_Twitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ NS_ASSUME_NONNULL_BEGIN
*/
@interface PF_Twitter : NSObject

/**
An instance of `PF_Twitter` configured to access device Twitter accounts with Accounts.framework,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Initializes an instance of ...

and remote Twitter accounts - if no accounts are found locally - through a built-in webview.

After setting `consumerKey` and `consumerSecret`, authorization to Twitter accounts can be requested with
`authorizeInBackground`, and then revoked with its opposite, `deauthorizeInBackground`.
*/
- (instancetype)init;

/**
Consumer key of the application that is used to authorize with Twitter.
*/
Expand Down Expand Up @@ -59,6 +68,14 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (BFTask *)authorizeInBackground;

/**
Invalidates an OAuth token for the current user, if the Twitter user has granted permission to the
current application.

@return The task, that encapsulates the work being done.
*/
- (BFTask *)deauthorizeInBackground;

/**
Displays an auth dialog and populates the authToken, authTokenSecret, userId, and screenName properties
if the Twitter user grants permission to the application.
Expand Down
44 changes: 44 additions & 0 deletions ParseTwitterUtils/PF_Twitter.m
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,30 @@ - (BFTask *)authorizeInBackground {
return source.task;
}];
}
- (BFTask *)deauthorizeInBackground {
if (self.consumerKey.length == 0 || self.consumerSecret.length == 0) {
//TODO: (nlutsenko) This doesn't look right, maybe we should add additional error code?
return [BFTask taskWithError:[NSError errorWithDomain:PFParseErrorDomain code:1 userInfo:nil]];
}

return [[self _performDeauthAsync] pftw_continueAsyncWithBlock:^id(BFTask *task) {
BFTaskCompletionSource *source = [BFTaskCompletionSource taskCompletionSource];
dispatch_async(dispatch_get_main_queue(), ^{
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure whether we need to actually dispatch to main thread here, since the model of tasks is to move to another thread on the next task, but not on this one.
This basically means - that you don't need to dispatch_async here, and can simply setLoginResultValues:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 will remove -- and confession: this was mostly copied from authorizeInBackground, which also has a dispatch_async to the main queue (and why theres a //TODO with your name in it, oops, heh). worth removing that dispatch_async at the same time, because of the same logic?

if (task.cancelled) {
[source cancel];
} else if (!task.error && !task.result) {
source.result = nil;
} else if (task.error) {
[source trySetError:task.error];
} else if (task.result) {
[self setLoginResultValues:@{}];
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are reading from all the fields and reading is safe from nil - you can simply pass nil here?
Doesn't change the behavior, but avoids creating another dictionary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with passing in an empty dictionary literal since it's nice to not have an optional when dealing with Swift, and, well, it's a cheap operation (iirc, it's like one extra mov instruction compared to passing in nil).

that said, i don't feel that too strongly about this, and can revert if you would prefer ¯_(ツ)_/¯

Copy link
Contributor Author

@zadr zadr Apr 28, 2016

Choose a reason for hiding this comment

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

thinking about it, i'll just make it nil, since, it matches what the rest of the project does, and thats the more important thing here anyway. sorry for those few stubborn minutes 😅


[source trySetResult:task.result];
}
});
return source.task;
}];
}

- (void)authorizeWithSuccess:(void (^)(void))success
failure:(void (^)(NSError *error))failure
Expand Down Expand Up @@ -428,6 +452,26 @@ - (BFTask *)_performWebViewAuthAsync {
return source.task;
}

- (BFTask *)_performDeauthAsync {
NSMutableURLRequest *request = [[NSMutableURLRequest alloc] init];
request.URL = [NSURL URLWithString:@"https://api.twitter.com/oauth2/invalidate_token"];
request.HTTPMethod = @"POST";

[self signRequest:request];

BFTaskCompletionSource *taskCompletionSource = [BFTaskCompletionSource taskCompletionSource];

[[self.urlSession dataTaskWithRequest:request completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) {
if (error) {
[taskCompletionSource trySetError:error];
} else {
[taskCompletionSource trySetResult:data];
}
}] resume];

return taskCompletionSource.task;
}

///--------------------------------------
#pragma mark - Sign Request
///--------------------------------------
Expand Down
63 changes: 63 additions & 0 deletions Tests/Unit/TwitterTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -622,4 +622,67 @@ - (void)testAuthorizeWithoutLocalAccountAndNetworkSuccess {
OCMVerifyAll(mockedURLSession);
}

- (void)testDeauthorizeLoggedOutAccount {
id store = PFStrictClassMock([ACAccountStore class]);
NSURLSession *session = PFStrictClassMock([NSURLSession class]);
id mockedDialog = PFStrictProtocolMock(@protocol(PFOAuth1FlowDialogInterface));
PF_Twitter *twitter = [[PF_Twitter alloc] initWithAccountStore:store urlSession:session dialogClass:mockedDialog];

XCTestExpectation *expectation = [self currentSelectorTestExpectation];
[[twitter authorizeInBackground] continueWithBlock:^id(BFTask *task) {
NSError *error = task.error;
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.domain, PFParseErrorDomain);
XCTAssertEqual(error.code, 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to have an error code of 1. Causes a test to fail :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops! a previous iteration of this commit added a new error code of 2, and I missed this in cleaning it up 😅

[expectation fulfill];
return nil;
}];
[self waitForTestExpectations];
}

- (void)testDeauthorizeLoggedInAccount {
id store = PFStrictClassMock([ACAccountStore class]);
NSURLSession *session = PFStrictClassMock([NSURLSession class]);

id mockedStore = PFStrictClassMock([ACAccountStore class]);
id mockedURLSession = PFStrictClassMock([NSURLSession class]);
id mockedOperationQueue = PFStrictClassMock([NSOperationQueue class]);

id mockedDialog = PFStrictProtocolMock(@protocol(PFOAuth1FlowDialogInterface));
PF_Twitter *twitter = [[PF_Twitter alloc] initWithAccountStore:store urlSession:session dialogClass:mockedDialog];
twitter.consumerKey = @"consumer_key";
twitter.consumerSecret = @"consumer_secret";
twitter.authToken = @"auth_token";
twitter.authTokenSecret = @"auth_token_secret";
twitter.userId = @"user_id";
twitter.screenName = @"screen_name";

__block NSURLSessionDataTaskCompletionHandler completionHandler = nil;
[OCMStub([mockedURLSession dataTaskWithRequest:[OCMArg checkWithBlock:^BOOL(id obj) {
NSURLRequest *request = obj;
return [request.URL.lastPathComponent isEqualToString:@"invalidate_token"];
}] completionHandler:[OCMArg checkWithBlock:^BOOL(id obj) {
completionHandler = obj;
return (obj != nil);
}]]).andDo(^(NSInvocation *invocation) {
completionHandler([NSData data], nil, nil);
}) andReturn:[OCMockObject niceMockForClass:[NSURLSessionDataTask class]]];

XCTestExpectation *expectation = [self currentSelectorTestExpectation];
[[twitter deauthorizeInBackground] continueWithBlock:^id(BFTask *task) {
NSError *error = task.error;
XCTAssertNil(error);
XCTAssertNotNil(task.result);

Copy link
Contributor

Choose a reason for hiding this comment

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

The checks for userId and screenName were still useful, sorry if I was not explicit enough about this portion. I feel like it's fine to leave it as is, but if you want to update it till next morning (will merge it then) - feel free to update this part.

XCTAssertNil(twitter.userId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Please fix indentation on lines 677-680

Copy link
Contributor

Choose a reason for hiding this comment

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

Owe, also this looks like this logic is redundant, since you need to check for authToken/authTokenSecret as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 good point! if we don't have an auth token/secret, we shouldn't have any info about the current twitter account anyway. will change it around.

XCTAssertNil(twitter.screenName);
XCTAssertNil(twitter.userId);
XCTAssertNil(twitter.userId);

[expectation fulfill];
return nil;
}];
[self waitForTestExpectations];
}

@end