From 6079e5a4d69f3f5964fc4f508987c37675f49a15 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 6 Aug 2025 16:13:07 +0200 Subject: [PATCH 1/7] Avoid code duplication --- .../android/appnav/di/MatrixSessionCache.kt | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/di/MatrixSessionCache.kt b/appnav/src/main/kotlin/io/element/android/appnav/di/MatrixSessionCache.kt index 302a12d99b..0941c7c8cc 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/di/MatrixSessionCache.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/di/MatrixSessionCache.kt @@ -42,12 +42,7 @@ class MatrixSessionCache @Inject constructor( init { authenticationService.listenToNewMatrixClients { matrixClient -> - val syncOrchestrator = syncOrchestratorFactory.create(matrixClient) - sessionIdsToMatrixSession[matrixClient.sessionId] = InMemoryMatrixSession( - matrixClient = matrixClient, - syncOrchestrator = syncOrchestrator, - ) - syncOrchestrator.start() + onMatrixClient(matrixClient) } } @@ -105,17 +100,21 @@ class MatrixSessionCache @Inject constructor( Timber.d("Restore matrix session: $sessionId") return authenticationService.restoreSession(sessionId) .onSuccess { matrixClient -> - val syncOrchestrator = syncOrchestratorFactory.create(matrixClient) - sessionIdsToMatrixSession[matrixClient.sessionId] = InMemoryMatrixSession( - matrixClient = matrixClient, - syncOrchestrator = syncOrchestrator, - ) - syncOrchestrator.start() + onMatrixClient(matrixClient) } .onFailure { Timber.e(it, "Fail to restore session") } } + + private fun onMatrixClient(matrixClient: MatrixClient) { + val syncOrchestrator = syncOrchestratorFactory.create(matrixClient) + sessionIdsToMatrixSession[matrixClient.sessionId] = InMemoryMatrixSession( + matrixClient = matrixClient, + syncOrchestrator = syncOrchestrator, + ) + syncOrchestrator.start() + } } private data class InMemoryMatrixSession( From 5577d2ca9beec52dc9a2f126a9e5e444a1cc463d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Aug 2025 11:31:05 +0200 Subject: [PATCH 2/7] Store log files in subfolder based on the homeserver domain. --- .../x/initializer/PlatformInitializer.kt | 14 +----- .../io/element/android/appnav/RootFlowNode.kt | 3 ++ features/rageshake/api/build.gradle.kts | 1 + .../logs/WriteToFilesConfigurationFactory.kt | 20 +++++++++ .../rageshake/api/reporter/BugReporter.kt | 8 ++++ .../impl/reporter/DefaultBugReporter.kt | 44 ++++++++++++++++--- .../matrix/api/tracing/TracingService.kt | 2 + .../auth/RustMatrixAuthenticationService.kt | 13 +++--- .../matrix/impl/tracing/RustTracingService.kt | 7 +++ 9 files changed, 90 insertions(+), 22 deletions(-) create mode 100644 features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/logs/WriteToFilesConfigurationFactory.kt diff --git a/app/src/main/kotlin/io/element/android/x/initializer/PlatformInitializer.kt b/app/src/main/kotlin/io/element/android/x/initializer/PlatformInitializer.kt index 6dd0d69dc0..40016922ae 100644 --- a/app/src/main/kotlin/io/element/android/x/initializer/PlatformInitializer.kt +++ b/app/src/main/kotlin/io/element/android/x/initializer/PlatformInitializer.kt @@ -10,11 +10,10 @@ package io.element.android.x.initializer import android.content.Context import android.system.Os import androidx.startup.Initializer -import io.element.android.features.rageshake.api.reporter.BugReporter +import io.element.android.features.rageshake.api.logs.createWriteToFilesConfiguration import io.element.android.libraries.architecture.bindings import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.tracing.TracingConfiguration -import io.element.android.libraries.matrix.api.tracing.WriteToFilesConfiguration import io.element.android.x.di.AppBindings import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking @@ -34,7 +33,7 @@ class PlatformInitializer : Initializer { val logLevel = runBlocking { preferencesStore.getTracingLogLevelFlow().first() } val tracingConfiguration = TracingConfiguration( writesToLogcat = runBlocking { featureFlagService.isFeatureEnabled(FeatureFlags.PrintLogsToLogcat) }, - writesToFilesConfiguration = defaultWriteToDiskConfiguration(bugReporter), + writesToFilesConfiguration = bugReporter.createWriteToFilesConfiguration(), logLevel = logLevel, extraTargets = listOf(ELEMENT_X_TARGET), traceLogPacks = runBlocking { preferencesStore.getTracingLogPacksFlow().first() }, @@ -45,14 +44,5 @@ class PlatformInitializer : Initializer { Os.setenv("RUST_BACKTRACE", "1", true) } - private fun defaultWriteToDiskConfiguration(bugReporter: BugReporter): WriteToFilesConfiguration.Enabled { - return WriteToFilesConfiguration.Enabled( - directory = bugReporter.logDirectory().absolutePath, - filenamePrefix = "logs", - // Keep a maximum of 1 week of log files. - numberOfFiles = 7 * 24, - ) - } - override fun dependencies(): List>> = mutableListOf() } 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 ae9f935061..db512ff2ea 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt @@ -36,6 +36,7 @@ import io.element.android.appnav.root.RootView import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.login.api.LoginParams 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.features.viewfolder.api.ViewFolderEntryPoint import io.element.android.libraries.architecture.BackstackView @@ -73,6 +74,7 @@ class RootFlowNode @AssistedInject constructor( private val signedOutEntryPoint: SignedOutEntryPoint, private val intentResolver: IntentResolver, private val oidcActionFlow: OidcActionFlow, + private val bugReporter: BugReporter, ) : BaseFlowNode( backstack = BackStack( initialElement = NavTarget.SplashScreen, @@ -123,6 +125,7 @@ class RootFlowNode @AssistedInject constructor( private fun switchToNotLoggedInFlow(params: LoginParams?) { matrixSessionCache.removeAll() + bugReporter.setLogDirectorySubfolder(null) backstack.safeRoot(NavTarget.NotLoggedInFlow(params)) } diff --git a/features/rageshake/api/build.gradle.kts b/features/rageshake/api/build.gradle.kts index f47b748c3e..7fe1620613 100644 --- a/features/rageshake/api/build.gradle.kts +++ b/features/rageshake/api/build.gradle.kts @@ -16,5 +16,6 @@ dependencies { implementation(projects.libraries.architecture) implementation(projects.libraries.designsystem) implementation(projects.libraries.androidutils) + implementation(projects.libraries.matrix.api) implementation(projects.libraries.uiStrings) } diff --git a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/logs/WriteToFilesConfigurationFactory.kt b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/logs/WriteToFilesConfigurationFactory.kt new file mode 100644 index 0000000000..52298b5414 --- /dev/null +++ b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/logs/WriteToFilesConfigurationFactory.kt @@ -0,0 +1,20 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.rageshake.api.logs + +import io.element.android.features.rageshake.api.reporter.BugReporter +import io.element.android.libraries.matrix.api.tracing.WriteToFilesConfiguration + +fun BugReporter.createWriteToFilesConfiguration(): WriteToFilesConfiguration { + return WriteToFilesConfiguration.Enabled( + directory = logDirectory().absolutePath, + filenamePrefix = "logs", + // Keep a maximum of 1 week of log files. + numberOfFiles = 7 * 24, + ) +} 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 1cc3cc8908..bb625370cf 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 @@ -34,6 +34,14 @@ 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 b9ed1c7778..5f3195c32f 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 @@ -13,6 +13,7 @@ import androidx.core.net.toFile import androidx.core.net.toUri import com.squareup.anvil.annotations.ContributesBinding import io.element.android.appconfig.RageshakeConfig +import io.element.android.features.rageshake.api.logs.createWriteToFilesConfiguration import io.element.android.features.rageshake.api.reporter.BugReporter import io.element.android.features.rageshake.api.reporter.BugReporterListener import io.element.android.features.rageshake.impl.crash.CrashDataStore @@ -28,11 +29,14 @@ import io.element.android.libraries.di.ApplicationContext import io.element.android.libraries.di.SingleIn 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 kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.OkHttpClient @@ -71,6 +75,8 @@ class DefaultBugReporter @Inject constructor( private val bugReporterUrlProvider: BugReporterUrlProvider, private val sdkMetadata: SdkMetadata, private val matrixClientProvider: MatrixClientProvider, + private val tracingService: TracingService, + matrixAuthenticationService: MatrixAuthenticationService, ) : BugReporter { companion object { // filenames @@ -81,7 +87,21 @@ class DefaultBugReporter @Inject constructor( private val logcatCommandDebug = arrayOf("logcat", "-d", "-v", "threadtime", "*:*") private var currentTracingLogLevel: String? = null - private val logCatErrFile = File(logDirectory().absolutePath, LOG_CAT_FILENAME) + private val logCatErrFile: File + get() = File(logDirectory(), LOG_CAT_FILENAME) + private val baseLogDirectory = File(context.cacheDir, LOG_DIRECTORY_NAME) + private var currentLogDirectory: File = baseLogDirectory + + init { + val logSubfolder = runBlocking { + sessionStore.getLatestSession() + }?.userId?.substringAfter(":") + setCurrentLogDirectory(logSubfolder) + matrixAuthenticationService.listenToNewMatrixClients { + // When a new Matrix client is created, we update the tracing configuration to write to files + setLogDirectorySubfolder(it.userIdServerName()) + } + } override suspend fun sendBugReport( withDevicesLogs: Boolean, @@ -286,11 +306,24 @@ class DefaultBugReporter @Inject constructor( } override fun logDirectory(): File { - return File(context.cacheDir, LOG_DIRECTORY_NAME).apply { + return currentLogDirectory.apply { mkdirs() } } + override fun setLogDirectorySubfolder(subfolderName: String?) { + setCurrentLogDirectory(subfolderName) + tracingService.updateWriteToFilesConfiguration(createWriteToFilesConfiguration()) + } + + private fun setCurrentLogDirectory(subfolderName: String?) { + currentLogDirectory = if (subfolderName == null) { + baseLogDirectory + } else { + File(baseLogDirectory, subfolderName) + } + } + suspend fun deleteAllFiles(predicate: (File) -> Boolean) { withContext(coroutineDispatchers.io) { getLogFiles() @@ -325,11 +358,12 @@ class DefaultBugReporter @Inject constructor( * @return the file if the operation succeeds */ override fun saveLogCat() { - if (logCatErrFile.exists()) { - logCatErrFile.safeDelete() + val file = logCatErrFile + if (file.exists()) { + file.safeDelete() } try { - logCatErrFile.writer().use { + file.writer().use { getLogCatError(it) } } catch (error: OutOfMemoryError) { diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/tracing/TracingService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/tracing/TracingService.kt index 5cef6cde02..8c183c80b4 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/tracing/TracingService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/tracing/TracingService.kt @@ -11,4 +11,6 @@ import timber.log.Timber interface TracingService { fun createTimberTree(target: String): Timber.Tree + + fun updateWriteToFilesConfiguration(config: WriteToFilesConfiguration) } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt index a1ffbc5da9..1a044a9f4a 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt @@ -71,9 +71,9 @@ class RustMatrixAuthenticationService @Inject constructor( private var currentClient: Client? = null private var currentHomeserver = MutableStateFlow(null) - private var newMatrixClientObserver: ((MatrixClient) -> Unit)? = null + private val newMatrixClientObservers = mutableListOf<(MatrixClient) -> Unit>() override fun listenToNewMatrixClients(lambda: (MatrixClient) -> Unit) { - newMatrixClientObserver = lambda + newMatrixClientObservers.add(lambda) } private fun rotateSessionPath(): SessionPaths { @@ -155,7 +155,8 @@ class RustMatrixAuthenticationService @Inject constructor( passphrase = pendingPassphrase, sessionPaths = currentSessionPaths, ) - newMatrixClientObserver?.invoke(rustMatrixClientFactory.create(client)) + val matrixClient = rustMatrixClientFactory.create(client) + newMatrixClientObservers.forEach { it.invoke(matrixClient) } sessionStore.storeData(sessionData) // Clean up the strong reference held here since it's no longer necessary @@ -246,7 +247,8 @@ class RustMatrixAuthenticationService @Inject constructor( pendingOAuthAuthorizationData?.close() pendingOAuthAuthorizationData = null - newMatrixClientObserver?.invoke(rustMatrixClientFactory.create(client)) + val matrixClient = rustMatrixClientFactory.create(client) + newMatrixClientObservers.forEach { it.invoke(matrixClient) } sessionStore.storeData(sessionData) // Clean up the strong reference held here since it's no longer necessary @@ -290,7 +292,8 @@ class RustMatrixAuthenticationService @Inject constructor( passphrase = pendingPassphrase, sessionPaths = emptySessionPaths, ) - newMatrixClientObserver?.invoke(rustMatrixClientFactory.create(client)) + val matrixClient = rustMatrixClientFactory.create(client) + newMatrixClientObservers.forEach { it.invoke(matrixClient) } sessionStore.storeData(sessionData) // Clean up the strong reference held here since it's no longer necessary diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/tracing/RustTracingService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/tracing/RustTracingService.kt index 912561d843..728bf136b8 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/tracing/RustTracingService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/tracing/RustTracingService.kt @@ -15,6 +15,7 @@ import io.element.android.libraries.matrix.api.tracing.TracingConfiguration import io.element.android.libraries.matrix.api.tracing.TracingService import io.element.android.libraries.matrix.api.tracing.WriteToFilesConfiguration import org.matrix.rustcomponents.sdk.TracingFileConfiguration +import org.matrix.rustcomponents.sdk.reloadTracingFileWriter import timber.log.Timber import javax.inject.Inject @@ -23,6 +24,12 @@ class RustTracingService @Inject constructor(private val buildMeta: BuildMeta) : override fun createTimberTree(target: String): Timber.Tree { return RustTracingTree(target = target, retrieveFromStackTrace = buildMeta.isDebuggable) } + + override fun updateWriteToFilesConfiguration(config: WriteToFilesConfiguration) { + config.toTracingFileConfiguration()?.let { + reloadTracingFileWriter(it) + } + } } private fun LogLevel.toRustLogLevel(): org.matrix.rustcomponents.sdk.LogLevel { From d71b639ada063aa3b67be978ae0d687dc7320a42 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Aug 2025 11:40:38 +0200 Subject: [PATCH 3/7] Ensure that deleteAllFiles will check all the log files. --- .../impl/reporter/DefaultBugReporter.kt | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) 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 5f3195c32f..60b4274689 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 @@ -326,9 +326,22 @@ class DefaultBugReporter @Inject constructor( suspend fun deleteAllFiles(predicate: (File) -> Boolean) { withContext(coroutineDispatchers.io) { - getLogFiles() - .filter(predicate) - .forEach { it.safeDelete() } + deleteAllFilesRecursive(baseLogDirectory, predicate) + } + } + + private fun deleteAllFilesRecursive( + directory: File, + predicate: (File) -> Boolean, + ) { + directory.listFiles()?.forEach { file -> + if (file.isDirectory) { + deleteAllFilesRecursive(file, predicate) + } else { + if (predicate(file)) { + file.safeDelete() + } + } } } From 0c215f2d1e553ec53c7b6f1b3111dc2bde6e3432 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Aug 2025 12:19:26 +0200 Subject: [PATCH 4/7] Fix tests --- .../impl/bugreport/FakeBugReporter.kt | 4 ++ .../impl/reporter/DefaultBugReporterTest.kt | 59 +++++++------------ .../matrix/test/tracing/FakeTracingService.kt | 26 ++++++++ 3 files changed, 52 insertions(+), 37 deletions(-) create mode 100644 libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/tracing/FakeTracingService.kt 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 1f10cd0ab1..ebaa524bd5 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 @@ -53,6 +53,10 @@ 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 e73c7863b4..d85c35595c 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 @@ -10,14 +10,19 @@ package io.element.android.features.rageshake.impl.reporter import com.google.common.truth.Truth.assertThat import io.element.android.appconfig.RageshakeConfig import io.element.android.features.rageshake.api.reporter.BugReporterListener +import io.element.android.features.rageshake.impl.crash.CrashDataStore import io.element.android.features.rageshake.impl.crash.FakeCrashDataStore import io.element.android.features.rageshake.impl.screenshot.FakeScreenshotHolder +import io.element.android.libraries.matrix.api.MatrixClientProvider 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.tracing.FakeTracingService import io.element.android.libraries.network.useragent.DefaultUserAgentProvider +import io.element.android.libraries.sessionstorage.api.SessionStore import io.element.android.libraries.sessionstorage.impl.memory.InMemorySessionStore import io.element.android.libraries.sessionstorage.test.aSessionData import io.element.android.tests.testutils.testCoroutineDispatchers @@ -45,7 +50,7 @@ class DefaultBugReporterTest { .setResponseCode(200) ) server.start() - val sut = createDefaultBugReporter(server) + val sut = createDefaultBugReporter(server = server) var onUploadCancelledCalled = false var onUploadFailedCalled = false val progressValues = mutableListOf() @@ -97,22 +102,14 @@ class DefaultBugReporterTest { storeData(aSessionData(sessionId = "@foo:example.com", deviceId = "ABCDEFGH")) } - val buildMeta = aBuildMeta() val fakeEncryptionService = FakeEncryptionService() val matrixClient = FakeMatrixClient(encryptionService = fakeEncryptionService) fakeEncryptionService.givenDeviceKeys("CURVECURVECURVE", "EDKEYEDKEYEDKY") - val sut = DefaultBugReporter( - context = RuntimeEnvironment.getApplication(), - screenshotHolder = FakeScreenshotHolder(), + val sut = createDefaultBugReporter( + server = server, crashDataStore = FakeCrashDataStore(), - coroutineDispatchers = testCoroutineDispatchers(), - okHttpClient = { OkHttpClient.Builder().build() }, - userAgentProvider = DefaultUserAgentProvider(buildMeta, FakeSdkMetadata("123456789")), sessionStore = mockSessionStore, - buildMeta = buildMeta, - bugReporterUrlProvider = { server.url("/") }, - sdkMetadata = FakeSdkMetadata("123456789"), matrixClientProvider = FakeMatrixClientProvider(getClient = { Result.success(matrixClient) }) ) @@ -166,22 +163,13 @@ class DefaultBugReporterTest { storeData(aSessionData("@foo:example.com", "ABCDEFGH")) } - val buildMeta = aBuildMeta() val fakeEncryptionService = FakeEncryptionService() val matrixClient = FakeMatrixClient(encryptionService = fakeEncryptionService) fakeEncryptionService.givenDeviceKeys(null, null) - val sut = DefaultBugReporter( - context = RuntimeEnvironment.getApplication(), - screenshotHolder = FakeScreenshotHolder(), - crashDataStore = FakeCrashDataStore(), - coroutineDispatchers = testCoroutineDispatchers(), - okHttpClient = { OkHttpClient.Builder().build() }, - userAgentProvider = DefaultUserAgentProvider(buildMeta, FakeSdkMetadata("123456789")), + val sut = createDefaultBugReporter( + server = server, sessionStore = mockSessionStore, - buildMeta = buildMeta, - bugReporterUrlProvider = { server.url("/") }, - sdkMetadata = FakeSdkMetadata("123456789"), matrixClientProvider = FakeMatrixClientProvider(getClient = { Result.success(matrixClient) }) ) @@ -209,21 +197,13 @@ class DefaultBugReporterTest { ) server.start() - val buildMeta = aBuildMeta() val fakeEncryptionService = FakeEncryptionService() fakeEncryptionService.givenDeviceKeys(null, null) - val sut = DefaultBugReporter( - context = RuntimeEnvironment.getApplication(), - screenshotHolder = FakeScreenshotHolder(), + val sut = createDefaultBugReporter( + server = server, crashDataStore = FakeCrashDataStore("I did crash", true), - coroutineDispatchers = testCoroutineDispatchers(), - okHttpClient = { OkHttpClient.Builder().build() }, - userAgentProvider = DefaultUserAgentProvider(buildMeta, FakeSdkMetadata("123456789")), sessionStore = InMemorySessionStore(), - buildMeta = buildMeta, - bugReporterUrlProvider = { server.url("/") }, - sdkMetadata = FakeSdkMetadata("123456789"), matrixClientProvider = FakeMatrixClientProvider(getClient = { Result.failure(Exception("Mock no client")) }) ) @@ -276,7 +256,7 @@ class DefaultBugReporterTest { .setBody("""{"error": "An error body"}""") ) server.start() - val sut = createDefaultBugReporter(server) + val sut = createDefaultBugReporter(server = server) var onUploadCancelledCalled = false var onUploadFailedCalled = false var onUploadFailedReason: String? = null @@ -319,21 +299,26 @@ class DefaultBugReporterTest { } private fun TestScope.createDefaultBugReporter( - server: MockWebServer + sessionStore: SessionStore = InMemorySessionStore(), + matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider(), + crashDataStore: CrashDataStore = FakeCrashDataStore(), + server: MockWebServer = MockWebServer(), ): DefaultBugReporter { val buildMeta = aBuildMeta() return DefaultBugReporter( context = RuntimeEnvironment.getApplication(), screenshotHolder = FakeScreenshotHolder(), - crashDataStore = FakeCrashDataStore(), + crashDataStore = crashDataStore, coroutineDispatchers = testCoroutineDispatchers(), okHttpClient = { OkHttpClient.Builder().build() }, userAgentProvider = DefaultUserAgentProvider(buildMeta, FakeSdkMetadata("123456789")), - sessionStore = InMemorySessionStore(), + sessionStore = sessionStore, buildMeta = buildMeta, bugReporterUrlProvider = { server.url("/") }, sdkMetadata = FakeSdkMetadata("123456789"), - matrixClientProvider = FakeMatrixClientProvider() + matrixClientProvider = matrixClientProvider, + tracingService = FakeTracingService(), + matrixAuthenticationService = FakeMatrixAuthenticationService(), ) } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/tracing/FakeTracingService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/tracing/FakeTracingService.kt new file mode 100644 index 0000000000..52ffe8f32d --- /dev/null +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/tracing/FakeTracingService.kt @@ -0,0 +1,26 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.libraries.matrix.test.tracing + +import io.element.android.libraries.matrix.api.tracing.TracingService +import io.element.android.libraries.matrix.api.tracing.WriteToFilesConfiguration +import io.element.android.tests.testutils.lambda.lambdaError +import timber.log.Timber + +class FakeTracingService( + private val createTimberTreeResult: (String) -> Timber.Tree = { lambdaError() }, + private val updateWriteToFilesConfigurationResult: (WriteToFilesConfiguration) -> Unit = { lambdaError() } +) : TracingService { + override fun createTimberTree(target: String): Timber.Tree { + return createTimberTreeResult(target) + } + + override fun updateWriteToFilesConfiguration(config: WriteToFilesConfiguration) { + updateWriteToFilesConfigurationResult(config) + } +} From 0b0eab22817c964983e52cec0a8d618e2790de38 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Aug 2025 12:38:10 +0200 Subject: [PATCH 5/7] Add tests --- .../impl/reporter/DefaultBugReporterTest.kt | 93 ++++++++++++++++++- 1 file changed, 91 insertions(+), 2 deletions(-) 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 d85c35595c..05fcbd26df 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 @@ -14,6 +14,9 @@ import io.element.android.features.rageshake.impl.crash.CrashDataStore import io.element.android.features.rageshake.impl.crash.FakeCrashDataStore import io.element.android.features.rageshake.impl.screenshot.FakeScreenshotHolder 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.FakeMatrixClient import io.element.android.libraries.matrix.test.FakeMatrixClientProvider import io.element.android.libraries.matrix.test.FakeSdkMetadata @@ -25,6 +28,7 @@ import io.element.android.libraries.network.useragent.DefaultUserAgentProvider import io.element.android.libraries.sessionstorage.api.SessionStore import io.element.android.libraries.sessionstorage.impl.memory.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.test.TestScope import kotlinx.coroutines.test.runTest @@ -298,11 +302,96 @@ class DefaultBugReporterTest { assertThat(onUploadSucceedCalled).isFalse() } + @Test + fun `the log directory is initialized using the last session store data`() = runTest { + val sut = createDefaultBugReporter( + sessionStore = InMemorySessionStore().apply { + storeData(aSessionData(sessionId = "@alice:domain.com")) + } + ) + assertThat(sut.logDirectory().absolutePath).endsWith("/cache/logs/domain.com") + } + + @Test + fun `when the log directory is updated, the tracing service is invoked`() = runTest { + var param: WriteToFilesConfiguration? = null + val updateWriteToFilesConfigurationResult = lambdaRecorder { + param = it + } + val sut = createDefaultBugReporter( + tracingService = FakeTracingService( + updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult, + ) + ) + sut.setLogDirectorySubfolder("my.sub.folder") + 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).filenamePrefix).isEqualTo("logs") + assertThat((param as WriteToFilesConfiguration.Enabled).numberOfFiles).isEqualTo(168) + assertThat((param as WriteToFilesConfiguration.Enabled).filenameSuffix).isEqualTo("log") + } + + @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( + tracingService = FakeTracingService( + updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult, + ) + ) + sut.setLogDirectorySubfolder(null) + updateWriteToFilesConfigurationResult.assertions().isCalledOnce() + assertThat(param).isNotNull() + assertThat(param).isInstanceOf(WriteToFilesConfiguration.Enabled::class.java) + assertThat((param as WriteToFilesConfiguration.Enabled).directory).endsWith("/cache/logs") + 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 `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( + 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") + } + private fun TestScope.createDefaultBugReporter( sessionStore: SessionStore = InMemorySessionStore(), matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider(), crashDataStore: CrashDataStore = FakeCrashDataStore(), server: MockWebServer = MockWebServer(), + tracingService: TracingService = FakeTracingService(), + matrixAuthenticationService: MatrixAuthenticationService = FakeMatrixAuthenticationService(), ): DefaultBugReporter { val buildMeta = aBuildMeta() return DefaultBugReporter( @@ -317,8 +406,8 @@ class DefaultBugReporterTest { bugReporterUrlProvider = { server.url("/") }, sdkMetadata = FakeSdkMetadata("123456789"), matrixClientProvider = matrixClientProvider, - tracingService = FakeTracingService(), - matrixAuthenticationService = FakeMatrixAuthenticationService(), + tracingService = tracingService, + matrixAuthenticationService = matrixAuthenticationService, ) } From 861758b47dd41f48026fdf346d0d9c842bd5ae4c Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Aug 2025 14:02:07 +0200 Subject: [PATCH 6/7] Only change the log folder on enterprise build --- .../impl/reporter/DefaultBugReporter.kt | 23 ++++--- .../impl/reporter/DefaultBugReporterTest.kt | 67 ++++++++++++++++++- 2 files changed, 78 insertions(+), 12 deletions(-) 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 60b4274689..028b671eb6 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 @@ -93,13 +93,16 @@ class DefaultBugReporter @Inject constructor( private var currentLogDirectory: File = baseLogDirectory init { - val logSubfolder = runBlocking { - sessionStore.getLatestSession() - }?.userId?.substringAfter(":") - setCurrentLogDirectory(logSubfolder) - matrixAuthenticationService.listenToNewMatrixClients { - // When a new Matrix client is created, we update the tracing configuration to write to files - setLogDirectorySubfolder(it.userIdServerName()) + if (buildMeta.isEnterpriseBuild) { + val logSubfolder = runBlocking { + sessionStore.getLatestSession() + }?.userId?.substringAfter(":") + 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()) + } } } @@ -312,8 +315,10 @@ class DefaultBugReporter @Inject constructor( } override fun setLogDirectorySubfolder(subfolderName: String?) { - setCurrentLogDirectory(subfolderName) - tracingService.updateWriteToFilesConfiguration(createWriteToFilesConfiguration()) + if (buildMeta.isEnterpriseBuild) { + setCurrentLogDirectory(subfolderName) + tracingService.updateWriteToFilesConfiguration(createWriteToFilesConfiguration()) + } } private fun setCurrentLogDirectory(subfolderName: String?) { 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 05fcbd26df..c7c424bfae 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 @@ -13,6 +13,7 @@ import io.element.android.features.rageshake.api.reporter.BugReporterListener import io.element.android.features.rageshake.impl.crash.CrashDataStore 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 @@ -305,6 +306,7 @@ class DefaultBugReporterTest { @Test fun `the log directory is initialized using the last session store data`() = runTest { val sut = createDefaultBugReporter( + buildMeta = aBuildMeta(isEnterpriseBuild = true), sessionStore = InMemorySessionStore().apply { storeData(aSessionData(sessionId = "@alice:domain.com")) } @@ -312,6 +314,16 @@ class DefaultBugReporterTest { assertThat(sut.logDirectory().absolutePath).endsWith("/cache/logs/domain.com") } + @Test + fun `foss build - the log directory is initialized to the root log directory`() = runTest { + val sut = createDefaultBugReporter( + sessionStore = InMemorySessionStore().apply { + storeData(aSessionData(sessionId = "@alice:domain.com")) + } + ) + assertThat(sut.logDirectory().absolutePath).endsWith("/cache/logs") + } + @Test fun `when the log directory is updated, the tracing service is invoked`() = runTest { var param: WriteToFilesConfiguration? = null @@ -319,9 +331,10 @@ class DefaultBugReporterTest { param = it } val sut = createDefaultBugReporter( + buildMeta = aBuildMeta(isEnterpriseBuild = true), tracingService = FakeTracingService( updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult, - ) + ), ) sut.setLogDirectorySubfolder("my.sub.folder") updateWriteToFilesConfigurationResult.assertions().isCalledOnce() @@ -333,6 +346,18 @@ class DefaultBugReporterTest { assertThat((param as WriteToFilesConfiguration.Enabled).filenameSuffix).isEqualTo("log") } + @Test + fun `foss build - when the log directory is updated, 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 @@ -340,9 +365,10 @@ class DefaultBugReporterTest { param = it } val sut = createDefaultBugReporter( + buildMeta = aBuildMeta(isEnterpriseBuild = true), tracingService = FakeTracingService( updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult, - ) + ), ) sut.setLogDirectorySubfolder(null) updateWriteToFilesConfigurationResult.assertions().isCalledOnce() @@ -354,6 +380,18 @@ class DefaultBugReporterTest { assertThat((param as WriteToFilesConfiguration.Enabled).filenameSuffix).isEqualTo("log") } + @Test + fun `foss build - when the log directory is reset, the tracing service is not invoked`() = runTest { + val updateWriteToFilesConfigurationResult = lambdaRecorder {} + val sut = createDefaultBugReporter( + tracingService = FakeTracingService( + updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult, + ) + ) + sut.setLogDirectorySubfolder(null) + updateWriteToFilesConfigurationResult.assertions().isNeverCalled() + } + @Test fun `when a new MatrixClient is created the logs folder is updated`() = runTest { var param: WriteToFilesConfiguration? = null @@ -368,6 +406,7 @@ class DefaultBugReporterTest { ) } val sut = createDefaultBugReporter( + buildMeta = aBuildMeta(isEnterpriseBuild = true), matrixAuthenticationService = matrixAuthenticationService, tracingService = FakeTracingService( updateWriteToFilesConfigurationResult = updateWriteToFilesConfigurationResult, @@ -385,7 +424,30 @@ class DefaultBugReporterTest { 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") + updateWriteToFilesConfigurationResult.assertions().isNeverCalled() + } + private fun TestScope.createDefaultBugReporter( + buildMeta: BuildMeta = aBuildMeta(), sessionStore: SessionStore = InMemorySessionStore(), matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider(), crashDataStore: CrashDataStore = FakeCrashDataStore(), @@ -393,7 +455,6 @@ class DefaultBugReporterTest { tracingService: TracingService = FakeTracingService(), matrixAuthenticationService: MatrixAuthenticationService = FakeMatrixAuthenticationService(), ): DefaultBugReporter { - val buildMeta = aBuildMeta() return DefaultBugReporter( context = RuntimeEnvironment.getApplication(), screenshotHolder = FakeScreenshotHolder(), From 5c3054bee43525e8ce207054eb9aaf94a9a9e5af Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Aug 2025 16:04:22 +0200 Subject: [PATCH 7/7] Rename fun. --- .../io/element/android/appnav/di/MatrixSessionCache.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/di/MatrixSessionCache.kt b/appnav/src/main/kotlin/io/element/android/appnav/di/MatrixSessionCache.kt index 0941c7c8cc..1e173474cc 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/di/MatrixSessionCache.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/di/MatrixSessionCache.kt @@ -42,7 +42,7 @@ class MatrixSessionCache @Inject constructor( init { authenticationService.listenToNewMatrixClients { matrixClient -> - onMatrixClient(matrixClient) + onNewMatrixClient(matrixClient) } } @@ -100,14 +100,14 @@ class MatrixSessionCache @Inject constructor( Timber.d("Restore matrix session: $sessionId") return authenticationService.restoreSession(sessionId) .onSuccess { matrixClient -> - onMatrixClient(matrixClient) + onNewMatrixClient(matrixClient) } .onFailure { Timber.e(it, "Fail to restore session") } } - private fun onMatrixClient(matrixClient: MatrixClient) { + private fun onNewMatrixClient(matrixClient: MatrixClient) { val syncOrchestrator = syncOrchestratorFactory.create(matrixClient) sessionIdsToMatrixSession[matrixClient.sessionId] = InMemoryMatrixSession( matrixClient = matrixClient,