From e32ff7116d2064b1a7465dd91cbdfe5d2923ba95 Mon Sep 17 00:00:00 2001 From: Rodrigo Damazio Date: Tue, 12 Jul 2011 20:19:59 -0300 Subject: [PATCH] Simplyfing the authentication dance by handling both success and failure together, and both of them outside onActivityResult. --- .../android/apps/mytracks/MyMapsList.java | 12 +- .../android/apps/mytracks/io/AuthManager.java | 35 ++++-- .../apps/mytracks/io/AuthManagerFactory.java | 16 ++- .../apps/mytracks/io/AuthManagerOld.java | 111 ++++++------------ .../apps/mytracks/io/ModernAuthManager.java | 56 +++++---- .../apps/mytracks/io/gdata/GDataWrapper.java | 47 +++++--- .../io/mymaps/MyMapsGDataWrapper.java | 8 +- 7 files changed, 140 insertions(+), 145 deletions(-) diff --git a/MyTracks/src/com/google/android/apps/mytracks/MyMapsList.java b/MyTracks/src/com/google/android/apps/mytracks/MyMapsList.java index 5aa1d255f..ece0c2b37 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/MyMapsList.java +++ b/MyTracks/src/com/google/android/apps/mytracks/MyMapsList.java @@ -19,6 +19,7 @@ import static com.google.android.apps.mytracks.Constants.TAG; import com.google.android.accounts.Account; import com.google.android.apps.mytracks.io.AuthManager; +import com.google.android.apps.mytracks.io.AuthManager.AuthCallback; import com.google.android.apps.mytracks.io.AuthManagerFactory; import com.google.android.apps.mytracks.io.mymaps.MapsFacade; import com.google.android.apps.mytracks.io.mymaps.MyMapsConstants; @@ -104,8 +105,15 @@ public class MyMapsList extends Activity implements MapsFacade.MapsListCallback private void doLogin(final Account account) { // Starts in the UI thread. - auth.doLogin(new Runnable() { - public void run() { + auth.doLogin(new AuthCallback() { + @Override + public void onAuthResult(boolean success) { + if (!success) { + setResult(RESULT_CANCELED); + finish(); + return; + } + // Runs in UI thread. mapsClient = new MapsFacade(MyMapsList.this, auth); diff --git a/MyTracks/src/com/google/android/apps/mytracks/io/AuthManager.java b/MyTracks/src/com/google/android/apps/mytracks/io/AuthManager.java index 388717b73..15325d8e6 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/io/AuthManager.java +++ b/MyTracks/src/com/google/android/apps/mytracks/io/AuthManager.java @@ -25,15 +25,27 @@ import android.content.Intent; */ public interface AuthManager { /** - * Initializes the login process. The user should be asked to login if they - * haven't already. The {@link Runnable} provided will be executed when the - * auth token is successfully fetched. - * - * @param whenFinished A {@link Runnable} to execute when the auth token - * has been successfully fetched and is available via - * {@link #getAuthToken()} + * Callback for authentication token retrieval operations. */ - void doLogin(Runnable whenFinished, Object o); + public interface AuthCallback { + /** + * Indicates that we're done fetching an auth token. + * + * @param success if true, indicates we have the requested auth token available + * to be retrieved using {@link AuthManager#getAuthToken} + */ + void onAuthResult(boolean success); + } + + /** + * Initializes the login process. The user should be asked to login if they + * haven't already. The {@link AuthCallback} provided will be executed when the + * auth token fetching is done (successfully or not). + * + * @param whenFinished A {@link AuthCallback} to execute when the auth token + * fetching is done + */ + void doLogin(AuthCallback whenFinished, Object o); /** * The {@link android.app.Activity} owner of this class should call this @@ -48,11 +60,8 @@ public interface AuthManager { * {@link android.app.Activity#onActivityResult} function * @param results The data passed in to the {@link android.app.Activity}'s * {@link android.app.Activity#onActivityResult} function - * @return True if the auth token was fetched or we aren't done fetching - * the auth token, or False if there was an error or the request was - * canceled */ - boolean authResult(int resultCode, Intent results); + void authResult(int resultCode, Intent results); /** * Returns the current auth token. Response may be null if no valid auth @@ -71,5 +80,5 @@ public interface AuthManager { * @param whenFinished A {@link Runnable} to execute when a new auth token * is successfully fetched */ - void invalidateAndRefresh(Runnable whenFinished); + void invalidateAndRefresh(AuthCallback whenFinished); } diff --git a/MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerFactory.java b/MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerFactory.java index 8532e807d..1e3532ab5 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerFactory.java +++ b/MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerFactory.java @@ -1,12 +1,12 @@ /* * Copyright 2010 Google Inc. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -24,7 +24,7 @@ import android.util.Log; /** * A factory for getting the platform specific AuthManager. - * + * * @author Sandor Dornbush */ public class AuthManagerFactory { @@ -46,12 +46,10 @@ public class AuthManagerFactory { public static AuthManager getAuthManager(Activity activity, int code, Bundle extras, boolean requireGoogle, String service) { if (useModernAuthManager()) { - Log.i(Constants.TAG, - "Creating modern auth manager: " + service); - return new ModernAuthManager(activity, code, extras, requireGoogle, service); + Log.i(Constants.TAG, "Creating modern auth manager: " + service); + return new ModernAuthManager(activity, service); } else { - Log.i(Constants.TAG, - "Creating legacy auth manager: " + service); + Log.i(Constants.TAG, "Creating legacy auth manager: " + service); return new AuthManagerOld(activity, code, extras, requireGoogle, service); } } diff --git a/MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerOld.java b/MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerOld.java index 1ccaf7330..fe0c1cecb 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerOld.java +++ b/MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerOld.java @@ -22,7 +22,6 @@ import android.app.Activity; import android.content.Intent; import android.os.Bundle; -import java.util.Iterator; import java.util.Vector; /** @@ -48,17 +47,11 @@ public class AuthManagerOld implements AuthManager { private final String service; /** A list of handlers to call when a new auth token is fetched. */ - private final Vector newTokenListeners = new Vector(); + private final Vector newTokenListeners = new Vector(); /** The most recently fetched auth token or null if none is available. */ private String authToken; - /** - * The number of handlers at the beginning of the above list that shouldn't - * be removed after they are called. - */ - private int stickyNewTokenListenerCount; - /** * AuthManager requires many of the same parameters as * {@link GoogleLoginServiceHelper#getCredentials(Activity, int, Bundle, @@ -88,10 +81,8 @@ public class AuthManagerOld implements AuthManager { this.service = service; } - /* (non-Javadoc) - * @see com.google.android.apps.mytracks.io.AuthManager#doLogin(java.lang.Runnable) - */ - public void doLogin(Runnable whenFinished, Object o) { + @Override + public void doLogin(AuthCallback whenFinished, Object o) { synchronized (newTokenListeners) { if (whenFinished != null) { newTokenListeners.add(whenFinished); @@ -111,45 +102,43 @@ public class AuthManagerOld implements AuthManager { } } - /* (non-Javadoc) - * @see com.google.android.apps.mytracks.io.AuthManager#authResult(int, android.content.Intent) - */ - public boolean authResult(int resultCode, Intent results) { - if (resultCode == Activity.RESULT_OK) { - authToken = results.getStringExtra( - GoogleLoginServiceConstants.AUTHTOKEN_KEY); - if (authToken == null) { - GoogleLoginServiceHelper.getCredentials( - activity, code, extras, requireGoogle, service, false); - return true; - } else { - // Notify all active listeners that we have a new auth token. - synchronized (newTokenListeners) { - Iterator iter = newTokenListeners.iterator(); - while (iter.hasNext()) { - iter.next().run(); - } - iter = null; - // Remove anything not in the sticky part of the list. - newTokenListeners.setSize(stickyNewTokenListenerCount); - } - return true; - } + @Override + public void authResult(int resultCode, Intent results) { + if (resultCode != Activity.RESULT_OK) { + notifyListeners(false); + return; + } + + authToken = results.getStringExtra( + GoogleLoginServiceConstants.AUTHTOKEN_KEY); + if (authToken == null) { + // Retry, without prompting the user. + GoogleLoginServiceHelper.getCredentials( + activity, code, extras, requireGoogle, service, false); + } else { + // Notify all active listeners that we have a new auth token. + notifyListeners(true); } - return false; } - /* (non-Javadoc) - * @see com.google.android.apps.mytracks.io.AuthManager#getAuthToken() - */ + private void notifyListeners(boolean success) { + synchronized (newTokenListeners) { + for (AuthCallback callback : newTokenListeners) { + callback.onAuthResult(success); + } + + // Remove anything not in the sticky part of the list. + newTokenListeners.clear(); + } + } + + @Override public String getAuthToken() { return authToken; } - /* (non-Javadoc) - * @see com.google.android.apps.mytracks.io.AuthManager#invalidateAndRefresh(java.lang.Runnable) - */ - public void invalidateAndRefresh(Runnable whenFinished) { + @Override + public void invalidateAndRefresh(AuthCallback whenFinished) { synchronized (newTokenListeners) { if (whenFinished != null) { newTokenListeners.add(whenFinished); @@ -161,38 +150,4 @@ public class AuthManagerOld implements AuthManager { } }); } - - /** - * Adds a {@link Runnable} to be executed every time the auth token is - * updated. The {@link Runnable} will not be removed until manually removed - * with {@link #removeStickyNewTokenListener(Runnable)}. - * - * @param listener The {@link Runnable} to execute every time a new auth - * token is fetched - */ - public void addStickyNewTokenListener(Runnable listener) { - synchronized (newTokenListeners) { - newTokenListeners.add(0, listener); - stickyNewTokenListenerCount++; - } - } - - /** - * Stops executing the given {@link Runnable} every time the auth token is - * updated. This {@link Runnable} must have been added with - * {@link #addStickyNewTokenListener(Runnable)} above. If the - * {@link Runnable} was added more than once, only the first occurrence - * will be removed. - * - * @param listener The {@link Runnable} to stop executing every time a new - * auth token is fetched - */ - public void removeStickyNewTokenListener(Runnable listener) { - synchronized (newTokenListeners) { - if (stickyNewTokenListenerCount > 0 - && newTokenListeners.remove(listener)) { - stickyNewTokenListenerCount--; - } - } - } } diff --git a/MyTracks/src/com/google/android/apps/mytracks/io/ModernAuthManager.java b/MyTracks/src/com/google/android/apps/mytracks/io/ModernAuthManager.java index 6d5c206ec..806ab2dfb 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/io/ModernAuthManager.java +++ b/MyTracks/src/com/google/android/apps/mytracks/io/ModernAuthManager.java @@ -15,6 +15,8 @@ */ package com.google.android.apps.mytracks.io; +import static com.google.android.apps.mytracks.Constants.TAG; + import com.google.android.accounts.Account; import com.google.android.accounts.AccountManager; import com.google.android.accounts.AccountManagerCallback; @@ -49,7 +51,9 @@ public class ModernAuthManager implements AuthManager { private final AccountManager accountManager; - private Runnable whenFinished; + private AuthCallback whenFinished; + + private Account lastAccount; /** * AuthManager requires many of the same parameters as @@ -63,16 +67,9 @@ public class ModernAuthManager implements AuthManager { * {@link Activity#onActivityResult} that calls * {@link #authResult(int, Intent)} when {@literal code} is the request * code - * @param code The request code to pass to - * {@link Activity#onActivityResult} when - * {@link #authResult(int, Intent)} should be called - * @param extras A {@link Bundle} of extras for - * {@link com.google.android.googlelogindist.GoogleLoginServiceHelper} - * @param requireGoogle True if the account must be a Google account * @param service The name of the service to authenticate as */ - public ModernAuthManager(Activity activity, int code, Bundle extras, - boolean requireGoogle, String service) { + public ModernAuthManager(Activity activity, String service) { this.activity = activity; this.service = service; this.accountManager = AccountManager.get(activity); @@ -87,12 +84,19 @@ public class ModernAuthManager implements AuthManager { * has been successfully fetched and is available via * {@link #getAuthToken()} */ - public void doLogin(final Runnable runnable, Object o) { + public void doLogin(AuthCallback runnable, Object o) { this.whenFinished = runnable; if (!(o instanceof Account)) { - throw new IllegalArgumentException("FroyoAuthManager requires an account."); + throw new IllegalArgumentException("ModernAuthManager requires an account."); } Account account = (Account) o; + doLogin(account); + } + + private void doLogin(Account account) { + // Keep the account in case we need to retry. + this.lastAccount = account; + accountManager.getAuthToken(account, service, true, new AccountManagerCallback() { public void run(AccountManagerFuture future) { @@ -109,7 +113,7 @@ public class ModernAuthManager implements AuthManager { authToken = result.getString( AccountManager.KEY_AUTHTOKEN); - Log.e(Constants.TAG, "Got auth token."); + Log.i(Constants.TAG, "Got auth token."); runWhenFinished(); } catch (OperationCanceledException e) { Log.e(Constants.TAG, "Operation Canceled", e); @@ -144,16 +148,23 @@ public class ModernAuthManager implements AuthManager { * the auth token, or False if there was an error or the request was * canceled */ - public boolean authResult(int resultCode, Intent results) { + public void authResult(int resultCode, Intent results) { + boolean retry = false; if (results != null) { - authToken = results.getStringExtra( - AccountManager.KEY_AUTHTOKEN); - Log.w(Constants.TAG, "authResult: " + authToken); + authToken = results.getStringExtra(AccountManager.KEY_AUTHTOKEN); + retry = results.getBooleanExtra("retry", false); + Log.w(Constants.TAG, "authResult: token=" + authToken + "; extras=" + results.getExtras()); } else { - Log.e(Constants.TAG, "No auth result results!!"); + Log.e(Constants.TAG, "No auth token!!"); } + + if (authToken == null && retry) { + Log.i(TAG, "Retrying to get auth result"); + doLogin(lastAccount); + return; + } + runWhenFinished(); - return authToken != null; } /** @@ -175,13 +186,14 @@ public class ModernAuthManager implements AuthManager { * @param runnable A {@link Runnable} to execute when a new auth token * is successfully fetched */ - public void invalidateAndRefresh(final Runnable runnable) { + public void invalidateAndRefresh(final AuthCallback runnable) { this.whenFinished = runnable; activity.runOnUiThread(new Runnable() { public void run() { accountManager.invalidateAuthToken(Constants.ACCOUNT_TYPE, authToken); + authToken = null; AccountChooser accountChooser = new AccountChooser(); accountChooser.chooseAccount(activity, @@ -189,7 +201,7 @@ public class ModernAuthManager implements AuthManager { @Override public void onAccountSelected(Account account) { if (account != null) { - doLogin(whenFinished, account); + doLogin(account); } else { runWhenFinished(); } @@ -200,11 +212,13 @@ public class ModernAuthManager implements AuthManager { } private void runWhenFinished() { + lastAccount = null; + if (whenFinished != null) { (new Thread() { @Override public void run() { - whenFinished.run(); + whenFinished.onAuthResult(authToken != null); } }).start(); } diff --git a/MyTracks/src/com/google/android/apps/mytracks/io/gdata/GDataWrapper.java b/MyTracks/src/com/google/android/apps/mytracks/io/gdata/GDataWrapper.java index e26cbf947..2a6d63f06 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/io/gdata/GDataWrapper.java +++ b/MyTracks/src/com/google/android/apps/mytracks/io/gdata/GDataWrapper.java @@ -1,12 +1,12 @@ /* * Copyright 2008 Google Inc. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -17,6 +17,7 @@ package com.google.android.apps.mytracks.io.gdata; import com.google.android.apps.mytracks.Constants; import com.google.android.apps.mytracks.io.AuthManager; +import com.google.android.apps.mytracks.io.AuthManager.AuthCallback; import android.util.Log; @@ -26,6 +27,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.FutureTask; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; /** @@ -132,7 +134,7 @@ public class GDataWrapper { public static final int ERROR_CLEANED_UP = 7; // An unknown error occurred. public static final int ERROR_UNKNOWN = 100; - + private static final int AUTH_TOKEN_INVALIDATE_REFRESH_NUM_RETRIES = 1; private static final int AUTH_TOKEN_INVALIDATE_REFRESH_TIMEOUT = 5000; @@ -141,7 +143,7 @@ public class GDataWrapper { private C gdataServiceClient; private AuthManager auth; private boolean retryOnAuthFailure; - + public GDataWrapper() { errorType = ERROR_NO_ERROR; errorMessage = null; @@ -179,7 +181,7 @@ public class GDataWrapper { return false; } } - + Log.d(Constants.TAG, "retrying function/query"); } return false; @@ -189,7 +191,7 @@ public class GDataWrapper { * Execute a given function or query. If one is executed, errorType and * errorMessage will contain the result/status of the function/query. */ - private void runOne(final AuthenticatedFunction function, + private void runOne(final AuthenticatedFunction function, final QueryFunction query) { try { if (function != null) { @@ -210,7 +212,7 @@ public class GDataWrapper { errorType = ERROR_AUTH; errorMessage = e.getMessage(); } catch (HttpException e) { - Log.e(Constants.TAG, + Log.e(Constants.TAG, "HttpException, code " + e.getStatusCode() + " message " + e.getMessage(), e); errorMessage = e.getMessage(); if (e.getStatusCode() == 401) { @@ -241,10 +243,10 @@ public class GDataWrapper { } } - /** + /** * Invalidates and refreshes the auth token. Blocks until the refresh has * completed or until we deem the refresh as having timed out. - * + * * @return true If the invalidate/refresh succeeds, false if it fails or * times out. */ @@ -252,19 +254,26 @@ public class GDataWrapper { Log.d(Constants.TAG, "Retrying due to auth failure"); // This FutureTask doesn't do anything -- it exists simply to be // blocked upon using get(). - FutureTask whenFinishedFuture = new FutureTask(new Runnable() { + final FutureTask whenFinishedFuture = new FutureTask(new Runnable() { public void run() {} }, null); - auth.invalidateAndRefresh(whenFinishedFuture); + final AtomicBoolean finalSuccess = new AtomicBoolean(false); + auth.invalidateAndRefresh(new AuthCallback() { + @Override + public void onAuthResult(boolean success) { + finalSuccess.set(success); + whenFinishedFuture.run(); + } + }); try { Log.d(Constants.TAG, "waiting for invalidate"); - whenFinishedFuture.get(AUTH_TOKEN_INVALIDATE_REFRESH_TIMEOUT, + whenFinishedFuture.get(AUTH_TOKEN_INVALIDATE_REFRESH_TIMEOUT, TimeUnit.MILLISECONDS); - Log.d(Constants.TAG, "invalidate finished"); - return true; - + boolean success = finalSuccess.get(); + Log.d(Constants.TAG, "invalidate finished, success = " + success); + return success; } catch (InterruptedException e) { Log.e(Constants.TAG, "Failed to invalidate", e); } catch (ExecutionException e) { @@ -274,7 +283,7 @@ public class GDataWrapper { } finally { whenFinishedFuture.cancel(false); } - + return false; } @@ -289,11 +298,11 @@ public class GDataWrapper { public void setAuthManager(AuthManager auth) { this.auth = auth; } - + public AuthManager getAuthManager() { return auth; } - + public void setRetryOnAuthFailure(boolean retry) { retryOnAuthFailure = retry; } diff --git a/MyTracks/src/com/google/android/apps/mytracks/io/mymaps/MyMapsGDataWrapper.java b/MyTracks/src/com/google/android/apps/mytracks/io/mymaps/MyMapsGDataWrapper.java index 3e5cb9c3f..70518b0b8 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/io/mymaps/MyMapsGDataWrapper.java +++ b/MyTracks/src/com/google/android/apps/mytracks/io/mymaps/MyMapsGDataWrapper.java @@ -2,6 +2,7 @@ package com.google.android.apps.mytracks.io.mymaps; import com.google.android.apps.mytracks.io.AuthManager; +import com.google.android.apps.mytracks.io.AuthManager.AuthCallback; import com.google.android.apps.mytracks.io.gdata.GDataClientFactory; import com.google.android.common.gdata.AndroidXmlParserFactory; import com.google.wireless.gdata.client.GDataClient; @@ -126,11 +127,12 @@ class MyMapsGDataWrapper { } Log.d(MyMapsConstants.TAG, "GData error encountered: " + errorMessage); if (errorType == ERROR_AUTH && auth != null) { - Runnable whenFinished = null; + AuthCallback whenFinished = null; if (retryOnAuthFailure) { retriesPending++; - whenFinished = new Runnable() { - public void run() { + whenFinished = new AuthCallback() { + @Override + public void onAuthResult(boolean success) { retriesPending--; retryOnAuthFailure = false; runQuery(query);