diff --git a/.cursor/rules/wsl-unison-setup.mdc b/.cursor/rules/wsl-unison-setup.mdc index 37908ebfa..bc00e0723 100644 --- a/.cursor/rules/wsl-unison-setup.mdc +++ b/.cursor/rules/wsl-unison-setup.mdc @@ -101,3 +101,35 @@ Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data, The `scripts/bundle_or_skip.js` wrapper automatically detects and copies the pre-built bundle, skipping Metro on Windows. This is configured via `cliFile` in `android/app/build.gradle`. See: https://github.com/nativewind/nativewind/issues/1667 + +## Running Android Instrumentation Tests + +Android instrumentation tests (`connectedAndroidTest`) must be run from **Windows**, not WSL. The test runner communicates with the emulator over ports that don't work properly through WSL. + +### From WSL Terminal (via PowerShell) + +```bash +# Wait for Unison sync after file changes (~15 seconds) +sleep 15 + +# Run all tests for a specific class +powershell.exe -Command 'cd C:\dev\CN\android; $env:JAVA_HOME = "C:\Program Files\Android\Android_Studio\jbr"; .\gradlew.bat :app:connectedX8664DebugAndroidTest "-Pandroid.testInstrumentationRunnerArguments.class=com.github.quarck.calnotify.monitorstorage.MonitorStorageMigrationTest"' + +# Run all instrumentation tests +powershell.exe -Command 'cd C:\dev\CN\android; $env:JAVA_HOME = "C:\Program Files\Android\Android_Studio\jbr"; .\gradlew.bat :app:connectedX8664DebugAndroidTest' +``` + +### Important Notes + +1. **Wait for sync**: Unison syncs every 10 seconds (no inotify on Windows mounts). Wait 15s after file changes before running tests. + +2. **JAVA_HOME**: Must be set in PowerShell. Android Studio's bundled JBR is at `C:\Program Files\Android\Android_Studio\jbr` + +3. **Test filtering**: Use `-Pandroid.testInstrumentationRunnerArguments.class=...`, NOT `--tests` (which doesn't work for `connectedAndroidTest`) + +4. **Signature conflicts**: If you get `INSTALL_FAILED_UPDATE_INCOMPATIBLE`, uninstall the app first: + ```bash + adb uninstall com.github.quarck.calnotify + ``` + +5. **Building APKs**: Can be done from either side, but running connected tests requires Windows. diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 7ccc4a486..d6d6405d1 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -809,7 +809,8 @@ jobs: if: always() && steps.verify_results.outputs.has_results == 'true' with: name: 'Integration Tests (Shard ${{ matrix.shard }})' - path: android/app/build/outputs/*.xml,android/app/build/outputs/**/*.xml + # Exclude allure-results which contain non-JUnit XML files (use comma-separated paths) + path: 'android/app/build/outputs/*.xml,android/app/build/outputs/androidTest-results/**/*.xml,android/app/build/outputs/connected/**/*.xml' reporter: java-junit fail-on-error: true diff --git a/android/app/build.gradle b/android/app/build.gradle index 9acb8802e..ec1a354b9 100644 --- a/android/app/build.gradle +++ b/android/app/build.gradle @@ -344,18 +344,19 @@ dependencies { // Jacoco androidTestImplementation 'org.jacoco:org.jacoco.core:0.8.12' - // Room POC (test only - verifying cr-sqlite compatibility) + // Room database library // See docs/dev_todo/database_modernization_plan.md // Note: Room 2.7.0+ required for Kotlin 2.0.x metadata compatibility // https://developer.android.com/jetpack/androidx/releases/room def roomVersion = "2.8.4" - androidTestImplementation "androidx.room:room-runtime:$roomVersion" - androidTestImplementation "androidx.room:room-ktx:$roomVersion" - androidTestImplementation "androidx.room:room-testing:$roomVersion" - kspAndroidTest "androidx.room:room-compiler:$roomVersion" + implementation "androidx.room:room-runtime:$roomVersion" + implementation "androidx.room:room-ktx:$roomVersion" + ksp "androidx.room:room-compiler:$roomVersion" // Room 2.8+ uses the new androidx.sqlite library - androidTestImplementation "androidx.sqlite:sqlite:2.6.2" - androidTestImplementation "androidx.sqlite:sqlite-framework:2.6.2" + implementation "androidx.sqlite:sqlite:2.6.2" + implementation "androidx.sqlite:sqlite-framework:2.6.2" + // Room testing support + androidTestImplementation "androidx.room:room-testing:$roomVersion" } // https://reactnative.dev/docs/react-native-gradle-plugin @@ -396,6 +397,15 @@ afterEvaluate { .withPathSensitivity(PathSensitivity.RELATIVE) .optional() } + + // Wire KSP tasks (Room annotation processor) + tasks.matching { it.name.matches('ksp.*Kotlin') }.configureEach { task -> + task.dependsOn(autolinkTask) + task.inputs.dir(autolinkingDir) + .withPropertyName('autolinkingSources') + .withPathSensitivity(PathSensitivity.RELATIVE) + .optional() + } } } diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/RoomPocDatabase.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/RoomPocDatabase.kt index 2ecd4e8a9..529b58cdf 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/RoomPocDatabase.kt +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/RoomPocDatabase.kt @@ -23,6 +23,7 @@ import android.content.Context import androidx.room.Database import androidx.room.Room import androidx.room.RoomDatabase +import com.github.quarck.calnotify.database.CrSqliteRoomFactory /** * POC Room database configured to use cr-sqlite via CrSqliteRoomFactory. diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/monitorstorage/MonitorStorageMigrationTest.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/monitorstorage/MonitorStorageMigrationTest.kt new file mode 100644 index 000000000..65a0186df --- /dev/null +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/monitorstorage/MonitorStorageMigrationTest.kt @@ -0,0 +1,386 @@ +// +// Calendar Notifications Plus +// Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com) +// +// 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, write to the Free Software Foundation, +// Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +package com.github.quarck.calnotify.monitorstorage + +import android.content.Context +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.github.quarck.calnotify.logs.DevLog +import org.junit.After +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import java.io.File + +/** + * Tests Room's ability to open and use a pre-existing SQLite database + * that was created by the legacy SQLiteOpenHelper (without room_master_table). + * + * This validates the database migration path for existing users upgrading + * from the old implementation to Room. + */ +@RunWith(AndroidJUnit4::class) +class MonitorStorageMigrationTest { + + companion object { + private const val LOG_TAG = "MonitorStorageMigrationTest" + private const val LEGACY_DB_NAME = "CalendarMonitor_MigrationTest" + } + + private lateinit var context: Context + private lateinit var dbFile: File + + @Before + fun setup() { + context = InstrumentationRegistry.getInstrumentation().targetContext + dbFile = context.getDatabasePath(LEGACY_DB_NAME) + + // Ensure clean state + if (dbFile.exists()) { + dbFile.delete() + } + dbFile.parentFile?.mkdirs() + + DevLog.info(LOG_TAG, "Test setup complete, dbFile=${dbFile.absolutePath}") + } + + @After + fun cleanup() { + // Clean up test database + if (dbFile.exists()) { + dbFile.delete() + } + // Also clean up any Room-generated files + File(dbFile.absolutePath + "-shm").delete() + File(dbFile.absolutePath + "-wal").delete() + + DevLog.info(LOG_TAG, "Test cleanup complete") + } + + /** + * Creates a legacy database with the exact schema that SQLiteOpenHelper would create, + * WITHOUT room_master_table. This simulates a user's existing database before upgrade. + * + * Uses the actual MonitorStorageImplV1.createDb() to ensure we test the real schema. + */ + private fun createLegacyDatabase(): List { + DevLog.info(LOG_TAG, "Creating legacy database at ${dbFile.absolutePath}") + + // Use requery SQLiteDatabase - same as the legacy code uses + val db = io.requery.android.database.sqlite.SQLiteDatabase.openOrCreateDatabase( + dbFile.absolutePath, null + ) + + try { + // Use the ACTUAL legacy code to create the schema - this is what we're migrating from + val legacyImpl = MonitorStorageImplV1(context) + legacyImpl.createDb(db) + + // Insert test data using the legacy implementation + val testData = listOf( + TestAlertData(calendarId = 1, eventId = 12345, alertTime = 1735500000000, + instanceStart = 1735500000000, instanceEnd = 1735503600000, + allDay = 0, alertCreatedByUs = 1, wasHandled = 0), + TestAlertData(calendarId = 1, eventId = 12346, alertTime = 1735510000000, + instanceStart = 1735510000000, instanceEnd = 1735513600000, + allDay = 0, alertCreatedByUs = 0, wasHandled = 1), + TestAlertData(calendarId = 2, eventId = 12347, alertTime = 1735520000000, + instanceStart = 1735520000000, instanceEnd = 1735523600000, + allDay = 1, alertCreatedByUs = 1, wasHandled = 1) + ) + + for (data in testData) { + db.execSQL(""" + INSERT INTO ${MonitorAlertEntity.TABLE_NAME} VALUES ( + ${data.calendarId}, ${data.eventId}, ${data.alertTime}, + ${data.instanceStart}, ${data.instanceEnd}, ${data.allDay}, + ${data.alertCreatedByUs}, ${data.wasHandled}, 0, 0 + ) + """.trimIndent()) + } + + // Verify NO room_master_table exists (this is the key test condition) + val cursor = db.rawQuery( + "SELECT name FROM sqlite_master WHERE type='table' AND name='room_master_table'", + null + ) + val hasRoomTable = cursor.moveToFirst() + cursor.close() + + assertFalse("Legacy database should NOT have room_master_table", hasRoomTable) + + DevLog.info(LOG_TAG, "Created legacy database with ${testData.size} test rows, no room_master_table") + return testData + + } finally { + db.close() + } + } + + /** + * Performs pre-Room migration on a legacy database. + * This replicates the logic in MonitorDatabase.migrateLegacyDatabaseIfNeeded() + * to add NOT NULL constraints to PK columns (required by Room). + */ + private fun performPreRoomMigration() { + val t = MonitorAlertEntity.TABLE_NAME + val idx = MonitorAlertEntity.INDEX_NAME + // Column names from Entity + val calendarId = MonitorAlertEntity.COL_CALENDAR_ID + val eventId = MonitorAlertEntity.COL_EVENT_ID + val alertTime = MonitorAlertEntity.COL_ALERT_TIME + val instanceStart = MonitorAlertEntity.COL_INSTANCE_START + val instanceEnd = MonitorAlertEntity.COL_INSTANCE_END + val allDay = MonitorAlertEntity.COL_ALL_DAY + val alertCreatedByUs = MonitorAlertEntity.COL_ALERT_CREATED_BY_US + val wasHandled = MonitorAlertEntity.COL_WAS_HANDLED + val i1 = MonitorAlertEntity.COL_RESERVED_INT1 + val i2 = MonitorAlertEntity.COL_RESERVED_INT2 + + DevLog.info(LOG_TAG, "Running pre-Room migration on test database") + + val db = android.database.sqlite.SQLiteDatabase.openDatabase( + dbFile.absolutePath, null, android.database.sqlite.SQLiteDatabase.OPEN_READWRITE + ) + + try { + db.beginTransaction() + try { + // Recreate table with NOT NULL on primary key columns + db.execSQL(""" + CREATE TABLE ${t}_new ( + $calendarId INTEGER, + $eventId INTEGER NOT NULL, + $alertTime INTEGER NOT NULL, + $instanceStart INTEGER NOT NULL, + $instanceEnd INTEGER, + $allDay INTEGER, + $alertCreatedByUs INTEGER, + $wasHandled INTEGER, + $i1 INTEGER, + $i2 INTEGER, + PRIMARY KEY ($eventId, $alertTime, $instanceStart) + ) + """) + + // Copy data + db.execSQL(""" + INSERT INTO ${t}_new + SELECT $calendarId, COALESCE($eventId, 0), COALESCE($alertTime, 0), + COALESCE($instanceStart, 0), $instanceEnd, $allDay, + $alertCreatedByUs, $wasHandled, $i1, $i2 + FROM $t + """) + + db.execSQL("DROP TABLE $t") + db.execSQL("ALTER TABLE ${t}_new RENAME TO $t") + db.execSQL("CREATE UNIQUE INDEX $idx ON $t ($eventId, $alertTime, $instanceStart)") + + db.setTransactionSuccessful() + } finally { + db.endTransaction() + } + DevLog.info(LOG_TAG, "Pre-Room migration complete") + } finally { + db.close() + } + } + + /** + * Tests that Room can open a legacy database and read existing data. + * + * This is the critical migration test - if this passes, existing users + * can safely upgrade to the Room-based implementation. + */ + @Test + fun test_room_opens_legacy_database_without_crash() { + DevLog.info(LOG_TAG, "=== test_room_opens_legacy_database_without_crash ===") + + // Step 1: Create legacy database with test data (no room_master_table) + val testData = createLegacyDatabase() + + // Step 2: Run pre-Room migration (same as MonitorDatabase does) + performPreRoomMigration() + + // Step 3: Open database with Room via MonitorDatabase + // This should NOT crash - Room should handle the pre-existing table + DevLog.info(LOG_TAG, "Opening legacy database with Room...") + + val roomDb = androidx.room.Room.databaseBuilder( + context, + MonitorDatabase::class.java, + LEGACY_DB_NAME + ) + .openHelperFactory(com.github.quarck.calnotify.database.CrSqliteRoomFactory()) + .allowMainThreadQueries() + .build() + + try { + // Step 4: Read data via Room DAO + val dao = roomDb.monitorAlertDao() + val alerts = dao.getAll() + + DevLog.info(LOG_TAG, "Room read ${alerts.size} alerts from legacy database") + + // Step 5: Verify data integrity + assertEquals("Should read all test alerts", testData.size, alerts.size) + + // Verify each row + for (expected in testData) { + val found = alerts.find { + it.eventId == expected.eventId && + it.alertTime == expected.alertTime && + it.instanceStartTime == expected.instanceStart + } + assertNotNull("Should find alert for eventId=${expected.eventId}", found) + assertEquals("alertCreatedByUs should match", expected.alertCreatedByUs, found!!.alertCreatedByUs) + assertEquals("wasHandled should match", expected.wasHandled, found.wasHandled) + assertEquals("allDay should match", expected.allDay, found.isAllDay) + } + + // Step 6: Verify Room created its metadata table + val supportDb = roomDb.openHelper.writableDatabase + supportDb.query("SELECT name FROM sqlite_master WHERE type='table' AND name='room_master_table'").use { cursor -> + assertTrue("Room should have created room_master_table", cursor.moveToFirst()) + } + + DevLog.info(LOG_TAG, "✅ Room successfully opened legacy database and preserved all data!") + + } finally { + roomDb.close() + } + } + + /** + * Tests that Room can write to a database that was originally created by SQLiteOpenHelper. + */ + @Test + fun test_room_can_write_to_legacy_database() { + DevLog.info(LOG_TAG, "=== test_room_can_write_to_legacy_database ===") + + // Create legacy database and migrate + createLegacyDatabase() + performPreRoomMigration() + + // Open with Room + val roomDb = androidx.room.Room.databaseBuilder( + context, + MonitorDatabase::class.java, + LEGACY_DB_NAME + ) + .openHelperFactory(com.github.quarck.calnotify.database.CrSqliteRoomFactory()) + .allowMainThreadQueries() + .build() + + try { + val dao = roomDb.monitorAlertDao() + + // Insert a new alert via Room + val newAlert = MonitorAlertEntity( + calendarId = 99, + eventId = 99999, + alertTime = 1735600000000, + instanceStartTime = 1735600000000, + instanceEndTime = 1735603600000, + isAllDay = 0, + alertCreatedByUs = 1, + wasHandled = 0 + ) + + dao.insert(newAlert) + DevLog.info(LOG_TAG, "Inserted new alert via Room") + + // Verify it was inserted + val retrieved = dao.getByKey(99999, 1735600000000, 1735600000000) + assertNotNull("Should retrieve newly inserted alert", retrieved) + assertEquals("eventId should match", 99999L, retrieved!!.eventId) + + // Verify total count increased + val allAlerts = dao.getAll() + assertEquals("Should have 4 alerts total (3 legacy + 1 new)", 4, allAlerts.size) + + DevLog.info(LOG_TAG, "✅ Room successfully wrote to legacy database!") + + } finally { + roomDb.close() + } + } + + /** + * Tests that Room can update existing records in a legacy database. + */ + @Test + fun test_room_can_update_legacy_records() { + DevLog.info(LOG_TAG, "=== test_room_can_update_legacy_records ===") + + // Create legacy database and migrate + createLegacyDatabase() + performPreRoomMigration() + + // Open with Room + val roomDb = androidx.room.Room.databaseBuilder( + context, + MonitorDatabase::class.java, + LEGACY_DB_NAME + ) + .openHelperFactory(com.github.quarck.calnotify.database.CrSqliteRoomFactory()) + .allowMainThreadQueries() + .build() + + try { + val dao = roomDb.monitorAlertDao() + + // Get the first alert (wasHandled = 0) + val alert = dao.getByKey(12345, 1735500000000, 1735500000000) + assertNotNull("Should find test alert", alert) + assertEquals("Should initially be unhandled", 0, alert!!.wasHandled) + + // Update via Room + val updated = alert.copy(wasHandled = 1) + dao.update(updated) + DevLog.info(LOG_TAG, "Updated alert via Room") + + // Verify update + val retrieved = dao.getByKey(12345, 1735500000000, 1735500000000) + assertEquals("wasHandled should be updated", 1, retrieved!!.wasHandled) + + DevLog.info(LOG_TAG, "✅ Room successfully updated legacy records!") + + } finally { + roomDb.close() + } + } + + /** + * Helper data class for test data + */ + private data class TestAlertData( + val calendarId: Long, + val eventId: Long, + val alertTime: Long, + val instanceStart: Long, + val instanceEnd: Long, + val allDay: Int, + val alertCreatedByUs: Int, + val wasHandled: Int + ) +} + diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/testutils/UITestFixture.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/testutils/UITestFixture.kt index 925a4e0b0..9ccedc414 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/testutils/UITestFixture.kt +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/testutils/UITestFixture.kt @@ -12,6 +12,7 @@ import androidx.test.uiautomator.Configurator import androidx.test.uiautomator.UiDevice import androidx.test.uiautomator.UiSelector import com.github.quarck.calnotify.Consts +import com.github.quarck.calnotify.Settings import com.github.quarck.calnotify.app.ApplicationController import com.github.quarck.calnotify.calendar.EventAlertRecord import com.github.quarck.calnotify.calendar.EventDisplayStatus @@ -52,6 +53,9 @@ class UITestFixture { // Track if calendar reload prevention is active private var calendarReloadPrevented = false + // Track if dialogs have been pre-suppressed (no need to dismiss them) + private var dialogsSuppressed = false + /** * Sets up the fixture. Call in @Before. * @@ -62,25 +66,42 @@ class UITestFixture { * to full MockCalendarProvider setup. * @param grantPermissions If true, grants runtime permissions (calendar, notifications) programmatically. * This speeds up tests by avoiding permission dialogs. + * @param suppressBatteryDialog If true, sets SharedPreference to suppress battery optimization dialog. + * Combined with grantPermissions, this eliminates all startup dialogs. */ fun setup( waitForAsyncTasks: Boolean = false, preventCalendarReload: Boolean = false, - grantPermissions: Boolean = false + grantPermissions: Boolean = false, + suppressBatteryDialog: Boolean = false ) { - DevLog.info(LOG_TAG, "Setting up UITestFixture (waitForAsyncTasks=$waitForAsyncTasks, preventCalendarReload=$preventCalendarReload, grantPermissions=$grantPermissions)") + DevLog.info(LOG_TAG, "Setting up UITestFixture (waitForAsyncTasks=$waitForAsyncTasks, preventCalendarReload=$preventCalendarReload, grantPermissions=$grantPermissions, suppressBatteryDialog=$suppressBatteryDialog)") // Reset dialog flag so each test can handle dialogs if they appear startupDialogsDismissed = false + dialogsSuppressed = false clearAllEvents() + // Suppress battery optimization dialog if requested + // This must be done BEFORE launching activity + if (suppressBatteryDialog) { + suppressBatteryOptimizationDialog() + } + // Grant runtime permissions programmatically if requested // This speeds up tests by avoiding permission dialogs if (grantPermissions) { grantPermissions() } + // If both permissions granted and battery dialog suppressed, no dialogs will appear + if (grantPermissions && suppressBatteryDialog) { + dialogsSuppressed = true + startupDialogsDismissed = true // Skip dialog dismissal entirely + DevLog.info(LOG_TAG, "All dialogs suppressed - skipping dialog dismissal") + } + // Prevent calendar reload if needed - this stops CalendarReloadManager from clearing test events if (preventCalendarReload) { setupCalendarReloadPrevention() @@ -96,6 +117,20 @@ class UITestFixture { } } + /** + * Suppresses the battery optimization dialog by setting SharedPreference. + * Must be called before launching MainActivity. + */ + private fun suppressBatteryOptimizationDialog() { + try { + val settings = Settings(context) + settings.doNotShowBatteryOptimisationWarning = true + DevLog.info(LOG_TAG, "Suppressed battery optimization dialog via SharedPreference") + } catch (e: Exception) { + DevLog.error(LOG_TAG, "Failed to suppress battery dialog: ${e.message}") + } + } + /** * Grants runtime permissions programmatically for UI tests. * This ensures tests work in isolation without requiring permission dialogs. @@ -283,6 +318,15 @@ class UITestFixture { clearAllEvents() calendarReloadPrevented = false + dialogsSuppressed = false + + // Reset battery optimization dialog setting + try { + val settings = Settings(context) + settings.doNotShowBatteryOptimisationWarning = false + } catch (e: Exception) { + // Ignore - settings might not be accessible + } // Unregister IdlingResource if it was registered try { diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/BaseUltronTest.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/BaseUltronTest.kt index 8f5ee6ac8..11497bf89 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/BaseUltronTest.kt +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/BaseUltronTest.kt @@ -8,18 +8,35 @@ import org.junit.BeforeClass * Base class for Ultron UI tests with centralized configuration. * * All UI test classes should extend this to get: - * - 15-second operation timeout (faster than default) + * - 5-second operation timeout (fast, assumes IdlingResource handles async waits) * - Automatic screenshot capture on failures (Allure integration) * + * For tests using waitForAsyncTasks=true in UITestFixture, the IdlingResource + * handles synchronization with background operations, so Ultron doesn't need + * to poll for long periods. + * * Individual test classes can override setConfig() if they need custom settings. */ abstract class BaseUltronTest { companion object { + /** + * Default timeout for Ultron operations. + * With IdlingResource synchronization, 5 seconds is plenty. + * Tests without IdlingResource may need longer timeouts. + */ + const val DEFAULT_TIMEOUT_MS = 5_000L + + /** + * Longer timeout for tests without IdlingResource. + * Use sparingly - prefer IdlingResource for proper synchronization. + */ + const val EXTENDED_TIMEOUT_MS = 15_000L + @BeforeClass @JvmStatic fun setConfig() { UltronConfig.apply { - operationTimeoutMs = 15_000 // 15 seconds instead of 60s default + operationTimeoutMs = DEFAULT_TIMEOUT_MS } UltronAllureConfig.applyRecommended() // Enable automatic screenshots on failure } diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/DismissedEventsActivityTest.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/DismissedEventsActivityTest.kt index a70bf29db..f9e2ec125 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/DismissedEventsActivityTest.kt +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/DismissedEventsActivityTest.kt @@ -31,7 +31,15 @@ class DismissedEventsActivityTest : BaseUltronTest() { @Before fun setup() { fixture = UITestFixture.create() - fixture.setup() + // Full optimization for fast UI tests: + // - waitForAsyncTasks: Wait for background data loading instead of polling with timeouts + // - grantPermissions: Avoid permission dialogs + // - suppressBatteryDialog: Suppress battery optimization dialog + fixture.setup( + waitForAsyncTasks = true, + grantPermissions = true, + suppressBatteryDialog = true + ) } @After diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/MainActivityTest.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/MainActivityTest.kt index 9d568d0b9..17dee4385 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/MainActivityTest.kt +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/MainActivityTest.kt @@ -33,11 +33,17 @@ class MainActivityTest : BaseUltronTest() { @Before fun setup() { fixture = UITestFixture.create() - // Prevent calendar reloads that would clear test events from EventsStorage - // This is a lightweight mock that just stops ApplicationController.onMainActivityResumed - // from triggering background calendar scans - // Grant permissions to avoid permission dialogs (faster tests) - fixture.setup(preventCalendarReload = true, grantPermissions = true) + // Full optimization for fast UI tests: + // - waitForAsyncTasks: Wait for background data loading instead of polling with timeouts + // - preventCalendarReload: Stop calendar rescans from clearing test events + // - grantPermissions: Avoid permission dialogs + // - suppressBatteryDialog: Suppress battery optimization dialog + fixture.setup( + waitForAsyncTasks = true, + preventCalendarReload = true, + grantPermissions = true, + suppressBatteryDialog = true + ) } @After diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/SettingsActivityTest.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/SettingsActivityTest.kt index cee0ce57a..e24a18c7f 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/SettingsActivityTest.kt +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/SettingsActivityTest.kt @@ -28,7 +28,14 @@ class SettingsActivityTest : BaseUltronTest() { @Before fun setup() { fixture = UITestFixture.create() - fixture.setup() + // Optimized for fast UI tests: + // - grantPermissions: Avoid permission dialogs + // - suppressBatteryDialog: Suppress battery optimization dialog + // Note: No waitForAsyncTasks needed - SettingsActivity is a static screen + fixture.setup( + grantPermissions = true, + suppressBatteryDialog = true + ) } @After diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/SnoozeAllActivityTest.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/SnoozeAllActivityTest.kt index 2ab8932ce..f3e8ab363 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/SnoozeAllActivityTest.kt +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/SnoozeAllActivityTest.kt @@ -32,7 +32,14 @@ class SnoozeAllActivityTest : BaseUltronTest() { @Before fun setup() { fixture = UITestFixture.create() - fixture.setup() + // Optimized for fast UI tests: + // - grantPermissions: Avoid permission dialogs + // - suppressBatteryDialog: Suppress battery optimization dialog + // Note: No waitForAsyncTasks needed - SnoozeAllActivity has no background data loading + fixture.setup( + grantPermissions = true, + suppressBatteryDialog = true + ) } @After diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/ViewEventActivityTest.kt b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/ViewEventActivityTest.kt index 0e894855c..907d2ec5e 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/ViewEventActivityTest.kt +++ b/android/app/src/androidTest/java/com/github/quarck/calnotify/ui/ViewEventActivityTest.kt @@ -32,9 +32,14 @@ class ViewEventActivityTest : BaseUltronTest() { @Before fun setup() { fixture = UITestFixture.create() - // Grant permissions programmatically so ViewEventActivity doesn't finish() immediately - // This allows the test to work in isolation while other tests can still test permission dialogs - fixture.setup(grantPermissions = true) + // Optimized for fast UI tests: + // - grantPermissions: Avoid permission dialogs (also prevents activity from finishing) + // - suppressBatteryDialog: Suppress battery optimization dialog + // Note: No waitForAsyncTasks needed - event is already in storage from test setup + fixture.setup( + grantPermissions = true, + suppressBatteryDialog = true + ) } @After diff --git a/android/app/src/androidTest/java/de/schroepf/androidxmlrunlistener/XmlRunListener.java b/android/app/src/androidTest/java/de/schroepf/androidxmlrunlistener/XmlRunListener.java index 9cdbf429f..ced78984c 100644 --- a/android/app/src/androidTest/java/de/schroepf/androidxmlrunlistener/XmlRunListener.java +++ b/android/app/src/androidTest/java/de/schroepf/androidxmlrunlistener/XmlRunListener.java @@ -19,8 +19,12 @@ import androidx.test.internal.runner.listener.InstrumentationRunListener; /** - * An InstrumentationRunListener which writes the test results to JUnit style XML files to the - * {@code /storage/emulated/0/Android/data//files/} directory on the device. + * An InstrumentationRunListener which writes the test results to JUnit style XML files. + *

