Skip to content

Commit

Permalink
[android-sdk] Remove some open* overloads from Session.
Browse files Browse the repository at this point in the history
Summary:
Session had a number of instance and static methods related to opening sessions and active sessions, and it was not
always clear which one was the right one to use. Removed some of them and renamed others to be clearer about their
intention.

Test Plan:
- Ran unit tests
- Ran samples

Revert Plan:

Reviewers: mmarucheck, mingfli, karthiks, ekoneil

Reviewed By: mingfli

CC: msdkexp@, caabernathy

Differential Revision: https://phabricator.fb.com/D652216
  • Loading branch information
Chris Lang committed Dec 10, 2012
1 parent ba8c07d commit 4c68439
Show file tree
Hide file tree
Showing 13 changed files with 80 additions and 201 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ facebook/lint.xml
facebook/tests/lint.xml
facebook/tests/assets/config.json
.idea/workspace.xml
.idea/dictionaries/
.idea/dictionaries/
.idea/inspectionProfiles/
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package com.facebook;

/**
* An Exception indicating that a Session failed to open or reauthorize.
* An Exception indicating that a Session failed to open or obtain new permissions.
*/
public class FacebookAuthorizationException extends FacebookException {
static final long serialVersionUID = 1;
Expand Down
203 changes: 44 additions & 159 deletions facebook/src/com/facebook/Session.java

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions facebook/src/com/facebook/SessionLoginBehavior.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
* Specifies the behaviors to try during
* {@link Session#openForRead(com.facebook.Session.OpenRequest) openForRead},
* {@link Session#openForPublish(com.facebook.Session.OpenRequest) openForPublish},
* {@link Session#reauthorizeForRead(com.facebook.Session.ReauthorizeRequest) reauthorizeForRead}, or
* {@link Session#reauthorizeForPublish(com.facebook.Session.ReauthorizeRequest) reauthorizeForPublish}.
* {@link Session#requestNewReadPermissions(com.facebook.Session.NewPermissionsRequest) requestNewReadPermissions}, or
* {@link Session#requestNewPublishPermissions(com.facebook.Session.NewPermissionsRequest) requestNewPublishPermissions}.
*/
public enum SessionLoginBehavior {
/**
Expand All @@ -32,7 +32,7 @@ public enum SessionLoginBehavior {

/**
* Specifies that Session should only attempt SSO. If SSO fails, then the
* open or reauthorize call fails.
* open or new permissions call fails.
*/
SSO_ONLY(true, false),

Expand Down
2 changes: 1 addition & 1 deletion facebook/src/com/facebook/UiLifecycleHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void onResume() {
session.addCallback(callback);
}
if (SessionState.CREATED_TOKEN_LOADED.equals(session.getState())) {
session.open();
session.openForRead(null);
}
}

Expand Down
9 changes: 1 addition & 8 deletions facebook/src/com/facebook/widget/LoginButton.java
Original file line number Diff line number Diff line change
Expand Up @@ -547,14 +547,7 @@ private boolean initializeActiveSessionWithCachedToken(Context context) {
return false;
}

session = new Session(context);
if (session.getState() != SessionState.CREATED_TOKEN_LOADED) {
return false;
}

Session.setActiveSession(session);
session.open();
return true;
return Session.openActiveSessionFromCache(context) != null;
}

private void fetchUserInfo() {
Expand Down
20 changes: 10 additions & 10 deletions facebook/tests/src/com/facebook/SessionTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public void run() {

// When we open it, then we should see the Opened event.
receiverOpened.incrementExpectCount();
session0.open();
session0.openForRead(null);
receiverOpened.waitForExpectedCalls();

// Setting to itself should not fire events
Expand All @@ -223,7 +223,7 @@ public void run() {
setTokenCachingStrategy(new MockTokenCachingStrategy()).
build();
assertEquals(SessionState.CREATED_TOKEN_LOADED, session1.getState());
session1.open();
session1.openForRead(null);
assertEquals(SessionState.OPENED, session1.getState());
Session.setActiveSession(session1);
WaitForBroadcastReceiver.waitForExpectedCalls(receiverClosed, receiverUnset, receiverSet, receiverOpened);
Expand Down Expand Up @@ -408,7 +408,7 @@ public void testOpenFromTokenCache() {
public void testOpenActiveFromEmptyTokenCache() {
new SharedPreferencesTokenCachingStrategy(getActivity()).clear();

assertNull(Session.openActiveSession(getActivity(), false));
assertNull(Session.openActiveSessionFromCache(getActivity()));
}


Expand Down Expand Up @@ -459,7 +459,7 @@ public void testOpenForReadFailure() {
@SmallTest
@MediumTest
@LargeTest
public void testReauthorizeSuccess() {
public void testRequestNewReadPermissionsSuccess() {
ArrayList<String> permissions = new ArrayList<String>();
SessionStatusCallbackRecorder statusRecorder = new SessionStatusCallbackRecorder();
MockTokenCachingStrategy cache = new MockTokenCachingStrategy(null, 0);
Expand Down Expand Up @@ -487,7 +487,7 @@ public void testReauthorizeSuccess() {

session.addAccessTokenToFbidMapping(reauthorizeToken, USER_1_FBID);
session.addAuthorizeResult(reauthorizeToken, "play_outside", "eat_ice_cream");
session.reauthorizeForRead(new Session.ReauthorizeRequest(getActivity(), permissions));
session.requestNewReadPermissions(new Session.NewPermissionsRequest(getActivity(), permissions));
statusRecorder.waitForCall(session, SessionState.OPENED_TOKEN_UPDATED, null);

verifySessionHasToken(session, reauthorizeToken);
Expand All @@ -499,7 +499,7 @@ public void testReauthorizeSuccess() {
permissions.add("run_with_scissors");

session.addAuthorizeResult(reauthorizeException);
session.reauthorizeForRead(new Session.ReauthorizeRequest(getActivity(), permissions));
session.requestNewReadPermissions(new Session.NewPermissionsRequest(getActivity(), permissions));
statusRecorder.waitForCall(session, SessionState.OPENED_TOKEN_UPDATED, reauthorizeException);

// Verify we do not overwrite cache if reauthorize fails
Expand All @@ -515,7 +515,7 @@ public void testReauthorizeSuccess() {
@SmallTest
@MediumTest
@LargeTest
public void testReauthorizeForPublishSuccess() {
public void testRequestNewPublishPermissionsSuccess() {
ArrayList<String> permissions = new ArrayList<String>();
SessionStatusCallbackRecorder statusRecorder = new SessionStatusCallbackRecorder();
MockTokenCachingStrategy cache = new MockTokenCachingStrategy(null, 0);
Expand Down Expand Up @@ -543,7 +543,7 @@ public void testReauthorizeForPublishSuccess() {

session.addAccessTokenToFbidMapping(reauthorizeToken, USER_1_FBID);
session.addAuthorizeResult(reauthorizeToken, "play_outside", "publish_eat_ice_cream");
session.reauthorizeForPublish(new Session.ReauthorizeRequest(getActivity(), permissions));
session.requestNewPublishPermissions(new Session.NewPermissionsRequest(getActivity(), permissions));
statusRecorder.waitForCall(session, SessionState.OPENED_TOKEN_UPDATED, null);

verifySessionHasToken(session, reauthorizeToken);
Expand All @@ -554,7 +554,7 @@ public void testReauthorizeForPublishSuccess() {
permissions.add("publish_run_with_scissors");

try {
session.reauthorizeForRead(new Session.ReauthorizeRequest(getActivity(), permissions));
session.requestNewReadPermissions(new Session.NewPermissionsRequest(getActivity(), permissions));
fail("Should not reach here without an exception");
} catch (FacebookException e) {
assertTrue(e.getMessage().contains("Cannot pass a publish permission"));
Expand Down Expand Up @@ -600,7 +600,7 @@ public void testReauthorizeWithDifferentUserTokenFails() {
.createFromString(USER_2_ACCESS_TOKEN, reauthPermissions, AccessTokenSource.TEST_USER);

session.addAuthorizeResult(reauthorizeToken, Arrays.asList("user_photos"));
session.reauthorizeForPublish(new Session.ReauthorizeRequest(getActivity(), reauthPermissions));
session.requestNewPublishPermissions(new Session.NewPermissionsRequest(getActivity(), reauthPermissions));
statusRecorder.waitForCall(session, SessionState.OPENED, new FacebookAuthorizationException());

verifySessionHasToken(session, openToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void testDelegateWithActiveSession() throws Exception {
final SessionTracker tracker = new SessionTracker(getActivity(), statusRecorder);
Session.setActiveSession(session);

session.openForRead(getActivity());
session.openForRead(new Session.OpenRequest(getActivity()));

statusRecorder.waitForCall(session, SessionState.OPENING, null);
statusRecorder.waitForCall(session, SessionState.OPENED, null);
Expand Down Expand Up @@ -74,7 +74,7 @@ public void testDelegateWithSessionInConstructor() throws Exception {

SessionTracker tracker = new SessionTracker(getActivity(), statusRecorder, session);

session.openForRead(getActivity());
session.openForRead(new Session.OpenRequest(getActivity()));

statusRecorder.waitForCall(session, SessionState.OPENING, null);
statusRecorder.waitForCall(session, SessionState.OPENED, null);
Expand Down Expand Up @@ -103,7 +103,7 @@ public void testDelegateWithActiveSessionThenNewSession() throws Exception {
SessionTracker tracker = new SessionTracker(getActivity(), statusRecorder);
Session.setActiveSession(session);

session.openForRead(getActivity());
session.openForRead(new Session.OpenRequest(getActivity()));

statusRecorder.waitForCall(session, SessionState.OPENING, null);
statusRecorder.waitForCall(session, SessionState.OPENED, null);
Expand All @@ -117,7 +117,7 @@ public void testDelegateWithActiveSessionThenNewSession() throws Exception {

tracker.setSession(newSession);
assertNull("Session should not be open", tracker.getOpenSession());
newSession.openForRead(getActivity());
newSession.openForRead(new Session.OpenRequest(getActivity()));

statusRecorder.waitForCall(newSession, SessionState.OPENING, null);
statusRecorder.waitForCall(newSession, SessionState.OPENED, null);
Expand Down Expand Up @@ -145,7 +145,7 @@ public void testDelegateWithSessionThenActiveSession() throws Exception {

final SessionTracker tracker = new SessionTracker(getActivity(), statusRecorder, session);

session.openForRead(getActivity());
session.openForRead(new Session.OpenRequest(getActivity()));

statusRecorder.waitForCall(session, SessionState.OPENING, null);
statusRecorder.waitForCall(session, SessionState.OPENED, null);
Expand All @@ -167,7 +167,7 @@ public void run() {
}, true);

assertNull("Session should not be open", tracker.getOpenSession());
newSession.openForRead(getActivity());
newSession.openForRead(new Session.OpenRequest(getActivity()));

statusRecorder.waitForCall(newSession, SessionState.OPENING, null);
statusRecorder.waitForCall(newSession, SessionState.OPENED, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ private void sendPendingPost() {

List<String> permissions = session.getPermissions();
if (!permissions.containsAll(PERMISSIONS)) {
Session.ReauthorizeRequest reauthRequest = new Session.ReauthorizeRequest(this, PERMISSIONS);
session.reauthorizeForPublish(reauthRequest);
Session.NewPermissionsRequest newPermissionsRequest = new Session.NewPermissionsRequest(this, PERMISSIONS);
session.requestNewPublishPermissions(newPermissionsRequest);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void onClick(View view) {

if (Session.getActiveSession() == null ||
Session.getActiveSession().isClosed()) {
Session.openActiveSession(this, true);
Session.openActiveSession(this, true, null);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,8 @@ private void performPublish(PendingAction action) {
// We can do the action right away.
handlePendingAction();
} else {
// We need to reauthorize, then complete the action when we get called back.
session.reauthorizeForPublish(new Session.ReauthorizeRequest(this, PERMISSIONS));
// We need to get new permissions, then complete the action when we get called back.
session.requestNewPublishPermissions(new Session.NewPermissionsRequest(this, PERMISSIONS));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void onClick(View view) {

if (Session.getActiveSession() == null ||
Session.getActiveSession().isClosed()) {
Session.openActiveSession(this, true);
Session.openActiveSession(this, true, null);
}

locationManager = (LocationManager) getSystemService(Context.LOCATION_SERVICE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ private void handleAnnounce() {
List<String> permissions = session.getPermissions();
if (!permissions.containsAll(PERMISSIONS)) {
pendingAnnounce = true;
reauthPublishPermissions(session);
requestPublishPermissions(session);
return;
}

Expand Down Expand Up @@ -233,11 +233,11 @@ protected void onPostExecute(Response response) {
task.execute();
}

private void reauthPublishPermissions(Session session) {
private void requestPublishPermissions(Session session) {
if (session != null) {
Session.ReauthorizeRequest reauthRequest = new Session.ReauthorizeRequest(this, PERMISSIONS).
Session.NewPermissionsRequest newPermissionsRequest = new Session.NewPermissionsRequest(this, PERMISSIONS).
setRequestCode(REAUTH_ACTIVITY_CODE);
session.reauthorizeForPublish(reauthRequest);
session.requestNewPublishPermissions(newPermissionsRequest);
}
}

Expand Down Expand Up @@ -308,13 +308,13 @@ public void onClick(DialogInterface dialogInterface, int i) {
break;

case PERMISSION:
// reauthorize for the publish permission
// request the publish permission
dialogBody = getString(R.string.error_permission);
listener = new DialogInterface.OnClickListener() {
@Override
public void onClick(DialogInterface dialogInterface, int i) {
pendingAnnounce = true;
reauthPublishPermissions(Session.getActiveSession());
requestPublishPermissions(Session.getActiveSession());
}
};
break;
Expand Down

0 comments on commit 4c68439

Please sign in to comment.