nst / STTwitter

A stable, mature and comprehensive Objective-C library for Twitter REST API 1.1
BSD 3-Clause "New" or "Revised" License
999 stars 161 forks source link

Must the account be considered invalid ? #213

Closed es-kumagai closed 9 years ago

es-kumagai commented 9 years ago

Must the account be considered invalid when received ACAccountStoreDidChangeNotification ?

https://github.com/nst/STTwitter/blob/bf42c90b1023fa4e65fd1882c245922aa990dea2/STTwitter/STTwitterAPI.m#L45-L47

It seems that sometimes an ACAccountStoreDidChangeNotification sent even if any of accounts is not edited. I don't know why the notification sent. It seems that some of the notification are sent on OS (OSX 10.10) startup, and also sent when seeing accounts in Internet Account preferences pane.

In this time, the account using with STTwitter is not necessarily the disabled. But always nil will set to STTwitterAPI's oauth property without any condition.

If oauth property become nil during a request processing (e.g. verifyCredentialsWithUserSuccessBlock:successBlock:errorBlock:), the process will be aborted without invoking both successBlock and errorBlock.

https://github.com/nst/STTwitter/blob/bf42c90b1023fa4e65fd1882c245922aa990dea2/STTwitter/STTwitterAPI.m#L237

I think if the account which using with STTwitter become invalidate while processing a request, SLRequest will report error. Then I guess I can detect that the account may be changed without set nil to oauth. In this reason, it may not be necessary to set nil to oauth during processing a request.

At least, I want to errorBlock is called when oauth property become nil.

nst commented 9 years ago

Here is what Apple documentation says about ACAccountStoreDidChangeNotification:

Posted when the accounts managed by this account store changed in the database. There is no userInfo dictionary associated with this notification.

This notification is sent if an account is saved or removed locally or externally. If you receive this notification, you should refetch all account objects.

For now, STTwitter assumes that the account became invalid (which is not always the case) and resets the credentials. So, this behaviour must be improved.

However I'm not yet convinced that asking a delegate such as proposed in PR https://github.com/nst/STTwitter/pull/214 is the way to go. In fact, a good implementation of the delegate method would be sending a request to Twitter API to check if the credentials are still valid or not. But of course this is an asynchronous call so it can't be used to answer the delegate method.

I have to think a little more about this case.

nst commented 9 years ago

I ended up with a protocol as you proposed.

The delegate method is called only when the current account is invalidated.

@protocol STTwitterAPIOSProtocol <NSObject>
- (void)twitterAPI:(STTwitterAPI *)twitterAPI accountWasInvalidated:(ACAccount *)invalidatedAccount;
@end

If the account is modified (eg. for a password change) the delegate method will be sent, but the account will still be valid.

es-kumagai commented 9 years ago

Thank you so much ☺️