load status from provider asynchronously

k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java

@@ -279,7 +279,7 @@ public class MessageCompose extends K9Activity implements OnClickListener,
         ComposePgpInlineDecider composePgpInlineDecider = new ComposePgpInlineDecider();
         recipientPresenter = new RecipientPresenter(getApplicationContext(), getLoaderManager(), recipientMvpView,
                 account, composePgpInlineDecider, new ReplyToParser(), this);
-        recipientPresenter.updateCryptoStatus();
+        recipientPresenter.asyncUpdateCryptoStatus();
 
 
         subjectView = (EditText) findViewById(R.id.subject);
@@ -630,8 +630,11 @@ public class MessageCompose extends K9Activity implements OnClickListener,
     private MessageBuilder createMessageBuilder(boolean isDraft) {
         MessageBuilder builder;
 
-        recipientPresenter.updateCryptoStatus();
+        ComposeCryptoStatus cryptoStatus = recipientPresenter.getCurrentCachedCryptoStatus();
-        ComposeCryptoStatus cryptoStatus = recipientPresenter.getCurrentCryptoStatus();
+        if (cryptoStatus == null) {
+            return null;
+        }
+
         // TODO encrypt drafts for storage
         if (!isDraft && cryptoStatus.shouldUsePgpMessageBuilder()) {
             SendErrorState maybeSendErrorState = cryptoStatus.getSendErrorStateOrNull();
@@ -641,14 +644,13 @@ public class MessageCompose extends K9Activity implements OnClickListener,
             }
 
             PgpMessageBuilder pgpBuilder = PgpMessageBuilder.newInstance();
-            recipientPresenter.builderSetProperties(pgpBuilder);
+            recipientPresenter.builderSetProperties(pgpBuilder, cryptoStatus);
             builder = pgpBuilder;
         } else {
             builder = SimpleMessageBuilder.newInstance();
+            recipientPresenter.builderSetProperties(builder);
         }
 
-        recipientPresenter.builderSetProperties(builder);
-
         builder.setSubject(Utility.stripNewLines(subjectView.getText().toString()))
                 .setSentDate(new Date())
                 .setHideTimeZone(K9.hideTimeZone())
@@ -1484,8 +1486,7 @@ public class MessageCompose extends K9Activity implements OnClickListener,
                 message.setUid(relatedMessageReference.getUid());
             }
 
