diff --git a/core/database/build.gradle.kts b/core/database/build.gradle.kts index ae347b804..1f84e8740 100644 --- a/core/database/build.gradle.kts +++ b/core/database/build.gradle.kts @@ -38,5 +38,6 @@ dependencies { implementation(libs.kotlinx.serialization.json) implementation(libs.timber) + androidTestImplementation(libs.androidx.test.runner) androidTestImplementation(libs.androidx.test.ext.junit) } diff --git a/core/database/src/androidTest/kotlin/org/meshtastic/core/database/DatabaseManagerLegacyCleanupTest.kt b/core/database/src/androidTest/kotlin/org/meshtastic/core/database/DatabaseManagerLegacyCleanupTest.kt new file mode 100644 index 000000000..f55f0f018 --- /dev/null +++ b/core/database/src/androidTest/kotlin/org/meshtastic/core/database/DatabaseManagerLegacyCleanupTest.kt @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2025 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package org.meshtastic.core.database + +import android.app.Application +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class DatabaseManagerLegacyCleanupTest { + @Test + fun deletes_legacy_db_on_switch_when_flag_not_set() = runBlocking { + val app = ApplicationProvider.getApplicationContext() + val prefs = app.getSharedPreferences("db-manager-prefs", Context.MODE_PRIVATE) + + // Reset the one-time flag + prefs.edit().remove(DatabaseConstants.LEGACY_DB_CLEANED_KEY).apply() + + // Ensure legacy DB file exists + val legacyName = DatabaseConstants.LEGACY_DB_NAME + val legacyFile = app.getDatabasePath(legacyName) + // Create or overwrite the legacy DB file by opening it once + app.openOrCreateDatabase(legacyName, Context.MODE_PRIVATE, null).close() + assertTrue("Precondition: legacy DB should exist before switch", legacyFile.exists()) + + val manager = DatabaseManager(app) + + // Switch to a non-null address so active DB != legacy + manager.switchActiveDatabase("01:23:45:67:89:AB") + + // Cleanup runs asynchronously; wait briefly for deletion + var attempts = 0 + while (legacyFile.exists() && attempts < 20) { + delay(100) + attempts++ + } + + assertFalse("Legacy DB should be deleted after switch", legacyFile.exists()) + } +} diff --git a/core/database/src/main/kotlin/org/meshtastic/core/database/DatabaseManager.kt b/core/database/src/main/kotlin/org/meshtastic/core/database/DatabaseManager.kt index 23b380a6c..936e50e13 100644 --- a/core/database/src/main/kotlin/org/meshtastic/core/database/DatabaseManager.kt +++ b/core/database/src/main/kotlin/org/meshtastic/core/database/DatabaseManager.kt @@ -79,6 +79,9 @@ class DatabaseManager @Inject constructor(private val app: Application) { suspend fun switchActiveDatabase(address: String?) = mutex.withLock { val dbName = buildDbName(address) + // Remember the previously active DB name (if any) so we can record its last-used time as well. + val previousDbName = _currentDb.value?.openHelper?.databaseName + // Fast path: no-op if already on this address if (_currentAddress.value == address && _currentDb.value != null) { markLastUsed(dbName) @@ -93,10 +96,16 @@ class DatabaseManager @Inject constructor(private val app: Application) { _currentDb.value = db _currentAddress.value = address markLastUsed(dbName) + // Also mark the previous DB as used "just now" so LRU has an accurate, recent timestamp + // even on first run after upgrade where no timestamp might exist yet. + previousDbName?.let { markLastUsed(it) } // Defer LRU eviction so switch is not blocked by filesystem work managerScope.launch(Dispatchers.IO) { enforceCacheLimit(activeDbName = dbName) } + // One-time cleanup: remove legacy DB if present and not active + managerScope.launch(Dispatchers.IO) { cleanupLegacyDbIfNeeded(activeDbName = dbName) } + Timber.i("Switched active DB to ${anonymizeDbName(dbName)} for address ${anonymizeAddress(address)}") } @@ -126,8 +135,23 @@ class DatabaseManager @Inject constructor(private val app: Application) { private suspend fun enforceCacheLimit(activeDbName: String) = mutex.withLock { val limit = getCacheLimit() val all = listExistingDbNames() - if (all.size <= limit) return - val victims = all.filter { it != activeDbName }.sortedBy { lastUsed(it) }.take(all.size - limit) + // Only enforce the limit over device-specific DBs; exclude legacy and default DBs + val deviceDbs = + all.filterNot { it == DatabaseConstants.LEGACY_DB_NAME || it == DatabaseConstants.DEFAULT_DB_NAME } + Timber.d( + "LRU check: limit=%d, active=%s, deviceDbs=%s", + limit, + anonymizeDbName(activeDbName), + deviceDbs.joinToString(", ") { anonymizeDbName(it) }, + ) + if (deviceDbs.size <= limit) return + val usageSnapshot = deviceDbs.associateWith { lastUsed(it) } + Timber.d( + "LRU lastUsed(ms): %s", + usageSnapshot.entries.joinToString(", ") { (name, ts) -> "${anonymizeDbName(name)}=$ts" }, + ) + val victims = selectEvictionVictims(deviceDbs, activeDbName, limit, usageSnapshot) + Timber.i("LRU victims: %s", victims.joinToString(", ") { anonymizeDbName(it) }) victims.forEach { name -> runCatching { dbCache.remove(name)?.close() } .onFailure { Timber.w(it, "Failed to close database %s", name) } @@ -150,6 +174,28 @@ class DatabaseManager @Inject constructor(private val app: Application) { val active = _currentDb.value?.openHelper?.databaseName ?: defaultDbName() managerScope.launch(Dispatchers.IO) { enforceCacheLimit(activeDbName = active) } } + + private suspend fun cleanupLegacyDbIfNeeded(activeDbName: String) = mutex.withLock { + if (prefs.getBoolean(DatabaseConstants.LEGACY_DB_CLEANED_KEY, false)) return + val legacy = DatabaseConstants.LEGACY_DB_NAME + if (legacy == activeDbName) { + // Never delete the active DB; mark as cleaned to avoid repeated checks + prefs.edit().putBoolean(DatabaseConstants.LEGACY_DB_CLEANED_KEY, true).apply() + return + } + val legacyFile = getDbFile(app, legacy) + if (legacyFile != null) { + runCatching { dbCache.remove(legacy)?.close() } + .onFailure { Timber.w(it, "Failed to close legacy database %s before deletion", legacy) } + val deleted = app.deleteDatabase(legacy) + if (deleted) { + Timber.i("Deleted legacy DB ${anonymizeDbName(legacy)}") + } else { + Timber.w("Attempted to delete legacy DB %s but deleteDatabase returned false", legacy) + } + } + prefs.edit().putBoolean(DatabaseConstants.LEGACY_DB_CLEANED_KEY, true).apply() + } } object DatabaseConstants { @@ -162,6 +208,8 @@ object DatabaseConstants { const val MIN_CACHE_LIMIT: Int = 1 const val MAX_CACHE_LIMIT: Int = 10 + const val LEGACY_DB_CLEANED_KEY: String = "legacy_db_cleaned" + // Display/truncation and hash sizing for DB names const val DB_NAME_HASH_LEN: Int = 10 const val DB_NAME_SEPARATOR_LEN: Int = 1 @@ -175,7 +223,16 @@ object DatabaseConstants { // File-private helpers (kept outside the class to reduce class function count) private fun defaultDbName(): String = DatabaseConstants.DEFAULT_DB_NAME -private fun normalizeAddress(addr: String?): String = addr?.uppercase()?.replace(":", "") ?: "DEFAULT" +private fun normalizeAddress(addr: String?): String { + val u = addr?.trim()?.uppercase() + val normalized = + when { + u.isNullOrBlank() -> "DEFAULT" + u == "N" || u == "NULL" -> "DEFAULT" + else -> u.replace(":", "") + } + return normalized +} private fun shortSha1(s: String): String = MessageDigest.getInstance("SHA-1") .digest(s.toByteArray()) @@ -216,3 +273,37 @@ private fun buildRoomDb(app: Application, dbName: String): MeshtasticDatabase = .build() private fun getDbFile(app: Application, dbName: String): File? = app.getDatabasePath(dbName).takeIf { it.exists() } + +/** + * Compute which DBs to evict using LRU policy. + * + * Rules: + * - Only consider device-specific DBs (exclude legacy and default) + * - Never evict the active DB + * - If number of device DBs is within the limit, evict none + * - Otherwise evict the (size - limit) least-recently-used DBs + * + * Pass a precomputed [lastUsedMsByDb] snapshot to avoid redundant IO/lookups. + */ +internal fun selectEvictionVictims( + dbNames: List, + activeDbName: String, + limit: Int, + lastUsedMsByDb: Map, +): List { + val deviceDbNames = + dbNames.filterNot { it == DatabaseConstants.LEGACY_DB_NAME || it == DatabaseConstants.DEFAULT_DB_NAME } + val victims = + if (limit < 1 || deviceDbNames.size <= limit) { + emptyList() + } else { + val candidates = deviceDbNames.filter { it != activeDbName } + if (candidates.isEmpty()) { + emptyList() + } else { + val toEvict = deviceDbNames.size - limit + candidates.sortedBy { lastUsedMsByDb[it] ?: 0L }.take(toEvict) + } + } + return victims +} diff --git a/core/database/src/test/kotlin/org/meshtastic/core/database/DatabaseManagerEvictionTest.kt b/core/database/src/test/kotlin/org/meshtastic/core/database/DatabaseManagerEvictionTest.kt new file mode 100644 index 000000000..872b3021a --- /dev/null +++ b/core/database/src/test/kotlin/org/meshtastic/core/database/DatabaseManagerEvictionTest.kt @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2025 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package org.meshtastic.core.database + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class DatabaseManagerEvictionTest { + private val a = "meshtastic_database_a111111111" + private val b = "meshtastic_database_b222222222" + private val c = "meshtastic_database_c333333333" + private val d = "meshtastic_database_d444444444" + private val legacy = DatabaseConstants.LEGACY_DB_NAME // "meshtastic_database" + private val defaultDb = DatabaseConstants.DEFAULT_DB_NAME // "meshtastic_database_default" + + @Test + fun `does not evict when count equals limit`() { + val names = listOf(a, b, c) + val victims = + selectEvictionVictims(names, activeDbName = a, limit = 3, lastUsedMsByDb = names.associateWith { 100L }) + assertTrue(victims.isEmpty()) + } + + @Test + fun `never evicts active even if oldest`() { + val names = listOf(a, b, c, d) + val lastUsed = mapOf(a to 1L, b to 2L, c to 3L, d to 4L) + val victims = selectEvictionVictims(names, activeDbName = a, limit = 3, lastUsedMsByDb = lastUsed) + // Oldest overall is a, but active must not be evicted -> next oldest is b + assertEquals(listOf(b), victims) + } + + @Test + fun `evicts two oldest when over limit by two`() { + val names = listOf(a, b, c, d) + val lastUsed = mapOf(a to 10L, b to 20L, c to 30L, d to 40L) + val victims = selectEvictionVictims(names, activeDbName = d, limit = 2, lastUsedMsByDb = lastUsed) + // Need to evict 2; oldest are a, then b + assertEquals(listOf(a, b), victims) + } + + @Test + fun `excludes legacy and default from accounting`() { + val names = listOf(a, b, legacy, defaultDb) + val lastUsed = mapOf(a to 10L, b to 5L) + val victims = selectEvictionVictims(names, activeDbName = a, limit = 1, lastUsedMsByDb = lastUsed) + // Only device DBs a & b are counted; with limit 1 and active=a, evict b + assertEquals(listOf(b), victims) + } +}