mirror of
https://github.com/meshtastic/Meshtastic-Android.git
synced 2026-07-13 06:42:23 -04:00
fix(database): make the injected DiscoveryDao follow the active database (#6236)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,131 @@
|
||||
/*
|
||||
* Copyright (c) 2026 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 <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
package org.meshtastic.core.database.dao
|
||||
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.flatMapLatest
|
||||
import org.meshtastic.core.database.DatabaseProvider
|
||||
import org.meshtastic.core.database.entity.DiscoveredNodeEntity
|
||||
import org.meshtastic.core.database.entity.DiscoveryPresetResultEntity
|
||||
import org.meshtastic.core.database.entity.DiscoverySessionEntity
|
||||
|
||||
/**
|
||||
* A switch-aware [DiscoveryDao] that resolves the active database on every call instead of pinning the one that was
|
||||
* current at injection time. This is what Koin hands to `feature:discovery` consumers (ViewModels and the scan engine),
|
||||
* which hold their DAO for their whole lifetime:
|
||||
* - Flow methods re-latch via `currentDb.flatMapLatest`, so an open discovery screen follows a device/DB switch instead
|
||||
* of watching the old database forever.
|
||||
* - Suspend methods go through [DatabaseProvider.withDb], so writes register with the cross-transport merge drain
|
||||
* barrier (a mid-scan merge can't snapshot-then-retire the DB underneath an in-flight session write) and every call
|
||||
* picks up withDb's closed-pool retry — a pinned DAO used to throw unrecoverably once its DB was retired.
|
||||
*
|
||||
* `withDb` only returns null when no database is open, which [DatabaseProvider] guarantees can't happen (the default DB
|
||||
* is the floor), so the non-null coercions below are structural, not behavioral.
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
@Suppress("TooManyFunctions")
|
||||
class SwitchingDiscoveryDao(private val dbManager: DatabaseProvider) : DiscoveryDao {
|
||||
|
||||
// region Session operations
|
||||
|
||||
override suspend fun insertSession(session: DiscoverySessionEntity): Long =
|
||||
checkNotNull(dbManager.withDb { it.discoveryDao().insertSession(session) })
|
||||
|
||||
override suspend fun updateSession(session: DiscoverySessionEntity) {
|
||||
dbManager.withDb { it.discoveryDao().updateSession(session) }
|
||||
}
|
||||
|
||||
override fun getAllSessions(): Flow<List<DiscoverySessionEntity>> =
|
||||
dbManager.currentDb.flatMapLatest { it.discoveryDao().getAllSessions() }
|
||||
|
||||
override suspend fun getAllSessionsSnapshot(): List<DiscoverySessionEntity> =
|
||||
dbManager.withDb { it.discoveryDao().getAllSessionsSnapshot() }.orEmpty()
|
||||
|
||||
override suspend fun getSession(sessionId: Long): DiscoverySessionEntity? =
|
||||
dbManager.withDb { it.discoveryDao().getSession(sessionId) }
|
||||
|
||||
override fun getSessionFlow(sessionId: Long): Flow<DiscoverySessionEntity?> =
|
||||
dbManager.currentDb.flatMapLatest { it.discoveryDao().getSessionFlow(sessionId) }
|
||||
|
||||
override suspend fun deleteSession(sessionId: Long) {
|
||||
dbManager.withDb { it.discoveryDao().deleteSession(sessionId) }
|
||||
}
|
||||
|
||||
override suspend fun markInterruptedSessions() {
|
||||
dbManager.withDb { it.discoveryDao().markInterruptedSessions() }
|
||||
}
|
||||
|
||||
override suspend fun getInterruptedSession(deviceAddress: String): DiscoverySessionEntity? =
|
||||
dbManager.withDb { it.discoveryDao().getInterruptedSession(deviceAddress) }
|
||||
|
||||
// endregion
|
||||
|
||||
// region Preset result operations
|
||||
|
||||
override suspend fun insertPresetResult(result: DiscoveryPresetResultEntity): Long =
|
||||
checkNotNull(dbManager.withDb { it.discoveryDao().insertPresetResult(result) })
|
||||
|
||||
override suspend fun updatePresetResult(result: DiscoveryPresetResultEntity) {
|
||||
dbManager.withDb { it.discoveryDao().updatePresetResult(result) }
|
||||
}
|
||||
|
||||
override suspend fun getPresetResults(sessionId: Long): List<DiscoveryPresetResultEntity> =
|
||||
dbManager.withDb { it.discoveryDao().getPresetResults(sessionId) }.orEmpty()
|
||||
|
||||
override fun getPresetResultsFlow(sessionId: Long): Flow<List<DiscoveryPresetResultEntity>> =
|
||||
dbManager.currentDb.flatMapLatest { it.discoveryDao().getPresetResultsFlow(sessionId) }
|
||||
|
||||
// endregion
|
||||
|
||||
// region Discovered node operations
|
||||
|
||||
override suspend fun insertDiscoveredNode(node: DiscoveredNodeEntity): Long =
|
||||
checkNotNull(dbManager.withDb { it.discoveryDao().insertDiscoveredNode(node) })
|
||||
|
||||
override suspend fun insertDiscoveredNodes(nodes: List<DiscoveredNodeEntity>) {
|
||||
dbManager.withDb { it.discoveryDao().insertDiscoveredNodes(nodes) }
|
||||
}
|
||||
|
||||
override suspend fun updateDiscoveredNode(node: DiscoveredNodeEntity) {
|
||||
dbManager.withDb { it.discoveryDao().updateDiscoveredNode(node) }
|
||||
}
|
||||
|
||||
override suspend fun getDiscoveredNodes(presetResultId: Long): List<DiscoveredNodeEntity> =
|
||||
dbManager.withDb { it.discoveryDao().getDiscoveredNodes(presetResultId) }.orEmpty()
|
||||
|
||||
override fun getDiscoveredNodesFlow(presetResultId: Long): Flow<List<DiscoveredNodeEntity>> =
|
||||
dbManager.currentDb.flatMapLatest { it.discoveryDao().getDiscoveredNodesFlow(presetResultId) }
|
||||
|
||||
override suspend fun getUniqueNodeNums(sessionId: Long): List<Long> =
|
||||
dbManager.withDb { it.discoveryDao().getUniqueNodeNums(sessionId) }.orEmpty()
|
||||
|
||||
// endregion
|
||||
|
||||
// region Aggregate queries
|
||||
|
||||
override suspend fun getUniqueNodeCount(sessionId: Long): Int =
|
||||
dbManager.withDb { it.discoveryDao().getUniqueNodeCount(sessionId) } ?: 0
|
||||
|
||||
override suspend fun getMaxDistance(sessionId: Long): Double? =
|
||||
dbManager.withDb { it.discoveryDao().getMaxDistance(sessionId) }
|
||||
|
||||
override suspend fun getSessionWithResults(sessionId: Long): DiscoverySessionEntity? =
|
||||
dbManager.withDb { it.discoveryDao().getSessionWithResults(sessionId) }
|
||||
|
||||
// endregion
|
||||
}
|
||||
@@ -24,6 +24,7 @@ import org.koin.core.annotation.Single
|
||||
import org.meshtastic.core.database.DatabaseProvider
|
||||
import org.meshtastic.core.database.createDatabaseDataStore
|
||||
import org.meshtastic.core.database.dao.DiscoveryDao
|
||||
import org.meshtastic.core.database.dao.SwitchingDiscoveryDao
|
||||
|
||||
@Module
|
||||
@ComponentScan("org.meshtastic.core.database")
|
||||
@@ -32,7 +33,11 @@ class CoreDatabaseModule {
|
||||
@Named("DatabaseDataStore")
|
||||
fun provideDatabaseDataStore() = createDatabaseDataStore("db-manager-prefs")
|
||||
|
||||
/**
|
||||
* Long-lived consumers (discovery ViewModels, the scan engine) hold this DAO across device/DB switches, so hand
|
||||
* them the switch-aware delegate — never a DAO pinned to the injection-time `currentDb.value`, which would keep
|
||||
* reading a stale DB and crash once a cross-transport merge retires it. See [SwitchingDiscoveryDao].
|
||||
*/
|
||||
@Factory
|
||||
fun provideDiscoveryDao(databaseProvider: DatabaseProvider): DiscoveryDao =
|
||||
databaseProvider.currentDb.value.discoveryDao()
|
||||
fun provideDiscoveryDao(databaseProvider: DatabaseProvider): DiscoveryDao = SwitchingDiscoveryDao(databaseProvider)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
/*
|
||||
* Copyright (c) 2026 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 <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
package org.meshtastic.core.database.dao
|
||||
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.flow.onEach
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.meshtastic.core.database.DatabaseProvider
|
||||
import org.meshtastic.core.database.MeshtasticDatabase
|
||||
import org.meshtastic.core.database.entity.DiscoverySessionEntity
|
||||
import org.meshtastic.core.database.getInMemoryDatabaseBuilder
|
||||
import kotlin.test.AfterTest
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* Regression for #6235: the DAO Koin hands to `feature:discovery` used to be pinned to the injection-time database, so
|
||||
* a device/DB switch left long-lived consumers reading (and writing!) the old DB. [SwitchingDiscoveryDao] must resolve
|
||||
* the active database per call/collection instead.
|
||||
*/
|
||||
class SwitchingDiscoveryDaoTest {
|
||||
|
||||
private val dbA: MeshtasticDatabase = getInMemoryDatabaseBuilder().build()
|
||||
private val dbB: MeshtasticDatabase = getInMemoryDatabaseBuilder().build()
|
||||
private val provider = TestProvider(dbA)
|
||||
private val dao = SwitchingDiscoveryDao(provider)
|
||||
|
||||
@AfterTest
|
||||
fun tearDown() {
|
||||
dbA.close()
|
||||
dbB.close()
|
||||
}
|
||||
|
||||
private fun session(timestamp: Long) =
|
||||
DiscoverySessionEntity(timestamp = timestamp, presetsScanned = "LongFast", homePreset = "LongFast")
|
||||
|
||||
@Test
|
||||
fun suspendCallsResolveTheCurrentDbPerCall() = runTest {
|
||||
dao.insertSession(session(timestamp = 1))
|
||||
assertEquals(1, dbA.discoveryDao().getAllSessionsSnapshot().size, "first insert lands in the active DB (A)")
|
||||
|
||||
provider.switchTo(dbB)
|
||||
|
||||
dao.insertSession(session(timestamp = 2))
|
||||
assertEquals(1, dbA.discoveryDao().getAllSessionsSnapshot().size, "old DB untouched after switch")
|
||||
assertEquals(1, dbB.discoveryDao().getAllSessionsSnapshot().size, "post-switch insert lands in the new DB (B)")
|
||||
assertEquals(1, dao.getAllSessionsSnapshot().size, "reads resolve the new DB too")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun flowsRelatchOntoTheCurrentDb() = runTest {
|
||||
dao.insertSession(session(timestamp = 1))
|
||||
|
||||
// One live collection end-to-end: the first emission (DB A's session) triggers the switch, and the SAME
|
||||
// collector must then receive DB B's empty state. A delegate that resolved currentDb only at flow creation
|
||||
// would never emit the empty list, and the test would fail on runTest's timeout.
|
||||
var sawDbA = false
|
||||
val relatched =
|
||||
dao.getAllSessions()
|
||||
.onEach { sessions ->
|
||||
if (sessions.isNotEmpty()) {
|
||||
sawDbA = true
|
||||
provider.switchTo(dbB)
|
||||
}
|
||||
}
|
||||
.first { it.isEmpty() }
|
||||
|
||||
assertTrue(sawDbA, "collector observed DB A's session before the switch")
|
||||
assertEquals(0, relatched.size, "the same collector re-latched onto the new (empty) DB")
|
||||
}
|
||||
|
||||
/** Minimal [DatabaseProvider] whose active DB the test can swap, mirroring a device/DB switch. */
|
||||
private class TestProvider(db: MeshtasticDatabase) : DatabaseProvider {
|
||||
private val _currentDb = MutableStateFlow(db)
|
||||
override val currentDb: StateFlow<MeshtasticDatabase> = _currentDb
|
||||
|
||||
override suspend fun <T> withDb(block: suspend (MeshtasticDatabase) -> T): T? = block(_currentDb.value)
|
||||
|
||||
fun switchTo(db: MeshtasticDatabase) {
|
||||
_currentDb.value = db
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user