+ * By default, writes to internal storage {@code /data/data//files/} which works + * reliably on all Android versions including API 30+ with scoped storage. + *

+ * Supports custom output path via {@code -e reportFile } instrumentation argument. *

* This listener will not override existing XML reports and instead will generate unique file names * for the report file (report-0.xml, report-1.xml ...). @@ -102,14 +106,29 @@ public void setInstrumentation(Instrumentation instr) { * Get a {@link File} for the test report. *

* Override this to change the default file location. + *

+ * Supports the -e reportFile argument to specify a custom output path. + * If not specified, defaults to internal storage (getFilesDir) which works + * reliably on API 30+ where scoped storage blocks external storage access. * * @param instrumentation the {@link Instrumentation} for this test run * @return the file which should be used to store the XML report of the test run */ protected File getOutputFile(Instrumentation instrumentation) { - // Seems like we need to put this into the target application's context as for the instrumentation app's - // context we can never be sure if we have the correct permissions - and getFilesDir() seems to return null - return new File(instrumentation.getTargetContext().getExternalFilesDir(null), getFileName(instrumentation)); + // Check for custom reportFile argument + android.os.Bundle args = androidx.test.platform.app.InstrumentationRegistry.getArguments(); + String reportFilePath = args.getString("reportFile"); + + if (reportFilePath != null && !reportFilePath.isEmpty()) { + Log.d(TAG, "Using custom report file path: " + reportFilePath); + return new File(reportFilePath); + } + + // Default to internal storage (getFilesDir) - works reliably on API 30+ + // External storage (getExternalFilesDir) is blocked by scoped storage on newer Android + File filesDir = instrumentation.getTargetContext().getFilesDir(); + Log.d(TAG, "Using internal storage for report: " + filesDir); + return new File(filesDir, getFileName(instrumentation)); } /** @@ -126,7 +145,8 @@ protected String getFileName(Instrumentation instrumentation) { private String findFile(String fileNamePrefix, int iterator, String fileNamePostfix, Instrumentation instr) { String fileName = fileNamePrefix + "-" + iterator + fileNamePostfix; - File file = new File(instr.getTargetContext().getExternalFilesDir(null), fileName); + // Use internal storage (getFilesDir) for consistency with getOutputFile + File file = new File(instr.getTargetContext().getFilesDir(), fileName); if (file.exists()) { return findFile(fileNamePrefix, iterator + 1, fileNamePostfix, instr); diff --git a/android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/CrSqliteSupportHelper.kt b/android/app/src/main/java/com/github/quarck/calnotify/database/CrSqliteRoomFactory.kt similarity index 98% rename from android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/CrSqliteSupportHelper.kt rename to android/app/src/main/java/com/github/quarck/calnotify/database/CrSqliteRoomFactory.kt index 954240198..09f258db7 100644 --- a/android/app/src/androidTest/java/com/github/quarck/calnotify/database/poc/CrSqliteSupportHelper.kt +++ b/android/app/src/main/java/com/github/quarck/calnotify/database/CrSqliteRoomFactory.kt @@ -17,7 +17,7 @@ // Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // -package com.github.quarck.calnotify.database.poc +package com.github.quarck.calnotify.database import androidx.sqlite.db.SupportSQLiteOpenHelper import com.github.quarck.calnotify.logs.DevLog @@ -77,3 +77,4 @@ class CrSqliteFinalizeWrapper( delegate.close() } } + diff --git a/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorAlertDao.kt b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorAlertDao.kt new file mode 100644 index 000000000..b133dc217 --- /dev/null +++ b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorAlertDao.kt @@ -0,0 +1,83 @@ +// +// Calendar Notifications Plus +// Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com) +// +// 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, write to the Free Software Foundation, +// Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +package com.github.quarck.calnotify.monitorstorage + +import androidx.room.Dao +import androidx.room.Delete +import androidx.room.Insert +import androidx.room.OnConflictStrategy +import androidx.room.Query +import androidx.room.Update + +/** + * Room DAO for MonitorAlertEntity. + * + * Provides all database operations needed by MonitorStorageInterface. + * + * Note: Room @Query annotations require string literals, so we can't use + * MonitorAlertEntity constants directly in queries. The table/column names + * are validated at compile time against the @Entity definition. + */ +@Dao +interface MonitorAlertDao { + + @Query("SELECT * FROM ${MonitorAlertEntity.TABLE_NAME}") + fun getAll(): List + + @Query("SELECT * FROM ${MonitorAlertEntity.TABLE_NAME} WHERE ${MonitorAlertEntity.COL_EVENT_ID} = :eventId AND ${MonitorAlertEntity.COL_ALERT_TIME} = :alertTime AND ${MonitorAlertEntity.COL_INSTANCE_START} = :instanceStart") + fun getByKey(eventId: Long, alertTime: Long, instanceStart: Long): MonitorAlertEntity? + + @Query("SELECT * FROM ${MonitorAlertEntity.TABLE_NAME} WHERE ${MonitorAlertEntity.COL_EVENT_ID} = :eventId AND ${MonitorAlertEntity.COL_INSTANCE_START} = :instanceStart") + fun getByEventAndInstance(eventId: Long, instanceStart: Long): List + + @Query("SELECT * FROM ${MonitorAlertEntity.TABLE_NAME} WHERE ${MonitorAlertEntity.COL_ALERT_TIME} = :time") + fun getByAlertTime(time: Long): List + + @Query("SELECT MIN(${MonitorAlertEntity.COL_ALERT_TIME}) FROM ${MonitorAlertEntity.TABLE_NAME} WHERE ${MonitorAlertEntity.COL_ALERT_TIME} >= :since") + fun getNextAlertTime(since: Long): Long? + + @Query("SELECT * FROM ${MonitorAlertEntity.TABLE_NAME} WHERE ${MonitorAlertEntity.COL_INSTANCE_START} >= :scanFrom AND ${MonitorAlertEntity.COL_INSTANCE_START} <= :scanTo") + fun getByInstanceStartRange(scanFrom: Long, scanTo: Long): List + + @Query("SELECT * FROM ${MonitorAlertEntity.TABLE_NAME} WHERE ${MonitorAlertEntity.COL_ALERT_TIME} >= :scanFrom AND ${MonitorAlertEntity.COL_ALERT_TIME} <= :scanTo") + fun getByAlertTimeRange(scanFrom: Long, scanTo: Long): List + + @Insert(onConflict = OnConflictStrategy.REPLACE) + fun insert(entity: MonitorAlertEntity) + + @Insert(onConflict = OnConflictStrategy.REPLACE) + fun insertAll(entities: List) + + @Update + fun update(entity: MonitorAlertEntity) + + @Update + fun updateAll(entities: List) + + @Delete + fun delete(entity: MonitorAlertEntity) + + @Delete + fun deleteAll(entities: List) + + @Query("DELETE FROM ${MonitorAlertEntity.TABLE_NAME} WHERE ${MonitorAlertEntity.COL_EVENT_ID} = :eventId AND ${MonitorAlertEntity.COL_ALERT_TIME} = :alertTime AND ${MonitorAlertEntity.COL_INSTANCE_START} = :instanceStart") + fun deleteByKey(eventId: Long, alertTime: Long, instanceStart: Long) +} + diff --git a/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorAlertEntity.kt b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorAlertEntity.kt new file mode 100644 index 000000000..5d0a96520 --- /dev/null +++ b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorAlertEntity.kt @@ -0,0 +1,99 @@ +// +// Calendar Notifications Plus +// Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com) +// +// 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, write to the Free Software Foundation, +// Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +package com.github.quarck.calnotify.monitorstorage + +import androidx.room.ColumnInfo +import androidx.room.Entity +import androidx.room.Index +import com.github.quarck.calnotify.calendar.MonitorEventAlertEntry + +/** + * Room entity matching the manualAlertsV1 table schema. + * + * Schema compatibility: + * - Primary key columns (eventId, alertTime, instanceStart) are NOT NULL + * - Other columns are nullable to match legacy schema + * - Index matches legacy `manualAlertsV1IdxV1` index + * + * The legacy database is migrated to add NOT NULL constraints on PK columns + * before Room opens it. See MonitorDatabase.migrateLegacyDatabaseIfNeeded(). + */ +@Entity( + tableName = MonitorAlertEntity.TABLE_NAME, + primaryKeys = [MonitorAlertEntity.COL_EVENT_ID, MonitorAlertEntity.COL_ALERT_TIME, MonitorAlertEntity.COL_INSTANCE_START], + indices = [Index( + value = [MonitorAlertEntity.COL_EVENT_ID, MonitorAlertEntity.COL_ALERT_TIME, MonitorAlertEntity.COL_INSTANCE_START], + unique = true, + name = MonitorAlertEntity.INDEX_NAME + )] +) +data class MonitorAlertEntity( + @ColumnInfo(name = COL_CALENDAR_ID) val calendarId: Long? = -1, + @ColumnInfo(name = COL_EVENT_ID) val eventId: Long, + @ColumnInfo(name = COL_ALERT_TIME) val alertTime: Long, + @ColumnInfo(name = COL_INSTANCE_START) val instanceStartTime: Long, + @ColumnInfo(name = COL_INSTANCE_END) val instanceEndTime: Long? = 0, + @ColumnInfo(name = COL_ALL_DAY) val isAllDay: Int? = 0, + @ColumnInfo(name = COL_ALERT_CREATED_BY_US) val alertCreatedByUs: Int? = 0, + @ColumnInfo(name = COL_WAS_HANDLED) val wasHandled: Int? = 0, + @ColumnInfo(name = COL_RESERVED_INT1) val reservedInt1: Long? = 0, + @ColumnInfo(name = COL_RESERVED_INT2) val reservedInt2: Long? = 0 +) { + /** Convert to domain model (handles potential nulls) */ + fun toAlertEntry() = MonitorEventAlertEntry( + eventId = eventId, + alertTime = alertTime, + instanceStartTime = instanceStartTime, + instanceEndTime = instanceEndTime ?: 0, + isAllDay = (isAllDay ?: 0) != 0, + alertCreatedByUs = (alertCreatedByUs ?: 0) != 0, + wasHandled = (wasHandled ?: 0) != 0 + ) + + companion object { + // Table and index names + const val TABLE_NAME = "manualAlertsV1" + const val INDEX_NAME = "manualAlertsV1IdxV1" + + // Column names (must match legacy schema exactly) + const val COL_CALENDAR_ID = "calendarId" + const val COL_EVENT_ID = "eventId" + const val COL_ALERT_TIME = "alertTime" + const val COL_INSTANCE_START = "instanceStart" + const val COL_INSTANCE_END = "instanceEnd" + const val COL_ALL_DAY = "allDay" + const val COL_ALERT_CREATED_BY_US = "alertCreatedByUs" + const val COL_WAS_HANDLED = "wasHandled" + const val COL_RESERVED_INT1 = "i1" + const val COL_RESERVED_INT2 = "i2" + + /** Convert from domain model */ + fun fromAlertEntry(entry: MonitorEventAlertEntry) = MonitorAlertEntity( + eventId = entry.eventId, + alertTime = entry.alertTime, + instanceStartTime = entry.instanceStartTime, + instanceEndTime = entry.instanceEndTime, + isAllDay = if (entry.isAllDay) 1 else 0, + alertCreatedByUs = if (entry.alertCreatedByUs) 1 else 0, + wasHandled = if (entry.wasHandled) 1 else 0 + ) + } +} + diff --git a/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorDatabase.kt b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorDatabase.kt new file mode 100644 index 000000000..47e52e5d5 --- /dev/null +++ b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorDatabase.kt @@ -0,0 +1,205 @@ +// +// Calendar Notifications Plus +// Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com) +// +// 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, write to the Free Software Foundation, +// Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +package com.github.quarck.calnotify.monitorstorage + +import android.content.Context +import android.database.sqlite.SQLiteDatabase +import androidx.room.Database +import androidx.room.Room +import androidx.room.RoomDatabase +import androidx.sqlite.db.SupportSQLiteDatabase +import com.github.quarck.calnotify.database.CrSqliteRoomFactory +import com.github.quarck.calnotify.logs.DevLog +import java.io.File + +/** + * Room database for MonitorStorage. + * + * Uses the existing "CalendarMonitor" database file to read existing data. + * Uses CrSqliteRoomFactory for cr-sqlite extension support. + * + * MIGRATION NOTE: This Room database takes over from legacy SQLiteOpenHelper. + * Before Room opens, we migrate the legacy schema to add NOT NULL constraints + * on primary key columns (required by Room but missing in legacy schema). + */ +@Database( + entities = [MonitorAlertEntity::class], + version = 1, + exportSchema = false +) +abstract class MonitorDatabase : RoomDatabase() { + + abstract fun monitorAlertDao(): MonitorAlertDao + + companion object { + private const val LOG_TAG = "MonitorDatabase" + private const val DATABASE_NAME = "CalendarMonitor" + + @Volatile + private var INSTANCE: MonitorDatabase? = null + + fun getInstance(context: Context): MonitorDatabase { + return INSTANCE ?: synchronized(this) { + INSTANCE ?: run { + // Migrate legacy database BEFORE Room opens it + migrateLegacyDatabaseIfNeeded(context) + buildDatabase(context).also { INSTANCE = it } + } + } + } + + private fun buildDatabase(context: Context): MonitorDatabase { + return Room.databaseBuilder( + context.applicationContext, + MonitorDatabase::class.java, + DATABASE_NAME + ) + .openHelperFactory(CrSqliteRoomFactory()) + .allowMainThreadQueries() // Match existing behavior - MonitorStorage uses sync queries + .addCallback(LoggingCallback()) + .build() + } + + /** + * Migrate legacy SQLiteOpenHelper database to Room-compatible schema. + * + * This runs BEFORE Room opens the database because Room validates schema + * before running any migrations. For pre-Room databases (no room_master_table), + * the schema must match exactly or Room will reject it. + * + * Changes made: + * - Adds NOT NULL constraints to primary key columns (Room requirement) + * - Preserves the existing index + * - All data is preserved + */ + private fun migrateLegacyDatabaseIfNeeded(context: Context) { + val dbFile = context.getDatabasePath(DATABASE_NAME) + if (!dbFile.exists()) { + DevLog.info(LOG_TAG, "No existing database - fresh install") + return + } + + val db = SQLiteDatabase.openDatabase(dbFile.absolutePath, null, SQLiteDatabase.OPEN_READWRITE) + try { + if (!isLegacyDatabase(db)) return + performLegacyMigration(db) + } finally { + db.close() + } + } + + private fun isLegacyDatabase(db: SQLiteDatabase): Boolean { + val tableName = MonitorAlertEntity.TABLE_NAME + + val hasTable = db.rawQuery( + "SELECT 1 FROM sqlite_master WHERE type='table' AND name='$tableName'", null + ).use { it.moveToFirst() } + + val hasRoomTable = db.rawQuery( + "SELECT 1 FROM sqlite_master WHERE type='table' AND name='room_master_table'", null + ).use { it.moveToFirst() } + + return when { + !hasTable -> { DevLog.info(LOG_TAG, "No legacy table - nothing to migrate"); false } + hasRoomTable -> { DevLog.info(LOG_TAG, "Already Room-managed"); false } + else -> { DevLog.info(LOG_TAG, "Legacy database detected"); true } + } + } + + private fun performLegacyMigration(db: SQLiteDatabase) { + val t = MonitorAlertEntity.TABLE_NAME + val idx = MonitorAlertEntity.INDEX_NAME + // Column names + val calendarId = MonitorAlertEntity.COL_CALENDAR_ID + val eventId = MonitorAlertEntity.COL_EVENT_ID + val alertTime = MonitorAlertEntity.COL_ALERT_TIME + val instanceStart = MonitorAlertEntity.COL_INSTANCE_START + val instanceEnd = MonitorAlertEntity.COL_INSTANCE_END + val allDay = MonitorAlertEntity.COL_ALL_DAY + val alertCreatedByUs = MonitorAlertEntity.COL_ALERT_CREATED_BY_US + val wasHandled = MonitorAlertEntity.COL_WAS_HANDLED + val i1 = MonitorAlertEntity.COL_RESERVED_INT1 + val i2 = MonitorAlertEntity.COL_RESERVED_INT2 + + val rowCountBefore = db.rawQuery("SELECT COUNT(*) FROM $t", null) + .use { if (it.moveToFirst()) it.getLong(0) else 0 } + DevLog.info(LOG_TAG, "Migrating $rowCountBefore rows to Room-compatible schema") + + db.beginTransaction() + try { + // Recreate table with NOT NULL on primary key columns + db.execSQL(""" + CREATE TABLE ${t}_new ( + $calendarId INTEGER, + $eventId INTEGER NOT NULL, + $alertTime INTEGER NOT NULL, + $instanceStart INTEGER NOT NULL, + $instanceEnd INTEGER, + $allDay INTEGER, + $alertCreatedByUs INTEGER, + $wasHandled INTEGER, + $i1 INTEGER, + $i2 INTEGER, + PRIMARY KEY ($eventId, $alertTime, $instanceStart) + ) + """) + + // Copy data (COALESCE ensures no nulls in PK columns) + db.execSQL(""" + INSERT INTO ${t}_new + SELECT $calendarId, COALESCE($eventId, 0), COALESCE($alertTime, 0), + COALESCE($instanceStart, 0), $instanceEnd, $allDay, + $alertCreatedByUs, $wasHandled, $i1, $i2 + FROM $t + """) + + db.execSQL("DROP TABLE $t") + db.execSQL("ALTER TABLE ${t}_new RENAME TO $t") + db.execSQL("CREATE UNIQUE INDEX $idx ON $t ($eventId, $alertTime, $instanceStart)") + + db.setTransactionSuccessful() + } finally { + db.endTransaction() + } + + val rowCountAfter = db.rawQuery("SELECT COUNT(*) FROM $t", null) + .use { if (it.moveToFirst()) it.getLong(0) else 0 } + DevLog.info(LOG_TAG, "Migration complete: $rowCountAfter rows") + + if (rowCountAfter != rowCountBefore) { + DevLog.error(LOG_TAG, "Row count mismatch! Before=$rowCountBefore, After=$rowCountAfter") + } + } + + /** Simple callback for logging */ + private class LoggingCallback : Callback() { + override fun onOpen(db: SupportSQLiteDatabase) { + super.onOpen(db) + DevLog.info(LOG_TAG, "Room database opened, version=${db.version}") + } + + override fun onCreate(db: SupportSQLiteDatabase) { + super.onCreate(db) + DevLog.info(LOG_TAG, "Room database created (fresh install)") + } + } + } +} + diff --git a/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorStorage.kt b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorStorage.kt index 9e9f0b970..fc3726264 100644 --- a/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorStorage.kt +++ b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorStorage.kt @@ -1,6 +1,7 @@ // // Calendar Notifications Plus // Copyright (C) 2017 Sergey Parshin (s.parshin.sc@gmail.com) +// Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com) // // 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 @@ -17,92 +18,30 @@ // Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // - package com.github.quarck.calnotify.monitorstorage import android.content.Context -import io.requery.android.database.sqlite.SQLiteDatabase -import com.github.quarck.calnotify.database.SQLiteOpenHelper -import com.github.quarck.calnotify.calendar.MonitorEventAlertEntry -import com.github.quarck.calnotify.logs.DevLog -//import com.github.quarck.calnotify.logs.Logger import java.io.Closeable -import com.github.quarck.calnotify.database.SQLiteDatabaseExtensions.customUse - - - -class MonitorStorage(val context: Context) - : SQLiteOpenHelper(context, DATABASE_NAME, null, DATABASE_CURRENT_VERSION), Closeable, MonitorStorageInterface { - - private var impl: MonitorStorageImplInterface - - init { - impl = MonitorStorageImplV1(context); - } - - override fun onCreate(db: SQLiteDatabase) - = impl.createDb(db) - - override fun onUpgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { - DevLog.info(LOG_TAG, "onUpgrade $oldVersion -> $newVersion") - - if (oldVersion != newVersion) { - throw Exception("DB storage error: upgrade from $oldVersion to $newVersion is not supported") - } - } - - override fun addAlert(entry: MonitorEventAlertEntry) - = synchronized(MonitorStorage::class.java) { writableDatabase.customUse { impl.addAlert(it, entry) } } - - override fun addAlerts(entries: Collection) - = synchronized(MonitorStorage::class.java) { writableDatabase.customUse { impl.addAlerts(it, entries) } } - - override fun deleteAlert(entry: MonitorEventAlertEntry) - = deleteAlert(entry.eventId, entry.alertTime, entry.instanceStartTime) - - override fun deleteAlerts(entries: Collection) - = synchronized(MonitorStorage::class.java) { writableDatabase.customUse { impl.deleteAlerts(it, entries) } } - - override fun deleteAlert(eventId: Long, alertTime: Long, instanceStart: Long) - = synchronized(MonitorStorage::class.java) { writableDatabase.customUse { impl.deleteAlert(it, eventId, alertTime, instanceStart) } } - - override fun deleteAlertsMatching(filter: (MonitorEventAlertEntry) -> Boolean) - = synchronized(MonitorStorage::class.java) { writableDatabase.customUse { impl.deleteAlertsMatching(it, filter) } } - - override fun updateAlert(entry: MonitorEventAlertEntry) - = synchronized(MonitorStorage::class.java) { writableDatabase.customUse { impl.updateAlert(it, entry) } } - - override fun updateAlerts(entries: Collection) - = synchronized(MonitorStorage::class.java) { writableDatabase.customUse { impl.updateAlerts(it, entries) } } - - override fun getAlert(eventId: Long, alertTime: Long, instanceStart: Long): MonitorEventAlertEntry? - = synchronized(MonitorStorage::class.java) { readableDatabase.customUse { impl.getAlert(it, eventId, alertTime, instanceStart) } } - - override fun getInstanceAlerts(eventId: Long, instanceStart: Long): List - = synchronized(MonitorStorage::class.java) { readableDatabase.customUse { impl.getInstanceAlerts(it, eventId, instanceStart) } } - - override fun getNextAlert(since: Long): Long? - = synchronized(MonitorStorage::class.java) { readableDatabase.customUse { impl.getNextAlert(it, since) } } - - override fun getAlertsAt(time: Long): List - = synchronized(MonitorStorage::class.java) { readableDatabase.customUse { impl.getAlertsAt(it, time) } } - - override val alerts: List - get() = synchronized(MonitorStorage::class.java) { readableDatabase.customUse { impl.getAlerts(it) } } - - override fun getAlertsForInstanceStartRange(scanFrom: Long, scanTo: Long): List - = synchronized(MonitorStorage::class.java) { readableDatabase.customUse { impl.getAlertsForInstanceStartRange(it, scanFrom, scanTo) } } - - override fun getAlertsForAlertRange(scanFrom: Long, scanTo: Long): List - = synchronized(MonitorStorage::class.java) { readableDatabase.customUse { impl.getAlertsForAlertRange(it, scanFrom, scanTo) } } +/** + * MonitorStorage - tracks calendar alerts being monitored. + * + * Now backed by Room database with cr-sqlite support. + * Maintains API compatibility with existing callers via delegation. + * + * Note: This class previously extended SQLiteOpenHelper with: + * - DATABASE_NAME = "CalendarMonitor" → now in MonitorDatabase.DATABASE_NAME + * - DATABASE_VERSION = 1 → now in @Database(version = 1) on MonitorDatabase + * Room handles version tracking automatically via room_master_table. + */ +class MonitorStorage private constructor( + private val delegate: RoomMonitorStorage +) : MonitorStorageInterface by delegate, Closeable by delegate { + + constructor(context: Context) : this(RoomMonitorStorage(context)) companion object { + @Suppress("unused") private const val LOG_TAG = "MonitorStorage" - - private const val DATABASE_VERSION_V1 = 1 - private const val DATABASE_CURRENT_VERSION = DATABASE_VERSION_V1 - - private const val DATABASE_NAME = "CalendarMonitor" } } diff --git a/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorStorageImplV1.kt b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorStorageImplV1.kt index a8ed356e2..2cf823b62 100644 --- a/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorStorageImplV1.kt +++ b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/MonitorStorageImplV1.kt @@ -466,21 +466,24 @@ class MonitorStorageImplV1(val context: Context) : MonitorStorageImplInterface { companion object { private const val LOG_TAG = "MonitorStorageImplV1" - private const val TABLE_NAME = "manualAlertsV1" - private const val INDEX_NAME = "manualAlertsV1IdxV1" - - private const val KEY_CALENDAR_ID = "calendarId" - private const val KEY_EVENTID = "eventId" - private const val KEY_ALL_DAY = "allDay" - private const val KEY_ALERT_TIME = "alertTime" - private const val KEY_INSTANCE_START = "instanceStart" - private const val KEY_INSTANCE_END = "instanceEnd" - private const val KEY_WE_CREATED_ALERT = "alertCreatedByUs" - private const val KEY_WAS_HANDLED = "wasHandled" - - - private const val KEY_RESERVED_INT1 = "i1" - private const val KEY_RESERVED_INT2 = "i2" + // Delegate to MonitorAlertEntity - the canonical source for schema constants + @Deprecated("Use MonitorAlertEntity.TABLE_NAME", ReplaceWith("MonitorAlertEntity.TABLE_NAME")) + internal const val TABLE_NAME = MonitorAlertEntity.TABLE_NAME + @Deprecated("Use MonitorAlertEntity.INDEX_NAME", ReplaceWith("MonitorAlertEntity.INDEX_NAME")) + internal const val INDEX_NAME = MonitorAlertEntity.INDEX_NAME + + // Column name constants - delegate to Entity + private const val KEY_CALENDAR_ID = MonitorAlertEntity.COL_CALENDAR_ID + private const val KEY_EVENTID = MonitorAlertEntity.COL_EVENT_ID + private const val KEY_ALL_DAY = MonitorAlertEntity.COL_ALL_DAY + private const val KEY_ALERT_TIME = MonitorAlertEntity.COL_ALERT_TIME + private const val KEY_INSTANCE_START = MonitorAlertEntity.COL_INSTANCE_START + private const val KEY_INSTANCE_END = MonitorAlertEntity.COL_INSTANCE_END + private const val KEY_WE_CREATED_ALERT = MonitorAlertEntity.COL_ALERT_CREATED_BY_US + private const val KEY_WAS_HANDLED = MonitorAlertEntity.COL_WAS_HANDLED + + private const val KEY_RESERVED_INT1 = MonitorAlertEntity.COL_RESERVED_INT1 + private const val KEY_RESERVED_INT2 = MonitorAlertEntity.COL_RESERVED_INT2 private val SELECT_COLUMNS = arrayOf( KEY_CALENDAR_ID, diff --git a/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/RoomMonitorStorage.kt b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/RoomMonitorStorage.kt new file mode 100644 index 000000000..af4c4db87 --- /dev/null +++ b/android/app/src/main/java/com/github/quarck/calnotify/monitorstorage/RoomMonitorStorage.kt @@ -0,0 +1,108 @@ +// +// Calendar Notifications Plus +// Copyright (C) 2025 William Harris (wharris+cnplus@upscalews.com) +// +// 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, write to the Free Software Foundation, +// Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +package com.github.quarck.calnotify.monitorstorage + +import android.content.Context +import com.github.quarck.calnotify.calendar.MonitorEventAlertEntry +import java.io.Closeable + +/** + * Room-based implementation of MonitorStorageInterface. + * + * Replaces the legacy SQLiteOpenHelper-based MonitorStorage. + * Uses MonitorDatabase with cr-sqlite support via CrSqliteRoomFactory. + */ +class RoomMonitorStorage(context: Context) : MonitorStorageInterface, Closeable { + + private val database = MonitorDatabase.getInstance(context) + private val dao = database.monitorAlertDao() + + override fun addAlert(entry: MonitorEventAlertEntry) { + dao.insert(MonitorAlertEntity.fromAlertEntry(entry)) + } + + override fun addAlerts(entries: Collection) { + dao.insertAll(entries.map { MonitorAlertEntity.fromAlertEntry(it) }) + } + + override fun deleteAlert(entry: MonitorEventAlertEntry) { + deleteAlert(entry.eventId, entry.alertTime, entry.instanceStartTime) + } + + override fun deleteAlerts(entries: Collection) { + dao.deleteAll(entries.map { MonitorAlertEntity.fromAlertEntry(it) }) + } + + override fun deleteAlert(eventId: Long, alertTime: Long, instanceStart: Long) { + dao.deleteByKey(eventId, alertTime, instanceStart) + } + + override fun deleteAlertsMatching(filter: (MonitorEventAlertEntry) -> Boolean) { + // Wrap in transaction to ensure atomicity (read + delete as single unit) + database.runInTransaction { + val allAlerts = dao.getAll() + val toDelete = allAlerts.filter { filter(it.toAlertEntry()) } + if (toDelete.isNotEmpty()) { + dao.deleteAll(toDelete) + } + } + } + + override fun updateAlert(entry: MonitorEventAlertEntry) { + dao.update(MonitorAlertEntity.fromAlertEntry(entry)) + } + + override fun updateAlerts(entries: Collection) { + dao.updateAll(entries.map { MonitorAlertEntity.fromAlertEntry(it) }) + } + + override fun getAlert(eventId: Long, alertTime: Long, instanceStart: Long): MonitorEventAlertEntry? { + return dao.getByKey(eventId, alertTime, instanceStart)?.toAlertEntry() + } + + override fun getInstanceAlerts(eventId: Long, instanceStart: Long): List { + return dao.getByEventAndInstance(eventId, instanceStart).map { it.toAlertEntry() } + } + + override fun getNextAlert(since: Long): Long? { + return dao.getNextAlertTime(since) + } + + override fun getAlertsAt(time: Long): List { + return dao.getByAlertTime(time).map { it.toAlertEntry() } + } + + override val alerts: List + get() = dao.getAll().map { it.toAlertEntry() } + + override fun getAlertsForInstanceStartRange(scanFrom: Long, scanTo: Long): List { + return dao.getByInstanceStartRange(scanFrom, scanTo).map { it.toAlertEntry() } + } + + override fun getAlertsForAlertRange(scanFrom: Long, scanTo: Long): List { + return dao.getByAlertTimeRange(scanFrom, scanTo).map { it.toAlertEntry() } + } + + override fun close() { + // Room handles connection management via singleton pattern + // Individual close is not needed - database will be closed when app terminates + } +} + diff --git a/docs/dev_todo/database_modernization_plan.md b/docs/dev_todo/database_modernization_plan.md index 3ed55fcb6..047278d95 100644 --- a/docs/dev_todo/database_modernization_plan.md +++ b/docs/dev_todo/database_modernization_plan.md @@ -1,6 +1,6 @@ # Database Modernization Plan -## Status: POC COMPLETE ✅ - Ready for Phase 1 +## Status: Phase 1 COMPLETE ✅ - Ready for Phase 2 ## Executive Summary @@ -45,9 +45,9 @@ While [jOOQ](https://www.jooq.org/) is an excellent library for type-safe SQL on | Database | File | Version History | Complexity | Coverage | Priority | |----------|------|-----------------|------------|----------|----------| -| **MonitorStorage** | `monitorstorage/` | V1 only | Low | 730/925 (79%) | **1st (Pilot)** | -| **DismissedEventsStorage** | `dismissedeventsstorage/` | V1→V2 | Medium | 597/623 (96%) | 2nd | -| **EventsStorage** | `eventsstorage/` | V6→V7→V8→V9 | **High** | 1015/1137 (89%) | 3rd | +| **MonitorStorage** | `monitorstorage/` | V1 only | Low | 730/925 (79%) | **1st (Pilot)** ✅ | +| **DismissedEventsStorage** | `dismissedeventsstorage/` | V1→V2 | Medium | 597/623 (96%) | **2nd** | +| **EventsStorage** | `eventsstorage/` | V6→V7→V8→V9 | **High** | 1015/1137 (89%) | **3rd (Last)** | ### NOT SQLite (Skip) @@ -349,18 +349,31 @@ interface MonitorAlertDao { ## Phase 2: DismissedEventsStorage +### Why DismissedEventsStorage Second? +1. **Medium complexity** - Only V1→V2 migration (simpler than EventsStorage) +2. **High test coverage** - 96% already covered +3. **Builds confidence** - Establishes patterns before tackling high-risk storage +4. **Lower risk than EventsStorage** - Not the primary notification database + ### Migration Complexity: Medium - Has V1→V2 migration history - Need Room migration from V2 ### Approach -- Same parallel implementation pattern +- Same parallel implementation pattern as MonitorStorage - Room schema matches V2 +- Pre-Room migration if needed (check for NOT NULL on PKs) - Migration class handles legacy→Room transition --- -## Phase 3: EventsStorage (Most Critical) +## Phase 3: EventsStorage (Most Critical - LAST) + +### Why EventsStorage Last? +1. **Highest complexity** - V6→V7→V8→V9 migration history +2. **Highest risk** - Core notification functionality depends on this +3. **Patterns established** - By now we've migrated 2 databases successfully +4. **Confidence built** - Pre-Room migration pattern proven, edge cases known ### Migration Complexity: HIGH - V6→V7→V8→V9 migration history diff --git a/docs/dev_todo/room_database_migration.md b/docs/dev_todo/room_database_migration.md index fdf65df2c..fe0041331 100644 --- a/docs/dev_todo/room_database_migration.md +++ b/docs/dev_todo/room_database_migration.md @@ -1,8 +1,8 @@ # Room Database Migration -## Status: POC COMPLETE ✅ - See [Database Modernization Plan](database_modernization_plan.md) +## Status: Phase 1 COMPLETE ✅ - MonitorStorage migrated, ready for Phase 2 -> **Note:** This document contains preliminary research. For the actual implementation plan with POC results and validated configurations, see **[Database Modernization Plan](database_modernization_plan.md)**. +> **Note:** This document contains implementation details and patterns discovered during migration. For the overall plan, see **[Database Modernization Plan](database_modernization_plan.md)**. ## Overview @@ -164,17 +164,24 @@ abstract class AppDatabase : RoomDatabase() { **Note:** Originally suggested BTCarModeStorage but it's SharedPreferences, not SQLite. MonitorStorage is the actual simplest SQLite database (V1 only, no migration history). -### Phase 2: Migrate Critical Storage +### Phase 2: DismissedEventsStorage (Medium Complexity) +1. `DismissedEventsStorage` - Medium complexity, V1→V2 migration +2. Builds confidence with migration patterns before tackling highest-risk database +3. High test coverage (96%) provides safety net + +### Phase 3: EventsStorage (Highest Risk - LAST) 1. `EventsStorage` - most important, most complex -2. Need to handle existing V6→V7→V8→V9 migration history -3. Room's `Migration` class handles this cleanly +2. Tackle LAST after patterns are proven on simpler databases +3. Need to handle existing V6→V7→V8→V9 migration history +4. Room's `Migration` class handles schema versioning -### Phase 3: Complete Migration -1. `DismissedEventsStorage` -2. `MonitorStorage` -3. `CalendarChangeRequestsStorage` +### Deprecated (Skip) +- `CalendarChangeRequestsStorage` - DEPRECATED, remove instead of migrate ### Schema Migration from Current to Room + +> ⚠️ **Important:** Standard Room migrations only work for version upgrades AFTER Room has opened the database. For pre-existing SQLiteOpenHelper databases, see the next section. + ```kotlin val MIGRATION_LEGACY_TO_ROOM = object : Migration(9, 10) { override fun migrate(database: SupportSQLiteDatabase) { @@ -185,6 +192,109 @@ val MIGRATION_LEGACY_TO_ROOM = object : Migration(9, 10) { } ``` +### Pre-Room Migration Pattern (CRITICAL for existing databases) + +Room validates schema **before** running any migrations. For databases created by `SQLiteOpenHelper` (no `room_master_table`), Room checks the schema first and fails if it doesn't match exactly. + +``` +Standard Room Migration Flow: +┌─────────────────┐ ┌──────────────────┐ ┌─────────────────┐ +│ Room opens DB │ ──► │ Validate schema │ ──► │ Run migrations │ +└─────────────────┘ └──────────────────┘ └─────────────────┘ + ▲ + │ FAILS HERE for legacy DBs + │ (before migrations can run) +``` + +**Solution:** Pre-migrate the database BEFORE Room opens it: + +``` +Our Pattern: +┌─────────────────┐ ┌─────────────────┐ ┌──────────────────┐ +│ Pre-migrate │ ──► │ Room opens DB │ ──► │ Validate schema │ ✅ +└─────────────────┘ └─────────────────┘ └──────────────────┘ +``` + +#### Schema Differences Found (MonitorStorage example) + +| Issue | Legacy Schema | Room Requires | +|-------|---------------|---------------| +| **PK NOT NULL** | `eventId INTEGER` | `eventId INTEGER NOT NULL` | +| **Indexes** | Has `manualAlertsV1IdxV1` | Must be declared in `@Entity` | + +Room requires: +- Primary key columns to have `NOT NULL` constraint +- Any indexes to be declared in the `@Entity` annotation + +#### Implementation Pattern + +```kotlin +// In MonitorDatabase.kt +fun getInstance(context: Context): MonitorDatabase { + return INSTANCE ?: synchronized(this) { + INSTANCE ?: run { + // Pre-migrate BEFORE Room opens + migrateLegacyDatabaseIfNeeded(context) + buildDatabase(context).also { INSTANCE = it } + } + } +} + +private fun migrateLegacyDatabaseIfNeeded(context: Context) { + val dbFile = context.getDatabasePath(DATABASE_NAME) + if (!dbFile.exists()) return + + val db = SQLiteDatabase.openDatabase(dbFile.absolutePath, null, OPEN_READWRITE) + try { + if (!isLegacyDatabase(db)) return + performLegacyMigration(db) + } finally { + db.close() + } +} + +private fun isLegacyDatabase(db: SQLiteDatabase): Boolean { + // Legacy = has our table but no room_master_table + val hasTable = db.rawQuery("SELECT 1 FROM sqlite_master WHERE name='myTable'", null) + .use { it.moveToFirst() } + val hasRoomTable = db.rawQuery("SELECT 1 FROM sqlite_master WHERE name='room_master_table'", null) + .use { it.moveToFirst() } + return hasTable && !hasRoomTable +} + +private fun performLegacyMigration(db: SQLiteDatabase) { + db.beginTransaction() + try { + // Recreate table with Room-compatible schema + db.execSQL("CREATE TABLE myTable_new (...)") // with NOT NULL on PKs + db.execSQL("INSERT INTO myTable_new SELECT ... FROM myTable") + db.execSQL("DROP TABLE myTable") + db.execSQL("ALTER TABLE myTable_new RENAME TO myTable") + db.execSQL("CREATE INDEX ...") // recreate indexes + db.setTransactionSuccessful() + } finally { + db.endTransaction() + } +} +``` + +#### Entity with Index Declaration + +```kotlin +@Entity( + tableName = "manualAlertsV1", + primaryKeys = ["eventId", "alertTime", "instanceStart"], + indices = [Index( + value = ["eventId", "alertTime", "instanceStart"], + unique = true, + name = "manualAlertsV1IdxV1" // Must match existing index name! + )] +) +data class MonitorAlertEntity(...) +``` + +**This pattern is documented for SQLiteOpenHelper → Room migrations.** It's not a hack - it's the correct approach when schemas don't match exactly. + ## Effort Estimate | Task | Time | Risk | @@ -257,7 +367,7 @@ See [CR-SQLite + Room Testing Guide](../testing/crsqlite_room_testing.md) for fu ## Recommendation -**Priority: Medium-High** | **Status: POC COMPLETE ✅** +**Priority: Medium-High** | **Status: Phase 1 COMPLETE ✅** This is a good candidate for migration because: 1. Database code is mission-critical (event notifications) @@ -265,14 +375,29 @@ This is a good candidate for migration because: 3. Type safety would catch issues at compile time 4. Reduces maintenance burden long-term -### POC Validated (Dec 2025) +### Progress (Dec 2025) + +#### POC Validated ✅ - ✅ Room works with cr-sqlite extension via custom `SupportSQLiteOpenHelper.Factory` - ✅ All 8 POC tests pass (CRUD, extension loading, finalize, coexistence) - ✅ APK packaging requirements documented -**Next step:** Phase 1 - Migrate `MonitorStorage` (simplest SQLite database, V1 only) +#### Phase 1: MonitorStorage Migration ✅ +- ✅ Created `MonitorAlertEntity`, `MonitorAlertDao`, `MonitorDatabase` +- ✅ Implemented pre-Room migration for legacy schema compatibility +- ✅ Live upgrade test passed (legacy DB → Room-managed DB, data preserved) +- ✅ `room_master_table` created successfully +- ✅ Migration tests pass locally (3/3) + +**Key files:** +- `monitorstorage/MonitorAlertEntity.kt` - Room entity with index +- `monitorstorage/MonitorAlertDao.kt` - Data access object +- `monitorstorage/MonitorDatabase.kt` - Database with pre-Room migration +- `monitorstorage/RoomMonitorStorage.kt` - Implementation of `MonitorStorageInterface` + +**Next step:** Phase 2 - Migrate `DismissedEventsStorage` (medium complexity, build confidence before EventsStorage) -**See:** [Database Modernization Plan](database_modernization_plan.md) for the detailed implementation plan with POC results and validated configurations. +**See:** [Database Modernization Plan](database_modernization_plan.md) for the detailed implementation plan. ## References diff --git a/scripts/generate_android_coverage.sh b/scripts/generate_android_coverage.sh index b58de56cd..55338b619 100755 --- a/scripts/generate_android_coverage.sh +++ b/scripts/generate_android_coverage.sh @@ -231,7 +231,14 @@ echo "Coverage data preparation completed" # Now handle the XML test results for dorny/test-reporter echo "Processing XML test results for test reporting..." -# Define paths for test results +# Define paths for test results (shard-specific if sharding enabled) +if [ -n "$SHARD_INDEX" ]; then + # Internal storage path where XmlRunListener writes (configured via -e reportFile) + DEVICE_XML_PATH="/data/data/${APP_PACKAGE}/files/test-results_shard_${SHARD_INDEX}.xml" +else + DEVICE_XML_PATH="/data/data/${APP_PACKAGE}/files/test-results.xml" +fi +# Fallback paths for legacy/external storage locations DEVICE_TEST_RESULT_PATH="/storage/emulated/0/Android/data/com.github.quarck.calnotify/files/report-0.xml" APP_CACHE_XML_PATH="/data/data/${APP_PACKAGE}/cache/test-results.xml" LOCAL_TEST_RESULT_PATH="./$MAIN_PROJECT_MODULE/build/outputs/androidTest-results/connected/TEST-${APP_PACKAGE}.xml" @@ -241,16 +248,37 @@ DORNY_TEST_RESULT_PATH="./$MAIN_PROJECT_MODULE/build/outputs/connected/TEST-${AP mkdir -p "$(dirname "$LOCAL_TEST_RESULT_PATH")" mkdir -p "$(dirname "$DORNY_TEST_RESULT_PATH")" -# First try direct pull (if we have permission) -echo "Attempting to pull XML test results directly..." -if adb pull "$DEVICE_TEST_RESULT_PATH" "$LOCAL_TEST_RESULT_PATH" 2>/dev/null; then - echo "✅ Successfully pulled test results directly!" - XML_RETRIEVED=true -else - echo "⚠️ Direct pull failed. Trying alternative methods..." - XML_RETRIEVED=false - - # Check if the file exists in the app's cache directory (XmlRunListener fallback location) +XML_RETRIEVED=false + +# First try internal storage (where XmlRunListener is configured to write) +echo "Attempting to pull XML test results from internal storage..." +if adb shell "run-as $APP_PACKAGE ls -la $DEVICE_XML_PATH" 2>/dev/null | grep -q "test-results"; then + echo "Found XML file at $DEVICE_XML_PATH" + # Extract using base64 (run-as required for internal storage) + TEMP_DIR=$(mktemp -d) + if adb shell "run-as $APP_PACKAGE cat $DEVICE_XML_PATH | base64" > "$TEMP_DIR/test-results.b64" 2>/dev/null; then + base64 --decode < "$TEMP_DIR/test-results.b64" > "$LOCAL_TEST_RESULT_PATH" && { + echo "✅ Successfully extracted XML from internal storage!" + XML_RETRIEVED=true + } + fi + rm -rf "$TEMP_DIR" +fi + +# Fallback: try external storage direct pull (legacy path) +if [ "$XML_RETRIEVED" = false ]; then + echo "Trying external storage fallback..." + if adb pull "$DEVICE_TEST_RESULT_PATH" "$LOCAL_TEST_RESULT_PATH" 2>/dev/null; then + # Verify file is not empty + if [ -s "$LOCAL_TEST_RESULT_PATH" ]; then + echo "✅ Successfully pulled test results from external storage!" + XML_RETRIEVED=true + fi + fi +fi + +# Fallback: check app's cache directory +if [ "$XML_RETRIEVED" = false ]; then echo "Checking for XML in app's cache directory..." if adb shell "run-as $APP_PACKAGE ls -la $APP_CACHE_XML_PATH" 2>/dev/null | grep -q "test-results.xml"; then echo "Found XML file in app's cache directory!" diff --git a/scripts/matrix_run_android_tests.sh b/scripts/matrix_run_android_tests.sh index 15f3faacd..c8a4a45eb 100644 --- a/scripts/matrix_run_android_tests.sh +++ b/scripts/matrix_run_android_tests.sh @@ -144,6 +144,7 @@ install_apks() { build_instrument_command() { local coverage_path="$1" + local xml_report_path="$2" local cmd="am instrument -w -r" cmd+=" -e debug false" @@ -156,6 +157,8 @@ build_instrument_command() { cmd+=" -e jaco-agent.destfile \"$coverage_path\"" cmd+=" -e jaco-agent.includes \"com.github.quarck.calnotify.*\"" cmd+=" -e listener \"de.schroepf.androidxmlrunlistener.XmlRunListener\"" + # Write XML report to internal storage (external storage blocked by scoped storage on API 30+) + cmd+=" -e reportFile \"$xml_report_path\"" # Smart sharding: UI tests get shards 0-(UI_SHARD_COUNT-1), non-UI get the rest # With 4 total shards and UI_SHARD_COUNT=2: @@ -272,12 +275,15 @@ main() { # Install APKs install_apks "$app_apk" "$test_apk" - # Determine coverage file path (shard-specific if sharding enabled) + # Determine coverage and XML report paths (shard-specific if sharding enabled) local coverage_path + local xml_report_path if [ -n "$SHARD_INDEX" ]; then coverage_path="/data/data/$APP_PACKAGE/files/coverage_shard_${SHARD_INDEX}.ec" + xml_report_path="/data/data/$APP_PACKAGE/files/test-results_shard_${SHARD_INDEX}.xml" else coverage_path="/data/data/$APP_PACKAGE/files/coverage.ec" + xml_report_path="/data/data/$APP_PACKAGE/files/test-results.xml" fi # Create coverage directory on device @@ -285,10 +291,11 @@ main() { # Build and run instrument command local instrument_cmd - instrument_cmd=$(build_instrument_command "$coverage_path") + instrument_cmd=$(build_instrument_command "$coverage_path" "$xml_report_path") echo "Running tests with timeout $TEST_TIMEOUT..." echo "Coverage file: $coverage_path" + echo "XML report file: $xml_report_path" local test_exit_code=0 timeout "$TEST_TIMEOUT" adb shell "$instrument_cmd" || {