From f992e4e559a276de1dc9194f4c7bdc30cdfe9f16 Mon Sep 17 00:00:00 2001 From: ohall-m Date: Fri, 9 Feb 2024 13:27:56 -0500 Subject: [PATCH] Bug 1877278 - AC Translations Check for if the Engine is Supported This patch supports the workflow for checking if the device architecture supports translations. This patch adds: * New `isEngineSupported` on `TranslationsState` * New `FETCH_IS_ENGINE_SUPPORTED` to request fetching and setting the value * New `SetEngineSupportAction` to set the `isEngineSupported` value * `TabListAction.AddTabAction`, `TabListAction.RestoreAction`, and `TabListAction.SelectTabAction` to eagerly request `FETCH_IS_ENGINE_SUPPORTED` --- .../browser/state/action/BrowserAction.kt | 11 ++ .../middleware/TranslationsMiddleware.kt | 93 +++++++++++++ .../state/reducer/TranslationsStateReducer.kt | 31 ++++- .../browser/state/state/TranslationsState.kt | 2 + .../state/action/TranslationsActionTest.kt | 70 ++++++++++ .../middleware/TranslationsMiddlewareTest.kt | 128 ++++++++++++++++++ .../engine/translate/TranslationError.kt | 8 ++ .../engine/translate/TranslationOperation.kt | 5 + .../telemetry/TelemetryMiddlewareTest.kt | 2 + 9 files changed, 349 insertions(+), 1 deletion(-) diff --git a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt index 53b72ac3b5ff..b9ca723137f7 100644 --- a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt +++ b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt @@ -948,6 +948,17 @@ sealed class TranslationsAction : BrowserAction() { val operation: TranslationOperation, ) : TranslationsAction(), ActionWithTab + /** + * Sets whether the device architecture supports translations or not. + * + * @property tabId The ID of the tab the [EngineSession] that requested the list. + * @property isEngineSupported If the engine supports translations on this device. + */ + data class SetEngineSupportAction( + override val tabId: String, + val isEngineSupported: Boolean, + ) : TranslationsAction(), ActionWithTab + /** * Sets the languages that are supported by the translations engine. * diff --git a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt index ab984747ef92..e4fe17de5bba 100644 --- a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt +++ b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt @@ -7,9 +7,11 @@ package mozilla.components.browser.state.engine.middleware import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import mozilla.components.browser.state.action.BrowserAction +import mozilla.components.browser.state.action.TabListAction import mozilla.components.browser.state.action.TranslationsAction import mozilla.components.browser.state.selector.findTab import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.TranslationsState import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.translate.LanguageSetting @@ -39,8 +41,52 @@ class TranslationsMiddleware( ) { // Pre process actions when (action) { + // Initially adding the tab + is TabListAction.AddTabAction -> { + if (translationsState(context, action.tab.id)?.isEngineSupported == null) { + context.store.dispatch( + TranslationsAction.OperationRequestedAction( + tabId = action.tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ), + ) + } + } + + // Restoring a tab from a closed app + is TabListAction.RestoreAction -> { + if (translationsState(context, action.selectedTabId)?.isEngineSupported == null) { + action.selectedTabId?.let { id -> + context.store.dispatch( + TranslationsAction.OperationRequestedAction( + tabId = id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ), + ) + } + } + } + + // Switching to a new tab + is TabListAction.SelectTabAction -> { + if (translationsState(context, action.tabId)?.isEngineSupported == null) { + context.store.dispatch( + TranslationsAction.OperationRequestedAction( + tabId = action.tabId, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ), + ) + } + } + is TranslationsAction.OperationRequestedAction -> { when (action.operation) { + TranslationOperation.FETCH_IS_ENGINE_SUPPORTED -> { + scope.launch { + requestEngineSupport(context, action.tabId) + } + } + TranslationOperation.FETCH_SUPPORTED_LANGUAGES -> { scope.launch { requestSupportedLanguages(context, action.tabId) @@ -70,6 +116,53 @@ class TranslationsMiddleware( next(action) } + /** + * Convenience method to get the translations state from the store. + * + * @param context Context to use to find the state of the store. + * @param tabId Tab ID associated with the request. + */ + private fun translationsState( + context: MiddlewareContext, + tabId: String?, + ): TranslationsState? { + return tabId?.let { context.store.state.findTab(it)?.translationsState } + } + + /** + * Checks if the translations engine supports the device architecture. + * + * @param context Context to use to dispatch to the store. + * @param tabId Tab ID associated with the request. + */ + private fun requestEngineSupport( + context: MiddlewareContext, + tabId: String, + ) { + engine.isTranslationsEngineSupported( + onSuccess = { isEngineSupported -> + context.store.dispatch( + TranslationsAction.SetEngineSupportAction( + tabId = tabId, + isEngineSupported = isEngineSupported, + ), + ) + logger.info("Success requesting engine support.") + }, + + onError = { error -> + context.store.dispatch( + TranslationsAction.TranslateExceptionAction( + tabId = tabId, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + translationError = TranslationError.UnknownEngineSupportError(error), + ), + ) + logger.error("Error requesting engine support: ", error) + }, + ) + } + /** * Retrieves the list of supported languages using [scope] and dispatches the result to the * store via [TranslationsAction.TranslateSetLanguagesAction] or else dispatches the failure diff --git a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TranslationsStateReducer.kt b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TranslationsStateReducer.kt index 154ae8f4a99b..54e56d51dda3 100644 --- a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TranslationsStateReducer.kt +++ b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TranslationsStateReducer.kt @@ -83,6 +83,15 @@ internal object TranslationsStateReducer { } } + TranslationOperation.FETCH_IS_ENGINE_SUPPORTED -> { + state.copyWithTranslationsState(action.tabId) { + it.copy( + isEngineSupported = true, + translationError = null, + ) + } + } + TranslationOperation.FETCH_SUPPORTED_LANGUAGES -> { // Reset the error state, and then generally expect // [TranslationsAction.SetSupportedLanguagesAction] to update state in the @@ -138,6 +147,15 @@ internal object TranslationsStateReducer { } } + TranslationOperation.FETCH_IS_ENGINE_SUPPORTED -> + state.copyWithTranslationsState(action.tabId) { + it.copy( + // An error here doesn't necessarily imply the device doesn't support + // the engine, so not changing [isEngineSupported] state. + translationError = action.translationError, + ) + } + TranslationOperation.FETCH_SUPPORTED_LANGUAGES -> { state.copyWithTranslationsState(action.tabId) { it.copy( @@ -167,6 +185,14 @@ internal object TranslationsStateReducer { } } + is TranslationsAction.SetEngineSupportAction -> + state.copyWithTranslationsState(action.tabId) { + it.copy( + isEngineSupported = action.isEngineSupported, + translationError = null, + ) + } + is TranslationsAction.SetSupportedLanguagesAction -> state.copyWithTranslationsState(action.tabId) { it.copy( @@ -213,7 +239,10 @@ internal object TranslationsStateReducer { ) } } - TranslationOperation.TRANSLATE, TranslationOperation.RESTORE -> { + TranslationOperation.TRANSLATE, + TranslationOperation.RESTORE, + TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + -> { // No state change for these operations state } diff --git a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/state/TranslationsState.kt b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/state/TranslationsState.kt index 3bc7b288da0a..1ed7c02227b5 100644 --- a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/state/TranslationsState.kt +++ b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/state/TranslationsState.kt @@ -19,6 +19,7 @@ import mozilla.components.concept.engine.translate.TranslationSupport * @property isTranslated The page is currently translated. * @property isTranslateProcessing The page is currently attempting a translation. * @property isRestoreProcessing The page is currently attempting a restoration. + * @property isEngineSupported The translations engine supports the device architecture. * @property supportedLanguages Set of languages the translation engine supports. * @property pageSettings The translation engine settings that relate to the current page. * @property neverTranslateSites List of sites the user has opted to never translate. @@ -33,6 +34,7 @@ data class TranslationsState( val isTranslated: Boolean = false, val isTranslateProcessing: Boolean = false, val isRestoreProcessing: Boolean = false, + val isEngineSupported: Boolean? = null, val supportedLanguages: TranslationSupport? = null, val pageSettings: TranslationPageSettings? = null, val neverTranslateSites: List? = null, diff --git a/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TranslationsActionTest.kt b/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TranslationsActionTest.kt index 0eed569f0604..db93776a8555 100644 --- a/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TranslationsActionTest.kt +++ b/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TranslationsActionTest.kt @@ -387,4 +387,74 @@ class TranslationsActionTest { // Action success assertNull(tabState().translationsState.supportedLanguages) } + + @Test + fun `WHEN a TranslateSetEngineSupportAction is dispatched THEN update the store to match`() { + // Initial state + assertNull(tabState().translationsState.isEngineSupported) + + // Dispatch + store.dispatch( + TranslationsAction.SetEngineSupportAction( + tabId = tab.id, + isEngineSupported = true, + ), + ).joinBlocking() + + // Final state + assertTrue(tabState().translationsState.isEngineSupported!!) + } + + @Test + fun `WHEN a OperationRequestedAction is dispatched for FETCH_IS_ENGINE_SUPPORTED THEN do NOT clear isEngineSupported`() { + // Setting first to have a more robust initial state + assertNull(tabState().translationsState.isEngineSupported) + + store.dispatch( + TranslationsAction.SetEngineSupportAction( + tabId = tab.id, + isEngineSupported = true, + ), + ).joinBlocking() + + assertTrue(tabState().translationsState.isEngineSupported!!) + + // Action started + store.dispatch( + TranslationsAction.OperationRequestedAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ), + ).joinBlocking() + + // Action success + assertTrue(tabState().translationsState.isEngineSupported!!) + } + + @Test + fun `WHEN a TranslateExceptionAction is dispatched for FETCH_IS_ENGINE_SUPPORTED THEN do NOT clear or alter isEngineSupported`() { + // Setting first to have a more robust initial state + assertNull(tabState().translationsState.isEngineSupported) + + store.dispatch( + TranslationsAction.SetEngineSupportAction( + tabId = tab.id, + isEngineSupported = true, + ), + ).joinBlocking() + + assertTrue(tabState().translationsState.isEngineSupported!!) + + // Action started + store.dispatch( + TranslationsAction.TranslateExceptionAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + translationError = TranslationError.UnknownEngineSupportError(null), + ), + ).joinBlocking() + + // Action success + assertTrue(tabState().translationsState.isEngineSupported!!) + } } diff --git a/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt b/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt index 237d69a5a85a..0065a08eb269 100644 --- a/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt +++ b/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt @@ -6,6 +6,7 @@ package mozilla.components.browser.state.engine.middleware import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.action.BrowserAction +import mozilla.components.browser.state.action.TabListAction import mozilla.components.browser.state.action.TranslationsAction import mozilla.components.browser.state.selector.findTab import mozilla.components.browser.state.state.BrowserState @@ -34,6 +35,7 @@ import mozilla.components.support.test.whenever import org.junit.Before import org.junit.Rule import org.junit.Test +import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.verify @@ -281,4 +283,130 @@ class TranslationsMiddlewareTest { ) waitForIdle() } + + @Test + fun `WHEN OperationRequestedAction FETCH_IS_ENGINE_SUPPORTED is dispatched THEN SetEngineSupportAction is dispatched`() = runTest { + // Send Action + val action = TranslationsAction.OperationRequestedAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ) + translationsMiddleware.invoke(context, {}, action) + waitForIdle() + + // Check expectations + val engineSupportedCallback = argumentCaptor<((Boolean) -> Unit)>() + verify(engine).isTranslationsEngineSupported(onSuccess = engineSupportedCallback.capture(), onError = any()) + engineSupportedCallback.value.invoke(true) + waitForIdle() + + verify(store).dispatch( + TranslationsAction.SetEngineSupportAction( + tabId = tab.id, + isEngineSupported = true, + ), + ) + waitForIdle() + } + + @Test + fun `WHEN OperationRequestedAction FETCH_IS_ENGINE_SUPPORTED is dispatched AND has an issue THEN TranslateExceptionAction is dispatched`() = runTest() { + // Send Action + val action = TranslationsAction.OperationRequestedAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ) + translationsMiddleware.invoke(context, {}, action) + + waitForIdle() + + // Check expectations + val errorCallback = argumentCaptor<((Throwable) -> Unit)>() + verify(engine).isTranslationsEngineSupported( + onSuccess = any(), + onError = errorCallback.capture(), + ) + errorCallback.value.invoke(IllegalStateException()) + waitForIdle() + + verify(store).dispatch( + TranslationsAction.TranslateExceptionAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + translationError = TranslationError.UnknownEngineSupportError(any()), + ), + ) + } + + @Test + fun `WHEN AddTabAction is dispatched THEN OperationRequestedAction FETCH_IS_ENGINE_SUPPORTED is dispatched`() = runTest { + val action = TabListAction.AddTabAction(tab = tab) + translationsMiddleware.invoke(context, {}, action) + + waitForIdle() + + verify(store).dispatch( + TranslationsAction.OperationRequestedAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ), + ) + waitForIdle() + } + + @Test + fun `WHEN RestoreAction is dispatched WITH a tab id THEN OperationRequestedAction FETCH_IS_ENGINE_SUPPORTED is dispatched`() = runTest { + val action = TabListAction.RestoreAction( + tabs = mock(), + selectedTabId = tab.id, + restoreLocation = TabListAction.RestoreAction.RestoreLocation.AT_INDEX, + ) + translationsMiddleware.invoke(context, {}, action) + + waitForIdle() + + verify(store).dispatch( + TranslationsAction.OperationRequestedAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ), + ) + waitForIdle() + } + + @Test + fun `WHEN RestoreAction is dispatched AND there is no selected tab id THEN OperationRequestedAction FETCH_IS_ENGINE_SUPPORTED is NOT dispatched`() = runTest { + val action = TabListAction.RestoreAction( + tabs = mock(), + selectedTabId = null, + restoreLocation = TabListAction.RestoreAction.RestoreLocation.AT_INDEX, + ) + translationsMiddleware.invoke(context, {}, action) + + waitForIdle() + + verify(store, never()).dispatch( + TranslationsAction.OperationRequestedAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ), + ) + waitForIdle() + } + + @Test + fun `WHEN SelectTabAction is dispatched THEN OperationRequestedAction FETCH_IS_ENGINE_SUPPORTED is dispatched`() = runTest { + val action = TabListAction.SelectTabAction(tabId = tab.id) + translationsMiddleware.invoke(context, {}, action) + + waitForIdle() + + verify(store).dispatch( + TranslationsAction.OperationRequestedAction( + tabId = tab.id, + operation = TranslationOperation.FETCH_IS_ENGINE_SUPPORTED, + ), + ) + waitForIdle() + } } diff --git a/android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationError.kt b/android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationError.kt index 812ffe88f563..4ecca6b174e8 100644 --- a/android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationError.kt +++ b/android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationError.kt @@ -48,6 +48,14 @@ sealed class TranslationError( class EngineNotSupportedError(override val cause: Throwable?) : TranslationError(errorName = "engine-not-supported", displayError = false, cause = cause) + /** + * Could not determine if the translations engine works on the device architecture. + * + * @param cause The original throwable before it was converted into this error state. + */ + class UnknownEngineSupportError(override val cause: Throwable?) : + TranslationError(errorName = "unknown-engine-support", displayError = false, cause = cause) + /** * Generic could not compete a translation error. * diff --git a/android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationOperation.kt b/android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationOperation.kt index 7de131a7a8b0..72b31bd82edf 100644 --- a/android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationOperation.kt +++ b/android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationOperation.kt @@ -18,6 +18,11 @@ enum class TranslationOperation { */ RESTORE, + /** + * The device architecture supports translations. + */ + FETCH_IS_ENGINE_SUPPORTED, + /** * The list of languages that the translation engine should fetch. This includes * the languages supported for translating both "to" and "from" with their BCP-47 language tag diff --git a/fenix/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt index 4c8eace865f2..195a8096af74 100644 --- a/fenix/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt +++ b/fenix/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt @@ -86,6 +86,8 @@ class TelemetryMiddlewareTest { val engine: Engine = mockk() every { engine.enableExtensionProcessSpawning() } just runs every { engine.disableExtensionProcessSpawning() } just runs + every { engine.isTranslationsEngineSupported(any(), any()) } just runs + store = BrowserStore( middleware = listOf(telemetryMiddleware) + EngineMiddleware.create(engine), initialState = BrowserState(),