[Login] Remove dependency on XMLRPC for the Account Mismatch flow

RELEASE-NOTES.txt

@@ -6,6 +6,7 @@
 - [**] Enhanced WebView to dynamically scale receipt content, ensuring optimal fit across all device screen sizes. [https://github.com/woocommerce/woocommerce-android/pull/13266]
 - [*] Fixes missing text in Blaze Campaigns card on larger display and font sizes [https://github.com/woocommerce/woocommerce-android/pull/13300]
 - [*] Puerto Rico is now available in the list of countries that are supported by in-person payments [https://github.com/woocommerce/woocommerce-android/pull/13200]
+- [Internal] [*] XMLRPC is not needed now for Jetpack Connection during an Account Mismatch flow [https://github.com/woocommerce/woocommerce-android/pull/13308]
 - [*] Removed the outdated feedback survey for shipping labels [https://github.com/woocommerce/woocommerce-android/pull/13319]
 
 21.4

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/accountmismatch/AccountMismatchErrorViewModel.kt

@@ -263,6 +263,10 @@ class AccountMismatchErrorViewModel @Inject constructor(
                 val site = site.await() ?: error("The site is not cached")
                 accountMismatchRepository.fetchJetpackConnectedEmail(site).fold(
                     onSuccess = { email ->
+                        // Remove the WPAPI site from DB before continuing
+                        // This ensure we don't have a duplicate site with a wrong origin
+                        accountMismatchRepository.removeSiteFromDB(site)
+
                         val isUserAuthenticated = accountRepository.isUserLoggedIn() &&
                             accountRepository.getUserAccount()?.email == email
                         triggerEvent(OnJetpackConnectedEvent(email, isAuthenticated = isUserAuthenticated))

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/accountmismatch/AccountMismatchRepository.kt

@@ -1,36 +1,33 @@
 package com.woocommerce.android.ui.login.accountmismatch
 
 import com.woocommerce.android.OnChangedException
+import com.woocommerce.android.ui.login.WPApiSiteRepository
 import com.woocommerce.android.util.WooLog
-import com.woocommerce.android.util.awaitAny
-import com.woocommerce.android.util.awaitEvent
-import com.woocommerce.android.util.dispatchAndAwait
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.async
-import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.withContext
 import org.wordpress.android.fluxc.Dispatcher
-import org.wordpress.android.fluxc.generated.AuthenticationActionBuilder
 import org.wordpress.android.fluxc.generated.SiteActionBuilder
 import org.wordpress.android.fluxc.model.SiteModel
-import org.wordpress.android.fluxc.store.AccountStore.OnAuthenticationChanged
-import org.wordpress.android.fluxc.store.AccountStore.OnDiscoveryResponse
+import org.wordpress.android.fluxc.model.jetpack.JetpackUser
 import org.wordpress.android.fluxc.store.JetpackStore
 import org.wordpress.android.fluxc.store.SiteStore
-import org.wordpress.android.fluxc.store.SiteStore.OnSiteChanged
-import org.wordpress.android.fluxc.store.SiteStore.RefreshSitesXMLRPCPayload
 import org.wordpress.android.login.util.SiteUtils
 import javax.inject.Inject
 
 class AccountMismatchRepository @Inject constructor(
     private val jetpackStore: JetpackStore,
     private val siteStore: SiteStore,
+    private val wpApiSiteRepository: WPApiSiteRepository,
     private val dispatcher: Dispatcher
 ) {
     suspend fun getSiteByUrl(url: String): SiteModel? = withContext(Dispatchers.IO) {
         SiteUtils.getSiteByMatchingUrl(siteStore, url)
     }
 
+    fun removeSiteFromDB(site: SiteModel) {
+        dispatcher.dispatch(SiteActionBuilder.newRemoveSiteAction(site))
+    }
+
     suspend fun fetchJetpackConnectionUrl(site: SiteModel): Result<String> {
         WooLog.d(WooLog.T.LOGIN, "Fetching Jetpack Connection URL")
         val result = jetpackStore.fetchJetpackConnectionUrl(site, autoRegisterSiteIfNeeded = true)
@@ -39,6 +36,7 @@ class AccountMismatchRepository @Inject constructor(
                 WooLog.w(WooLog.T.LOGIN, "Fetching Jetpack Connection URL failed: ${result.error.message}")
                 Result.failure(OnChangedException(result.error, result.error.message))
             }
+
             else -> {
                 WooLog.d(WooLog.T.LOGIN, "Jetpack connection URL fetched successfully")
                 Result.success(result.url)
@@ -48,21 +46,21 @@ class AccountMismatchRepository @Inject constructor(
 
     suspend fun fetchJetpackConnectedEmail(site: SiteModel): Result<String> {
         WooLog.d(WooLog.T.LOGIN, "Fetching email of Jetpack User")
-        val result = jetpackStore.fetchJetpackUser(site)
-        return when {
-            result.isError -> {
-                WooLog.w(WooLog.T.LOGIN, "Fetching Jetpack User failed error: $result.error.message")
-                Result.failure(OnChangedException(result.error, result.error.message))
-            }
-            result.user?.wpcomEmail.isNullOrEmpty() -> {
-                WooLog.w(WooLog.T.LOGIN, "Cannot find Jetpack Email in response")
-                Result.failure(Exception("Email missing from response"))
-            }
-            else -> {
-                WooLog.d(WooLog.T.LOGIN, "Jetpack User fetched successfully")
-                Result.success(result.user!!.wpcomEmail)
+
+        return fetchJetpackUser(site)
+            .onFailure {
+                WooLog.w(WooLog.T.LOGIN, "Fetching Jetpack User failed error: ${it.message}")
+            }.mapCatching {
+                val wpcomEmail = it?.wpcomEmail
+                if (wpcomEmail.isNullOrEmpty()) {
+                    WooLog.w(WooLog.T.LOGIN, "Cannot find Jetpack Email in response")
+                    @Suppress("TooGenericExceptionThrown")
+                    throw Exception("Email missing from response")
+                } else {
+                    WooLog.d(WooLog.T.LOGIN, "Jetpack User fetched successfully")
+                    wpcomEmail
+                }
             }
-        }
     }
 
     suspend fun checkJetpackConnection(
@@ -72,121 +70,39 @@ class AccountMismatchRepository @Inject constructor(
     ): Result<JetpackConnectionStatus> {
         WooLog.d(WooLog.T.LOGIN, "Checking Jetpack Connection status for site $siteUrl")
 
-        return discoverXMLRPCAddress(siteUrl)
-            .mapCatching { xmlrpcEndpoint ->
-                val xmlrpcSite = fetchXMLRPCSite(siteUrl, xmlrpcEndpoint, username, password).getOrThrow()
-                when {
-                    xmlrpcSite.jetpackUserEmail.isNullOrEmpty() -> {
-                        WooLog.d(WooLog.T.LOGIN, "Jetpack site is not connected to a WPCom account")
-                        JetpackConnectionStatus.NotConnected
-                    }
-                    else -> {
-                        WooLog.d(
-                            tag = WooLog.T.LOGIN,
-                            message = "Jetpack site is connected to different WPCom account:" +
-                                " ${xmlrpcSite.jetpackUserEmail}"
-                        )
-                        JetpackConnectionStatus.ConnectedToDifferentAccount(xmlrpcSite.jetpackUserEmail)
-                    }
-                }
-            }
-    }
-
-    /**
-     * Represents Jetpack Connection status for the current wp-admin account
-     */
-    sealed interface JetpackConnectionStatus {
-        object NotConnected : JetpackConnectionStatus
-        data class ConnectedToDifferentAccount(val wpcomEmail: String) : JetpackConnectionStatus
-    }
-
-    private suspend fun discoverXMLRPCAddress(siteUrl: String): Result<String> {
-        WooLog.d(WooLog.T.LOGIN, "Running discovery to fetch XMLRPC endpoint for site $siteUrl")
-
-        val action = AuthenticationActionBuilder.newDiscoverEndpointAction(siteUrl)
-        val event: OnDiscoveryResponse = dispatcher.dispatchAndAwait(action)
-
-        return if (event.isError) {
-            WooLog.w(WooLog.T.LOGIN, "XMLRPC Discovery failed, error: ${event.error}")
-            Result.failure(OnChangedException(event.error, event.error.name))
-        } else {
-            WooLog.d(
-                tag = WooLog.T.LOGIN,
-                message = "XMLRPC Discovery succeeded, xmrlpc endpoint: ${event.xmlRpcEndpoint}"
-            )
-            Result.success(event.xmlRpcEndpoint)
+        val site = wpApiSiteRepository.fetchSite(siteUrl, username, password).getOrElse {
+            WooLog.w(WooLog.T.LOGIN, "Site fetch failed, error: ${it.message}")
+            return Result.failure(it)
         }
-    }
 
-    @Suppress("ReturnCount")
-    private suspend fun fetchXMLRPCSite(
-        siteUrl: String,
-        xmlrpcEndpoint: String,
-        username: String,
-        password: String
-    ): Result<SiteModel> = coroutineScope {
-        val authenticationTask = async {
-            dispatcher.awaitEvent<OnAuthenticationChanged>().also { event ->
-                if (event.isError) {
-                    WooLog.w(
-                        tag = WooLog.T.LOGIN,
-                        message = "Authenticating to XMLRPC site $xmlrpcEndpoint failed, " +
-                            "error: ${event.error.message}"
-                    )
-                }
+        return fetchJetpackUser(site).onFailure {
+            WooLog.w(WooLog.T.LOGIN, "Jetpack User fetch failed, error: ${it.message}")
+        }.map {
+            if (it?.isConnected != true) {
+                WooLog.w(WooLog.T.LOGIN, "Account is not connected to a WPCom account")
+                JetpackConnectionStatus.NotConnected
+            } else {
+                WooLog.d(WooLog.T.LOGIN, "Account is connected to different WPCom account: ${it.wpcomEmail}")
+                JetpackConnectionStatus.ConnectedToDifferentAccount(it.wpcomEmail)
             }
         }
-        val siteTask = async {
-            val selfHostedPayload = RefreshSitesXMLRPCPayload(
-                username = username,
-                password = password,
-                url = xmlrpcEndpoint
-            )
-            dispatcher.dispatchAndAwait<RefreshSitesXMLRPCPayload, OnSiteChanged>(
-                action = SiteActionBuilder.newFetchSitesXmlRpcAction(
-                    selfHostedPayload
-                )
-            ).also { event ->
-                if (event.isError) {
-                    WooLog.w(
-                        tag = WooLog.T.LOGIN,
-                        message = "XMLRPC site $xmlrpcEndpoint fetch failed, error: ${event.error.message}"
-                    )
-                } else {
-                    WooLog.d(WooLog.T.LOGIN, "XMLRPC site $xmlrpcEndpoint fetch succeeded")
-                }
-            }
-        }
-
-        val tasks = listOf(authenticationTask, siteTask)
-
-        val event = tasks.awaitAny()
-
-        if (event.isError) return@coroutineScope Result.failure(OnChangedException(event.error))
-
-        return@coroutineScope fetchSiteOptions(siteUrl)
     }
 
-    private suspend fun fetchSiteOptions(siteUrl: String): Result<SiteModel> {
-        WooLog.d(WooLog.T.LOGIN, "Fetch XMLRPC site options")
-        val site = getSiteByUrl(siteUrl) ?: run {
-            WooLog.e(WooLog.T.LOGIN, "XMLRPC site null after fetching!!")
-            return Result.failure(IllegalStateException("XMLRPC Site not found"))
-        }
-
-        val fetchSiteResult = siteStore.fetchSite(site)
-        return when {
-            fetchSiteResult.isError -> {
-                WooLog.w(
-                    tag = WooLog.T.LOGIN,
-                    message = "XMLRPC site $siteUrl fetch failed, error: ${fetchSiteResult.error.message}"
-                )
-                Result.failure(OnChangedException(fetchSiteResult.error, fetchSiteResult.error.message))
-            }
-            else -> {
-                WooLog.d(WooLog.T.LOGIN, "XMLRPC site $siteUrl fetch succeeded")
-                Result.success(getSiteByUrl(siteUrl)!!)
+    private suspend fun fetchJetpackUser(site: SiteModel): Result<JetpackUser?> {
+        return jetpackStore.fetchJetpackUser(site).let {
+            if (it.isError) {
+                Result.failure(OnChangedException(it.error, it.error.message))
+            } else {
+                Result.success(it.user)
             }
         }
     }
+
+    /**
+     * Represents Jetpack Connection status for the current wp-admin account
+     */
+    sealed interface JetpackConnectionStatus {
+        data object NotConnected : JetpackConnectionStatus
+        data class ConnectedToDifferentAccount(val wpcomEmail: String) : JetpackConnectionStatus
+    }
 }

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/login/accountmismatch/AccountMismatchRepositoryTest.kt

@@ -0,0 +1,108 @@
+package com.woocommerce.android.ui.login.accountmismatch
+
+import com.woocommerce.android.FakeDispatcher
+import com.woocommerce.android.ui.login.WPApiSiteRepository
+import com.woocommerce.android.viewmodel.BaseUnitTest
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Test
+import org.mockito.kotlin.any
+import org.mockito.kotlin.eq
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.whenever
+import org.wordpress.android.fluxc.model.SiteModel
+import org.wordpress.android.fluxc.model.jetpack.JetpackUser
+import org.wordpress.android.fluxc.store.JetpackStore
+import org.wordpress.android.fluxc.store.SiteStore
+
+@OptIn(ExperimentalCoroutinesApi::class)
+class AccountMismatchRepositoryTest : BaseUnitTest() {
+    private val jetpackStore = mock<JetpackStore>()
+    private val siteStore = mock<SiteStore>()
+    private val wpApiSiteRepository = mock<WPApiSiteRepository> {
+        onBlocking { fetchSite(any(), any(), any()) }.thenReturn(Result.success(SiteModel()))
+    }
+    private val dispatcher = FakeDispatcher()
+
+    private val sut = AccountMismatchRepository(
+        jetpackStore = jetpackStore,
+        siteStore = siteStore,
+        wpApiSiteRepository = wpApiSiteRepository,
+        dispatcher = dispatcher
+    )
+
+    @Test
+    fun `given a non-connected Jetpack Account, when fetching status, then return correct status`() = testBlocking {
+        whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
+            .thenReturn(JetpackStore.JetpackUserResult(createJetpackUser(isConnected = false)))
+
+        val result = sut.checkJetpackConnection(
+            siteUrl = "https://example.com",
+            username = "username",
+            password = "password"
+        )
+
+        assertThat(result.getOrNull()).isEqualTo(AccountMismatchRepository.JetpackConnectionStatus.NotConnected)
+    }
+
+    @Test
+    fun `given a null jetpack user, when fetching connection status, then assume non-connected`() = testBlocking {
+        whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
+            .thenReturn(JetpackStore.JetpackUserResult(null))
+
+        val result = sut.checkJetpackConnection(
+            siteUrl = "https://example.com",
+            username = "username",
+            password = "password"
+        )
+
+        assertThat(result.getOrNull()).isEqualTo(AccountMismatchRepository.JetpackConnectionStatus.NotConnected)
+    }
+
+    @Test
+    fun `given a connected Jetpack Account, when fetching status, then return correct status`() = testBlocking {
+        whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
+            .thenReturn(JetpackStore.JetpackUserResult(createJetpackUser(isConnected = true, wpcomEmail = "email")))
+
+        val result = sut.checkJetpackConnection(
+            siteUrl = "https://example.com",
+            username = "username",
+            password = "password"
+        )
+
+        assertThat(result.getOrNull())
+            .isEqualTo(AccountMismatchRepository.JetpackConnectionStatus.ConnectedToDifferentAccount("email"))
+    }
+
+    @Test
+    fun `given a correctly connected Jetpack account, when fetching email, then return it`() = testBlocking {
+        whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
+            .thenReturn(JetpackStore.JetpackUserResult(createJetpackUser(isConnected = true, wpcomEmail = "email")))
+
+        val result = sut.fetchJetpackConnectedEmail(SiteModel())
+
+        assertThat(result.getOrNull()).isEqualTo("email")
+    }
+
+    @Test
+    fun `given an issue with Jetpack account, when fetching email, then return error`() = testBlocking {
+        whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
+            .thenReturn(JetpackStore.JetpackUserResult(createJetpackUser(isConnected = true, wpcomEmail = "")))
+
+        val result = sut.fetchJetpackConnectedEmail(SiteModel())
+
+        assertThat(result.isFailure).isTrue()
+    }
+
+    private fun createJetpackUser(
+        isConnected: Boolean = false,
+        wpcomEmail: String = ""
+    ) = JetpackUser(
+        isConnected = isConnected,
+        wpcomEmail = wpcomEmail,
+        isMaster = false,
+        username = "username",
+        wpcomId = 1L,
+        wpcomUsername = "wpcomUsername"
+    )
+}