diff --git a/k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java b/k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java index ab895ca2c..84544f4bb 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java +++ b/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.getCurrentCryptoStatus(); + ComposeCryptoStatus cryptoStatus = recipientPresenter.getCurrentCachedCryptoStatus(); + 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.getCurrentCryptoStatus().shouldUsePgpMessageBuilder(); + boolean saveRemotely = recipientPresenter.shouldSaveRemotely(); new SaveMessageTask(getApplicationContext(), account, contacts, internalMessageHandler, message, draftId, saveRemotely).execute(); if (finishAfterDraftSaved) { diff --git a/k9mail/src/main/java/com/fsck/k9/activity/compose/AttachmentPresenter.java b/k9mail/src/main/java/com/fsck/k9/activity/compose/AttachmentPresenter.java index 44db23a5a..46e1d4fd4 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/compose/AttachmentPresenter.java +++ b/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 = - recipientPresenter.getCurrentCryptoStatus().getAttachErrorStateOrNull(); + ComposeCryptoStatus currentCachedCryptoStatus = recipientPresenter.getCurrentCachedCryptoStatus(); + if (currentCachedCryptoStatus == null) { + return; + } + + AttachErrorState maybeAttachErrorState = currentCachedCryptoStatus.getAttachErrorStateOrNull(); if (maybeAttachErrorState != null) { recipientPresenter.showPgpAttachError(maybeAttachErrorState); return; diff --git a/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java b/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java index 408f70144..48a44b45a 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java +++ b/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(); - if (accountCryptoKey == Account.NO_OPENPGP_KEY) { - accountCryptoKey = null; - } + new AsyncTask() { + @Override + protected ComposeCryptoStatus doInBackground(Void... voids) { + Long accountCryptoKey = account.getCryptoKey(); + if (accountCryptoKey == Account.NO_OPENPGP_KEY) { + accountCryptoKey = null; + } - ComposeCryptoStatus composeCryptoStatus = new ComposeCryptoStatusBuilder() - .setCryptoProviderState(cryptoProviderState) - .setCryptoMode(currentCryptoMode) - .setEnablePgpInline(cryptoEnablePgpInline) - .setRecipients(getAllRecipients()) - .setSigningKeyId(accountCryptoKey) - .setSelfEncryptId(accountCryptoKey) - .build(); + ComposeCryptoStatus composeCryptoStatus = new ComposeCryptoStatusBuilder() + .setCryptoProviderState(cryptoProviderState) + .setCryptoMode(currentCryptoMode) + .setEnablePgpInline(cryptoEnablePgpInline) + .setRecipients(getAllRecipients()) + .setSigningKeyId(accountCryptoKey) + .setSelfEncryptId(accountCryptoKey) + .build(); - if (composeCryptoStatus.isCryptoStatusRecipientDependent()) { - PgpMessageBuilder pgpMessageBuilder = PgpMessageBuilder.newInstance(); - builderSetProperties(pgpMessageBuilder); - pgpMessageBuilder.setCryptoStatus(composeCryptoStatus); + if (composeCryptoStatus.isCryptoStatusRecipientDependent()) { + PgpMessageBuilder pgpMessageBuilder = PgpMessageBuilder.newInstance(); + builderSetProperties(pgpMessageBuilder, composeCryptoStatus); - CryptoProviderDryRunStatus cryptoProviderDryRunStatus = - pgpMessageBuilder.retrieveCryptoProviderRecipientStatus(); + // this calls out to the crypto provider, hence the need to do this in a background thread + CryptoProviderDryRunStatus cryptoProviderDryRunStatus = + pgpMessageBuilder.retrieveCryptoProviderRecipientStatus(); - composeCryptoStatus = composeCryptoStatus.withCryptoProviderRecipientStatus(cryptoProviderDryRunStatus); - } + composeCryptoStatus = composeCryptoStatus.withCryptoProviderRecipientStatus( + cryptoProviderDryRunStatus); + } - cachedCryptoStatus = composeCryptoStatus; - recipientMvpView.showCryptoStatus(composeCryptoStatus.getCryptoStatusDisplayType()); - recipientMvpView.showCryptoSpecialMode(composeCryptoStatus.getCryptoSpecialModeDisplayType()); + return composeCryptoStatus; + } + + @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) { - PgpMessageBuilder pgpMessageBuilder = (PgpMessageBuilder) messageBuilder; - pgpMessageBuilder.setOpenPgpApi(getOpenPgpApi()); - pgpMessageBuilder.setCryptoStatus(getCurrentCryptoStatus()); - } + public void builderSetProperties(PgpMessageBuilder pgpMessageBuilder, ComposeCryptoStatus cryptoStatus) { + pgpMessageBuilder.setTo(getToAddresses()); + pgpMessageBuilder.setCc(getCcAddresses()); + 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, diff --git a/k9mail/src/main/java/com/fsck/k9/message/PgpMessageBuilder.java b/k9mail/src/main/java/com/fsck/k9/message/PgpMessageBuilder.java index 649895fa5..0d8aac1c7 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/PgpMessageBuilder.java +++ b/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(); diff --git a/k9mail/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java b/k9mail/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java index f431daef3..5b51e8e2d 100644 --- a/k9mail/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java +++ b/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(); } }