Merge pull request #14538 from woocommerce/issue/WOOMOB-1181-fetch-app-passwords-on-launch

[Jetpack Performance] Fetch app passwords support on app launch

Author: hichamboushaba

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/jetpack/JetpackActivationRepository.kt

@@ -165,12 +165,9 @@ class JetpackActivationRepository @Inject constructor(
                 WooLog.d(WooLog.T.LOGIN, "Jetpack Activation: Site $siteUrl is missing from account sites")
                 Result.failure(IllegalStateException("Site missing"))
             } else {
-                if (!site.hasWooCommerce) {
-                    // If the site doesn't have WooCommerce, let's do one additional fetch using `fetchSite`,
-                    // this function will make sure to fetch data from the remote site, which might result in more
-                    // accurate result
-                    siteStore.fetchSite(site)
-                }
+                // Fetch full site details from the remote site, this will also allow us to fetch the
+                // Applications Passwords support on the site
+                siteStore.fetchSite(site)
                 Result.success(site)
             }
         }

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/system/WooSystemRestClient.kt

@@ -5,17 +5,13 @@ import com.google.gson.annotations.SerializedName
 import org.wordpress.android.fluxc.generated.endpoint.WOOCOMMERCE
 import org.wordpress.android.fluxc.generated.endpoint.WPAPI
 import org.wordpress.android.fluxc.model.SiteModel
-import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse
 import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooNetwork
 import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooPayload
-import org.wordpress.android.fluxc.network.rest.wpcom.wc.toWooError
 import org.wordpress.android.fluxc.utils.toWooPayload
 import javax.inject.Inject
 
 class WooSystemRestClient @Inject constructor(private val wooNetwork: WooNetwork) {
     companion object {
-        private const val NOT_FOUND = 404
-        private const val FORBIDDEN = 403
         private const val SAVE_SITE_TITLE_RESPONSE_FIELD = "title"
     }
 
@@ -44,51 +40,6 @@ class WooSystemRestClient @Inject constructor(private val wooNetwork: WooNetwork
         return response.toWooPayload()
     }
 
-    /**
-     * Test the settings endpoint of WooCommerce to confirm if the plugin is available or not.
-     * We pass an empty _fields just to reduce the response payload size, as we don't care about the contents
-     *
-     * @return JetpackSuccess(true) if WooCommerce is installed, JetpackSuccess(false) is we get a 404,
-     *         and JetpackError otherwise
-     */
-    suspend fun checkIfWooCommerceIsAvailable(site: SiteModel): WooPayload<Boolean> {
-        val url = WOOCOMMERCE.settings.pathV3
-
-        val response = wooNetwork.executeGetGsonRequest(
-            site = site,
-            path = url,
-            params = mapOf("_fields" to ""),
-            clazz = Any::class.java
-        )
-
-        return when (response) {
-            is WPAPIResponse.Success -> WooPayload(true)
-            is WPAPIResponse.Error -> {
-                when (response.error.volleyError?.networkResponse?.statusCode) {
-                    NOT_FOUND -> WooPayload(false)
-                    FORBIDDEN -> {
-                        // If we get a 403 status code, we can infer that Woo is installed,
-                        // the user just doesn't have permission to access the Woo API
-                        WooPayload(true)
-                    }
-                    else -> WooPayload(response.error.toWooError())
-                }
-            }
-        }
-    }
-
-    suspend fun fetchSiteSettings(site: SiteModel): WooPayload<WPSiteSettingsResponse> {
-        val url = WPAPI.settings.urlV2
-
-        val response = wooNetwork.executeGetGsonRequest(
-            site = site,
-            path = url,
-            clazz = WPSiteSettingsResponse::class.java
-        )
-
-        return response.toWooPayload()
-    }
-
     suspend fun saveSiteTitle(site: SiteModel, siteTitle: String): WooPayload<SaveSiteTitleResponse> {
         val url = WPAPI.settings.urlV2
 
@@ -139,12 +90,4 @@ class WooSystemRestClient @Inject constructor(private val wooNetwork: WooNetwork
         val security: JsonElement? = null,
         val pages: JsonElement? = null
     )
-
-    data class WPSiteSettingsResponse(
-        @SerializedName("title") val title: String? = null,
-        @SerializedName("description") val description: String? = null,
-        @SerializedName("url") val url: String? = null,
-        @SerializedName("show_on_front") val showOnFront: String? = null,
-        @SerializedName("page_on_front") val pageOnFront: Long? = null
-    )
 }

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WooCommerceStore.kt

@@ -3,9 +3,6 @@ package org.wordpress.android.fluxc.store
 import android.content.Context
 import com.wellsql.generated.SiteModelTable
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.async
-import kotlinx.coroutines.awaitAll
-import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.runBlocking
@@ -92,9 +89,6 @@ open class WooCommerceStore @Inject internal constructor(
         const val WOO_API_NAMESPACE_V3 = "wc/v3"
     }
 
-    private val SiteModel.needsAdditionalCheckForWooInstallation
-        get() = origin == SiteModel.ORIGIN_XMLRPC || (origin == SiteModel.ORIGIN_WPCOM_REST && isJetpackCPConnected)
-
     override fun onRegister() = AppLog.d(T.API, "WooCommerceStore onRegister")
 
     @Subscribe(threadMode = ThreadMode.ASYNC)
@@ -108,33 +102,7 @@ open class WooCommerceStore @Inject internal constructor(
             OnSiteChanged()
         }
 
-        if (fetchResult.isError) {
-            emitChange(fetchResult)
-
-            return WooResult(
-                WooError(
-                    type = GENERIC_ERROR,
-                    message = fetchResult.error.message,
-                    original = UNKNOWN
-                )
-            )
-        }
-
-        // Fetch WooCommerce availability for non-Jetpack sites
-        val updatedSites = siteStore.sites
-            .filter { it.needsAdditionalCheckForWooInstallation }
-            .pmap { site ->
-                val isResultUpdated = fetchAndUpdateNonJetpackSite(site)
-                if (isResultUpdated.model == true && !fetchResult.updatedSites.contains(site)) {
-                    1
-                } else {
-                    0
-                }
-            }.sum()
-
-        val rowsAffected = fetchResult.rowsAffected + updatedSites
-
-        emitChange(OnSiteChanged(rowsAffected, fetchResult.updatedSites))
+        emitChange(fetchResult)
 
         return withContext(Dispatchers.IO) { WooResult(getWooCommerceSites()) }
     }
