Skip to content

Commit b40145e

Browse files
jkmasseljkmassel
committed
Unify editor capability detection behind a single per-site detector
Introduces EditorCapabilityDetector — one app-scoped owner of editor REST capability state, exposed as a per-site StateFlow — so the connectivity banner and editor preloader share a single, deduplicated probe instead of each re-deriving the state and racing the same async preconditions. Folds in the authenticated direct-host probe fallback so private Atomic sites detect correctly on trunk. Part of #22942.
1 parent d086608 commit b40145e

9 files changed

Lines changed: 492 additions & 253 deletions

File tree

‎WordPress/src/main/java/org/wordpress/android/AppInitializer.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ import org.wordpress.android.networking.ConnectionChangeReceiver
7171
import org.wordpress.android.networking.OAuthAuthenticator
7272
import org.wordpress.android.networking.RestClientUtils
7373
import org.wordpress.android.push.GCMRegistrationScheduler
74+
import org.wordpress.android.repositories.EditorCapabilityDetector
7475
import org.wordpress.android.support.ZendeskHelper
7576
import org.wordpress.android.ui.ActivityId
7677
import org.wordpress.android.ui.debug.cookies.DebugCookieManager
@@ -229,6 +230,9 @@ class AppInitializer @Inject constructor(
229230
@Inject
230231
lateinit var wpApiClientProvider: WpApiClientProvider
231232

233+
@Inject
234+
lateinit var editorCapabilityDetector: EditorCapabilityDetector
235+
232236
@Inject
233237
lateinit var openWebLinksWithJetpackHelper: DeepLinkOpenWebLinksWithJetpackHelper
234238

@@ -717,6 +721,9 @@ class AppInitializer @Inject constructor(
717721
// Clear cached wordpress-rs services and API clients
718722
wpServiceProvider.clearAll()
719723
wpApiClientProvider.clearAllClients()
724+
725+
// Drop per-site editor-capability detection state for the signed-out user
726+
editorCapabilityDetector.clear()
720727
}
721728

722729
/*

‎WordPress/src/main/java/org/wordpress/android/repositories/EditorCapabilityDetector.kt‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
package org.wordpress.android.repositories
2+
3+
import kotlinx.coroutines.CoroutineScope
4+
import kotlinx.coroutines.Job
5+
import kotlinx.coroutines.flow.MutableStateFlow
6+
import kotlinx.coroutines.flow.StateFlow
7+
import kotlinx.coroutines.launch
8+
import org.wordpress.android.fluxc.model.SiteModel
9+
import org.wordpress.android.modules.APPLICATION_SCOPE
10+
import org.wordpress.android.util.NetworkUtilsWrapper
11+
import java.util.concurrent.ConcurrentHashMap
12+
import javax.inject.Inject
13+
import javax.inject.Named
14+
import javax.inject.Singleton
15+
16+
/**
17+
* Single owner of "what does this site's editor REST API support" as an
18+
* observable, per-site state.
19+
*
20+
* Capability detection ([EditorSettingsRepository.fetchEditorCapabilitiesForSite])
21+
* has two async preconditions on Atomic sites — an application password and a
22+
* recovered REST root — provisioned elsewhere on the My Site screen. Routing
23+
* every consumer (connectivity banner, editor preloader) through this detector
24+
* means one probe per site, shared and deduplicated, instead of each consumer
25+
* re-deriving the same state and racing the same preconditions.
26+
*
27+
* State is keyed by [SiteModel.id] — the local DB row id, stable across the
28+
* process lifetime — mirroring `GutenbergEditorPreloader`.
29+
*
30+
* ## Entry points
31+
* - [stateFor] — the reactive entry point. Returns a shared [StateFlow]; the
32+
* first access starts detection, later accesses reuse the cached result
33+
* (capabilities rarely change). A failed probe is retried on the next access.
34+
* - [awaitProbe] — the one-shot entry point for callers that just need the
35+
* probe to have run (and its capabilities persisted) before continuing.
36+
* - [refresh] — forces a re-probe, bypassing the once-per-site gate
37+
* (pull-to-refresh, banner retry, newly established credentials).
38+
* - [clear] — cancels all work and drops all state; wire into sign-out.
39+
*/
40+
@Singleton
41+
class EditorCapabilityDetector @Inject constructor(
42+
private val editorSettingsRepository: EditorSettingsRepository,
43+
private val networkUtilsWrapper: NetworkUtilsWrapper,
44+
@Named(APPLICATION_SCOPE) private val appScope: CoroutineScope,
45+
) {
46+
private val states =
47+
ConcurrentHashMap<Int, MutableStateFlow<EditorCapabilityDetectionState>>()
48+
private val jobs = ConcurrentHashMap<Int, Job>()
49+
50+
// Sites whose live probe succeeded this process — the dedup gate. Only a
51+
// successful fetch latches; a failed one is left to retry on the next
52+
// access, matching the connectivity banner's previous per-slice behaviour.
53+
// Reset by refresh / clear.
54+
private val probedOk = ConcurrentHashMap.newKeySet<Int>()
55+
56+
/**
57+
* The shared detection state for [site]. The first call starts detection;
58+
* later calls return the same flow without re-probing once it has
59+
* succeeded. Collect it to react to capability changes.
60+
*/
61+
@Synchronized
62+
fun stateFor(site: SiteModel): StateFlow<EditorCapabilityDetectionState> {
63+
val flow = flowFor(site.id)
64+
if (shouldProbe(site.id)) launchDetection(site)
65+
return flow
66+
}
67+
68+
/**
69+
* Ensures detection has run for [site] (so its capabilities are persisted)
70+
* and returns the settled state. Respects the once-per-site gate; call
71+
* [refresh] first to force a fresh probe.
72+
*/
73+
suspend fun awaitProbe(site: SiteModel): EditorCapabilityDetectionState {
74+
stateFor(site)
75+
jobs[site.id]?.join()
76+
return states[site.id]?.value ?: EditorCapabilityDetectionState.Pending
77+
}
78+
79+
/**
80+
* Forces a re-probe for [site], bypassing the once-per-site gate. A no-op
81+
* while a probe is already in flight — that probe's result is fresh enough.
82+
*/
83+
@Synchronized
84+
fun refresh(site: SiteModel) {
85+
if (jobs[site.id]?.isActive == true) return
86+
probedOk.remove(site.id)
87+
launchDetection(site)
88+
}
89+
90+
/** Cancels all in-flight detection and drops all cached state (sign-out). */
91+
@Synchronized
92+
fun clear() {
93+
jobs.values.forEach { it.cancel() }
94+
jobs.clear()
95+
states.clear()
96+
probedOk.clear()
97+
}
98+
99+
@Synchronized
100+
private fun launchDetection(site: SiteModel) {
101+
jobs[site.id]?.cancel()
102+
val flow = flowFor(site.id)
103+
jobs[site.id] = appScope.launch {
104+
flow.value = detect(site)
105+
}
106+
}
107+
108+
private fun flowFor(siteLocalId: Int): MutableStateFlow<EditorCapabilityDetectionState> =
109+
states.getOrPut(siteLocalId) {
110+
MutableStateFlow(EditorCapabilityDetectionState.Pending)
111+
}
112+
113+
private fun shouldProbe(siteLocalId: Int): Boolean =
114+
jobs[siteLocalId]?.isActive != true && siteLocalId !in probedOk
115+
116+
private suspend fun detect(site: SiteModel): EditorCapabilityDetectionState {
117+
val ok = editorSettingsRepository.fetchEditorCapabilitiesForSite(site)
118+
if (ok) probedOk.add(site.id)
119+
val hasCache = editorSettingsRepository.hasCachedCapabilities(site)
120+
return when {
121+
ok || hasCache -> EditorCapabilityDetectionState.Ready
122+
editorSettingsRepository.isAwaitingApplicationPassword(site) ->
123+
EditorCapabilityDetectionState.Pending
124+
!networkUtilsWrapper.isNetworkAvailable() ->
125+
EditorCapabilityDetectionState.TransientError
126+
else -> EditorCapabilityDetectionState.Unreachable
127+
}
128+
}
129+
}
130+
131+
/**
132+
* Observable lifecycle of editor-capability detection for one site — distinct
133+
* from `org.wordpress.android.ui.posts.EditorCapabilityState`, which models a
134+
* resolved settings-row capability. This is the *detection* state the
135+
* connectivity banner and editor preloader subscribe to.
136+
*/
137+
sealed interface EditorCapabilityDetectionState {
138+
/**
139+
* Not determined yet — still probing, or waiting on an application password
140+
* being minted asynchronously. Consumers hold; the banner stays hidden.
141+
*/
142+
data object Pending : EditorCapabilityDetectionState
143+
144+
/**
145+
* Capabilities are known (freshly detected, or cached from a prior run).
146+
* Read them via [EditorSettingsRepository]'s getters.
147+
*/
148+
data object Ready : EditorCapabilityDetectionState
149+
150+
/**
151+
* Credentials are present but the transport probe failed — the site looks
152+
* unreachable. The only state that surfaces the connectivity banner.
153+
*/
154+
data object Unreachable : EditorCapabilityDetectionState
155+
156+
/** A transient failure (e.g. device offline). Retried on the next probe. */
157+
data object TransientError : EditorCapabilityDetectionState
158+
}

‎WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSlice.kt‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ import org.wordpress.android.fluxc.network.xmlrpc.site.SiteXMLRPCClient
1717
import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider
1818
import org.wordpress.android.fluxc.store.SiteStore
1919
import org.wordpress.android.fluxc.utils.AppLogWrapper
20+
import org.wordpress.android.repositories.EditorCapabilityDetector
2021
import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper
21-
import org.wordpress.android.ui.accounts.login.CredentialsChangedNotifier
2222
import org.wordpress.android.ui.accounts.login.SiteApiRestUrlRecoverer
2323
import org.wordpress.android.ui.mysite.MySiteCardAndItem
2424
import org.wordpress.android.ui.mysite.MySiteCardAndItem.Card.QuickLinksItem.QuickLinkItem
@@ -42,7 +42,7 @@ class ApplicationPasswordViewModelSlice @Inject constructor(
4242
private val siteXMLRPCClient: SiteXMLRPCClient,
4343
private val siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer,
4444
private val dispatcher: Dispatcher,
45-
private val credentialsChangedNotifier: CredentialsChangedNotifier,
45+
private val editorCapabilityDetector: EditorCapabilityDetector,
4646
@Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher,
4747
) {
4848
lateinit var scope: CoroutineScope
@@ -114,7 +114,11 @@ class ApplicationPasswordViewModelSlice @Inject constructor(
114114
if (!createResult.isError && createResult.credentials != null) {
115115
wpApiClientProvider.clearSelfHostedClient(storedSite.id)
116116
appLogWrapper.d(AppLog.T.MAIN, "A_P: Headless mint succeeded for ${storedSite.url}")
117-
credentialsChangedNotifier.notifyChanged(storedSite.id)
117+
// The first-login capability probe can lose the race to this async mint. storedSite
118+
// was just mutated in place with the new credentials (SiteStore
119+
// .persistApplicationPasswordCredentials), so re-probe against this exact instance —
120+
// no stale-SiteModel re-read, and capabilities settle without a manual pull-to-refresh.
121+
editorCapabilityDetector.refresh(storedSite)
118122
// The mint goes through the Jetpack tunnel and never runs discovery — without this
119123
// step, freshly minted Atomic sites end up with working creds but a NULL
120124
// wpApiRestUrl in the local DB. Run in the background so the card hides immediately.

‎WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSlice.kt‎

Lines changed: 24 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7,83 +7,51 @@ import kotlinx.coroutines.Job
77
import kotlinx.coroutines.launch
88
import org.wordpress.android.R
99
import org.wordpress.android.fluxc.model.SiteModel
10-
import org.wordpress.android.repositories.EditorSettingsRepository
11-
import org.wordpress.android.ui.accounts.login.CredentialsChangedNotifier
10+
import org.wordpress.android.repositories.EditorCapabilityDetectionState
11+
import org.wordpress.android.repositories.EditorCapabilityDetector
1212
import org.wordpress.android.ui.mysite.MySiteCardAndItem
13-
import org.wordpress.android.ui.mysite.SelectedSiteRepository
14-
import org.wordpress.android.util.NetworkUtilsWrapper
1513
import javax.inject.Inject
1614

1715
class SiteConnectivityBannerViewModelSlice @Inject constructor(
18-
private val editorSettingsRepository: EditorSettingsRepository,
19-
private val networkUtilsWrapper: NetworkUtilsWrapper,
20-
private val credentialsChangedNotifier: CredentialsChangedNotifier,
21-
private val selectedSiteRepository: SelectedSiteRepository,
16+
private val editorCapabilityDetector: EditorCapabilityDetector,
2217
) {
2318
private lateinit var scope: CoroutineScope
24-
private var currentJob: Job? = null
19+
private var collectJob: Job? = null
2520
private var currentSite: SiteModel? = null
2621

2722
private val _uiModel = MutableLiveData<MySiteCardAndItem?>()
2823
val uiModel: LiveData<MySiteCardAndItem?> = _uiModel
2924

30-
/* Site capabilities rarely change, so once we've successfully fetched them for a site we
31-
skip subsequent non-user-initiated fetches in this slice's lifetime. Failed fetches do
32-
not populate this set, so a transient network failure recovers on the next onResume.
33-
User-initiated calls (PTR, banner retry) always bypass this gate. */
34-
private val fetchedCapabilitiesForSite = mutableSetOf<Int>()
35-
3625
fun initialize(scope: CoroutineScope) {
3726
this.scope = scope
38-
// Re-run detection the moment an application password is established for the selected site
39-
// (e.g. the headless mint finished after our first fetch lost the race), instead of waiting
40-
// for the next resume/refresh. Re-read the selected site so we see the just-persisted
41-
// credentials; isUserInitiated = false so a replayed event is a no-op once cached.
42-
scope.launch {
43-
credentialsChangedNotifier.events.collect { siteLocalId ->
44-
val site = selectedSiteRepository.getSelectedSite()
45-
if (site != null && site.id == siteLocalId) {
46-
fetchCapabilities(site, isUserInitiated = false)
47-
}
48-
}
49-
}
5027
}
5128

29+
/**
30+
* Subscribes the banner to [site]'s editor-capability detection state. The
31+
* banner is a thin view over that state — it surfaces only when detection
32+
* reports the site [Unreachable][EditorCapabilityDetectionState.Unreachable].
33+
* Every other state (probing, pending credentials, offline, ready) leaves it
34+
* hidden, so the dedup, offline-suppression, and pending-credential handling
35+
* that used to live here now belong to the one detector. [isUserInitiated]
36+
* (pull-to-refresh, banner retry) forces a fresh probe.
37+
*/
5238
fun fetchCapabilities(site: SiteModel, isUserInitiated: Boolean) {
53-
currentJob?.cancel()
39+
collectJob?.cancel()
5440
currentSite = site
55-
currentJob = scope.launch {
56-
if (site.id in fetchedCapabilitiesForSite && !isUserInitiated) {
57-
return@launch
58-
}
59-
val ok = editorSettingsRepository.fetchEditorCapabilitiesForSite(site)
60-
if (ok) {
61-
fetchedCapabilitiesForSite.add(site.id)
41+
if (isUserInitiated) editorCapabilityDetector.refresh(site)
42+
collectJob = scope.launch {
43+
editorCapabilityDetector.stateFor(site).collect { state ->
44+
// Bail if the user switched sites while suspended — postValue is
45+
// not a suspension point, so cancellation alone won't catch this.
46+
if (currentSite?.id != site.id) return@collect
47+
val showBanner = state is EditorCapabilityDetectionState.Unreachable
48+
_uiModel.postValue(if (showBanner) buildBanner() else null)
6249
}
63-
val hasCache = editorSettingsRepository.hasCachedCapabilities(site)
64-
// Bail if the user switched sites while we were suspended — postValue
65-
// isn't a suspension point, so cancellation alone won't catch this.
66-
if (currentSite?.id != site.id) return@launch
67-
// Suppress the banner when the device is offline — the global "no
68-
// connection" banner already covers this case, and stacking warnings
69-
// for the same root cause is just noise.
70-
val suppressForOffline = !ok && !networkUtilsWrapper.isNetworkAvailable()
71-
// Atomic sites probe the direct host with an application password that's minted
72-
// asynchronously on this same screen, so a first-login fetch can fail purely because
73-
// the credential isn't ready yet. Treat that as pending, not a connection failure —
74-
// the application-password card owns that state and a later fetch will succeed.
75-
val suppressForPendingAuth =
76-
!ok && editorSettingsRepository.isAwaitingApplicationPassword(site)
77-
// Show the banner only as a last resort — not when detection succeeded, when we have
78-
// cached capabilities, or while a transient non-error state (offline / pending creds)
79-
// already explains the failure.
80-
val suppressBanner = ok || hasCache || suppressForOffline || suppressForPendingAuth
81-
_uiModel.postValue(if (suppressBanner) null else buildBanner())
8250
}
8351
}
8452

8553
fun clearBanner() {
86-
currentJob?.cancel()
54+
collectJob?.cancel()
8755
currentSite = null
8856
_uiModel.postValue(null)
8957
}
@@ -93,10 +61,7 @@ class SiteConnectivityBannerViewModelSlice @Inject constructor(
9361
textResource = R.string.site_connectivity_banner_text,
9462
imageResource = R.drawable.ic_cloud_off_themed_24dp,
9563
onActionClick = {
96-
val site = currentSite
97-
if (site != null && currentJob?.isActive != true) {
98-
fetchCapabilities(site, isUserInitiated = true)
99-
}
64+
currentSite?.let { fetchCapabilities(it, isUserInitiated = true) }
10065
},
10166
showLearnMore = false,
10267
centerImageVertically = true,

‎WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergEditorPreloader.kt‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import org.wordpress.android.datasets.SiteSettingsProvider
1111
import org.wordpress.android.fluxc.model.SiteModel
1212
import org.wordpress.android.fluxc.store.AccountStore
1313
import org.wordpress.android.modules.BG_THREAD
14-
import org.wordpress.android.repositories.EditorSettingsRepository
14+
import org.wordpress.android.repositories.EditorCapabilityDetector
1515
import org.wordpress.android.ui.accounts.login.SiteApiRestUrlRecoverer
1616
import org.wordpress.android.util.AppLog
1717
import org.wordpress.gutenberg.model.EditorDependencies
@@ -63,7 +63,7 @@ class GutenbergEditorPreloader @Inject constructor(
6363
private val gutenbergKitSettingsBuilder: GutenbergKitSettingsBuilder,
6464
private val siteSettingsProvider: SiteSettingsProvider,
6565
private val editorServiceProvider: EditorServiceProvider,
66-
private val editorSettingsRepository: EditorSettingsRepository,
66+
private val editorCapabilityDetector: EditorCapabilityDetector,
6767
private val siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer,
6868
@Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher
6969
) {
@@ -99,8 +99,11 @@ class GutenbergEditorPreloader @Inject constructor(
9999
siteApiRestUrlRecoverer.discoverApiRootUrl(site.url)
100100
?.let { site.wpApiRestUrl = it }
101101
}
102-
editorSettingsRepository
103-
.fetchEditorCapabilitiesForSite(site)
102+
// Detect (and persist) editor capabilities via the shared
103+
// detector so the preloader and connectivity banner can't
104+
// double-probe. We only need the probe to have run before
105+
// building config, so the settled state itself is ignored.
106+
editorCapabilityDetector.awaitProbe(site)
104107
// Preloading produces EditorDependencies, which the editor
105108
// consumes alongside its own per-launch EditorConfiguration.
106109
// Cookies and network-logging are per-launch concerns the
@@ -146,6 +149,9 @@ class GutenbergEditorPreloader @Inject constructor(
146149
@MainThread
147150
fun refreshPreloading(site: SiteModel, scope: CoroutineScope) {
148151
clearSite(site)
152+
// Pull-to-refresh: force a fresh capability probe so the awaitProbe in
153+
// preloadIfNeeded re-detects instead of returning the cached result.
154+
editorCapabilityDetector.refresh(site)
149155
preloadIfNeeded(site, scope)
150156
}
151157

0 commit comments

Comments
 (0)