From e4aa00d8a780b709670d41940dfde97ae05bcc8c Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 28 Mar 2018 16:56:52 +0200 Subject: [PATCH] Make OpenPgpApiManager more testable, and fix related unit tests --- .../com/fsck/k9/activity/MessageCompose.java | 7 ++- .../activity/compose/RecipientPresenter.java | 10 ++-- .../compose/RecipientPresenterTest.java | 55 ++++++++++--------- .../k9/message/PgpMessageBuilderTest.java | 24 ++------ .../openpgp/OpenPgpApiManager.java | 23 ++++++-- 5 files changed, 62 insertions(+), 57 deletions(-) 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 9f4c31e58..cb26c25b9 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java +++ b/k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java @@ -99,6 +99,7 @@ import com.fsck.k9.search.LocalSearch; import com.fsck.k9.ui.EolConvertingEditText; import com.fsck.k9.ui.compose.QuotedMessageMvpView; import com.fsck.k9.ui.compose.QuotedMessagePresenter; +import org.openintents.openpgp.OpenPgpApiManager; import org.openintents.openpgp.util.OpenPgpApi; import timber.log.Timber; @@ -282,9 +283,11 @@ public class MessageCompose extends K9Activity implements OnClickListener, RecipientMvpView recipientMvpView = new RecipientMvpView(this); ComposePgpInlineDecider composePgpInlineDecider = new ComposePgpInlineDecider(); ComposePgpEnableByDefaultDecider composePgpEnableByDefaultDecider = new ComposePgpEnableByDefaultDecider(); - recipientPresenter = new RecipientPresenter(getApplicationContext(), getLifecycle(), getLoaderManager(), + OpenPgpApiManager openPgpApiManager = new OpenPgpApiManager(getApplicationContext(), getLifecycle()); + recipientPresenter = new RecipientPresenter(getApplicationContext(), getLoaderManager(), openPgpApiManager, recipientMvpView, account, composePgpInlineDecider, composePgpEnableByDefaultDecider, - AutocryptStatusInteractor.getInstance(), new ReplyToParser(), this); + AutocryptStatusInteractor.getInstance(), new ReplyToParser(), this + ); recipientPresenter.asyncUpdateCryptoStatus(); 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 e1d64f2b5..c320fb175 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 @@ -9,7 +9,6 @@ import java.util.List; import android.app.Activity; import android.app.LoaderManager; import android.app.PendingIntent; -import android.arch.lifecycle.Lifecycle; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; @@ -88,8 +87,9 @@ public class RecipientPresenter { private boolean cryptoEnablePgpInline = false; - public RecipientPresenter(Context context, Lifecycle lifecycle, LoaderManager loaderManager, - RecipientMvpView recipientMvpView, Account account, ComposePgpInlineDecider composePgpInlineDecider, + public RecipientPresenter(Context context, LoaderManager loaderManager, + OpenPgpApiManager openPgpApiManager, RecipientMvpView recipientMvpView, Account account, + ComposePgpInlineDecider composePgpInlineDecider, ComposePgpEnableByDefaultDecider composePgpEnableByDefaultDecider, AutocryptStatusInteractor autocryptStatusInteractor, ReplyToParser replyToParser, RecipientsChangedListener recipientsChangedListener) { @@ -100,7 +100,7 @@ public class RecipientPresenter { this.composePgpEnableByDefaultDecider = composePgpEnableByDefaultDecider; this.replyToParser = replyToParser; this.listener = recipientsChangedListener; - this.openPgpApiManager = new OpenPgpApiManager(context, lifecycle, openPgpCallback, account.getOpenPgpProvider()); + this.openPgpApiManager = openPgpApiManager; recipientMvpView.setPresenter(this); recipientMvpView.setLoaderManager(loaderManager); @@ -308,7 +308,7 @@ public class RecipientPresenter { String openPgpProvider = account.getOpenPgpProvider(); recipientMvpView.setCryptoProvider(openPgpProvider); - // openPgpApiManager.setOpenPgpProvider(openPgpProvider); + openPgpApiManager.setOpenPgpProvider(openPgpProvider, openPgpCallback); } @SuppressWarnings("UnusedParameters") 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 f545fdb91..c5293518b 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 @@ -6,11 +6,8 @@ import java.util.List; import android.app.LoaderManager; import android.content.Context; -import android.content.Intent; -import android.os.ParcelFileDescriptor; import com.fsck.k9.Account; -import com.fsck.k9.K9; import com.fsck.k9.K9RobolectricTest; import com.fsck.k9.activity.compose.RecipientMvpView.CryptoSpecialModeDisplayType; import com.fsck.k9.activity.compose.RecipientMvpView.CryptoStatusDisplayType; @@ -28,9 +25,11 @@ import com.fsck.k9.message.ComposePgpInlineDecider; import com.fsck.k9.view.RecipientSelectView.Recipient; import org.junit.Before; import org.junit.Test; -import org.openintents.openpgp.IOpenPgpService2; +import org.mockito.ArgumentCaptor; +import org.openintents.openpgp.OpenPgpApiManager; +import org.openintents.openpgp.OpenPgpApiManager.OpenPgpApiManagerCallback; +import org.openintents.openpgp.OpenPgpApiManager.OpenPgpProviderState; import org.openintents.openpgp.util.OpenPgpApi; -import org.openintents.openpgp.util.OpenPgpServiceConnection; import org.openintents.openpgp.util.ShadowOpenPgpAsyncTask; import org.robolectric.Robolectric; import org.robolectric.annotation.Config; @@ -42,6 +41,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -66,6 +66,8 @@ public class RecipientPresenterTest extends K9RobolectricTest { private RecipientPresenter.RecipientsChangedListener listener; private AutocryptStatusInteractor autocryptStatusInteractor; private RecipientAutocryptStatus noRecipientsAutocryptResult; + private OpenPgpApiManager openPgpApiManager; + private OpenPgpApiManagerCallback openPgpApiManagerCallback; @Before @@ -74,6 +76,7 @@ public class RecipientPresenterTest extends K9RobolectricTest { Robolectric.getBackgroundThreadScheduler().pause(); recipientMvpView = mock(RecipientMvpView.class); + openPgpApiManager = mock(OpenPgpApiManager.class); account = mock(Account.class); composePgpInlineDecider = mock(ComposePgpInlineDecider.class); composePgpEnableByDefaultDecider = mock(ComposePgpEnableByDefaultDecider.class); @@ -82,10 +85,16 @@ public class RecipientPresenterTest extends K9RobolectricTest { LoaderManager loaderManager = mock(LoaderManager.class); listener = mock(RecipientPresenter.RecipientsChangedListener.class); + when(openPgpApiManager.getOpenPgpProviderState()).thenReturn(OpenPgpProviderState.UNCONFIGURED); + recipientPresenter = new RecipientPresenter( - context, getLifecycle(), loaderManager, recipientMvpView, account, composePgpInlineDecider, - composePgpEnableByDefaultDecider, autocryptStatusInteractor, replyToParser, listener); - runBackgroundTask(); + context, loaderManager, openPgpApiManager, recipientMvpView, account, composePgpInlineDecider, + composePgpEnableByDefaultDecider, autocryptStatusInteractor, replyToParser, listener + ); + + ArgumentCaptor callbackCaptor = ArgumentCaptor.forClass(OpenPgpApiManagerCallback.class); + verify(openPgpApiManager).setOpenPgpProvider(isNull(String.class), callbackCaptor.capture()); + openPgpApiManagerCallback = callbackCaptor.getValue(); noRecipientsAutocryptResult = new RecipientAutocryptStatus(RecipientAutocryptStatusType.NO_RECIPIENTS, null); } @@ -129,6 +138,9 @@ public class RecipientPresenterTest extends K9RobolectricTest { @Test public void getCurrentCryptoStatus_withoutCryptoProvider() throws Exception { + when(openPgpApiManager.getOpenPgpProviderState()).thenReturn(OpenPgpProviderState.UNCONFIGURED); + recipientPresenter.asyncUpdateCryptoStatus(); + ComposeCryptoStatus status = recipientPresenter.getCurrentCachedCryptoStatus(); assertEquals(CryptoStatusDisplayType.UNCONFIGURED, status.getCryptoStatusDisplayType()); @@ -319,28 +331,21 @@ public class RecipientPresenterTest extends K9RobolectricTest { assertTrue(taskRun); } - private void setupCryptoProvider(RecipientAutocryptStatus autocryptStatusResult) throws android.os.RemoteException { + private void setupCryptoProvider(RecipientAutocryptStatus autocryptStatusResult) throws Exception { Account account = mock(Account.class); - OpenPgpServiceConnection openPgpServiceConnection = mock(OpenPgpServiceConnection.class); - IOpenPgpService2 openPgpService2 = mock(IOpenPgpService2.class); - Intent permissionPingIntent = new Intent(); + OpenPgpApi openPgpApi = mock(OpenPgpApi.class); + when(account.getOpenPgpProvider()).thenReturn(CRYPTO_PROVIDER); + when(account.isOpenPgpProviderConfigured()).thenReturn(true); + when(account.getOpenPgpKey()).thenReturn(CRYPTO_KEY_ID); + recipientPresenter.onSwitchAccount(account); + + when(openPgpApiManager.getOpenPgpProviderState()).thenReturn(OpenPgpProviderState.OK); + when(openPgpApiManager.getOpenPgpApi()).thenReturn(openPgpApi); when(autocryptStatusInteractor.retrieveCryptoProviderRecipientStatus( any(OpenPgpApi.class), any(String[].class))).thenReturn(autocryptStatusResult); - permissionPingIntent.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_SUCCESS); - when(account.getOpenPgpKey()).thenReturn(CRYPTO_KEY_ID); - when(account.isOpenPgpProviderConfigured()).thenReturn(true); - when(account.getOpenPgpProvider()).thenReturn(CRYPTO_PROVIDER); - when(openPgpServiceConnection.isBound()).thenReturn(true); - when(openPgpServiceConnection.getService()).thenReturn(openPgpService2); - when(openPgpService2.execute(any(Intent.class), any(ParcelFileDescriptor.class), any(Integer.class))) - .thenReturn(permissionPingIntent); - - recipientPresenter.setOpenPgpServiceConnection(openPgpServiceConnection, CRYPTO_PROVIDER); - recipientPresenter.onSwitchAccount(account); - // one for the permission ping, one for the async status update - runBackgroundTask(); + openPgpApiManagerCallback.onOpenPgpProviderStatusChanged(); runBackgroundTask(); } } diff --git a/k9mail/src/test/java/com/fsck/k9/message/PgpMessageBuilderTest.java b/k9mail/src/test/java/com/fsck/k9/message/PgpMessageBuilderTest.java index 90cdc2e0d..c123bcad0 100644 --- a/k9mail/src/test/java/com/fsck/k9/message/PgpMessageBuilderTest.java +++ b/k9mail/src/test/java/com/fsck/k9/message/PgpMessageBuilderTest.java @@ -21,7 +21,6 @@ import com.fsck.k9.K9RobolectricTest; import com.fsck.k9.activity.compose.ComposeCryptoStatus; import com.fsck.k9.activity.compose.ComposeCryptoStatus.ComposeCryptoStatusBuilder; import com.fsck.k9.activity.compose.RecipientPresenter.CryptoMode; -import com.fsck.k9.activity.compose.RecipientPresenter.CryptoProviderState; import com.fsck.k9.activity.misc.Attachment; import com.fsck.k9.autocrypt.AutocryptOpenPgpApiInteractor; import com.fsck.k9.autocrypt.AutocryptOperations; @@ -45,6 +44,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.openintents.openpgp.OpenPgpApiManager.OpenPgpProviderState; import org.openintents.openpgp.OpenPgpError; import org.openintents.openpgp.util.OpenPgpApi; import org.openintents.openpgp.util.OpenPgpApi.OpenPgpDataSource; @@ -86,7 +86,7 @@ public class PgpMessageBuilderTest extends K9RobolectricTest { public void build__withCryptoProviderUnconfigured__shouldThrow() throws MessagingException { cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); - cryptoStatusBuilder.setOpenPgpProviderState(CryptoProviderState.UNCONFIGURED); + cryptoStatusBuilder.setOpenPgpProviderState(OpenPgpProviderState.UNCONFIGURED); pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); Callback mockCallback = mock(Callback.class); @@ -100,7 +100,7 @@ public class PgpMessageBuilderTest extends K9RobolectricTest { public void build__withCryptoProviderUninitialized__shouldThrow() throws MessagingException { cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); - cryptoStatusBuilder.setOpenPgpProviderState(CryptoProviderState.UNINITIALIZED); + cryptoStatusBuilder.setOpenPgpProviderState(OpenPgpProviderState.UNINITIALIZED); pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); Callback mockCallback = mock(Callback.class); @@ -114,21 +114,7 @@ public class PgpMessageBuilderTest extends K9RobolectricTest { public void build__withCryptoProviderError__shouldThrow() throws MessagingException { cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); - cryptoStatusBuilder.setOpenPgpProviderState(CryptoProviderState.ERROR); - pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); - - Callback mockCallback = mock(Callback.class); - pgpMessageBuilder.buildAsync(mockCallback); - - verify(mockCallback).onMessageBuildException(any(MessagingException.class)); - verifyNoMoreInteractions(mockCallback); - } - - @Test - public void build__withCryptoProviderLostConnection__shouldThrow() throws MessagingException { - cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); - - cryptoStatusBuilder.setOpenPgpProviderState(CryptoProviderState.LOST_CONNECTION); + cryptoStatusBuilder.setOpenPgpProviderState(OpenPgpProviderState.ERROR); pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); Callback mockCallback = mock(Callback.class); @@ -607,7 +593,7 @@ public class PgpMessageBuilderTest extends K9RobolectricTest { .setPreferEncryptMutual(false) .setOpenPgpKeyId(TEST_KEY_ID) .setRecipients(new ArrayList()) - .setOpenPgpProviderState(CryptoProviderState.OK); + .setOpenPgpProviderState(OpenPgpProviderState.OK); } private static PgpMessageBuilder createDefaultPgpMessageBuilder(OpenPgpApi openPgpApi, diff --git a/plugins/openpgp-api-lib/openpgp-api/src/main/java/org/openintents/openpgp/OpenPgpApiManager.java b/plugins/openpgp-api-lib/openpgp-api/src/main/java/org/openintents/openpgp/OpenPgpApiManager.java index 3d14b4fb9..d095487f4 100644 --- a/plugins/openpgp-api-lib/openpgp-api/src/main/java/org/openintents/openpgp/OpenPgpApiManager.java +++ b/plugins/openpgp-api-lib/openpgp-api/src/main/java/org/openintents/openpgp/OpenPgpApiManager.java @@ -20,19 +20,19 @@ import timber.log.Timber; public class OpenPgpApiManager implements LifecycleObserver { private final Context context; - private final String openPgpProvider; + + @Nullable + private String openPgpProvider; + @Nullable + private OpenPgpApiManagerCallback callback; private OpenPgpServiceConnection openPgpServiceConnection; private OpenPgpApi openPgpApi; private PendingIntent userInteractionPendingIntent; - private OpenPgpApiManagerCallback callback; private OpenPgpProviderState openPgpProviderState = OpenPgpProviderState.UNCONFIGURED; - public OpenPgpApiManager(Context context, Lifecycle lifecycle, - OpenPgpApiManagerCallback callback, String openPgpProvider) { + public OpenPgpApiManager(Context context, Lifecycle lifecycle) { this.context = context; - this.callback = callback; - this.openPgpProvider = openPgpProvider; lifecycle.addObserver(this); } @@ -52,6 +52,17 @@ public class OpenPgpApiManager implements LifecycleObserver { disconnect(); } + public void setOpenPgpProvider(@Nullable String openPgpProvider, OpenPgpApiManagerCallback callback) { + if (openPgpProvider == null || !openPgpProvider.equals(this.openPgpProvider)) { + disconnect(); + } + + this.openPgpProvider = openPgpProvider; + this.callback = callback; + + setupCryptoProvider(); + } + private void setupCryptoProvider() { if (TextUtils.isEmpty(openPgpProvider)) { setOpenPgpProviderState(OpenPgpProviderState.UNCONFIGURED);