This repository has been archived by the owner on Jun 17, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 329
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Bug 1856729 - Switch to new app-services FxaClient
Use the new functionality from app-services to replace code in
service-firefox-accounts
General:
- Updated code to use FxaConfig class rather than Config. Config
wrapped FxaConfig, but provided little value.
- Updated `handleFxaExceptions` and removed all other Utils functions
- Use FxaClient's CoroutineContext to run all fxa-related jobs.
- Many FxaClient methods no longer directly return a result, but instead
input an optional `CompletableDeferred<T>` that they complete when the
operation is finish. See the app-services PR for details.
FirefoxAccount:
- Don't wrap FxaClient calls in withContext, this is handled in
app-services now.
- Updated FirefoxAccount docs to describe its relationship to
OAuthAccount.
- Make AccountStorage return a FirefoxAccount directly instead of an
OAuthAccount
FxaDeviceConstellation:
- Let FxaClient handle auth errors when sending device commands
- Updating FxaDeviceSettingsCache is now handled in the
FxaAccountManager.onFxaEvent method
FxaAccountManager:
- Replaced state machine code with the new `queueAction()` +
`onFxaEvent` functions instead.
Tests:
- Removed a good deal of tests, the app-services tests should replace
these.
- There were several tests that dealt with state persistence plus
oauth. I think these should be convered by the regular state change
tests, were they testing something else?
- Auth recovery is handled by several tests starting with
`FxaActionProcessor calls checkAuthorizationStatus after auth
errors`
- The main tests I tried to keep were the ones focused on observer
notification and other integrations with the larger code base.
We're mostly just forwarding the events that FxaClient sends us to
handle this.- Loading branch information
Showing
29 changed files
with
945 additions
and
2,990 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,29 +9,31 @@ import android.os.Looper.getMainLooper | |
| import androidx.test.ext.junit.runners.AndroidJUnit4 | ||
| import kotlinx.coroutines.CompletableDeferred | ||
| import kotlinx.coroutines.test.runTest | ||
| import mozilla.appservices.fxaclient.FxaAction | ||
| import mozilla.appservices.fxaclient.FxaServer | ||
| import mozilla.components.concept.engine.request.RequestInterceptor | ||
| import mozilla.components.concept.sync.AccountEventsObserver | ||
| import mozilla.components.concept.sync.AuthFlowUrl | ||
| import mozilla.components.concept.sync.AuthType | ||
| import mozilla.components.concept.sync.DeviceConfig | ||
| import mozilla.components.concept.sync.DeviceType | ||
| import mozilla.components.concept.sync.OAuthAccount | ||
| import mozilla.components.concept.sync.FxAEntryPoint | ||
| import mozilla.components.concept.sync.Profile | ||
| import mozilla.components.service.fxa.FirefoxAccount | ||
| import mozilla.components.service.fxa.FxaAuthData | ||
| import mozilla.components.service.fxa.Server | ||
| import mozilla.components.service.fxa.ServerConfig | ||
| import mozilla.components.service.fxa.StorageWrapper | ||
| import mozilla.components.service.fxa.manager.FxaAccountManager | ||
| import mozilla.components.support.base.observer.ObserverRegistry | ||
| import mozilla.components.support.test.any | ||
| import mozilla.components.support.test.argumentCaptor | ||
| import mozilla.components.support.test.mock | ||
| import mozilla.components.support.test.robolectric.testContext | ||
| import mozilla.components.support.test.whenever | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertNull | ||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
| import org.mockito.ArgumentMatchers.anyBoolean | ||
| import org.mockito.ArgumentMatchers.anyString | ||
| import org.mockito.Mockito.never | ||
| import org.mockito.Mockito.verify | ||
| import org.mockito.Mockito.`when` | ||
|
|
@@ -43,13 +45,9 @@ internal class TestableStorageWrapper( | |
| manager: FxaAccountManager, | ||
| accountEventObserverRegistry: ObserverRegistry<AccountEventsObserver>, | ||
| serverConfig: ServerConfig, | ||
| private val block: () -> OAuthAccount = { | ||
| val account: OAuthAccount = mock() | ||
| `when`(account.deviceConstellation()).thenReturn(mock()) | ||
| account | ||
| }, | ||
| val mockAccount: FirefoxAccount, | ||
| ) : StorageWrapper(manager, accountEventObserverRegistry, serverConfig) { | ||
| override fun obtainAccount(): OAuthAccount = block() | ||
| override fun obtainAccount(): FirefoxAccount = mockAccount | ||
| } | ||
|
|
||
| // Same as the actual account manager, except we get to control how FirefoxAccountShaped instances | ||
|
|
@@ -61,14 +59,20 @@ class TestableFxaAccountManager( | |
| config: ServerConfig, | ||
| scopes: Set<String>, | ||
| coroutineContext: CoroutineContext, | ||
| block: () -> OAuthAccount = { mock() }, | ||
| ) : FxaAccountManager(context, config, DeviceConfig("test", DeviceType.MOBILE, setOf()), null, scopes, null, coroutineContext) { | ||
| private val testableStorageWrapper = TestableStorageWrapper(this, accountEventObserverRegistry, serverConfig, block) | ||
| ) : FxaAccountManager(context, config, DeviceConfig("test", DeviceType.MOBILE, setOf()), null, scopes, null) { | ||
| val mockAccount: FirefoxAccount = mock<FirefoxAccount>().also { | ||
| whenever(it.clientCoroutineContext).thenReturn(coroutineContext) | ||
| } | ||
| private val testableStorageWrapper = TestableStorageWrapper(this, accountEventObserverRegistry, serverConfig, mockAccount) | ||
| override fun getStorageWrapper(): StorageWrapper { | ||
| return testableStorageWrapper | ||
| } | ||
| } | ||
|
|
||
| val entryPoint: FxAEntryPoint = mock<FxAEntryPoint>().apply { | ||
| whenever(entryName).thenReturn("home-menu") | ||
| } | ||
|
|
||
| @RunWith(AndroidJUnit4::class) | ||
| class FirefoxAccountsAuthFeatureTest { | ||
| // Note that tests that involve secure storage specify API=21, because of issues testing secure storage on | ||
|
|
@@ -88,9 +92,9 @@ class FirefoxAccountsAuthFeatureTest { | |
| ) { _, url -> | ||
| authUrl.complete(url) | ||
| } | ||
| feature.beginAuthentication(testContext, mock()) | ||
| feature.beginAuthentication(testContext, entryPoint) | ||
| authUrl.await() | ||
| assertEquals("auth://url", authUrl.getCompleted()) | ||
| assertEquals("auth://url?state=test-state", authUrl.getCompleted()) | ||
| } | ||
|
|
||
| @Config(sdk = [22]) | ||
|
|
@@ -107,9 +111,9 @@ class FirefoxAccountsAuthFeatureTest { | |
| ) { _, url -> | ||
| authUrl.complete(url) | ||
| } | ||
| feature.beginPairingAuthentication(testContext, "auth://pair", mock()) | ||
| feature.beginPairingAuthentication(testContext, "auth://pair", entryPoint) | ||
| authUrl.await() | ||
| assertEquals("auth://url", authUrl.getCompleted()) | ||
| assertEquals("auth://url?state=test-state", authUrl.getCompleted()) | ||
| } | ||
|
|
||
| @Config(sdk = [22]) | ||
|
|
@@ -127,7 +131,7 @@ class FirefoxAccountsAuthFeatureTest { | |
| ) { _, url -> | ||
| authUrl.complete(url) | ||
| } | ||
| feature.beginAuthentication(testContext, mock()) | ||
| feature.beginAuthentication(testContext, entryPoint) | ||
| authUrl.await() | ||
| // Fallback url is invoked. | ||
| assertEquals("https://accounts.firefox.com/signin", authUrl.getCompleted()) | ||
|
|
@@ -148,7 +152,7 @@ class FirefoxAccountsAuthFeatureTest { | |
| ) { _, url -> | ||
| authUrl.complete(url) | ||
| } | ||
| feature.beginPairingAuthentication(testContext, "auth://pair", mock()) | ||
| feature.beginPairingAuthentication(testContext, "auth://pair", entryPoint) | ||
| authUrl.await() | ||
| // Fallback url is invoked. | ||
| assertEquals("https://accounts.firefox.com/signin", authUrl.getCompleted()) | ||
|
|
@@ -246,24 +250,26 @@ class FirefoxAccountsAuthFeatureTest { | |
| private suspend fun prepareAccountManagerForSuccessfulAuthentication( | ||
| coroutineContext: CoroutineContext, | ||
| ): TestableFxaAccountManager { | ||
| val mockAccount: OAuthAccount = mock() | ||
| val profile = Profile(uid = "testUID", avatar = null, email = "[email protected]", displayName = "test profile") | ||
|
|
||
| `when`(mockAccount.deviceConstellation()).thenReturn(mock()) | ||
| `when`(mockAccount.getProfile(anyBoolean())).thenReturn(profile) | ||
| `when`(mockAccount.beginOAuthFlow(any(), any())).thenReturn(AuthFlowUrl("authState", "auth://url")) | ||
| `when`(mockAccount.beginPairingFlow(anyString(), any(), any())).thenReturn(AuthFlowUrl("authState", "auth://url")) | ||
| `when`(mockAccount.completeOAuthFlow(anyString(), anyString())).thenReturn(true) | ||
|
|
||
| val manager = TestableFxaAccountManager( | ||
| testContext, | ||
| ServerConfig(Server.RELEASE, "dummyId", "bad://url"), | ||
| ServerConfig(FxaServer.Release, "dummyId", "bad://url", null), | ||
| setOf("test-scope"), | ||
| coroutineContext, | ||
| ) { | ||
| mockAccount | ||
| ) | ||
| manager.mockAccount.let { | ||
| `when`(it.deviceConstellation()).thenReturn(mock()) | ||
| `when`(it.getProfile(anyBoolean())).thenReturn(profile) | ||
| val captor = argumentCaptor<FxaAction>() | ||
| `when`(it.queueAction(captor.capture())).thenAnswer { | ||
| val action = captor.value | ||
| when (action) { | ||
| is FxaAction.BeginOAuthFlow -> action.result?.complete("auth://url?state=test-state") | ||
| is FxaAction.BeginPairingFlow -> action.result?.complete("auth://url?state=test-state") | ||
| else -> Unit | ||
| } | ||
| } | ||
| } | ||
|
|
||
| manager.start() | ||
|
|
||
| return manager | ||
|
|
@@ -273,24 +279,26 @@ class FirefoxAccountsAuthFeatureTest { | |
| private suspend fun prepareAccountManagerForFailedAuthentication( | ||
| coroutineContext: CoroutineContext, | ||
| ): TestableFxaAccountManager { | ||
| val mockAccount: OAuthAccount = mock() | ||
| val profile = Profile(uid = "testUID", avatar = null, email = "[email protected]", displayName = "test profile") | ||
|
|
||
| `when`(mockAccount.getProfile(anyBoolean())).thenReturn(profile) | ||
| `when`(mockAccount.deviceConstellation()).thenReturn(mock()) | ||
| `when`(mockAccount.beginOAuthFlow(any(), any())).thenReturn(null) | ||
| `when`(mockAccount.beginPairingFlow(anyString(), any(), any())).thenReturn(null) | ||
| `when`(mockAccount.completeOAuthFlow(anyString(), anyString())).thenReturn(true) | ||
|
|
||
| val manager = TestableFxaAccountManager( | ||
| testContext, | ||
| ServerConfig(Server.RELEASE, "dummyId", "bad://url"), | ||
| ServerConfig(FxaServer.Release, "dummyId", "bad://url", null), | ||
| setOf("test-scope"), | ||
| coroutineContext, | ||
| ) { | ||
| mockAccount | ||
| ) | ||
| manager.mockAccount.let { | ||
| `when`(it.deviceConstellation()).thenReturn(mock()) | ||
| `when`(it.getProfile(anyBoolean())).thenReturn(profile) | ||
| val captor = argumentCaptor<FxaAction>() | ||
| `when`(it.queueAction(captor.capture())).thenAnswer { | ||
| val action = captor.value | ||
| when (action) { | ||
| is FxaAction.BeginOAuthFlow -> action.result?.complete(null) | ||
| is FxaAction.BeginPairingFlow -> action.result?.complete(null) | ||
| else -> Unit | ||
| } | ||
| } | ||
| } | ||
|
|
||
| manager.start() | ||
|
|
||
| return manager | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
87bd22eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh! Looks like an error! Details
Failed to fetch task artifact
public/github/customCheckRunText.mdfor GitHub integration.Make sure the artifact exists on the worker or other location.