Skip to content

Commit b22b43b

Browse files
williscoolwilliscool
authored
feat: Complete Room migration for EventsStorage (Phase 3) (#139)
feat: Migrate EventsStorage from SQLiteOpenHelper to Room with: - Copy-based migration preserving legacy DB - Automatic fallback to legacy on migration failure - Full behavioral parity with legacy implementation - Comprehensive migration tests including fallback verification * feat: room integration p3 EventsStorage * fix: bug bot Missing transaction causes potential data loss on update The updateEventAndInstanceTimes method performs a deleteByKey followed by insert without wrapping them in a transaction. The code comment acknowledges this is needed because the primary key changes, but if the delete succeeds and the insert fails (disk full, database error, etc.), the event data is permanently lost with no way to recover. The batch method updateEventsAndInstanceTimes correctly uses database.runInTransaction, but direct calls to updateEventAndInstanceTimes lack this protection. The legacy implementation uses a single update call so doesn't have this issue. * fix: catch from validator chat ABORT forces explicit duplicate handling - The manual check + update path is intentional to preserve notificationId REPLACE would break notification ID preservation - It would delete the old row (losing the notification ID) then insert new Defensive against race conditions - If somehow a duplicate slips through the check, ABORT throws rather than silently corrupting data Faithful to legacy behavior - insertOrThrow + catch-and-update was the original pattern * fix: POC * docs: update * docs: data domain * docs: more data domain * docs: more domain_model and snoozing * docs: couple more key behaviors for domain_model * docs: domain_model more * docs: domain_model quick reference * docs: domain_model legacy names * fix: safer getInstance * fix: updateEventAndInstanceTimes updateEventAndInstanceTimes creates record when none exists The updateEventAndInstanceTimes implementation has different semantics than the legacy version. The legacy code uses UPDATE and returns numRowsAffected == 1, so it returns false if the record doesn't exist. The new implementation does deleteByKey (which succeeds with 0 deletions if record doesn't exist) then insert (which succeeds), returning true. This creates a new record instead of indicating the update failed, which could mask bugs in calling code that relies on the return value. * fix: addEvents Transaction doesn't rollback on partial failure in addEvents The addEvents method continues processing events after a failure and commits partial results, whereas the legacy implementation breaks on first failure and rolls back the entire transaction. When addEvent returns false, the loop continues (success = false without break), and Room's runInTransaction commits whatever succeeded. The legacy code explicitly breaks from the loop and skips setTransactionSuccessful() to ensure rollback. This can cause data integrity issues where some events are inserted while others fail silently. The same issue affects updateEvents * fix: RoomEventsStorage thread safe notification id Race condition in notification ID generation The nextNotificationId() function reads the max notification ID and returns maxId + 1, but this compound operation is not atomic. The legacy LegacyEventsStorage synchronized all operations on the class object, but RoomEventsStorage has no synchronization. Two concurrent addEvent calls could both read the same max ID and generate identical notification IDs, causing Android notifications to silently overwrite each other. This is a regression from the thread-safe legacy implementation. * fix: count perf and double transaction wrapper * fix: and test for legacy fallback if room db creation fails * fix: legacy match updateEventsAndInstanceTimesImpl Missing exception handling changes behavior from legacy The legacy updateEventsAndInstanceTimesImpl in EventsStorageImplV9 catches SQLiteConstraintException and returns a boolean, but RoomEventsStorage.updateEventsAndInstanceTimes propagates the exception to callers. When the new instance time matches an existing row's primary key, legacy logs and returns ret while Room throws, potentially causing unexpected crashes in callers that expect a boolean return value. * fix: more legacy match * docs: RoomEventsStorage
1 parent 06f36ae commit b22b43b

18 files changed

Lines changed: 2247 additions & 232 deletions

File tree

‎android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/RoomCrSqlitePocTest.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ package com.github.quarck.calnotify.database.poc
2222
import android.content.Context
2323
import androidx.test.ext.junit.runners.AndroidJUnit4
2424
import androidx.test.platform.app.InstrumentationRegistry
25-
import com.github.quarck.calnotify.eventsstorage.EventsStorage
25+
import com.github.quarck.calnotify.eventsstorage.LegacyEventsStorage
2626
import com.github.quarck.calnotify.logs.DevLog
2727
import org.junit.After
2828
import org.junit.Assert.*
@@ -100,8 +100,8 @@ class RoomCrSqlitePocTest {
100100
DevLog.error(LOG_TAG, "Native lib dir does not exist!")
101101
}
102102

103-
// Use EventsStorage which uses the main app's SQLiteOpenHelper
104-
val eventsStorage = EventsStorage(context)
103+
// Use LegacyEventsStorage which extends SQLiteOpenHelper directly
104+
val eventsStorage = LegacyEventsStorage(context)
105105

106106
try {
107107
// Get the underlying database directly

‎android/app/src/androidTest/java/com/github/quarck/calnotify/eventsstorage/EventsStorageMigrationTest.kt‎

Lines changed: 637 additions & 0 deletions
Large diffs are not rendered by default.

‎android/app/src/main/java/com/github/quarck/calnotify/dismissedeventsstorage/DismissedEventDao.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ interface DismissedEventDao {
3636
@Query("SELECT * FROM ${DismissedEventEntity.TABLE_NAME}")
3737
fun getAll(): List<DismissedEventEntity>
3838

39+
@Query("SELECT COUNT(*) FROM ${DismissedEventEntity.TABLE_NAME}")
40+
fun count(): Int
41+
3942
@Insert(onConflict = OnConflictStrategy.REPLACE)
4043
fun insert(entity: DismissedEventEntity)
4144

‎android/app/src/main/java/com/github/quarck/calnotify/dismissedeventsstorage/DismissedEventsDatabase.kt‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,20 @@ abstract class DismissedEventsDatabase : RoomDatabase() {
7676
fun getInstance(context: Context): DismissedEventsDatabase {
7777
return INSTANCE ?: synchronized(this) {
7878
INSTANCE ?: run {
79-
val db = buildDatabase(context)
79+
var db: DismissedEventsDatabase? = null
8080
try {
81+
db = buildDatabase(context)
8182
// Copy from legacy DB if needed (throws exception on failure)
8283
copyFromLegacyIfNeeded(context, db)
8384
INSTANCE = db
8485
db
8586
} catch (e: DismissedEventsMigrationException) {
86-
db.close() // Clean up on failure to avoid leaking connection
87+
db?.close()
8788
throw e
89+
} catch (e: RuntimeException) {
90+
// Wrap unexpected runtime exceptions so caller can fall back to legacy
91+
db?.close()
92+
throw DismissedEventsMigrationException("Migration failed with unexpected error: ${e.message}", e)
8893
}
8994
}
9095
}
@@ -126,7 +131,7 @@ abstract class DismissedEventsDatabase : RoomDatabase() {
126131

127132
try {
128133
// Check if Room DB already has data (migration already done)
129-
val existingCount = dao.getAll().size
134+
val existingCount = dao.count()
130135
if (existingCount > 0) {
131136
DevLog.info(LOG_TAG, "Room database already has $existingCount rows - skipping migration")
132137
return
@@ -160,7 +165,7 @@ abstract class DismissedEventsDatabase : RoomDatabase() {
160165
dao.insertAll(entities)
161166

162167
// Validate row count in Room matches legacy
163-
val newCount = dao.getAll().size
168+
val newCount = dao.count()
164169
DevLog.info(LOG_TAG, "Inserted $newCount rows into Room database")
165170

166171
if (newCount != legacyCount) {

‎android/app/src/main/java/com/github/quarck/calnotify/dismissedeventsstorage/DismissedEventsStorage.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class DismissedEventsStorage private constructor(
4545
val isUsingRoom: Boolean = result.second
4646

4747
constructor(context: Context, clock: CNPlusClockInterface = CNPlusSystemClock()) : this(createStorage(context, clock))
48-
48+
4949
override fun close() {
5050
(delegate as? Closeable)?.close()
5151
}

‎android/app/src/main/java/com/github/quarck/calnotify/eventsstorage/EventAlertDao.kt‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
//
2+
// Calendar Notifications Plus
3+
// Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com)
4+
//
5+
// This program is free software; you can redistribute it and/or modify
6+
// it under the terms of the GNU General Public License as published by
7+
// the Free Software Foundation; either version 3 of the License, or
8+
// (at your option) any later version.
9+
//
10+
// This program is distributed in the hope that it will be useful,
11+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
// GNU General Public License for more details.
14+
//
15+
// You should have received a copy of the GNU General Public License
16+
// along with this program; if not, write to the Free Software Foundation,
17+
// Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
18+
//
19+
20+
package com.github.quarck.calnotify.eventsstorage
21+
22+
import androidx.room.Dao
23+
import androidx.room.Delete
24+
import androidx.room.Insert
25+
import androidx.room.OnConflictStrategy
26+
import androidx.room.Query
27+
import androidx.room.Update
28+
29+
/**
30+
* Room DAO for EventAlertEntity.
31+
*
32+
* Provides all database operations needed by EventsStorageInterface.
33+
*/
34+
@Dao
35+
interface EventAlertDao {
36+
37+
@Query("SELECT * FROM ${EventAlertEntity.TABLE_NAME}")
38+
fun getAll(): List<EventAlertEntity>
39+
40+
@Query("SELECT COUNT(*) FROM ${EventAlertEntity.TABLE_NAME}")
41+
fun count(): Int
42+
43+
@Query("SELECT * FROM ${EventAlertEntity.TABLE_NAME} WHERE ${EventAlertEntity.COL_EVENT_ID} = :eventId AND ${EventAlertEntity.COL_INSTANCE_START} = :instanceStart")
44+
fun getByKey(eventId: Long, instanceStart: Long): EventAlertEntity?
45+
46+
@Query("SELECT * FROM ${EventAlertEntity.TABLE_NAME} WHERE ${EventAlertEntity.COL_EVENT_ID} = :eventId")
47+
fun getByEventId(eventId: Long): List<EventAlertEntity>
48+
49+
@Query("SELECT MAX(${EventAlertEntity.COL_NOTIFICATION_ID}) FROM ${EventAlertEntity.TABLE_NAME}")
50+
fun getMaxNotificationId(): Int?
51+
52+
// ABORT matches legacy insertOrThrow behavior - duplicates are handled explicitly in RoomEventsStorage
53+
@Insert(onConflict = OnConflictStrategy.ABORT)
54+
fun insert(entity: EventAlertEntity): Long
55+
56+
@Insert(onConflict = OnConflictStrategy.ABORT)
57+
fun insertAll(entities: List<EventAlertEntity>): List<Long>
58+
59+
@Update
60+
fun update(entity: EventAlertEntity): Int
61+
62+
@Update
63+
fun updateAll(entities: List<EventAlertEntity>): Int
64+
65+
@Delete
66+
fun delete(entity: EventAlertEntity): Int
67+
68+
@Delete
69+
fun deleteAll(entities: List<EventAlertEntity>): Int
70+
71+
@Query("DELETE FROM ${EventAlertEntity.TABLE_NAME} WHERE ${EventAlertEntity.COL_EVENT_ID} = :eventId AND ${EventAlertEntity.COL_INSTANCE_START} = :instanceStart")
72+
fun deleteByKey(eventId: Long, instanceStart: Long): Int
73+
74+
@Query("DELETE FROM ${EventAlertEntity.TABLE_NAME}")
75+
fun deleteAllRows()
76+
}
77+

‎android/app/src/main/java/com/github/quarck/calnotify/eventsstorage/EventAlertEntity.kt‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
//
2+
// Calendar Notifications Plus
3+
// Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com)
4+
//
5+
// This program is free software; you can redistribute it and/or modify
6+
// it under the terms of the GNU General Public License as published by
7+
// the Free Software Foundation; either version 3 of the License, or
8+
// (at your option) any later version.
9+
//
10+
// This program is distributed in the hope that it will be useful,
11+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
// GNU General Public License for more details.
14+
//
15+
// You should have received a copy of the GNU General Public License
16+
// along with this program; if not, write to the Free Software Foundation,
17+
// Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
18+
//
19+
20+
package com.github.quarck.calnotify.eventsstorage
21+
22+
import androidx.room.ColumnInfo
23+
import androidx.room.Entity
24+
import androidx.room.Index
25+
import com.github.quarck.calnotify.calendar.AttendanceStatus
26+
import com.github.quarck.calnotify.calendar.EventAlertRecord
27+
import com.github.quarck.calnotify.calendar.EventDisplayStatus
28+
import com.github.quarck.calnotify.calendar.EventOrigin
29+
import com.github.quarck.calnotify.calendar.EventStatus
30+
31+
/**
32+
* Room entity matching the eventsV9 table schema.
33+
*
34+
* Schema compatibility:
35+
* - Primary key columns (id, istart) are NOT NULL
36+
* - Other columns are nullable to match legacy schema
37+
* - Index matches legacy `eventsIdxV9` index
38+
* - Column names use the shortened V9 format (cid, id, ttl, etc.)
39+
*/
40+
@Entity(
41+
tableName = EventAlertEntity.TABLE_NAME,
42+
primaryKeys = [EventAlertEntity.COL_EVENT_ID, EventAlertEntity.COL_INSTANCE_START],
43+
indices = [Index(
44+
value = [EventAlertEntity.COL_EVENT_ID, EventAlertEntity.COL_INSTANCE_START],
45+
unique = true,
46+
name = EventAlertEntity.INDEX_NAME
47+
)]
48+
)
49+
data class EventAlertEntity(
50+
@ColumnInfo(name = COL_CALENDAR_ID) val calendarId: Long? = -1,
51+
@ColumnInfo(name = COL_EVENT_ID) val eventId: Long,
52+
@ColumnInfo(name = COL_ALERT_TIME) val alertTime: Long? = 0,
53+
@ColumnInfo(name = COL_NOTIFICATION_ID) val notificationId: Int? = 0,
54+
@ColumnInfo(name = COL_TITLE) val title: String? = "",
55+
@ColumnInfo(name = COL_DESCRIPTION) val description: String? = "",
56+
@ColumnInfo(name = COL_START) val startTime: Long? = 0,
57+
@ColumnInfo(name = COL_END) val endTime: Long? = 0,
58+
@ColumnInfo(name = COL_INSTANCE_START) val instanceStartTime: Long,
59+
@ColumnInfo(name = COL_INSTANCE_END) val instanceEndTime: Long? = 0,
60+
@ColumnInfo(name = COL_LOCATION) val location: String? = "",
61+
@ColumnInfo(name = COL_SNOOZED_UNTIL) val snoozedUntil: Long? = 0,
62+
@ColumnInfo(name = COL_LAST_STATUS_CHANGE) val lastStatusChangeTime: Long? = 0,
63+
@ColumnInfo(name = COL_DISPLAY_STATUS) val displayStatus: Int? = 0,
64+
@ColumnInfo(name = COL_COLOR) val color: Int? = 0,
65+
@ColumnInfo(name = COL_IS_REPEATING) val isRepeating: Int? = 0,
66+
@ColumnInfo(name = COL_ALL_DAY) val isAllDay: Int? = 0,
67+
@ColumnInfo(name = COL_EVENT_ORIGIN) val origin: Int? = 0,
68+
@ColumnInfo(name = COL_TIME_FIRST_SEEN) val timeFirstSeen: Long? = 0,
69+
@ColumnInfo(name = COL_EVENT_STATUS) val eventStatus: Int? = 0,
70+
@ColumnInfo(name = COL_ATTENDANCE_STATUS) val attendanceStatus: Int? = 0,
71+
@ColumnInfo(name = COL_FLAGS) val flags: Long? = 0,
72+
// Reserved fields (match legacy schema)
73+
@ColumnInfo(name = COL_RESERVED_INT2) val reservedInt2: Long? = 0,
74+
@ColumnInfo(name = COL_RESERVED_INT3) val reservedInt3: Long? = 0,
75+
@ColumnInfo(name = COL_RESERVED_INT4) val reservedInt4: Long? = 0,
76+
@ColumnInfo(name = COL_RESERVED_INT5) val reservedInt5: Long? = 0,
77+
@ColumnInfo(name = COL_RESERVED_INT6) val reservedInt6: Long? = 0,
78+
@ColumnInfo(name = COL_RESERVED_INT7) val reservedInt7: Long? = 0,
79+
@ColumnInfo(name = COL_RESERVED_INT8) val reservedInt8: Long? = 0,
80+
@ColumnInfo(name = COL_RESERVED_STR2) val reservedStr2: String? = ""
81+
) {
82+
/** Convert to domain model */
83+
fun toRecord() = EventAlertRecord(
84+
calendarId = calendarId ?: -1L,
85+
eventId = eventId,
86+
alertTime = alertTime ?: 0,
87+
notificationId = notificationId ?: 0,
88+
title = title ?: "",
89+
desc = description ?: "",
90+
startTime = startTime ?: 0,
91+
endTime = endTime ?: 0,
92+
instanceStartTime = instanceStartTime,
93+
instanceEndTime = instanceEndTime ?: 0,
94+
location = location ?: "",
95+
snoozedUntil = snoozedUntil ?: 0,
96+
lastStatusChangeTime = lastStatusChangeTime ?: 0,
97+
displayStatus = EventDisplayStatus.fromInt(displayStatus ?: 0),
98+
color = color ?: 0,
99+
isRepeating = (isRepeating ?: 0) != 0,
100+
isAllDay = (isAllDay ?: 0) != 0,
101+
origin = EventOrigin.fromInt(origin ?: 0),
102+
timeFirstSeen = timeFirstSeen ?: 0,
103+
eventStatus = EventStatus.fromInt(eventStatus ?: 0),
104+
attendanceStatus = AttendanceStatus.fromInt(attendanceStatus ?: 0),
105+
flags = flags ?: 0
106+
)
107+
108+
companion object {
109+
// Table and index names (must match legacy schema)
110+
const val TABLE_NAME = "eventsV9"
111+
const val INDEX_NAME = "eventsIdxV9"
112+
113+
// Column names (must match legacy V9 schema - shortened names)
114+
const val COL_CALENDAR_ID = "cid"
115+
const val COL_EVENT_ID = "id"
116+
const val COL_ALERT_TIME = "altm"
117+
const val COL_NOTIFICATION_ID = "nid"
118+
const val COL_TITLE = "ttl"
119+
const val COL_DESCRIPTION = "s1"
120+
const val COL_START = "estart"
121+
const val COL_END = "eend"
122+
const val COL_INSTANCE_START = "istart"
123+
const val COL_INSTANCE_END = "iend"
124+
const val COL_LOCATION = "loc"
125+
const val COL_SNOOZED_UNTIL = "snz"
126+
const val COL_LAST_STATUS_CHANGE = "ls"
127+
const val COL_DISPLAY_STATUS = "dsts"
128+
const val COL_COLOR = "clr"
129+
const val COL_IS_REPEATING = "rep"
130+
const val COL_ALL_DAY = "alld"
131+
const val COL_EVENT_ORIGIN = "ogn"
132+
const val COL_TIME_FIRST_SEEN = "fsn"
133+
const val COL_EVENT_STATUS = "attsts"
134+
const val COL_ATTENDANCE_STATUS = "oattsts"
135+
const val COL_FLAGS = "i1"
136+
const val COL_RESERVED_INT2 = "i2"
137+
const val COL_RESERVED_INT3 = "i3"
138+
const val COL_RESERVED_INT4 = "i4"
139+
const val COL_RESERVED_INT5 = "i5"
140+
const val COL_RESERVED_INT6 = "i6"
141+
const val COL_RESERVED_INT7 = "i7"
142+
const val COL_RESERVED_INT8 = "i8"
143+
const val COL_RESERVED_STR2 = "s2"
144+
145+
/** Convert from domain model */
146+
fun fromRecord(record: EventAlertRecord) = EventAlertEntity(
147+
calendarId = record.calendarId,
148+
eventId = record.eventId,
149+
alertTime = record.alertTime,
150+
notificationId = record.notificationId,
151+
title = record.title,
152+
description = record.desc,
153+
startTime = record.startTime,
154+
endTime = record.endTime,
155+
instanceStartTime = record.instanceStartTime,
156+
instanceEndTime = record.instanceEndTime,
157+
location = record.location,
158+
snoozedUntil = record.snoozedUntil,
159+
lastStatusChangeTime = record.lastStatusChangeTime,
160+
displayStatus = record.displayStatus.code,
161+
color = record.color,
162+
isRepeating = if (record.isRepeating) 1 else 0,
163+
isAllDay = if (record.isAllDay) 1 else 0,
164+
origin = record.origin.code,
165+
timeFirstSeen = record.timeFirstSeen,
166+
eventStatus = record.eventStatus.code,
167+
attendanceStatus = record.attendanceStatus.code,
168+
flags = record.flags
169+
)
170+
}
171+
}
172+

0 commit comments

Comments
 (0)