diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index f1ec4f3f18b2..ab4a56249f69 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -2,7 +2,7 @@ 26.9 ----- - +* [*] Reworked editor capability detection to be more reliable and prevent a false "Unable to connect to your site" banner on private Atomic sites. 26.8 ----- diff --git a/WordPress/src/main/java/org/wordpress/android/AppInitializer.kt b/WordPress/src/main/java/org/wordpress/android/AppInitializer.kt index 300566324748..2113fe519073 100644 --- a/WordPress/src/main/java/org/wordpress/android/AppInitializer.kt +++ b/WordPress/src/main/java/org/wordpress/android/AppInitializer.kt @@ -71,6 +71,7 @@ import org.wordpress.android.networking.ConnectionChangeReceiver import org.wordpress.android.networking.OAuthAuthenticator import org.wordpress.android.networking.RestClientUtils import org.wordpress.android.push.GCMRegistrationScheduler +import org.wordpress.android.repositories.SiteProvisioningSource import org.wordpress.android.support.ZendeskHelper import org.wordpress.android.ui.ActivityId import org.wordpress.android.ui.debug.cookies.DebugCookieManager @@ -229,6 +230,9 @@ class AppInitializer @Inject constructor( @Inject lateinit var wpApiClientProvider: WpApiClientProvider + @Inject + lateinit var siteProvisioningSource: SiteProvisioningSource + @Inject lateinit var openWebLinksWithJetpackHelper: DeepLinkOpenWebLinksWithJetpackHelper @@ -717,6 +721,9 @@ class AppInitializer @Inject constructor( // Clear cached wordpress-rs services and API clients wpServiceProvider.clearAll() wpApiClientProvider.clearAllClients() + + // Drop per-site provisioning + capability state for the signed-out user + siteProvisioningSource.clear() } /* diff --git a/WordPress/src/main/java/org/wordpress/android/repositories/EditorSettingsRepository.kt b/WordPress/src/main/java/org/wordpress/android/repositories/EditorSettingsRepository.kt index bce5a2d3dcf4..d57b7b8c44f9 100644 --- a/WordPress/src/main/java/org/wordpress/android/repositories/EditorSettingsRepository.kt +++ b/WordPress/src/main/java/org/wordpress/android/repositories/EditorSettingsRepository.kt @@ -108,11 +108,12 @@ class EditorSettingsRepository @Inject constructor( private suspend fun fetchRouteSupport( site: SiteModel ): Boolean = try { - // For Atomic sites the editor fetches `wp-block-editor/v1/settings` - // from the direct host — proxy and direct host can advertise - // different route lists, so detection has to probe the direct host - // too. See #22879. - if (site.isWPComAtomic) { + // Atomic and Jetpack-WPCom-REST sites have their own REST host that the editor talks to + // directly — the WP.com proxy and the direct host advertise different route lists, so + // detection has to probe the direct host too. The proxy is only for minting the application + // password. WP.com Simple sites have no direct host (the WP.com REST API *is* their API), + // and self-hosted sites are already direct via the configured client. See #22879. + if (site.isUsingWpComRestApi && !site.isWPComSimpleSite) { fetchRouteSupportViaDirectHostDiscovery(site) } else { fetchRouteSupportViaConfiguredClient(site) diff --git a/WordPress/src/main/java/org/wordpress/android/repositories/SiteProvisioningSource.kt b/WordPress/src/main/java/org/wordpress/android/repositories/SiteProvisioningSource.kt new file mode 100644 index 000000000000..78704b09428b --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/repositories/SiteProvisioningSource.kt @@ -0,0 +1,302 @@ +package org.wordpress.android.repositories + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.async +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.launch +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider +import org.wordpress.android.fluxc.store.SiteStore +import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.modules.APPLICATION_SCOPE +import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper +import org.wordpress.android.ui.accounts.login.SiteApiRestUrlRecoverer +import org.wordpress.android.ui.accounts.login.SiteXmlRpcUrlRecoverer +import org.wordpress.android.ui.mysite.cards.applicationpassword.ApplicationPasswordValidator +import org.wordpress.android.util.AppLog +import org.wordpress.android.util.NetworkUtilsWrapper +import java.util.concurrent.ConcurrentHashMap +import javax.inject.Inject +import javax.inject.Named +import javax.inject.Singleton + +/** + * The single source of truth for getting a site ready to use: it provisions + * application-password credentials, recovers the REST API root, recovers the + * XML-RPC endpoint (self-hosted), and detects editor capabilities. + * + * Capability detection can't run until a credential exists (you can't probe a + * private Atomic host's REST API unauthenticated), so **auth is awaited first**; + * after that, the REST-capability branch and the XML-RPC branch are independent + * and run **in parallel**. Routing every consumer (connectivity banner, editor + * preloader, application-password card) through this one pipeline means the + * first-login race is structurally impossible — the probe is downstream of the + * mint — and there's one shared, deduplicated run per site instead of each + * consumer racing the others. + * + * ### No model is held across stages + * Stages take a **`siteLocalId`**, read the `SiteModel` fresh from the store at + * the point of use, and write back **only the one column they changed** + * (`persistApiRootUrl` / `persistXmlRpcUrl`). Nothing keeps a mutated `SiteModel` + * around, so the two parallel branches can't clobber each other and there's no + * stale-model write (see #22905). The passed `SiteModel` is used for its id only. + * + * Per site there is at most one in-flight pipeline (single-flight, keyed by + * [SiteModel.id]); concurrent callers join it — which also subsumes the + * application-password card's old single-flight guard (two concurrent mints hit + * a 409 that destroys the winner's credentials). + * + * ## Entry points + * - [stateFor] — reactive: returns a shared [StateFlow]; first access runs the + * pipeline, later accesses reuse a [SiteReadiness.Ready] result. + * - [await] — one-shot: runs the pipeline (if needed) and returns the result. + * - [invalidate] — forces a re-run (pull-to-refresh, retry). + * - [clear] — cancels all work and drops all state; wire into sign-out. + */ +@Singleton +@Suppress("LongParameterList") +class SiteProvisioningSource @Inject constructor( + private val siteStore: SiteStore, + private val applicationPasswordLoginHelper: ApplicationPasswordLoginHelper, + private val applicationPasswordValidator: ApplicationPasswordValidator, + private val wpApiClientProvider: WpApiClientProvider, + private val siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer, + private val siteXmlRpcUrlRecoverer: SiteXmlRpcUrlRecoverer, + private val editorSettingsRepository: EditorSettingsRepository, + private val networkUtilsWrapper: NetworkUtilsWrapper, + private val appLogWrapper: AppLogWrapper, + @Named(APPLICATION_SCOPE) private val appScope: CoroutineScope, +) { + private val states = ConcurrentHashMap>() + private val jobs = ConcurrentHashMap() + + // Sites whose pipeline reached Ready this process — the dedup gate. Only a fully-ready site + // latches; auth-needed / unreachable / transient outcomes are left to re-run on the next + // access. Reset by invalidate / clear. + private val ready = ConcurrentHashMap.newKeySet() + + /** + * The shared readiness state for [site]. The first call starts the pipeline; + * later calls return the same flow without re-running once it reached + * [SiteReadiness.Ready]. Only [SiteModel.id] is read from [site]. + */ + @Synchronized + fun stateFor(site: SiteModel): StateFlow { + val flow = flowFor(site.id) + if (shouldRun(site.id)) launchPipeline(site.id) + return flow + } + + /** + * Runs the pipeline for [site] (if it hasn't reached Ready) and returns the + * settled readiness. Respects the once-per-site gate; call [invalidate] first + * to force a fresh run. + */ + suspend fun await(site: SiteModel): SiteReadiness { + stateFor(site) + jobs[site.id]?.join() + return states[site.id]?.value ?: SiteReadiness.Probing + } + + /** + * Forces a re-run for [site], bypassing the once-per-site gate. A no-op while + * a run is already in flight — that run already reflects current state. + */ + @Synchronized + fun invalidate(site: SiteModel) { + if (jobs[site.id]?.isActive == true) return + ready.remove(site.id) + launchPipeline(site.id) + } + + /** Cancels all in-flight pipelines and drops all cached state (sign-out). */ + @Synchronized + fun clear() { + jobs.values.forEach { it.cancel() } + jobs.clear() + states.clear() + ready.clear() + } + + @Synchronized + private fun launchPipeline(siteLocalId: Int) { + jobs[siteLocalId]?.cancel() + val flow = flowFor(siteLocalId) + jobs[siteLocalId] = appScope.launch { + val readiness = runPipeline(siteLocalId) + flow.value = readiness + if (readiness is SiteReadiness.Ready) ready.add(siteLocalId) + } + } + + private fun flowFor(siteLocalId: Int): MutableStateFlow = + states.getOrPut(siteLocalId) { MutableStateFlow(SiteReadiness.Probing) } + + private fun shouldRun(siteLocalId: Int): Boolean = + jobs[siteLocalId]?.isActive != true && siteLocalId !in ready + + private suspend fun runPipeline(siteLocalId: Int): SiteReadiness { + val auth = ensureAuth(siteLocalId) + return when (auth) { + SiteAuthState.Provisioned, SiteAuthState.NotApplicable -> coroutineScope { + // Post-auth, the REST-capability chain and the XML-RPC recovery are independent — each + // reads the site fresh and writes only its own column — so run them in parallel. + // recoverRestUrlIfNeeded precedes detectCapabilities within its branch because the probe + // needs the recovered REST root. Both branches read the credentials fresh from the store: + // the mint persists them via a single writer that the generic full-row update can no + // longer clobber (#22947), so the re-read is now trustworthy (#22905). + val capabilities = async { + recoverRestUrlIfNeeded(siteLocalId) + detectCapabilities(siteLocalId) + } + val xmlRpc = async { recoverXmlRpcIfNeeded(siteLocalId) } + xmlRpc.await() + capabilities.await() + } + else -> SiteReadiness.NeedsAuth(auth) + } + } + + /** + * Stage 1 — ensure the site has working application-password credentials. Validates stored creds + * with Basic auth against the direct host; on a confirmed rejection wipes them and mints fresh + * ones via the FluxC Jetpack tunnel. The mint persists the credentials (single-writer, #22947), so + * the downstream stages read them back from a fresh [SiteModel] rather than having them threaded. + */ + // Each return is a distinct auth outcome (missing site, valid, transient, minted, failed); + // collapsing to one return would thread a result through nested branches and read worse. + @Suppress("ReturnCount") + private suspend fun ensureAuth(siteLocalId: Int): SiteAuthState { + val site = siteStore.getSiteByLocalId(siteLocalId) + ?: return SiteAuthState.Unprovisionable(hadCredentials = false) + // WP.com Simple sites are fully proxied and OAuth-bearer-authed — no application password + // applies (the mint returns NotSupported). Capability detection works through the proxy, so + // treat them as ready instead of blocking detection behind a mint that can never run. + if (site.isWPComSimpleSite) return SiteAuthState.NotApplicable + val hadCredentials = !applicationPasswordLoginHelper.siteHasBadCredentials(site) + if (hadCredentials) { + when (applicationPasswordValidator.validate(site)) { + ApplicationPasswordValidator.Outcome.Valid -> return SiteAuthState.Provisioned + ApplicationPasswordValidator.Outcome.NetworkUnavailable -> { + appLogWrapper.d(AppLog.T.MAIN, "A_P: Validation network error for ${site.url}") + return SiteAuthState.Provisioning + } + ApplicationPasswordValidator.Outcome.Invalid -> { + appLogWrapper.d(AppLog.T.MAIN, "A_P: Stored creds invalid for ${site.url}, clearing") + siteStore.deleteStoredApplicationPasswordCredentials(site) + wpApiClientProvider.clearSelfHostedClient(site.id) + } + } + } + // createApplicationPassword mints and persists the credentials; downstream stages read them + // back from a fresh SiteModel. + val createResult = siteStore.createApplicationPassword(site) + if (!createResult.isError && createResult.credentials != null) { + wpApiClientProvider.clearSelfHostedClient(site.id) + appLogWrapper.d(AppLog.T.MAIN, "A_P: Headless mint succeeded for ${site.url}") + return SiteAuthState.Provisioned + } + appLogWrapper.d( + AppLog.T.MAIN, + "A_P: Headless mint failed for ${site.url} (notSupported=${createResult.error?.notSupported})" + ) + return SiteAuthState.Unprovisionable(hadCredentials = hadCredentials) + } + + /** + * Stage 2a — recover the REST API root for Atomic sites minted through the + * Jetpack tunnel (which never runs discovery and leaves `wpApiRestUrl` null). + * Persists the one column; the capability probe (sequenced after it) re-reads it. + */ + private suspend fun recoverRestUrlIfNeeded(siteLocalId: Int) { + val site = siteStore.getSiteByLocalId(siteLocalId) ?: return + // WP.com Simple sites are proxy-served — no direct REST host to recover (their wpApiRestUrl + // is legitimately null), so don't burn a discovery call on them. + if (site.isWPComSimpleSite || !site.wpApiRestUrl.isNullOrEmpty()) return + siteApiRestUrlRecoverer.discoverApiRootUrl(site.url)?.let { apiRootUrl -> + siteApiRestUrlRecoverer.persistApiRootUrl(siteLocalId, apiRootUrl) + } + } + + /** + * Stage 2b (parallel) — recover the XML-RPC endpoint for true self-hosted + * sites that don't have one. Discovers + authenticates against it with the + * site's application-password credentials (which work for XML-RPC just as for + * REST), and on success persists the one column; the application-password card + * re-reads it. + */ + private suspend fun recoverXmlRpcIfNeeded(siteLocalId: Int) { + val site = siteStore.getSiteByLocalId(siteLocalId) ?: return + // WP.com / Atomic / Jetpack-WPCom-REST sites talk REST end-to-end and don't use XML-RPC. + if (site.isUsingWpComRestApi || !site.xmlRpcUrl.isNullOrEmpty()) return + siteXmlRpcUrlRecoverer.discoverAndVerifyXmlRpcUrl(site)?.let { endpoint -> + siteXmlRpcUrlRecoverer.persistXmlRpcUrl(siteLocalId, endpoint) + } + } + + /** + * Stage 3 — probe the REST API for editor-capability support and persist it. + * Reached only once auth is [SiteAuthState.Provisioned] / [SiteAuthState.NotApplicable]. + * Reads the site fresh: the mint has already persisted the credentials (single-writer, #22947), + * so a probe failure here is a real transport problem, not a pending mint. + */ + private suspend fun detectCapabilities(siteLocalId: Int): SiteReadiness { + val site = siteStore.getSiteByLocalId(siteLocalId) ?: return SiteReadiness.Unreachable + val ok = editorSettingsRepository.fetchEditorCapabilitiesForSite(site) + val hasCache = editorSettingsRepository.hasCachedCapabilities(site) + return when { + ok || hasCache -> SiteReadiness.Ready + !networkUtilsWrapper.isNetworkAvailable() -> SiteReadiness.TransientError + else -> SiteReadiness.Unreachable + } + } +} + +/** + * Whether a site's application password is usable. Owned by [SiteProvisioningSource]; + * rendered by the application-password card. + */ +sealed interface SiteAuthState { + /** Credentials are usable (validated, or freshly minted). */ + data object Provisioned : SiteAuthState + + /** No application password applies — a WP.com Simple site, which is proxy-served and + * OAuth-bearer-authed. Treated like [Provisioned]: capability detection runs via the proxy. */ + data object NotApplicable : SiteAuthState + + /** Not usable yet, but not a terminal failure — a mint is implied / a transient + * validation error occurred. The card stays hidden; the next run retries. */ + data object Provisioning : SiteAuthState + + /** Terminal: the mint failed. [hadCredentials] distinguishes a re-authentication + * (creds went bad) from a first-time authentication prompt. */ + data class Unprovisionable(val hadCredentials: Boolean) : SiteAuthState +} + +/** + * The combined per-site readiness the [SiteProvisioningSource] exposes. The + * connectivity banner renders [Unreachable], the application-password card + * renders [NeedsAuth], and the editor preloader awaits a non-[Probing] value — + * each a slice of the one state, so they can't disagree. + */ +sealed interface SiteReadiness { + /** The pipeline is running and hasn't produced a result yet. */ + data object Probing : SiteReadiness + + /** Stopped at the auth stage — credentials aren't usable. Carries the + * [SiteAuthState] so the card can pick re-auth vs. first-auth. */ + data class NeedsAuth(val auth: SiteAuthState) : SiteReadiness + + /** Provisioned and editor capabilities are known (detected or cached). */ + data object Ready : SiteReadiness + + /** Provisioned, but the capability probe failed — the site looks unreachable. + * The only state that surfaces the connectivity banner. */ + data object Unreachable : SiteReadiness + + /** Provisioned, but a transient failure (e.g. offline). Retried on the next run. */ + data object TransientError : SiteReadiness +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt index 5a9c5e493269..7b5c6c01fe0d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt @@ -46,7 +46,6 @@ class ApplicationPasswordLoginHelper @Inject constructor( private val discoverSuccessWrapper: DiscoverSuccessWrapper, private val crashLogging: CrashLogging, private val wpApiClientProvider: WpApiClientProvider, - private val credentialsChangedNotifier: CredentialsChangedNotifier, ) { private var processedAppPasswordData: String? = null @@ -149,7 +148,6 @@ class ApplicationPasswordLoginHelper @Inject constructor( } wpApiClientProvider.clearSelfHostedClient(site.id) dispatcherWrapper.updateApplicationPassword(site) - credentialsChangedNotifier.notifyChanged(site.id) trackSuccessful(effectiveUrlLogin.siteUrl) trackCreated(creationSource, success = true) processedAppPasswordData = effectiveUrlLogin.siteUrl diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/CredentialsChangedNotifier.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/CredentialsChangedNotifier.kt deleted file mode 100644 index 0046dc400595..000000000000 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/CredentialsChangedNotifier.kt +++ /dev/null @@ -1,34 +0,0 @@ -package org.wordpress.android.ui.accounts.login - -import kotlinx.coroutines.channels.BufferOverflow -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.SharedFlow -import kotlinx.coroutines.flow.asSharedFlow -import javax.inject.Inject -import javax.inject.Singleton - -/** - * App-scoped signal that an application password was newly established for a site — either by the - * headless Jetpack-tunnel mint on the My Site screen or by the interactive application-password - * login. Lets credential-dependent work (e.g. editor capability detection) re-run as soon as the - * password exists, instead of waiting for the next My Site resume/refresh. - * - * Emits the site's local id; collectors should re-read a fresh SiteModel so they observe the - * just-persisted credentials rather than a stale in-memory copy. - */ -@Singleton -class CredentialsChangedNotifier @Inject constructor() { - // replay = 1 so a collector that subscribes just after an emit still sees it — closes the - // emit-before-collect race. DROP_OLDEST keeps tryEmit non-suspending without an unbounded buffer. - private val _events = MutableSharedFlow( - replay = 1, - extraBufferCapacity = 1, - onBufferOverflow = BufferOverflow.DROP_OLDEST, - ) - val events: SharedFlow = _events.asSharedFlow() - - /** Signals that [siteLocalId]'s application-password credentials were just established. */ - fun notifyChanged(siteLocalId: Int) { - _events.tryEmit(siteLocalId) - } -} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteXmlRpcUrlRecoverer.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteXmlRpcUrlRecoverer.kt new file mode 100644 index 000000000000..6278d15fc424 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteXmlRpcUrlRecoverer.kt @@ -0,0 +1,78 @@ +package org.wordpress.android.ui.accounts.login + +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.withContext +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder +import org.wordpress.android.fluxc.network.xmlrpc.site.SiteXMLRPCClient +import org.wordpress.android.fluxc.persistence.SiteSqlUtils +import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.modules.BG_THREAD +import org.wordpress.android.util.AppLog +import javax.inject.Inject +import javax.inject.Named +import javax.inject.Singleton +import kotlin.coroutines.cancellation.CancellationException + +/** + * Heals [SiteModel.xmlRpcUrl] for true self-hosted sites whose XML-RPC endpoint was never + * discovered (or was previously gated). The REST counterpart is [SiteApiRestUrlRecoverer]; this + * follows the same shape so callers never hold a mutated [SiteModel]: + * + * - [discoverAndVerifyXmlRpcUrl] discovers the endpoint and confirms it works with an authenticated + * call using the site's application-password credentials, returning the verified URL (or `null` + * if discovery/verification fails). + * - [persistXmlRpcUrl] writes only that one column to the DB row for `localId`. + */ +@Singleton +class SiteXmlRpcUrlRecoverer @Inject constructor( + private val selfHostedEndpointFinder: SelfHostedEndpointFinder, + private val siteXMLRPCClient: SiteXMLRPCClient, + private val siteSqlUtils: SiteSqlUtils, + private val appLogWrapper: AppLogWrapper, + @param:Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, +) { + @Suppress("SwallowedException", "TooGenericExceptionCaught") + suspend fun discoverAndVerifyXmlRpcUrl(site: SiteModel): String? = withContext(bgDispatcher) { + try { + val endpoint = selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url) + val result = siteXMLRPCClient.fetchSites( + endpoint, + site.apiRestUsernamePlain, + site.apiRestPasswordPlain, + ) + if (result.isError) { + appLogWrapper.w(AppLog.T.API, "XML-RPC verification failed for ${site.url}") + null + } else { + endpoint + } + } catch (e: CancellationException) { + throw e + } catch (e: SelfHostedEndpointFinder.DiscoveryException) { + // Expected when the site has no reachable XML-RPC endpoint — surfaces as the + // XML-RPC-disabled card (xmlRpcUrl stays empty) and retries on the next run. + appLogWrapper.w(AppLog.T.API, "XML-RPC discovery failed for ${site.url}") + null + } catch (e: Exception) { + // Best-effort recovery must never let an unexpected throw escape and cancel the + // provisioning pipeline (mirrors SiteApiRestUrlRecoverer). Same null -> disabled-card surface. + appLogWrapper.e( + AppLog.T.API, + "XML-RPC discovery threw for ${site.url}: ${e::class.simpleName}: ${e.message}" + ) + null + } + } + + suspend fun persistXmlRpcUrl(localId: Int, xmlRpcUrl: String): Boolean = withContext(bgDispatcher) { + val rowsUpdated = siteSqlUtils.updateXmlRpcUrl(localId, xmlRpcUrl) + if (rowsUpdated == 0) { + appLogWrapper.w(AppLog.T.API, "Cannot persist xmlRpcUrl: no site with localId=$localId") + false + } else { + appLogWrapper.d(AppLog.T.API, "Persisted xmlRpcUrl=$xmlRpcUrl for localId=$localId") + true + } + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSlice.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSlice.kt index 3ae1fdaa37c0..69a1cc998b86 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSlice.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSlice.kt @@ -2,22 +2,17 @@ package org.wordpress.android.ui.mysite.cards.applicationpassword import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import org.wordpress.android.R -import androidx.annotation.VisibleForTesting import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder -import org.wordpress.android.fluxc.network.xmlrpc.site.SiteXMLRPCClient -import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.repositories.SiteAuthState +import org.wordpress.android.repositories.SiteProvisioningSource +import org.wordpress.android.repositories.SiteReadiness import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper -import org.wordpress.android.ui.accounts.login.CredentialsChangedNotifier -import org.wordpress.android.ui.accounts.login.SiteApiRestUrlRecoverer import org.wordpress.android.ui.mysite.MySiteCardAndItem import org.wordpress.android.ui.mysite.MySiteCardAndItem.Card.QuickLinksItem.QuickLinkItem import org.wordpress.android.ui.mysite.SiteNavigationAction @@ -25,22 +20,26 @@ import org.wordpress.android.ui.pages.SnackbarMessageHolder import org.wordpress.android.ui.utils.ListItemInteraction import org.wordpress.android.ui.utils.UiString.UiStringRes import org.wordpress.android.util.AppLog -import org.wordpress.android.modules.IO_THREAD import org.wordpress.android.viewmodel.Event import javax.inject.Inject -import javax.inject.Named +/** + * Renders the application-password card from the site's readiness. All the + * provisioning mechanics — validate, mint, REST-root recovery, XML-RPC + * recovery — live in [SiteProvisioningSource]; this slice is a thin view over + * the result: + * + * - [SiteAuthState.Unprovisionable] → a re-authentication banner (creds went + * bad) or a first-time "authenticate" card, looked up lazily. + * - provisioned (any non-[SiteReadiness.NeedsAuth] terminal state) → hidden, + * except true self-hosted sites whose XML-RPC endpoint the pipeline couldn't + * recover, which get the XML-RPC-disabled card. + */ class ApplicationPasswordViewModelSlice @Inject constructor( private val applicationPasswordLoginHelper: ApplicationPasswordLoginHelper, private val siteStore: SiteStore, private val appLogWrapper: AppLogWrapper, - private val wpApiClientProvider: WpApiClientProvider, - private val applicationPasswordValidator: ApplicationPasswordValidator, - private val selfHostedEndpointFinder: SelfHostedEndpointFinder, - private val siteXMLRPCClient: SiteXMLRPCClient, - private val siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer, - private val credentialsChangedNotifier: CredentialsChangedNotifier, - @Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher, + private val siteProvisioningSource: SiteProvisioningSource, ) { lateinit var scope: CoroutineScope @@ -57,98 +56,51 @@ class ApplicationPasswordViewModelSlice @Inject constructor( val uiModelMutable = MutableLiveData() val uiModel: LiveData = uiModelMutable - // Single-flight guard: buildCard is invoked from onResume / refresh / onSitePicked, which can - // fire close together. Without this, two coroutines both pass the "creds missing" check in - // ApplicationPasswordsManager and issue two server-side mints. Worse, the 409 conflict handler - // then deletes-and-recreates the winner's password, so the losing racer destroys working creds. - private var buildJob: Job? = null + private var collectJob: Job? = null + private var currentSite: SiteModel? = null fun buildCard(siteModel: SiteModel) { - if (buildJob?.isActive == true) { - appLogWrapper.d( - AppLog.T.MAIN, - "A_P: Skipping buildCard for ${siteModel.url} - previous run still in flight" - ) - return - } - buildJob = scope.launch { - val storedSite = siteStore.sites.firstOrNull { it.id == siteModel.id } ?: siteModel - val hadCreds = !applicationPasswordLoginHelper.siteHasBadCredentials(storedSite) - - // Step 1: if we already have stored creds, validate them with Basic auth against the - // direct host. This actually exercises the application password (unlike - // WpApiClientProvider.getWpApiClient, which routes WPCom-flagged sites through the - // bearer-token path and would not catch a revoked password). - if (hadCreds) { - when (applicationPasswordValidator.validate(storedSite)) { - ApplicationPasswordValidator.Outcome.Valid -> { - // Heal in the background so the card hides immediately on a slow network. - scope.launch { healApiRestUrlIfMissing(storedSite) } - handleValidAuth(storedSite) - return@launch - } - ApplicationPasswordValidator.Outcome.NetworkUnavailable -> { - // Don't punish flaky networks — leave the card hidden and try again next time. - uiModelMutable.postValue(null) - appLogWrapper.d(AppLog.T.MAIN, "A_P: Validation network error for ${storedSite.url}") - return@launch - } - ApplicationPasswordValidator.Outcome.Invalid -> { - // Stored creds are stale (revoked, deleted, etc.) — clear them so the next - // mint creates fresh ones, and invalidate the cached client. - appLogWrapper.d(AppLog.T.MAIN, "A_P: Stored creds invalid for ${storedSite.url}, clearing") - siteStore.deleteStoredApplicationPasswordCredentials(storedSite) - wpApiClientProvider.clearSelfHostedClient(storedSite.id) - } - } - } - - // Step 2: mint a fresh application password via the FluxC Jetpack tunnel. wordpress-rs - // can't do this today — the WP.com REST proxy doesn't expose the application-passwords - // endpoint under /wp/v2/sites/{id}/... (see Automattic/wordpress-rs#1350) — so FluxC's - // Jetpack-tunnel client is the only working path for Atomic / Jetpack-WPCom-REST sites. - val createResult = siteStore.createApplicationPassword(storedSite) - if (!createResult.isError && createResult.credentials != null) { - wpApiClientProvider.clearSelfHostedClient(storedSite.id) - appLogWrapper.d(AppLog.T.MAIN, "A_P: Headless mint succeeded for ${storedSite.url}") - credentialsChangedNotifier.notifyChanged(storedSite.id) - // The mint goes through the Jetpack tunnel and never runs discovery — without this - // step, freshly minted Atomic sites end up with working creds but a NULL - // wpApiRestUrl in the local DB. Run in the background so the card hides immediately. - scope.launch { healApiRestUrlIfMissing(storedSite) } - handleValidAuth(storedSite) - return@launch - } - appLogWrapper.d( - AppLog.T.MAIN, - "A_P: Headless mint failed for ${storedSite.url} (notSupported=" + - "${createResult.error?.notSupported})" - ) - - // Step 3: mint failed. If we started with creds, show the reauth banner; otherwise the - // standard "authenticate" card. Either way, discovery is required to populate the URL. - if (hadCreds) { - buildReauthenticationBanner(storedSite) - } else { - buildAuthenticationCard(storedSite) + collectJob?.cancel() + currentSite = siteModel + collectJob = scope.launch { + siteProvisioningSource.stateFor(siteModel).collect { readiness -> + // Bail if the user switched sites while suspended — postValue is not a + // suspension point, so cancellation alone won't catch this. + if (currentSite?.id != siteModel.id) return@collect + renderCard(siteModel, readiness) } } } - private suspend fun healApiRestUrlIfMissing(site: SiteModel) { - if (!site.wpApiRestUrl.isNullOrEmpty()) return - siteApiRestUrlRecoverer.discoverApiRootUrl(site.url)?.let { apiRootUrl -> - site.wpApiRestUrl = apiRootUrl - siteApiRestUrlRecoverer.persistApiRootUrl(site.id, apiRootUrl) + private suspend fun renderCard(site: SiteModel, readiness: SiteReadiness) { + when (readiness) { + is SiteReadiness.NeedsAuth -> when (val auth = readiness.auth) { + is SiteAuthState.Unprovisionable -> + if (auth.hadCredentials) buildReauthenticationBanner(site) else buildAuthenticationCard(site) + SiteAuthState.Provisioning -> { + // Mint in flight / transient validation error — hide and let the next run retry. + uiModelMutable.postValue(null) + appLogWrapper.d(AppLog.T.MAIN, "A_P: Provisioning in progress for ${site.url}") + } + SiteAuthState.Provisioned, SiteAuthState.NotApplicable -> + Unit // never wrap in NeedsAuth — they proceed to detection + } + // Any terminal provisioned state — the credentials are usable, so the only card left to + // show is the self-hosted XML-RPC fallback. Capability outcome (Ready/Unreachable) is the + // connectivity banner's concern, not this card's. + SiteReadiness.Ready, + SiteReadiness.Unreachable, + SiteReadiness.TransientError -> handleProvisioned(site) + SiteReadiness.Probing -> Unit // leave the card unchanged while the pipeline runs } } - private fun handleValidAuth(site: SiteModel) { - // Only true self-hosted sites need the XML-RPC fallback path — Atomic and Jetpack-WPCom-REST - // sites talk REST end-to-end and don't need XML-RPC. - if (!site.isUsingWpComRestApi && site.xmlRpcUrl.isNullOrEmpty()) { - buildXmlRpcDisabledCard(site) - attemptXmlRpcRediscovery(site) + private fun handleProvisioned(site: SiteModel) { + // Read fresh: the pipeline's parallel XML-RPC branch may have just recovered the endpoint. + val storedSite = siteStore.getSiteByLocalId(site.id) ?: site + // Only true self-hosted sites need XML-RPC; if the pipeline couldn't recover it, surface it. + if (!storedSite.isUsingWpComRestApi && storedSite.xmlRpcUrl.isNullOrEmpty()) { + buildXmlRpcDisabledCard(storedSite) } else { uiModelMutable.postValue(null) appLogWrapper.d(AppLog.T.MAIN, "A_P: Hiding card for ${site.url} - authenticated") @@ -232,42 +184,6 @@ class ApplicationPasswordViewModelSlice @Inject constructor( ) } - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - internal fun attemptXmlRpcRediscovery(site: SiteModel) { - scope.launch { - try { - val xmlRpcEndpoint = withContext(ioDispatcher) { - selfHostedEndpointFinder - .verifyOrDiscoverXMLRPCEndpoint(site.url) - } - - // Verify with an authenticated call - val result = withContext(ioDispatcher) { - siteXMLRPCClient.fetchSites( - xmlRpcEndpoint, - site.apiRestUsernamePlain, - site.apiRestPasswordPlain - ) - } - if (result.isError) { - return@launch - } - - site.xmlRpcUrl = xmlRpcEndpoint - // Persist only the rediscovered column — mirrors healApiRestUrlIfMissing. A full-row - // updateSite would rewrite ~80 columns from this in-memory model for a one-field change - // (risking clobbering other out-of-band values), so write just xmlRpcUrl. - siteStore.persistXmlRpcUrl(site.id, xmlRpcEndpoint) - buildCard(site) - } catch ( - @Suppress("SwallowedException") - e: SelfHostedEndpointFinder.DiscoveryException - ) { - // XML-RPC rediscovery failed; card remains visible - } - } - } - private fun onClick(site: SiteModel, alternativeUrl: String) { _onNavigation.postValue( Event( diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSlice.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSlice.kt index 5a93cd7df624..fc17191982db 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSlice.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSlice.kt @@ -7,83 +7,50 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.launch import org.wordpress.android.R import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.repositories.EditorSettingsRepository -import org.wordpress.android.ui.accounts.login.CredentialsChangedNotifier +import org.wordpress.android.repositories.SiteProvisioningSource +import org.wordpress.android.repositories.SiteReadiness import org.wordpress.android.ui.mysite.MySiteCardAndItem -import org.wordpress.android.ui.mysite.SelectedSiteRepository -import org.wordpress.android.util.NetworkUtilsWrapper import javax.inject.Inject class SiteConnectivityBannerViewModelSlice @Inject constructor( - private val editorSettingsRepository: EditorSettingsRepository, - private val networkUtilsWrapper: NetworkUtilsWrapper, - private val credentialsChangedNotifier: CredentialsChangedNotifier, - private val selectedSiteRepository: SelectedSiteRepository, + private val siteProvisioningSource: SiteProvisioningSource, ) { private lateinit var scope: CoroutineScope - private var currentJob: Job? = null + private var collectJob: Job? = null private var currentSite: SiteModel? = null private val _uiModel = MutableLiveData() val uiModel: LiveData = _uiModel - /* Site capabilities rarely change, so once we've successfully fetched them for a site we - skip subsequent non-user-initiated fetches in this slice's lifetime. Failed fetches do - not populate this set, so a transient network failure recovers on the next onResume. - User-initiated calls (PTR, banner retry) always bypass this gate. */ - private val fetchedCapabilitiesForSite = mutableSetOf() - fun initialize(scope: CoroutineScope) { this.scope = scope - // Re-run detection the moment an application password is established for the selected site - // (e.g. the headless mint finished after our first fetch lost the race), instead of waiting - // for the next resume/refresh. Re-read the selected site so we see the just-persisted - // credentials; isUserInitiated = false so a replayed event is a no-op once cached. - scope.launch { - credentialsChangedNotifier.events.collect { siteLocalId -> - val site = selectedSiteRepository.getSelectedSite() - if (site != null && site.id == siteLocalId) { - fetchCapabilities(site, isUserInitiated = false) - } - } - } } + /** + * Subscribes the banner to [site]'s readiness. The banner is a thin view over + * that state — it surfaces only when the site is provisioned but the capability + * probe failed ([SiteReadiness.Unreachable]). Every other state (probing, needs + * auth, offline, ready) leaves it hidden: when credentials are the problem the + * application-password card owns it, and the banner stays out of the way. + * [isUserInitiated] (pull-to-refresh, retry) forces a fresh run. + */ fun fetchCapabilities(site: SiteModel, isUserInitiated: Boolean) { - currentJob?.cancel() + collectJob?.cancel() currentSite = site - currentJob = scope.launch { - if (site.id in fetchedCapabilitiesForSite && !isUserInitiated) { - return@launch - } - val ok = editorSettingsRepository.fetchEditorCapabilitiesForSite(site) - if (ok) { - fetchedCapabilitiesForSite.add(site.id) + if (isUserInitiated) siteProvisioningSource.invalidate(site) + collectJob = scope.launch { + siteProvisioningSource.stateFor(site).collect { readiness -> + // Bail if the user switched sites while suspended — postValue is + // not a suspension point, so cancellation alone won't catch this. + if (currentSite?.id != site.id) return@collect + val showBanner = readiness is SiteReadiness.Unreachable + _uiModel.postValue(if (showBanner) buildBanner() else null) } - val hasCache = editorSettingsRepository.hasCachedCapabilities(site) - // Bail if the user switched sites while we were suspended — postValue - // isn't a suspension point, so cancellation alone won't catch this. - if (currentSite?.id != site.id) return@launch - // Suppress the banner when the device is offline — the global "no - // connection" banner already covers this case, and stacking warnings - // for the same root cause is just noise. - val suppressForOffline = !ok && !networkUtilsWrapper.isNetworkAvailable() - // Atomic sites probe the direct host with an application password that's minted - // asynchronously on this same screen, so a first-login fetch can fail purely because - // the credential isn't ready yet. Treat that as pending, not a connection failure — - // the application-password card owns that state and a later fetch will succeed. - val suppressForPendingAuth = - !ok && editorSettingsRepository.isAwaitingApplicationPassword(site) - // Show the banner only as a last resort — not when detection succeeded, when we have - // cached capabilities, or while a transient non-error state (offline / pending creds) - // already explains the failure. - val suppressBanner = ok || hasCache || suppressForOffline || suppressForPendingAuth - _uiModel.postValue(if (suppressBanner) null else buildBanner()) } } fun clearBanner() { - currentJob?.cancel() + collectJob?.cancel() currentSite = null _uiModel.postValue(null) } @@ -93,10 +60,7 @@ class SiteConnectivityBannerViewModelSlice @Inject constructor( textResource = R.string.site_connectivity_banner_text, imageResource = R.drawable.ic_cloud_off_themed_24dp, onActionClick = { - val site = currentSite - if (site != null && currentJob?.isActive != true) { - fetchCapabilities(site, isUserInitiated = true) - } + currentSite?.let { fetchCapabilities(it, isUserInitiated = true) } }, showLearnMore = false, centerImageVertically = true, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergEditorPreloader.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergEditorPreloader.kt index 5a9a116b8769..e90c5e510f0d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergEditorPreloader.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergEditorPreloader.kt @@ -10,9 +10,9 @@ import kotlinx.coroutines.launch import org.wordpress.android.datasets.SiteSettingsProvider import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.AccountStore +import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.modules.BG_THREAD -import org.wordpress.android.repositories.EditorSettingsRepository -import org.wordpress.android.ui.accounts.login.SiteApiRestUrlRecoverer +import org.wordpress.android.repositories.SiteProvisioningSource import org.wordpress.android.util.AppLog import org.wordpress.gutenberg.model.EditorDependencies import java.util.concurrent.ConcurrentHashMap @@ -63,8 +63,8 @@ class GutenbergEditorPreloader @Inject constructor( private val gutenbergKitSettingsBuilder: GutenbergKitSettingsBuilder, private val siteSettingsProvider: SiteSettingsProvider, private val editorServiceProvider: EditorServiceProvider, - private val editorSettingsRepository: EditorSettingsRepository, - private val siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer, + private val siteProvisioningSource: SiteProvisioningSource, + private val siteStore: SiteStore, @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher ) { private sealed class PreloadState { @@ -95,12 +95,14 @@ class GutenbergEditorPreloader @Inject constructor( val siteId = site.id val job = scope.launch(bgDispatcher) { try { - if (site.wpApiRestUrl.isNullOrEmpty()) { - siteApiRestUrlRecoverer.discoverApiRootUrl(site.url) - ?.let { site.wpApiRestUrl = it } - } - editorSettingsRepository - .fetchEditorCapabilitiesForSite(site) + // The provisioning source mints creds, recovers the REST root, and + // detects capabilities — one shared, deduplicated run, so the preloader + // and connectivity banner can't double-probe. We only need it to have + // run; re-read the provisioned site so the config points at the + // recovered REST root. + siteProvisioningSource.await(site) + val provisionedSite = siteStore.sites + .firstOrNull { it.id == siteId } ?: site // Preloading produces EditorDependencies, which the editor // consumes alongside its own per-launch EditorConfiguration. // Cookies and network-logging are per-launch concerns the @@ -108,7 +110,7 @@ class GutenbergEditorPreloader @Inject constructor( // defaults here. val config = gutenbergKitSettingsBuilder .buildPostConfiguration( - site = site, + site = provisionedSite, accessToken = accountStore.accessToken, cookies = emptyMap(), isNetworkLoggingEnabled = false, @@ -146,6 +148,9 @@ class GutenbergEditorPreloader @Inject constructor( @MainThread fun refreshPreloading(site: SiteModel, scope: CoroutineScope) { clearSite(site) + // Pull-to-refresh: force a fresh provisioning run so the await in + // preloadIfNeeded re-detects instead of returning the cached result. + siteProvisioningSource.invalidate(site) preloadIfNeeded(site, scope) } diff --git a/WordPress/src/test/java/org/wordpress/android/repositories/EditorSettingsRepositoryTest.kt b/WordPress/src/test/java/org/wordpress/android/repositories/EditorSettingsRepositoryTest.kt index c4371b8675fb..a3bd6301355c 100644 --- a/WordPress/src/test/java/org/wordpress/android/repositories/EditorSettingsRepositoryTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/repositories/EditorSettingsRepositoryTest.kt @@ -229,6 +229,36 @@ class EditorSettingsRepositoryTest : BaseUnitTest() { verify(wpApiClientProvider, never()).getWpApiClient(atomicSite) } + @Test + fun `jetpack site probes the direct host, not the WP_com proxy`() = + runTest { + val jetpackSite = SiteModel().apply { + id = 7 + url = "https://jetpack.example.com" + // isUsingWpComRestApi via Jetpack, but not Atomic and not WP.com Simple → direct host. + setIsJetpackConnected(true) + setOrigin(SiteModel.ORIGIN_WPCOM_REST) + } + mockDiscoverySuccess( + siteUrl = jetpackSite.url, + hasEditorSettings = true, + hasEditorAssets = true + ) + whenever(themeRepository.fetchCurrentTheme(jetpackSite)) + .thenReturn(buildTheme(isBlockTheme = false)) + + val result = + repository.fetchEditorCapabilitiesForSite(jetpackSite) + + assertThat(result).isTrue() + verify(appPrefsWrapper) + .setSiteSupportsEditorSettings(jetpackSite, true) + verify(appPrefsWrapper) + .setSiteSupportsEditorAssets(jetpackSite, true) + // The proxy is only for minting — capability detection goes direct. + verify(wpApiClientProvider, never()).getWpApiClient(jetpackSite) + } + @Test fun `atomic site returns false when discovery fails`() = runTest { diff --git a/WordPress/src/test/java/org/wordpress/android/repositories/SiteProvisioningSourceTest.kt b/WordPress/src/test/java/org/wordpress/android/repositories/SiteProvisioningSourceTest.kt new file mode 100644 index 000000000000..aa44d02a5e42 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/repositories/SiteProvisioningSourceTest.kt @@ -0,0 +1,288 @@ +package org.wordpress.android.repositories + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +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.wpapi.applicationpasswords.ApplicationPasswordCredentials +import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider +import org.wordpress.android.fluxc.store.SiteStore +import org.wordpress.android.fluxc.store.SiteStore.OnApplicationPasswordCreated +import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper +import org.wordpress.android.ui.accounts.login.SiteApiRestUrlRecoverer +import org.wordpress.android.ui.accounts.login.SiteXmlRpcUrlRecoverer +import org.wordpress.android.ui.mysite.cards.applicationpassword.ApplicationPasswordValidator +import org.wordpress.android.util.NetworkUtilsWrapper + +private const val TEST_SITE_LOCAL_ID = 7 + +@ExperimentalCoroutinesApi +class SiteProvisioningSourceTest : BaseUnitTest(StandardTestDispatcher()) { + @Mock lateinit var siteStore: SiteStore + @Mock lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper + @Mock lateinit var applicationPasswordValidator: ApplicationPasswordValidator + @Mock lateinit var wpApiClientProvider: WpApiClientProvider + @Mock lateinit var siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer + @Mock lateinit var siteXmlRpcUrlRecoverer: SiteXmlRpcUrlRecoverer + @Mock lateinit var editorSettingsRepository: EditorSettingsRepository + @Mock lateinit var networkUtilsWrapper: NetworkUtilsWrapper + @Mock lateinit var appLogWrapper: AppLogWrapper + + private lateinit var site: SiteModel + private lateinit var source: SiteProvisioningSource + + @Before + fun setUp() { + site = SiteModel().apply { + id = TEST_SITE_LOCAL_ID + url = "https://test.example.com" + // Non-null REST root + XML-RPC url so both recovery branches short-circuit unless a + // test clears them — keeping the auth/capability tests focused. + wpApiRestUrl = "https://test.example.com/wp-json" + xmlRpcUrl = "https://test.example.com/xmlrpc.php" + } + whenever(siteStore.getSiteByLocalId(TEST_SITE_LOCAL_ID)).thenReturn(site) + source = SiteProvisioningSource( + siteStore, + applicationPasswordLoginHelper, + applicationPasswordValidator, + wpApiClientProvider, + siteApiRestUrlRecoverer, + siteXmlRpcUrlRecoverer, + editorSettingsRepository, + networkUtilsWrapper, + appLogWrapper, + testScope(), + ) + } + + private fun stubHasStoredCredentials(value: Boolean) = + whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(!value) + + private suspend fun stubValidate(outcome: ApplicationPasswordValidator.Outcome) = + whenever(applicationPasswordValidator.validate(any())).thenReturn(outcome) + + private suspend fun stubMintSuccess() = + whenever(siteStore.createApplicationPassword(any())).thenReturn( + OnApplicationPasswordCreated(site, ApplicationPasswordCredentials("user", "pass", uuid = "u")) + ) + + private suspend fun stubMintFailure() = + whenever(siteStore.createApplicationPassword(any())).thenReturn( + OnApplicationPasswordCreated(site, BaseNetworkError(GenericErrorType.UNKNOWN, "fail"), notSupported = false) + ) + + private suspend fun stubCapabilityProbe(ok: Boolean, cached: Boolean = false) { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(any())).thenReturn(ok) + // `ok || hasCache` short-circuits, so only stub the cache when the live probe failed. + if (!ok) whenever(editorSettingsRepository.hasCachedCapabilities(any())).thenReturn(cached) + } + + // region auth stage + + @Test + fun `given valid stored credentials, then provisioned and ready`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + stubCapabilityProbe(ok = true) + + assertThat(source.await(site)).isEqualTo(SiteReadiness.Ready) + verify(siteStore, never()).createApplicationPassword(any()) + } + + @Test + fun `given no credentials, then mints and is ready`() = test { + stubHasStoredCredentials(false) + stubMintSuccess() + stubCapabilityProbe(ok = true) + + assertThat(source.await(site)).isEqualTo(SiteReadiness.Ready) + verify(siteStore).createApplicationPassword(any()) + } + + @Test + fun `given mint fails with no prior credentials, then needs first-time auth`() = test { + stubHasStoredCredentials(false) + stubMintFailure() + + val result = source.await(site) + + assertThat(result).isEqualTo(SiteReadiness.NeedsAuth(SiteAuthState.Unprovisionable(hadCredentials = false))) + verify(editorSettingsRepository, never()).fetchEditorCapabilitiesForSite(any()) + } + + @Test + fun `given stored credentials that fail to mint, then needs re-auth`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Invalid) + stubMintFailure() + + val result = source.await(site) + + assertThat(result).isEqualTo(SiteReadiness.NeedsAuth(SiteAuthState.Unprovisionable(hadCredentials = true))) + verify(siteStore).deleteStoredApplicationPasswordCredentials(any()) + } + + @Test + fun `given a transient validation error, then provisioning and no mint or probe`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.NetworkUnavailable) + + val result = source.await(site) + + assertThat(result).isEqualTo(SiteReadiness.NeedsAuth(SiteAuthState.Provisioning)) + verify(siteStore, never()).createApplicationPassword(any()) + verify(editorSettingsRepository, never()).fetchEditorCapabilitiesForSite(any()) + } + + @Test + fun `given the site is gone from the store, then unprovisionable`() = test { + whenever(siteStore.getSiteByLocalId(TEST_SITE_LOCAL_ID)).thenReturn(null) + + val result = source.await(site) + + assertThat(result).isEqualTo(SiteReadiness.NeedsAuth(SiteAuthState.Unprovisionable(hadCredentials = false))) + } + + @Test + fun `given a WPCom Simple site, then it detects capabilities without minting`() = test { + val simple = SiteModel().apply { + id = TEST_SITE_LOCAL_ID + url = "https://simple.wordpress.com" + setIsWPCom(true) // isWPComSimpleSite = isWPCom && !isWPComAtomic + wpApiRestUrl = "https://simple.wordpress.com/wp-json" + } + whenever(siteStore.getSiteByLocalId(TEST_SITE_LOCAL_ID)).thenReturn(simple) + stubCapabilityProbe(ok = true) + + assertThat(source.await(simple)).isEqualTo(SiteReadiness.Ready) + // No application password applies — it must not validate or mint, just probe via the proxy. + verify(applicationPasswordValidator, never()).validate(any()) + verify(siteStore, never()).createApplicationPassword(any()) + } + + // endregion + + // region recovery + capability stages + + @Test + fun `given provisioned with a missing REST root, then it recovers and persists the url`() = test { + site.wpApiRestUrl = "" + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(site.url)) + .thenReturn("https://test.example.com/custom-rest") + stubCapabilityProbe(ok = true) + + source.await(site) + + verify(siteApiRestUrlRecoverer).persistApiRootUrl(eq(site.id), eq("https://test.example.com/custom-rest")) + } + + @Test + fun `given a provisioned self-hosted site without XML-RPC, then it recovers and persists xmlRpcUrl`() = test { + val selfHosted = SiteModel().apply { + id = TEST_SITE_LOCAL_ID + url = "https://selfhosted.example.com" + wpApiRestUrl = "https://selfhosted.example.com/wp-json" // REST branch short-circuits + // not WP.com and no xmlRpcUrl → the XML-RPC branch runs + } + whenever(siteStore.getSiteByLocalId(TEST_SITE_LOCAL_ID)).thenReturn(selfHosted) + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + stubCapabilityProbe(ok = true) + whenever(siteXmlRpcUrlRecoverer.discoverAndVerifyXmlRpcUrl(selfHosted)) + .thenReturn("https://selfhosted.example.com/xmlrpc.php") + + source.await(selfHosted) + + verify(siteXmlRpcUrlRecoverer) + .persistXmlRpcUrl(eq(TEST_SITE_LOCAL_ID), eq("https://selfhosted.example.com/xmlrpc.php")) + } + + @Test + fun `given probe fails while offline, then transient error`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + stubCapabilityProbe(ok = false) + whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(false) + + assertThat(source.await(site)).isEqualTo(SiteReadiness.TransientError) + } + + @Test + fun `given probe fails while online, then unreachable`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + stubCapabilityProbe(ok = false) + whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(true) + + assertThat(source.await(site)).isEqualTo(SiteReadiness.Unreachable) + } + + @Test + fun `given probe fails but capabilities are cached, then ready`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + stubCapabilityProbe(ok = false, cached = true) + + assertThat(source.await(site)).isEqualTo(SiteReadiness.Ready) + } + + // endregion + + // region single-flight / dedup / invalidate / clear + + @Test + fun `given a prior ready run, when awaited again, then it does not re-run`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + stubCapabilityProbe(ok = true) + + source.await(site) + source.await(site) + + verify(applicationPasswordValidator, times(1)).validate(any()) + } + + @Test + fun `given a prior ready run, when invalidated, then it re-runs`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + stubCapabilityProbe(ok = true) + + source.await(site) + source.invalidate(site) + source.await(site) + + verify(applicationPasswordValidator, times(2)).validate(any()) + } + + @Test + fun `given a ready site, when cleared, then the next run re-runs`() = test { + stubHasStoredCredentials(true) + stubValidate(ApplicationPasswordValidator.Outcome.Valid) + stubCapabilityProbe(ok = true) + + source.await(site) + source.clear() + source.await(site) + + verify(applicationPasswordValidator, times(2)).validate(any()) + } + + // endregion +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt index 6c97ee23eb3b..b1df5c270090 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt @@ -76,9 +76,6 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var wpApiClientProvider: WpApiClientProvider - @Mock - lateinit var credentialsChangedNotifier: CredentialsChangedNotifier - private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -95,8 +92,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { apiRootUrlCache, discoverSuccessWrapper, crashLogging, - wpApiClientProvider, - credentialsChangedNotifier + wpApiClientProvider ) } @@ -210,7 +206,6 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { verify(siteStore).sites verify(dispatcherWrapper).updateApplicationPassword(eq(siteModel)) verify(wpApiClientProvider).clearSelfHostedClient(eq(siteModel.id)) - verify(credentialsChangedNotifier).notifyChanged(eq(siteModel.id)) } @Test diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteXmlRpcUrlRecovererTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteXmlRpcUrlRecovererTest.kt new file mode 100644 index 000000000000..3ce345c0fc41 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteXmlRpcUrlRecovererTest.kt @@ -0,0 +1,107 @@ +package org.wordpress.android.ui.accounts.login + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.junit.MockitoJUnitRunner +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.model.SitesModel +import org.wordpress.android.fluxc.network.BaseRequest.BaseNetworkError +import org.wordpress.android.fluxc.network.BaseRequest.GenericErrorType +import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder +import org.wordpress.android.fluxc.network.xmlrpc.site.SiteXMLRPCClient +import org.wordpress.android.fluxc.persistence.SiteSqlUtils +import org.wordpress.android.fluxc.utils.AppLogWrapper + +private const val ENDPOINT = "https://selfhosted.example.com/xmlrpc.php" +private const val SITE_LOCAL_ID = 5 + +@ExperimentalCoroutinesApi +@RunWith(MockitoJUnitRunner::class) +class SiteXmlRpcUrlRecovererTest : BaseUnitTest() { + @Mock lateinit var selfHostedEndpointFinder: SelfHostedEndpointFinder + @Mock lateinit var siteXMLRPCClient: SiteXMLRPCClient + @Mock lateinit var siteSqlUtils: SiteSqlUtils + @Mock lateinit var appLogWrapper: AppLogWrapper + + private lateinit var site: SiteModel + private lateinit var recoverer: SiteXmlRpcUrlRecoverer + + @Before + fun setUp() { + site = SiteModel().apply { + id = SITE_LOCAL_ID + url = "https://selfhosted.example.com" + apiRestUsernamePlain = "user" + apiRestPasswordPlain = "pass" + } + recoverer = SiteXmlRpcUrlRecoverer( + selfHostedEndpointFinder, + siteXMLRPCClient, + siteSqlUtils, + appLogWrapper, + testDispatcher(), + ) + } + + @Test + fun `given discovery and authenticated verify succeed, then returns the endpoint`() = test { + whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url)).thenReturn(ENDPOINT) + // The site's stored credentials are forwarded to the authenticated verify call. + whenever(siteXMLRPCClient.fetchSites(eq(ENDPOINT), eq("user"), eq("pass"))) + .thenReturn(SitesModel(listOf(SiteModel()))) + + assertThat(recoverer.discoverAndVerifyXmlRpcUrl(site)).isEqualTo(ENDPOINT) + } + + @Test + fun `given discovery throws, then returns null`() = test { + whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url)) + .thenThrow(mock()) + + assertThat(recoverer.discoverAndVerifyXmlRpcUrl(site)).isNull() + } + + @Test + fun `given discovery throws an unexpected exception, then returns null`() = test { + // A non-DiscoveryException (e.g. a RuntimeException from the network/parse path) must be + // contained, not propagated — otherwise it cancels the whole provisioning pipeline. + whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url)) + .thenThrow(RuntimeException("unexpected")) + + assertThat(recoverer.discoverAndVerifyXmlRpcUrl(site)).isNull() + } + + @Test + fun `given the authenticated verify errors, then returns null`() = test { + whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(site.url)).thenReturn(ENDPOINT) + whenever(siteXMLRPCClient.fetchSites(eq(ENDPOINT), any(), any())) + .thenReturn(SitesModel().apply { error = BaseNetworkError(GenericErrorType.UNKNOWN, "x") }) + + assertThat(recoverer.discoverAndVerifyXmlRpcUrl(site)).isNull() + } + + @Test + fun `given persist updates a row, then returns true`() = test { + whenever(siteSqlUtils.updateXmlRpcUrl(eq(SITE_LOCAL_ID), eq(ENDPOINT))).thenReturn(1) + + assertThat(recoverer.persistXmlRpcUrl(SITE_LOCAL_ID, ENDPOINT)).isTrue + verify(siteSqlUtils).updateXmlRpcUrl(eq(SITE_LOCAL_ID), eq(ENDPOINT)) + } + + @Test + fun `given persist matches no row, then returns false`() = test { + whenever(siteSqlUtils.updateXmlRpcUrl(eq(SITE_LOCAL_ID), eq(ENDPOINT))).thenReturn(0) + + assertThat(recoverer.persistXmlRpcUrl(SITE_LOCAL_ID, ENDPOINT)).isFalse + } +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSliceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSliceTest.kt index 6276f50d4a58..090966a4b3e1 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSliceTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSliceTest.kt @@ -1,9 +1,9 @@ package org.wordpress.android.ui.mysite.cards.applicationpassword import junit.framework.TestCase.assertNull -import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.flow.MutableStateFlow +import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -11,435 +11,129 @@ import org.mockito.Mock import org.mockito.MockitoAnnotations import org.mockito.junit.MockitoJUnitRunner import org.mockito.kotlin.any -import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.never -import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest -import org.mockito.kotlin.mock +import org.wordpress.android.R import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.fluxc.model.SitesModel -import org.wordpress.android.fluxc.network.BaseRequest.BaseNetworkError -import org.wordpress.android.fluxc.network.BaseRequest.GenericErrorType -import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder -import org.wordpress.android.fluxc.network.xmlrpc.site.SiteXMLRPCClient -import org.wordpress.android.fluxc.network.rest.wpapi.applicationpasswords.ApplicationPasswordCredentials -import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider import org.wordpress.android.fluxc.store.SiteStore -import org.wordpress.android.fluxc.store.SiteStore.OnApplicationPasswordCreated import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.repositories.SiteAuthState +import org.wordpress.android.repositories.SiteProvisioningSource +import org.wordpress.android.repositories.SiteReadiness import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper -import org.wordpress.android.ui.accounts.login.CredentialsChangedNotifier -import org.wordpress.android.ui.accounts.login.SiteApiRestUrlRecoverer import org.wordpress.android.ui.mysite.MySiteCardAndItem -import kotlin.test.assertNotNull private const val TEST_URL = "https://www.test.com" -private const val TEST_SITE_NAME = "My Site" private const val TEST_SITE_ID = 1 -private const val TEST_SITE_ICON = "http://site.com/icon.jpg" -private const val TEST_URL_AUTH = "https://www.test.com/auth" -private const val TEST_URL_AUTH_SUFFIX = "?app_name=android-jetpack-client&success_url=callback://callback" +private const val TEST_AUTH_URL = "https://www.test.com/auth" @ExperimentalCoroutinesApi @RunWith(MockitoJUnitRunner::class) class ApplicationPasswordViewModelSliceTest : BaseUnitTest() { - @Mock - lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper - - @Mock - lateinit var siteStore: SiteStore - - @Mock - lateinit var appLogWrapper: AppLogWrapper - - @Mock - lateinit var wpApiClientProvider: WpApiClientProvider - - @Mock - lateinit var applicationPasswordValidator: ApplicationPasswordValidator - - @Mock - lateinit var selfHostedEndpointFinder: SelfHostedEndpointFinder - - @Mock - lateinit var siteXMLRPCClient: SiteXMLRPCClient - - @Mock - lateinit var siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer - - @Mock - lateinit var credentialsChangedNotifier: CredentialsChangedNotifier + @Mock lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper + @Mock lateinit var siteStore: SiteStore + @Mock lateinit var appLogWrapper: AppLogWrapper + @Mock lateinit var siteProvisioningSource: SiteProvisioningSource private lateinit var siteTest: SiteModel - - private var applicationPasswordCard: MySiteCardAndItem? = null - - private lateinit var applicationPasswordViewModelSlice: ApplicationPasswordViewModelSlice + private var card: MySiteCardAndItem? = null + private lateinit var slice: ApplicationPasswordViewModelSlice @Before fun setUp() { MockitoAnnotations.openMocks(this) - - applicationPasswordViewModelSlice = ApplicationPasswordViewModelSlice( + slice = ApplicationPasswordViewModelSlice( applicationPasswordLoginHelper, siteStore, appLogWrapper, - wpApiClientProvider, - applicationPasswordValidator, - selfHostedEndpointFinder, - siteXMLRPCClient, - siteApiRestUrlRecoverer, - credentialsChangedNotifier, - testDispatcher() - ).apply { - initialize(testScope()) - } + siteProvisioningSource, + ).apply { initialize(testScope()) } siteTest = SiteModel().apply { id = TEST_SITE_ID url = TEST_URL - name = TEST_SITE_NAME - iconUrl = TEST_SITE_ICON - siteId = TEST_SITE_ID.toLong() - apiRestUsernamePlain = "testuser" - apiRestPasswordPlain = "testpass" - // Mark xmlRpcUrl so handleValidAuth's XML-RPC-disabled fallback doesn't fire — that - // path is exercised by the dedicated xmlRpcRediscovery tests below. - xmlRpcUrl = "https://www.test.com/xmlrpc.php" } - - applicationPasswordCard = null - applicationPasswordViewModelSlice.uiModel.observeForever { card -> - applicationPasswordCard = card - } - - // By default, treat the site as having no stored credentials. Tests that exercise the - // validate-stored-creds path override this. - whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(true) + card = null + slice.uiModel.observeForever { card = it } } - private suspend fun stubMintFailure(notSupported: Boolean = false) { - whenever(siteStore.createApplicationPassword(any())).thenReturn( - OnApplicationPasswordCreated( - siteTest, - BaseNetworkError(GenericErrorType.UNKNOWN, "fail"), - notSupported = notSupported, - ) - ) - } - - private suspend fun stubMintSuccess() { - whenever(siteStore.createApplicationPassword(any())).thenReturn( - OnApplicationPasswordCreated( - siteTest, - ApplicationPasswordCredentials("user", "pass", uuid = "u") - ) - ) + private fun stubReadiness(readiness: SiteReadiness): MutableStateFlow { + val flow = MutableStateFlow(readiness) + whenever(siteProvisioningSource.stateFor(siteTest)).thenReturn(flow) + return flow } - @Test - fun `given proper site, when api discovery is success, then add the application password card`() = runTest { - stubMintFailure() + private suspend fun stubAuthorized() = whenever(applicationPasswordLoginHelper.getAuthorizationUrlComplete(eq(TEST_URL))) - .thenReturn( - ApplicationPasswordLoginHelper.DiscoveryResult.Authorized("$TEST_URL_AUTH$TEST_URL_AUTH_SUFFIX") - ) - - applicationPasswordViewModelSlice.buildCard(siteTest) - - assertNotNull(applicationPasswordCard) - verify(applicationPasswordLoginHelper).getAuthorizationUrlComplete(eq(TEST_URL)) - } - - @Test - fun `given login scenario, when api discovery is empty, then show no card`() = runTest { - stubMintFailure() - whenever(applicationPasswordLoginHelper.getAuthorizationUrlComplete(eq(TEST_URL))) - .thenReturn(ApplicationPasswordLoginHelper.DiscoveryResult.Failed("test discovery failure")) - - applicationPasswordViewModelSlice.buildCard(siteTest) - - assertNull(applicationPasswordCard) - verify(applicationPasswordLoginHelper).getAuthorizationUrlComplete(eq(TEST_URL)) - } - - @Test - fun `given headless mint succeeds, then hide card and skip discovery`() = runTest { - stubMintSuccess() - - applicationPasswordViewModelSlice.buildCard(siteTest) - - assertNull(applicationPasswordCard) - verify(siteStore).createApplicationPassword(any()) - verify(applicationPasswordLoginHelper, never()).getAuthorizationUrlComplete(any()) - } - - @Test - fun `given headless mint succeeds, then notify credentials changed`() = runTest { - stubMintSuccess() - - applicationPasswordViewModelSlice.buildCard(siteTest) - - verify(credentialsChangedNotifier).notifyChanged(TEST_SITE_ID) - } + .thenReturn(ApplicationPasswordLoginHelper.DiscoveryResult.Authorized(TEST_AUTH_URL)) @Test - fun `given headless mint succeeds, card hides without waiting for the recoverer`() = runTest { - stubMintSuccess() - val recoverGate = CompletableDeferred() - whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(any())) - .doSuspendableAnswer { recoverGate.await(); null } + fun `given unprovisionable without prior creds, then show the create card`() = test { + stubReadiness(SiteReadiness.NeedsAuth(SiteAuthState.Unprovisionable(hadCredentials = false))) + stubAuthorized() - applicationPasswordViewModelSlice.buildCard(siteTest) + slice.buildCard(siteTest) - // Card has been hidden even though the recoverer is still suspended on the gate. - assertNull(applicationPasswordCard) - verify(siteApiRestUrlRecoverer).discoverApiRootUrl(siteTest.url) - - // Release the recoverer so the test scope doesn't carry a dangling coroutine. - recoverGate.complete(Unit) + assertThat(card).isInstanceOf(MySiteCardAndItem.Card.QuickLinksItem::class.java) } @Test - fun `given valid stored creds, card hides without waiting for the recoverer`() = runTest { - whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(false) - whenever(siteStore.sites).thenReturn( - listOf( - SiteModel().apply { - id = siteTest.id - url = TEST_URL - apiRestUsernamePlain = "user" - apiRestPasswordPlain = "password" - xmlRpcUrl = siteTest.xmlRpcUrl - } - ) - ) - whenever(applicationPasswordValidator.validate(any())) - .thenReturn(ApplicationPasswordValidator.Outcome.Valid) - val recoverGate = CompletableDeferred() - whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(any())) - .doSuspendableAnswer { recoverGate.await(); null } + fun `given unprovisionable with prior creds, then show the reauthentication card`() = test { + stubReadiness(SiteReadiness.NeedsAuth(SiteAuthState.Unprovisionable(hadCredentials = true))) + stubAuthorized() - applicationPasswordViewModelSlice.buildCard(siteTest) + slice.buildCard(siteTest) - assertNull(applicationPasswordCard) - verify(siteApiRestUrlRecoverer).discoverApiRootUrl(TEST_URL) - - recoverGate.complete(Unit) + val banner = card as MySiteCardAndItem.Item.SingleActionCard + assertThat(banner.textResource).isEqualTo(R.string.application_password_reauthentication_banner) } @Test - fun `given headless mint returns NotSupported, then fall back to discovery`() = runTest { - stubMintFailure(notSupported = true) + fun `given unprovisionable but discovery fails, then no card`() = test { + stubReadiness(SiteReadiness.NeedsAuth(SiteAuthState.Unprovisionable(hadCredentials = false))) whenever(applicationPasswordLoginHelper.getAuthorizationUrlComplete(eq(TEST_URL))) - .thenReturn( - ApplicationPasswordLoginHelper.DiscoveryResult.Authorized("$TEST_URL_AUTH$TEST_URL_AUTH_SUFFIX") - ) + .thenReturn(ApplicationPasswordLoginHelper.DiscoveryResult.Failed("bad discovery")) - applicationPasswordViewModelSlice.buildCard(siteTest) + slice.buildCard(siteTest) - assertNotNull(applicationPasswordCard) - verify(siteStore).createApplicationPassword(any()) - verify(applicationPasswordLoginHelper).getAuthorizationUrlComplete(eq(TEST_URL)) - verify(credentialsChangedNotifier, never()).notifyChanged(any()) + assertNull(card) } @Test - fun `given site already authenticated and validation succeeds, then show no card`() = runTest { - whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(false) - whenever(siteStore.sites).thenReturn( - listOf( - SiteModel().apply { - id = siteTest.id - url = TEST_URL - apiRestUsernamePlain = "user" - apiRestPasswordPlain = "password" - xmlRpcUrl = siteTest.xmlRpcUrl - } - ) - ) - whenever(applicationPasswordValidator.validate(any())) - .thenReturn(ApplicationPasswordValidator.Outcome.Valid) + fun `given provisioning, then no card`() = test { + stubReadiness(SiteReadiness.NeedsAuth(SiteAuthState.Provisioning)) - applicationPasswordViewModelSlice.buildCard(siteTest) + slice.buildCard(siteTest) - assertNull(applicationPasswordCard) - verify(applicationPasswordValidator).validate(any()) - verify(siteStore, never()).createApplicationPassword(any()) - verify(applicationPasswordLoginHelper, times(0)).getAuthorizationUrlComplete(any()) + assertNull(card) } @Test - fun `given stored creds invalid, clear them and try headless mint`() = runTest { - whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(false) - whenever(siteStore.sites).thenReturn( - listOf( - SiteModel().apply { - id = siteTest.id - url = TEST_URL - apiRestUsernamePlain = "stale-user" - apiRestPasswordPlain = "stale-pass" - xmlRpcUrl = siteTest.xmlRpcUrl - } - ) + fun `given ready on a WPCom-REST site, then no card`() = test { + stubReadiness(SiteReadiness.Ready) + whenever(siteStore.getSiteByLocalId(TEST_SITE_ID)).thenReturn( + SiteModel().apply { id = TEST_SITE_ID; url = TEST_URL; setIsWPCom(true) } ) - whenever(applicationPasswordValidator.validate(any())) - .thenReturn(ApplicationPasswordValidator.Outcome.Invalid) - stubMintSuccess() - applicationPasswordViewModelSlice.buildCard(siteTest) + slice.buildCard(siteTest) - assertNull(applicationPasswordCard) - verify(siteStore).deleteStoredApplicationPasswordCredentials(any()) - // clearSelfHostedClient is invoked twice — once on invalidation, once after the fresh mint - verify(wpApiClientProvider, times(2)).clearSelfHostedClient(siteTest.id) - verify(siteStore).createApplicationPassword(any()) - } - - @Test - fun `given stored creds invalid and mint fails, show reauthentication banner`() = runTest { - whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(false) - whenever(siteStore.sites).thenReturn( - listOf( - SiteModel().apply { - id = siteTest.id - url = TEST_URL - apiRestUsernamePlain = "stale-user" - apiRestPasswordPlain = "stale-pass" - xmlRpcUrl = siteTest.xmlRpcUrl - } - ) - ) - whenever(applicationPasswordValidator.validate(any())) - .thenReturn(ApplicationPasswordValidator.Outcome.Invalid) - stubMintFailure(notSupported = true) - whenever(applicationPasswordLoginHelper.getAuthorizationUrlComplete(eq(TEST_URL))) - .thenReturn( - ApplicationPasswordLoginHelper.DiscoveryResult.Authorized("$TEST_URL_AUTH$TEST_URL_AUTH_SUFFIX") - ) - - applicationPasswordViewModelSlice.buildCard(siteTest) - - assertNotNull(applicationPasswordCard) - // Reauth banner uses SingleActionCard (not the QuickLinksItem create card) - assert(applicationPasswordCard is MySiteCardAndItem.Item.SingleActionCard) + assertNull(card) + verify(applicationPasswordLoginHelper, never()).getAuthorizationUrlComplete(any()) } @Test - fun `given validation hits a network error, leave the card hidden without re-minting`() = runTest { - whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(false) - whenever(siteStore.sites).thenReturn( - listOf( - SiteModel().apply { - id = siteTest.id - url = TEST_URL - apiRestUsernamePlain = "user" - apiRestPasswordPlain = "pass" - xmlRpcUrl = siteTest.xmlRpcUrl - } - ) + fun `given ready on a self-hosted site whose XML-RPC stayed unrecovered, then show the disabled card`() = test { + stubReadiness(SiteReadiness.Ready) + // Not WP.com-REST and still no XML-RPC endpoint after the pipeline's recovery attempt. + whenever(siteStore.getSiteByLocalId(TEST_SITE_ID)).thenReturn( + SiteModel().apply { id = TEST_SITE_ID; url = TEST_URL } ) - whenever(applicationPasswordValidator.validate(any())) - .thenReturn(ApplicationPasswordValidator.Outcome.NetworkUnavailable) - applicationPasswordViewModelSlice.buildCard(siteTest) + slice.buildCard(siteTest) - assertNull(applicationPasswordCard) - verify(siteStore, never()).createApplicationPassword(any()) - verify(siteStore, never()).deleteStoredApplicationPasswordCredentials(any()) + val xmlRpcCard = card as MySiteCardAndItem.Item.SingleActionCard + assertThat(xmlRpcCard.textResource).isEqualTo(R.string.xmlrpc_disabled_card_text) } - - @Test - fun `concurrent buildCard calls coalesce to a single mint`() = runTest { - // Gate the mint so the first buildCard suspends mid-call and the second arrives while it's - // still in flight. Without the single-flight guard, both calls would issue separate - // server-side mints and race the 409 conflict handler in ApplicationPasswordsManager. - val mintGate = CompletableDeferred() - whenever(siteStore.createApplicationPassword(any())).doSuspendableAnswer { - mintGate.await() - OnApplicationPasswordCreated( - siteTest, - ApplicationPasswordCredentials("user", "pass", uuid = "u") - ) - } - - applicationPasswordViewModelSlice.buildCard(siteTest) - applicationPasswordViewModelSlice.buildCard(siteTest) - - mintGate.complete(Unit) - advanceUntilIdle() - - verify(siteStore, times(1)).createApplicationPassword(any()) - } - - @Test - fun `given xmlRpc rediscovery and auth check succeed, then persist the discovered xmlRpcUrl`() = - runTest { - // @Before seeds siteTest.xmlRpcUrl; clear it so the final assertion proves rediscovery set it. - siteTest.xmlRpcUrl = null - val xmlRpcUrl = "https://www.test.com/xmlrpc.php" - whenever( - selfHostedEndpointFinder - .verifyOrDiscoverXMLRPCEndpoint(TEST_URL) - ).thenReturn(xmlRpcUrl) - whenever( - siteXMLRPCClient.fetchSites( - eq(xmlRpcUrl), any(), any() - ) - ).thenReturn(SitesModel(listOf(SiteModel()))) - whenever(siteStore.persistXmlRpcUrl(any(), any())).thenReturn(SiteStore.OnSiteChanged(0)) - - applicationPasswordViewModelSlice - .attemptXmlRpcRediscovery(siteTest) - - verify(siteStore).persistXmlRpcUrl(siteTest.id, xmlRpcUrl) - assert(siteTest.xmlRpcUrl == xmlRpcUrl) - } - - @Test - fun `given xmlRpc rediscovery succeeds but auth check fails, then do not persist`() = - runTest { - siteTest.xmlRpcUrl = null - val xmlRpcUrl = "https://www.test.com/xmlrpc.php" - whenever( - selfHostedEndpointFinder - .verifyOrDiscoverXMLRPCEndpoint(TEST_URL) - ).thenReturn(xmlRpcUrl) - val errorResult = SitesModel().apply { - error = mock() - } - whenever( - siteXMLRPCClient.fetchSites( - eq(xmlRpcUrl), any(), any() - ) - ).thenReturn(errorResult) - - applicationPasswordViewModelSlice - .attemptXmlRpcRediscovery(siteTest) - - verify(siteStore, never()).persistXmlRpcUrl(any(), any()) - assert(siteTest.xmlRpcUrl.isNullOrEmpty()) - } - - @Test - fun `given xmlRpc rediscovery fails, then do not persist`() = - runTest { - siteTest.xmlRpcUrl = null - whenever( - selfHostedEndpointFinder - .verifyOrDiscoverXMLRPCEndpoint(TEST_URL) - ).thenThrow( - mock() - ) - - applicationPasswordViewModelSlice - .attemptXmlRpcRediscovery(siteTest) - - verify(selfHostedEndpointFinder) - .verifyOrDiscoverXMLRPCEndpoint(TEST_URL) - verify(siteStore, never()).persistXmlRpcUrl(any(), any()) - assert(siteTest.xmlRpcUrl.isNullOrEmpty()) - } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSliceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSliceTest.kt index 59ed881f498e..a1e9fa9b3d51 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSliceTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSliceTest.kt @@ -1,29 +1,25 @@ package org.wordpress.android.ui.mysite.cards.connectivity -import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.advanceUntilIdle import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock -import org.mockito.Mockito.lenient import org.mockito.junit.MockitoJUnitRunner -import org.mockito.kotlin.doSuspendableAnswer -import org.mockito.kotlin.eq -import org.mockito.kotlin.times +import org.mockito.kotlin.any +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest import org.wordpress.android.R import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.repositories.EditorSettingsRepository -import org.wordpress.android.ui.accounts.login.CredentialsChangedNotifier +import org.wordpress.android.repositories.SiteAuthState +import org.wordpress.android.repositories.SiteProvisioningSource +import org.wordpress.android.repositories.SiteReadiness import org.wordpress.android.ui.mysite.MySiteCardAndItem -import org.wordpress.android.ui.mysite.SelectedSiteRepository -import org.wordpress.android.util.NetworkUtilsWrapper private const val TEST_SITE_LOCAL_ID = 42 @@ -31,18 +27,7 @@ private const val TEST_SITE_LOCAL_ID = 42 @RunWith(MockitoJUnitRunner::class) class SiteConnectivityBannerViewModelSliceTest : BaseUnitTest() { @Mock - lateinit var editorSettingsRepository: EditorSettingsRepository - - @Mock - lateinit var networkUtilsWrapper: NetworkUtilsWrapper - - @Mock - lateinit var credentialsChangedNotifier: CredentialsChangedNotifier - - @Mock - lateinit var selectedSiteRepository: SelectedSiteRepository - - private val credentialsChangedFlow = MutableSharedFlow(extraBufferCapacity = 1) + lateinit var siteProvisioningSource: SiteProvisioningSource private lateinit var siteTest: SiteModel private lateinit var slice: SiteConnectivityBannerViewModelSlice @@ -51,34 +36,23 @@ class SiteConnectivityBannerViewModelSliceTest : BaseUnitTest() { @Before fun setUp() { siteTest = SiteModel().apply { id = TEST_SITE_LOCAL_ID } - // Default network state is available; tests that need offline override per-test. Lenient - // because tests where the fetch succeeds never reach the network check. - lenient().`when`(networkUtilsWrapper.isNetworkAvailable()).thenReturn(true) - whenever(credentialsChangedNotifier.events).thenReturn(credentialsChangedFlow) - slice = SiteConnectivityBannerViewModelSlice( - editorSettingsRepository, - networkUtilsWrapper, - credentialsChangedNotifier, - selectedSiteRepository, - ) + slice = SiteConnectivityBannerViewModelSlice(siteProvisioningSource) slice.initialize(testScope()) slice.uiModel.observeForever { emittedBanners.add(it) } } - @Test - fun `given fetch succeeds, when fetchCapabilities invoked, then banner is null`() = test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) - - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() - - assertThat(emittedBanners.last()).isNull() + private fun stubReadiness( + site: SiteModel, + readiness: SiteReadiness, + ): MutableStateFlow { + val flow = MutableStateFlow(readiness) + whenever(siteProvisioningSource.stateFor(site)).thenReturn(flow) + return flow } @Test - fun `given fetch fails with no cache, when fetchCapabilities invoked, then banner is shown`() = test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + fun `given unreachable, when fetchCapabilities invoked, then banner is shown`() = test { + stubReadiness(siteTest, SiteReadiness.Unreachable) slice.fetchCapabilities(siteTest, isUserInitiated = false) advanceUntilIdle() @@ -89,48 +63,19 @@ class SiteConnectivityBannerViewModelSliceTest : BaseUnitTest() { } @Test - fun `given fetch fails with no cache but device offline, when fetchCapabilities invoked, then banner is null`() = - test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) - whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(false) - - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() - - // Global offline indicator covers this case — suppress to avoid stacked warnings. - assertThat(emittedBanners.last()).isNull() - } - - @Test - fun `given fetch fails but app password pending, when fetchCapabilities invoked, then banner is null`() = - test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.isAwaitingApplicationPassword(siteTest)).thenReturn(true) - - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() - - // Credentials are still being minted — pending, not a connection failure. - assertThat(emittedBanners.last()).isNull() - } - - @Test - fun `when credentials change for the selected site, then capabilities are re-fetched`() = test { - whenever(selectedSiteRepository.getSelectedSite()).thenReturn(siteTest) - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) + fun `given ready, when fetchCapabilities invoked, then banner is null`() = test { + stubReadiness(siteTest, SiteReadiness.Ready) - credentialsChangedFlow.emit(TEST_SITE_LOCAL_ID) + slice.fetchCapabilities(siteTest, isUserInitiated = false) advanceUntilIdle() - verify(editorSettingsRepository).fetchEditorCapabilitiesForSite(siteTest) + assertThat(emittedBanners.last()).isNull() } @Test - fun `given fetch fails but cache exists, when fetchCapabilities invoked, then banner is null`() = test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(true) + fun `given needs auth, when fetchCapabilities invoked, then banner is null`() = test { + // Credentials are the problem — the application-password card owns it, banner stays hidden. + stubReadiness(siteTest, SiteReadiness.NeedsAuth(SiteAuthState.Unprovisionable(hadCredentials = false))) slice.fetchCapabilities(siteTest, isUserInitiated = false) advanceUntilIdle() @@ -139,131 +84,89 @@ class SiteConnectivityBannerViewModelSliceTest : BaseUnitTest() { } @Test - fun `given prior successful fetch, when fetchCapabilities invoked again non-user-initiated, then fetch skipped`() = - test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) + fun `given probing, when fetchCapabilities invoked, then banner is null`() = test { + stubReadiness(siteTest, SiteReadiness.Probing) - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() - - verify(editorSettingsRepository, times(1)).fetchEditorCapabilitiesForSite(siteTest) - } - - @Test - fun `given prior failed fetch, when fetchCapabilities invoked again non-user-initiated, then fetch retries`() = - test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false, true) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) - - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() - assertThat(emittedBanners.last()).isNotNull - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() - verify(editorSettingsRepository, times(2)).fetchEditorCapabilitiesForSite(siteTest) - assertThat(emittedBanners.last()).isNull() - } + assertThat(emittedBanners.last()).isNull() + } @Test - fun `given prior successful fetch, when user-initiated fetchCapabilities invoked, then fetch runs again`() = - test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) + fun `given transient error, when fetchCapabilities invoked, then banner is null`() = test { + stubReadiness(siteTest, SiteReadiness.TransientError) - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() - slice.fetchCapabilities(siteTest, isUserInitiated = true) - advanceUntilIdle() + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() - verify(editorSettingsRepository, times(2)).fetchEditorCapabilitiesForSite(siteTest) - } + assertThat(emittedBanners.last()).isNull() + } @Test - fun `given banner showing, when retry tapped, then fetch runs and bypasses session dedup`() = test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + fun `given unreachable then recovered to ready, when state changes, then banner clears`() = test { + val flow = stubReadiness(siteTest, SiteReadiness.Unreachable) slice.fetchCapabilities(siteTest, isUserInitiated = false) advanceUntilIdle() - val banner = emittedBanners.last() as MySiteCardAndItem.Item.SingleActionCard + assertThat(emittedBanners.last()).isNotNull - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) - banner.onActionClick() + flow.value = SiteReadiness.Ready advanceUntilIdle() - verify(editorSettingsRepository, times(2)).fetchEditorCapabilitiesForSite(siteTest) assertThat(emittedBanners.last()).isNull() } @Test - fun `when clearBanner invoked, then banner is null`() = test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + fun `given user-initiated, when fetchCapabilities invoked, then source is invalidated`() = test { + stubReadiness(siteTest, SiteReadiness.Ready) - slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() - assertThat(emittedBanners.last()).isNotNull - slice.clearBanner() + slice.fetchCapabilities(siteTest, isUserInitiated = true) advanceUntilIdle() - assertThat(emittedBanners.last()).isNull() + verify(siteProvisioningSource).invalidate(siteTest) } @Test - fun `given two different sites, when fetched in sequence, then both fetches run`() = test { - val otherSite = SiteModel().apply { id = TEST_SITE_LOCAL_ID + 1 } - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(otherSite)).thenReturn(true) + fun `given non-user-initiated, when fetchCapabilities invoked, then source is not invalidated`() = test { + stubReadiness(siteTest, SiteReadiness.Ready) slice.fetchCapabilities(siteTest, isUserInitiated = false) advanceUntilIdle() - slice.fetchCapabilities(otherSite, isUserInitiated = false) - advanceUntilIdle() - verify(editorSettingsRepository, times(1)).fetchEditorCapabilitiesForSite(eq(siteTest)) - verify(editorSettingsRepository, times(1)).fetchEditorCapabilitiesForSite(eq(otherSite)) + verify(siteProvisioningSource, never()).invalidate(any()) } @Test - fun `given fetch in flight, when clearBanner invoked, then banner stays null after fetch completes`() = test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + fun `given banner showing, when retry tapped, then source is invalidated`() = test { + stubReadiness(siteTest, SiteReadiness.Unreachable) slice.fetchCapabilities(siteTest, isUserInitiated = false) - slice.clearBanner() advanceUntilIdle() + val banner = emittedBanners.last() as MySiteCardAndItem.Item.SingleActionCard - assertThat(emittedBanners.last()).isNull() + banner.onActionClick() + advanceUntilIdle() + + verify(siteProvisioningSource).invalidate(siteTest) } @Test - fun `given retry in flight, when banner tapped again, then second tap is a no-op`() = test { - val gate = CompletableDeferred() - var callCount = 0 - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).doSuspendableAnswer { - callCount++ - if (callCount == 1) false else gate.await() - } - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + fun `when clearBanner invoked, then banner is null`() = test { + stubReadiness(siteTest, SiteReadiness.Unreachable) slice.fetchCapabilities(siteTest, isUserInitiated = false) advanceUntilIdle() - val banner = emittedBanners.last() as MySiteCardAndItem.Item.SingleActionCard - - banner.onActionClick() // first tap — retry suspends on gate - banner.onActionClick() // second tap — should be ignored - gate.complete(true) + assertThat(emittedBanners.last()).isNotNull + slice.clearBanner() advanceUntilIdle() - verify(editorSettingsRepository, times(2)).fetchEditorCapabilitiesForSite(siteTest) + assertThat(emittedBanners.last()).isNull() } @Test - fun `given banner cleared, when retry tapped, then no fetch runs`() = test { - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) - whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + fun `given banner cleared, when retry tapped, then no invalidate runs`() = test { + stubReadiness(siteTest, SiteReadiness.Unreachable) slice.fetchCapabilities(siteTest, isUserInitiated = false) advanceUntilIdle() @@ -271,31 +174,28 @@ class SiteConnectivityBannerViewModelSliceTest : BaseUnitTest() { slice.clearBanner() advanceUntilIdle() - // Simulate a tap that landed before LiveData propagated the null clear. banner.onActionClick() advanceUntilIdle() - verify(editorSettingsRepository, times(1)).fetchEditorCapabilitiesForSite(siteTest) + verify(siteProvisioningSource, never()).invalidate(any()) } @Test - fun `given fetch in flight for site A, when fetch starts for site B, then site A result is discarded`() = test { + fun `given site switched, when old site becomes unreachable, then banner ignores it`() = test { val siteB = SiteModel().apply { id = TEST_SITE_LOCAL_ID + 1 } - val gateA = CompletableDeferred() - // Site A's fetch suspends on a gate so we can interleave site B's call before A completes. - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).doSuspendableAnswer { - gateA.await() - } - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteB)).thenReturn(true) + val flowA = stubReadiness(siteTest, SiteReadiness.Ready) + stubReadiness(siteB, SiteReadiness.Ready) slice.fetchCapabilities(siteTest, isUserInitiated = false) - advanceUntilIdle() // siteA suspended in fetch + advanceUntilIdle() slice.fetchCapabilities(siteB, isUserInitiated = false) - advanceUntilIdle() // siteB completes; currentSite is now siteB - gateA.complete(false) // release siteA — its result must NOT post a banner advanceUntilIdle() - // No banner card should ever have been emitted for site A. - assertThat(emittedBanners.filterIsInstance()).isEmpty() + flowA.value = SiteReadiness.Unreachable + advanceUntilIdle() + + assertThat( + emittedBanners.filterIsInstance() + ).isEmpty() } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/GutenbergEditorPreloaderTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/GutenbergEditorPreloaderTest.kt index b009b9f3cc2d..c5989af09870 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/GutenbergEditorPreloaderTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/GutenbergEditorPreloaderTest.kt @@ -19,8 +19,8 @@ import org.wordpress.android.BaseUnitTest import org.wordpress.android.datasets.SiteSettingsProvider import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.AccountStore -import org.wordpress.android.repositories.EditorSettingsRepository -import org.wordpress.android.ui.accounts.login.SiteApiRestUrlRecoverer +import org.wordpress.android.fluxc.store.SiteStore +import org.wordpress.android.repositories.SiteProvisioningSource import org.wordpress.gutenberg.model.EditorAssetBundle import org.wordpress.gutenberg.model.EditorConfiguration import org.wordpress.gutenberg.model.EditorDependencies @@ -49,10 +49,10 @@ class GutenbergEditorPreloaderTest : lateinit var editorServiceProvider: EditorServiceProvider @Mock - lateinit var editorSettingsRepository: EditorSettingsRepository + lateinit var siteProvisioningSource: SiteProvisioningSource @Mock - lateinit var siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer + lateinit var siteStore: SiteStore private val editorDependencies = EditorDependencies.empty @@ -75,8 +75,8 @@ class GutenbergEditorPreloaderTest : gutenbergKitSettingsBuilder = gutenbergKitSettingsBuilder, siteSettingsProvider = siteSettingsProvider, editorServiceProvider = editorServiceProvider, - editorSettingsRepository = editorSettingsRepository, - siteApiRestUrlRecoverer = siteApiRestUrlRecoverer, + siteProvisioningSource = siteProvisioningSource, + siteStore = siteStore, bgDispatcher = testDispatcher() ) } @@ -194,7 +194,7 @@ class GutenbergEditorPreloaderTest : } @Test - fun `successful preload fetches editor capabilities`() = test { + fun `successful preload runs the provisioning source`() = test { val site = createSite() enablePreloading(site) stubSuccessfulPreload() @@ -203,8 +203,7 @@ class GutenbergEditorPreloaderTest : preloader.preloadIfNeeded(site, this) advanceUntilIdle() - verify(editorSettingsRepository) - .fetchEditorCapabilitiesForSite(site) + verify(siteProvisioningSource).await(site) } @Test @@ -482,22 +481,4 @@ class GutenbergEditorPreloaderTest : } // endregion - - // region wpApiRestUrl recovery - - @Test - fun `successful preload invokes discovery only — slice owns persistence`() = test { - val site = createSite() - enablePreloading(site) - stubSuccessfulPreload() - stubEditorService() - - preloader.preloadIfNeeded(site, this) - advanceUntilIdle() - - verify(siteApiRestUrlRecoverer).discoverApiRootUrl(site.url) - verify(siteApiRestUrlRecoverer, never()).persistApiRootUrl(any(), any()) - } - - // endregion }