@@ -158,28 +126,7 @@ open class WooCommerceStore @Inject internal constructor(
             }
         }
 
-        if (!site.isJetpackCPConnected) {
-            // The endpoint used by siteStore to fetch a single site is broken for Jetpack CP sites, so skip it for them
-            siteStore.fetchSite(site).let {
-                if (it.isError) {
-                    return WooResult(
-                        WooError(
-                            type = GENERIC_ERROR,
-                            message = it.error.message,
-                            original = UNKNOWN
-                        )
-                    )
-                }
-            }
-        }
-
-        val isSiteUpdated = if (site.needsAdditionalCheckForWooInstallation) {
-            fetchAndUpdateNonJetpackSite(site).let {
-                if (it.isError) return WooResult(it.error)
-
-                it.model!!
-            }
-        } else false
+        val fetchResult = siteStore.fetchSite(site)
 
         val updatedSite = withContext(Dispatchers.IO) {
             siteStore.getSiteByLocalId(site.id)
@@ -191,7 +138,7 @@ open class WooCommerceStore @Inject internal constructor(
             )
         )
 
-        if (isSiteUpdated) {
+        if (fetchResult.rowsAffected > 0) {
             emitChange(OnSiteChanged(1, listOf(updatedSite)))
         }
 
@@ -330,83 +277,6 @@ open class WooCommerceStore @Inject internal constructor(
         }
     }
 
