mirror of
https://github.com/meshtastic/Meshtastic-Android.git
synced 2026-09-20 10:52:22 -04:00
fix(database): delete nodes and metadata atomically (#6623)
This commit is contained in:
1 parent
0c98de8ae6
commit
fd8414abe3
8 files changed
+344
-20
No files matched your search
+2
-4
@@ -30,11 +30,9 @@ interface NodeInfoWriteDataSource {
|
||||
|
||||
suspend fun clearMyNodeInfo()
|
||||
|
||||
suspend fun deleteNode(num: Int)
|
||||
suspend fun deleteNodeAndMetadata(num: Int)
|
||||
|
||||
suspend fun deleteNodes(nodeNums: List<Int>)
|
||||
|
||||
suspend fun deleteMetadata(num: Int)
|
||||
suspend fun deleteNodesAndMetadata(nodeNums: List<Int>)
|
||||
|
||||
suspend fun upsert(metadata: MetadataEntity)
|
||||
|
||||
|
||||
+4
-8
@@ -51,16 +51,12 @@ class SwitchingNodeInfoWriteDataSource(
|
||||
withContext(dispatchers.io) { dbManager.withDb { it.nodeInfoDao().clearMyNodeInfo() } }
|
||||
}
|
||||
|
||||
override suspend fun deleteNode(num: Int) {
|
||||
withContext(dispatchers.io) { dbManager.withDb { it.nodeInfoDao().deleteNode(num) } }
|
||||
override suspend fun deleteNodeAndMetadata(num: Int) {
|
||||
withContext(dispatchers.io) { dbManager.withDb { it.nodeInfoDao().deleteNodeAndMetadata(num) } }
|
||||
}
|
||||
|
||||
override suspend fun deleteNodes(nodeNums: List<Int>) {
|
||||
withContext(dispatchers.io) { dbManager.withDb { it.nodeInfoDao().deleteNodes(nodeNums) } }
|
||||
}
|
||||
|
||||
override suspend fun deleteMetadata(num: Int) {
|
||||
withContext(dispatchers.io) { dbManager.withDb { it.nodeInfoDao().deleteMetadata(num) } }
|
||||
override suspend fun deleteNodesAndMetadata(nodeNums: List<Int>) {
|
||||
withContext(dispatchers.io) { dbManager.withDb { it.nodeInfoDao().deleteNodesAndMetadata(nodeNums) } }
|
||||
}
|
||||
|
||||
override suspend fun upsert(metadata: MetadataEntity) {
|
||||
|
||||
+4
-8
@@ -211,16 +211,12 @@ class NodeRepositoryImpl(
|
||||
override suspend fun clearMyNodeInfo() = withContext(dispatchers.io) { nodeInfoWriteDataSource.clearMyNodeInfo() }
|
||||
|
||||
/** Deletes a node and its metadata by [num]. */
|
||||
override suspend fun deleteNode(num: Int) = withContext(dispatchers.io) {
|
||||
nodeInfoWriteDataSource.deleteNode(num)
|
||||
nodeInfoWriteDataSource.deleteMetadata(num)
|
||||
}
|
||||
override suspend fun deleteNode(num: Int) =
|
||||
withContext(dispatchers.io) { nodeInfoWriteDataSource.deleteNodeAndMetadata(num) }
|
||||
|
||||
/** Deletes multiple nodes and their metadata. */
|
||||
override suspend fun deleteNodes(nodeNums: List<Int>) = withContext(dispatchers.io) {
|
||||
nodeInfoWriteDataSource.deleteNodes(nodeNums)
|
||||
nodeNums.forEach { nodeInfoWriteDataSource.deleteMetadata(it) }
|
||||
}
|
||||
override suspend fun deleteNodes(nodeNums: List<Int>) =
|
||||
withContext(dispatchers.io) { nodeInfoWriteDataSource.deleteNodesAndMetadata(nodeNums) }
|
||||
|
||||
override suspend fun getNodesOlderThan(lastHeard: Int): List<Node> =
|
||||
withContext(dispatchers.io) { nodeInfoReadDataSource.getNodesOlderThan(lastHeard).map { it.toModel() } }
|
||||
|
||||
+156
@@ -0,0 +1,156 @@
|
||||
/*
|
||||
* 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.data.datasource
|
||||
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.LifecycleOwner
|
||||
import androidx.lifecycle.LifecycleRegistry
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.flatMapLatest
|
||||
import kotlinx.coroutines.test.UnconfinedTestDispatcher
|
||||
import kotlinx.coroutines.test.resetMain
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlinx.coroutines.test.setMain
|
||||
import org.meshtastic.core.data.repository.NodeRepositoryImpl
|
||||
import org.meshtastic.core.database.DatabaseProvider
|
||||
import org.meshtastic.core.database.MeshtasticDatabase
|
||||
import org.meshtastic.core.database.entity.MetadataEntity
|
||||
import org.meshtastic.core.database.entity.NodeEntity
|
||||
import org.meshtastic.core.database.getInMemoryDatabaseBuilder
|
||||
import org.meshtastic.core.di.CoroutineDispatchers
|
||||
import org.meshtastic.core.testing.FakeLocalStatsDataSource
|
||||
import org.meshtastic.proto.DeviceMetadata
|
||||
import org.meshtastic.proto.User
|
||||
import kotlin.test.AfterTest
|
||||
import kotlin.test.BeforeTest
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertNotNull
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
class NodeDeletionDatabaseSwitchTest {
|
||||
private val testDispatcher = UnconfinedTestDispatcher()
|
||||
private val dispatchers = CoroutineDispatchers(main = testDispatcher, io = testDispatcher, default = testDispatcher)
|
||||
private lateinit var dbA: MeshtasticDatabase
|
||||
private lateinit var dbB: MeshtasticDatabase
|
||||
private lateinit var provider: SwitchAfterFirstWriteProvider
|
||||
private lateinit var lifecycleOwner: TestLifecycleOwner
|
||||
private lateinit var repository: NodeRepositoryImpl
|
||||
|
||||
@BeforeTest
|
||||
fun setUp() {
|
||||
Dispatchers.setMain(testDispatcher)
|
||||
dbA = getInMemoryDatabaseBuilder().build()
|
||||
dbB = getInMemoryDatabaseBuilder().build()
|
||||
provider = SwitchAfterFirstWriteProvider(dbA, dbB)
|
||||
lifecycleOwner = TestLifecycleOwner()
|
||||
lifecycleOwner.moveTo(Lifecycle.Event.ON_CREATE)
|
||||
val switchingWriteDataSource = SwitchingNodeInfoWriteDataSource(provider, dispatchers)
|
||||
val writeDataSourceWithoutStartupBackfill =
|
||||
object : NodeInfoWriteDataSource by switchingWriteDataSource {
|
||||
override suspend fun backfillDenormalizedNames() = Unit
|
||||
}
|
||||
repository =
|
||||
NodeRepositoryImpl(
|
||||
processLifecycle = lifecycleOwner.lifecycle,
|
||||
nodeInfoReadDataSource = SwitchingNodeInfoReadDataSource(provider),
|
||||
nodeInfoWriteDataSource = writeDataSourceWithoutStartupBackfill,
|
||||
dispatchers = dispatchers,
|
||||
localStatsDataSource = FakeLocalStatsDataSource(),
|
||||
)
|
||||
provider.arm()
|
||||
}
|
||||
|
||||
@AfterTest
|
||||
fun tearDown() {
|
||||
lifecycleOwner.moveTo(Lifecycle.Event.ON_DESTROY)
|
||||
dbA.close()
|
||||
dbB.close()
|
||||
Dispatchers.resetMain()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun deleteNodeUsesOneDatabaseCaptureForNodeAndMetadata() = runTest(testDispatcher) {
|
||||
seedNodeAndMetadata(dbA, "A")
|
||||
seedNodeAndMetadata(dbB, "B")
|
||||
|
||||
repository.deleteNode(NODE_NUM)
|
||||
|
||||
assertEquals(1, provider.withDbCalls, "one logical deletion must capture the active database exactly once")
|
||||
assertNull(dbA.nodeInfoDao().getNodeByNum(NODE_NUM), "database A node must be deleted")
|
||||
assertFalse(dbA.hasMetadata(NODE_NUM), "database A metadata must not be orphaned")
|
||||
assertNotNull(dbB.nodeInfoDao().getNodeByNum(NODE_NUM), "database B node must be untouched")
|
||||
assertTrue(dbB.hasMetadata(NODE_NUM), "database B metadata must be untouched")
|
||||
}
|
||||
|
||||
private suspend fun seedNodeAndMetadata(database: MeshtasticDatabase, label: String) {
|
||||
database.nodeInfoDao().upsert(NodeEntity(num = NODE_NUM, user = User(id = "!node-$label")))
|
||||
database.nodeInfoDao().upsert(MetadataEntity(num = NODE_NUM, proto = DeviceMetadata(firmware_version = label)))
|
||||
}
|
||||
|
||||
private suspend fun MeshtasticDatabase.hasMetadata(num: Int): Boolean =
|
||||
nodeInfoDao().getAllMetadataSnapshot().any { it.num == num }
|
||||
|
||||
private class TestLifecycleOwner : LifecycleOwner {
|
||||
override val lifecycle = LifecycleRegistry.createUnsafe(this)
|
||||
|
||||
fun moveTo(event: Lifecycle.Event) {
|
||||
lifecycle.handleLifecycleEvent(event)
|
||||
}
|
||||
}
|
||||
|
||||
/** Switches A -> B after the first armed callback, reproducing a device change between two logical writes. */
|
||||
private class SwitchAfterFirstWriteProvider(dbA: MeshtasticDatabase, private val dbB: MeshtasticDatabase) :
|
||||
DatabaseProvider {
|
||||
private val mutableCurrentDb = MutableStateFlow(dbA)
|
||||
override val currentDb: StateFlow<MeshtasticDatabase> = mutableCurrentDb
|
||||
private var armed = false
|
||||
var withDbCalls: Int = 0
|
||||
private set
|
||||
|
||||
override fun <T> observeCurrentDb(query: (MeshtasticDatabase) -> Flow<T>): Flow<T> =
|
||||
currentDb.flatMapLatest(query)
|
||||
|
||||
override suspend fun <T> withReadDb(block: suspend (MeshtasticDatabase) -> T): T = block(currentDb.value)
|
||||
|
||||
override suspend fun <T> withDb(block: suspend (MeshtasticDatabase) -> T): T? {
|
||||
val capturedDb = currentDb.value
|
||||
val result = block(capturedDb)
|
||||
if (armed) {
|
||||
withDbCalls += 1
|
||||
if (withDbCalls == 1) mutableCurrentDb.value = dbB
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
fun arm() {
|
||||
armed = true
|
||||
withDbCalls = 0
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val NODE_NUM = 0x1234
|
||||
}
|
||||
}
|
||||
+25
@@ -0,0 +1,25 @@
|
||||
/*
|
||||
* 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 androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.annotation.Config
|
||||
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
@Config(sdk = [34])
|
||||
class NodeInfoDaoAtomicDeleteTest : CommonNodeInfoDaoAtomicDeleteTest()
|
||||
@@ -366,6 +366,28 @@ interface NodeInfoDao {
|
||||
@Query("DELETE FROM metadata WHERE num=:num")
|
||||
suspend fun deleteMetadata(num: Int)
|
||||
|
||||
@Query("DELETE FROM metadata WHERE num IN (:nodeNums)")
|
||||
suspend fun deleteMetadataForNodes(nodeNums: List<Int>)
|
||||
|
||||
/** Atomically deletes one node and its separately stored device metadata. */
|
||||
@Transaction
|
||||
suspend fun deleteNodeAndMetadata(num: Int) {
|
||||
deleteNode(num)
|
||||
deleteMetadata(num)
|
||||
}
|
||||
|
||||
/**
|
||||
* Atomically deletes nodes and their metadata, chunking both `IN` queries below SQLite's bind-parameter limit.
|
||||
* Every chunk participates in the same transaction, so cancellation or a query failure rolls back the whole batch.
|
||||
*/
|
||||
@Transaction
|
||||
suspend fun deleteNodesAndMetadata(nodeNums: List<Int>) {
|
||||
for (chunk in nodeNums.chunked(MAX_BIND_PARAMS)) {
|
||||
deleteNodes(chunk)
|
||||
deleteMetadataForNodes(chunk)
|
||||
}
|
||||
}
|
||||
|
||||
/** Snapshot used by DatabaseMerger to carry per-node DeviceMetadata across transports (newest timestamp wins). */
|
||||
@Query("SELECT * FROM metadata")
|
||||
suspend fun getAllMetadataSnapshot(): List<MetadataEntity>
|
||||
|
||||
+112
@@ -0,0 +1,112 @@
|
||||
/*
|
||||
* 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 androidx.room3.executeSQL
|
||||
import androidx.room3.useWriterConnection
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.meshtastic.core.database.MeshtasticDatabase
|
||||
import org.meshtastic.core.database.entity.MetadataEntity
|
||||
import org.meshtastic.core.database.entity.NodeEntity
|
||||
import org.meshtastic.core.database.getInMemoryDatabaseBuilder
|
||||
import org.meshtastic.core.testing.setupTestContext
|
||||
import org.meshtastic.proto.DeviceMetadata
|
||||
import org.meshtastic.proto.User
|
||||
import kotlin.test.AfterTest
|
||||
import kotlin.test.BeforeTest
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertFails
|
||||
import kotlin.test.assertNotNull
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
abstract class CommonNodeInfoDaoAtomicDeleteTest {
|
||||
private lateinit var database: MeshtasticDatabase
|
||||
private lateinit var dao: NodeInfoDao
|
||||
|
||||
@BeforeTest
|
||||
fun setUp() {
|
||||
setupTestContext()
|
||||
database = getInMemoryDatabaseBuilder().build()
|
||||
dao = database.nodeInfoDao()
|
||||
}
|
||||
|
||||
@AfterTest
|
||||
fun tearDown() {
|
||||
database.close()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun deleteNodeAndMetadataRemovesBothRows() = runTest {
|
||||
seedNodeAndMetadata(NODE_NUM, "test")
|
||||
|
||||
dao.deleteNodeAndMetadata(NODE_NUM)
|
||||
|
||||
assertNull(dao.getNodeByNum(NODE_NUM))
|
||||
assertTrue(dao.getAllMetadataSnapshot().none { it.num == NODE_NUM })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun deleteNodesAndMetadataChunksPastBindLimit() = runTest {
|
||||
val secondNum = NODE_NUM + 1
|
||||
seedNodeAndMetadata(NODE_NUM, "first")
|
||||
seedNodeAndMetadata(secondNum, "second")
|
||||
val nodeNums = buildList {
|
||||
add(NODE_NUM)
|
||||
repeat(NodeInfoDao.MAX_BIND_PARAMS - 1) { add(-(it + 1)) }
|
||||
add(secondNum)
|
||||
}
|
||||
|
||||
dao.deleteNodesAndMetadata(nodeNums)
|
||||
|
||||
assertNull(dao.getNodeByNum(NODE_NUM))
|
||||
assertNull(dao.getNodeByNum(secondNum))
|
||||
assertTrue(dao.getAllMetadataSnapshot().none { it.num == NODE_NUM || it.num == secondNum })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun deleteNodeAndMetadataRollsBackWhenMetadataDeleteFails() = runTest {
|
||||
seedNodeAndMetadata(NODE_NUM, "test")
|
||||
database.useWriterConnection {
|
||||
it.executeSQL(
|
||||
"""
|
||||
CREATE TRIGGER fail_metadata_delete
|
||||
BEFORE DELETE ON metadata
|
||||
WHEN OLD.num = $NODE_NUM
|
||||
BEGIN
|
||||
SELECT RAISE(ABORT, 'forced metadata delete failure');
|
||||
END
|
||||
"""
|
||||
.trimIndent(),
|
||||
)
|
||||
}
|
||||
|
||||
assertFails { dao.deleteNodeAndMetadata(NODE_NUM) }
|
||||
|
||||
assertNotNull(dao.getNodeByNum(NODE_NUM), "node deletion must roll back with metadata deletion")
|
||||
assertTrue(dao.getAllMetadataSnapshot().any { it.num == NODE_NUM }, "metadata must remain after rollback")
|
||||
}
|
||||
|
||||
private suspend fun seedNodeAndMetadata(nodeNum: Int, label: String) {
|
||||
dao.upsert(NodeEntity(num = nodeNum, user = User(id = "!node-$label")))
|
||||
dao.upsert(MetadataEntity(num = nodeNum, proto = DeviceMetadata(firmware_version = label)))
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val NODE_NUM = 0x1234
|
||||
}
|
||||
}
|
||||
+19
@@ -0,0 +1,19 @@
|
||||
/*
|
||||
* 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
|
||||
|
||||
class NodeInfoDaoAtomicDeleteTest : CommonNodeInfoDaoAtomicDeleteTest()
|
||||
Reference in new issue
Block a user