Skip to content

Logout of Twitter #645

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.

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 1 commit into from
Mar 22, 2017
Merged

Logout of Twitter #645

merged 1 commit into from
Mar 22, 2017

Conversation

SUPERCILEX
Copy link
Collaborator

Signed-off-by: Alex Saveau <[email protected]>
@@ -239,6 +247,10 @@ public Void then(@NonNull Task<GoogleApiClient> task) throws Exception {
// Facebook sign out
LoginManager.getInstance().logOut();

// Twitter sign out
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@samtstern Signing out of Twitter and Facebook should probably a method (something like signOutProviders), but that will cause more thrashing in #609. Would you like me to do the refactor anyway and then undo it in #609 or are we not going through with #609?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep sign out all as one method for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SGTM!

@@ -192,6 +196,10 @@ public Void then(@NonNull Task<GoogleApiClient> task) throws Exception {
// Facebook sign out
LoginManager.getInstance().logOut();

// Twitter sign out
if (!Fabric.isInitialized()) TwitterProvider.initialize(activity);
Twitter.logOut();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we also want to call:
Twitter.getSessionManager().clearActiveSession();

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@samtstern I thought so too, but Twitter.logOut() already does that for us:

// In Twitter.java
public static void logOut() {
    checkInitialized();
    getInstance().core.logOut(); // core == TwitterCore
}

// In TwitterCore.java
public void logOut() {
    checkInitialized();
    final SessionManager<TwitterSession> sessionManager = getSessionManager();
    if (sessionManager != null) {
        sessionManager.clearActiveSession();
    }
}

public static SessionManager<TwitterSession> getSessionManager() {
    checkInitialized();
    return getInstance().core.getSessionManager(); // Same reference as logOut in TwitterCore.java
}

Actually, now that I'm paying more attention, it looks like that's all Twitter#logOut() does.

The docs for TwitterCore#logOut() say it logs the user out, so I think that's all we have to do:

/**
 * Logs out the user, clearing user session. This will not make a network request to invalidate
 * the session.
 *
 * @throws java.lang.IllegalStateException if {@link io.fabric.sdk.android.Fabric}
 *          or {@link TwitterCore} has not been initialized.
 */

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for digging, works for me!

The Twitter docs don't have anything at all on signOut ... oversight I guess.

@samtstern samtstern merged commit 6e388b3 into firebase:version-2.0.0-dev Mar 22, 2017
@SUPERCILEX SUPERCILEX deleted the twitter-logout branch March 22, 2017 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants