[Jetpack Performance] App Passwords error handling

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

@@ -1,16 +1,15 @@
 package org.wordpress.android.fluxc.network.rest.wpcom.wc
 
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.withContext
 import org.wordpress.android.fluxc.model.SiteModel
 import org.wordpress.android.fluxc.network.rest.wpapi.WPAPINetwork
 import org.wordpress.android.fluxc.network.rest.wpapi.WPAPINetworkError
 import org.wordpress.android.fluxc.network.rest.wpapi.WPAPINetworkingMode
 import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse
 import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration
 import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsNetwork
+import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.JetpackApplicationPasswordsErrorHandler
+import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.JetpackApplicationPasswordsSupport
 import org.wordpress.android.fluxc.network.rest.wpcom.JetpackTunnelWPAPINetwork
-import org.wordpress.android.fluxc.persistence.SiteSqlUtils
 import org.wordpress.android.util.AppLog
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -30,7 +29,8 @@ class WooNetwork @Inject constructor(
     private val applicationPasswordsConfiguration: ApplicationPasswordsConfiguration,
     private val applicationPasswordsNetwork: ApplicationPasswordsNetwork,
     private val jetpackTunnelWPAPINetwork: JetpackTunnelWPAPINetwork,
-    private val siteSqlUtils: SiteSqlUtils
+    private val jetpackApplicationPasswordsSupport: JetpackApplicationPasswordsSupport,
+    private val jetpackApplicationPasswordsErrorHandler: JetpackApplicationPasswordsErrorHandler
 ) : WPAPINetwork {
     override suspend fun <T : Any> executeGetGsonRequest(
         site: SiteModel,
@@ -128,7 +128,7 @@ class WooNetwork @Inject constructor(
         request: suspend WPAPINetwork.() -> WPAPIResponse<T>
     ): WPAPIResponse<T> {
         val appPasswordsEnabled = applicationPasswordsConfiguration.isEnabledForJetpackAccess()
-        if (!appPasswordsEnabled || !site.isApplicationPasswordsSupported) {
+        if (!appPasswordsEnabled || !jetpackApplicationPasswordsSupport.supportsAppPasswords(site)) {
             if (appPasswordsEnabled) {
                 AppLog.v(
                     AppLog.T.API,
@@ -150,15 +150,12 @@ class WooNetwork @Inject constructor(
             is WPAPIResponse.Error<*> -> {
                 logMessageForFailedRequest(requestContext, site.url, appPasswordsResponse.error)
 
-                if (appPasswordsResponse.error.errorCode ==
-                    ApplicationPasswordsNetwork.APPLICATION_PASSWORDS_NOT_SUPPORT_ERROR_CODE
-                ) {
-                    withContext(Dispatchers.IO) {
-                        site.applicationPasswordsAuthorizeUrl = null
-                        siteSqlUtils.insertOrUpdateSite(site)
+                jetpackTunnelWPAPINetwork.request().also {
+                    if (it is WPAPIResponse.Success) {
+                        // when the fallback to Jetpack Tunnel succeeds, notify the error handler
+                        jetpackApplicationPasswordsErrorHandler.handleError(site, appPasswordsResponse.error)
                     }
-                }
-                jetpackTunnelWPAPINetwork.request().copyWith(
+                }.copyWith(
                     networkingMode = WPAPINetworkingMode.JetpackTunnel(
                         isFallback = true,
                         applicationPasswordsError = appPasswordsResponse.error

libs/fluxc-tests/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetworkTest.kt

@@ -7,38 +7,42 @@ import org.mockito.kotlin.given
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
 import org.mockito.kotlin.verify
+import org.mockito.kotlin.whenever
 import org.robolectric.RobolectricTestRunner
 import org.wordpress.android.fluxc.model.SiteModel
 import org.wordpress.android.fluxc.network.rest.wpapi.WPAPINetworkError
 import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse
 import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration
 import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsNetwork
+import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.JetpackApplicationPasswordsErrorHandler
+import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.JetpackApplicationPasswordsSupport
 import org.wordpress.android.fluxc.network.rest.wpcom.JetpackTunnelWPAPINetwork
-import org.wordpress.android.fluxc.persistence.SiteSqlUtils
 import org.wordpress.android.fluxc.test
 
 @RunWith(RobolectricTestRunner::class)
 class WooNetworkTest {
     private val testSite = SiteModel().apply {
+        origin = SiteModel.ORIGIN_WPCOM_REST
         url = "https://example.com"
     }
     private val testPath = "path"
     private val applicationPasswordsConfiguration = FakeApplicationPasswordsConfiguration()
     private val applicationPasswordsNetwork: ApplicationPasswordsNetwork = mock()
     private val jetpackTunnelWPAPINetwork: JetpackTunnelWPAPINetwork = mock()
-    private val siteSqlUtils: SiteSqlUtils = mock()
+    private val jetpackApplicationPasswordsErrorHandler: JetpackApplicationPasswordsErrorHandler = mock()
+    private val jetpackApplicationPasswordsSupport: JetpackApplicationPasswordsSupport = mock()
 
     private val sut = WooNetwork(
         applicationPasswordsConfiguration = applicationPasswordsConfiguration,
         applicationPasswordsNetwork = applicationPasswordsNetwork,
         jetpackTunnelWPAPINetwork = jetpackTunnelWPAPINetwork,
-        siteSqlUtils = siteSqlUtils
+        jetpackApplicationPasswordsSupport = jetpackApplicationPasswordsSupport,
+        jetpackApplicationPasswordsErrorHandler = jetpackApplicationPasswordsErrorHandler
     )
 
     @Test
     fun `given Jetpack site supports app passwords, when making request, then use app passwords network`() = test {
-        testSite.origin = SiteModel.ORIGIN_WPCOM_REST
-        testSite.applicationPasswordsAuthorizeUrl = "authorize_url"
+        whenever(jetpackApplicationPasswordsSupport.supportsAppPasswords(testSite)).thenReturn(true)
         val sampleResponse = SampleResponse("value")
         givenAppPasswordsResponse(WPAPIResponse.Success(SampleResponse("value"), emptyList()))
 
@@ -59,8 +63,7 @@ class WooNetworkTest {
     @Test
     fun `given jetpack site that supports app passwords, when request fails, then fall back to jetpack tunnel`() =
         test {
-            testSite.origin = SiteModel.ORIGIN_WPCOM_REST
-            testSite.applicationPasswordsAuthorizeUrl = "authorize_url"
+            whenever(jetpackApplicationPasswordsSupport.supportsAppPasswords(testSite)).thenReturn(true)
             givenAppPasswordsResponse(
                 WPAPIResponse.Error(
                     WPAPINetworkError(
@@ -84,8 +87,7 @@ class WooNetworkTest {
     @Test
     fun `given jetpack site that does not support app passwords, when making request, then use jetpack tunnel`() =
         test {
-            testSite.origin = SiteModel.ORIGIN_WPCOM_REST
-            testSite.applicationPasswordsAuthorizeUrl = null
+            whenever(jetpackApplicationPasswordsSupport.supportsAppPasswords(testSite)).thenReturn(false)
             val sampleResponse = SampleResponse("value")
             givenJetpackTunnelResponse(WPAPIResponse.Success(sampleResponse, emptyList()))
 
@@ -101,8 +103,7 @@ class WooNetworkTest {
     @Test
     fun `when app passwords are disabled for jetpack access, then always use jetpack tunnel`() = test {
         applicationPasswordsConfiguration.isEnabledForJetpackAccessValue = false
-        testSite.origin = SiteModel.ORIGIN_WPCOM_REST
-        testSite.applicationPasswordsAuthorizeUrl = "authorize_url"
+        whenever(jetpackApplicationPasswordsSupport.supportsAppPasswords(testSite)).thenReturn(true)
         val sampleResponse = SampleResponse("value")
         givenJetpackTunnelResponse(WPAPIResponse.Success(sampleResponse, emptyList()))
 
@@ -121,33 +122,21 @@ class WooNetworkTest {
     }
 
     @Test
-    fun `when detecting that a site doesn't support app passwords, then update cached site with correct status`() =
-        test {
-            testSite.origin = SiteModel.ORIGIN_WPCOM_REST
-            testSite.applicationPasswordsAuthorizeUrl = "authorize_url"
-            givenAppPasswordsResponse(
-                WPAPIResponse.Error(
-                    WPAPINetworkError(
-                        mock(),
-                        errorCode = ApplicationPasswordsNetwork.APPLICATION_PASSWORDS_NOT_SUPPORT_ERROR_CODE
-                    )
-                )
-            )
-            givenJetpackTunnelResponse(
-                WPAPIResponse.Success(SampleResponse("value"), emptyList())
-            )
+    fun `when jetpack tunnel fallback succeeds after app passwords failure, then notify error handler`() = test {
+        whenever(jetpackApplicationPasswordsSupport.supportsAppPasswords(testSite)).thenReturn(true)
+        val appPasswordsNetworkError = WPAPINetworkError(mock(), "error")
+        givenAppPasswordsResponse(WPAPIResponse.Error(appPasswordsNetworkError))
+        val sampleResponse = SampleResponse("value")
+        givenJetpackTunnelResponse(WPAPIResponse.Success(sampleResponse, emptyList()))
 
-            sut.executeGetGsonRequest(
-                site = testSite,
-                path = testPath,
-                clazz = SampleResponse::class.java
-            )
+        sut.executeGetGsonRequest(
+            site = testSite,
+            path = testPath,
+            clazz = SampleResponse::class.java
+        )
 
-            val expectedSite = testSite.apply {
-                applicationPasswordsAuthorizeUrl = null
-            }
-            verify(siteSqlUtils).insertOrUpdateSite(expectedSite)
-        }
+        verify(jetpackApplicationPasswordsErrorHandler).handleError(testSite, appPasswordsNetworkError)
+    }
 
     private suspend fun givenAppPasswordsResponse(response: WPAPIResponse<SampleResponse>) {
         given(

libs/fluxc/build.gradle

@@ -65,6 +65,7 @@ android.buildTypes.all { buildType ->
 dependencies {
     implementation libs.androidx.exifinterface
     implementation libs.androidx.security.crypto
+    implementation libs.androidx.core.ktx
 
     implementation(libs.wordpress.utils) {
         // Using official volley package

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsManager.kt

@@ -19,6 +19,7 @@ private const val UNAUTHORIZED = 401
 private const val CONFLICT = 409
 private const val NOT_FOUND = 404
 private const val APPLICATION_PASSWORDS_DISABLED_ERROR_CODE = "application_passwords_disabled"
+private const val APPLICATION_PASSWORDS_DISABLED_USER_ERROR_CODE = "application_passwords_disabled_for_user"
 
 @Singleton
 internal class ApplicationPasswordsManager @Inject constructor(
@@ -162,7 +163,8 @@ internal class ApplicationPasswordsManager @Inject constructor(
                     }
 
                     statusCode == NOT_FOUND ||
-                        errorCode == APPLICATION_PASSWORDS_DISABLED_ERROR_CODE -> {
+                        errorCode == APPLICATION_PASSWORDS_DISABLED_ERROR_CODE ||
+                        errorCode == APPLICATION_PASSWORDS_DISABLED_USER_ERROR_CODE -> {
                         appLogWrapper.w(
                             MAIN,
                             "Application Password feature not supported, " +
@@ -236,8 +238,8 @@ internal class ApplicationPasswordsManager @Inject constructor(
                 val error = payload.error
                 appLogWrapper.w(
                     AppLog.T.MAIN, "Application password deletion failed, error: " +
-                    "${error.type} ${error.message}\n" +
-                    "${error.volleyError?.toString()}"
+                        "${error.type} ${error.message}\n" +
+                        "${error.volleyError?.toString()}"
                 )
                 ApplicationPasswordDeletionResult.Failure(error)
             }

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsNetwork.kt

@@ -105,15 +105,11 @@ class ApplicationPasswordsNetwork @Inject constructor(
             }
 
             is ApplicationPasswordCreationResult.NotSupported -> {
+                val networkError = credentialsResult.originalError.toWPAPINetworkError()
                 if (listener.isPresent) {
-                    listener.get().onFeatureUnavailable(site, credentialsResult.originalError.toWPAPINetworkError())
+                    listener.get().onFeatureUnavailable(site, networkError)
                 }
-                return WPAPIResponse.Error(
-                    WPAPINetworkError(
-                        baseError = credentialsResult.originalError.toWPAPINetworkError(),
-                        errorCode = APPLICATION_PASSWORDS_NOT_SUPPORT_ERROR_CODE
-                    )
-                )
+                return WPAPIResponse.Error(networkError)
             }
         }
 
@@ -187,10 +183,6 @@ class ApplicationPasswordsNetwork @Inject constructor(
         clazz: Class<T>,
         params: Map<String, String>
     ) = executeGsonRequest(site, HttpMethod.DELETE, path, clazz, params)
-
-    companion object {
-        const val APPLICATION_PASSWORDS_NOT_SUPPORT_ERROR_CODE = "application_passwords_not_supported"
-    }
 }
 
 private fun BaseNetworkError.toWPAPINetworkError(): WPAPINetworkError {

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/JetpackApplicationPasswordsErrorHandler.kt

@@ -0,0 +1,55 @@
+package org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords
+
+import org.wordpress.android.fluxc.model.SiteModel
+import org.wordpress.android.fluxc.network.rest.wpapi.WPAPINetworkError
+import org.wordpress.android.fluxc.utils.AppLogWrapper
+import org.wordpress.android.util.AppLog
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Singleton
+class JetpackApplicationPasswordsErrorHandler @Inject constructor(
+    private val jetpackApplicationPasswordsSupport: JetpackApplicationPasswordsSupport,
+    private val appLogWrapper: AppLogWrapper
+) {
+    private var failuresCount: MutableMap<Long, Int> = mutableMapOf()
+
+    @Suppress("ComplexCondition")
+    fun handleError(siteModel: SiteModel, error: WPAPINetworkError) {
+        val httpStatusCode = error.volleyError?.networkResponse?.statusCode
+        val apiErrorCode = error.errorCode
+
+        if (httpStatusCode == UNAUTHORIZED ||
+            httpStatusCode == FORBIDDEN ||
+            httpStatusCode == TOO_MANY_REQUESTS ||
+            apiErrorCode == "incorrect_password" ||
+            apiErrorCode == "application_passwords_disabled_for_user"
+        ) {
+            appLogWrapper.w(
+                AppLog.T.API,
+                "Disabling Jetpack Application Passwords support for site ${siteModel.siteId} " +
+                    "due to error: $httpStatusCode / $apiErrorCode"
+            )
+            jetpackApplicationPasswordsSupport.flagAsUnsupported(siteModel)
+        } else {
+            val siteFailuresCount = (failuresCount[siteModel.siteId] ?: 0) + 1
+            failuresCount[siteModel.siteId] = siteFailuresCount
+
+            if (siteFailuresCount >= FAILURES_THRESHOLD) {
+                appLogWrapper.w(
+                    AppLog.T.API,
+                    "Disabling Jetpack Application Passwords support for site ${siteModel.siteId} " +
+                        "after $siteFailuresCount failures"
+                )
+                jetpackApplicationPasswordsSupport.flagAsUnsupported(siteModel)
+            }
+        }
+    }
+
+    companion object {
+        private const val UNAUTHORIZED = 401
+        private const val FORBIDDEN = 403
+        private const val TOO_MANY_REQUESTS = 429
+        private const val FAILURES_THRESHOLD = 10
+    }
+}

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/JetpackApplicationPasswordsSupport.kt

@@ -0,0 +1,28 @@
+package org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords
+
+import android.content.Context
+import androidx.core.content.edit
+import org.wordpress.android.fluxc.model.SiteModel
+import org.wordpress.android.fluxc.utils.PreferenceUtils
+import javax.inject.Inject
+
+class JetpackApplicationPasswordsSupport @Inject constructor(context: Context) {
+    private val fluxCPreferences by lazy { PreferenceUtils.getFluxCPreferences(context) }
+
+    fun supportsAppPasswords(siteModel: SiteModel): Boolean {
+        return siteModel.isApplicationPasswordsSupported &&
+            fluxCPreferences.getStringSet(UNSUPPORTED_JETPACK_APP_PASSWORDS_SITES, null)
+                ?.contains(siteModel.siteId.toString()) != true
+    }
+
+    fun flagAsUnsupported(siteModel: SiteModel) {
+        val unsupportedSites = fluxCPreferences.getStringSet(UNSUPPORTED_JETPACK_APP_PASSWORDS_SITES, null) ?: setOf()
+        fluxCPreferences.edit {
+            putStringSet(UNSUPPORTED_JETPACK_APP_PASSWORDS_SITES, unsupportedSites + siteModel.siteId.toString())
+        }
+    }
+
+    companion object {
+        private const val UNSUPPORTED_JETPACK_APP_PASSWORDS_SITES = "unsupported_jetpack_app_passwords_sites"
+    }
+}

libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordManagerTests.kt

@@ -166,6 +166,29 @@ class ApplicationPasswordManagerTests {
             assertEquals(ApplicationPasswordCreationResult.NotSupported(networkError), result)
         }
 
+    @Test
+    fun `when a jetpack site returns application_passwords_disabled_for_user, then return feature not available`() =
+        runTest {
+            val site = testSite.apply {
+                origin = SiteModel.ORIGIN_WPCOM_REST
+            }
+            val networkError = WPComGsonNetworkError(BaseNetworkError(GenericErrorType.SERVER_ERROR)).apply {
+                apiError = "application_passwords_disabled_for_user"
+            }
+
+            whenever(applicationPasswordsStore.getCredentials(testSite)).thenReturn(null)
+            whenever(mJetpackApplicationPasswordsRestClient.fetchWPAdminUsername(site))
+                .thenReturn(UsernameFetchPayload(testCredentials.userName))
+            whenever(mJetpackApplicationPasswordsRestClient.createApplicationPassword(site, applicationName))
+                .thenReturn(ApplicationPasswordCreationPayload(networkError))
+
+            val result = mApplicationPasswordsManager.getApplicationCredentials(
+                testSite
+            )
+
+            assertEquals(ApplicationPasswordCreationResult.NotSupported(networkError), result)
+        }
+
     @Test
     fun `when a non-jetpack site returns 404, then return feature not available`() =
         runTest {