From 6b80bec3c9a3a963dc79752fec251b889274c5f2 Mon Sep 17 00:00:00 2001 From: Mikel Date: Fri, 18 Aug 2017 17:06:16 +0100 Subject: [PATCH 1/4] Simplify `GetAuthCookie` task --- .../commons/auth/AuthenticatedActivity.java | 53 +++++-------------- 1 file changed, 14 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java index e89c5c528..8b12aa306 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java @@ -12,6 +12,9 @@ import java.io.IOException; import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.theme.NavigationBaseActivity; +import io.reactivex.Single; +import io.reactivex.android.schedulers.AndroidSchedulers; +import io.reactivex.schedulers.Schedulers; import timber.log.Timber; public abstract class AuthenticatedActivity extends NavigationBaseActivity { @@ -24,41 +27,15 @@ public abstract class AuthenticatedActivity extends NavigationBaseActivity { public AuthenticatedActivity() { this.accountType = AccountUtil.accountType(); } - - private class GetAuthCookieTask extends AsyncTask { - private Account account; - private AccountManager accountManager; - public GetAuthCookieTask(Account account, AccountManager accountManager) { - this.account = account; - this.accountManager = accountManager; - } - - @Override - protected void onPostExecute(String result) { - super.onPostExecute(result); - if(result != null) { - authCookie = result; - onAuthCookieAcquired(result); - } else { - onAuthFailure(); - } - } - @Override - protected String doInBackground(Void... params) { - try { - return accountManager.blockingGetAuthToken(account, "", false); - } catch (OperationCanceledException e) { - e.printStackTrace(); - return null; - } catch (AuthenticatorException e) { - e.printStackTrace(); - return null; - } catch (IOException e) { - e.printStackTrace(); - return null; - } - } + private void getAuthCookie(Account account, AccountManager accountManager) { + Single.fromCallable(() -> accountManager.blockingGetAuthToken(account, "", false)) + .subscribeOn(Schedulers.io()) + .doOnError(Timber::e) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe( + cookie -> onAuthCookieAcquired(cookie), + throwable -> onAuthFailure()); } private class AddAccountTask extends AsyncTask { @@ -71,10 +48,9 @@ public abstract class AuthenticatedActivity extends NavigationBaseActivity { protected void onPostExecute(String result) { super.onPostExecute(result); if(result != null) { - Account[] allAccounts =accountManager.getAccountsByType(accountType); + Account[] allAccounts = accountManager.getAccountsByType(accountType); Account curAccount = allAccounts[0]; - GetAuthCookieTask getCookieTask = new GetAuthCookieTask(curAccount, accountManager); - getCookieTask.execute(); + getAuthCookie(curAccount, accountManager); } else { onAuthFailure(); } @@ -124,8 +100,7 @@ public abstract class AuthenticatedActivity extends NavigationBaseActivity { // See: https://groups.google.com/forum/?fromgroups=#!topic/android-developers/8M0RTFfO7-M addAccountTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); } else { - GetAuthCookieTask task = new GetAuthCookieTask(curAccount, accountManager); - task.execute(); + getAuthCookie(curAccount, accountManager); } } From f2e8891db9ba3000b69fe1fee6fa82de57b25059 Mon Sep 17 00:00:00 2001 From: Mikel Date: Fri, 18 Aug 2017 17:35:24 +0100 Subject: [PATCH 2/4] Simplify `AddAccount` task --- .../commons/auth/AuthenticatedActivity.java | 75 +++++-------------- 1 file changed, 19 insertions(+), 56 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java index 8b12aa306..60a967139 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java @@ -3,13 +3,8 @@ package fr.free.nrw.commons.auth; import android.accounts.Account; import android.accounts.AccountManager; import android.accounts.AccountManagerFuture; -import android.accounts.AuthenticatorException; -import android.accounts.OperationCanceledException; -import android.os.AsyncTask; import android.os.Bundle; -import java.io.IOException; - import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.theme.NavigationBaseActivity; import io.reactivex.Single; @@ -37,50 +32,25 @@ public abstract class AuthenticatedActivity extends NavigationBaseActivity { cookie -> onAuthCookieAcquired(cookie), throwable -> onAuthFailure()); } - - private class AddAccountTask extends AsyncTask { - private AccountManager accountManager; - public AddAccountTask(AccountManager accountManager) { - this.accountManager = accountManager; - } - - @Override - protected void onPostExecute(String result) { - super.onPostExecute(result); - if(result != null) { - Account[] allAccounts = accountManager.getAccountsByType(accountType); - Account curAccount = allAccounts[0]; - getAuthCookie(curAccount, accountManager); - } else { - onAuthFailure(); - } - } - @Override - protected String doInBackground(Void... params) { - AccountManagerFuture resultFuture = accountManager.addAccount(accountType, null, null, null, AuthenticatedActivity.this, null, null); - Bundle result; - try { - result = resultFuture.getResult(); - } catch (OperationCanceledException e) { - e.printStackTrace(); - return null; - } catch (AuthenticatorException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - return null; - } catch (IOException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - return null; - } - if(result.containsKey(AccountManager.KEY_ACCOUNT_NAME)) { - return result.getString(AccountManager.KEY_ACCOUNT_NAME); - } else { - return null; - } - - } + private void addAccount(AccountManager accountManager) { + Single.just(accountManager.addAccount(accountType, null, null, null, AuthenticatedActivity.this, null, null)) + .subscribeOn(Schedulers.io()) + .map(AccountManagerFuture::getResult) + .doOnEvent((bundle, throwable) -> { + if (bundle.containsKey(AccountManager.KEY_ACCOUNT_NAME)) { + throw new RuntimeException(); + } + }) + .map(bundle -> bundle.getString(AccountManager.KEY_ACCOUNT_NAME)) + .doOnError(Timber::e) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(s -> { + Account[] allAccounts = accountManager.getAccountsByType(accountType); + Account curAccount = allAccounts[0]; + getAuthCookie(curAccount, accountManager); + }, + throwable -> onAuthFailure()); } protected void requestAuthToken() { @@ -91,14 +61,7 @@ public abstract class AuthenticatedActivity extends NavigationBaseActivity { AccountManager accountManager = AccountManager.get(this); Account curAccount = app.getCurrentAccount(); if(curAccount == null) { - AddAccountTask addAccountTask = new AddAccountTask(accountManager); - // This AsyncTask blocks until the Login Activity returns - // And since in Android 4.x+ only one background thread runs all AsyncTasks - // And since LoginActivity can't return until it's own AsyncTask (that does the login) - // returns, we have a deadlock! - // Fixed by explicitly asking this to be executed in parallel - // See: https://groups.google.com/forum/?fromgroups=#!topic/android-developers/8M0RTFfO7-M - addAccountTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); + addAccount(accountManager); } else { getAuthCookie(curAccount, accountManager); } From 591b10f4c9e1e4371db3d737f60fe2da1889ca9a Mon Sep 17 00:00:00 2001 From: Mikel Date: Fri, 18 Aug 2017 20:04:59 +0100 Subject: [PATCH 3/4] Add error message to exception --- .../java/fr/free/nrw/commons/auth/AuthenticatedActivity.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java index 60a967139..ad60574bc 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java @@ -39,7 +39,8 @@ public abstract class AuthenticatedActivity extends NavigationBaseActivity { .map(AccountManagerFuture::getResult) .doOnEvent((bundle, throwable) -> { if (bundle.containsKey(AccountManager.KEY_ACCOUNT_NAME)) { - throw new RuntimeException(); + throw new RuntimeException("Bundle doesn't contain account-name key: " + + AccountManager.KEY_ACCOUNT_NAME); } }) .map(bundle -> bundle.getString(AccountManager.KEY_ACCOUNT_NAME)) From 5cb1f7281be56dde8001c60e30ba60a31beca0ce Mon Sep 17 00:00:00 2001 From: Mikel Date: Fri, 18 Aug 2017 20:06:18 +0100 Subject: [PATCH 4/4] Fix wrong condition --- .../java/fr/free/nrw/commons/auth/AuthenticatedActivity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java index ad60574bc..0db56e66b 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java @@ -38,7 +38,7 @@ public abstract class AuthenticatedActivity extends NavigationBaseActivity { .subscribeOn(Schedulers.io()) .map(AccountManagerFuture::getResult) .doOnEvent((bundle, throwable) -> { - if (bundle.containsKey(AccountManager.KEY_ACCOUNT_NAME)) { + if (!bundle.containsKey(AccountManager.KEY_ACCOUNT_NAME)) { throw new RuntimeException("Bundle doesn't contain account-name key: " + AccountManager.KEY_ACCOUNT_NAME); }