-    /**
-     * Fetch additional data about non-Jetpack connected sites, and here we mean two types:
-     * 1. Jetpack CP connected sites.
-     * 2. Self-hosted sites accessed using site credentials.
-     *
-     * @return [WooResult] representing whether the result succeeded and whether the site was updated
-     */
-    @Suppress("ReturnCount")
-    private suspend fun fetchAndUpdateNonJetpackSite(site: SiteModel): WooResult<Boolean> {
-        // Fetch site metadata for Jetpack CP sites
-        val isMetadataUpdated = if (site.isJetpackCPConnected) {
-            fetchAndUpdateMetaDataFromSiteSettings(site).let {
-                if (it.isError) {
-                    return it
-                }
-                it.model!!
-            }
-        } else false
-
-        // Check Woo installation status for Jetpack CP sites and non-Jetpack sites
-        val isWooStatusUpdated = fetchAndUpdateWooCommerceAvailability(site).let {
-            if (it.isError) {
-                return it
-            }
-            it.model!!
-        }
-
-        return WooResult(isMetadataUpdated || isWooStatusUpdated)
-    }
-
-    private suspend fun fetchAndUpdateMetaDataFromSiteSettings(site: SiteModel): WooResult<Boolean> {
-        fun SiteModel.updateFromSiteSettings(response: WooSystemRestClient.WPSiteSettingsResponse) = apply {
-            name = response.title ?: name
-            description = response.description ?: description
-            url = response.url ?: url
-            showOnFront = response.showOnFront ?: showOnFront
-            pageOnFront = response.pageOnFront ?: pageOnFront
-        }
-
-        return coroutineEngine.withDefaultContext(T.API, this, "fetchAndUpdateMetaDataFromSiteSettings") {
-            val response = systemRestClient.fetchSiteSettings(site)
-            return@withDefaultContext when {
-                response.isError -> {
-                    AppLog.w(T.API, "fetching site settings  site ${site.siteId}")
-                    WooResult(response.error)
-                }
-
-                else -> {
-                    WooResult(response.result?.let {
-                        siteSqlUtils.insertOrUpdateSite(site.updateFromSiteSettings(it)) == 1
-                    } ?: false)
-                }
-            }
-        }
-    }
-
-    private suspend fun fetchAndUpdateWooCommerceAvailability(site: SiteModel): WooResult<Boolean> {
-        return coroutineEngine.withDefaultContext(T.API, this, "fetchAndUpdateWooCommerceAvailability") {
-            val response = systemRestClient.checkIfWooCommerceIsAvailable(site)
-            return@withDefaultContext when {
-                response.isError -> {
-                    AppLog.w(T.API, "Checking WooCommerce availability failed for site ${site.siteId}")
-                    WooResult(response.error)
-                }
-
-                else -> {
-                    val updated = response.result?.takeIf { it != site.hasWooCommerce }?.let {
-                        site.hasWooCommerce = it
-                        siteSqlUtils.insertOrUpdateSite(site)
-                        true
-                    } ?: false
-                    WooResult(updated)
-                }
-            }
-        }
-    }
-
     suspend fun fetchSupportedApiVersion(
         site: SiteModel,
         overrideRetryPolicy: Boolean = false
@@ -531,11 +401,13 @@ open class WooCommerceStore @Inject internal constructor(
                     )
                     WooResult(response.error)
                 }
+
                 response.result != null -> {
                     val settings = settingsMapper.mapTaxBasedOnSettings(response.result, site.localId())
                     taxBasedOnDao.insertOrUpdate(settings)
                     WooResult(settings)
                 }
+
                 else -> {
                     WooResult(WooError(GENERIC_ERROR, UNKNOWN))
                 }
@@ -554,12 +426,14 @@ open class WooCommerceStore @Inject internal constructor(
                     )
                     WooResult(response.error)
                 }
+
                 response.result != null -> {
                     val settingsMap = response.result.associate { setting ->
                         setting.id to (setting.value ?: setting.default ?: "")
                     }
                     WooResult(settingsMap)
                 }
+
                 else -> {
                     WooResult(WooError(GENERIC_ERROR, UNKNOWN))
                 }
@@ -648,8 +522,4 @@ open class WooCommerceStore @Inject internal constructor(
     ): String {
         return formatCurrencyForDisplay(amount.toString(), site, currencyCode, applyDecimalFormatting)
     }
-
-    private suspend fun <A, B> Iterable<A>.pmap(f: suspend (A) -> B): List<B> = coroutineScope {
-        map { async { f(it) } }.awaitAll()
-    }
 }