From 6af28f5acffd49aef17b17a5570991f8f5aaec6f Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 27 May 2026 10:37:14 -0600 Subject: [PATCH 1/5] Populate missing `wpApiRestUrl` via API discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #22899. Atomic sites can land in the DB with `wpApiRestUrl=NULL` because the WP.com `/me/sites` endpoint omits the field, and the headless application- password mint goes through the Jetpack tunnel without running discovery, then persists via `UPDATE_APPLICATION_PASSWORD` whose reducer copies the in-memory NULL into the DB row alongside the freshly minted credentials. The editor's `${site.url}/wp-json/` fallback is wrong for sites with rewritten REST namespaces or non-default roots — those 404 on `/wp-block-editor/v1/settings`. Add `SiteApiRestUrlRecoverer` which runs `WpLoginClient.apiDiscovery` and writes the result back via `SiteSqlUtils.insertOrUpdateSite` (re-reading the row first so it preserves anything other code paths wrote since the in-memory model was loaded). Two entry points keep persistence scoped to one code path: - `recoverAndPersistIfMissing` — used by `ApplicationPasswordViewModelSlice` on both auth-success paths (validator returned Valid, or headless mint succeeded), launched in the background so the card hides immediately on slow networks. - `discoverInMemoryIfMissing` — used by `GutenbergEditorPreloader` so a same-launch race ahead of the slice still gets the correct URL. `runDiscovery` rethrows `CancellationException` to preserve structured concurrency and falls back to the existing `${site.url}/wp-json/` URL on any other failure. --- RELEASE-NOTES.txt | 1 + .../accounts/login/SiteApiRestUrlRecoverer.kt | 105 +++++++++ .../ApplicationPasswordViewModelSlice.kt | 8 + .../ui/posts/GutenbergEditorPreloader.kt | 3 + .../login/SiteApiRestUrlRecovererTest.kt | 211 ++++++++++++++++++ .../ApplicationPasswordViewModelSliceTest.kt | 50 +++++ .../ui/posts/GutenbergEditorPreloaderTest.kt | 24 ++ 7 files changed, 402 insertions(+) create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index a50e9d597132..d1a0e3f8e6c3 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -5,6 +5,7 @@ * [**] Resolved an issue where the editor could become impossible to exit when it failed to load. * [*] Atomic sites can now create application passwords without leaving the app. * [**] Fixed a case where the editor failed to load on WP.com Atomic sites whose host doesn't expose `wp-block-editor/v1/settings`. +* [*] Editor now discovers the correct REST API root for sites with non-default API URLs. 26.7 ----- diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt new file mode 100644 index 000000000000..b7c488d976e8 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt @@ -0,0 +1,105 @@ +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.persistence.SiteSqlUtils +import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.modules.BG_THREAD +import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.DiscoverSuccessWrapper +import org.wordpress.android.util.AppLog +import rs.wordpress.api.kotlin.ApiDiscoveryResult +import rs.wordpress.api.kotlin.WpLoginClient +import javax.inject.Inject +import javax.inject.Named +import javax.inject.Singleton +import kotlin.coroutines.cancellation.CancellationException + +/** + * Discovers and populates [SiteModel.wpApiRestUrl] when it's missing, healing sites that + * landed in the DB without one (WP.com `/me/sites` omits the field; headless application- + * password mint goes through the Jetpack tunnel without running discovery). + * + * - [recoverAndPersistIfMissing] writes to the DB. Used from the auth flow. + * - [discoverInMemoryIfMissing] sets the in-memory model only — for short-lived consumers + * (editor preloader) that just need the URL for one call. + */ +@Singleton +class SiteApiRestUrlRecoverer @Inject constructor( + private val wpLoginClient: WpLoginClient, + private val discoverSuccessWrapper: DiscoverSuccessWrapper, + private val siteSqlUtils: SiteSqlUtils, + private val appLogWrapper: AppLogWrapper, + @param:Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, +) { + suspend fun recoverAndPersistIfMissing(site: SiteModel) { + if (!site.wpApiRestUrl.isNullOrEmpty()) return + withContext(bgDispatcher) { + val apiRootUrl = runDiscovery(site) ?: return@withContext + site.wpApiRestUrl = apiRootUrl + persist(site.id, apiRootUrl) + } + } + + suspend fun discoverInMemoryIfMissing(site: SiteModel) { + if (!site.wpApiRestUrl.isNullOrEmpty()) return + withContext(bgDispatcher) { + val apiRootUrl = runDiscovery(site) ?: return@withContext + site.wpApiRestUrl = apiRootUrl + appLogWrapper.d( + AppLog.T.API, + "Discovered wpApiRestUrl=$apiRootUrl for ${site.url} (in-memory only)" + ) + } + } + + // Re-reads the DB row by local ID and writes only [SiteModel.wpApiRestUrl]. This preserves + // anything that other code paths (e.g. an UPDATE_APPLICATION_PASSWORD that ran between site + // load and here) wrote since the in-memory model was last loaded. Writing the in-memory site + // directly is *unsafe* — it would clobber concurrent updates to other fields. + @Suppress("SwallowedException") + private fun persist(localId: Int, apiRootUrl: String) { + val siteFromDB = siteSqlUtils.getSitesWithLocalId(localId).firstOrNull() ?: run { + appLogWrapper.w(AppLog.T.API, "Cannot persist wpApiRestUrl: no site with localId=$localId") + return + } + siteFromDB.wpApiRestUrl = apiRootUrl + try { + siteSqlUtils.insertOrUpdateSite(siteFromDB) + appLogWrapper.d( + AppLog.T.API, + "Recovered wpApiRestUrl=$apiRootUrl for ${siteFromDB.url} (persisted)" + ) + } catch (e: SiteSqlUtils.DuplicateSiteException) { + appLogWrapper.e( + AppLog.T.API, + "DuplicateSiteException persisting wpApiRestUrl=$apiRootUrl for ${siteFromDB.url}" + ) + } + } + + @Suppress("TooGenericExceptionCaught") + private suspend fun runDiscovery(site: SiteModel): String? = try { + when (val result = wpLoginClient.apiDiscovery(site.url)) { + is ApiDiscoveryResult.Success -> { + val apiRootUrl = discoverSuccessWrapper.getApiRootUrl(result) + if (apiRootUrl.isBlank()) null else apiRootUrl + } + else -> { + appLogWrapper.w( + AppLog.T.API, + "API discovery failed for ${site.url}" + ) + null + } + } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + appLogWrapper.e( + AppLog.T.API, + "API discovery threw for ${site.url}: ${e::class.simpleName}: ${e.message}" + ) + null + } +} 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 942be2898366..37d1edb86955 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 @@ -18,6 +18,7 @@ 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.ui.accounts.login.ApplicationPasswordLoginHelper +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 @@ -38,6 +39,7 @@ class ApplicationPasswordViewModelSlice @Inject constructor( private val applicationPasswordValidator: ApplicationPasswordValidator, private val selfHostedEndpointFinder: SelfHostedEndpointFinder, private val siteXMLRPCClient: SiteXMLRPCClient, + private val siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer, private val dispatcher: Dispatcher, @Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher, ) { @@ -81,6 +83,8 @@ class ApplicationPasswordViewModelSlice @Inject constructor( if (hadCreds) { when (applicationPasswordValidator.validate(storedSite)) { ApplicationPasswordValidator.Outcome.Valid -> { + // Heal in the background so the card hides immediately on a slow network. + scope.launch { siteApiRestUrlRecoverer.recoverAndPersistIfMissing(storedSite) } handleValidAuth(storedSite) return@launch } @@ -108,6 +112,10 @@ class ApplicationPasswordViewModelSlice @Inject constructor( if (!createResult.isError && createResult.credentials != null) { wpApiClientProvider.clearSelfHostedClient(storedSite.id) appLogWrapper.d(AppLog.T.MAIN, "A_P: Headless mint succeeded for ${storedSite.url}") + // 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 { siteApiRestUrlRecoverer.recoverAndPersistIfMissing(storedSite) } handleValidAuth(storedSite) return@launch } 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 d420c6103b12..9a5ac1422bbe 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 @@ -12,6 +12,7 @@ import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.AccountStore 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.util.AppLog import org.wordpress.gutenberg.model.EditorDependencies import java.util.concurrent.ConcurrentHashMap @@ -63,6 +64,7 @@ class GutenbergEditorPreloader @Inject constructor( private val siteSettingsProvider: SiteSettingsProvider, private val editorServiceProvider: EditorServiceProvider, private val editorSettingsRepository: EditorSettingsRepository, + private val siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer, @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher ) { private sealed class PreloadState { @@ -93,6 +95,7 @@ class GutenbergEditorPreloader @Inject constructor( val siteId = site.id val job = scope.launch(bgDispatcher) { try { + siteApiRestUrlRecoverer.discoverInMemoryIfMissing(site) editorSettingsRepository .fetchEditorCapabilitiesForSite(site) // Preloading produces EditorDependencies, which the editor diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt new file mode 100644 index 000000000000..491e35345892 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt @@ -0,0 +1,211 @@ +package org.wordpress.android.ui.accounts.login + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.Mockito.mock +import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any +import org.mockito.kotlin.doThrow +import org.mockito.kotlin.eq +import org.mockito.kotlin.never +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.persistence.SiteSqlUtils +import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.DiscoverSuccessWrapper +import rs.wordpress.api.kotlin.ApiDiscoveryResult +import rs.wordpress.api.kotlin.WpLoginClient +import uniffi.wp_api.AutoDiscoveryAttemptSuccess +import uniffi.wp_api.DiscoveredAuthenticationMechanism +import uniffi.wp_api.ParseUrlException +import kotlin.coroutines.cancellation.CancellationException +import kotlin.test.assertFailsWith + +private const val SITE_URL = "https://example.test" +private const val DISCOVERED_API_ROOT = "https://example.test/custom-api/" +private const val LOCAL_ID = 1 + +@ExperimentalCoroutinesApi +class SiteApiRestUrlRecovererTest : BaseUnitTest() { + @Mock lateinit var wpLoginClient: WpLoginClient + @Mock lateinit var discoverSuccessWrapper: DiscoverSuccessWrapper + @Mock lateinit var siteSqlUtils: SiteSqlUtils + @Mock lateinit var appLogWrapper: AppLogWrapper + + private lateinit var recoverer: SiteApiRestUrlRecoverer + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + recoverer = SiteApiRestUrlRecoverer( + wpLoginClient = wpLoginClient, + discoverSuccessWrapper = discoverSuccessWrapper, + siteSqlUtils = siteSqlUtils, + appLogWrapper = appLogWrapper, + bgDispatcher = testDispatcher(), + ) + } + + private fun siteWithoutUrl(): SiteModel = SiteModel().apply { + id = LOCAL_ID + url = SITE_URL + wpApiRestUrl = null + } + + private suspend fun stubDiscoverySuccess(apiRootUrl: String) { + val result = ApiDiscoveryResult.Success( + AutoDiscoveryAttemptSuccess( + mock(), mock(), mock(), + DiscoveredAuthenticationMechanism.ApplicationPasswords(mock()) + ) + ) + whenever(wpLoginClient.apiDiscovery(any())).thenReturn(result) + whenever(discoverSuccessWrapper.getApiRootUrl(eq(result))) + .thenReturn(apiRootUrl) + } + + @Test + fun `recoverAndPersist populates wpApiRestUrl in memory and writes the DB row`() = runTest { + val site = siteWithoutUrl() + val siteFromDB = siteWithoutUrl() + stubDiscoverySuccess(DISCOVERED_API_ROOT) + whenever(siteSqlUtils.getSitesWithLocalId(LOCAL_ID)).thenReturn(listOf(siteFromDB)) + + recoverer.recoverAndPersistIfMissing(site) + + assertThat(site.wpApiRestUrl).isEqualTo(DISCOVERED_API_ROOT) + assertThat(siteFromDB.wpApiRestUrl).isEqualTo(DISCOVERED_API_ROOT) + verify(siteSqlUtils).insertOrUpdateSite(siteFromDB) + } + + @Test + fun `recoverAndPersist skips discovery when wpApiRestUrl is already populated`() = runTest { + val site = siteWithoutUrl().apply { wpApiRestUrl = "https://example.test/wp-json/" } + + recoverer.recoverAndPersistIfMissing(site) + + verify(wpLoginClient, never()).apiDiscovery(any()) + verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + } + + @Test + fun `recoverAndPersist does nothing when discovery returns a blank apiRootUrl`() = runTest { + val site = siteWithoutUrl() + stubDiscoverySuccess(apiRootUrl = "") + + recoverer.recoverAndPersistIfMissing(site) + + assertThat(site.wpApiRestUrl).isNull() + verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + } + + @Test + fun `recoverAndPersist does nothing when discovery returns a failure`() = runTest { + val site = siteWithoutUrl() + whenever(wpLoginClient.apiDiscovery(any())).thenReturn( + ApiDiscoveryResult.FailureParseSiteUrl(ParseUrlException.Generic("")) + ) + + recoverer.recoverAndPersistIfMissing(site) + + assertThat(site.wpApiRestUrl).isNull() + verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + } + + @Test + fun `recoverAndPersist swallows non-cancellation exceptions thrown by discovery`() = runTest { + val site = siteWithoutUrl() + whenever(wpLoginClient.apiDiscovery(any())) + .doThrow(RuntimeException("network error")) + + recoverer.recoverAndPersistIfMissing(site) + + assertThat(site.wpApiRestUrl).isNull() + verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + } + + @Test + fun `recoverAndPersist rethrows CancellationException to preserve structured concurrency`() = runTest { + val site = siteWithoutUrl() + whenever(wpLoginClient.apiDiscovery(any())) + .doThrow(CancellationException("cancelled")) + + assertFailsWith { + recoverer.recoverAndPersistIfMissing(site) + } + verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + } + + @Test + fun `recoverAndPersist skips the DB write when the site is not in the DB`() = runTest { + val site = siteWithoutUrl() + stubDiscoverySuccess(DISCOVERED_API_ROOT) + whenever(siteSqlUtils.getSitesWithLocalId(LOCAL_ID)).thenReturn(emptyList()) + + recoverer.recoverAndPersistIfMissing(site) + + assertThat(site.wpApiRestUrl).isEqualTo(DISCOVERED_API_ROOT) + verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + } + + @Test + fun `discoverInMemory populates wpApiRestUrl when missing but never touches the DB`() = runTest { + val site = siteWithoutUrl() + stubDiscoverySuccess(DISCOVERED_API_ROOT) + + recoverer.discoverInMemoryIfMissing(site) + + assertThat(site.wpApiRestUrl).isEqualTo(DISCOVERED_API_ROOT) + verify(siteSqlUtils, never()).getSitesWithLocalId(any()) + verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + } + + @Test + fun `discoverInMemory skips discovery when wpApiRestUrl is already populated`() = runTest { + val site = siteWithoutUrl().apply { wpApiRestUrl = "https://example.test/wp-json/" } + + recoverer.discoverInMemoryIfMissing(site) + + verify(wpLoginClient, never()).apiDiscovery(any()) + } + + @Test + fun `discoverInMemory does nothing when discovery fails`() = runTest { + val site = siteWithoutUrl() + whenever(wpLoginClient.apiDiscovery(any())).thenReturn( + ApiDiscoveryResult.FailureParseSiteUrl(ParseUrlException.Generic("")) + ) + + recoverer.discoverInMemoryIfMissing(site) + + assertThat(site.wpApiRestUrl).isNull() + } + + @Test + fun `discoverInMemory swallows non-cancellation exceptions thrown by discovery`() = runTest { + val site = siteWithoutUrl() + whenever(wpLoginClient.apiDiscovery(any())) + .doThrow(RuntimeException("network error")) + + recoverer.discoverInMemoryIfMissing(site) + + assertThat(site.wpApiRestUrl).isNull() + } + + @Test + fun `discoverInMemory rethrows CancellationException to preserve structured concurrency`() = runTest { + val site = siteWithoutUrl() + whenever(wpLoginClient.apiDiscovery(any())) + .doThrow(CancellationException("cancelled")) + + assertFailsWith { + recoverer.discoverInMemoryIfMissing(site) + } + } +} 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 d7ab7ed71d04..67ec59c0662f 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 @@ -32,6 +32,7 @@ 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.mysite.MySiteCardAndItem import kotlin.test.assertNotNull @@ -66,6 +67,9 @@ class ApplicationPasswordViewModelSliceTest : BaseUnitTest() { @Mock lateinit var siteXMLRPCClient: SiteXMLRPCClient + @Mock + lateinit var siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer + @Mock lateinit var dispatcher: Dispatcher @@ -87,6 +91,7 @@ class ApplicationPasswordViewModelSliceTest : BaseUnitTest() { applicationPasswordValidator, selfHostedEndpointFinder, siteXMLRPCClient, + siteApiRestUrlRecoverer, dispatcher, testDispatcher() ).apply { @@ -171,6 +176,51 @@ class ApplicationPasswordViewModelSliceTest : BaseUnitTest() { verify(applicationPasswordLoginHelper, never()).getAuthorizationUrlComplete(any()) } + @Test + fun `given headless mint succeeds, card hides without waiting for the recoverer`() = runTest { + stubMintSuccess() + val recoverGate = CompletableDeferred() + whenever(siteApiRestUrlRecoverer.recoverAndPersistIfMissing(any())) + .doSuspendableAnswer { recoverGate.await() } + + applicationPasswordViewModelSlice.buildCard(siteTest) + + // Card has been hidden even though the recoverer is still suspended on the gate. + assertNull(applicationPasswordCard) + verify(siteApiRestUrlRecoverer).recoverAndPersistIfMissing(siteTest) + + // Release the recoverer so the test scope doesn't carry a dangling coroutine. + recoverGate.complete(Unit) + } + + @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.recoverAndPersistIfMissing(any())) + .doSuspendableAnswer { recoverGate.await() } + + applicationPasswordViewModelSlice.buildCard(siteTest) + + assertNull(applicationPasswordCard) + verify(siteApiRestUrlRecoverer).recoverAndPersistIfMissing(any()) + + recoverGate.complete(Unit) + } + @Test fun `given headless mint returns NotSupported, then fall back to discovery`() = runTest { stubMintFailure(notSupported = true) 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 bafa4cfc7346..15e6165b0236 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 @@ -20,6 +20,7 @@ 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.gutenberg.model.EditorAssetBundle import org.wordpress.gutenberg.model.EditorConfiguration import org.wordpress.gutenberg.model.EditorDependencies @@ -50,6 +51,9 @@ class GutenbergEditorPreloaderTest : @Mock lateinit var editorSettingsRepository: EditorSettingsRepository + @Mock + lateinit var siteApiRestUrlRecoverer: SiteApiRestUrlRecoverer + private val editorDependencies = EditorDependencies.empty private lateinit var preloader: GutenbergEditorPreloader @@ -58,6 +62,7 @@ class GutenbergEditorPreloaderTest : val site = SiteModel() site.id = id site.name = "Site $id" + site.url = "https://example.test" return site } @@ -71,6 +76,7 @@ class GutenbergEditorPreloaderTest : siteSettingsProvider = siteSettingsProvider, editorServiceProvider = editorServiceProvider, editorSettingsRepository = editorSettingsRepository, + siteApiRestUrlRecoverer = siteApiRestUrlRecoverer, bgDispatcher = testDispatcher() ) } @@ -479,4 +485,22 @@ class GutenbergEditorPreloaderTest : } // endregion + + // region wpApiRestUrl recovery + + @Test + fun `successful preload invokes in-memory discovery only — slice owns persistence`() = test { + val site = createSite() + enablePreloading(site) + stubSuccessfulPreload() + stubEditorService() + + preloader.preloadIfNeeded(site, this) + advanceUntilIdle() + + verify(siteApiRestUrlRecoverer).discoverInMemoryIfMissing(site) + verify(siteApiRestUrlRecoverer, never()).recoverAndPersistIfMissing(any()) + } + + // endregion } From 2d8a1875bb7438663b3f21bb7ff799fea58f7ffa Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 27 May 2026 13:27:37 -0600 Subject: [PATCH 2/5] Address review: pure recoverer API + targeted UPDATE for `wpApiRestUrl` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `SiteSqlUtils.updateWpApiRestUrl(localId, url)`: targeted single-column UPDATE, no read-modify-write race window with concurrent site writes. - `SiteApiRestUrlRecoverer` shape changes: - `discoverApiRootUrl(siteUrl: String): String?` — pure, returns the URL instead of mutating a `SiteModel` argument. - `persistApiRootUrl(localId: Int, apiRootUrl: String): Boolean` — wraps the new SqlUtils method. - Dropped `recoverAndPersistIfMissing` and `discoverInMemoryIfMissing`; callers handle the if-missing check and in-memory assignment. - `ApplicationPasswordViewModelSlice` gains a private `healApiRestUrlIfMissing` helper used by both call sites; the convenience is slice-local rather than recoverer-public. - `GutenbergEditorPreloader` inlines the discover-and-assign at the call site since it's only one path. Net -95 lines across the change, including tests. --- .../accounts/login/SiteApiRestUrlRecoverer.kt | 99 +++++--------- .../ApplicationPasswordViewModelSlice.kt | 12 +- .../ui/posts/GutenbergEditorPreloader.kt | 5 +- .../login/SiteApiRestUrlRecovererTest.kt | 125 +++--------------- .../ApplicationPasswordViewModelSliceTest.kt | 12 +- .../ui/posts/GutenbergEditorPreloaderTest.kt | 6 +- .../android/fluxc/persistence/SiteSqlUtils.kt | 10 ++ .../fluxc/persistence/SiteSqlUtilsTest.kt | 52 ++++++++ 8 files changed, 139 insertions(+), 182 deletions(-) create mode 100644 libs/fluxc/src/test/java/org/wordpress/android/fluxc/persistence/SiteSqlUtilsTest.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt index b7c488d976e8..d1ab88e992bf 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt @@ -16,13 +16,14 @@ import javax.inject.Singleton import kotlin.coroutines.cancellation.CancellationException /** - * Discovers and populates [SiteModel.wpApiRestUrl] when it's missing, healing sites that - * landed in the DB without one (WP.com `/me/sites` omits the field; headless application- - * password mint goes through the Jetpack tunnel without running discovery). + * Heals [SiteModel.wpApiRestUrl] when it's missing — WP.com `/me/sites` omits the field, and + * headless application-password mint runs through the Jetpack tunnel without doing discovery. * - * - [recoverAndPersistIfMissing] writes to the DB. Used from the auth flow. - * - [discoverInMemoryIfMissing] sets the in-memory model only — for short-lived consumers - * (editor preloader) that just need the URL for one call. + * - [discoverApiRootUrl] runs REST API autodiscovery and returns the discovered root URL. + * - [persistApiRootUrl] writes only that one column to the DB row for `localId`. + * + * Callers handle the "is it missing?" check and the in-memory assignment themselves so the + * mutation stays visible at the call site. */ @Singleton class SiteApiRestUrlRecoverer @Inject constructor( @@ -32,74 +33,38 @@ class SiteApiRestUrlRecoverer @Inject constructor( private val appLogWrapper: AppLogWrapper, @param:Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, ) { - suspend fun recoverAndPersistIfMissing(site: SiteModel) { - if (!site.wpApiRestUrl.isNullOrEmpty()) return - withContext(bgDispatcher) { - val apiRootUrl = runDiscovery(site) ?: return@withContext - site.wpApiRestUrl = apiRootUrl - persist(site.id, apiRootUrl) - } - } - - suspend fun discoverInMemoryIfMissing(site: SiteModel) { - if (!site.wpApiRestUrl.isNullOrEmpty()) return - withContext(bgDispatcher) { - val apiRootUrl = runDiscovery(site) ?: return@withContext - site.wpApiRestUrl = apiRootUrl - appLogWrapper.d( - AppLog.T.API, - "Discovered wpApiRestUrl=$apiRootUrl for ${site.url} (in-memory only)" - ) - } - } - - // Re-reads the DB row by local ID and writes only [SiteModel.wpApiRestUrl]. This preserves - // anything that other code paths (e.g. an UPDATE_APPLICATION_PASSWORD that ran between site - // load and here) wrote since the in-memory model was last loaded. Writing the in-memory site - // directly is *unsafe* — it would clobber concurrent updates to other fields. - @Suppress("SwallowedException") - private fun persist(localId: Int, apiRootUrl: String) { - val siteFromDB = siteSqlUtils.getSitesWithLocalId(localId).firstOrNull() ?: run { - appLogWrapper.w(AppLog.T.API, "Cannot persist wpApiRestUrl: no site with localId=$localId") - return - } - siteFromDB.wpApiRestUrl = apiRootUrl + @Suppress("TooGenericExceptionCaught") + suspend fun discoverApiRootUrl(siteUrl: String): String? = withContext(bgDispatcher) { try { - siteSqlUtils.insertOrUpdateSite(siteFromDB) - appLogWrapper.d( - AppLog.T.API, - "Recovered wpApiRestUrl=$apiRootUrl for ${siteFromDB.url} (persisted)" - ) - } catch (e: SiteSqlUtils.DuplicateSiteException) { + when (val result = wpLoginClient.apiDiscovery(siteUrl)) { + is ApiDiscoveryResult.Success -> { + val apiRootUrl = discoverSuccessWrapper.getApiRootUrl(result) + if (apiRootUrl.isBlank()) null else apiRootUrl + } + else -> { + appLogWrapper.w(AppLog.T.API, "API discovery failed for $siteUrl") + null + } + } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { appLogWrapper.e( AppLog.T.API, - "DuplicateSiteException persisting wpApiRestUrl=$apiRootUrl for ${siteFromDB.url}" + "API discovery threw for $siteUrl: ${e::class.simpleName}: ${e.message}" ) + null } } - @Suppress("TooGenericExceptionCaught") - private suspend fun runDiscovery(site: SiteModel): String? = try { - when (val result = wpLoginClient.apiDiscovery(site.url)) { - is ApiDiscoveryResult.Success -> { - val apiRootUrl = discoverSuccessWrapper.getApiRootUrl(result) - if (apiRootUrl.isBlank()) null else apiRootUrl - } - else -> { - appLogWrapper.w( - AppLog.T.API, - "API discovery failed for ${site.url}" - ) - null - } + suspend fun persistApiRootUrl(localId: Int, apiRootUrl: String): Boolean = withContext(bgDispatcher) { + val rowsUpdated = siteSqlUtils.updateWpApiRestUrl(localId, apiRootUrl) + if (rowsUpdated == 0) { + appLogWrapper.w(AppLog.T.API, "Cannot persist wpApiRestUrl: no site with localId=$localId") + false + } else { + appLogWrapper.d(AppLog.T.API, "Persisted wpApiRestUrl=$apiRootUrl for localId=$localId") + true } - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - appLogWrapper.e( - AppLog.T.API, - "API discovery threw for ${site.url}: ${e::class.simpleName}: ${e.message}" - ) - null } } 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 37d1edb86955..0a57e8992e1b 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 @@ -84,7 +84,7 @@ class ApplicationPasswordViewModelSlice @Inject constructor( when (applicationPasswordValidator.validate(storedSite)) { ApplicationPasswordValidator.Outcome.Valid -> { // Heal in the background so the card hides immediately on a slow network. - scope.launch { siteApiRestUrlRecoverer.recoverAndPersistIfMissing(storedSite) } + scope.launch { healApiRestUrlIfMissing(storedSite) } handleValidAuth(storedSite) return@launch } @@ -115,7 +115,7 @@ class ApplicationPasswordViewModelSlice @Inject constructor( // 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 { siteApiRestUrlRecoverer.recoverAndPersistIfMissing(storedSite) } + scope.launch { healApiRestUrlIfMissing(storedSite) } handleValidAuth(storedSite) return@launch } @@ -135,6 +135,14 @@ class ApplicationPasswordViewModelSlice @Inject constructor( } } + 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 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. 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 9a5ac1422bbe..cf8bd49977fc 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 @@ -95,7 +95,10 @@ class GutenbergEditorPreloader @Inject constructor( val siteId = site.id val job = scope.launch(bgDispatcher) { try { - siteApiRestUrlRecoverer.discoverInMemoryIfMissing(site) + if (site.wpApiRestUrl.isNullOrEmpty()) { + siteApiRestUrlRecoverer.discoverApiRootUrl(site.url) + ?.let { site.wpApiRestUrl = it } + } editorSettingsRepository .fetchEditorCapabilitiesForSite(site) // Preloading produces EditorDependencies, which the editor diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt index 491e35345892..c56204ce7e3b 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt @@ -11,11 +11,8 @@ import org.mockito.MockitoAnnotations import org.mockito.kotlin.any import org.mockito.kotlin.doThrow import org.mockito.kotlin.eq -import org.mockito.kotlin.never -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.persistence.SiteSqlUtils import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.DiscoverSuccessWrapper @@ -52,12 +49,6 @@ class SiteApiRestUrlRecovererTest : BaseUnitTest() { ) } - private fun siteWithoutUrl(): SiteModel = SiteModel().apply { - id = LOCAL_ID - url = SITE_URL - wpApiRestUrl = null - } - private suspend fun stubDiscoverySuccess(apiRootUrl: String) { val result = ApiDiscoveryResult.Success( AutoDiscoveryAttemptSuccess( @@ -71,141 +62,69 @@ class SiteApiRestUrlRecovererTest : BaseUnitTest() { } @Test - fun `recoverAndPersist populates wpApiRestUrl in memory and writes the DB row`() = runTest { - val site = siteWithoutUrl() - val siteFromDB = siteWithoutUrl() + fun `discoverApiRootUrl returns the discovered URL on success`() = runTest { stubDiscoverySuccess(DISCOVERED_API_ROOT) - whenever(siteSqlUtils.getSitesWithLocalId(LOCAL_ID)).thenReturn(listOf(siteFromDB)) - - recoverer.recoverAndPersistIfMissing(site) - - assertThat(site.wpApiRestUrl).isEqualTo(DISCOVERED_API_ROOT) - assertThat(siteFromDB.wpApiRestUrl).isEqualTo(DISCOVERED_API_ROOT) - verify(siteSqlUtils).insertOrUpdateSite(siteFromDB) - } - @Test - fun `recoverAndPersist skips discovery when wpApiRestUrl is already populated`() = runTest { - val site = siteWithoutUrl().apply { wpApiRestUrl = "https://example.test/wp-json/" } - - recoverer.recoverAndPersistIfMissing(site) + val result = recoverer.discoverApiRootUrl(SITE_URL) - verify(wpLoginClient, never()).apiDiscovery(any()) - verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + assertThat(result).isEqualTo(DISCOVERED_API_ROOT) } @Test - fun `recoverAndPersist does nothing when discovery returns a blank apiRootUrl`() = runTest { - val site = siteWithoutUrl() + fun `discoverApiRootUrl returns null when the discovered URL is blank`() = runTest { stubDiscoverySuccess(apiRootUrl = "") - recoverer.recoverAndPersistIfMissing(site) + val result = recoverer.discoverApiRootUrl(SITE_URL) - assertThat(site.wpApiRestUrl).isNull() - verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + assertThat(result).isNull() } @Test - fun `recoverAndPersist does nothing when discovery returns a failure`() = runTest { - val site = siteWithoutUrl() + fun `discoverApiRootUrl returns null when discovery returns a failure`() = runTest { whenever(wpLoginClient.apiDiscovery(any())).thenReturn( ApiDiscoveryResult.FailureParseSiteUrl(ParseUrlException.Generic("")) ) - recoverer.recoverAndPersistIfMissing(site) + val result = recoverer.discoverApiRootUrl(SITE_URL) - assertThat(site.wpApiRestUrl).isNull() - verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + assertThat(result).isNull() } @Test - fun `recoverAndPersist swallows non-cancellation exceptions thrown by discovery`() = runTest { - val site = siteWithoutUrl() + fun `discoverApiRootUrl swallows non-cancellation exceptions and returns null`() = runTest { whenever(wpLoginClient.apiDiscovery(any())) .doThrow(RuntimeException("network error")) - recoverer.recoverAndPersistIfMissing(site) + val result = recoverer.discoverApiRootUrl(SITE_URL) - assertThat(site.wpApiRestUrl).isNull() - verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + assertThat(result).isNull() } @Test - fun `recoverAndPersist rethrows CancellationException to preserve structured concurrency`() = runTest { - val site = siteWithoutUrl() + fun `discoverApiRootUrl rethrows CancellationException to preserve structured concurrency`() = runTest { whenever(wpLoginClient.apiDiscovery(any())) .doThrow(CancellationException("cancelled")) assertFailsWith { - recoverer.recoverAndPersistIfMissing(site) + recoverer.discoverApiRootUrl(SITE_URL) } - verify(siteSqlUtils, never()).insertOrUpdateSite(any()) } @Test - fun `recoverAndPersist skips the DB write when the site is not in the DB`() = runTest { - val site = siteWithoutUrl() - stubDiscoverySuccess(DISCOVERED_API_ROOT) - whenever(siteSqlUtils.getSitesWithLocalId(LOCAL_ID)).thenReturn(emptyList()) + fun `persistApiRootUrl returns true and writes the column when a row matches`() = runTest { + whenever(siteSqlUtils.updateWpApiRestUrl(LOCAL_ID, DISCOVERED_API_ROOT)).thenReturn(1) - recoverer.recoverAndPersistIfMissing(site) + val updated = recoverer.persistApiRootUrl(LOCAL_ID, DISCOVERED_API_ROOT) - assertThat(site.wpApiRestUrl).isEqualTo(DISCOVERED_API_ROOT) - verify(siteSqlUtils, never()).insertOrUpdateSite(any()) + assertThat(updated).isTrue() } @Test - fun `discoverInMemory populates wpApiRestUrl when missing but never touches the DB`() = runTest { - val site = siteWithoutUrl() - stubDiscoverySuccess(DISCOVERED_API_ROOT) + fun `persistApiRootUrl returns false when no row matches the local id`() = runTest { + whenever(siteSqlUtils.updateWpApiRestUrl(LOCAL_ID, DISCOVERED_API_ROOT)).thenReturn(0) - recoverer.discoverInMemoryIfMissing(site) + val updated = recoverer.persistApiRootUrl(LOCAL_ID, DISCOVERED_API_ROOT) - assertThat(site.wpApiRestUrl).isEqualTo(DISCOVERED_API_ROOT) - verify(siteSqlUtils, never()).getSitesWithLocalId(any()) - verify(siteSqlUtils, never()).insertOrUpdateSite(any()) - } - - @Test - fun `discoverInMemory skips discovery when wpApiRestUrl is already populated`() = runTest { - val site = siteWithoutUrl().apply { wpApiRestUrl = "https://example.test/wp-json/" } - - recoverer.discoverInMemoryIfMissing(site) - - verify(wpLoginClient, never()).apiDiscovery(any()) - } - - @Test - fun `discoverInMemory does nothing when discovery fails`() = runTest { - val site = siteWithoutUrl() - whenever(wpLoginClient.apiDiscovery(any())).thenReturn( - ApiDiscoveryResult.FailureParseSiteUrl(ParseUrlException.Generic("")) - ) - - recoverer.discoverInMemoryIfMissing(site) - - assertThat(site.wpApiRestUrl).isNull() - } - - @Test - fun `discoverInMemory swallows non-cancellation exceptions thrown by discovery`() = runTest { - val site = siteWithoutUrl() - whenever(wpLoginClient.apiDiscovery(any())) - .doThrow(RuntimeException("network error")) - - recoverer.discoverInMemoryIfMissing(site) - - assertThat(site.wpApiRestUrl).isNull() - } - - @Test - fun `discoverInMemory rethrows CancellationException to preserve structured concurrency`() = runTest { - val site = siteWithoutUrl() - whenever(wpLoginClient.apiDiscovery(any())) - .doThrow(CancellationException("cancelled")) - - assertFailsWith { - recoverer.discoverInMemoryIfMissing(site) - } + assertThat(updated).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 67ec59c0662f..8ef1fcf80454 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 @@ -180,14 +180,14 @@ class ApplicationPasswordViewModelSliceTest : BaseUnitTest() { fun `given headless mint succeeds, card hides without waiting for the recoverer`() = runTest { stubMintSuccess() val recoverGate = CompletableDeferred() - whenever(siteApiRestUrlRecoverer.recoverAndPersistIfMissing(any())) - .doSuspendableAnswer { recoverGate.await() } + whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(any())) + .doSuspendableAnswer { recoverGate.await(); null } applicationPasswordViewModelSlice.buildCard(siteTest) // Card has been hidden even though the recoverer is still suspended on the gate. assertNull(applicationPasswordCard) - verify(siteApiRestUrlRecoverer).recoverAndPersistIfMissing(siteTest) + verify(siteApiRestUrlRecoverer).discoverApiRootUrl(siteTest.url) // Release the recoverer so the test scope doesn't carry a dangling coroutine. recoverGate.complete(Unit) @@ -210,13 +210,13 @@ class ApplicationPasswordViewModelSliceTest : BaseUnitTest() { whenever(applicationPasswordValidator.validate(any())) .thenReturn(ApplicationPasswordValidator.Outcome.Valid) val recoverGate = CompletableDeferred() - whenever(siteApiRestUrlRecoverer.recoverAndPersistIfMissing(any())) - .doSuspendableAnswer { recoverGate.await() } + whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(any())) + .doSuspendableAnswer { recoverGate.await(); null } applicationPasswordViewModelSlice.buildCard(siteTest) assertNull(applicationPasswordCard) - verify(siteApiRestUrlRecoverer).recoverAndPersistIfMissing(any()) + verify(siteApiRestUrlRecoverer).discoverApiRootUrl(TEST_URL) recoverGate.complete(Unit) } 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 15e6165b0236..6023d29f3423 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 @@ -489,7 +489,7 @@ class GutenbergEditorPreloaderTest : // region wpApiRestUrl recovery @Test - fun `successful preload invokes in-memory discovery only — slice owns persistence`() = test { + fun `successful preload invokes discovery only — slice owns persistence`() = test { val site = createSite() enablePreloading(site) stubSuccessfulPreload() @@ -498,8 +498,8 @@ class GutenbergEditorPreloaderTest : preloader.preloadIfNeeded(site, this) advanceUntilIdle() - verify(siteApiRestUrlRecoverer).discoverInMemoryIfMissing(site) - verify(siteApiRestUrlRecoverer, never()).recoverAndPersistIfMissing(any()) + verify(siteApiRestUrlRecoverer).discoverApiRootUrl(site.url) + verify(siteApiRestUrlRecoverer, never()).persistApiRootUrl(any(), any()) } // endregion diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/SiteSqlUtils.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/SiteSqlUtils.kt index 1f7d99260605..26a0955b5d7c 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/SiteSqlUtils.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/SiteSqlUtils.kt @@ -260,6 +260,16 @@ class SiteSqlUtils }).execute() } + fun updateWpApiRestUrl(localId: Int, wpApiRestUrl: String): Int { + return WellSql.update(SiteModel::class.java) + .whereId(localId) + .put(wpApiRestUrl, { value -> + val cv = ContentValues() + cv.put(SiteModelTable.WP_API_REST_URL, value) + cv + }).execute() + } + val wPComSites: SelectQuery get() = WellSql.select(SiteModel::class.java) .where().beginGroup() diff --git a/libs/fluxc/src/test/java/org/wordpress/android/fluxc/persistence/SiteSqlUtilsTest.kt b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/persistence/SiteSqlUtilsTest.kt new file mode 100644 index 000000000000..28885f9cdf0e --- /dev/null +++ b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/persistence/SiteSqlUtilsTest.kt @@ -0,0 +1,52 @@ +package org.wordpress.android.fluxc.persistence + +import com.yarolegovich.wellsql.WellSql +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.wordpress.android.fluxc.encryption.EncryptionUtils +import org.wordpress.android.fluxc.model.SiteModel + +@RunWith(RobolectricTestRunner::class) +class SiteSqlUtilsTest { + private val siteSqlUtils = SiteSqlUtils(EncryptionUtils()) + + @Before + fun setUp() { + val appContext = RuntimeEnvironment.getApplication().applicationContext + val config = WellSqlConfig(appContext) + WellSql.init(config) + config.reset() + } + + @Test + fun `updateWpApiRestUrl writes the column and leaves other fields alone`() { + val site = SiteModel().apply { + id = 1 + siteId = 42 + url = "https://example.test" + name = "Example" + wpApiRestUrl = null + } + WellSql.insert(site).execute() + + val rowsUpdated = siteSqlUtils.updateWpApiRestUrl(localId = 1, wpApiRestUrl = "https://example.test/wp-json/") + + assertThat(rowsUpdated).isEqualTo(1) + val stored = siteSqlUtils.getSitesWithLocalId(1).single() + assertThat(stored.wpApiRestUrl).isEqualTo("https://example.test/wp-json/") + assertThat(stored.url).isEqualTo("https://example.test") + assertThat(stored.name).isEqualTo("Example") + assertThat(stored.siteId).isEqualTo(42) + } + + @Test + fun `updateWpApiRestUrl returns 0 when no site row matches the local id`() { + val rowsUpdated = siteSqlUtils.updateWpApiRestUrl(localId = 999, wpApiRestUrl = "https://example.test/wp-json/") + + assertThat(rowsUpdated).isEqualTo(0) + } +} From 33e503f7882a60128c73b043aad14d080e56680f Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 27 May 2026 14:25:02 -0600 Subject: [PATCH 3/5] Preserve `wpApiRestUrl` from DB when `/me/sites` response omits it The WP.com `/me/sites` payload never includes `wpApiRestUrl`. The existing preservation block in `createOrUpdateSites` already copies the DB value forward, but it was gated on the site having encrypted application-password credentials. For Atomic sites that have a recovered `wpApiRestUrl` but no stored creds yet (or where the DB read races ahead of the recovery's single-column UPDATE), the URL was clobbered to NULL by the full-row write. Move the `wpApiRestUrl` preservation out of the credentials gate. Credentials and the API root URL are independent fields and shouldn't share a precondition. The new behavior matches the surrounding pattern for `mobileEditor`/`webEditor` (always preserved when the response omits them). Reported by @oguzkocer with a logcat reproduction on `quick-six.jurassic.ninja`. --- .../android/fluxc/store/SiteStore.kt | 4 ++ .../android/fluxc/store/SiteStoreTest.kt | 50 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt index de9ecba91a4c..b054d7e828e4 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt @@ -1789,6 +1789,10 @@ open class SiteStore @Inject constructor( site.apiRestPasswordEncrypted = siteFromDB.apiRestPasswordEncrypted site.apiRestUsernameIV = siteFromDB.apiRestUsernameIV site.apiRestPasswordIV = siteFromDB.apiRestPasswordIV + } + // /me/sites never includes wpApiRestUrl. Preserve the DB value when the + // response omits one so a concurrent recovery write isn't clobbered. + if (site.wpApiRestUrl.isNullOrEmpty()) { site.wpApiRestUrl = siteFromDB.wpApiRestUrl } } diff --git a/libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt index 68cdc7821aa0..cfecc8914cdd 100644 --- a/libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt +++ b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt @@ -189,6 +189,56 @@ class SiteStoreTest { inOrder.verify(siteSqlUtils).removeWPComRestSitesAbsentFromList(postSqlUtils, sitesModel.sites) } + @Test + fun `fetchSites preserves DB wpApiRestUrl when the response omits it`() = test { + val payload = FetchSitesPayload(listOf(WPCOM)) + val fetchedSite = SiteModel().apply { + siteId = 42 + url = "https://atomic.example/" + // /me/sites doesn't include wpApiRestUrl — leave it null. + } + val sitesModel = SitesModel().apply { sites = listOf(fetchedSite) } + val siteFromDB = SiteModel().apply { + siteId = 42 + url = "https://atomic.example/" + wpApiRestUrl = "https://atomic.example/wp-json/" + } + whenever(siteRestClient.fetchSites(payload.filters, false)).thenReturn(sitesModel) + whenever(siteSqlUtils.getSitesWithRemoteId(42)).thenReturn(listOf(siteFromDB)) + whenever(siteSqlUtils.insertOrUpdateSite(any())).thenReturn(1) + + siteStore.fetchSites(payload) + + verify(siteSqlUtils).insertOrUpdateSite( + argWhere { it.wpApiRestUrl == "https://atomic.example/wp-json/" } + ) + } + + @Test + fun `fetchSites uses the response wpApiRestUrl when present`() = test { + val payload = FetchSitesPayload(listOf(WPCOM)) + val fetchedSite = SiteModel().apply { + siteId = 42 + url = "https://atomic.example/" + wpApiRestUrl = "https://atomic.example/wp-json/v2/" + } + val sitesModel = SitesModel().apply { sites = listOf(fetchedSite) } + val siteFromDB = SiteModel().apply { + siteId = 42 + url = "https://atomic.example/" + wpApiRestUrl = "https://atomic.example/wp-json/" + } + whenever(siteRestClient.fetchSites(payload.filters, false)).thenReturn(sitesModel) + whenever(siteSqlUtils.getSitesWithRemoteId(42)).thenReturn(listOf(siteFromDB)) + whenever(siteSqlUtils.insertOrUpdateSite(any())).thenReturn(1) + + siteStore.fetchSites(payload) + + verify(siteSqlUtils).insertOrUpdateSite( + argWhere { it.wpApiRestUrl == "https://atomic.example/wp-json/v2/" } + ) + } + @Test fun `fetchSites saves jetpack CP connected sites to DB`() = test { val payload = FetchSitesPayload(listOf(WPCOM)) From 61a0d55239c94158c9ce321b587b624ab1bd9c46 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 27 May 2026 16:06:38 -0600 Subject: [PATCH 4/5] Skip DB persist for WPCom-REST sites in the recovery heal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /me/sites response doesn't include wpApiRestUrl, so persisting a recovered URL for sites that flow through that path gets clobbered to NULL on the next FETCH_SITES (the response handler's full-row insertOrUpdateSite overwrites the recovery's single-column write). For these sites, stick to the in-memory heal — it survives long enough for the editor session, and the next launch re-discovers. Self-hosted sites continue to get a durable persist (their wpApiRestUrl isn't fetched from /me/sites and isn't clobbered). Simple WP.com sites are unaffected because `SiteModel.getWpApiRestUrl()` returns a synthetic `public-api.wordpress.com/wp/v2/sites/` URL for them, so the heal's early-return fires before reaching this branch. --- .../ApplicationPasswordViewModelSlice.kt | 8 ++- .../ApplicationPasswordViewModelSliceTest.kt | 60 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) 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 0a57e8992e1b..2f5db801f749 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 @@ -139,7 +139,13 @@ class ApplicationPasswordViewModelSlice @Inject constructor( if (!site.wpApiRestUrl.isNullOrEmpty()) return siteApiRestUrlRecoverer.discoverApiRootUrl(site.url)?.let { apiRootUrl -> site.wpApiRestUrl = apiRootUrl - siteApiRestUrlRecoverer.persistApiRootUrl(site.id, apiRootUrl) + // Sites going through /me/sites can have their wpApiRestUrl clobbered to NULL on + // a subsequent FETCH_SITES because the response omits the field. Skip the DB write + // for those — the in-memory heal is enough for the editor session, and we'll + // re-discover on the next launch. Self-hosted sites get a durable persist. + if (!site.isUsingWpComRestApi) { + siteApiRestUrlRecoverer.persistApiRootUrl(site.id, apiRootUrl) + } } } 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 8ef1fcf80454..7250f6373962 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 @@ -193,6 +193,66 @@ class ApplicationPasswordViewModelSliceTest : BaseUnitTest() { recoverGate.complete(Unit) } + @Test + fun `heal persists when site is self-hosted`() = 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 + // setIsWPCom not called -> isUsingWpComRestApi() == false (self-hosted) + } + ) + ) + whenever(applicationPasswordValidator.validate(any())) + .thenReturn(ApplicationPasswordValidator.Outcome.Valid) + whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(eq(TEST_URL))) + .thenReturn("$TEST_URL/wp-json/") + + applicationPasswordViewModelSlice.buildCard(siteTest) + advanceUntilIdle() + + verify(siteApiRestUrlRecoverer).discoverApiRootUrl(TEST_URL) + verify(siteApiRestUrlRecoverer).persistApiRootUrl(eq(siteTest.id), eq("$TEST_URL/wp-json/")) + } + + @Test + fun `heal does not persist when site is Atomic`() = runTest { + // Atomic sites go through /me/sites which doesn't include wpApiRestUrl. Persisting would + // get clobbered on the next FETCH_SITES, so skip the DB write — the in-memory heal is + // enough for the editor session and we'll re-discover next launch. + // (Simple WP.com sites get a synthetic public-api URL from getWpApiRestUrl(), so the heal + // early-returns for them and this branch is never reached.) + 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 + setIsWPCom(true) + setIsWPComAtomic(true) + } + ) + ) + whenever(applicationPasswordValidator.validate(any())) + .thenReturn(ApplicationPasswordValidator.Outcome.Valid) + whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(eq(TEST_URL))) + .thenReturn("$TEST_URL/wp-json/") + + applicationPasswordViewModelSlice.buildCard(siteTest) + advanceUntilIdle() + + verify(siteApiRestUrlRecoverer).discoverApiRootUrl(TEST_URL) + verify(siteApiRestUrlRecoverer, never()).persistApiRootUrl(any(), any()) + } + @Test fun `given valid stored creds, card hides without waiting for the recoverer`() = runTest { whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(false) From 3801c69b6fe51a9a71113687a0fc02b9dca04a73 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 27 May 2026 17:15:58 -0600 Subject: [PATCH 5/5] Revert FluxC clobber-mitigation attempts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts both: - 61a0d55239c "Skip DB persist for WPCom-REST sites in the recovery heal" - 33e503f7882 "Preserve wpApiRestUrl from DB when /me/sites response omits it" Investigation showed both commits modify code paths that aren't the actual source of the clobber reported in review. The "Site found by (local) ID: 9" log in the reproduction can't originate from createOrUpdateSites — that handler passes sites with id=0 (sites flow in from siteResponseToSiteModel which never sets the local id), so the lookup would log "Site found by SITE_ID" instead. The real clobber comes from a different handler taking a stale full-row snapshot, most likely FETCH_SITE → updateSite triggered on every foreground. A complete fix requires either preserving wpApiRestUrl at every full-row writer (~10 call sites in SiteStore plus ReactNativeStore and CookieNonceAuthenticator), or excluding the column from UpdateAllExceptId so the recovery's single-column UPDATE is the sole writer. Neither is in scope for this PR — shipping with the in-memory heal as approved; the durable DB heal is a follow-up. --- .../ApplicationPasswordViewModelSlice.kt | 8 +-- .../ApplicationPasswordViewModelSliceTest.kt | 60 ------------------- .../android/fluxc/store/SiteStore.kt | 4 -- .../android/fluxc/store/SiteStoreTest.kt | 50 ---------------- 4 files changed, 1 insertion(+), 121 deletions(-) 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 2f5db801f749..0a57e8992e1b 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 @@ -139,13 +139,7 @@ class ApplicationPasswordViewModelSlice @Inject constructor( if (!site.wpApiRestUrl.isNullOrEmpty()) return siteApiRestUrlRecoverer.discoverApiRootUrl(site.url)?.let { apiRootUrl -> site.wpApiRestUrl = apiRootUrl - // Sites going through /me/sites can have their wpApiRestUrl clobbered to NULL on - // a subsequent FETCH_SITES because the response omits the field. Skip the DB write - // for those — the in-memory heal is enough for the editor session, and we'll - // re-discover on the next launch. Self-hosted sites get a durable persist. - if (!site.isUsingWpComRestApi) { - siteApiRestUrlRecoverer.persistApiRootUrl(site.id, apiRootUrl) - } + siteApiRestUrlRecoverer.persistApiRootUrl(site.id, apiRootUrl) } } 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 7250f6373962..8ef1fcf80454 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 @@ -193,66 +193,6 @@ class ApplicationPasswordViewModelSliceTest : BaseUnitTest() { recoverGate.complete(Unit) } - @Test - fun `heal persists when site is self-hosted`() = 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 - // setIsWPCom not called -> isUsingWpComRestApi() == false (self-hosted) - } - ) - ) - whenever(applicationPasswordValidator.validate(any())) - .thenReturn(ApplicationPasswordValidator.Outcome.Valid) - whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(eq(TEST_URL))) - .thenReturn("$TEST_URL/wp-json/") - - applicationPasswordViewModelSlice.buildCard(siteTest) - advanceUntilIdle() - - verify(siteApiRestUrlRecoverer).discoverApiRootUrl(TEST_URL) - verify(siteApiRestUrlRecoverer).persistApiRootUrl(eq(siteTest.id), eq("$TEST_URL/wp-json/")) - } - - @Test - fun `heal does not persist when site is Atomic`() = runTest { - // Atomic sites go through /me/sites which doesn't include wpApiRestUrl. Persisting would - // get clobbered on the next FETCH_SITES, so skip the DB write — the in-memory heal is - // enough for the editor session and we'll re-discover next launch. - // (Simple WP.com sites get a synthetic public-api URL from getWpApiRestUrl(), so the heal - // early-returns for them and this branch is never reached.) - 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 - setIsWPCom(true) - setIsWPComAtomic(true) - } - ) - ) - whenever(applicationPasswordValidator.validate(any())) - .thenReturn(ApplicationPasswordValidator.Outcome.Valid) - whenever(siteApiRestUrlRecoverer.discoverApiRootUrl(eq(TEST_URL))) - .thenReturn("$TEST_URL/wp-json/") - - applicationPasswordViewModelSlice.buildCard(siteTest) - advanceUntilIdle() - - verify(siteApiRestUrlRecoverer).discoverApiRootUrl(TEST_URL) - verify(siteApiRestUrlRecoverer, never()).persistApiRootUrl(any(), any()) - } - @Test fun `given valid stored creds, card hides without waiting for the recoverer`() = runTest { whenever(applicationPasswordLoginHelper.siteHasBadCredentials(any())).thenReturn(false) diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt index b054d7e828e4..de9ecba91a4c 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt @@ -1789,10 +1789,6 @@ open class SiteStore @Inject constructor( site.apiRestPasswordEncrypted = siteFromDB.apiRestPasswordEncrypted site.apiRestUsernameIV = siteFromDB.apiRestUsernameIV site.apiRestPasswordIV = siteFromDB.apiRestPasswordIV - } - // /me/sites never includes wpApiRestUrl. Preserve the DB value when the - // response omits one so a concurrent recovery write isn't clobbered. - if (site.wpApiRestUrl.isNullOrEmpty()) { site.wpApiRestUrl = siteFromDB.wpApiRestUrl } } diff --git a/libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt index cfecc8914cdd..68cdc7821aa0 100644 --- a/libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt +++ b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt @@ -189,56 +189,6 @@ class SiteStoreTest { inOrder.verify(siteSqlUtils).removeWPComRestSitesAbsentFromList(postSqlUtils, sitesModel.sites) } - @Test - fun `fetchSites preserves DB wpApiRestUrl when the response omits it`() = test { - val payload = FetchSitesPayload(listOf(WPCOM)) - val fetchedSite = SiteModel().apply { - siteId = 42 - url = "https://atomic.example/" - // /me/sites doesn't include wpApiRestUrl — leave it null. - } - val sitesModel = SitesModel().apply { sites = listOf(fetchedSite) } - val siteFromDB = SiteModel().apply { - siteId = 42 - url = "https://atomic.example/" - wpApiRestUrl = "https://atomic.example/wp-json/" - } - whenever(siteRestClient.fetchSites(payload.filters, false)).thenReturn(sitesModel) - whenever(siteSqlUtils.getSitesWithRemoteId(42)).thenReturn(listOf(siteFromDB)) - whenever(siteSqlUtils.insertOrUpdateSite(any())).thenReturn(1) - - siteStore.fetchSites(payload) - - verify(siteSqlUtils).insertOrUpdateSite( - argWhere { it.wpApiRestUrl == "https://atomic.example/wp-json/" } - ) - } - - @Test - fun `fetchSites uses the response wpApiRestUrl when present`() = test { - val payload = FetchSitesPayload(listOf(WPCOM)) - val fetchedSite = SiteModel().apply { - siteId = 42 - url = "https://atomic.example/" - wpApiRestUrl = "https://atomic.example/wp-json/v2/" - } - val sitesModel = SitesModel().apply { sites = listOf(fetchedSite) } - val siteFromDB = SiteModel().apply { - siteId = 42 - url = "https://atomic.example/" - wpApiRestUrl = "https://atomic.example/wp-json/" - } - whenever(siteRestClient.fetchSites(payload.filters, false)).thenReturn(sitesModel) - whenever(siteSqlUtils.getSitesWithRemoteId(42)).thenReturn(listOf(siteFromDB)) - whenever(siteSqlUtils.insertOrUpdateSite(any())).thenReturn(1) - - siteStore.fetchSites(payload) - - verify(siteSqlUtils).insertOrUpdateSite( - argWhere { it.wpApiRestUrl == "https://atomic.example/wp-json/v2/" } - ) - } - @Test fun `fetchSites saves jetpack CP connected sites to DB`() = test { val payload = FetchSitesPayload(listOf(WPCOM))