diff --git a/docs/adr/0027-boards-are-tombstoned-never-deleted.md b/docs/adr/0027-boards-are-tombstoned-never-deleted.md new file mode 100644 index 00000000..2eb53d60 --- /dev/null +++ b/docs/adr/0027-boards-are-tombstoned-never-deleted.md @@ -0,0 +1,20 @@ +# Boards Are Tombstoned, Never Deleted + +Deleting a **Board** sets `boards.deleted_at` instead of removing the row. The Board disappears from every Rider-facing list, its configuration (Board settings, Board warnings, **Alert Rules**, Last Known Board Config Values) is hard-deleted as before, and its **Ride History** is untouched — as it already was. + +The reason is that **Ride History** outlives the Board that produced it. The app has always kept telemetry after a Board delete, but the `boards` row vanishing left those rides pointing at a Board id that resolves to nothing. History could only fall back to the `device_name` snapshotted on each row: a frozen label, not an identity. A tombstone keeps the row resolvable, so a deleted Board's rides still name it and still group by it. + +## Considered Options + +- **Cascade** — deleting a Board deletes its Ride History too. Rejected outright: it deletes the thing worth keeping. +- **Leave the hard delete and lean on the snapshotted `device_name`.** Rejected because a name is not an identity: renames before the delete produce rides labelled inconsistently, and nothing links a ride back to the Board it came from. +- **Move Board identity onto the history rows** (denormalize more at write time). Rejected as strictly more storage for strictly less: it still cannot answer "which rides came from this Board" after the Board is gone. + +## Consequences + +- `getBoards()` filters `deleted_at IS NULL`. `getBoard(id)` deliberately does not — **Ride History** must still be able to name a deleted Board. Callers that act on a Board rather than describe one (`buildSessionConfig`, `BoardConnectConfig.resolve`) check `deletedAt` and refuse. +- An ordinary upsert never clears an existing tombstone, so deletion is terminal. Only the delete path stamps one, and deleting an already-tombstoned Board is a no-op. +- **Tune Profiles** are deliberately outside the cascade. Tuning work is expensive to recreate and survives its Board; removing one takes its own deletion. +- Telemetry can carry a stable `board_id` instead of keying on the mutable BLE identifier, because the row it points at never disappears. That unblocks the identity half of the `device_name` question (#274); the label half stays governed by ADR-0005. +- Tombstones accumulate. They are one small row per deleted Board, bounded by how many Boards a rider ever owned, so no pruning rule is warranted. +- The server half of this decision — tombstones crossing the wire, and the Board **Delete Action** that carries the configuration cascade — lands with Ride History backup (#276) and is out of scope here. diff --git a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/connection/SessionConfigBuilder.kt b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/connection/SessionConfigBuilder.kt index 0c8f07e0..f9e9b9fc 100644 --- a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/connection/SessionConfigBuilder.kt +++ b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/connection/SessionConfigBuilder.kt @@ -24,6 +24,10 @@ internal suspend fun buildSessionConfig( val repo = AppDataRepository.get(context.applicationContext) val board = repo.getBoard(boardId) ?: throw IllegalArgumentException("Board not found: $boardId") + // Lookup resolves tombstones so Ride History can name them; connecting to one is refused (ADR 0027). + if (board["deletedAt"] != null) { + throw IllegalArgumentException("Board is deleted: $boardId") + } @Suppress("UNCHECKED_CAST") val link = board["link"] as? Map val bleId = link?.get("bleId") as? String diff --git a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/AppDataRepository.kt b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/AppDataRepository.kt index bd4d7e94..6ffa083c 100644 --- a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/AppDataRepository.kt +++ b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/AppDataRepository.kt @@ -207,8 +207,13 @@ class AppDataRepository private constructor(private val context: Context) { notifyDataChanged(AppDataScope.BOARDS) } + /** + * Tombstones the Board rather than removing it, so its Ride History keeps a resolvable Board + * identity (ADR 0027). Board-owned configuration is still hard-deleted. + * @parity /modules/vescape-core/ios/telemetry/AppDataRepository.swift `deleteBoard` + */ suspend fun deleteBoard(id: String): Unit = withContext(Dispatchers.IO) { - dao.deleteBoardWithSettings(id) + dao.deleteBoardWithSettings(id, System.currentTimeMillis()) dao.deleteBoardConfigValues(id) dao.deleteBoardConfigChangeNotice(id) notifyDataChanged(AppDataScope.BOARDS) @@ -908,6 +913,7 @@ fun BoardEntity.toMap(settings: List): Map { "name" to name, "description" to values["description"], "createdAt" to createdAt, + "deletedAt" to deletedAt, "batteryConfig" to values["batteryConfig"], "lastBattery" to values["lastBattery"], "dismissedWarnings" to values["dismissedWarnings"], diff --git a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDao.kt b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDao.kt index a8fb6625..de83454e 100644 --- a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDao.kt +++ b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDao.kt @@ -368,14 +368,31 @@ interface TelemetryDao { clearExclusions() } - @Query("SELECT * FROM boards ORDER BY created_at ASC") + /** Live Boards only — a tombstoned Board is gone from every Rider-facing list (ADR 0027). */ + @Query("SELECT * FROM boards WHERE deleted_at IS NULL ORDER BY created_at ASC") suspend fun getBoards(): List + /** + * Resolves tombstones too, deliberately: Ride History still has to name a deleted Board. Callers + * that act on a Board rather than describe one check [BoardEntity.deletedAt] and refuse. + */ @Query("SELECT * FROM boards WHERE id = :id LIMIT 1") suspend fun getBoard(id: String): BoardEntity? @Insert(onConflict = OnConflictStrategy.REPLACE) - suspend fun upsertBoard(board: BoardEntity) + suspend fun insertBoardRow(board: BoardEntity) + + @Query("SELECT deleted_at FROM boards WHERE id = :id") + suspend fun getBoardDeletedAt(id: String): Long? + + /** + * An existing tombstone survives the write, so an ordinary upsert can never resurrect a deleted + * Board — deletion is terminal (ADR 0027). Only [deleteBoardWithSettings] stamps a new one. + */ + @Transaction + suspend fun upsertBoard(board: BoardEntity) { + insertBoardRow(board.copy(deletedAt = board.deletedAt ?: getBoardDeletedAt(board.id))) + } @Query("SELECT * FROM board_settings WHERE board_id = :boardId") suspend fun getBoardSettings(boardId: String): List @@ -399,16 +416,21 @@ interface TelemetryDao { @Query("DELETE FROM board_settings WHERE board_id = :boardId") suspend fun deleteBoardSettings(boardId: String) - @Query("DELETE FROM boards WHERE id = :id") - suspend fun deleteBoard(id: String) - + /** + * Tombstones the Board and hard-deletes its configuration. The `boards` row itself survives so + * Ride History can still name it (ADR 0027); deleting a Board that is already tombstoned is a + * no-op, so the stamp is never moved. + * + * @parity /modules/vescape-core/ios/telemetry/AppDataRepository.swift `deleteBoard` + */ @Transaction - suspend fun deleteBoardWithSettings(id: String) { + suspend fun deleteBoardWithSettings(id: String, deletedAt: Long) { + val board = getBoard(id)?.takeIf { it.deletedAt == null } ?: return deleteBoardSettings(id) deleteBoardWarnings(id) // Alert Rules are Board-owned (#254) — drop them with the Board so no orphan rows survive. deleteAlertRules(id) - deleteBoard(id) + insertBoardRow(board.copy(deletedAt = deletedAt)) } @Query("SELECT * FROM alerts WHERE board_id = :boardId ORDER BY created_at ASC") diff --git a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt index 84091a68..0a2d0ebb 100644 --- a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt +++ b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt @@ -13,7 +13,7 @@ import java.io.File // @parity /modules/vescape-core/ios/VescapeCoreModule.swift internal const val TELEMETRY_DATABASE_NAME = "vescape.db" internal const val LEGACY_TELEMETRY_DATABASE_NAME = "telemetry.db" -internal const val TELEMETRY_DATABASE_VERSION = 36 +internal const val TELEMETRY_DATABASE_VERSION = 37 @Database( entities = [ @@ -613,6 +613,18 @@ abstract class TelemetryDatabase : RoomDatabase() { * restored as `lastKnown` on connect. * @parity /modules/vescape-core/ios/telemetry/TelemetryDatabase.swift `v36_motor_config_values` */ + /** + * Board tombstones (#428). Deleting a Board stops removing its row and stamps `deleted_at` + * instead, so Ride History keeps a resolvable Board identity (ADR 0027). Additive and nullable; + * existing rows stay live. + * @parity /modules/vescape-core/ios/telemetry/TelemetryDatabase.swift `v37_board_deleted_at` + */ + internal val MIGRATION_36_37 = object : Migration(36, 37) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE boards ADD COLUMN deleted_at INTEGER") + } + } + internal val MIGRATION_35_36 = object : Migration(35, 36) { override fun migrate(db: SupportSQLiteDatabase) { db.execSQL( @@ -714,6 +726,7 @@ abstract class TelemetryDatabase : RoomDatabase() { MIGRATION_33_34, MIGRATION_34_35, MIGRATION_35_36, + MIGRATION_36_37, ) .fallbackToDestructiveMigration(true) .addCallback(object : Callback() { diff --git a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryEntities.kt b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryEntities.kt index c400bf76..00f479b2 100644 --- a/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryEntities.kt +++ b/modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryEntities.kt @@ -240,6 +240,17 @@ data class BoardEntity( val bleId: String?, @ColumnInfo(name = "created_at") val createdAt: Long, + /** + * Tombstone stamp: epoch ms of the rider's delete, null while the Board is alive. A deleted Board + * keeps its row so Ride History can still name it; only the Board's configuration is hard-deleted + * (ADR 0027). + * + * Written by the delete path only — an upsert from the bridge never authors it. + * @parity /modules/vescape-core/ios/telemetry/TelemetryDatabase.swift `v37_board_deleted_at` + * @parity /modules/vescape-core/src/index.ts `Board` + */ + @ColumnInfo(name = "deleted_at") + val deletedAt: Long? = null, ) @Entity( diff --git a/modules/vescape-core/ios/connection/BoardSessionController.swift b/modules/vescape-core/ios/connection/BoardSessionController.swift index c7add1e6..fe77e62e 100644 --- a/modules/vescape-core/ios/connection/BoardSessionController.swift +++ b/modules/vescape-core/ios/connection/BoardSessionController.swift @@ -38,6 +38,8 @@ internal struct BoardConnectConfig { recordingEnabled: Bool = false ) -> BoardConnectConfig? { guard let board = appData.getBoard(boardId) else { return nil } + // Lookup resolves tombstones so Ride History can name them; connecting to one is refused (ADR 0027). + guard (board["deletedAt"] ?? nil) == nil else { return nil } guard let link = board["link"] as? [String: Any?] else { return nil } guard let bleId = link["bleId"] as? String, !bleId.isEmpty else { return nil } let transport = BoardTransport.fromBridge(link["transport"] ?? nil) ?? .direct diff --git a/modules/vescape-core/ios/telemetry/AppDataRepository.swift b/modules/vescape-core/ios/telemetry/AppDataRepository.swift index 9c5bbe3f..19b889e5 100644 --- a/modules/vescape-core/ios/telemetry/AppDataRepository.swift +++ b/modules/vescape-core/ios/telemetry/AppDataRepository.swift @@ -70,7 +70,11 @@ final class AppDataRepository { read([]) { db in let boards = try Row.fetchAll( db, - sql: "SELECT id, name, ble_id, transport, created_at FROM boards ORDER BY created_at ASC" + // Live Boards only — a tombstoned Board is gone from every Rider-facing list (ADR 0027). + sql: """ + SELECT id, name, ble_id, transport, created_at, deleted_at FROM boards + WHERE deleted_at IS NULL ORDER BY created_at ASC + """ ) let settings = try Row.fetchAll(db, sql: "SELECT board_id, key, value_json FROM board_settings") var byBoard: [String: [(String, String)]] = [:] @@ -82,11 +86,13 @@ final class AppDataRepository { } } + /// Resolves tombstones too, deliberately: Ride History still has to name a deleted Board. Callers + /// that act on a Board rather than describe one check `deletedAt` and refuse (ADR 0027). func getBoard(_ id: String) -> [String: Any?]? { read(nil) { db in guard let board = try Row.fetchOne( db, - sql: "SELECT id, name, ble_id, transport, created_at FROM boards WHERE id = ? LIMIT 1", + sql: "SELECT id, name, ble_id, transport, created_at, deleted_at FROM boards WHERE id = ? LIMIT 1", arguments: [id] ) else { return nil } let settings = try Row.fetchAll( @@ -121,9 +127,15 @@ final class AppDataRepository { let updatedAt = nowMs() write { db in + // An existing tombstone survives the write, so an ordinary upsert can never resurrect a + // deleted Board — deletion is terminal (ADR 0027). Only `deleteBoard` stamps a new one. + let deletedAt = try Int64.fetchOne(db, sql: "SELECT deleted_at FROM boards WHERE id = ?", arguments: [id]) try db.execute( - sql: "INSERT OR REPLACE INTO boards (id, name, ble_id, transport, created_at) VALUES (?, ?, ?, ?, ?)", - arguments: [id, name, bleId, transport, createdAt] + sql: """ + INSERT OR REPLACE INTO boards (id, name, ble_id, transport, created_at, deleted_at) + VALUES (?, ?, ?, ?, ?, ?) + """, + arguments: [id, name, bleId, transport, createdAt, deletedAt] ) for (key, value) in settings { guard let value, let json = Self.encodeJson(value) else { @@ -139,12 +151,22 @@ final class AppDataRepository { notifyDataChanged(.boards) } + /// Tombstones the Board and hard-deletes its configuration. The `boards` row itself survives so + /// Ride History can still name it (ADR 0027); deleting an already-tombstoned Board is a no-op, so + /// the stamp is never moved. + /// @parity /modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/AppDataRepository.kt `deleteBoard` func deleteBoard(_ id: String) { + let deletedAt = nowMs() write { db in + guard try Bool.fetchOne( + db, + sql: "SELECT deleted_at IS NULL FROM boards WHERE id = ?", + arguments: [id] + ) == true else { return } try db.execute(sql: "DELETE FROM board_settings WHERE board_id = ?", arguments: [id]) // Alert Rules are Board-owned (#254) — drop them with the Board so no orphan rows survive. try db.execute(sql: "DELETE FROM alerts WHERE board_id = ?", arguments: [id]) - try db.execute(sql: "DELETE FROM boards WHERE id = ?", arguments: [id]) + try db.execute(sql: "UPDATE boards SET deleted_at = ? WHERE id = ?", arguments: [deletedAt, id]) } BoardConfigStore.shared.clear(boardId: id) notifyDataChanged(.boards) @@ -191,6 +213,7 @@ final class AppDataRepository { "name": row["name"] as String, "description": values["description"], "createdAt": row["created_at"] as Int64, + "deletedAt": row["deleted_at"] as Int64?, "batteryConfig": values["batteryConfig"], "lastBattery": values["lastBattery"], "dismissedWarnings": values["dismissedWarnings"], diff --git a/modules/vescape-core/ios/telemetry/TelemetryDatabase.swift b/modules/vescape-core/ios/telemetry/TelemetryDatabase.swift index 43540c93..b2e79d55 100644 --- a/modules/vescape-core/ios/telemetry/TelemetryDatabase.swift +++ b/modules/vescape-core/ios/telemetry/TelemetryDatabase.swift @@ -541,6 +541,14 @@ enum TelemetryDatabase { try MotorConfigStore.createTables(db) } + /// Board tombstones (#428). Deleting a Board stops removing its row and stamps `deleted_at` + /// instead, so Ride History keeps a resolvable Board identity (ADR 0027). Additive and + /// nullable; existing rows stay live. + /// @parity /modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt `MIGRATION_36_37` + migrator.registerMigration("v37_board_deleted_at") { db in + try db.execute(sql: "ALTER TABLE boards ADD COLUMN deleted_at INTEGER") + } + return migrator } } diff --git a/modules/vescape-core/ios/telemetry/TelemetryMigrationTests.swift b/modules/vescape-core/ios/telemetry/TelemetryMigrationTests.swift index 76343f83..25e79be1 100644 --- a/modules/vescape-core/ios/telemetry/TelemetryMigrationTests.swift +++ b/modules/vescape-core/ios/telemetry/TelemetryMigrationTests.swift @@ -89,6 +89,25 @@ final class TelemetryMigrationTests: XCTestCase { } } + /// Boards are tombstoned rather than deleted (ADR 0027), so Ride History keeps a resolvable Board + /// identity. The column is nullable and additive: an existing Board upgrades as live. + /// @parity /modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt `MIGRATION_36_37` + func testBoardsGainANullableDeletedAtColumn() throws { + try migrate() + + XCTAssertTrue(try columnNames("boards").contains("deleted_at")) + try queue.write { db in + try db.execute( + sql: "INSERT INTO boards (id, name, ble_id, created_at) VALUES (?, ?, ?, ?)", + arguments: ["b1", "Board", "AA:BB", 1_000] + ) + } + let deletedAt = try queue.read { db in + try Int64.fetchOne(db, sql: "SELECT deleted_at FROM boards WHERE id = ?", arguments: ["b1"]) + } + XCTAssertNil(deletedAt) + } + /// Alert Rules are owned by one Board: `board_id` is part of the primary key so preset ids repeat /// per board instead of colliding. Incremental sync keys off this shape. func testAlertsAreBoardOwnedAfterEveryMigration() throws { diff --git a/modules/vescape-core/src/index.ts b/modules/vescape-core/src/index.ts index 73da37f6..5ecd1136 100644 --- a/modules/vescape-core/src/index.ts +++ b/modules/vescape-core/src/index.ts @@ -188,6 +188,14 @@ export interface Board { name: string description: string | null createdAt: number + /** + * Tombstone stamp: epoch ms of the rider's delete, null/absent while the Board is alive. Deleting + * a Board keeps its row so Ride History can still name it (ADR 0027). Board lists never contain + * tombstones — only a lookup by id resolves one. + * @parity /modules/vescape-core/ios/telemetry/AppDataRepository.swift `composeBoard` + * @parity /modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/AppDataRepository.kt `toMap` + */ + deletedAt?: number | null batteryConfig: BatteryConfig | null /** Last Battery SoC Estimate persisted natively; survives full app kill. `undefined` before first session. */ lastBattery?: LastBattery | null