diff --git a/features/migration/impl/build.gradle.kts b/features/migration/impl/build.gradle.kts index 6978975070..69a1c2bfe9 100644 --- a/features/migration/impl/build.gradle.kts +++ b/features/migration/impl/build.gradle.kts @@ -40,6 +40,7 @@ dependencies { testImplementation(libs.test.robolectric) testImplementation(libs.test.truth) testImplementation(libs.test.turbine) + testImplementation(projects.libraries.matrix.test) testImplementation(projects.libraries.sessionStorage.implMemory) testImplementation(projects.libraries.sessionStorage.test) testImplementation(projects.libraries.preferences.test) diff --git a/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/DefaultMigrationStore.kt b/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/DefaultMigrationStore.kt index a0158061e2..3a0c211421 100644 --- a/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/DefaultMigrationStore.kt +++ b/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/DefaultMigrationStore.kt @@ -46,7 +46,7 @@ class DefaultMigrationStore @Inject constructor( override fun applicationMigrationVersion(): Flow { return store.data.map { prefs -> - prefs[applicationMigrationVersion] ?: 0 + prefs[applicationMigrationVersion] ?: -1 } } } diff --git a/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/MigrationPresenter.kt b/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/MigrationPresenter.kt index d157566e24..4c452b688a 100644 --- a/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/MigrationPresenter.kt +++ b/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/MigrationPresenter.kt @@ -53,6 +53,12 @@ class MigrationPresenter @Inject constructor( LaunchedEffect(migrationStoreVersion) { val migrationValue = migrationStoreVersion ?: return@LaunchedEffect + if (migrationValue == -1) { + // Fresh install, no migration needed + Timber.d("Fresh install, no migration needed.") + migrationStore.setApplicationMigrationVersion(lastMigration) + return@LaunchedEffect + } if (migrationValue == lastMigration) { Timber.d("Current app migration version: $migrationValue. No migration needed.") migrationAction = AsyncData.Success(Unit) diff --git a/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/migrations/AppMigration07.kt b/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/migrations/AppMigration07.kt new file mode 100644 index 0000000000..c0da439f9b --- /dev/null +++ b/features/migration/impl/src/main/kotlin/io/element/android/features/migration/impl/migrations/AppMigration07.kt @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.migration.impl.migrations + +import com.squareup.anvil.annotations.ContributesMultibinding +import io.element.android.features.rageshake.api.logs.LogFilesRemover +import io.element.android.libraries.di.AppScope +import javax.inject.Inject + +/** + * Delete the previous log files. + */ +@ContributesMultibinding(AppScope::class) +class AppMigration07 @Inject constructor( + private val logFilesRemover: LogFilesRemover, +) : AppMigration { + override val order: Int = 7 + + override suspend fun migrate() { + logFilesRemover.perform { file -> + file.name.startsWith("logs-") + } + } +} diff --git a/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/MigrationPresenterTest.kt b/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/MigrationPresenterTest.kt index 29be8682e3..26cb0df3c1 100644 --- a/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/MigrationPresenterTest.kt +++ b/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/MigrationPresenterTest.kt @@ -35,6 +35,32 @@ class MigrationPresenterTest { @get:Rule val warmUpRule = WarmUpRule() + @Test + fun `present - no migration should occurs on fresh installation, and last version should be stored`() = runTest { + val migrations = (1..10).map { order -> + FakeAppMigration( + order = order, + migrateLambda = LambdaNoParamRecorder(ensureNeverCalled = true) { }, + ) + } + val store = InMemoryMigrationStore(initialApplicationMigrationVersion = -1) + val presenter = createPresenter( + migrationStore = store, + migrations = migrations.toSet(), + ) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitItem() + assertThat(initialState.migrationAction).isEqualTo(AsyncData.Uninitialized) + skipItems(1) + awaitItem().also { state -> + assertThat(state.migrationAction).isEqualTo(AsyncData.Success(Unit)) + } + assertThat(store.applicationMigrationVersion().first()).isEqualTo(migrations.maxOf { it.order }) + } + } + @Test fun `present - no migration should occurs if ApplicationMigrationVersion is the last one`() = runTest { val migrations = (1..10).map { FakeAppMigration(it) } @@ -89,7 +115,7 @@ private fun createPresenter( private class FakeAppMigration( override val order: Int, - var migrateLambda: LambdaNoParamRecorder = lambdaRecorder { -> }, + val migrateLambda: LambdaNoParamRecorder = lambdaRecorder { -> }, ) : AppMigration { override suspend fun migrate() { migrateLambda() diff --git a/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration05Test.kt b/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration05Test.kt new file mode 100644 index 0000000000..dd2e969364 --- /dev/null +++ b/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration05Test.kt @@ -0,0 +1,59 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.migration.impl.migrations + +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.matrix.test.A_SESSION_ID +import io.element.android.libraries.sessionstorage.impl.memory.InMemorySessionStore +import io.element.android.libraries.sessionstorage.test.aSessionData +import kotlinx.coroutines.test.runTest +import org.junit.Test +import java.io.File + +class AppMigration05Test { + @Test + fun `empty session path should be set to an expected path`() = runTest { + val sessionStore = InMemorySessionStore().apply { + updateData( + aSessionData( + sessionId = A_SESSION_ID, + sessionPath = "", + ) + ) + } + val migration = AppMigration05(sessionStore = sessionStore, baseDirectory = File("/a/path")) + migration.migrate() + val storedData = sessionStore.getSession(A_SESSION_ID.value)!! + assertThat(storedData.sessionPath).isEqualTo("/a/path/${A_SESSION_ID.value.replace(':', '_')}") + } + + @Test + fun `non empty session path should not be impacted by the migration`() = runTest { + val sessionStore = InMemorySessionStore().apply { + updateData( + aSessionData( + sessionId = A_SESSION_ID, + sessionPath = "/a/path/existing", + ) + ) + } + val migration = AppMigration05(sessionStore = sessionStore, baseDirectory = File("/a/path")) + migration.migrate() + val storedData = sessionStore.getSession(A_SESSION_ID.value)!! + assertThat(storedData.sessionPath).isEqualTo("/a/path/existing") + } +} diff --git a/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration06Test.kt b/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration06Test.kt new file mode 100644 index 0000000000..be5a8879de --- /dev/null +++ b/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration06Test.kt @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.migration.impl.migrations + +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.matrix.test.A_SESSION_ID +import io.element.android.libraries.sessionstorage.impl.memory.InMemorySessionStore +import io.element.android.libraries.sessionstorage.test.aSessionData +import kotlinx.coroutines.test.runTest +import org.junit.Test +import java.io.File + +class AppMigration06Test { + @Test + fun `empty cache path should be set to an expected path`() = runTest { + val sessionStore = InMemorySessionStore().apply { + updateData( + aSessionData( + sessionId = A_SESSION_ID, + sessionPath = "/a/path/to/a/session/AN_ID", + cachePath = "", + ) + ) + } + val migration = AppMigration06(sessionStore = sessionStore, cacheDirectory = File("/a/path/cache")) + migration.migrate() + val storedData = sessionStore.getSession(A_SESSION_ID.value)!! + assertThat(storedData.cachePath).isEqualTo("/a/path/cache/AN_ID") + } + + @Test + fun `non empty cache path should not be impacted by the migration`() = runTest { + val sessionStore = InMemorySessionStore().apply { + updateData( + aSessionData( + sessionId = A_SESSION_ID, + cachePath = "/a/path/existing", + ) + ) + } + val migration = AppMigration05(sessionStore = sessionStore, baseDirectory = File("/a/path/cache")) + migration.migrate() + val storedData = sessionStore.getSession(A_SESSION_ID.value)!! + assertThat(storedData.cachePath).isEqualTo("/a/path/existing") + } +} diff --git a/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration07Test.kt b/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration07Test.kt new file mode 100644 index 0000000000..95816b898f --- /dev/null +++ b/features/migration/impl/src/test/kotlin/io/element/android/features/migration/impl/migrations/AppMigration07Test.kt @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.migration.impl.migrations + +import com.google.common.truth.Truth.assertThat +import io.element.android.features.rageshake.test.logs.FakeLogFilesRemover +import io.element.android.tests.testutils.lambda.lambdaRecorder +import kotlinx.coroutines.test.runTest +import org.junit.Test +import java.io.File + +class AppMigration07Test { + @Test + fun `test migration`() = runTest { + val performLambda = lambdaRecorder<(File) -> Boolean, Unit> { predicate -> + // Test the predicate + assertThat(predicate(File("logs-0433.log.gz"))).isTrue() + assertThat(predicate(File("logs.2024-08-01-20.log.gz"))).isFalse() + } + val logsFileRemover = FakeLogFilesRemover(performLambda = performLambda) + val migration = AppMigration07(logsFileRemover) + migration.migrate() + performLambda.assertions().isCalledOnce() + } +} diff --git a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/logs/LogFilesRemover.kt b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/logs/LogFilesRemover.kt index dd73133060..2b1fa29261 100644 --- a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/logs/LogFilesRemover.kt +++ b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/logs/LogFilesRemover.kt @@ -16,6 +16,12 @@ package io.element.android.features.rageshake.api.logs +import java.io.File + interface LogFilesRemover { - suspend fun perform() + /** + * Perform the log files removal. + * @param predicate a predicate to filter the files to remove. By default, all files are removed. + */ + suspend fun perform(predicate: (File) -> Boolean = { true }) } diff --git a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/logs/DefaultLogFilesRemover.kt b/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/logs/DefaultLogFilesRemover.kt index c0c7c8c346..dd551d517e 100644 --- a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/logs/DefaultLogFilesRemover.kt +++ b/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/logs/DefaultLogFilesRemover.kt @@ -20,13 +20,14 @@ import com.squareup.anvil.annotations.ContributesBinding import io.element.android.features.rageshake.api.logs.LogFilesRemover import io.element.android.features.rageshake.impl.reporter.DefaultBugReporter import io.element.android.libraries.di.AppScope +import java.io.File import javax.inject.Inject @ContributesBinding(AppScope::class) class DefaultLogFilesRemover @Inject constructor( private val bugReporter: DefaultBugReporter, ) : LogFilesRemover { - override suspend fun perform() { - bugReporter.deleteAllFiles() + override suspend fun perform(predicate: (File) -> Boolean) { + bugReporter.deleteAllFiles(predicate) } } 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 72e668c3f4..925577ce77 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 @@ -301,9 +301,11 @@ class DefaultBugReporter @Inject constructor( } } - suspend fun deleteAllFiles() { + suspend fun deleteAllFiles(predicate: (File) -> Boolean) { withContext(coroutineDispatchers.io) { - getLogFiles().forEach { it.safeDelete() } + getLogFiles() + .filter(predicate) + .forEach { it.safeDelete() } } } diff --git a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/BugReportPresenterTest.kt b/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/BugReportPresenterTest.kt index 9b07b1c942..0863962f6d 100644 --- a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/BugReportPresenterTest.kt +++ b/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/BugReportPresenterTest.kt @@ -32,7 +32,7 @@ import io.element.android.features.rageshake.test.screenshot.FakeScreenshotHolde import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.matrix.test.A_FAILURE_REASON import io.element.android.tests.testutils.WarmUpRule -import io.element.android.tests.testutils.lambda.lambdaRecorder +import io.element.android.tests.testutils.lambda.LambdaOneParamRecorder import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.junit.Rule @@ -120,11 +120,11 @@ class BugReportPresenterTest { @Test fun `present - reset all`() = runTest { - val logFilesRemoverLambda = lambdaRecorder { -> } + val logFilesRemover = FakeLogFilesRemover() val presenter = createPresenter( crashDataStore = FakeCrashDataStore(crashData = A_CRASH_DATA, appHasCrashed = true), screenshotHolder = FakeScreenshotHolder(screenshotUri = A_SCREENSHOT_URI), - logFilesRemover = FakeLogFilesRemover(logFilesRemoverLambda), + logFilesRemover = logFilesRemover, ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() @@ -136,7 +136,7 @@ class BugReportPresenterTest { initialState.eventSink.invoke(BugReportEvents.ResetAll) val resetState = awaitItem() assertThat(resetState.hasCrashLogs).isFalse() - logFilesRemoverLambda.assertions().isCalledOnce() + logFilesRemover.performLambda.assertions().isCalledOnce() // TODO Make it live assertThat(resetState.screenshotUri).isNull() } } @@ -245,7 +245,7 @@ class BugReportPresenterTest { bugReporter: BugReporter = FakeBugReporter(), crashDataStore: CrashDataStore = FakeCrashDataStore(), screenshotHolder: ScreenshotHolder = FakeScreenshotHolder(), - logFilesRemover: LogFilesRemover = FakeLogFilesRemover(lambdaRecorder(ensureNeverCalled = true) { -> }), + logFilesRemover: LogFilesRemover = FakeLogFilesRemover(LambdaOneParamRecorder(ensureNeverCalled = true) { }), ) = BugReportPresenter( bugReporter = bugReporter, crashDataStore = crashDataStore, diff --git a/features/rageshake/test/src/main/kotlin/io/element/android/features/rageshake/test/logs/FakeLogFilesRemover.kt b/features/rageshake/test/src/main/kotlin/io/element/android/features/rageshake/test/logs/FakeLogFilesRemover.kt index a416f2eeb4..0999a76293 100644 --- a/features/rageshake/test/src/main/kotlin/io/element/android/features/rageshake/test/logs/FakeLogFilesRemover.kt +++ b/features/rageshake/test/src/main/kotlin/io/element/android/features/rageshake/test/logs/FakeLogFilesRemover.kt @@ -17,13 +17,14 @@ package io.element.android.features.rageshake.test.logs import io.element.android.features.rageshake.api.logs.LogFilesRemover -import io.element.android.tests.testutils.lambda.LambdaNoParamRecorder +import io.element.android.tests.testutils.lambda.LambdaOneParamRecorder import io.element.android.tests.testutils.lambda.lambdaRecorder +import java.io.File class FakeLogFilesRemover( - var performLambda: LambdaNoParamRecorder = lambdaRecorder { -> }, + val performLambda: LambdaOneParamRecorder<(File) -> Boolean, Unit> = lambdaRecorder<(File) -> Boolean, Unit> { }, ) : LogFilesRemover { - override suspend fun perform() { - performLambda() + override suspend fun perform(predicate: (File) -> Boolean) { + performLambda(predicate) } } diff --git a/libraries/session-storage/test/src/main/kotlin/io/element/android/libraries/sessionstorage/test/SessionData.kt b/libraries/session-storage/test/src/main/kotlin/io/element/android/libraries/sessionstorage/test/SessionData.kt index c33ab78d61..659d59e8ec 100644 --- a/libraries/session-storage/test/src/main/kotlin/io/element/android/libraries/sessionstorage/test/SessionData.kt +++ b/libraries/session-storage/test/src/main/kotlin/io/element/android/libraries/sessionstorage/test/SessionData.kt @@ -23,6 +23,8 @@ import io.element.android.libraries.sessionstorage.api.SessionData fun aSessionData( sessionId: SessionId = SessionId("@alice:server.org"), isTokenValid: Boolean = false, + sessionPath: String = "/a/path/to/a/session", + cachePath: String = "/a/path/to/a/cache", ): SessionData { return SessionData( userId = sessionId.value, @@ -36,7 +38,7 @@ fun aSessionData( isTokenValid = isTokenValid, loginType = LoginType.UNKNOWN, passphrase = null, - sessionPath = "/a/path/to/a/session", - cachePath = "/a/path/to/a/cache", + sessionPath = sessionPath, + cachePath = cachePath, ) }