PM-19620: Auto-delete flight recorder logs after 30 days (#5062)

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<String, Job> = 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()
         }
     }
 }

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<FlightRecorderDataSet> {
-            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))