diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index b529272367c4..5c724c3b9a5b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -11,6 +11,7 @@ import org.greenrobot.eventbus.Subscribe import org.greenrobot.eventbus.ThreadMode import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.generated.SiteActionBuilder +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.fluxc.store.SiteStore.OnSiteChanged @@ -22,6 +23,8 @@ import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.Ur import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.util.AppLog import org.wordpress.android.util.UrlUtils +import com.automattic.android.tracks.crashlogging.CrashLogging +import org.wordpress.android.util.crashlogging.sendReportWithTag import javax.inject.Inject import javax.inject.Named @@ -34,6 +37,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( private val siteStore: SiteStore, private val appPrefsWrapper: AppPrefsWrapper, private val appLogWrapper: AppLogWrapper, + private val crashLogging: CrashLogging, ) : ViewModel() { private val _onFinishedEvent = MutableSharedFlow() /** @@ -62,16 +66,12 @@ class ApplicationPasswordLoginViewModel @Inject constructor( fun setupSite(rawData: String) { viewModelScope.launch { if (rawData.isEmpty()) { - appLogWrapper.e(AppLog.T.MAIN, "Cannot store credentials: rawData is empty") - _onFinishedEvent.emit( - NavigationActionData( - showSiteSelector = false, - showPostSignupInterstitial = false, - siteUrl = "", - oldSitesIDs = oldSitesIDs, - isError = true - ) + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: Cannot store credentials: rawData is empty" ) + applicationPasswordLoginHelper.trackStoringFailed("", "empty_raw_data") + emitError(siteUrl = "", errorMessage = "empty_raw_data") return@launch } val urlLogin = applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData) @@ -95,7 +95,14 @@ class ApplicationPasswordLoginViewModel @Inject constructor( try { applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(urlLogin) } catch (e: Exception) { - appLogWrapper.e(AppLog.T.DB, "Error storing credentials: ${e.stackTraceToString()}") + appLogWrapper.e( + AppLog.T.DB, + "A_P: Error storing credentials: ${e.stackTraceToString()}" + ) + applicationPasswordLoginHelper.trackStoringFailed( + urlLogin.siteUrl, "store_credentials_exception" + ) + crashLogging.sendReportWithTag(e, AppLog.T.DB) false } } @@ -109,10 +116,21 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) = withContext(ioDispatcher) { try { if (username.isEmpty() || password.isEmpty() || siteUrl.isEmpty() || apiRootUrl.isEmpty()) { - appLogWrapper.e(AppLog.T.MAIN, "Cannot fetch sites for credential storing: " + - "Username: $username, Password: ${password.isEmpty()}, SiteUrl: $siteUrl, " + - "API Root URL: $apiRootUrl") - emitErrorFetching(siteUrl) + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: Cannot fetch sites for credential storing" + + " - username isEmpty=${username.isEmpty()}" + + ", password isEmpty=${password.isEmpty()}" + + ", siteUrl isEmpty=${siteUrl.isEmpty()}" + + ", apiRootUrl isEmpty=${apiRootUrl.isEmpty()}" + ) + applicationPasswordLoginHelper.trackStoringFailed( + siteUrl, "empty_fetch_params" + ) + emitError( + siteUrl = siteUrl, + errorMessage = "empty_fetch_params" + ) } else { val xmlRpcEndpoint = selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(siteUrl) @@ -128,61 +146,161 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) } } catch (e: Exception) { - appLogWrapper.e(AppLog.T.API, "Error fetching sites: ${e.stackTraceToString()}") - emitErrorFetching(siteUrl) + appLogWrapper.e( + AppLog.T.API, + "A_P: Error fetching sites: ${e.stackTraceToString()}" + ) + applicationPasswordLoginHelper.trackStoringFailed( + siteUrl, "fetch_sites_exception" + ) + emitError(siteUrl = siteUrl, errorMessage = e.message, cause = e) } } - private suspend fun emitErrorFetching(siteUrl: String) = _onFinishedEvent.emit( - NavigationActionData( - showSiteSelector = false, - showPostSignupInterstitial = false, - siteUrl = siteUrl, - oldSitesIDs = oldSitesIDs, - isError = true + private suspend fun emitError( + siteUrl: String, + errorMessage: String? = null, + cause: Throwable? = null + ) { + val exception = cause + ?: Exception("Application password login failed: $errorMessage") + crashLogging.sendReportWithTag(exception, AppLog.T.MAIN) + _onFinishedEvent.emit( + NavigationActionData( + showSiteSelector = false, + showPostSignupInterstitial = false, + siteUrl = siteUrl, + oldSitesIDs = oldSitesIDs, + isError = true, + errorMessage = errorMessage + ) ) - ) + } @SuppressWarnings("unused") @Subscribe(threadMode = ThreadMode.BACKGROUND) fun onSiteChanged(event: OnSiteChanged) { viewModelScope.launch { - val currentNormalizedUrl = UrlUtils.normalizeUrl(currentUrlLogin?.siteUrl) - val site = siteStore.sites.firstOrNull { UrlUtils.normalizeUrl(it.url) == currentNormalizedUrl } - if (event.rowsAffected < 1 || site == null || applicationPasswordLoginHelper.siteHasBadCredentials(site)) { - appLogWrapper.e(AppLog.T.MAIN, "Site not found or credentials are empty.") - _onFinishedEvent.emit( - NavigationActionData( - showSiteSelector = false, - showPostSignupInterstitial = false, - siteUrl = currentUrlLogin?.siteUrl, - oldSitesIDs = oldSitesIDs, - isError = true - ) - ) + if (event.isError) { + handleSiteChangedError(event) } else { - _onFinishedEvent.emit( - NavigationActionData( - showSiteSelector = siteStore.hasSite() && - oldSitesIDs?.contains(site.id) != true, // null or false - showPostSignupInterstitial = !siteStore.hasSite() - && appPrefsWrapper.shouldShowPostSignupInterstitial, - siteUrl = currentUrlLogin?.siteUrl, - oldSitesIDs = oldSitesIDs, - isError = false, - newSiteLocalId = site.id - ) - ) + handleSiteChangedSuccess(event) + } + } + } + + private suspend fun handleSiteChangedError(event: OnSiteChanged) { + val error = event.error + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: onSiteChanged failed: " + + "SiteStore error ${error?.type}: ${error?.message}" + ) + applicationPasswordLoginHelper.trackStoringFailed( + currentUrlLogin?.siteUrl, + "site_changed_failed" + ) + emitError( + siteUrl = currentUrlLogin?.siteUrl.orEmpty(), + errorMessage = "site_store_error" + ) + } + + @Suppress("TooGenericExceptionCaught") + private suspend fun handleSiteChangedSuccess(event: OnSiteChanged) { + val normalizedUrl = + UrlUtils.normalizeUrl(currentUrlLogin?.siteUrl) + + val site = try { + siteStore.sites.firstOrNull { + UrlUtils.normalizeUrl(it.url) == normalizedUrl } + } catch (e: Exception) { + logAndEmitSiteChangedError( + logMessage = "exception reading sites from DB: " + + e.stackTraceToString(), + errorCode = "db_read_exception", + cause = e + ) + return + } + + val validationError = validateSiteChanged(event, site) + if (validationError != null) { + logAndEmitSiteChangedError( + logMessage = validationError.logMessage, + errorCode = validationError.errorCode + ) + } else { + val resolvedSite = site ?: return + _onFinishedEvent.emit( + NavigationActionData( + showSiteSelector = siteStore.hasSite() && + oldSitesIDs?.contains(resolvedSite.id) != true, + showPostSignupInterstitial = !siteStore.hasSite() + && appPrefsWrapper.shouldShowPostSignupInterstitial, + siteUrl = currentUrlLogin?.siteUrl, + oldSitesIDs = oldSitesIDs, + isError = false, + newSiteLocalId = resolvedSite.id + ) + ) } } + private fun validateSiteChanged( + event: OnSiteChanged, + site: SiteModel? + ): SiteChangedValidationError? = when { + event.rowsAffected < 1 -> SiteChangedValidationError( + logMessage = "No rows affected " + + "(rowsAffected=${event.rowsAffected})", + errorCode = "no_rows_affected" + ) + site == null -> SiteChangedValidationError( + logMessage = "Site not found after update", + errorCode = "site_not_found" + ) + applicationPasswordLoginHelper + .siteHasBadCredentials(site) -> SiteChangedValidationError( + logMessage = "Credentials are empty after store", + errorCode = "empty_credentials" + ) + else -> null + } + + private data class SiteChangedValidationError( + val logMessage: String, + val errorCode: String + ) + + private suspend fun logAndEmitSiteChangedError( + logMessage: String, + errorCode: String, + cause: Throwable? = null + ) { + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: onSiteChanged failed: $logMessage" + ) + applicationPasswordLoginHelper.trackStoringFailed( + currentUrlLogin?.siteUrl, + "site_changed_failed" + ) + emitError( + siteUrl = currentUrlLogin?.siteUrl.orEmpty(), + errorMessage = errorCode, + cause = cause + ) + } + data class NavigationActionData( val showSiteSelector: Boolean, val showPostSignupInterstitial: Boolean, val siteUrl: String?, val oldSitesIDs: ArrayList?, val isError: Boolean, - val newSiteLocalId: Int? = null + val newSiteLocalId: Int? = null, + val errorMessage: String? = null ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt index 1b9f4d39680d..d48b57889a49 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt @@ -1,6 +1,7 @@ package org.wordpress.android.ui.accounts.login import androidx.core.net.toUri +import com.automattic.android.tracks.crashlogging.CrashLogging import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext import org.wordpress.android.analytics.AnalyticsTracker @@ -14,6 +15,7 @@ import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.util.AppLog import org.wordpress.android.util.BuildConfigWrapper import org.wordpress.android.util.UrlUtils +import org.wordpress.android.util.crashlogging.sendReportWithTag import rs.wordpress.api.kotlin.ApiDiscoveryResult import rs.wordpress.api.kotlin.WpLoginClient import uniffi.wp_api.applicationPasswordsUrl @@ -22,6 +24,7 @@ import javax.inject.Named private const val URL_TAG = "url" private const val SUCCESS_TAG = "success" +private const val REASON_TAG = "reason" class ApplicationPasswordLoginHelper @Inject constructor( @param:Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, @@ -32,7 +35,8 @@ class ApplicationPasswordLoginHelper @Inject constructor( private val wpLoginClient: WpLoginClient, private val appLogWrapper: AppLogWrapper, private val apiRootUrlCache: ApiRootUrlCache, - private val discoverSuccessWrapper: DiscoverSuccessWrapper + private val discoverSuccessWrapper: DiscoverSuccessWrapper, + private val crashLogging: CrashLogging ) { private var processedAppPasswordData: String? = null @@ -82,17 +86,16 @@ class ApplicationPasswordLoginHelper @Inject constructor( } @Suppress("ComplexCondition") - suspend fun storeApplicationPasswordCredentialsFrom(urlLogin: UriLogin): Boolean { + suspend fun storeApplicationPasswordCredentialsFrom( + urlLogin: UriLogin + ): Boolean { if (urlLogin.apiRootUrl == null || urlLogin.user.isNullOrEmpty() || urlLogin.password.isNullOrEmpty() || urlLogin.siteUrl == null || urlLogin.siteUrl == processedAppPasswordData - ) { - appLogWrapper.e( - AppLog.T.DB, - "A_P: Cannot save application password credentials for: ${urlLogin.siteUrl} - bad data" - ) + ) { + logAndReportBadData(urlLogin) return false } @@ -114,15 +117,67 @@ class ApplicationPasswordLoginHelper @Inject constructor( processedAppPasswordData = urlLogin.siteUrl // Save locally to avoid duplicated calls true } else { - appLogWrapper.e( - AppLog.T.DB, - "A_P: Cannot save application password credentials for: ${urlLogin.siteUrl} - null site" + logAndReportSiteNotFound( + urlLogin.siteUrl, normalizedUrl ) false } } } + fun trackStoringFailed(siteUrl: String?, reason: String) { + val properties: MutableMap = HashMap() + properties[URL_TAG] = siteUrl + properties[REASON_TAG] = reason + AnalyticsTracker.track( + Stat.APPLICATION_PASSWORD_STORING_FAILED, + properties + ) + } + + private fun reportStoringFailedToSentry( + reason: String, + detail: String + ) { + crashLogging.sendReportWithTag( + Exception("A_P: $reason — $detail"), + AppLog.T.DB + ) + } + + private fun logAndReportBadData(urlLogin: UriLogin) { + val detail = + "apiRootUrl isNull=${urlLogin.apiRootUrl == null}" + + ", user isEmpty=${urlLogin.user.isNullOrEmpty()}" + + ", password isEmpty=" + + "${urlLogin.password.isNullOrEmpty()}" + + ", siteUrl isNull=${urlLogin.siteUrl == null}" + + ", alreadyProcessed=" + + "${urlLogin.siteUrl == processedAppPasswordData}" + appLogWrapper.e( + AppLog.T.DB, + "A_P: Cannot save credentials" + + " for: ${urlLogin.siteUrl} - $detail" + ) + trackStoringFailed(urlLogin.siteUrl, "bad_data") + reportStoringFailedToSentry("bad_data", detail) + } + + private fun logAndReportSiteNotFound( + siteUrl: String?, + normalizedUrl: String? + ) { + val detail = "$siteUrl (normalized: $normalizedUrl)" + + " — ${siteStore.sites.size} sites available" + appLogWrapper.e( + AppLog.T.DB, + "A_P: Cannot save credentials" + + " - site not found: $detail" + ) + trackStoringFailed(siteUrl, "site_not_found") + reportStoringFailedToSentry("site_not_found", detail) + } + private fun trackSuccessful(siteUrl: String) { val properties: MutableMap = HashMap() properties[URL_TAG] = siteUrl diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt index 39689966d8f9..36afdc5fe87e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt @@ -89,12 +89,12 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor( } else -> { - appLogWrapper.e(AppLog.T.API, "Error creating application password") + logCreationError(site.url, "response type: ${response::class.simpleName}") fallbackToManualLogin(site.url) } } } catch (e: Exception) { - appLogWrapper.e(AppLog.T.API, "Exception creating application password: ${e.message}") + logCreationError(site.url, e.message.orEmpty()) fallbackToManualLogin(site.url) } finally { _isLoading.value = false @@ -102,6 +102,13 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor( } } + private fun logCreationError(siteUrl: String, detail: String) { + appLogWrapper.e( + AppLog.T.API, + "A_P: Error creating application password for: $siteUrl - $detail" + ) + } + @Suppress("TooGenericExceptionCaught") private fun enableApplicationPasswordIfNecessary() { if (!experimentalFeatures.isEnabled(Feature.EXPERIMENTAL_APPLICATION_PASSWORD_FEATURE)) { @@ -115,7 +122,11 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor( val authUrl = applicationPasswordLoginHelper.getAuthorizationUrlComplete(siteUrl) _navigationEvent.emit(NavigationEvent.FallbackToManualLogin(authUrl)) } catch (e: Exception) { - appLogWrapper.e(AppLog.T.API, "Failed to get authorization URL: ${e.message}") + appLogWrapper.e( + AppLog.T.API, + "A_P: Failed to get authorization URL" + + " for: $siteUrl - ${e.message}" + ) _navigationEvent.emit(NavigationEvent.Error) } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt index 2636207cd207..a4a249e5ccee 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt @@ -1,26 +1,28 @@ package org.wordpress.android.ui.accounts.applicationpassword import app.cash.turbine.test +import com.automattic.android.tracks.crashlogging.CrashLogging import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test import org.mockito.Mock import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any +import org.mockito.kotlin.eq import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest -import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper -import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder -import org.wordpress.android.fluxc.store.SiteStore -import org.mockito.kotlin.any -import org.mockito.kotlin.eq import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder +import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper import org.wordpress.android.ui.prefs.AppPrefsWrapper import kotlin.test.assertEquals +import kotlin.test.assertTrue @ExperimentalCoroutinesApi @Suppress("MaxLineLength") @@ -43,6 +45,9 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { @Mock lateinit var appLogWrapper: AppLogWrapper + @Mock + lateinit var crashLogging: CrashLogging + private lateinit var viewModel: ApplicationPasswordLoginViewModel private val rawData = "url=callback?site_url=https://example.com&user_login=user&password=pass" @@ -63,7 +68,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { selfHostedEndpointFinder, siteStore, appPrefsWrapper, - appLogWrapper + appLogWrapper, + crashLogging ) whenever(applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData)).thenReturn(urlLogin) } @@ -77,7 +83,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showPostSignupInterstitial = false, siteUrl = "", oldSitesIDs = null, - isError = true + isError = true, + errorMessage = "empty_raw_data" ) // When @@ -104,7 +111,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showPostSignupInterstitial = false, siteUrl = "", oldSitesIDs = null, - isError = true + isError = true, + errorMessage = "empty_fetch_params" ) whenever(applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(malformedRawData)) .thenReturn( @@ -132,7 +140,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showPostSignupInterstitial = false, siteUrl = urlLogin.siteUrl, oldSitesIDs = null, - isError = true + isError = true, + errorMessage = null ) whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))).thenReturn(false) whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(any())).thenThrow(RuntimeException()) @@ -154,31 +163,31 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { runTest { // Given val xmlRpcEndpoint = "https://example.com/xmlrpc.php" - val expectedResult = ApplicationPasswordLoginViewModel.NavigationActionData( - showSiteSelector = false, - showPostSignupInterstitial = false, - siteUrl = urlLogin.siteUrl, - oldSitesIDs = null, - isError = true - ) - whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))).thenReturn(false) - whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)) - .thenReturn(xmlRpcEndpoint) + whenever( + applicationPasswordLoginHelper + .storeApplicationPasswordCredentialsFrom(eq(urlLogin)) + ).thenReturn(false) + whenever( + selfHostedEndpointFinder + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!) + ).thenReturn(xmlRpcEndpoint) // When viewModel.onFinishedEvent.test { viewModel.setupSite(rawData) // Mock onSiteChanged event viewModel.onSiteChanged( - SiteStore.OnSiteChanged( - rowsAffected = 1, - ) + SiteStore.OnSiteChanged(rowsAffected = 1) ) // Then val finishedEvent = awaitItem() - assertEquals(expectedResult, finishedEvent) - verify(selfHostedEndpointFinder, times(1)).verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) + assertTrue(finishedEvent.isError) + assertEquals( + "site_not_found", finishedEvent.errorMessage + ) + verify(selfHostedEndpointFinder, times(1)) + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) verify(siteStore, times(1)).sites cancelAndIgnoreRemainingEvents() } @@ -275,9 +284,14 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { ) whenever(siteStore.hasSite()).thenReturn(true) whenever(siteStore.sites).thenReturn(listOf(testSite)) - whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))).thenReturn(false) - whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)) - .thenReturn(xmlRpcEndpoint) + whenever( + applicationPasswordLoginHelper + .storeApplicationPasswordCredentialsFrom(eq(urlLogin)) + ).thenReturn(false) + whenever( + selfHostedEndpointFinder + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!) + ).thenReturn(xmlRpcEndpoint) // When viewModel.onFinishedEvent.test { @@ -293,34 +307,69 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { // Then val finishedEvent = awaitItem() assertEquals(expectedResult, finishedEvent) - verify(selfHostedEndpointFinder, times(1)).verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) + verify(selfHostedEndpointFinder, times(1)) + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) cancelAndIgnoreRemainingEvents() } } @Test - fun `given intent rawData, when setup site and not able to store credentials but store fetch, then emit ok with no interstitial by preferences`() = + fun `given onSiteChanged with error, then emit error with SiteStore details`() = runTest { // Given - val xmlRpcEndpoint = "https://example.com/xmlrpc.php" - val expectedResult = ApplicationPasswordLoginViewModel.NavigationActionData( - showSiteSelector = false, - showPostSignupInterstitial = false, - siteUrl = urlLogin.siteUrl, - oldSitesIDs = null, - isError = false, - newSiteLocalId = testSite.id + setupFetchSitesFlow() + val siteError = SiteStore.SiteError( + SiteStore.SiteErrorType.GENERIC_ERROR, "encryption failed" ) + val errorEvent = SiteStore.OnSiteChanged(0, siteError) + + // When + viewModel.onFinishedEvent.test { + viewModel.setupSite(rawData) + viewModel.onSiteChanged(errorEvent) + + // Then + val result = awaitItem() + assertTrue(result.isError) + assertEquals("site_store_error", result.errorMessage) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `given onSiteChanged with no rows affected, then emit error`() = + runTest { + // Given + setupFetchSitesFlow() + + // When + viewModel.onFinishedEvent.test { + viewModel.setupSite(rawData) + viewModel.onSiteChanged( + SiteStore.OnSiteChanged(rowsAffected = 0) + ) + + // Then + val result = awaitItem() + assertTrue(result.isError) + assertEquals("no_rows_affected", result.errorMessage) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `given onSiteChanged with bad credentials, then emit error`() = + runTest { + // Given + setupFetchSitesFlow() whenever(siteStore.sites).thenReturn(listOf(testSite)) - whenever(appPrefsWrapper.shouldShowPostSignupInterstitial).thenReturn(false) - whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))).thenReturn(false) - whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)) - .thenReturn(xmlRpcEndpoint) + whenever( + applicationPasswordLoginHelper.siteHasBadCredentials(any()) + ).thenReturn(true) // When viewModel.onFinishedEvent.test { viewModel.setupSite(rawData) - // Mock onSiteChanged event viewModel.onSiteChanged( SiteStore.OnSiteChanged( rowsAffected = 1, @@ -329,10 +378,62 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { ) // Then - val finishedEvent = awaitItem() - assertEquals(expectedResult, finishedEvent) - verify(selfHostedEndpointFinder, times(1)).verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) + val result = awaitItem() + assertTrue(result.isError) + assertEquals("empty_credentials", result.errorMessage) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `given onSiteChanged with DB exception, then emit error`() = + runTest { + // Given + setupFetchSitesFlow() + whenever(siteStore.sites) + .thenThrow(RuntimeException("DB corrupted")) + + // When + viewModel.onFinishedEvent.test { + viewModel.setupSite(rawData) + viewModel.onSiteChanged( + SiteStore.OnSiteChanged(rowsAffected = 1) + ) + + // Then + val result = awaitItem() + assertTrue(result.isError) + assertEquals("db_read_exception", result.errorMessage) cancelAndIgnoreRemainingEvents() } } + + @Test + fun `given error emitted, then crash report is sent`() = runTest { + // Given & When + viewModel.onFinishedEvent.test { + viewModel.setupSite("") + + // Then + awaitItem() + verify(crashLogging).sendReport( + exception = any(), + tags = eq(mapOf("tag" to "MAIN")), + message = eq(null) + ) + cancelAndIgnoreRemainingEvents() + } + } + + private suspend fun setupFetchSitesFlow() { + val xmlRpcEndpoint = "https://example.com/xmlrpc.php" + whenever( + applicationPasswordLoginHelper + .storeApplicationPasswordCredentialsFrom(eq(urlLogin)) + ).thenReturn(false) + whenever( + selfHostedEndpointFinder + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!) + ).thenReturn(xmlRpcEndpoint) + } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt index 9c4a55e708a8..b877ea62cfdc 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt @@ -1,5 +1,6 @@ package org.wordpress.android.ui.accounts.login +import com.automattic.android.tracks.crashlogging.CrashLogging import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Before @@ -63,6 +64,9 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache + @Mock + lateinit var crashLogging: CrashLogging + private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -77,7 +81,8 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { wpLoginClient, appLogWrapper, apiRootUrlCache, - discoverSuccessWrapper + discoverSuccessWrapper, + crashLogging ) } @@ -150,7 +155,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { val result = applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(testUriLogin) assertFalse(result) - verify(siteStore).sites + verify(siteStore, times(2)).sites verify(dispatcherWrapper, times(0)).updateApplicationPassword(any()) verify(dispatcherWrapper, times(0)).removeApplicationPassword(any()) } diff --git a/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java b/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java index 6ecaef5b2a47..6b050b6c96d1 100644 --- a/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java +++ b/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java @@ -1125,7 +1125,8 @@ public enum Stat { BACKGROUND_REST_AUTODISCOVERY_FAILED, WP_ANDROID_APPLICATION_PASSWORD_LOGIN, JP_ANDROID_APPLICATION_PASSWORD_LOGIN, - APPLICATION_PASSWORD_SET_OFF; + APPLICATION_PASSWORD_SET_OFF, + APPLICATION_PASSWORD_STORING_FAILED; /* * Please set the event name in the enum only if the new Stat's name in lower case does not match it. * In that case you also need to add the event in the `AnalyticsTrackerNosaraTest.specialNames` map. @@ -1133,7 +1134,7 @@ public enum Stat { private String mEventName; - Stat(String eventName) { + Stat(@Nullable String eventName) { this.mEventName = eventName; } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt index 514c23868790..6133c0bf61a6 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt @@ -1480,18 +1480,25 @@ open class SiteStore @Inject constructor( } } + @Suppress("TooGenericExceptionCaught") suspend fun fetchSitesXmlRpcFromApplicationPassword( payload: RefreshSitesXMLRPCApplicationPasswordCredentialsPayload ): OnSiteChanged { return coroutineEngine.withDefaultContext(T.API, this, "Fetch sites") { - updateSites( - siteXMLRPCClient.fetchSitesFromApplicationPassword( - payload.url, - payload.apiRootUrl, - payload.username, - payload.password + try { + updateSites( + siteXMLRPCClient.fetchSitesFromApplicationPassword( + payload.url, + payload.apiRootUrl, + payload.username, + payload.password + ) ) - ) + } catch (e: Exception) { + val errorMsg = e.message ?: e.javaClass.simpleName + AppLog.e(T.API, "Failed to fetch/store sites: $errorMsg", e) + OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, errorMsg)) + } } } @@ -1547,7 +1554,7 @@ open class SiteStore @Inject constructor( } } - @Suppress("SwallowedException") + @Suppress("SwallowedException", "TooGenericExceptionCaught") private fun updateApplicationPassword(siteModel: SiteModel): OnSiteChanged { return try { val siteFromDB = getSiteByLocalId(siteModel.id) @@ -1569,6 +1576,14 @@ open class SiteStore @Inject constructor( OnSiteChanged(siteSqlUtils.insertOrUpdateSite(siteToStore)) } catch (e: DuplicateSiteException) { OnSiteChanged(SiteError(DUPLICATE_SITE)) + } catch (e: Exception) { + val errorMsg = e.message ?: e.javaClass.simpleName + AppLog.e( + T.DB, + "Failed to update application password: $errorMsg", + e + ) + OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, errorMsg)) } }