diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/flightrecorder/FlightRecorderManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/flightrecorder/FlightRecorderManagerImpl.kt index f3b58b6355..182fe3e18d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/flightrecorder/FlightRecorderManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/flightrecorder/FlightRecorderManagerImpl.kt @@ -23,6 +23,7 @@ import timber.log.Timber import java.time.Clock import java.time.temporal.ChronoUnit import java.util.UUID +import java.util.concurrent.ConcurrentHashMap private const val EXPIRATION_DURATION_DAYS: Long = 30 @@ -39,6 +40,7 @@ internal class FlightRecorderManagerImpl( private val unconfinedScope = CoroutineScope(context = dispatcherManager.unconfined) private val ioScope = CoroutineScope(context = dispatcherManager.io) private var cancellationJob: Job = Job().apply { complete() } + private val expirationJobMap: ConcurrentHashMap = ConcurrentHashMap() private val flightRecorderTree = FlightRecorderTree() override val flightRecorderData: FlightRecorderDataSet @@ -58,7 +60,10 @@ internal class FlightRecorderManagerImpl( // Always plant the tree, it will not log anything if the flight recorder is off. Timber.plant(flightRecorderTree) flightRecorderDataFlow - .onEach { it.configureFlightRecorder() } + .onEach { + it.configureFlightRecorder() + it.configureExpirationJobs() + } .launchIn(scope = unconfinedScope) context.registerReceiver( ScreenStateBroadcastReceiver(), @@ -119,6 +124,7 @@ internal class FlightRecorderManagerImpl( data = originalData.data.filterNot { it == data }.toSet(), ) ioScope.launch { flightRecorderWriter.deleteLog(data = data) } + expirationJobMap.remove(data.id)?.cancel() } private fun cancelCancellationJob() { @@ -156,6 +162,27 @@ internal class FlightRecorderManagerImpl( } } + /** + * Configure the expiration jobs based on the [FlightRecorderDataSet]. + * + * This cancels each expiration job and rebuilds it with the up-to-date info. + */ + private fun FlightRecorderDataSet.configureExpirationJobs() { + this + .data + .filterNot { it.isActive } + .onEach { + expirationJobMap.remove(it.id)?.cancel() + expirationJobMap[it.id] = ioScope.launch { + // If an inactive job does not have an expiration time, something has gone + // wrong and we just delete it. + val delay = (it.expirationTimeMs ?: 0L) - clock.instant().toEpochMilli() + delay(timeMillis = delay) + deleteLog(data = it) + } + } + } + private inner class FlightRecorderTree : Timber.Tree() { var flightRecorderData: FlightRecorderDataSet.FlightRecorderData? = null set(value) { @@ -182,7 +209,7 @@ internal class FlightRecorderManagerImpl( /** * A custom [BroadcastReceiver] that listens for when the screen is powered on and restarts the - * cancellation job to ensure they complete at the correct time. + * cancellation job and expiration jobs to ensure they complete at the correct time. * * This is necessary because the [delay] function in a coroutine will not keep accurate time * when the screen is off. We do not cancel the job when the screen is off, this allows the @@ -191,6 +218,7 @@ internal class FlightRecorderManagerImpl( private inner class ScreenStateBroadcastReceiver : BroadcastReceiver() { override fun onReceive(context: Context, intent: Intent) { flightRecorderData.configureFlightRecorder() + flightRecorderData.configureExpirationJobs() } } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/flightrecorder/FlightRecorderManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/flightrecorder/FlightRecorderManagerTest.kt index ca4284b82f..c131f2cab7 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/flightrecorder/FlightRecorderManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/flightrecorder/FlightRecorderManagerTest.kt @@ -78,9 +78,21 @@ class FlightRecorderManagerTest { fakeSettingsDiskSource.flightRecorderData = null assertEquals(FlightRecorderDataSet(data = emptySet()), flightRecorder.flightRecorderData) - val expected = mockk { - every { activeFlightRecorderData } returns null - } + val expected = FlightRecorderDataSet( + data = setOf( + FlightRecorderDataSet.FlightRecorderData( + id = "mockUUID", + fileName = "flight_recorder_2023-10-27_12-00-00", + startTimeMs = FIXED_CLOCK_TIME, + durationMs = FlightRecorderDuration.ONE_HOUR.milliseconds, + isActive = false, + expirationTimeMs = FIXED_CLOCK + .instant() + .plus(1L, ChronoUnit.HOURS) + .toEpochMilli(), + ), + ), + ) fakeSettingsDiskSource.flightRecorderData = expected assertEquals(expected, flightRecorder.flightRecorderData) } @@ -93,7 +105,7 @@ class FlightRecorderManagerTest { .test { assertEquals(FlightRecorderDataSet(data = emptySet()), awaitItem()) - val expected = FlightRecorderDataSet( + val unexpired = FlightRecorderDataSet( data = setOf( FlightRecorderDataSet.FlightRecorderData( id = "mockUUID", @@ -101,14 +113,36 @@ class FlightRecorderManagerTest { startTimeMs = FIXED_CLOCK_TIME, durationMs = FlightRecorderDuration.ONE_HOUR.milliseconds, isActive = false, + expirationTimeMs = FIXED_CLOCK + .instant() + .plus(1L, ChronoUnit.HOURS) + .toEpochMilli(), ), ), ) - fakeSettingsDiskSource.flightRecorderData = expected - assertEquals(expected, awaitItem()) + fakeSettingsDiskSource.flightRecorderData = unexpired + assertEquals(unexpired, awaitItem()) + + val expired = FlightRecorderDataSet( + data = setOf( + FlightRecorderDataSet.FlightRecorderData( + id = "mockUUID", + fileName = "flight_recorder_2023-10-27_12-00-00", + startTimeMs = FIXED_CLOCK_TIME, + durationMs = FlightRecorderDuration.ONE_HOUR.milliseconds, + isActive = false, + expirationTimeMs = FIXED_CLOCK + .instant() + .minus(1L, ChronoUnit.HOURS) + .toEpochMilli(), + ), + ), + ) + fakeSettingsDiskSource.flightRecorderData = expired + assertEquals(FlightRecorderDataSet(data = emptySet()), awaitItem()) fakeSettingsDiskSource.flightRecorderData = null - assertEquals(FlightRecorderDataSet(data = emptySet()), awaitItem()) + expectNoEvents() } } @@ -152,7 +186,10 @@ class FlightRecorderManagerTest { startTimeMs = FIXED_CLOCK_TIME, durationMs = 60L, isActive = false, - expirationTimeMs = null, + expirationTimeMs = FIXED_CLOCK + .instant() + .plus(30, ChronoUnit.DAYS) + .toEpochMilli(), ), ), ) @@ -198,6 +235,7 @@ class FlightRecorderManagerTest { startTimeMs = FIXED_CLOCK_TIME, durationMs = 60L, isActive = false, + expirationTimeMs = FIXED_CLOCK.instant().plus(1L, ChronoUnit.HOURS).toEpochMilli(), ) val activeData = FlightRecorderDataSet.FlightRecorderData( id = "50", @@ -205,6 +243,7 @@ class FlightRecorderManagerTest { startTimeMs = FIXED_CLOCK_TIME, durationMs = 60L, isActive = true, + expirationTimeMs = null, ) val activeDataset = FlightRecorderDataSet(data = setOf(activeData)) val inactiveDataset = FlightRecorderDataSet(data = setOf(inactiveData)) @@ -249,6 +288,7 @@ class FlightRecorderManagerTest { startTimeMs = FIXED_CLOCK_TIME, durationMs = 60L, isActive = false, + expirationTimeMs = FIXED_CLOCK.instant().plus(1L, ChronoUnit.HOURS).toEpochMilli(), ) fakeSettingsDiskSource.flightRecorderData = FlightRecorderDataSet(data = setOf(data))