diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 719b06bd07d6..b220eba06021 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. * [*] Try out the next-generation block editor on a per-site basis from Site Settings. 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..d1ab88e992bf --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecoverer.kt @@ -0,0 +1,70 @@ +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 + +/** + * 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. + * + * - [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( + private val wpLoginClient: WpLoginClient, + private val discoverSuccessWrapper: DiscoverSuccessWrapper, + private val siteSqlUtils: SiteSqlUtils, + private val appLogWrapper: AppLogWrapper, + @param:Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, +) { + @Suppress("TooGenericExceptionCaught") + suspend fun discoverApiRootUrl(siteUrl: String): String? = withContext(bgDispatcher) { + try { + 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, + "API discovery threw for $siteUrl: ${e::class.simpleName}: ${e.message}" + ) + 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 + } + } +} 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..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 @@ -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 { healApiRestUrlIfMissing(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 { healApiRestUrlIfMissing(storedSite) } handleValidAuth(storedSite) return@launch } @@ -127,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 d420c6103b12..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 @@ -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,10 @@ 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) // 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..c56204ce7e3b --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/SiteApiRestUrlRecovererTest.kt @@ -0,0 +1,130 @@ +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.whenever +import org.wordpress.android.BaseUnitTest +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 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 `discoverApiRootUrl returns the discovered URL on success`() = runTest { + stubDiscoverySuccess(DISCOVERED_API_ROOT) + + val result = recoverer.discoverApiRootUrl(SITE_URL) + + assertThat(result).isEqualTo(DISCOVERED_API_ROOT) + } + + @Test + fun `discoverApiRootUrl returns null when the discovered URL is blank`() = runTest { + stubDiscoverySuccess(apiRootUrl = "") + + val result = recoverer.discoverApiRootUrl(SITE_URL) + + assertThat(result).isNull() + } + + @Test + fun `discoverApiRootUrl returns null when discovery returns a failure`() = runTest { + whenever(wpLoginClient.apiDiscovery(any())).thenReturn( + ApiDiscoveryResult.FailureParseSiteUrl(ParseUrlException.Generic("")) + ) + + val result = recoverer.discoverApiRootUrl(SITE_URL) + + assertThat(result).isNull() + } + + @Test + fun `discoverApiRootUrl swallows non-cancellation exceptions and returns null`() = runTest { + whenever(wpLoginClient.apiDiscovery(any())) + .doThrow(RuntimeException("network error")) + + val result = recoverer.discoverApiRootUrl(SITE_URL) + + assertThat(result).isNull() + } + + @Test + fun `discoverApiRootUrl rethrows CancellationException to preserve structured concurrency`() = runTest { + whenever(wpLoginClient.apiDiscovery(any())) + .doThrow(CancellationException("cancelled")) + + assertFailsWith { + recoverer.discoverApiRootUrl(SITE_URL) + } + } + + @Test + fun `persistApiRootUrl returns true and writes the column when a row matches`() = runTest { + whenever(siteSqlUtils.updateWpApiRestUrl(LOCAL_ID, DISCOVERED_API_ROOT)).thenReturn(1) + + val updated = recoverer.persistApiRootUrl(LOCAL_ID, DISCOVERED_API_ROOT) + + assertThat(updated).isTrue() + } + + @Test + fun `persistApiRootUrl returns false when no row matches the local id`() = runTest { + whenever(siteSqlUtils.updateWpApiRestUrl(LOCAL_ID, DISCOVERED_API_ROOT)).thenReturn(0) + + val updated = recoverer.persistApiRootUrl(LOCAL_ID, DISCOVERED_API_ROOT) + + 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 d7ab7ed71d04..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 @@ -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.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).discoverApiRootUrl(siteTest.url) + + // 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.discoverApiRootUrl(any())) + .doSuspendableAnswer { recoverGate.await(); null } + + applicationPasswordViewModelSlice.buildCard(siteTest) + + assertNull(applicationPasswordCard) + verify(siteApiRestUrlRecoverer).discoverApiRootUrl(TEST_URL) + + 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..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 @@ -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 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 } 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) + } +}