diff --git a/app/src/main/java/com/x8bit/bitwarden/BitwardenApplication.kt b/app/src/main/java/com/x8bit/bitwarden/BitwardenApplication.kt index 978a301dd..efca8263e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/BitwardenApplication.kt +++ b/app/src/main/java/com/x8bit/bitwarden/BitwardenApplication.kt @@ -7,7 +7,6 @@ import com.x8bit.bitwarden.data.platform.manager.CrashLogsManager import com.x8bit.bitwarden.data.platform.manager.NetworkConfigManager import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager import com.x8bit.bitwarden.data.platform.manager.restriction.RestrictionManager -import com.x8bit.bitwarden.data.platform.repository.ServerConfigRepository import dagger.hilt.android.HiltAndroidApp import javax.inject.Inject @@ -33,7 +32,4 @@ class BitwardenApplication : Application() { @Inject lateinit var restrictionManager: RestrictionManager - - @Inject - lateinit var serverConfigRepository: ServerConfigRepository } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/NetworkConfigManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/NetworkConfigManagerImpl.kt index 309a8a768..10e017b06 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/NetworkConfigManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/NetworkConfigManagerImpl.kt @@ -7,17 +7,23 @@ import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.AuthToke import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptors import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository +import com.x8bit.bitwarden.data.platform.repository.ServerConfigRepository import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +private const val ENVIRONMENT_DEBOUNCE_TIMEOUT_MS: Long = 500L + /** * Primary implementation of [NetworkConfigManager]. */ +@Suppress("LongParameterList") class NetworkConfigManagerImpl( authRepository: AuthRepository, private val authTokenInterceptor: AuthTokenInterceptor, environmentRepository: EnvironmentRepository, + serverConfigRepository: ServerConfigRepository, private val baseUrlInterceptors: BaseUrlInterceptors, refreshAuthenticator: RefreshAuthenticator, dispatcherManager: DispatcherManager, @@ -37,11 +43,18 @@ class NetworkConfigManagerImpl( } .launchIn(collectionScope) + @Suppress("OPT_IN_USAGE") environmentRepository .environmentStateFlow .onEach { environment -> baseUrlInterceptors.environment = environment } + .debounce(timeoutMillis = ENVIRONMENT_DEBOUNCE_TIMEOUT_MS) + .onEach { _ -> + // This updates the stored service configuration by performing a network request. + // We debounce it to avoid rapid repeated requests. + serverConfigRepository.getServerConfig(forceRefresh = true) + } .launchIn(collectionScope) refreshAuthenticator.authenticatorProvider = authRepository diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt index 3574b8184..5346ba18d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt @@ -161,6 +161,7 @@ object PlatformManagerModule { authRepository: AuthRepository, authTokenInterceptor: AuthTokenInterceptor, environmentRepository: EnvironmentRepository, + serverConfigRepository: ServerConfigRepository, baseUrlInterceptors: BaseUrlInterceptors, refreshAuthenticator: RefreshAuthenticator, dispatcherManager: DispatcherManager, @@ -169,6 +170,7 @@ object PlatformManagerModule { authRepository = authRepository, authTokenInterceptor = authTokenInterceptor, environmentRepository = environmentRepository, + serverConfigRepository = serverConfigRepository, baseUrlInterceptors = baseUrlInterceptors, refreshAuthenticator = refreshAuthenticator, dispatcherManager = dispatcherManager, diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepository.kt index 147bc8cb1..03aaae825 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepository.kt @@ -8,14 +8,14 @@ import kotlinx.coroutines.flow.StateFlow */ interface ServerConfigRepository { + /** + * Emits updates that track [ServerConfig]. + */ + val serverConfigStateFlow: StateFlow + /** * Gets the state [ServerConfig]. If needed or forced by [forceRefresh], * updates the values using server side data. */ suspend fun getServerConfig(forceRefresh: Boolean): ServerConfig? - - /** - * Emits updates that track [ServerConfig]. - */ - val serverConfigStateFlow: StateFlow } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepositoryImpl.kt index 661f3dd7a..8ece064ca 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepositoryImpl.kt @@ -7,8 +7,6 @@ import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import java.time.Clock import java.time.Instant @@ -20,20 +18,19 @@ class ServerConfigRepositoryImpl( private val configDiskSource: ConfigDiskSource, private val configService: ConfigService, private val clock: Clock, - environmentRepository: EnvironmentRepository, dispatcherManager: DispatcherManager, ) : ServerConfigRepository { private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) - init { - environmentRepository - .environmentStateFlow - .onEach { - getServerConfig(true) - } - .launchIn(unconfinedScope) - } + override val serverConfigStateFlow: StateFlow + get() = configDiskSource + .serverConfigFlow + .stateIn( + scope = unconfinedScope, + started = SharingStarted.Eagerly, + initialValue = configDiskSource.serverConfig, + ) override suspend fun getServerConfig(forceRefresh: Boolean): ServerConfig? { val localConfig = configDiskSource.serverConfig @@ -62,15 +59,6 @@ class ServerConfigRepositoryImpl( return localConfig } - override val serverConfigStateFlow: StateFlow - get() = configDiskSource - .serverConfigFlow - .stateIn( - scope = unconfinedScope, - started = SharingStarted.Eagerly, - initialValue = configDiskSource.serverConfig, - ) - companion object { private const val MINIMUM_CONFIG_SYNC_INTERVAL_SEC: Long = 60 * 60 } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/di/PlatformRepositoryModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/di/PlatformRepositoryModule.kt index e52f75c93..143689f26 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/di/PlatformRepositoryModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/di/PlatformRepositoryModule.kt @@ -37,14 +37,12 @@ object PlatformRepositoryModule { configDiskSource: ConfigDiskSource, configService: ConfigService, clock: Clock, - environmentRepository: EnvironmentRepository, dispatcherManager: DispatcherManager, ): ServerConfigRepository = ServerConfigRepositoryImpl( configDiskSource = configDiskSource, configService = configService, clock = clock, - environmentRepository = environmentRepository, dispatcherManager = dispatcherManager, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/NetworkConfigManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/NetworkConfigManagerTest.kt index 6a6efc018..60fd11e67 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/NetworkConfigManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/NetworkConfigManagerTest.kt @@ -8,28 +8,40 @@ import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.AuthToke import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptors import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository +import com.x8bit.bitwarden.data.platform.repository.ServerConfigRepository import com.x8bit.bitwarden.data.platform.repository.model.Environment +import com.x8bit.bitwarden.data.util.advanceTimeByAndRunCurrent +import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.UnconfinedTestDispatcher import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +@OptIn(ExperimentalCoroutinesApi::class) class NetworkConfigManagerTest { - private val dispatcherManager: DispatcherManager = FakeDispatcherManager() + private val testDispatcher = UnconfinedTestDispatcher() + private val dispatcherManager: DispatcherManager = FakeDispatcherManager( + unconfined = testDispatcher, + ) private val mutableAuthStateFlow = MutableStateFlow(AuthState.Uninitialized) private val mutableEnvironmentStateFlow = MutableStateFlow(Environment.Us) private val authRepository: AuthRepository = mockk { every { authStateFlow } returns mutableAuthStateFlow } - private val environmentRepository: EnvironmentRepository = mockk { every { environmentStateFlow } returns mutableEnvironmentStateFlow } + private val serverConfigRepository: ServerConfigRepository = mockk { + coEvery { getServerConfig(forceRefresh = true) } returns null + } private val refreshAuthenticator = RefreshAuthenticator() private val authTokenInterceptor = AuthTokenInterceptor() @@ -43,6 +55,7 @@ class NetworkConfigManagerTest { authRepository = authRepository, authTokenInterceptor = authTokenInterceptor, environmentRepository = environmentRepository, + serverConfigRepository = serverConfigRepository, baseUrlInterceptors = baseUrlInterceptors, refreshAuthenticator = refreshAuthenticator, dispatcherManager = dispatcherManager, @@ -85,5 +98,9 @@ class NetworkConfigManagerTest { Environment.Eu, baseUrlInterceptors.environment, ) + testDispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 500L) + coVerify(exactly = 1) { + serverConfigRepository.getServerConfig(forceRefresh = true) + } } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepositoryTest.kt index 364723dda..40062ea2b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/ServerConfigRepositoryTest.kt @@ -9,8 +9,6 @@ import com.x8bit.bitwarden.data.platform.datasource.network.model.ConfigResponse import com.x8bit.bitwarden.data.platform.datasource.network.model.ConfigResponseJson.ServerJson import com.x8bit.bitwarden.data.platform.datasource.network.service.ConfigService import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager -import com.x8bit.bitwarden.data.platform.repository.model.Environment -import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository import com.x8bit.bitwarden.data.platform.util.asSuccess import io.mockk.coEvery import io.mockk.mockk @@ -34,10 +32,6 @@ class ServerConfigRepositoryTest { } returns CONFIG_RESPONSE_JSON.asSuccess() } - private val environmentRepository = FakeEnvironmentRepository().apply { - environment = Environment.Us - } - private val fixedClock: Clock = Clock.fixed( Instant.parse("2023-10-27T12:00:00Z"), ZoneOffset.UTC, @@ -47,7 +41,6 @@ class ServerConfigRepositoryTest { configDiskSource = fakeConfigDiskSource, configService = configService, clock = fixedClock, - environmentRepository = environmentRepository, dispatcherManager = fakeDispatcherManager, ) @@ -56,23 +49,6 @@ class ServerConfigRepositoryTest { fakeConfigDiskSource.serverConfig = null } - @Test - fun `environmentRepository stateflow should trigger new server configuration`() = runTest { - assertNull( - fakeConfigDiskSource.serverConfig, - ) - - // This should trigger a new server config to be fetched - environmentRepository.environment = Environment.Eu - - repository.serverConfigStateFlow.test { - assertEquals( - SERVER_CONFIG, - awaitItem(), - ) - } - } - @Test fun `getServerConfig should fetch a new server configuration with force refresh as true`() = runTest {