Bug reporter: ensure the log are store in the correct folder.

appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt

@@ -38,7 +38,6 @@ import io.element.android.features.announcement.api.AnnouncementService
 import io.element.android.features.login.api.LoginParams
 import io.element.android.features.login.api.accesscontrol.AccountProviderAccessControl
 import io.element.android.features.rageshake.api.bugreport.BugReportEntryPoint
-import io.element.android.features.rageshake.api.reporter.BugReporter
 import io.element.android.features.signedout.api.SignedOutEntryPoint
 import io.element.android.libraries.accountselect.api.AccountSelectEntryPoint
 import io.element.android.libraries.architecture.BackstackView
@@ -80,7 +79,6 @@ class RootFlowNode(
     private val accountSelectEntryPoint: AccountSelectEntryPoint,
     private val intentResolver: IntentResolver,
     private val oidcActionFlow: OidcActionFlow,
-    private val bugReporter: BugReporter,
     private val featureFlagService: FeatureFlagService,
     private val announcementService: AnnouncementService,
 ) : BaseFlowNode<RootFlowNode.NavTarget>(
@@ -130,7 +128,6 @@ class RootFlowNode(
 
     private fun switchToNotLoggedInFlow(params: LoginParams?) {
         matrixSessionCache.removeAll()
-        bugReporter.setLogDirectorySubfolder(null)
         backstack.safeRoot(NavTarget.NotLoggedInFlow(params))
     }
 

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.userIdFlow
 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 {
+            sessionStore.userIdFlow()
-                // When a new Matrix client is created, we update the tracing configuration to write
+                .map {
-                // the files in a dedicated subfolders.
+                    it?.let(::UserId)?.domainName
-                setLogDirectorySubfolder(it.userIdServerName())
+                }
-            }
+                .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(
+        val sessionStore = InMemorySessionStore()
-            tracingService = FakeTracingService(
+        createDefaultBugReporter(
-                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(
             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)
+        sessionStore.removeSession("@alice:server.org")
-        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")
         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,
         )
     }