PM-10835: Make config request after environment update (#3720)

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
 }

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

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,

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<ServerConfig?>
+
     /**
      * 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<ServerConfig?>
 }

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 {
+    override val serverConfigStateFlow: StateFlow<ServerConfig?>
-        environmentRepository
+        get() = configDiskSource
-            .environmentStateFlow
+            .serverConfigFlow
-            .onEach {
+            .stateIn(
-                getServerConfig(true)
+                scope = unconfinedScope,
-            }
+                started = SharingStarted.Eagerly,
-            .launchIn(unconfinedScope)
+                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<ServerConfig?>
-        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
     }

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,
         )
 

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>(AuthState.Uninitialized)
     private val mutableEnvironmentStateFlow = MutableStateFlow<Environment>(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)
+        }
     }
 }

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 {