Simplifying discardOldestFileIfNeeded function

bugsnag · Sep 20, 2024 · 3aabb9e · 3aabb9e
1 parent 395e3f7
commit 3aabb9e
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 27 deletions.
diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/FileStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/FileStoreTest.kt
@@ -2,13 +2,11 @@ package com.bugsnag.android
 
 import android.app.Application
 import androidx.test.core.app.ApplicationProvider
-import com.bugsnag.android.EventStore.Companion.EVENT_COMPARATOR
 import org.junit.Assert.assertEquals
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.junit.MockitoJUnitRunner
 import java.io.File
-import java.util.Comparator
 
 @RunWith(MockitoJUnitRunner::class)
 class FileStoreTest {
@@ -18,7 +16,7 @@ class FileStoreTest {
 
         val delegate = CustomDelegate()
         val dir = File(ApplicationProvider.getApplicationContext<Application>().cacheDir, "tmp")
-        val store = CustomFileStore(dir, 1, EVENT_COMPARATOR, delegate)
+        val store = CustomFileStore(dir, 1, delegate)
         val exc = RuntimeException("Whoops")
         store.write(CustomStreamable(exc))
 
@@ -49,8 +47,7 @@ class CustomStreamable(private val exc: Throwable) : JsonStream.Streamable {
 internal class CustomFileStore(
     folder: File,
     maxStoreCount: Int,
-    comparator: Comparator<in File?>,
     delegate: Delegate?
-) : FileStore(folder, maxStoreCount, comparator, NoopLogger, delegate) {
+) : FileStore(folder, maxStoreCount, NoopLogger, delegate) {
     override fun getFilename(obj: Any?) = "foo.json"
 }
diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.kt
@@ -33,7 +33,6 @@ internal class EventStore(
 ) : FileStore(
     File(config.persistenceDirectory.value, "bugsnag/errors"),
     config.maxPersistedEvents,
-    EVENT_COMPARATOR,
     logger,
     delegate
 ) {

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt
@@ -7,16 +7,13 @@ import java.io.FileNotFoundException
 import java.io.FileOutputStream
 import java.io.OutputStreamWriter
 import java.io.Writer
-import java.util.Collections
-import java.util.Comparator
 import java.util.concurrent.ConcurrentSkipListSet
 import java.util.concurrent.locks.Lock
 import java.util.concurrent.locks.ReentrantLock
 
 internal abstract class FileStore(
     val storageDir: File,
     private val maxStoreCount: Int,
-    private val comparator: Comparator<in File?>,
     protected open val logger: Logger,
     protected val delegate: Delegate?
 ) {
@@ -115,23 +112,21 @@ internal abstract class FileStore(
         // Limit number of saved payloads to prevent disk space issues
         if (isStorageDirValid(storageDir)) {
             val listFiles = storageDir.listFiles() ?: return
-            val files: ArrayList<File> = arrayListOf(*listFiles)
-            if (files.size >= maxStoreCount) {
-                // Sort files then delete the first one (oldest timestamp)
-                Collections.sort(files, comparator)
-                var k = 0
-                while (k < files.size && files.size >= maxStoreCount) {
-                    val oldestFile = files[k]
-                    if (!queuedFiles.contains(oldestFile)) {
-                        logger.w(
-                            "Discarding oldest error as stored " +
-                                "error limit reached: '" + oldestFile.path + '\''
-                        )
-                        deleteStoredFiles(setOf(oldestFile))
-                        files.removeAt(k)
-                        k--
-                    }
-                    k++
+            if (listFiles.size < maxStoreCount) return
+            val sortedListFiles = listFiles.sortedBy { it.lastModified() }
+            // Number of files to discard takes into account that a new file may need to be written
+            val numberToDiscard = listFiles.size - maxStoreCount + 1
+            var discardedCount = 0
+            for (file in sortedListFiles) {
+                if (discardedCount == numberToDiscard) {
+                    return
+                } else if (!queuedFiles.contains(file)) {
+                    logger.w(
+                        "Discarding oldest error as stored error limit reached: '" +
+                            file.path + '\''
+                    )
+                    deleteStoredFiles(setOf(file))
+                    discardedCount++
                 }
             }
         }

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt
@@ -21,7 +21,6 @@ internal class SessionStore(
         config.persistenceDirectory.value, "bugsnag/sessions"
     ),
     config.maxPersistedSessions,
-    SESSION_COMPARATOR,
     logger,
     delegate
 ) {