diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt index 19290c5f8b..6d635df840 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt +++ b/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( @@ -130,7 +128,6 @@ class RootFlowNode( private fun switchToNotLoggedInFlow(params: LoginParams?) { matrixSessionCache.removeAll() - bugReporter.setLogDirectorySubfolder(null) backstack.safeRoot(NavTarget.NotLoggedInFlow(params)) } diff --git a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt index 02e53fbb1c..6d14ca63bf 100644 --- a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt +++ b/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. */ diff --git a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt b/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt index 5eecd17f31..2584ebe597 100755 --- a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt +++ b/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 { - // When a new Matrix client is created, we update the tracing configuration to write - // the files in a dedicated subfolders. - setLogDirectorySubfolder(it.userIdServerName()) - } + sessionStore.userIdFlow() + .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 diff --git a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt b/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt index bd9c538c0d..15a6c7d456 100644 --- a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt +++ b/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 } diff --git a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt b/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt index 794c5b56e0..ddc25b37f4 100755 --- a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt +++ b/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 { 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 {} - 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 { - 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 {} + 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 { + 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 {} - 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 { - 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 {} - 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, ) }