diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 1c4710713d9a..a50e9d597132 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -4,6 +4,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`. 26.7 ----- diff --git a/WordPress/src/main/java/org/wordpress/android/repositories/EditorSettingsRepository.kt b/WordPress/src/main/java/org/wordpress/android/repositories/EditorSettingsRepository.kt index c6c08ee25601..a63d68dcecd4 100644 --- a/WordPress/src/main/java/org/wordpress/android/repositories/EditorSettingsRepository.kt +++ b/WordPress/src/main/java/org/wordpress/android/repositories/EditorSettingsRepository.kt @@ -12,7 +12,11 @@ import org.wordpress.android.fluxc.persistence.EditorSettingsSqlUtils import org.wordpress.android.modules.IO_THREAD import org.wordpress.android.util.AppLog import org.wordpress.android.util.AppLog.T +import rs.wordpress.api.kotlin.ApiDiscoveryResult +import rs.wordpress.api.kotlin.WpLoginClient import rs.wordpress.api.kotlin.WpRequestResult +import uniffi.wp_api.ApiUrlResolver +import uniffi.wp_api.WpApiDetails import javax.inject.Inject import javax.inject.Named import javax.inject.Singleton @@ -20,6 +24,7 @@ import javax.inject.Singleton @Singleton class EditorSettingsRepository @Inject constructor( private val wpApiClientProvider: WpApiClientProvider, + private val wpLoginClient: WpLoginClient, private val appPrefsWrapper: AppPrefsWrapper, private val themeRepository: ThemeRepository, @Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher @@ -92,36 +97,14 @@ class EditorSettingsRepository @Inject constructor( private suspend fun fetchRouteSupport( site: SiteModel ): Boolean = try { - val client = - wpApiClientProvider.getWpApiClient(site) - val resolver = - wpApiClientProvider.getApiUrlResolver(site) - val response = - client.request { it.apiRoot().get() } - - if (response is WpRequestResult.Success) { - val data = response.response.data - appPrefsWrapper - .setSiteSupportsEditorSettings( - site, - data.hasRouteForEndpoint( - resolver, - "/wp-block-editor/v1", - "settings" - ) - ) - appPrefsWrapper - .setSiteSupportsEditorAssets( - site, - data.hasRouteForEndpoint( - resolver, - "/wpcom/v2", - "editor-assets" - ) - ) - true + // For Atomic sites the editor fetches `wp-block-editor/v1/settings` + // from the direct host — proxy and direct host can advertise + // different route lists, so detection has to probe the direct host + // too. See #22879. + if (site.isWPComAtomic) { + fetchRouteSupportViaDirectHostDiscovery(site) } else { - false + fetchRouteSupportViaConfiguredClient(site) } } catch (e: CancellationException) { throw e @@ -135,6 +118,70 @@ class EditorSettingsRepository @Inject constructor( false } + private suspend fun fetchRouteSupportViaConfiguredClient( + site: SiteModel + ): Boolean { + val client = wpApiClientProvider.getWpApiClient(site) + val resolver = wpApiClientProvider.getApiUrlResolver(site) + val response = client.request { it.apiRoot().get() } + return if (response is WpRequestResult.Success) { + persistRouteSupport(site, response.response.data, resolver) + true + } else { + false + } + } + + /** + * On WP.com Atomic sites the editor fetches `wp-block-editor/v1/settings` + * from the direct host — not the WP.com proxy — so detection has to + * match. Run REST API autodiscovery on the site URL so we don't have to + * assume the API lives at `/wp-json` (custom permalink structures or + * REST API paths would break that assumption), then use the routes list + * returned by discovery directly — no second request needed. + */ + private suspend fun fetchRouteSupportViaDirectHostDiscovery( + site: SiteModel + ): Boolean { + val discovery = wpLoginClient.apiDiscovery(site.url) + if (discovery !is ApiDiscoveryResult.Success) { + AppLog.w( + T.EDITOR, + "Direct-host API discovery failed for" + + " site=${site.name}: ${discovery::class.simpleName}" + ) + return false + } + val resolver = wpApiClientProvider.urlResolverFor( + discovery.success.apiRootUrl + ) + persistRouteSupport(site, discovery.success.apiDetails, resolver) + return true + } + + private fun persistRouteSupport( + site: SiteModel, + data: WpApiDetails, + resolver: ApiUrlResolver, + ) { + appPrefsWrapper.setSiteSupportsEditorSettings( + site, + data.hasRouteForEndpoint( + resolver, + "/wp-block-editor/v1", + "settings" + ) + ) + appPrefsWrapper.setSiteSupportsEditorAssets( + site, + data.hasRouteForEndpoint( + resolver, + "/wpcom/v2", + "editor-assets" + ) + ) + } + @Suppress("TooGenericExceptionCaught") private suspend fun fetchThemeBlockStyleSupport( site: SiteModel diff --git a/WordPress/src/test/java/org/wordpress/android/repositories/EditorSettingsRepositoryTest.kt b/WordPress/src/test/java/org/wordpress/android/repositories/EditorSettingsRepositoryTest.kt index 64cfbf825a6c..96082cf8df82 100644 --- a/WordPress/src/test/java/org/wordpress/android/repositories/EditorSettingsRepositoryTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/repositories/EditorSettingsRepositoryTest.kt @@ -15,10 +15,15 @@ import org.wordpress.android.BaseUnitTest import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider import org.wordpress.android.ui.prefs.AppPrefsWrapper +import rs.wordpress.api.kotlin.ApiDiscoveryResult import rs.wordpress.api.kotlin.WpApiClient +import rs.wordpress.api.kotlin.WpLoginClient import rs.wordpress.api.kotlin.WpRequestResult import uniffi.wp_api.ApiRootRequestGetResponse import uniffi.wp_api.ApiUrlResolver +import uniffi.wp_api.AutoDiscoveryAttemptSuccess +import uniffi.wp_api.DiscoveredAuthenticationMechanism +import uniffi.wp_api.ParseUrlException import uniffi.wp_api.ThemeAuthor import uniffi.wp_api.ThemeAuthorUri import uniffi.wp_api.ThemeDescription @@ -36,6 +41,9 @@ class EditorSettingsRepositoryTest : BaseUnitTest() { @Mock lateinit var wpApiClientProvider: WpApiClientProvider + @Mock + lateinit var wpLoginClient: WpLoginClient + @Mock lateinit var appPrefsWrapper: AppPrefsWrapper @@ -48,6 +56,9 @@ class EditorSettingsRepositoryTest : BaseUnitTest() { @Mock lateinit var apiUrlResolver: ApiUrlResolver + @Mock + lateinit var directHostResolver: ApiUrlResolver + private lateinit var repository: EditorSettingsRepository private val testSite = SiteModel().apply { @@ -64,6 +75,7 @@ class EditorSettingsRepositoryTest : BaseUnitTest() { repository = EditorSettingsRepository( wpApiClientProvider = wpApiClientProvider, + wpLoginClient = wpLoginClient, appPrefsWrapper = appPrefsWrapper, themeRepository = themeRepository, ioDispatcher = testDispatcher() @@ -186,22 +198,155 @@ class EditorSettingsRepositoryTest : BaseUnitTest() { .setSiteThemeIsBlockTheme(any(), any()) } - @Suppress("UNCHECKED_CAST") + @Test + fun `atomic site probes via api discovery`() = + runTest { + val atomicSite = SiteModel().apply { + id = 2 + url = "https://atomic.example.com" + setIsWPCom(true) + setIsWPComAtomic(true) + } + mockDiscoverySuccess( + siteUrl = atomicSite.url, + hasEditorSettings = false, + hasEditorAssets = false + ) + whenever(themeRepository.fetchCurrentTheme(atomicSite)) + .thenReturn(buildTheme(isBlockTheme = false)) + + val result = + repository.fetchEditorCapabilitiesForSite(atomicSite) + + assertThat(result).isTrue() + verify(appPrefsWrapper) + .setSiteSupportsEditorSettings(atomicSite, false) + verify(appPrefsWrapper) + .setSiteSupportsEditorAssets(atomicSite, false) + verify(wpApiClientProvider, never()).getWpApiClient(atomicSite) + } + + @Test + fun `atomic site returns false when discovery fails`() = + runTest { + val atomicSite = SiteModel().apply { + id = 4 + url = "https://atomic.example.com" + setIsWPCom(true) + setIsWPComAtomic(true) + } + whenever(wpLoginClient.apiDiscovery(atomicSite.url)) + .thenReturn( + ApiDiscoveryResult.FailureParseSiteUrl( + ParseUrlException.Generic("") + ) + ) + whenever(themeRepository.fetchCurrentTheme(atomicSite)) + .thenReturn(buildTheme(isBlockTheme = false)) + + val result = + repository.fetchEditorCapabilitiesForSite(atomicSite) + + assertThat(result).isFalse() + verify(appPrefsWrapper, never()) + .setSiteSupportsEditorSettings(any(), any()) + verify(appPrefsWrapper, never()) + .setSiteSupportsEditorAssets(any(), any()) + verify(wpApiClientProvider, never()).getWpApiClient(atomicSite) + } + + @Test + fun `atomic site with app password also probes via api discovery`() = + runTest { + val atomicSite = SiteModel().apply { + id = 3 + url = "https://atomic.example.com" + setIsWPCom(true) + setIsWPComAtomic(true) + apiRestUsernamePlain = "user" + apiRestPasswordPlain = "secret" + } + mockDiscoverySuccess( + siteUrl = atomicSite.url, + hasEditorSettings = true, + hasEditorAssets = true + ) + whenever(themeRepository.fetchCurrentTheme(atomicSite)) + .thenReturn(buildTheme(isBlockTheme = false)) + + val result = + repository.fetchEditorCapabilitiesForSite(atomicSite) + + assertThat(result).isTrue() + verify(appPrefsWrapper) + .setSiteSupportsEditorSettings(atomicSite, true) + verify(appPrefsWrapper) + .setSiteSupportsEditorAssets(atomicSite, true) + verify(wpApiClientProvider, never()).getWpApiClient(atomicSite) + } + private suspend fun mockApiRootResponse( hasEditorSettings: Boolean, hasEditorAssets: Boolean + ) = mockApiRootResponseFor( + client = wpApiClient, + resolver = apiUrlResolver, + hasEditorSettings = hasEditorSettings, + hasEditorAssets = hasEditorAssets, + ) + + private suspend fun mockDiscoverySuccess( + siteUrl: String, + hasEditorSettings: Boolean, + hasEditorAssets: Boolean, ) { val apiDetails = mock() whenever( apiDetails.hasRouteForEndpoint( - apiUrlResolver, + directHostResolver, "/wp-block-editor/v1", "settings" ) ).thenReturn(hasEditorSettings) whenever( apiDetails.hasRouteForEndpoint( - apiUrlResolver, + directHostResolver, + "/wpcom/v2", + "editor-assets" + ) + ).thenReturn(hasEditorAssets) + val apiRootUrl = mock() + whenever(wpApiClientProvider.urlResolverFor(apiRootUrl)) + .thenReturn(directHostResolver) + val success = AutoDiscoveryAttemptSuccess( + parsedSiteUrl = mock(), + apiRootUrl = apiRootUrl, + apiDetails = apiDetails, + authentication = DiscoveredAuthenticationMechanism + .ApplicationPasswords(mock()), + ) + whenever(wpLoginClient.apiDiscovery(siteUrl)) + .thenReturn(ApiDiscoveryResult.Success(success)) + } + + @Suppress("UNCHECKED_CAST") + private suspend fun mockApiRootResponseFor( + client: WpApiClient, + resolver: ApiUrlResolver, + hasEditorSettings: Boolean, + hasEditorAssets: Boolean + ) { + val apiDetails = mock() + whenever( + apiDetails.hasRouteForEndpoint( + resolver, + "/wp-block-editor/v1", + "settings" + ) + ).thenReturn(hasEditorSettings) + whenever( + apiDetails.hasRouteForEndpoint( + resolver, "/wpcom/v2", "editor-assets" ) @@ -211,7 +356,7 @@ class EditorSettingsRepositoryTest : BaseUnitTest() { data = apiDetails, headerMap = mock() ) - whenever(wpApiClient.request(any())) + whenever(client.request(any())) .thenReturn( WpRequestResult.Success(response) as WpRequestResult diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/rs/WpApiClientProvider.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/rs/WpApiClientProvider.kt index f0598b43b748..6e0d1980f57b 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/rs/WpApiClientProvider.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/rs/WpApiClientProvider.kt @@ -193,6 +193,16 @@ class WpApiClientProvider @Inject constructor( ) } + /** + * Builds a [WpOrgSiteApiUrlResolver] for an already-parsed REST API root + * URL (e.g. one returned by `WpLoginClient.apiDiscovery`). Exposed so + * callers don't have to construct the uniffi resolver directly — useful + * for testability. + */ + fun urlResolverFor( + apiRootUrl: ParsedUrl + ): uniffi.wp_api.ApiUrlResolver = WpOrgSiteApiUrlResolver(apiRootUrl) + fun getApiRootUrlFrom(site: SiteModel): String = site.buildUrl() private fun SiteModel.buildUrl(): String =