feat: Room Migration p2 DismissedEventsStorage (#138)

* test: maybe fix slowness in full concert test run

* fix: narrow exception catch

* feat: copy old db pattern for DismissedEventsStorage also docs

* test: DismissedEventsStorageMigrationTest

* test: hopefully fix concert perf

* ci: hopefully more stable gradle deps cache

* fix: bug bot close

* fix: another bug bot catch

Room database not closed when migration fails

In getInstance, if copyFromLegacyIfNeeded throws an exception, the Room
database db that was built is never closed. The database connection is
opened when dao.getAll() is called inside copyFromLegacyIfNeeded. When
an exception propagates, INSTANCE is never set and the db reference is
lost, but the underlying database connection remains open. This leaks a
database connection that will persist until process termination,
potentially locking the database file and consuming

* fix: another bug bot

Migration validation cannot detect partial legacy data reads
The migration count validation compares the Room count to the number of
alerts that were successfully READ from legacy (legacyAlerts.size), not
the actual total count in the legacy database. The underlying
MonitorStorageImplV1.getAlerts() method catches broad Exception and
returns partial results if an error occurs mid-read. If a cursor error
occurs after reading some rows, the migration would insert partial data,
pass validation (since Room count equals read count), and permanently
orphan the remaining legacy data. On subsequent launches, migration is
skipped because Room already has data.

* feat: room integration p1 DismissedEventsStorage

wip

Author: williscool

.github/actions/cache-update/action.yml

@@ -52,6 +52,17 @@ runs:
           android/app/build/intermediates/test_apk_androidTest
         key: ${{ runner.os }}-build-outputs-${{ inputs.arch }}-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }}-${{ github.sha }}
 
+    # Gradle dependencies cache - Save only (more stable than build cache)
+    # Prevents intermittent 403s from Maven Central rate limiting in CI
+    - name: Save Gradle Dependencies Cache
+      if: always()
+      uses: actions/cache/save@v4
+      with:
+        path: |
+          ~/.gradle/caches/modules-2
+          ~/.gradle/caches/jars-*
+        key: ${{ runner.os }}-gradle-deps-v1-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}
+
     # Gradle cache - Save only with architecture in key
     - name: Save Gradle Cache
       if: always()

.github/actions/common-setup/action.yml

@@ -198,6 +198,21 @@ runs:
         java-version: '21'
         distribution: 'temurin'
 
+    # Cache Gradle downloaded dependencies (separate from build cache for stability)
+    # This uses a stable key so dependencies persist across gradle file changes
+    # Prevents intermittent 403s from Maven Central rate limiting in CI
+    - name: Restore Gradle Dependencies Cache
+      uses: actions/cache/restore@v4
+      id: gradle-deps-cache-restore
+      with:
+        path: |
+          ~/.gradle/caches/modules-2
+          ~/.gradle/caches/jars-*
+        key: ${{ runner.os }}-gradle-deps-v1-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}
+        restore-keys: |
+          ${{ runner.os }}-gradle-deps-v1-
+          ${{ runner.os }}-gradle-deps-
+
     # Cache Gradle files
     # Note: Cache key includes gradle-wrapper.properties hash to invalidate on Gradle version changes
     - name: Restore Cache Gradle files

android/app/src/androidTest/java/com/github/quarck/calnotify/dismissedeventsstorage/DismissedEventsStorageMigrationTest.kt

@@ -46,5 +46,22 @@ class AsyncTaskIdlingResource(
      * Get current count (for debugging).
      */
     fun getCount(): Int = counter.get()
+    
+    /**
+     * Resets the counter to zero.
+     * Call this when clearing IdlingResources between tests to prevent stale
+     * counter values from causing subsequent tests to hang.
+     */
+    fun reset() {
+        val previousCount = counter.getAndSet(0)
+        if (previousCount > 0) {
+            com.github.quarck.calnotify.logs.DevLog.info(
+                "AsyncTaskIdlingResource",
+                "Reset counter from $previousCount to 0 (clearing stale async task count)"
+            )
+        }
+        // Notify callback that we're now idle (counter is 0)
+        callback?.onTransitionToIdle()
+    }
 }
 

android/app/src/androidTest/java/com/github/quarck/calnotify/testutils/AsyncTaskIdlingResource.kt