-            // TODO more appropriate logic here? not sure
+            boolean saveRemotely = recipientPresenter.shouldSaveRemotely();
-            boolean saveRemotely = !recipientPresenter.getCurrentCryptoStatus().shouldUsePgpMessageBuilder();
             new SaveMessageTask(getApplicationContext(), account, contacts, internalMessageHandler,
                     message, draftId, saveRemotely).execute();
             if (finishAfterDraftSaved) {

k9mail/src/main/java/com/fsck/k9/activity/compose/AttachmentPresenter.java

@@ -116,8 +116,12 @@ public class AttachmentPresenter {
     }
 
     public void onClickAddAttachment(RecipientPresenter recipientPresenter) {
-        AttachErrorState maybeAttachErrorState =
+        ComposeCryptoStatus currentCachedCryptoStatus = recipientPresenter.getCurrentCachedCryptoStatus();
-                recipientPresenter.getCurrentCryptoStatus().getAttachErrorStateOrNull();
+        if (currentCachedCryptoStatus == null) {
+            return;
+        }
+
+        AttachErrorState maybeAttachErrorState = currentCachedCryptoStatus.getAttachErrorStateOrNull();
         if (maybeAttachErrorState != null) {
             recipientPresenter.showPgpAttachError(maybeAttachErrorState);
             return;

k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java

@@ -14,7 +14,9 @@ import android.content.Intent;
 import android.content.pm.PackageManager;
 import android.content.pm.ResolveInfo;
 import android.net.Uri;
+import android.os.AsyncTask;
 import android.os.Bundle;
+import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
 import timber.log.Timber;
@@ -73,8 +75,9 @@ public class RecipientPresenter implements PermissionPingCallback {
     private Boolean hasContactPicker;
     private PendingIntent pendingUserInteractionIntent;
     private CryptoProviderState cryptoProviderState = CryptoProviderState.UNCONFIGURED;
-    private ComposeCryptoStatus cachedCryptoStatus;
     private OpenPgpServiceConnection openPgpServiceConnection;
+    @Nullable
+    private ComposeCryptoStatus cachedCryptoStatus;
 
 
     // persistent state, saved during onSaveInstanceState
@@ -254,7 +257,7 @@ public class RecipientPresenter implements PermissionPingCallback {
         menu.findItem(R.id.openpgp_inline_disable).setVisible(isCryptoConfigured && cryptoEnablePgpInline);
 
         boolean showSignOnly = isCryptoConfigured && K9.getOpenPgpSupportSignOnly();
-        boolean isSignOnly = cachedCryptoStatus.isSignOnly();
+        boolean isSignOnly = currentCryptoMode == CryptoMode.SIGN_ONLY;
         menu.findItem(R.id.openpgp_sign_only).setVisible(showSignOnly && !isSignOnly);
         menu.findItem(R.id.openpgp_sign_only_disable).setVisible(showSignOnly && isSignOnly);
 
@@ -344,7 +347,7 @@ public class RecipientPresenter implements PermissionPingCallback {
         recipientMvpView.setRecipientExpanderVisibility(notBothAreVisible);
     }
 
-    public void updateCryptoStatus() {
+    public void asyncUpdateCryptoStatus() {
         cachedCryptoStatus = null;
 
         boolean isOkStateButLostConnection = cryptoProviderState == CryptoProviderState.OK &&
@@ -354,37 +357,49 @@ public class RecipientPresenter implements PermissionPingCallback {
             pendingUserInteractionIntent = null;
         }
 
-        Long accountCryptoKey = account.getCryptoKey();
+        new AsyncTask<Void,Void,ComposeCryptoStatus>() {
-        if (accountCryptoKey == Account.NO_OPENPGP_KEY) {
+            @Override
-            accountCryptoKey = null;
+            protected ComposeCryptoStatus doInBackground(Void... voids) {
-        }
+                Long accountCryptoKey = account.getCryptoKey();
+                if (accountCryptoKey == Account.NO_OPENPGP_KEY) {
+                    accountCryptoKey = null;
+                }
 
-        ComposeCryptoStatus composeCryptoStatus = new ComposeCryptoStatusBuilder()
+                ComposeCryptoStatus composeCryptoStatus = new ComposeCryptoStatusBuilder()
-                .setCryptoProviderState(cryptoProviderState)
+                        .setCryptoProviderState(cryptoProviderState)
-                .setCryptoMode(currentCryptoMode)
+                        .setCryptoMode(currentCryptoMode)
-                .setEnablePgpInline(cryptoEnablePgpInline)
+                        .setEnablePgpInline(cryptoEnablePgpInline)
-                .setRecipients(getAllRecipients())
+                        .setRecipients(getAllRecipients())
-                .setSigningKeyId(accountCryptoKey)
+                        .setSigningKeyId(accountCryptoKey)
-                .setSelfEncryptId(accountCryptoKey)
+                        .setSelfEncryptId(accountCryptoKey)
-                .build();
+                        .build();
 
-        if (composeCryptoStatus.isCryptoStatusRecipientDependent()) {
+                if (composeCryptoStatus.isCryptoStatusRecipientDependent()) {
-            PgpMessageBuilder pgpMessageBuilder = PgpMessageBuilder.newInstance();
+                    PgpMessageBuilder pgpMessageBuilder = PgpMessageBuilder.newInstance();
-            builderSetProperties(pgpMessageBuilder);
+                    builderSetProperties(pgpMessageBuilder, composeCryptoStatus);
-            pgpMessageBuilder.setCryptoStatus(composeCryptoStatus);
 
-            CryptoProviderDryRunStatus cryptoProviderDryRunStatus =
+                    // this calls out to the crypto provider, hence the need to do this in a background thread
-                    pgpMessageBuilder.retrieveCryptoProviderRecipientStatus();
+                    CryptoProviderDryRunStatus cryptoProviderDryRunStatus =
+                            pgpMessageBuilder.retrieveCryptoProviderRecipientStatus();
 
-            composeCryptoStatus = composeCryptoStatus.withCryptoProviderRecipientStatus(cryptoProviderDryRunStatus);
+                    composeCryptoStatus = composeCryptoStatus.withCryptoProviderRecipientStatus(
-        }
+                            cryptoProviderDryRunStatus);
+                }
 
-        cachedCryptoStatus = composeCryptoStatus;
+                return composeCryptoStatus;
-        recipientMvpView.showCryptoStatus(composeCryptoStatus.getCryptoStatusDisplayType());
+            }
-        recipientMvpView.showCryptoSpecialMode(composeCryptoStatus.getCryptoSpecialModeDisplayType());
+
+            @Override
+            protected void onPostExecute(ComposeCryptoStatus composeCryptoStatus) {
+                cachedCryptoStatus = composeCryptoStatus;
+                recipientMvpView.showCryptoStatus(composeCryptoStatus.getCryptoStatusDisplayType());
+                recipientMvpView.showCryptoSpecialMode(composeCryptoStatus.getCryptoSpecialModeDisplayType());
+            }
+        }.execute();
     }
 
-    public ComposeCryptoStatus getCurrentCryptoStatus() {
+    @Nullable
+    public ComposeCryptoStatus getCurrentCachedCryptoStatus() {
         return cachedCryptoStatus;
     }
 
@@ -393,58 +408,58 @@ public class RecipientPresenter implements PermissionPingCallback {
     }
 
     void onToTokenAdded() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     void onToTokenRemoved() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     void onToTokenChanged() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     void onCcTokenAdded() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     void onCcTokenRemoved() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     void onCcTokenChanged() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     void onBccTokenAdded() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     void onBccTokenRemoved() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     void onBccTokenChanged() {
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
         listener.onRecipientsChanged();
     }
 
     public void onCryptoModeChanged(CryptoMode cryptoMode) {
         currentCryptoMode = cryptoMode;
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
     }
 
     public void onCryptoPgpInlineChanged(boolean enablePgpInline) {
         cryptoEnablePgpInline = enablePgpInline;
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
     }
 
     private void addRecipientsFromAddresses(final RecipientType recipientType, final Address... addresses) {
@@ -557,7 +572,7 @@ public class RecipientPresenter implements PermissionPingCallback {
                 Timber.e("click on crypto status while unconfigured - this should not really happen?!");
                 return;
             case OK:
-                if (cachedCryptoStatus.isSignOnly()) {
+                if (currentCryptoMode == CryptoMode.SIGN_ONLY) {
                     recipientMvpView.showErrorIsSignOnly();
                 } else {
                     recipientMvpView.showCryptoDialog(currentCryptoMode);
@@ -679,7 +694,7 @@ public class RecipientPresenter implements PermissionPingCallback {
         recipientMvpView.showErrorOpenPgpConnection();
         cryptoProviderState = CryptoProviderState.ERROR;
         Timber.e(e, "error connecting to crypto provider!");
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
     }
 
     @Override
@@ -702,7 +717,7 @@ public class RecipientPresenter implements PermissionPingCallback {
                 cryptoProviderState = CryptoProviderState.ERROR;
                 break;
         }
-        updateCryptoStatus();
+        asyncUpdateCryptoStatus();
     }
 
     public void onActivityDestroy() {
@@ -719,17 +734,23 @@ public class RecipientPresenter implements PermissionPingCallback {
         return new OpenPgpApi(context, openPgpServiceConnection.getService());
     }
 
-
     public void builderSetProperties(MessageBuilder messageBuilder) {
+        if (messageBuilder instanceof PgpMessageBuilder) {
+            throw new IllegalArgumentException("PpgMessageBuilder must be called with ComposeCryptoStatus argument!");
+        }
+
         messageBuilder.setTo(getToAddresses());
         messageBuilder.setCc(getCcAddresses());
         messageBuilder.setBcc(getBccAddresses());
+    }
 
-        if (messageBuilder instanceof PgpMessageBuilder) {
+    public void builderSetProperties(PgpMessageBuilder pgpMessageBuilder, ComposeCryptoStatus cryptoStatus) {
-            PgpMessageBuilder pgpMessageBuilder = (PgpMessageBuilder) messageBuilder;
+        pgpMessageBuilder.setTo(getToAddresses());
-            pgpMessageBuilder.setOpenPgpApi(getOpenPgpApi());
+        pgpMessageBuilder.setCc(getCcAddresses());
-            pgpMessageBuilder.setCryptoStatus(getCurrentCryptoStatus());
+        pgpMessageBuilder.setBcc(getBccAddresses());
-        }
+
+        pgpMessageBuilder.setOpenPgpApi(getOpenPgpApi());
+        pgpMessageBuilder.setCryptoStatus(cryptoStatus);
     }
 
     public void onMenuSetPgpInline(boolean enablePgpInline) {
@@ -795,6 +816,11 @@ public class RecipientPresenter implements PermissionPingCallback {
         this.openPgpProvider = cryptoProvider;
     }
 
+    public boolean shouldSaveRemotely() {
+        // TODO more appropriate logic?
+        return cryptoProviderState == CryptoProviderState.UNCONFIGURED || currentCryptoMode == CryptoMode.DISABLE;
+    }
+
     public enum CryptoProviderState {
         UNCONFIGURED,
         UNINITIALIZED,

k9mail/src/main/java/com/fsck/k9/message/PgpMessageBuilder.java

@@ -11,6 +11,7 @@ import android.content.Intent;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
+import android.support.annotation.WorkerThread;
 
 import com.fsck.k9.Globals;
 import com.fsck.k9.activity.compose.ComposeCryptoStatus;
@@ -327,6 +328,7 @@ public class PgpMessageBuilder extends MessageBuilder {
         this.cryptoStatus = cryptoStatus;
     }
 
+    @WorkerThread
     public CryptoProviderDryRunStatus retrieveCryptoProviderRecipientStatus() {
         boolean shouldSign = cryptoStatus.isSigningEnabled();
         boolean shouldEncrypt = cryptoStatus.isEncryptionEnabled();

k9mail/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java

@@ -45,6 +45,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 
+@SuppressWarnings("ConstantConditions")
 @RunWith(K9RobolectricTestRunner.class)
 @Config(shadows = {ShadowOpenPgpAsyncTask.class})
 public class RecipientPresenterTest {
@@ -77,7 +78,7 @@ public class RecipientPresenterTest {
 
         recipientPresenter = new RecipientPresenter(
                 context, loaderManager, recipientMvpView, account, composePgpInlineDecider, replyToParser, listener);
-        recipientPresenter.updateCryptoStatus();
+        recipientPresenter.asyncUpdateCryptoStatus();
 
         noUserIdsResultIntent = new Intent();
         noUserIdsResultIntent.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR);
@@ -120,7 +121,7 @@ public class RecipientPresenterTest {
 
     @Test
     public void getCurrentCryptoStatus_withoutCryptoProvider() throws Exception {
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.UNCONFIGURED, status.getCryptoStatusDisplayType());
         assertEquals(CryptoSpecialModeDisplayType.NONE, status.getCryptoSpecialModeDisplayType());
@@ -133,7 +134,8 @@ public class RecipientPresenterTest {
     public void getCurrentCryptoStatus_withCryptoProvider() throws Exception {
         setupCryptoProvider(noUserIdsResultIntent);
 
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.OPPORTUNISTIC_EMPTY, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -145,7 +147,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(noUserIdsResultIntent);
 
         recipientPresenter.onCryptoModeChanged(CryptoMode.OPPORTUNISTIC);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.OPPORTUNISTIC_EMPTY, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -160,7 +163,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(resultIntent);
 
         recipientPresenter.onCryptoModeChanged(CryptoMode.OPPORTUNISTIC);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.OPPORTUNISTIC_UNTRUSTED, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -176,7 +180,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(resultIntent);
 
         recipientPresenter.onCryptoModeChanged(CryptoMode.OPPORTUNISTIC);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.OPPORTUNISTIC_NOKEY, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -192,7 +197,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(resultIntent);
 
         recipientPresenter.onCryptoModeChanged(CryptoMode.PRIVATE);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.PRIVATE_NOKEY, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -207,7 +213,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(resultIntent);
 
         recipientPresenter.onCryptoModeChanged(CryptoMode.OPPORTUNISTIC);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.OPPORTUNISTIC_TRUSTED, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -219,7 +226,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(noUserIdsResultIntent);
 
         recipientPresenter.onCryptoModeChanged(CryptoMode.DISABLE);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.DISABLED, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -231,7 +239,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(noUserIdsResultIntent);
 
         recipientPresenter.onCryptoModeChanged(CryptoMode.PRIVATE);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.PRIVATE_EMPTY, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -243,7 +252,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(noUserIdsResultIntent);
 
         recipientPresenter.onMenuSetSignOnly(true);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.SIGN_ONLY, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -256,7 +266,8 @@ public class RecipientPresenterTest {
         setupCryptoProvider(noUserIdsResultIntent);
 
         recipientPresenter.onMenuSetPgpInline(true);
-        ComposeCryptoStatus status = recipientPresenter.getCurrentCryptoStatus();
+        Robolectric.getBackgroundThreadScheduler().runOneTask();
+        ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus();
 
         assertEquals(CryptoStatusDisplayType.OPPORTUNISTIC_EMPTY, status.getCryptoStatusDisplayType());
         assertTrue(status.isProviderStateOk());
@@ -334,7 +345,7 @@ public class RecipientPresenterTest {
         Robolectric.getBackgroundThreadScheduler().pause();
         recipientPresenter.setOpenPgpServiceConnection(openPgpServiceConnection, CRYPTO_PROVIDER);
         recipientPresenter.onSwitchAccount(account);
-        recipientPresenter.updateCryptoStatus();
+        recipientPresenter.asyncUpdateCryptoStatus();
         Robolectric.getBackgroundThreadScheduler().runOneTask();
     }
 }