Addressing review suggestions

This commit is contained in:
Rodrigo Damazio
2011-07-15 13:20:20 -03:00
parent 90a5cb7aae
commit 6e41033ed6
6 changed files with 36 additions and 50 deletions
@@ -15,7 +15,7 @@
*/ */
package com.google.android.apps.mytracks.io; package com.google.android.apps.mytracks.io;
import com.google.android.apps.mytracks.Constants; import static com.google.android.apps.mytracks.Constants.TAG;
import android.app.Activity; import android.app.Activity;
import android.os.Build; import android.os.Build;
@@ -46,10 +46,10 @@ public class AuthManagerFactory {
public static AuthManager getAuthManager(Activity activity, int code, public static AuthManager getAuthManager(Activity activity, int code,
Bundle extras, boolean requireGoogle, String service) { Bundle extras, boolean requireGoogle, String service) {
if (useModernAuthManager()) { if (useModernAuthManager()) {
Log.i(Constants.TAG, "Creating modern auth manager: " + service); Log.i(TAG, "Creating modern auth manager: " + service);
return new ModernAuthManager(activity, service); return new ModernAuthManager(activity, service);
} else { } else {
Log.i(Constants.TAG, "Creating legacy auth manager: " + service); Log.i(TAG, "Creating legacy auth manager: " + service);
return new AuthManagerOld(activity, code, extras, requireGoogle, service); return new AuthManagerOld(activity, code, extras, requireGoogle, service);
} }
} }
@@ -22,8 +22,6 @@ import android.app.Activity;
import android.content.Intent; import android.content.Intent;
import android.os.Bundle; import android.os.Bundle;
import java.util.Vector;
/** /**
* AuthManager keeps track of the current auth token for a user. The advantage * AuthManager keeps track of the current auth token for a user. The advantage
* over just passing around a String is that this class can renew the auth * over just passing around a String is that this class can renew the auth
@@ -46,8 +44,8 @@ public class AuthManagerOld implements AuthManager {
/** The name of the service to authorize for. */ /** The name of the service to authorize for. */
private final String service; private final String service;
/** A list of handlers to call when a new auth token is fetched. */ /** The handler to call when a new auth token is fetched. */
private final Vector<AuthCallback> newTokenListeners = new Vector<AuthCallback>(); private AuthCallback authCallback;
/** The most recently fetched auth token or null if none is available. */ /** The most recently fetched auth token or null if none is available. */
private String authToken; private String authToken;
@@ -82,12 +80,8 @@ public class AuthManagerOld implements AuthManager {
} }
@Override @Override
public void doLogin(AuthCallback whenFinished, Object o) { public void doLogin(AuthCallback callback, Object o) {
synchronized (newTokenListeners) { authCallback = callback;
if (whenFinished != null) {
newTokenListeners.add(whenFinished);
}
}
activity.runOnUiThread(new LoginRunnable()); activity.runOnUiThread(new LoginRunnable());
} }
@@ -105,31 +99,24 @@ public class AuthManagerOld implements AuthManager {
@Override @Override
public void authResult(int resultCode, Intent results) { public void authResult(int resultCode, Intent results) {
if (resultCode != Activity.RESULT_OK) { if (resultCode != Activity.RESULT_OK) {
notifyListeners(false); runAuthCallback(false);
return; return;
} }
authToken = results.getStringExtra( authToken = results.getStringExtra(GoogleLoginServiceConstants.AUTHTOKEN_KEY);
GoogleLoginServiceConstants.AUTHTOKEN_KEY);
if (authToken == null) { if (authToken == null) {
// Retry, without prompting the user. // Retry, without prompting the user.
GoogleLoginServiceHelper.getCredentials( GoogleLoginServiceHelper.getCredentials(
activity, code, extras, requireGoogle, service, false); activity, code, extras, requireGoogle, service, false);
} else { } else {
// Notify all active listeners that we have a new auth token. // Notify all active listeners that we have a new auth token.
notifyListeners(true); runAuthCallback(true);
} }
} }
private void notifyListeners(boolean success) { private void runAuthCallback(boolean success) {
synchronized (newTokenListeners) { authCallback.onAuthResult(success);
for (AuthCallback callback : newTokenListeners) { authCallback = null;
callback.onAuthResult(success);
}
// Remove anything not in the sticky part of the list.
newTokenListeners.clear();
}
} }
@Override @Override
@@ -138,12 +125,8 @@ public class AuthManagerOld implements AuthManager {
} }
@Override @Override
public void invalidateAndRefresh(AuthCallback whenFinished) { public void invalidateAndRefresh(AuthCallback callback) {
synchronized (newTokenListeners) { authCallback = callback;
if (whenFinished != null) {
newTokenListeners.add(whenFinished);
}
}
activity.runOnUiThread(new Runnable() { activity.runOnUiThread(new Runnable() {
public void run() { public void run() {
GoogleLoginServiceHelper.invalidateAuthToken(activity, code, authToken); GoogleLoginServiceHelper.invalidateAuthToken(activity, code, authToken);
@@ -51,7 +51,7 @@ public class ModernAuthManager implements AuthManager {
private final AccountManager accountManager; private final AccountManager accountManager;
private AuthCallback whenFinished; private AuthCallback authCallback;
private Account lastAccount; private Account lastAccount;
@@ -85,7 +85,7 @@ public class ModernAuthManager implements AuthManager {
* {@link #getAuthToken()} * {@link #getAuthToken()}
*/ */
public void doLogin(AuthCallback runnable, Object o) { public void doLogin(AuthCallback runnable, Object o) {
this.whenFinished = runnable; this.authCallback = runnable;
if (!(o instanceof Account)) { if (!(o instanceof Account)) {
throw new IllegalArgumentException("ModernAuthManager requires an account."); throw new IllegalArgumentException("ModernAuthManager requires an account.");
} }
@@ -114,7 +114,7 @@ public class ModernAuthManager implements AuthManager {
authToken = result.getString( authToken = result.getString(
AccountManager.KEY_AUTHTOKEN); AccountManager.KEY_AUTHTOKEN);
Log.i(Constants.TAG, "Got auth token."); Log.i(Constants.TAG, "Got auth token.");
runWhenFinished(); runAuthCallback();
} catch (OperationCanceledException e) { } catch (OperationCanceledException e) {
Log.e(Constants.TAG, "Operation Canceled", e); Log.e(Constants.TAG, "Operation Canceled", e);
} catch (IOException e) { } catch (IOException e) {
@@ -150,12 +150,11 @@ public class ModernAuthManager implements AuthManager {
*/ */
public void authResult(int resultCode, Intent results) { public void authResult(int resultCode, Intent results) {
boolean retry = false; boolean retry = false;
if (results != null) { if (results == null) {
Log.e(Constants.TAG, "No auth token!!");
} else {
authToken = results.getStringExtra(AccountManager.KEY_AUTHTOKEN); authToken = results.getStringExtra(AccountManager.KEY_AUTHTOKEN);
retry = results.getBooleanExtra("retry", false); retry = results.getBooleanExtra("retry", false);
Log.w(Constants.TAG, "authResult: token=" + authToken + "; extras=" + results.getExtras());
} else {
Log.e(Constants.TAG, "No auth token!!");
} }
if (authToken == null && retry) { if (authToken == null && retry) {
@@ -164,7 +163,7 @@ public class ModernAuthManager implements AuthManager {
return; return;
} }
runWhenFinished(); runAuthCallback();
} }
/** /**
@@ -186,8 +185,8 @@ public class ModernAuthManager implements AuthManager {
* @param runnable A {@link Runnable} to execute when a new auth token * @param runnable A {@link Runnable} to execute when a new auth token
* is successfully fetched * is successfully fetched
*/ */
public void invalidateAndRefresh(final AuthCallback runnable) { public void invalidateAndRefresh(final AuthCallback callback) {
this.whenFinished = runnable; this.authCallback = callback;
activity.runOnUiThread(new Runnable() { activity.runOnUiThread(new Runnable() {
public void run() { public void run() {
@@ -203,7 +202,7 @@ public class ModernAuthManager implements AuthManager {
if (account != null) { if (account != null) {
doLogin(account); doLogin(account);
} else { } else {
runWhenFinished(); runAuthCallback();
} }
} }
}); });
@@ -211,14 +210,15 @@ public class ModernAuthManager implements AuthManager {
}); });
} }
private void runWhenFinished() { private void runAuthCallback() {
lastAccount = null; lastAccount = null;
if (whenFinished != null) { if (authCallback != null) {
(new Thread() { (new Thread() {
@Override @Override
public void run() { public void run() {
whenFinished.onAuthResult(authToken != null); authCallback.onAuthResult(authToken != null);
authCallback = null;
} }
}).start(); }).start();
} }
@@ -80,6 +80,7 @@ class DatabaseDumper {
* *
* @param cursor the cursor to get columns from * @param cursor the cursor to get columns from
* @param numRows the number of rows that will be later written * @param numRows the number of rows that will be later written
* @param writer the output to write to
* @throws IOException if there are errors while writing * @throws IOException if there are errors while writing
*/ */
public void writeHeaders(Cursor cursor, int numRows, DataOutputStream writer) public void writeHeaders(Cursor cursor, int numRows, DataOutputStream writer)
@@ -93,6 +94,7 @@ class DatabaseDumper {
* This must be called after {@link #writeHeaders}. * This must be called after {@link #writeHeaders}.
* *
* @param cursor the cursor to write data from * @param cursor the cursor to write data from
* @param writer the output to write to
* @throws IOException if there are any errors while writing * @throws IOException if there are any errors while writing
*/ */
public void writeOneRow(Cursor cursor, DataOutputStream writer) public void writeOneRow(Cursor cursor, DataOutputStream writer)
@@ -146,8 +148,8 @@ class DatabaseDumper {
/** /**
* Writes metadata about the query to be dumped. * Writes metadata about the query to be dumped.
* *
* @param cursor the cursor that the data will come from
* @param numRows the number of rows that will be dumped * @param numRows the number of rows that will be dumped
* @param writer the output to write to
* @throws IOException if there are any errors while writing * @throws IOException if there are any errors while writing
*/ */
private void writeQueryMetadata( private void writeQueryMetadata(
@@ -171,6 +173,7 @@ class DatabaseDumper {
* @param columnIdx the column index to read from * @param columnIdx the column index to read from
* @param columnTypeId the type of the column to be read * @param columnTypeId the type of the column to be read
* @param cursor the cursor to read from * @param cursor the cursor to read from
* @param writer the output to write to
* @throws IOException if there are any errors while writing * @throws IOException if there are any errors while writing
*/ */
private void writeCell( private void writeCell(
@@ -174,8 +174,9 @@ public class SendActivity extends Activity implements ProgressIndicator {
restoreInstanceState(savedInstanceState); restoreInstanceState(savedInstanceState);
} }
// If we had the instance restored after it was done, reset it.
if (currentState == SendState.DONE) { if (currentState == SendState.DONE) {
currentState = SendState.SEND_OPTIONS; resetState();
} }
// Only consider the intent if we're not restoring from a previous state. // Only consider the intent if we're not restoring from a previous state.
@@ -998,8 +998,7 @@ public class TrackRecordingService extends Service {
long lastRecordedLocationId = long lastRecordedLocationId =
providerUtils.getLastLocationId(recordingTrackId); providerUtils.getLastLocationId(recordingTrackId);
ContentValues values = new ContentValues(); ContentValues values = new ContentValues();
if (lastRecordedLocationId >= 0 if (lastRecordedLocationId >= 0 && recordedTrack.getStopId() >= 0) {
&& recordedTrack.getStopId() >= 0) {
values.put(TracksColumns.STOPID, lastRecordedLocationId); values.put(TracksColumns.STOPID, lastRecordedLocationId);
} }
values.put(TracksColumns.STOPTIME, stats.getStopTime()); values.put(TracksColumns.STOPTIME, stats.getStopTime());