@@ -47,9 +47,6 @@ class UITestFixture {
     private var eventIdCounter = 100000L
     private val seededEvents = mutableListOf<EventAlertRecord>()
 
-    // IdlingResource for AsyncTask tracking
-    private val asyncTaskIdlingResource = AsyncTaskIdlingResource()
-    
     // Track if calendar reload prevention is active
     private var calendarReloadPrevented = false
 
@@ -77,6 +74,10 @@ class UITestFixture {
     ) {
         DevLog.info(LOG_TAG, "Setting up UITestFixture (waitForAsyncTasks=$waitForAsyncTasks, preventCalendarReload=$preventCalendarReload, grantPermissions=$grantPermissions, suppressBatteryDialog=$suppressBatteryDialog)")
 
+        // CRITICAL: Clear any leftover IdlingResources from previous tests FIRST
+        // This prevents accumulation when running the full test suite
+        clearIdlingResources()
+        
         // Reset dialog flag so each test can handle dialogs if they appear
         startupDialogsDismissed = false
         dialogsSuppressed = false
@@ -109,8 +110,8 @@ class UITestFixture {
 
         // Only register IdlingResource if we need to wait for async operations
         if (waitForAsyncTasks) {
-            IdlingRegistry.getInstance().register(asyncTaskIdlingResource)
-            globalAsyncTaskCallback = asyncTaskIdlingResource
+            IdlingRegistry.getInstance().register(sharedAsyncTaskIdlingResource)
+            globalAsyncTaskCallback = sharedAsyncTaskIdlingResource
             DevLog.info(LOG_TAG, "Registered AsyncTask IdlingResource")
         } else {
             DevLog.info(LOG_TAG, "Skipping AsyncTask IdlingResource for faster UI tests")
@@ -332,13 +333,8 @@ class UITestFixture {
             DevLog.error(LOG_TAG, "Failed to reset battery dialog setting: ${e.message}")
         }
 
-        // Unregister IdlingResource if it was registered
-        try {
-            IdlingRegistry.getInstance().unregister(asyncTaskIdlingResource)
-            globalAsyncTaskCallback = null
-        } catch (e: IllegalArgumentException) {
-            // Expected if IdlingResource wasn't registered - safe to ignore
-        }
+        // Clear IdlingResources to ensure clean state for next test
+        clearIdlingResources()
 
         unmockkAll()
     }
@@ -642,6 +638,34 @@ class UITestFixture {
         @Volatile
         private var startupDialogsDismissed = false
 
+        /**
+         * Shared IdlingResource instance to prevent accumulation across tests.
+         * Using a singleton ensures we don't register multiple IdlingResources
+         * when running the full test suite.
+         */
+        private val sharedAsyncTaskIdlingResource = AsyncTaskIdlingResource()
+        
+        /**
+         * Clears any leftover IdlingResources from previous test runs.
+         * Call at the START of setup to ensure clean state.
+         */
+        fun clearIdlingResources() {
+            // Clear the global callback first
+            globalAsyncTaskCallback = null
+            
+            // Reset the counter to clear any stale async task counts from previous tests
+            // This prevents tests from hanging if a previous test terminated with tasks in-flight
+            sharedAsyncTaskIdlingResource.reset()
+            
+            // Unregister our shared IdlingResource if it's registered
+            try {
+                IdlingRegistry.getInstance().unregister(sharedAsyncTaskIdlingResource)
+                DevLog.info(LOG_TAG, "Cleared previously registered IdlingResource")
+            } catch (e: IllegalArgumentException) {
+                // Not registered, that's fine
+            }
+        }
+        
         /**
          * Creates a fixture instance. Typically used with JUnit rules.
          */

android/app/src/androidTest/java/com/github/quarck/calnotify/testutils/UITestFixture.kt

@@ -19,6 +19,7 @@
 
 package com.github.quarck.calnotify.database
 
+import android.database.SQLException
 import androidx.sqlite.db.SupportSQLiteOpenHelper
 import com.github.quarck.calnotify.logs.DevLog
 import io.requery.android.database.sqlite.RequerySQLiteOpenHelperFactory
@@ -71,10 +72,13 @@ class CrSqliteFinalizeWrapper(
     override fun close() {
         try {
             writableDatabase.query("SELECT crsql_finalize()").use { it.moveToFirst() }
-        } catch (e: Exception) {
+        } catch (e: SQLException) {
             DevLog.error("CrSqliteFinalizeWrapper", "Error calling crsql_finalize: ${e.message}")
+        } catch (e: IllegalStateException) {
+            DevLog.error("CrSqliteFinalizeWrapper", "Error calling crsql_finalize: ${e.message}")
+        } finally {
+            delegate.close()
         }
-        delegate.close()
     }
 }
 

android/app/src/main/java/com/github/quarck/calnotify/database/CrSqliteRoomFactory.kt

@@ -0,0 +1,57 @@
+//
+//   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.dismissedeventsstorage
+
+import androidx.room.Dao
+import androidx.room.Delete
+import androidx.room.Insert
+import androidx.room.OnConflictStrategy
+import androidx.room.Query
+
+/**
+ * Room DAO for DismissedEventEntity.
+ * 
+ * Provides all database operations needed by DismissedEventsStorageInterface.
+ */
+@Dao
+interface DismissedEventDao {
+    
+    @Query("SELECT * FROM ${DismissedEventEntity.TABLE_NAME}")
+    fun getAll(): List<DismissedEventEntity>
+
+    @Insert(onConflict = OnConflictStrategy.REPLACE)
+    fun insert(entity: DismissedEventEntity)
+
+    @Insert(onConflict = OnConflictStrategy.REPLACE)
+    fun insertAll(entities: List<DismissedEventEntity>)
+
+    @Delete
+    fun delete(entity: DismissedEventEntity)
+
+    @Query("DELETE FROM ${DismissedEventEntity.TABLE_NAME} WHERE ${DismissedEventEntity.COL_EVENT_ID} = :eventId AND ${DismissedEventEntity.COL_INSTANCE_START} = :instanceStart")
+    fun deleteByKey(eventId: Long, instanceStart: Long)
+
+    @Query("DELETE FROM ${DismissedEventEntity.TABLE_NAME}")
+    fun deleteAll()
+
+    @Query("DELETE FROM ${DismissedEventEntity.TABLE_NAME} WHERE ${DismissedEventEntity.COL_DISMISS_TIME} < :cutoffTime")
+    fun deleteOlderThan(cutoffTime: Long)
+}
+