From 9789e8104fce5ef651b830533d0f25afe7d22cff Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 16:46:18 +0100 Subject: [PATCH 01/22] Refactor handling of Application Passwords configuration This gives the clients more freedom in enabling disabling features, which will allow us to control the feature using remote feature flag. --- .../WooApplicationPasswordsConfiguration.kt | 12 ++++++++++ .../android/di/ApplicationPasswordsModule.kt | 16 +++++--------- .../LoginSiteCredentialsViewModel.kt | 8 ++++--- .../module/ApplicationPasswordsClientId.kt | 13 ----------- .../module/ApplicationPasswordsModule.kt | 3 --- .../ApplicationPasswordsConfiguration.kt | 22 +------------------ 6 files changed, 24 insertions(+), 50 deletions(-) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt delete mode 100644 libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ApplicationPasswordsClientId.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt new file mode 100644 index 000000000000..5347a466a28c --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt @@ -0,0 +1,12 @@ +package com.woocommerce.android.applicationpasswords + +import com.woocommerce.android.BuildConfig +import com.woocommerce.android.util.DeviceInfo +import jakarta.inject.Inject +import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration + +class WooApplicationPasswordsConfiguration @Inject constructor() : ApplicationPasswordsConfiguration { + override val isEnabled: Boolean = true + override val applicationName: String = + "${BuildConfig.APPLICATION_ID}.app-client.${DeviceInfo.name.replace(' ', '-')}" +} diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/di/ApplicationPasswordsModule.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/di/ApplicationPasswordsModule.kt index 55ec5a7e5a88..edff7e3ca044 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/di/ApplicationPasswordsModule.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/di/ApplicationPasswordsModule.kt @@ -1,14 +1,12 @@ package com.woocommerce.android.di -import com.woocommerce.android.BuildConfig import com.woocommerce.android.applicationpasswords.ApplicationPasswordsNotifier -import com.woocommerce.android.util.DeviceInfo +import com.woocommerce.android.applicationpasswords.WooApplicationPasswordsConfiguration import dagger.Binds import dagger.Module -import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent -import org.wordpress.android.fluxc.module.ApplicationPasswordsClientId +import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsListener @Module @@ -19,10 +17,8 @@ interface ApplicationPasswordsModule { notifier: ApplicationPasswordsNotifier ): ApplicationPasswordsListener - companion object { - @Provides - @ApplicationPasswordsClientId - fun providesApplicationPasswordClientId() = - "${BuildConfig.APPLICATION_ID}.app-client.${DeviceInfo.name.replace(' ', '-')}" - } + @Binds + fun bindApplicationPasswordsConfiguration( + configuration: WooApplicationPasswordsConfiguration + ): ApplicationPasswordsConfiguration } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModel.kt index 121c57ba5399..f91a0332d050 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModel.kt @@ -37,8 +37,8 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.fluxc.module.ApplicationPasswordsClientId import org.wordpress.android.fluxc.network.rest.wpapi.Nonce.CookieNonceErrorType.INVALID_CREDENTIALS +import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration import org.wordpress.android.fluxc.store.SiteStore.SiteError import org.wordpress.android.login.LoginAnalyticsListener import org.wordpress.android.util.UrlUtils @@ -55,7 +55,7 @@ class LoginSiteCredentialsViewModel @Inject constructor( private val analyticsTracker: AnalyticsTrackerWrapper, private val appPrefs: AppPrefsWrapper, private val resourceProvider: ResourceProvider, - @ApplicationPasswordsClientId private val applicationPasswordsClientId: String + private val applicationPasswordsConfiguration: ApplicationPasswordsConfiguration ) : ScopedViewModel(savedStateHandle) { companion object { const val SITE_ADDRESS_KEY = "site-address" @@ -82,7 +82,9 @@ class LoginSiteCredentialsViewModel @Inject constructor( private val SiteModel?.fullAuthorizationUrl: String? get() = this?.applicationPasswordsAuthorizeUrl - ?.let { url -> "$url?app_name=$applicationPasswordsClientId&success_url=$REDIRECTION_URL" } + ?.let { url -> + "$url?app_name=${applicationPasswordsConfiguration.applicationName}&success_url=$REDIRECTION_URL" + } val viewState = combine( flowOf(siteAddress.removeSchemeAndSuffix()), diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ApplicationPasswordsClientId.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ApplicationPasswordsClientId.kt deleted file mode 100644 index c1ac54e31ab5..000000000000 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ApplicationPasswordsClientId.kt +++ /dev/null @@ -1,13 +0,0 @@ -package org.wordpress.android.fluxc.module - -import javax.inject.Qualifier -import kotlin.annotation.AnnotationRetention.RUNTIME - -/** - * Defines the name to use for naming the WordPress "application passwords" that the app - * will create. - */ -@Qualifier -@MustBeDocumented -@Retention(RUNTIME) -annotation class ApplicationPasswordsClientId diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ApplicationPasswordsModule.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ApplicationPasswordsModule.kt index 31137c3b456b..2779d6aa8da7 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ApplicationPasswordsModule.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/module/ApplicationPasswordsModule.kt @@ -8,7 +8,4 @@ import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.Appli interface ApplicationPasswordsModule { @BindsOptionalOf fun bindOptionalApplicationPasswordsListener(): ApplicationPasswordsListener - @BindsOptionalOf - @ApplicationPasswordsClientId - fun bindOptionalApplicationPasswordsClientId(): String } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt index f58b792fcb5c..7436b8877a53 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt @@ -1,26 +1,6 @@ package org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords -import org.wordpress.android.fluxc.module.ApplicationPasswordsClientId -import java.util.Optional -import javax.inject.Inject - -/** - * Note: the [ApplicationPasswordsClientId] is provided as [Optional] because we want to keep the feature optional and - * to not force the client apps to provide it. With this change, we will keep Dagger happy, and we move from a compile - * error to a runtime error if it's missing. - */ -internal data class ApplicationPasswordsConfiguration @Inject constructor( - @ApplicationPasswordsClientId private val applicationNameOptional: Optional -) { +interface ApplicationPasswordsConfiguration { val isEnabled: Boolean - get() = applicationNameOptional.isPresent - val applicationName: String - get() = applicationNameOptional.orElseThrow { - NoSuchElementException( - "Please make sure to inject a String instance with " + - "the annotation @${ApplicationPasswordsClientId::class.simpleName} to the Dagger graph" + - "to be able to use the Application Passwords feature" - ) - } } From 6161d745878bb4e60b7c0419a527409d35f87499 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 16:47:13 +0100 Subject: [PATCH 02/22] Rename property --- .../WooApplicationPasswordsConfiguration.kt | 2 +- .../ApplicationPasswordsConfiguration.kt | 2 +- .../java/org/wordpress/android/fluxc/store/MediaStore.java | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt index 5347a466a28c..ffc90e71cb99 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt @@ -6,7 +6,7 @@ import jakarta.inject.Inject import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration class WooApplicationPasswordsConfiguration @Inject constructor() : ApplicationPasswordsConfiguration { - override val isEnabled: Boolean = true + override val isEnabledForSiteCredentials: Boolean = true override val applicationName: String = "${BuildConfig.APPLICATION_ID}.app-client.${DeviceInfo.name.replace(' ', '-')}" } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt index 7436b8877a53..9d7ad6ea5b6f 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt @@ -1,6 +1,6 @@ package org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords interface ApplicationPasswordsConfiguration { - val isEnabled: Boolean + val isEnabledForSiteCredentials: Boolean val applicationName: String } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java index 8a269baf5ca1..0c174ca84b59 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java @@ -801,7 +801,7 @@ private void performUploadMedia(@NonNull UploadMediaPayload payload) { if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) { mWPComV2MediaRestClient.uploadMedia(payload.site, payload.media); } else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI - && mApplicationPasswordsConfiguration.isEnabled()) { + && mApplicationPasswordsConfiguration.isEnabledForSiteCredentials()) { mApplicationPasswordsMediaRestClient.uploadMedia(payload.site, payload.media); } else { mMediaXmlrpcClient.uploadMedia(payload.site, payload.media); @@ -823,7 +823,7 @@ private void performFetchMediaList(@NonNull FetchMediaListPayload payload) { if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) { mWPComV2MediaRestClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType); } else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI - && mApplicationPasswordsConfiguration.isEnabled()) { + && mApplicationPasswordsConfiguration.isEnabledForSiteCredentials()) { mApplicationPasswordsMediaRestClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType); } else { mMediaXmlrpcClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType); @@ -869,7 +869,7 @@ private void performCancelUpload(@NonNull CancelMediaPayload payload) { if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) { mWPComV2MediaRestClient.cancelUpload(media); } else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI - && mApplicationPasswordsConfiguration.isEnabled()) { + && mApplicationPasswordsConfiguration.isEnabledForSiteCredentials()) { mApplicationPasswordsMediaRestClient.cancelUpload(media); } else { mMediaXmlrpcClient.cancelUpload(media); From cb1cf6eefc14ecafdeee87eca04bfb1653549f97 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 16:47:49 +0100 Subject: [PATCH 03/22] Add new property to control whether feature is enabled or not for WPCom login --- .../applicationpasswords/WooApplicationPasswordsConfiguration.kt | 1 + .../applicationpasswords/ApplicationPasswordsConfiguration.kt | 1 + 2 files changed, 2 insertions(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt index ffc90e71cb99..64019357be30 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt @@ -7,6 +7,7 @@ import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.Appli class WooApplicationPasswordsConfiguration @Inject constructor() : ApplicationPasswordsConfiguration { override val isEnabledForSiteCredentials: Boolean = true + override val isEnabledForJetpack: Boolean = false override val applicationName: String = "${BuildConfig.APPLICATION_ID}.app-client.${DeviceInfo.name.replace(' ', '-')}" } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt index 9d7ad6ea5b6f..701bbcadb8ff 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt @@ -2,5 +2,6 @@ package org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords interface ApplicationPasswordsConfiguration { val isEnabledForSiteCredentials: Boolean + val isEnabledForJetpack: Boolean val applicationName: String } From 225395dd2a824c2b9d113683f65f1b5fa4c28763 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 18:01:09 +0100 Subject: [PATCH 04/22] Copy `WooExperimentalNetwork` logic to `WooNetwork` --- .../fluxc/network/rest/wpcom/wc/WooNetwork.kt | 117 ++++++++++++++++-- 1 file changed, 104 insertions(+), 13 deletions(-) diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt index 31c943af10f9..36bbdae46778 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt @@ -1,10 +1,16 @@ 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.WPAPINetworkingMode import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsNetwork +import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsStore 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 @@ -18,8 +24,10 @@ import javax.inject.Singleton */ @Singleton class WooNetwork @Inject constructor( + private val applicationPasswordsNetwork: ApplicationPasswordsNetwork, private val jetpackTunnelWPAPINetwork: JetpackTunnelWPAPINetwork, - private val applicationPasswordsNetwork: ApplicationPasswordsNetwork + private val applicationPasswordsStore: ApplicationPasswordsStore, + private val siteSqlUtils: SiteSqlUtils ) : WPAPINetwork { override suspend fun executeGetGsonRequest( site: SiteModel, @@ -31,8 +39,8 @@ class WooNetwork @Inject constructor( forced: Boolean, requestTimeout: Int, retries: Int - ): WPAPIResponse = site.getDelegate() - .executeGetGsonRequest( + ): WPAPIResponse = handleRequest(site) { + executeGetGsonRequest( site = site, path = path, clazz = clazz, @@ -43,49 +51,132 @@ class WooNetwork @Inject constructor( requestTimeout = requestTimeout, retries = retries ) + } override suspend fun executePostGsonRequest( site: SiteModel, path: String, clazz: Class, body: Map - ): WPAPIResponse = site.getDelegate() - .executePostGsonRequest( + ): WPAPIResponse = handleRequest(site) { + executePostGsonRequest( site = site, path = path, clazz = clazz, body = body ) + } override suspend fun executePutGsonRequest( site: SiteModel, path: String, clazz: Class, body: Map - ): WPAPIResponse = site.getDelegate() - .executePutGsonRequest( + ): WPAPIResponse = handleRequest(site) { + executePutGsonRequest( site = site, path = path, clazz = clazz, body = body ) + } override suspend fun executeDeleteGsonRequest( site: SiteModel, path: String, clazz: Class, params: Map - ): WPAPIResponse = site.getDelegate() - .executeDeleteGsonRequest( + ): WPAPIResponse = handleRequest(site) { + executeDeleteGsonRequest( site = site, path = path, clazz = clazz, params = params ) + } + + private suspend fun handleRequest( + site: SiteModel, + request: suspend WPAPINetwork.() -> WPAPIResponse + ): WPAPIResponse { + return when (site.origin) { + SiteModel.ORIGIN_WPAPI, SiteModel.ORIGIN_XMLRPC -> { + applicationPasswordsNetwork.request().copyWith( + networkingMode = WPAPINetworkingMode.ApplicationPasswords + ) + } + + SiteModel.ORIGIN_WPCOM_REST -> { + handleRequestForJetpackSites(site) { + request() + } + } + + else -> { + throw IllegalArgumentException("Unsupported site origin: ${site.origin}") + } + } + } + + private suspend fun handleRequestForJetpackSites( + site: SiteModel, + request: suspend WPAPINetwork.() -> WPAPIResponse + ): WPAPIResponse { + if (!site.isApplicationPasswordsSupported) { + AppLog.v( + AppLog.T.API, + "Application Passwords not supported for site: ${site.url}, use Jetpack Tunnel" + ) + return jetpackTunnelWPAPINetwork.request().copyWith( + networkingMode = WPAPINetworkingMode.JetpackTunnel() + ) + } + + val hasAppPassword = applicationPasswordsStore.hasCredentials(site) + + return when (val appPasswordsResponse = applicationPasswordsNetwork.request()) { + is WPAPIResponse.Success<*> -> { + AppLog.v( + AppLog.T.API, + "Request successful for site: ${site.url}, using Application Passwords" + ) + (appPasswordsResponse as WPAPIResponse).copyWith( + // When creating a new Application Password, we don't want to track this request, as its duration + // is not relevant to our experiment. + // So we track only requests where we already have a password saved. + networkingMode = if (hasAppPassword) WPAPINetworkingMode.ApplicationPasswordsWithJetpack else null + ) + } + + is WPAPIResponse.Error<*> -> { + AppLog.w( + AppLog.T.API, + "Request failed for site: ${site.url} using Application Passwords, falling back to Jetpack Tunnel" + ) + if (appPasswordsResponse.error.errorCode == + ApplicationPasswordsNetwork.APPLICATION_PASSWORDS_NOT_SUPPORT_ERROR_CODE + ) { + withContext(Dispatchers.IO) { + site.applicationPasswordsAuthorizeUrl = null + siteSqlUtils.insertOrUpdateSite(site) + } + } + jetpackTunnelWPAPINetwork.request().copyWith( + networkingMode = WPAPINetworkingMode.JetpackTunnel( + isFallback = true, + applicationPasswordsError = appPasswordsResponse.error + ) + ) + } + } + } - private fun SiteModel.getDelegate() = when (origin) { - SiteModel.ORIGIN_WPCOM_REST -> jetpackTunnelWPAPINetwork - SiteModel.ORIGIN_XMLRPC, SiteModel.ORIGIN_WPAPI -> applicationPasswordsNetwork - else -> error("Site with unsupported origin") + private fun WPAPIResponse.copyWith( + networkingMode: WPAPINetworkingMode? + ): WPAPIResponse { + return when (this) { + is WPAPIResponse.Success -> this.copy(networkingMode = networkingMode) + is WPAPIResponse.Error -> this.copy(networkingMode = networkingMode) + } } } From 856bbe112f99de7ebd1addf9b34d636f040e2af9 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 18:08:04 +0100 Subject: [PATCH 05/22] Remove WooExperimentalNetwork --- .../rest/wpcom/wc/WooExperimentalNetwork.kt | 181 ------------------ .../rest/wpcom/wc/order/OrderRestClient.kt | 64 ++++--- .../android/fluxc/store/WCOrderStore.kt | 27 +-- .../wpcom/wc/order/OrderRestClientTest.kt | 9 +- 4 files changed, 55 insertions(+), 226 deletions(-) delete mode 100644 libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooExperimentalNetwork.kt diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooExperimentalNetwork.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooExperimentalNetwork.kt deleted file mode 100644 index 2e4d2e955f6e..000000000000 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooExperimentalNetwork.kt +++ /dev/null @@ -1,181 +0,0 @@ -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.WPAPINetworkingMode -import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse -import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsNetwork -import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsStore -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 - -/** - * An experimental network class for WooCommerce that allows using Application Passwords for Jetpack sites too, - * it will allow us to confirm if the Jetpack Tunnel is causing performance issues. - * - * Depending on the results of this experiment, we will either move the logic to the existing WooNetwork class or - * drop the experiment. - */ -@Singleton -class WooExperimentalNetwork @Inject constructor( - private val applicationPasswordsNetwork: ApplicationPasswordsNetwork, - private val jetpackTunnelWPAPINetwork: JetpackTunnelWPAPINetwork, - private val applicationPasswordsStore: ApplicationPasswordsStore, - private val siteSqlUtils: SiteSqlUtils -) : WPAPINetwork { - override suspend fun executeGetGsonRequest( - site: SiteModel, - path: String, - clazz: Class, - params: Map, - enableCaching: Boolean, - cacheTimeToLive: Int, - forced: Boolean, - requestTimeout: Int, - retries: Int - ): WPAPIResponse = handleRequest(site) { - executeGetGsonRequest( - site = site, - path = path, - clazz = clazz, - params = params, - enableCaching = enableCaching, - cacheTimeToLive = cacheTimeToLive, - forced = forced, - requestTimeout = requestTimeout, - retries = retries - ) - } - - override suspend fun executePostGsonRequest( - site: SiteModel, - path: String, - clazz: Class, - body: Map - ): WPAPIResponse = handleRequest(site) { - executePostGsonRequest( - site = site, - path = path, - clazz = clazz, - body = body - ) - } - - override suspend fun executePutGsonRequest( - site: SiteModel, - path: String, - clazz: Class, - body: Map - ): WPAPIResponse = handleRequest(site) { - executePutGsonRequest( - site = site, - path = path, - clazz = clazz, - body = body - ) - } - - override suspend fun executeDeleteGsonRequest( - site: SiteModel, - path: String, - clazz: Class, - params: Map - ): WPAPIResponse = handleRequest(site) { - executeDeleteGsonRequest( - site = site, - path = path, - clazz = clazz, - params = params - ) - } - - private suspend fun handleRequest( - site: SiteModel, - request: suspend WPAPINetwork.() -> WPAPIResponse - ): WPAPIResponse { - return when (site.origin) { - SiteModel.ORIGIN_WPAPI, SiteModel.ORIGIN_XMLRPC -> { - applicationPasswordsNetwork.request().copyWith( - networkingMode = WPAPINetworkingMode.ApplicationPasswords - ) - } - - SiteModel.ORIGIN_WPCOM_REST -> { - handleRequestForJetpackSites(site) { - request() - } - } - - else -> { - throw IllegalArgumentException("Unsupported site origin: ${site.origin}") - } - } - } - - private suspend fun handleRequestForJetpackSites( - site: SiteModel, - request: suspend WPAPINetwork.() -> WPAPIResponse - ): WPAPIResponse { - if (!site.isApplicationPasswordsSupported) { - AppLog.v( - AppLog.T.API, - "Application Passwords not supported for site: ${site.url}, use Jetpack Tunnel" - ) - return jetpackTunnelWPAPINetwork.request().copyWith( - networkingMode = WPAPINetworkingMode.JetpackTunnel() - ) - } - - val hasAppPassword = applicationPasswordsStore.hasCredentials(site) - - return when (val appPasswordsResponse = applicationPasswordsNetwork.request()) { - is WPAPIResponse.Success<*> -> { - AppLog.v( - AppLog.T.API, - "Request successful for site: ${site.url}, using Application Passwords" - ) - (appPasswordsResponse as WPAPIResponse).copyWith( - // When creating a new Application Password, we don't want to track this request, as its duration - // is not relevant to our experiment. - // So we track only requests where we already have a password saved. - networkingMode = if (hasAppPassword) WPAPINetworkingMode.ApplicationPasswordsWithJetpack else null - ) - } - - is WPAPIResponse.Error<*> -> { - AppLog.w( - AppLog.T.API, - "Request failed for site: ${site.url} using Application Passwords, falling back to Jetpack Tunnel" - ) - if (appPasswordsResponse.error.errorCode == - ApplicationPasswordsNetwork.APPLICATION_PASSWORDS_NOT_SUPPORT_ERROR_CODE - ) { - withContext(Dispatchers.IO) { - site.applicationPasswordsAuthorizeUrl = null - siteSqlUtils.insertOrUpdateSite(site) - } - } - jetpackTunnelWPAPINetwork.request().copyWith( - networkingMode = WPAPINetworkingMode.JetpackTunnel( - isFallback = true, - applicationPasswordsError = appPasswordsResponse.error - ) - ) - } - } - } - - private fun WPAPIResponse.copyWith( - networkingMode: WPAPINetworkingMode? - ): WPAPIResponse { - return when (this) { - is WPAPIResponse.Success -> this.copy(networkingMode = networkingMode) - is WPAPIResponse.Error -> this.copy(networkingMode = networkingMode) - } - } -} diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/OrderRestClient.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/OrderRestClient.kt index fd084c15af5a..a0ec11ff79bf 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/OrderRestClient.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/OrderRestClient.kt @@ -21,7 +21,6 @@ import org.wordpress.android.fluxc.model.order.UpdateOrderRequest import org.wordpress.android.fluxc.network.BaseRequest 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.wpcom.wc.WooExperimentalNetwork 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.order.OrderDto.Billing @@ -64,7 +63,6 @@ class OrderRestClient @Inject constructor( private val dispatcher: Dispatcher, private val orderDtoMapper: OrderDtoMapper, private val wooNetwork: WooNetwork, - private val wooExperimentalNetwork: WooExperimentalNetwork, private val coroutineEngine: CoroutineEngine ) { /** @@ -108,6 +106,7 @@ class OrderRestClient @Inject constructor( ) dispatcher.dispatch(WCOrderActionBuilder.newFetchedOrdersAction(payload)) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) val payload = FetchOrdersResponsePayload(orderError, site) @@ -210,6 +209,7 @@ class OrderRestClient @Inject constructor( canLoadMore = canLoadMore ) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) return FetchOrdersResponsePayload(orderError, site) @@ -234,8 +234,7 @@ class OrderRestClient @Inject constructor( @Suppress("LongMethod") fun fetchOrderListSummaries( listDescriptor: WCOrderListDescriptor, - offset: Long, - useAppPasswordsForJetpackSites: Boolean + offset: Long ) { coroutineEngine.launch(T.API, this, "fetchOrderListSummaries") { val startTime = System.currentTimeMillis() @@ -256,13 +255,7 @@ class OrderRestClient @Inject constructor( "created_via" to listDescriptor.createdViaFilter.takeUnless { it.isNullOrBlank() } ) - val network = if (useAppPasswordsForJetpackSites) { - wooExperimentalNetwork - } else { - wooNetwork - } - - val response = network.executeGetGsonRequest( + val response = wooNetwork.executeGetGsonRequest( site = listDescriptor.site, path = url, clazz = Array::class.java, @@ -287,6 +280,7 @@ class OrderRestClient @Inject constructor( ) dispatcher.dispatch(WCOrderActionBuilder.newFetchedOrderListAction(payload)) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) val payload = FetchOrderListResponsePayload( @@ -370,6 +364,7 @@ class OrderRestClient @Inject constructor( ) dispatcher.dispatch(WCOrderActionBuilder.newFetchedOrdersByIdsAction(payload)) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) val payload = FetchOrdersByIdsResponsePayload( @@ -416,6 +411,7 @@ class OrderRestClient @Inject constructor( WCOrderActionBuilder.newFetchedOrderStatusOptionsAction(payload) ) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) val payload = FetchOrderStatusOptionsResponsePayload(orderError, site) @@ -471,6 +467,7 @@ class OrderRestClient @Inject constructor( ) dispatcher.dispatch(WCOrderActionBuilder.newSearchedOrdersAction(payload)) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) val payload = SearchOrdersResponsePayload(orderError, site, searchQuery) @@ -510,6 +507,7 @@ class OrderRestClient @Inject constructor( site ) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) RemoteOrderPayload.Fetching( @@ -534,8 +532,10 @@ class OrderRestClient @Inject constructor( */ fun fetchOrderCountSync(site: SiteModel, filterByStatus: String) { coroutineEngine.launch(T.API, this, "fetchOrderCount") { - dispatcher.dispatch(WCOrderActionBuilder.newFetchedOrdersCountAction( - doFetchOrderCount(site, filterByStatus)) + dispatcher.dispatch( + WCOrderActionBuilder.newFetchedOrdersCountAction( + doFetchOrderCount(site, filterByStatus) + ) ) } } @@ -587,6 +587,7 @@ class OrderRestClient @Inject constructor( site ) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) FetchHasOrdersResponsePayload( @@ -629,6 +630,7 @@ class OrderRestClient @Inject constructor( site ) } + is WPAPIResponse.Error -> { val orderError = wpAPINetworkErrorToOrderError(response.error) RemoteOrderPayload.Updating( @@ -658,10 +660,10 @@ class OrderRestClient @Inject constructor( paymentDetails?.cashPaymentChangeDueAmount?.let { val metaData = mapOf( "meta_data" to listOfNotNull( - mapOf( - "key" to "_cash_change_amount", - "value" to it - ) + mapOf( + "key" to "_cash_change_amount", + "value" to it + ) ) ) @@ -787,6 +789,7 @@ class OrderRestClient @Inject constructor( FetchOrderShipmentTrackingsResponsePayload(site, orderId, trackings) } + is WPAPIResponse.Error -> { val trackingsError = wpAPINetworkErrorToOrderError(response.error) FetchOrderShipmentTrackingsResponsePayload(trackingsError, site, orderId) @@ -843,6 +846,7 @@ class OrderRestClient @Inject constructor( tracking ) } + is WPAPIResponse.Error -> { val trackingsError = wpAPINetworkErrorToOrderError(response.error) AddOrderShipmentTrackingResponsePayload(trackingsError, site, orderId, tracking) @@ -892,6 +896,7 @@ class OrderRestClient @Inject constructor( tracking ) } + is WPAPIResponse.Error -> DeleteOrderShipmentTrackingResponsePayload( wpAPINetworkErrorToOrderError(response.error), site, @@ -1122,16 +1127,16 @@ class OrderRestClient @Inject constructor( } /** - * Performs a batch update of order statuses via the WooCommerce REST API. - * - * This endpoint enables updating multiple orders to the same status in a single network request. - * The WooCommerce API has a limit of 100 orders per batch update. - * - * @param site The site to perform the update on - * @param orderIds List of order IDs to update. Error if exceeds [BATCH_UPDATE_LIMIT] - * @param newStatus The new status to set for all specified orders - * @return [BulkUpdateOrderStatusResponsePayload] containing either the update results or an error - */ + * Performs a batch update of order statuses via the WooCommerce REST API. + * + * This endpoint enables updating multiple orders to the same status in a single network request. + * The WooCommerce API has a limit of 100 orders per batch update. + * + * @param site The site to perform the update on + * @param orderIds List of order IDs to update. Error if exceeds [BATCH_UPDATE_LIMIT] + * @param newStatus The new status to set for all specified orders + * @return [BulkUpdateOrderStatusResponsePayload] containing either the update results or an error + */ suspend fun batchUpdateOrdersStatus( site: SiteModel, orderIds: List, @@ -1199,7 +1204,10 @@ class OrderRestClient @Inject constructor( } } - private fun orderResponseToOrderSummaryModel(response: OrderSummaryApiResponse, siteId: LocalId): WCOrderSummaryModel { + private fun orderResponseToOrderSummaryModel( + response: OrderSummaryApiResponse, + siteId: LocalId + ): WCOrderSummaryModel { return WCOrderSummaryModel( siteId = siteId, orderId = RemoteId(response.id ?: 0), diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt index 48ce91c6ed50..5d20efec9df1 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt @@ -504,7 +504,7 @@ class WCOrderStore @Inject internal constructor( * Returns the notes belonging to supplied [OrderEntity] as a list of [OrderNoteEntity]. */ suspend fun getOrderNotesForOrder(site: SiteModel, orderId: Long): List = - orderNotesDao.queryNotesOfOrder(localSiteId = site.localId(), orderId = RemoteId(orderId)) + orderNotesDao.queryNotesOfOrder(localSiteId = site.localId(), orderId = RemoteId(orderId)) /** * Returns the order status options available for the provided site [SiteModel] as a list of [WCOrderStatusModel]. @@ -539,6 +539,7 @@ class WCOrderStore @Inject internal constructor( WCOrderAction.FETCH_ORDERS_COUNT -> fetchOrdersCount(action.payload as FetchOrdersCountPayload) WCOrderAction.UPDATE_ORDER_STATUS -> throw IllegalStateException("Invalid action. Use suspendable updateOrderStatus(..) directly") + WCOrderAction.SEARCH_ORDERS -> searchOrders(action.payload as SearchOrdersPayload) WCOrderAction.FETCH_ORDER_STATUS_OPTIONS -> fetchOrderStatusOptions(action.payload as FetchOrderStatusOptionsPayload) @@ -547,10 +548,13 @@ class WCOrderStore @Inject internal constructor( WCOrderAction.FETCHED_ORDERS -> handleFetchOrdersCompleted(action.payload as FetchOrdersResponsePayload) WCOrderAction.FETCHED_ORDER_LIST -> handleFetchOrderListCompleted(action.payload as FetchOrderListResponsePayload) + WCOrderAction.FETCHED_ORDERS_BY_IDS -> handleFetchOrderByIdsCompleted(action.payload as FetchOrdersByIdsResponsePayload) + WCOrderAction.FETCHED_ORDERS_COUNT -> handleFetchOrdersCountCompleted(action.payload as FetchOrdersCountResponsePayload) + WCOrderAction.SEARCHED_ORDERS -> handleSearchOrdersCompleted(action.payload as SearchOrdersResponsePayload) WCOrderAction.FETCHED_ORDER_STATUS_OPTIONS -> handleFetchOrderStatusOptionsCompleted(action.payload as FetchOrderStatusOptionsResponsePayload) @@ -569,8 +573,7 @@ class WCOrderStore @Inject internal constructor( private fun fetchOrderList(payload: FetchOrderListPayload) { wcOrderRestClient.fetchOrderListSummaries( listDescriptor = payload.listDescriptor, - offset = payload.offset, - useAppPasswordsForJetpackSites = payload.useAppPasswordsForJetpackSites + offset = payload.offset ) } @@ -592,7 +595,7 @@ class WCOrderStore @Inject internal constructor( return coroutineEngine.withDefaultContext(API, this, "checkIfHasOrders") { val result = wcOrderRestClient.fetchOrderCountSync(site, filterByStatus) return@withDefaultContext if (result.isError) { - OrdersCountResult.Failure(result.error) + OrdersCountResult.Failure(result.error) } else { OrdersCountResult.Success(result.count) } @@ -972,7 +975,9 @@ class WCOrderStore @Inject internal constructor( ) ) - mDispatcher.dispatch(ListActionBuilder.newFetchedListItemsAction(FetchedListItemsPayload( + mDispatcher.dispatch( + ListActionBuilder.newFetchedListItemsAction( + FetchedListItemsPayload( listDescriptor = payload.listDescriptor, remoteItemIds = payload.orderSummaries.map { it.orderId.value }, loadedMore = payload.loadedMore, @@ -1054,12 +1059,12 @@ class WCOrderStore @Inject internal constructor( mDispatcher.dispatch( ListActionBuilder.newListDataFailureAction( - OnListDataFailure(listTypeIdentifier).apply { - error = ListError( - errorType, - payload.error.message - ) - } + OnListDataFailure(listTypeIdentifier).apply { + error = ListError( + errorType, + payload.error.message + ) + } ) ) } diff --git a/libs/fluxc-plugin/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/OrderRestClientTest.kt b/libs/fluxc-plugin/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/OrderRestClientTest.kt index ec5f63805f90..02f6c74324a9 100644 --- a/libs/fluxc-plugin/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/OrderRestClientTest.kt +++ b/libs/fluxc-plugin/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/order/OrderRestClientTest.kt @@ -16,7 +16,6 @@ import org.wordpress.android.fluxc.generated.endpoint.WOOCOMMERCE import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.model.WCOrderListDescriptor import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse -import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooExperimentalNetwork import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooNetwork import org.wordpress.android.fluxc.tools.CoroutineEngine import org.wordpress.android.fluxc.utils.initCoroutineEngine @@ -24,7 +23,6 @@ import org.wordpress.android.fluxc.utils.initCoroutineEngine @OptIn(ExperimentalCoroutinesApi::class) class OrderRestClientTest { private val wooNetwork: WooNetwork = mock() - private val wooExperimentalNetwork: WooExperimentalNetwork = mock() private val dispatcher: Dispatcher = mock() private val orderDtoMapper: OrderDtoMapper = mock() private val coroutineEngine: CoroutineEngine = initCoroutineEngine() @@ -38,7 +36,6 @@ class OrderRestClientTest { dispatcher = dispatcher, orderDtoMapper = orderDtoMapper, wooNetwork = wooNetwork, - wooExperimentalNetwork = wooExperimentalNetwork, coroutineEngine = coroutineEngine ) } @@ -75,7 +72,7 @@ class OrderRestClientTest { ) // When - orderRestClient.fetchOrderListSummaries(listDescriptor, 0, false) + orderRestClient.fetchOrderListSummaries(listDescriptor, 0) // Then val paramsCaptor = argumentCaptor>() @@ -119,7 +116,7 @@ class OrderRestClientTest { ) // When - orderRestClient.fetchOrderListSummaries(listDescriptor, 0, false) + orderRestClient.fetchOrderListSummaries(listDescriptor, 0) // Then val paramsCaptor = argumentCaptor>() @@ -163,7 +160,7 @@ class OrderRestClientTest { ) // When - orderRestClient.fetchOrderListSummaries(listDescriptor, 0, false) + orderRestClient.fetchOrderListSummaries(listDescriptor, 0) // Then val paramsCaptor = argumentCaptor>() From 3444de8797591f93f930c64334b45f80fb3779be Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 18:02:13 +0100 Subject: [PATCH 06/22] Force using Jetpack tunnel when feature is disabled --- .../fluxc/network/rest/wpcom/wc/WooNetwork.kt | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt index 36bbdae46778..9943314c3d1b 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt @@ -6,6 +6,7 @@ import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.network.rest.wpapi.WPAPINetwork 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.ApplicationPasswordsStore import org.wordpress.android.fluxc.network.rest.wpcom.JetpackTunnelWPAPINetwork @@ -17,13 +18,16 @@ import javax.inject.Singleton /** * The Woo app supports connecting to sites using either Jetpack or the site credentials (technically application * passwords). This class allows this by supporting multiple networking implementations depending on the type of site: - * - Jetpack Sites: the API call will use Jetpack Tunnel using [JetpackTunnelWPAPINetwork] + * - Jetpack Sites: + * - When enabled and if the site supports it, the API call will use [ApplicationPasswordsNetwork] + * - Otherwise, it will use [JetpackTunnelWPAPINetwork] * - Non-Jetpack Sites: the API call will use Application Passwords using [ApplicationPasswordsNetwork] * * The [SiteModel.ORIGIN_XMLRPC] support is kept for backward compatibility */ @Singleton class WooNetwork @Inject constructor( + private val applicationPasswordsConfiguration: ApplicationPasswordsConfiguration, private val applicationPasswordsNetwork: ApplicationPasswordsNetwork, private val jetpackTunnelWPAPINetwork: JetpackTunnelWPAPINetwork, private val applicationPasswordsStore: ApplicationPasswordsStore, @@ -122,11 +126,13 @@ class WooNetwork @Inject constructor( site: SiteModel, request: suspend WPAPINetwork.() -> WPAPIResponse ): WPAPIResponse { - if (!site.isApplicationPasswordsSupported) { - AppLog.v( - AppLog.T.API, - "Application Passwords not supported for site: ${site.url}, use Jetpack Tunnel" - ) + if (!applicationPasswordsConfiguration.isEnabledForJetpack || !site.isApplicationPasswordsSupported) { + if (applicationPasswordsConfiguration.isEnabledForJetpack) { + AppLog.v( + AppLog.T.API, + "Application Passwords not supported for site: ${site.url}, use Jetpack Tunnel" + ) + } return jetpackTunnelWPAPINetwork.request().copyWith( networkingMode = WPAPINetworkingMode.JetpackTunnel() ) From aeb3d6b01e3f3b7c301533ab0fdbc48afed38abd Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 18:10:01 +0100 Subject: [PATCH 07/22] Remove unused argument --- .../android/ui/orders/list/OrderListItemDataSource.kt | 3 +-- .../kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListItemDataSource.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListItemDataSource.kt index 8d0d7e9c93db..9ea080ffeda6 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListItemDataSource.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListItemDataSource.kt @@ -195,8 +195,7 @@ class OrderListItemDataSource @Inject constructor( override fun fetchList(listDescriptor: WCOrderListDescriptor, offset: Long) { val fetchOrderListPayload = FetchOrderListPayload( listDescriptor = listDescriptor, - offset = offset, - useAppPasswordsForJetpackSites = remoteConfigRepository.isJetpackAppPasswordsExperimentEnabled() + offset = offset ) dispatcher.dispatch(WCOrderActionBuilder.newFetchOrderListAction(fetchOrderListPayload)) } diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt index 5d20efec9df1..5f31bce5390c 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/WCOrderStore.kt @@ -84,8 +84,7 @@ class WCOrderStore @Inject internal constructor( class FetchOrderListPayload( val listDescriptor: WCOrderListDescriptor, - val offset: Long, - val useAppPasswordsForJetpackSites: Boolean + val offset: Long ) : Payload() class FetchOrdersByIdsPayload( From 7df90717596bcd72d586104d644ca6bc2f54f7f7 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 18:10:15 +0100 Subject: [PATCH 08/22] Remove unused remote config key --- .../android/config/FirebaseRemoteConfigRepository.kt | 12 +----------- .../android/config/RemoteConfigRepository.kt | 2 -- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/config/FirebaseRemoteConfigRepository.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/config/FirebaseRemoteConfigRepository.kt index 7b5b9d20f420..dc843451b652 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/config/FirebaseRemoteConfigRepository.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/config/FirebaseRemoteConfigRepository.kt @@ -21,8 +21,6 @@ class FirebaseRemoteConfigRepository @Inject constructor( companion object { private const val DEBUG_INTERVAL = 10L private const val RELEASE_INTERVAL = 31200L - - const val KEY_ENABLE_JETPACK_APP_PASSWORDS_EXPERIMENT = "wcandroid_enable_jetpack_app_passwords_experiment_2" } private val minimumFetchIntervalInSeconds = @@ -37,11 +35,7 @@ class FirebaseRemoteConfigRepository @Inject constructor( private val _fetchStatus = MutableStateFlow(RemoteConfigFetchStatus.Pending) override val fetchStatus: Flow = _fetchStatus.asStateFlow() - private val defaultValues by lazy { - mapOf( - KEY_ENABLE_JETPACK_APP_PASSWORDS_EXPERIMENT to false.toString() - ) - } + private val defaultValues = emptyMap() init { remoteConfig.apply { @@ -73,8 +67,4 @@ class FirebaseRemoteConfigRepository @Inject constructor( @VisibleForTesting fun observeStringRemoteValue(key: String) = changesTrigger .map { remoteConfig.getString(key) } - - override fun isJetpackAppPasswordsExperimentEnabled(): Boolean { - return remoteConfig.getBoolean(KEY_ENABLE_JETPACK_APP_PASSWORDS_EXPERIMENT) - } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/config/RemoteConfigRepository.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/config/RemoteConfigRepository.kt index d77bf3b3be66..38c2656d0d62 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/config/RemoteConfigRepository.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/config/RemoteConfigRepository.kt @@ -5,8 +5,6 @@ import kotlinx.coroutines.flow.Flow interface RemoteConfigRepository { val fetchStatus: Flow fun fetchRemoteConfig() - - fun isJetpackAppPasswordsExperimentEnabled(): Boolean } enum class RemoteConfigFetchStatus { From 65d2f1964adf90505a2388fc3c2d732791f1a7aa Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 18:44:29 +0100 Subject: [PATCH 09/22] Remove old code related to the initial experiment --- .../android/fluxc/network/rest/wpcom/wc/WooNetwork.kt | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt index 9943314c3d1b..8aaa86f1bb91 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt @@ -8,7 +8,6 @@ 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.ApplicationPasswordsStore import org.wordpress.android.fluxc.network.rest.wpcom.JetpackTunnelWPAPINetwork import org.wordpress.android.fluxc.persistence.SiteSqlUtils import org.wordpress.android.util.AppLog @@ -30,7 +29,6 @@ class WooNetwork @Inject constructor( private val applicationPasswordsConfiguration: ApplicationPasswordsConfiguration, private val applicationPasswordsNetwork: ApplicationPasswordsNetwork, private val jetpackTunnelWPAPINetwork: JetpackTunnelWPAPINetwork, - private val applicationPasswordsStore: ApplicationPasswordsStore, private val siteSqlUtils: SiteSqlUtils ) : WPAPINetwork { override suspend fun executeGetGsonRequest( @@ -138,8 +136,6 @@ class WooNetwork @Inject constructor( ) } - val hasAppPassword = applicationPasswordsStore.hasCredentials(site) - return when (val appPasswordsResponse = applicationPasswordsNetwork.request()) { is WPAPIResponse.Success<*> -> { AppLog.v( @@ -147,17 +143,14 @@ class WooNetwork @Inject constructor( "Request successful for site: ${site.url}, using Application Passwords" ) (appPasswordsResponse as WPAPIResponse).copyWith( - // When creating a new Application Password, we don't want to track this request, as its duration - // is not relevant to our experiment. - // So we track only requests where we already have a password saved. - networkingMode = if (hasAppPassword) WPAPINetworkingMode.ApplicationPasswordsWithJetpack else null + networkingMode = WPAPINetworkingMode.ApplicationPasswordsWithJetpack ) } is WPAPIResponse.Error<*> -> { AppLog.w( AppLog.T.API, - "Request failed for site: ${site.url} using Application Passwords, falling back to Jetpack Tunnel" + "Request to $ using Application Passwords, falling back to Jetpack Tunnel" ) if (appPasswordsResponse.error.errorCode == ApplicationPasswordsNetwork.APPLICATION_PASSWORDS_NOT_SUPPORT_ERROR_CODE From edc6d45f34c62ecd540cacef4215394776082191 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 19:31:38 +0100 Subject: [PATCH 10/22] Improve log message for failed requests --- .../fluxc/network/rest/wpcom/wc/WooNetwork.kt | 40 +++++++++++++------ .../android/fluxc/model/SiteModel.java | 2 +- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt index 8aaa86f1bb91..5a39fdd55836 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt @@ -4,6 +4,7 @@ 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 @@ -41,7 +42,7 @@ class WooNetwork @Inject constructor( forced: Boolean, requestTimeout: Int, retries: Int - ): WPAPIResponse = handleRequest(site) { + ): WPAPIResponse = handleRequest(site, RequestContext(path, "GET")) { executeGetGsonRequest( site = site, path = path, @@ -60,7 +61,7 @@ class WooNetwork @Inject constructor( path: String, clazz: Class, body: Map - ): WPAPIResponse = handleRequest(site) { + ): WPAPIResponse = handleRequest(site, RequestContext(path, "POST")) { executePostGsonRequest( site = site, path = path, @@ -74,7 +75,7 @@ class WooNetwork @Inject constructor( path: String, clazz: Class, body: Map - ): WPAPIResponse = handleRequest(site) { + ): WPAPIResponse = handleRequest(site, RequestContext(path, "PUT")) { executePutGsonRequest( site = site, path = path, @@ -88,7 +89,7 @@ class WooNetwork @Inject constructor( path: String, clazz: Class, params: Map - ): WPAPIResponse = handleRequest(site) { + ): WPAPIResponse = handleRequest(site, RequestContext(path, "DELETE")) { executeDeleteGsonRequest( site = site, path = path, @@ -99,6 +100,7 @@ class WooNetwork @Inject constructor( private suspend fun handleRequest( site: SiteModel, + requestContext: RequestContext, request: suspend WPAPINetwork.() -> WPAPIResponse ): WPAPIResponse { return when (site.origin) { @@ -109,7 +111,7 @@ class WooNetwork @Inject constructor( } SiteModel.ORIGIN_WPCOM_REST -> { - handleRequestForJetpackSites(site) { + handleRequestForJetpackSites(site, requestContext) { request() } } @@ -122,6 +124,7 @@ class WooNetwork @Inject constructor( private suspend fun handleRequestForJetpackSites( site: SiteModel, + requestContext: RequestContext, request: suspend WPAPINetwork.() -> WPAPIResponse ): WPAPIResponse { if (!applicationPasswordsConfiguration.isEnabledForJetpack || !site.isApplicationPasswordsSupported) { @@ -138,20 +141,14 @@ class WooNetwork @Inject constructor( return when (val appPasswordsResponse = applicationPasswordsNetwork.request()) { is WPAPIResponse.Success<*> -> { - AppLog.v( - AppLog.T.API, - "Request successful for site: ${site.url}, using Application Passwords" - ) (appPasswordsResponse as WPAPIResponse).copyWith( networkingMode = WPAPINetworkingMode.ApplicationPasswordsWithJetpack ) } is WPAPIResponse.Error<*> -> { - AppLog.w( - AppLog.T.API, - "Request to $ using Application Passwords, falling back to Jetpack Tunnel" - ) + logMessageForFailedRequest(requestContext, site.url, appPasswordsResponse.error) + if (appPasswordsResponse.error.errorCode == ApplicationPasswordsNetwork.APPLICATION_PASSWORDS_NOT_SUPPORT_ERROR_CODE ) { @@ -178,4 +175,21 @@ class WooNetwork @Inject constructor( is WPAPIResponse.Error -> this.copy(networkingMode = networkingMode) } } + + private fun logMessageForFailedRequest(requestContext: RequestContext, siteUrl: String, error: WPAPINetworkError) { + AppLog.w( + AppLog.T.API, + "Request failed using Application Passwords for Jetpack Site,\n" + + "site: ${siteUrl},\n" + + "path: ${requestContext.path},\n" + + "method: ${requestContext.method},\n" + + "error: HTTP status code ${error.volleyError.networkResponse?.statusCode}, " + + "error message: ${error.errorCode?.ifEmpty { null } ?: error.message}", + ) + } + + private data class RequestContext( + val path: String, + val method: String + ) } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/SiteModel.java b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/SiteModel.java index 8c75bd3950ae..358305b78c4c 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/SiteModel.java +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/SiteModel.java @@ -1090,7 +1090,7 @@ public void setApplicationPasswordsAuthorizeUrl(String applicationPasswordsAutho public boolean isApplicationPasswordsSupported() { return mApplicationPasswordsAuthorizeUrl != null && -!mApplicationPasswordsAuthorizeUrl.isEmpty(); + !mApplicationPasswordsAuthorizeUrl.isEmpty(); } public int getPublishedStatus() { From 90d18fb63294c62cd5bc65a5dd1c5ae01791c406 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Sat, 30 Aug 2025 19:31:48 +0100 Subject: [PATCH 11/22] Add a feature flag for the feature --- .../WooApplicationPasswordsConfiguration.kt | 4 +++- .../main/kotlin/com/woocommerce/android/util/FeatureFlag.kt | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt index 64019357be30..b8e65a1c0f2f 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt @@ -2,12 +2,14 @@ package com.woocommerce.android.applicationpasswords import com.woocommerce.android.BuildConfig import com.woocommerce.android.util.DeviceInfo +import com.woocommerce.android.util.FeatureFlag.APP_PASSWORDS_FOR_JETPACK_SITES import jakarta.inject.Inject import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration class WooApplicationPasswordsConfiguration @Inject constructor() : ApplicationPasswordsConfiguration { override val isEnabledForSiteCredentials: Boolean = true - override val isEnabledForJetpack: Boolean = false + override val isEnabledForJetpack: Boolean + get() = APP_PASSWORDS_FOR_JETPACK_SITES.isEnabled() override val applicationName: String = "${BuildConfig.APPLICATION_ID}.app-client.${DeviceInfo.name.replace(' ', '-')}" } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/util/FeatureFlag.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/util/FeatureFlag.kt index a59b54b144c3..ef6a016fbe76 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/util/FeatureFlag.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/util/FeatureFlag.kt @@ -16,7 +16,8 @@ enum class FeatureFlag { WOO_POS_HISTORICAL_ORDERS_M1, WOO_POS_LOCAL_CATALOG_M1, HIDE_SITES_FROM_SITE_PICKER, - AI_PRODUCT_IMAGE_BACKGROUND_REMOVAL; + AI_PRODUCT_IMAGE_BACKGROUND_REMOVAL, + APP_PASSWORDS_FOR_JETPACK_SITES; fun isEnabled(context: Context? = null): Boolean { return when (this) { @@ -29,7 +30,8 @@ enum class FeatureFlag { BETTER_CUSTOMER_SEARCH_M2, ORDER_CREATION_AUTO_TAX_RATE, AI_PRODUCT_IMAGE_BACKGROUND_REMOVAL, - WOO_POS_LOCAL_CATALOG_M1 -> PackageUtils.isDebugBuild() + WOO_POS_LOCAL_CATALOG_M1, + APP_PASSWORDS_FOR_JETPACK_SITES -> PackageUtils.isDebugBuild() NEW_SHIPPING_SUPPORT, BULK_UPDATE_ORDERS_STATUS, From a4424122c25f7e2715c5b11421db7a46d3600b85 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Mon, 1 Sep 2025 19:05:04 +0100 Subject: [PATCH 12/22] Minor refactoring for the configuration class - Renames the functions for better clarity. - Make `isEnabledForJetpackAccess` suspendable to be ready for remote feature flag access --- .../WooApplicationPasswordsConfiguration.kt | 6 +++--- .../fluxc/network/rest/wpcom/wc/WooNetwork.kt | 5 +++-- .../ApplicationPasswordsConfiguration.kt | 14 ++++++++++++-- .../wordpress/android/fluxc/store/MediaStore.java | 6 +++--- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt index b8e65a1c0f2f..841ae210d99c 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt @@ -7,9 +7,9 @@ import jakarta.inject.Inject import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration class WooApplicationPasswordsConfiguration @Inject constructor() : ApplicationPasswordsConfiguration { - override val isEnabledForSiteCredentials: Boolean = true - override val isEnabledForJetpack: Boolean - get() = APP_PASSWORDS_FOR_JETPACK_SITES.isEnabled() override val applicationName: String = "${BuildConfig.APPLICATION_ID}.app-client.${DeviceInfo.name.replace(' ', '-')}" + + override fun isEnabledForDirectAccess(): Boolean = true + override suspend fun isEnabledForJetpackAccess(): Boolean = APP_PASSWORDS_FOR_JETPACK_SITES.isEnabled() } diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt index 5a39fdd55836..6c5aa47891ea 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt @@ -127,8 +127,9 @@ class WooNetwork @Inject constructor( requestContext: RequestContext, request: suspend WPAPINetwork.() -> WPAPIResponse ): WPAPIResponse { - if (!applicationPasswordsConfiguration.isEnabledForJetpack || !site.isApplicationPasswordsSupported) { - if (applicationPasswordsConfiguration.isEnabledForJetpack) { + val appPasswordsEnabled = applicationPasswordsConfiguration.isEnabledForJetpackAccess() + if (!appPasswordsEnabled || !site.isApplicationPasswordsSupported) { + if (appPasswordsEnabled) { AppLog.v( AppLog.T.API, "Application Passwords not supported for site: ${site.url}, use Jetpack Tunnel" diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt index 701bbcadb8ff..4a743aceb231 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt @@ -1,7 +1,17 @@ package org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords interface ApplicationPasswordsConfiguration { - val isEnabledForSiteCredentials: Boolean - val isEnabledForJetpack: Boolean val applicationName: String + + /** + * Returns true if the Application Passwords feature is enabled for direct site access. + * This applies when sites are accessed using their URL directly (not through Jetpack). + */ + fun isEnabledForDirectAccess(): Boolean + + /** + * Returns true if the Application Passwords feature is enabled for Jetpack-mediated access. + * This applies when sites are accessed through Jetpack, even if they are self-hosted. + */ + suspend fun isEnabledForJetpackAccess(): Boolean } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java index 0c174ca84b59..4f7d64630628 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java @@ -801,7 +801,7 @@ private void performUploadMedia(@NonNull UploadMediaPayload payload) { if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) { mWPComV2MediaRestClient.uploadMedia(payload.site, payload.media); } else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI - && mApplicationPasswordsConfiguration.isEnabledForSiteCredentials()) { + && mApplicationPasswordsConfiguration.isEnabledForDirectAccess()) { mApplicationPasswordsMediaRestClient.uploadMedia(payload.site, payload.media); } else { mMediaXmlrpcClient.uploadMedia(payload.site, payload.media); @@ -823,7 +823,7 @@ private void performFetchMediaList(@NonNull FetchMediaListPayload payload) { if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) { mWPComV2MediaRestClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType); } else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI - && mApplicationPasswordsConfiguration.isEnabledForSiteCredentials()) { + && mApplicationPasswordsConfiguration.isEnabledForDirectAccess()) { mApplicationPasswordsMediaRestClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType); } else { mMediaXmlrpcClient.fetchMediaList(payload.site, payload.number, offset, payload.mimeType); @@ -869,7 +869,7 @@ private void performCancelUpload(@NonNull CancelMediaPayload payload) { if (payload.site.getOrigin() == SiteModel.ORIGIN_WPCOM_REST) { mWPComV2MediaRestClient.cancelUpload(media); } else if (payload.site.getOrigin() == SiteModel.ORIGIN_WPAPI - && mApplicationPasswordsConfiguration.isEnabledForSiteCredentials()) { + && mApplicationPasswordsConfiguration.isEnabledForDirectAccess()) { mApplicationPasswordsMediaRestClient.cancelUpload(media); } else { mMediaXmlrpcClient.cancelUpload(media); From 60312f7476437139d4a7ea037f5101bdcf4f8b72 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Mon, 1 Sep 2025 19:07:01 +0100 Subject: [PATCH 13/22] Make `isEnabledForDirectAccess` enabled by default --- .../WooApplicationPasswordsConfiguration.kt | 1 - .../applicationpasswords/ApplicationPasswordsConfiguration.kt | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt index 841ae210d99c..72fe056fe65f 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt @@ -10,6 +10,5 @@ class WooApplicationPasswordsConfiguration @Inject constructor() : ApplicationPa override val applicationName: String = "${BuildConfig.APPLICATION_ID}.app-client.${DeviceInfo.name.replace(' ', '-')}" - override fun isEnabledForDirectAccess(): Boolean = true override suspend fun isEnabledForJetpackAccess(): Boolean = APP_PASSWORDS_FOR_JETPACK_SITES.isEnabled() } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt index 4a743aceb231..feaa4274ad6c 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsConfiguration.kt @@ -7,7 +7,7 @@ interface ApplicationPasswordsConfiguration { * Returns true if the Application Passwords feature is enabled for direct site access. * This applies when sites are accessed using their URL directly (not through Jetpack). */ - fun isEnabledForDirectAccess(): Boolean + fun isEnabledForDirectAccess(): Boolean = true /** * Returns true if the Application Passwords feature is enabled for Jetpack-mediated access. From 043d3dca5fae50893bde2200506cafc0190e5f7c Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Mon, 1 Sep 2025 19:05:15 +0100 Subject: [PATCH 14/22] Update unit tests --- .../LoginSiteCredentialsViewModelTest.kt | 72 ++++++++++--------- .../fluxc/network/rest/wpcom/wc/WooNetwork.kt | 2 +- ...mentalNetworkTest.kt => WooNetworkTest.kt} | 54 ++++++++++++-- .../ApplicationPasswordManagerTests.kt | 6 +- 4 files changed, 91 insertions(+), 43 deletions(-) rename libs/fluxc-tests/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/{WooExperimentalNetworkTest.kt => WooNetworkTest.kt} (74%) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModelTest.kt index e310699ca7d0..17f55c1e206f 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModelTest.kt @@ -39,6 +39,7 @@ import org.wordpress.android.fluxc.network.BaseRequest.BaseNetworkError import org.wordpress.android.fluxc.network.BaseRequest.GenericErrorType import org.wordpress.android.fluxc.network.rest.wpapi.Nonce import org.wordpress.android.fluxc.network.rest.wpapi.WPAPINetworkError +import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration import org.wordpress.android.login.LoginAnalyticsListener @ExperimentalCoroutinesApi @@ -95,7 +96,10 @@ class LoginSiteCredentialsViewModelTest : BaseUnitTest() { applicationPasswordsNotifier = applicationPasswordsNotifier, analyticsTracker = analyticsTracker, appPrefs = appPrefs, - applicationPasswordsClientId = clientId, + applicationPasswordsConfiguration = object : ApplicationPasswordsConfiguration { + override val applicationName: String = clientId + override suspend fun isEnabledForJetpackAccess(): Boolean = true + }, resourceProvider = resourceProvider ) } @@ -292,22 +296,23 @@ class LoginSiteCredentialsViewModelTest : BaseUnitTest() { } @Test - fun `given application pwd disabled and wp-login-php accessible, when submitting login, then show error screen`() = testBlocking { - setup { - whenever(wpApiSiteRepository.checkIfUserIsEligible(testSite)).thenReturn(Result.failure(Exception())) - } - - viewModel.viewState.observeForTesting { - viewModel.onUsernameChanged(testUsername) - viewModel.onPasswordChanged(testPassword) - viewModel.onContinueClick() - applicationPasswordsUnavailableEvents.tryEmit(mock()) + fun `given application pwd disabled and wp-login-php accessible, when submitting login, then show error screen`() = + testBlocking { + setup { + whenever(wpApiSiteRepository.checkIfUserIsEligible(testSite)).thenReturn(Result.failure(Exception())) + } + + viewModel.viewState.observeForTesting { + viewModel.onUsernameChanged(testUsername) + viewModel.onPasswordChanged(testPassword) + viewModel.onContinueClick() + applicationPasswordsUnavailableEvents.tryEmit(mock()) + } + + assertThat(viewModel.event.value) + .isEqualTo(ShowApplicationPasswordsUnavailableScreen(siteAddress, isJetpackConnected)) } - assertThat(viewModel.event.value) - .isEqualTo(ShowApplicationPasswordsUnavailableScreen(siteAddress, isJetpackConnected)) - } - @Test fun `give user role fetch fails, when submitting login, then show a snackbar`() = testBlocking { setup { @@ -353,23 +358,24 @@ class LoginSiteCredentialsViewModelTest : BaseUnitTest() { } @Test - fun `given application passwords enabled and login fails for an unknown reason, when user attempts to sign-in, then show WebView login flow`() = testBlocking { - setup { - whenever(wpApiSiteRepository.login(siteAddress, testUsername, testPassword)) - .thenReturn(Result.failure(Exception())) - whenever(wpApiSiteRepository.fetchSite(siteAddress)) - .thenReturn(Result.success(testSite.apply { applicationPasswordsAuthorizeUrl = urlAuthBase })) + fun `given application passwords enabled and login fails for an unknown reason, when user attempts to sign-in, then show WebView login flow`() = + testBlocking { + setup { + whenever(wpApiSiteRepository.login(siteAddress, testUsername, testPassword)) + .thenReturn(Result.failure(Exception())) + whenever(wpApiSiteRepository.fetchSite(siteAddress)) + .thenReturn(Result.success(testSite.apply { applicationPasswordsAuthorizeUrl = urlAuthBase })) + } + + val event = viewModel.event.runAndCaptureValues { + viewModel.onUsernameChanged(testUsername) + viewModel.onPasswordChanged(testPassword) + viewModel.viewState.getOrAwaitValue() + viewModel.onContinueClick() + }.last() + + assertThat(event).isInstanceOf(LoginSiteCredentialsViewModel.ShowApplicationPasswordTutorialScreen::class.java) + assertThat((event as LoginSiteCredentialsViewModel.ShowApplicationPasswordTutorialScreen).url) + .isEqualTo(urlAuthFull) } - - val event = viewModel.event.runAndCaptureValues { - viewModel.onUsernameChanged(testUsername) - viewModel.onPasswordChanged(testPassword) - viewModel.viewState.getOrAwaitValue() - viewModel.onContinueClick() - }.last() - - assertThat(event).isInstanceOf(LoginSiteCredentialsViewModel.ShowApplicationPasswordTutorialScreen::class.java) - assertThat((event as LoginSiteCredentialsViewModel.ShowApplicationPasswordTutorialScreen).url) - .isEqualTo(urlAuthFull) - } } diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt index 6c5aa47891ea..743eb3d901be 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt @@ -184,7 +184,7 @@ class WooNetwork @Inject constructor( "site: ${siteUrl},\n" + "path: ${requestContext.path},\n" + "method: ${requestContext.method},\n" + - "error: HTTP status code ${error.volleyError.networkResponse?.statusCode}, " + + "error: HTTP status code ${error.volleyError?.networkResponse?.statusCode}, " + "error message: ${error.errorCode?.ifEmpty { null } ?: error.message}", ) } diff --git a/libs/fluxc-tests/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooExperimentalNetworkTest.kt b/libs/fluxc-tests/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetworkTest.kt similarity index 74% rename from libs/fluxc-tests/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooExperimentalNetworkTest.kt rename to libs/fluxc-tests/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetworkTest.kt index fc423654d309..206dfde7eeee 100644 --- a/libs/fluxc-tests/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooExperimentalNetworkTest.kt +++ b/libs/fluxc-tests/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetworkTest.kt @@ -11,25 +11,27 @@ 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.ApplicationPasswordsStore 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 WooExperimentalNetworkTest { - private val testSite = SiteModel() +class WooNetworkTest { + private val testSite = SiteModel().apply { + url = "https://example.com" + } private val testPath = "path" + private val applicationPasswordsConfiguration = FakeApplicationPasswordsConfiguration() private val applicationPasswordsNetwork: ApplicationPasswordsNetwork = mock() private val jetpackTunnelWPAPINetwork: JetpackTunnelWPAPINetwork = mock() - private val applicationPasswordsStore: ApplicationPasswordsStore = mock() private val siteSqlUtils: SiteSqlUtils = mock() - private val sut = WooExperimentalNetwork( + private val sut = WooNetwork( + applicationPasswordsConfiguration = applicationPasswordsConfiguration, applicationPasswordsNetwork = applicationPasswordsNetwork, jetpackTunnelWPAPINetwork = jetpackTunnelWPAPINetwork, - applicationPasswordsStore = applicationPasswordsStore, siteSqlUtils = siteSqlUtils ) @@ -59,7 +61,14 @@ class WooExperimentalNetworkTest { test { testSite.origin = SiteModel.ORIGIN_WPCOM_REST testSite.applicationPasswordsAuthorizeUrl = "authorize_url" - givenAppPasswordsResponse(WPAPIResponse.Error(WPAPINetworkError(mock(), "error"))) + givenAppPasswordsResponse( + WPAPIResponse.Error( + WPAPINetworkError( + mock(), + "error" + ) + ) + ) val sampleResponse = SampleResponse("value") givenJetpackTunnelResponse(WPAPIResponse.Success(sampleResponse, emptyList())) @@ -89,6 +98,28 @@ class WooExperimentalNetworkTest { assertThat((response as WPAPIResponse.Success).data).isEqualTo(sampleResponse) } + @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" + val sampleResponse = SampleResponse("value") + givenJetpackTunnelResponse(WPAPIResponse.Success(sampleResponse, emptyList())) + + val response = sut.executeGetGsonRequest( + site = testSite, + path = testPath, + clazz = SampleResponse::class.java + ) + + assertThat((response as WPAPIResponse.Success).data).isEqualTo(sampleResponse) + verify(applicationPasswordsNetwork, never()).executeGetGsonRequest( + site = testSite, + path = testPath, + clazz = SampleResponse::class.java + ) + } + @Test fun `when detecting that a site doesn't support app passwords, then update cached site with correct status`() = test { @@ -139,4 +170,13 @@ class WooExperimentalNetworkTest { } private data class SampleResponse(val value: String) + + private class FakeApplicationPasswordsConfiguration : ApplicationPasswordsConfiguration { + var isEnabledForJetpackAccessValue: Boolean = true + + override val applicationName: String + get() = "WooCommerce" + + override suspend fun isEnabledForJetpackAccess(): Boolean = isEnabledForJetpackAccessValue + } } diff --git a/libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordManagerTests.kt b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordManagerTests.kt index 1c6c27e18751..2f691d359328 100644 --- a/libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordManagerTests.kt +++ b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordManagerTests.kt @@ -14,7 +14,6 @@ import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.network.BaseRequest.BaseNetworkError import org.wordpress.android.fluxc.network.BaseRequest.GenericErrorType import org.wordpress.android.fluxc.network.rest.wpcom.WPComGsonRequest.WPComGsonNetworkError -import java.util.* import kotlin.test.assertEquals @ExperimentalCoroutinesApi @@ -34,7 +33,10 @@ class ApplicationPasswordManagerTests { private val mJetpackApplicationPasswordsRestClient: JetpackApplicationPasswordsRestClient = mock() private val mWpApiApplicationPasswordsRestClient: WPApiApplicationPasswordsRestClient = mock() - private val applicationPasswordsConfiguration = ApplicationPasswordsConfiguration(Optional.of(applicationName)) + private val applicationPasswordsConfiguration = object : ApplicationPasswordsConfiguration { + override val applicationName: String = this@ApplicationPasswordManagerTests.applicationName + override suspend fun isEnabledForJetpackAccess() = true + } private lateinit var mApplicationPasswordsManager: ApplicationPasswordsManager From de41eb55ebf530fb64d6794299e82991c6ab2a2e Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Mon, 1 Sep 2025 19:07:49 +0100 Subject: [PATCH 15/22] Fix detekt issues --- .../android/ui/orders/list/OrderListItemDataSource.kt | 4 +--- .../android/fluxc/network/rest/wpcom/wc/WooNetwork.kt | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListItemDataSource.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListItemDataSource.kt index 9ea080ffeda6..e147e19c019d 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListItemDataSource.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListItemDataSource.kt @@ -1,7 +1,6 @@ package com.woocommerce.android.ui.orders.list import com.woocommerce.android.R -import com.woocommerce.android.config.RemoteConfigRepository import com.woocommerce.android.extensions.getBillingName import com.woocommerce.android.model.TimeGroup import com.woocommerce.android.model.TimeGroup.GROUP_FUTURE @@ -45,8 +44,7 @@ class OrderListItemDataSource @Inject constructor( private val networkStatus: NetworkStatus, private val fetcher: FetchOrdersRepository, private val resourceProvider: ResourceProvider, - private val dateUtils: DateUtils, - private val remoteConfigRepository: RemoteConfigRepository + private val dateUtils: DateUtils ) : ListItemDataSourceInterface { override fun getItemsAndFetchIfNecessary( listDescriptor: WCOrderListDescriptor, diff --git a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt index 743eb3d901be..a87077de7fa5 100644 --- a/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt +++ b/libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooNetwork.kt @@ -181,7 +181,7 @@ class WooNetwork @Inject constructor( AppLog.w( AppLog.T.API, "Request failed using Application Passwords for Jetpack Site,\n" + - "site: ${siteUrl},\n" + + "site: $siteUrl,\n" + "path: ${requestContext.path},\n" + "method: ${requestContext.method},\n" + "error: HTTP status code ${error.volleyError?.networkResponse?.statusCode}, " + From c7f9c85f7b12d3edde7614399b8df6ea5462f647 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Mon, 1 Sep 2025 19:52:27 +0100 Subject: [PATCH 16/22] Fix wear app build issue --- .../wear/di/ApplicationPasswordsModule.kt | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 WooCommerce-Wear/src/main/java/com/woocommerce/android/wear/di/ApplicationPasswordsModule.kt diff --git a/WooCommerce-Wear/src/main/java/com/woocommerce/android/wear/di/ApplicationPasswordsModule.kt b/WooCommerce-Wear/src/main/java/com/woocommerce/android/wear/di/ApplicationPasswordsModule.kt new file mode 100644 index 000000000000..b8b819064155 --- /dev/null +++ b/WooCommerce-Wear/src/main/java/com/woocommerce/android/wear/di/ApplicationPasswordsModule.kt @@ -0,0 +1,24 @@ +package com.woocommerce.android.wear.di + +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent +import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration +import javax.inject.Inject + +@Module +@InstallIn(SingletonComponent::class) +interface ApplicationPasswordsModule { + @Binds + fun bindApplicationPasswordsConfiguration( + configuration: WooWearApplicationPasswordsConfiguration + ): ApplicationPasswordsConfiguration +} + +class WooWearApplicationPasswordsConfiguration @Inject constructor() : ApplicationPasswordsConfiguration { + override val applicationName: String = "" + + override fun isEnabledForDirectAccess(): Boolean = false + override suspend fun isEnabledForJetpackAccess(): Boolean = false +} From 8ea3d7f18e9da7831da89b0fffb27c91b32ebeb1 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Tue, 2 Sep 2025 11:23:21 +0100 Subject: [PATCH 17/22] Fix issue for Jetpack CP sites `isUsingWpComRestApi` was returning false for Jetpack CP sites, leading to always ignoring saved app passwords credentials, as for WPCom rest sites, `SiteModel#username` is always null. --- .../main/java/org/wordpress/android/fluxc/model/SiteModel.java | 2 +- .../wpapi/applicationpasswords/ApplicationPasswordsStore.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/SiteModel.java b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/SiteModel.java index 358305b78c4c..da5447908b34 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/SiteModel.java +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/SiteModel.java @@ -787,7 +787,7 @@ public void setOrigin(@SiteOrigin int origin) { } public boolean isUsingWpComRestApi() { - return isWPCom() || (isJetpackConnected() && getOrigin() == ORIGIN_WPCOM_REST); + return isWPCom() || getOrigin() == ORIGIN_WPCOM_REST; } public void setSpaceAvailable(long spaceAvailable) { diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsStore.kt index 63ee495030a8..43a94805978f 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/applicationpasswords/ApplicationPasswordsStore.kt @@ -39,7 +39,7 @@ class ApplicationPasswordsStore @Inject constructor( val uuid = encryptedPreferences.getString(site.uuidPrefKey, null) return when { - !site.isUsingWpComRestApi && site.username != username -> null + site.username != username && !site.isUsingWpComRestApi -> null username != null && password != null -> ApplicationPasswordCredentials( userName = username, From bd77057f9e1bc3febb2b2271db4290a382f83e4c Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Wed, 3 Sep 2025 11:36:08 +0100 Subject: [PATCH 18/22] Add preference key for the experimental toggle --- .../src/main/kotlin/com/woocommerce/android/AppPrefs.kt | 7 ++++++- .../kotlin/com/woocommerce/android/AppPrefsWrapper.kt | 2 ++ .../WooApplicationPasswordsConfiguration.kt | 8 ++++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt index 7230f5c253e5..cc7029b3caa1 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt @@ -133,7 +133,8 @@ object AppPrefs { CUSTOM_FIELDS_TOP_BANNER_DISMISSED, BLAZE_CAMPAIGN_SELECTED_OBJECTIVE, BLAZE_CAMPAIGN_OBJECTIVE_SWITCH_CHECKED, - IS_SITE_WPCOM_SUSPENDED + IS_SITE_WPCOM_SUSPENDED, + JETPACK_APP_PASSWORDS_ENABLED } /** @@ -296,6 +297,10 @@ object AppPrefs { get() = getBoolean(DeletablePrefKey.ORDER_SUMMARY_MIGRATED, false) set(value) = setBoolean(DeletablePrefKey.ORDER_SUMMARY_MIGRATED, value) + var jetpackAppPasswordsEnabled: Boolean + get() = getBoolean(DeletablePrefKey.JETPACK_APP_PASSWORDS_ENABLED, true) + set(value) = setBoolean(DeletablePrefKey.JETPACK_APP_PASSWORDS_ENABLED, value) + fun getProductSortingChoice(currentSiteId: Int) = getString(getProductSortingKey(currentSiteId)).orNullIfEmpty() fun setProductSortingChoice(currentSiteId: Int, value: String) { diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefsWrapper.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefsWrapper.kt index 7a75a5476227..2bf810cc6cbc 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefsWrapper.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefsWrapper.kt @@ -41,6 +41,8 @@ open class AppPrefsWrapper @Inject constructor() { open var orderSummaryMigrated by AppPrefs::orderSummaryMigrated + var jetpackAppPasswordsEnabled by AppPrefs::jetpackAppPasswordsEnabled + fun getAppInstallationDate() = AppPrefs.installationDate fun getReceiptUrl(localSiteId: Int, remoteSiteId: Long, selfHostedSiteId: Long, orderId: Long) = diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt index 72fe056fe65f..8de9a63f32b3 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/applicationpasswords/WooApplicationPasswordsConfiguration.kt @@ -1,14 +1,18 @@ package com.woocommerce.android.applicationpasswords +import com.woocommerce.android.AppPrefsWrapper import com.woocommerce.android.BuildConfig import com.woocommerce.android.util.DeviceInfo import com.woocommerce.android.util.FeatureFlag.APP_PASSWORDS_FOR_JETPACK_SITES import jakarta.inject.Inject import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordsConfiguration -class WooApplicationPasswordsConfiguration @Inject constructor() : ApplicationPasswordsConfiguration { +class WooApplicationPasswordsConfiguration @Inject constructor( + private val appPrefs: AppPrefsWrapper +) : ApplicationPasswordsConfiguration { override val applicationName: String = "${BuildConfig.APPLICATION_ID}.app-client.${DeviceInfo.name.replace(' ', '-')}" - override suspend fun isEnabledForJetpackAccess(): Boolean = APP_PASSWORDS_FOR_JETPACK_SITES.isEnabled() + override suspend fun isEnabledForJetpackAccess(): Boolean = + APP_PASSWORDS_FOR_JETPACK_SITES.isEnabled() && appPrefs.jetpackAppPasswordsEnabled } From 44044c92607ff71188b3bbe4f7de0d1a498fe09e Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Wed, 3 Sep 2025 13:33:33 +0100 Subject: [PATCH 19/22] Add a preference toggle to the experimental features screen --- .../android/ui/prefs/BetaFeaturesFragment.kt | 20 +++++++++++++++++++ .../res/layout/fragment_settings_beta.xml | 7 +++++++ WooCommerce/src/main/res/values/strings.xml | 2 ++ 3 files changed, 29 insertions(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/prefs/BetaFeaturesFragment.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/prefs/BetaFeaturesFragment.kt index 1f492a5d1c89..0947d31aaf0e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/prefs/BetaFeaturesFragment.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/prefs/BetaFeaturesFragment.kt @@ -3,6 +3,7 @@ package com.woocommerce.android.ui.prefs import android.os.Bundle import android.view.View import android.widget.CompoundButton +import androidx.core.view.isVisible import androidx.fragment.app.Fragment import com.google.android.material.snackbar.BaseTransientBottomBar import com.google.android.material.snackbar.Snackbar @@ -11,9 +12,13 @@ import com.woocommerce.android.R import com.woocommerce.android.analytics.AnalyticsEvent.PRODUCT_ADDONS_BETA_FEATURES_SWITCH_TOGGLED import com.woocommerce.android.analytics.AnalyticsTracker import com.woocommerce.android.databinding.FragmentSettingsBetaBinding +import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.tools.SiteConnectionType import com.woocommerce.android.ui.prefs.MainSettingsFragment.AppSettingsListener import com.woocommerce.android.util.AnalyticsUtils +import com.woocommerce.android.util.FeatureFlag import dagger.hilt.android.AndroidEntryPoint +import javax.inject.Inject @AndroidEntryPoint class BetaFeaturesFragment : Fragment(R.layout.fragment_settings_beta) { @@ -21,6 +26,9 @@ class BetaFeaturesFragment : Fragment(R.layout.fragment_settings_beta) { const val TAG = "beta-features" } + @Inject + lateinit var selectedSite: SelectedSite + private val settingsListener by lazy { activity as? AppSettingsListener } @@ -30,6 +38,7 @@ class BetaFeaturesFragment : Fragment(R.layout.fragment_settings_beta) { with(FragmentSettingsBetaBinding.bind(view)) { bindProductAddonsToggle() + bindJetpackAppPasswordsToggle() } } @@ -49,6 +58,17 @@ class BetaFeaturesFragment : Fragment(R.layout.fragment_settings_beta) { } } + private fun FragmentSettingsBetaBinding.bindJetpackAppPasswordsToggle() { + // TODO check the remote feature flag state once available + val isJetpackSite = selectedSite.connectionType != SiteConnectionType.ApplicationPasswords + jetpackAppPasswordsToggle.isVisible = FeatureFlag.APP_PASSWORDS_FOR_JETPACK_SITES.isEnabled() && isJetpackSite + + jetpackAppPasswordsToggle.isChecked = AppPrefs.jetpackAppPasswordsEnabled + jetpackAppPasswordsToggle.setOnCheckedChangeListener { _, isChecked -> + AppPrefs.jetpackAppPasswordsEnabled = isChecked + } + } + override fun onResume() { super.onResume() AnalyticsTracker.trackViewShown(this) diff --git a/WooCommerce/src/main/res/layout/fragment_settings_beta.xml b/WooCommerce/src/main/res/layout/fragment_settings_beta.xml index 02d1a53b01f4..fddd26ee91d0 100644 --- a/WooCommerce/src/main/res/layout/fragment_settings_beta.xml +++ b/WooCommerce/src/main/res/layout/fragment_settings_beta.xml @@ -14,6 +14,13 @@ app:toggleOptionDesc="@string/settings_enable_product_addons_teaser_message" app:toggleOptionTitle="@string/settings_enable_product_addons_teaser_title" /> + + diff --git a/WooCommerce/src/main/res/values/strings.xml b/WooCommerce/src/main/res/values/strings.xml index 8ca143083550..8a2461f8037c 100644 --- a/WooCommerce/src/main/res/values/strings.xml +++ b/WooCommerce/src/main/res/values/strings.xml @@ -2610,6 +2610,8 @@ Inactive Auto-managed An error occurred while loading installed plugins + Application Passwords + Enable application passwords to let the app directly fetch data from your WooCommerce site instead of doing so through Jetpack connection.