Merge pull request #5565 from element-hq/bma/wellknownBrandColor

Improve code around Element .well-known configuration

features/enterprise/api/src/main/kotlin/io/element/android/features/enterprise/api/EnterpriseService.kt

@@ -21,9 +21,10 @@ interface EnterpriseService {
 
     /**
      * Override the brand color.
+     * @param sessionId the session to override the brand color for, or null to set the brand color to use when there is no session.
      * @param brandColor the color in hex format (#RRGGBBAA or #RRGGBB), or null to reset to default.
      */
-    fun overrideBrandColor(brandColor: String?)
+    suspend fun overrideBrandColor(sessionId: SessionId?, brandColor: String?)
 
     @Composable
     fun semanticColorsLight(): State<SemanticColors>

features/enterprise/impl-foss/src/main/kotlin/io/element/android/features/enterprise/impl/DefaultEnterpriseService.kt

@@ -32,7 +32,7 @@ class DefaultEnterpriseService : EnterpriseService {
     override fun defaultHomeserverList(): List<String> = emptyList()
     override suspend fun isAllowedToConnectToHomeserver(homeserverUrl: String) = true
 
-    override fun overrideBrandColor(brandColor: String?) = Unit
+    override suspend fun overrideBrandColor(sessionId: SessionId?, brandColor: String?) = Unit
 
     @Composable
     override fun semanticColorsLight(): State<SemanticColors> {

features/enterprise/impl-foss/src/test/kotlin/io/element/android/features/enterprise/impl/DefaultEnterpriseServiceTest.kt

@@ -51,7 +51,7 @@ class DefaultEnterpriseServiceTest {
         }.test {
             val initialState = awaitItem()
             assertThat(initialState).isEqualTo(compoundColorsLight)
-            defaultEnterpriseService.overrideBrandColor("#87654321")
+            defaultEnterpriseService.overrideBrandColor(A_SESSION_ID, "#87654321")
             expectNoEvents()
         }
     }
@@ -64,7 +64,7 @@ class DefaultEnterpriseServiceTest {
         }.test {
             val initialState = awaitItem()
             assertThat(initialState).isEqualTo(compoundColorsDark)
-            defaultEnterpriseService.overrideBrandColor("#87654321")
+            defaultEnterpriseService.overrideBrandColor(A_SESSION_ID, "#87654321")
             expectNoEvents()
         }
     }

features/enterprise/test/src/main/kotlin/io/element/android/features/enterprise/test/FakeEnterpriseService.kt

@@ -26,7 +26,7 @@ class FakeEnterpriseService(
     private val isAllowedToConnectToHomeserverResult: (String) -> Boolean = { lambdaError() },
     private val semanticColorsLightResult: () -> State<SemanticColors> = { lambdaError() },
     private val semanticColorsDarkResult: () -> State<SemanticColors> = { lambdaError() },
-    private val overrideBrandColorResult: (String?) -> Unit = { lambdaError() },
+    private val overrideBrandColorResult: (SessionId?, String?) -> Unit = { _, _ -> lambdaError() },
     private val firebasePushGatewayResult: () -> String? = { lambdaError() },
     private val unifiedPushDefaultPushGatewayResult: () -> String? = { lambdaError() },
 ) : EnterpriseService {
@@ -42,8 +42,8 @@ class FakeEnterpriseService(
         isAllowedToConnectToHomeserverResult(homeserverUrl)
     }
 
-    override fun overrideBrandColor(brandColor: String?) {
-        overrideBrandColorResult(brandColor)
+    override suspend fun overrideBrandColor(sessionId: SessionId?, brandColor: String?) = simulateLongTask {
+        overrideBrandColorResult(sessionId, brandColor)
     }
 
     @Composable

features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt

@@ -38,6 +38,7 @@ import io.element.android.libraries.core.meta.BuildType
 import io.element.android.libraries.featureflag.api.FeatureFlagService
 import io.element.android.libraries.featureflag.api.FeatureFlags
 import io.element.android.libraries.featureflag.ui.model.FeatureUiModel
+import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.preferences.api.store.AppPreferencesStore
 import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.persistentListOf
@@ -50,6 +51,7 @@ import java.net.URL
 
 @Inject
 class DeveloperSettingsPresenter(
+    private val sessionId: SessionId,
     private val featureFlagService: FeatureFlagService,
     private val computeCacheSizeUseCase: ComputeCacheSizeUseCase,
     private val clearCacheUseCase: ClearCacheUseCase,
@@ -135,10 +137,10 @@ class DeveloperSettingsPresenter(
                     }
                     appPreferencesStore.setTracingLogPacks(currentPacks)
                 }
-                is DeveloperSettingsEvents.ChangeBrandColor -> {
+                is DeveloperSettingsEvents.ChangeBrandColor -> coroutineScope.launch {
                     showColorPicker = false
                     val color = event.color?.value?.toHexString(HexFormat.UpperCase)?.substring(2, 8)
-                    enterpriseService.overrideBrandColor(color)
+                    enterpriseService.overrideBrandColor(sessionId, color)
                 }
                 is DeveloperSettingsEvents.SetShowColorPicker -> {
                     showColorPicker = event.show

features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt

@@ -25,6 +25,8 @@ import io.element.android.libraries.featureflag.api.Feature
 import io.element.android.libraries.featureflag.api.FeatureFlags
 import io.element.android.libraries.featureflag.test.FakeFeature
 import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
+import io.element.android.libraries.matrix.api.core.SessionId
+import io.element.android.libraries.matrix.test.A_SESSION_ID
 import io.element.android.libraries.matrix.test.core.aBuildMeta
 import io.element.android.libraries.preferences.test.InMemoryAppPreferencesStore
 import io.element.android.tests.testutils.WarmUpRule
@@ -184,7 +186,7 @@ class DeveloperSettingsPresenterTest {
 
     @Test
     fun `present - enterprise build can change the brand color`() = runTest {
-        val overrideBrandColorResult = lambdaRecorder<String?, Unit> { }
+        val overrideBrandColorResult = lambdaRecorder<SessionId?, String?, Unit> { _, _ -> }
         val presenter = createDeveloperSettingsPresenter(
             enterpriseService = FakeEnterpriseService(
                 isEnterpriseBuild = true,
@@ -203,12 +205,14 @@ class DeveloperSettingsPresenterTest {
             assertThat(awaitItem().showColorPicker).isTrue()
             initialState.eventSink(DeveloperSettingsEvents.ChangeBrandColor(Color.Green))
             assertThat(awaitItem().showColorPicker).isFalse()
+            skipItems(1)
             overrideBrandColorResult.assertions().isCalledOnce()
-                .with(value("00FF00"))
+                .with(value(A_SESSION_ID), value("00FF00"))
         }
     }
 
     private fun createDeveloperSettingsPresenter(
+        sessionId: SessionId = A_SESSION_ID,
         featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService(
             getAvailableFeaturesResult = { _, _ ->
                 listOf(
@@ -227,6 +231,7 @@ class DeveloperSettingsPresenterTest {
         enterpriseService: EnterpriseService = FakeEnterpriseService(),
     ): DeveloperSettingsPresenter {
         return DeveloperSettingsPresenter(
+            sessionId = sessionId,
             featureFlagService = featureFlagService,
             computeCacheSizeUseCase = cacheSizeUseCase,
             clearCacheUseCase = clearCacheUseCase,

features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt

@@ -36,14 +36,6 @@ interface BugReporter {
      */
     fun logDirectory(): File
 
-    /**
-     * Set the subfolder name for the log directory.
-     * This will create a subfolder in the log directory with the given name.
-     * It will also configure the Rust SDK to use this subfolder for its logs.
-     * If the name is null, the log files will be stored in the base folder for the logs.
-     */
-    fun setLogDirectorySubfolder(subfolderName: String?)
-
     /**
      * Set the current tracing log level.
      */

features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt

@@ -28,16 +28,22 @@ import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.core.data.tryOrNull
 import io.element.android.libraries.core.meta.BuildMeta
 import io.element.android.libraries.core.mimetype.MimeTypes
+import io.element.android.libraries.di.annotations.AppCoroutineScope
 import io.element.android.libraries.di.annotations.ApplicationContext
 import io.element.android.libraries.matrix.api.MatrixClientProvider
 import io.element.android.libraries.matrix.api.SdkMetadata
-import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.tracing.TracingService
 import io.element.android.libraries.network.useragent.UserAgentProvider
 import io.element.android.libraries.sessionstorage.api.SessionStore
+import io.element.android.libraries.sessionstorage.api.sessionIdFlow
 import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withContext
 import okhttp3.MediaType.Companion.toMediaTypeOrNull
@@ -67,6 +73,8 @@ import java.util.Locale
 @Inject
 class DefaultBugReporter(
     @ApplicationContext private val context: Context,
+    @AppCoroutineScope
+    private val appCoroutineScope: CoroutineScope,
     private val screenshotHolder: ScreenshotHolder,
     private val crashDataStore: CrashDataStore,
     private val coroutineDispatchers: CoroutineDispatchers,
@@ -78,7 +86,6 @@ class DefaultBugReporter(
     private val sdkMetadata: SdkMetadata,
     private val matrixClientProvider: MatrixClientProvider,
     private val tracingService: TracingService,
-    matrixAuthenticationService: MatrixAuthenticationService,
 ) : BugReporter {
     companion object {
         // filenames
@@ -98,13 +105,18 @@ class DefaultBugReporter(
         if (buildMeta.isEnterpriseBuild) {
             val logSubfolder = runBlocking {
                 sessionStore.getLatestSession()
-            }?.userId?.substringAfter(":")
+            }?.userId?.let(::UserId)?.domainName
             setCurrentLogDirectory(logSubfolder)
-            matrixAuthenticationService.listenToNewMatrixClients {
-                // When a new Matrix client is created, we update the tracing configuration to write
-                // the files in a dedicated subfolders.
-                setLogDirectorySubfolder(it.userIdServerName())
-            }
+            sessionStore.sessionIdFlow()
+                .map {
+                    it?.let(::UserId)?.domainName
+                }
+                .distinctUntilChanged()
+                .onEach { logSubfolder ->
+                    setCurrentLogDirectory(logSubfolder)
+                    tracingService.updateWriteToFilesConfiguration(createWriteToFilesConfiguration())
+                }
+                .launchIn(appCoroutineScope)
         }
     }
 
@@ -335,13 +347,6 @@ class DefaultBugReporter(
         }
     }
 
-    override fun setLogDirectorySubfolder(subfolderName: String?) {
-        if (buildMeta.isEnterpriseBuild) {
-            setCurrentLogDirectory(subfolderName)
-            tracingService.updateWriteToFilesConfiguration(createWriteToFilesConfiguration())
-        }
-    }
-
     private fun setCurrentLogDirectory(subfolderName: String?) {
         currentLogDirectory = if (subfolderName == null) {
             baseLogDirectory

features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt

@@ -54,10 +54,6 @@ class FakeBugReporter(val mode: Mode = Mode.Success) : BugReporter {
         return File("fake")
     }
 
-    override fun setLogDirectorySubfolder(subfolderName: String?) {
-        // No op
-    }
-
     override fun setCurrentTracingLogLevel(logLevel: String) {
         // No op
     }

features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt

@@ -15,7 +15,6 @@ import io.element.android.features.rageshake.impl.crash.FakeCrashDataStore
 import io.element.android.features.rageshake.impl.screenshot.FakeScreenshotHolder
 import io.element.android.libraries.core.meta.BuildMeta
 import io.element.android.libraries.matrix.api.MatrixClientProvider
-import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
 import io.element.android.libraries.matrix.api.tracing.TracingService
 import io.element.android.libraries.matrix.api.tracing.WriteToFilesConfiguration
 import io.element.android.libraries.matrix.test.A_DEVICE_ID
@@ -23,7 +22,6 @@ import io.element.android.libraries.matrix.test.A_USER_ID
 import io.element.android.libraries.matrix.test.FakeMatrixClient
 import io.element.android.libraries.matrix.test.FakeMatrixClientProvider
 import io.element.android.libraries.matrix.test.FakeSdkMetadata
-import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService
 import io.element.android.libraries.matrix.test.core.aBuildMeta
 import io.element.android.libraries.matrix.test.encryption.FakeEncryptionService
 import io.element.android.libraries.matrix.test.notificationsettings.FakeNotificationSettingsService
@@ -34,8 +32,10 @@ import io.element.android.libraries.sessionstorage.test.InMemorySessionStore
 import io.element.android.libraries.sessionstorage.test.aSessionData
 import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.testCoroutineDispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import okhttp3.MultipartReader
 import okhttp3.OkHttpClient
@@ -405,53 +405,85 @@ class DefaultBugReporterTest {
         assertThat(sut.logDirectory().absolutePath).endsWith("/cache/logs")
     }
 
+    @OptIn(ExperimentalCoroutinesApi::class)
     @Test
-    fun `when the log directory is updated, the tracing service is invoked`() = runTest {
+    fun `when a session is added, the tracing service is invoked`() = runTest {
         var param: WriteToFilesConfiguration? = null
         val updateWriteToFilesConfigurationResult = lambdaRecorder<WriteToFilesConfiguration, Unit> {
             param = it
         }
-        val sut = createDefaultBugReporter(
+        val sessionStore = InMemorySessionStore()
+        createDefaultBugReporter(
             buildMeta = aBuildMeta(isEnterpriseBuild = true),
+            sessionStore = sessionStore,
             tracingService = FakeTracingService(
                 updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult,
             ),
         )
-        sut.setLogDirectorySubfolder("my.sub.folder")
+        sessionStore.addSession(aSessionData(sessionId = "@alice:server.org"))
+        runCurrent()
         updateWriteToFilesConfigurationResult.assertions().isCalledOnce()
         assertThat(param).isNotNull()
         assertThat(param).isInstanceOf(WriteToFilesConfiguration.Enabled::class.java)
-        assertThat((param as WriteToFilesConfiguration.Enabled).directory).endsWith("/cache/logs/my.sub.folder")
+        assertThat((param as WriteToFilesConfiguration.Enabled).directory).endsWith("/cache/logs/server.org")
         assertThat((param as WriteToFilesConfiguration.Enabled).filenamePrefix).isEqualTo("logs")
         assertThat((param as WriteToFilesConfiguration.Enabled).numberOfFiles).isEqualTo(168)
         assertThat((param as WriteToFilesConfiguration.Enabled).filenameSuffix).isEqualTo("log")
     }
 
+    @OptIn(ExperimentalCoroutinesApi::class)
     @Test
-    fun `foss build - when the log directory is updated, the tracing service is not invoked`() = runTest {
+    fun `when another session is added on same domain, the tracing service is not invoked`() = runTest {
         val updateWriteToFilesConfigurationResult = lambdaRecorder<WriteToFilesConfiguration, Unit> {}
-        val sut = createDefaultBugReporter(
-            tracingService = FakeTracingService(
-                updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult,
-            )
-        )
-        sut.setLogDirectorySubfolder("my.sub.folder")
-        updateWriteToFilesConfigurationResult.assertions().isNeverCalled()
-    }
-
-    @Test
-    fun `when the log directory is reset, the tracing service is invoked`() = runTest {
-        var param: WriteToFilesConfiguration? = null
-        val updateWriteToFilesConfigurationResult = lambdaRecorder<WriteToFilesConfiguration, Unit> {
-            param = it
-        }
-        val sut = createDefaultBugReporter(
+        val sessionStore = InMemorySessionStore()
+        createDefaultBugReporter(
             buildMeta = aBuildMeta(isEnterpriseBuild = true),
+            sessionStore = sessionStore,
             tracingService = FakeTracingService(
                 updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult,
             ),
         )
-        sut.setLogDirectorySubfolder(null)
+        sessionStore.addSession(aSessionData(sessionId = "@alice:server.org"))
+        runCurrent()
+        updateWriteToFilesConfigurationResult.assertions().isCalledOnce()
+        sessionStore.addSession(aSessionData(sessionId = "@bob:server.org"))
+        runCurrent()
+        updateWriteToFilesConfigurationResult.assertions().isCalledOnce()
+    }
+
+    @Test
+    fun `foss build - when a session is added, the tracing service is not invoked`() = runTest {
+        val updateWriteToFilesConfigurationResult = lambdaRecorder<WriteToFilesConfiguration, Unit> {}
+        val sessionStore = InMemorySessionStore()
+        createDefaultBugReporter(
+            tracingService = FakeTracingService(
+                updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult,
+            ),
+            sessionStore = sessionStore,
+        )
+        sessionStore.addSession(aSessionData(sessionId = "@alice:server.org"))
+        updateWriteToFilesConfigurationResult.assertions().isNeverCalled()
+    }
+
+    @OptIn(ExperimentalCoroutinesApi::class)
+    @Test
+    fun `when the user signs out, the tracing service is invoked`() = runTest {
+        var param: WriteToFilesConfiguration? = null
+        val updateWriteToFilesConfigurationResult = lambdaRecorder<WriteToFilesConfiguration, Unit> {
+            param = it
+        }
+        val sessionStore = InMemorySessionStore(
+            initialList = listOf(aSessionData(sessionId = "@alice:server.org")),
+        )
+        createDefaultBugReporter(
+            buildMeta = aBuildMeta(isEnterpriseBuild = true),
+            tracingService = FakeTracingService(
+                updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult,
+            ),
+            sessionStore = sessionStore,
+        )
+        sessionStore.removeSession("@alice:server.org")
+        runCurrent()
         updateWriteToFilesConfigurationResult.assertions().isCalledOnce()
         assertThat(param).isNotNull()
         assertThat(param).isInstanceOf(WriteToFilesConfiguration.Enabled::class.java)
@@ -464,66 +496,16 @@ class DefaultBugReporterTest {
     @Test
     fun `foss build - when the log directory is reset, the tracing service is not invoked`() = runTest {
         val updateWriteToFilesConfigurationResult = lambdaRecorder<WriteToFilesConfiguration, Unit> {}
-        val sut = createDefaultBugReporter(
+        val sessionStore = InMemorySessionStore(
+            initialList = listOf(aSessionData(sessionId = "@alice:server.org")),
+        )
+        createDefaultBugReporter(
             tracingService = FakeTracingService(
                 updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult,
-            )
+            ),
+            sessionStore = sessionStore,
         )
-        sut.setLogDirectorySubfolder(null)
-        updateWriteToFilesConfigurationResult.assertions().isNeverCalled()
-    }
-
-    @Test
-    fun `when a new MatrixClient is created the logs folder is updated`() = runTest {
-        var param: WriteToFilesConfiguration? = null
-        val updateWriteToFilesConfigurationResult = lambdaRecorder<WriteToFilesConfiguration, Unit> {
-            param = it
-        }
-        val matrixAuthenticationService = FakeMatrixAuthenticationService().apply {
-            givenMatrixClient(
-                FakeMatrixClient(
-                    userIdServerNameLambda = { "domain.foo.org" },
-                )
-            )
-        }
-        val sut = createDefaultBugReporter(
-            buildMeta = aBuildMeta(isEnterpriseBuild = true),
-            matrixAuthenticationService = matrixAuthenticationService,
-            tracingService = FakeTracingService(
-                updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult,
-            )
-        )
-        assertThat(sut.logDirectory().absolutePath).endsWith("/cache/logs")
-        matrixAuthenticationService.login("alice", "password")
-        assertThat(sut.logDirectory().absolutePath).endsWith("/cache/logs/domain.foo.org")
-        updateWriteToFilesConfigurationResult.assertions().isCalledOnce()
-        assertThat(param).isNotNull()
-        assertThat(param).isInstanceOf(WriteToFilesConfiguration.Enabled::class.java)
-        assertThat((param as WriteToFilesConfiguration.Enabled).directory).endsWith("/cache/logs/domain.foo.org")
-        assertThat((param as WriteToFilesConfiguration.Enabled).filenamePrefix).isEqualTo("logs")
-        assertThat((param as WriteToFilesConfiguration.Enabled).numberOfFiles).isEqualTo(168)
-        assertThat((param as WriteToFilesConfiguration.Enabled).filenameSuffix).isEqualTo("log")
-    }
-
-    @Test
-    fun `foss build - when a new MatrixClient is created the logs folder is not updated`() = runTest {
-        val updateWriteToFilesConfigurationResult = lambdaRecorder<WriteToFilesConfiguration, Unit> {}
-        val matrixAuthenticationService = FakeMatrixAuthenticationService().apply {
-            givenMatrixClient(
-                FakeMatrixClient(
-                    userIdServerNameLambda = { "domain.foo.org" },
-                )
-            )
-        }
-        val sut = createDefaultBugReporter(
-            matrixAuthenticationService = matrixAuthenticationService,
-            tracingService = FakeTracingService(
-                updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult,
-            )
-        )
-        assertThat(sut.logDirectory().absolutePath).endsWith("/cache/logs")
-        matrixAuthenticationService.login("alice", "password")
-        assertThat(sut.logDirectory().absolutePath).endsWith("/cache/logs")
+        sessionStore.removeSession("@alice:server.org")
         updateWriteToFilesConfigurationResult.assertions().isNeverCalled()
     }
 
@@ -534,10 +516,10 @@ class DefaultBugReporterTest {
         crashDataStore: CrashDataStore = FakeCrashDataStore(),
         server: MockWebServer = MockWebServer(),
         tracingService: TracingService = FakeTracingService(),
-        matrixAuthenticationService: MatrixAuthenticationService = FakeMatrixAuthenticationService(),
     ): DefaultBugReporter {
         return DefaultBugReporter(
             context = RuntimeEnvironment.getApplication(),
+            appCoroutineScope = backgroundScope,
             screenshotHolder = FakeScreenshotHolder(),
             crashDataStore = crashDataStore,
             coroutineDispatchers = testCoroutineDispatchers(),
@@ -549,7 +531,6 @@ class DefaultBugReporterTest {
             sdkMetadata = FakeSdkMetadata("123456789"),
             matrixClientProvider = matrixClientProvider,
             tracingService = tracingService,
-            matrixAuthenticationService = matrixAuthenticationService,
         )
     }