From ba9c44ca727904ec1d03726778ded7021731031a Mon Sep 17 00:00:00 2001 From: anod <171704+anod@users.noreply.github.com> Date: Sat, 1 Aug 2026 11:20:52 +0300 Subject: [PATCH 1/4] Code review fixes: DB integrity, concurrency, security, perf + tests Bug fixes - ShortcutsDatabase write path: replace INSERT-then-UPDATE fallback with delete-then-insert so last_insert_rowid() is always valid. Folder children were being attached to a stale parent id when overwriting an occupied (targetId, position) slot, and copyShortcut threw an uncaught SQLiteConstraintException onto an occupied slot. FK ON DELETE CASCADE now cleans up the replaced row's folder items. - Suppress corrupt shortcut rows (empty / unparseable intent) by mapping them to ID_UNKNOWN so isValid gates hide them; observeFolder now filters invalid items to match loadFolderItems. - ModeDetector: guard shared pref/event state with the existing lock across onRegister/forceState/switchOn/switchOff and the onBroadcastReceive state block; prefState getter returns a defensive copy. Security - Set exported=false on OverlayActivity, ShortcutActivity and SwitchInCarActivity (launched only via internal PendingIntents). Add a debug-only manifest overlay re-exporting SwitchInCarActivity for the adb switch script. Performance - ModeService: stop calling runBlocking on the main thread. Start foreground immediately with a lightweight notification, then build the rich one off the main thread and swap it in. - BitmapLruCache: sizeOf() no longer rounds sub-kilobyte bitmaps to 0; cache budget measured in kilobytes to match. - BackupManager: replace newSingleThreadContext with Dispatchers.IO. Cleanup - Remove dead AcceptCallActivity (never launched; superseded by ModePhoneStateListener.acceptRingingCall()). Tests - content: BitmapLruCacheUnitTest, ShortcutCorruptRowUnitTest, ShortcutOverwriteUnitTest. - app: bootstrap local unit-test infra (junit4 catalog entry + Robolectric) and add ModeDetectorTest and ExportedActivitiesTest. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 698d9ed2-0a45-4e67-9777-549537a61c98 --- app/build.gradle.kts | 12 ++ app/src/debug/AndroidManifest.xml | 16 ++ app/src/main/AndroidManifest.xml | 14 +- .../anod/car/home/incar/AcceptCallActivity.kt | 138 ------------- .../com/anod/car/home/incar/ModeDetector.kt | 61 +++--- .../com/anod/car/home/incar/ModeService.kt | 48 ++++- .../InCarModeNotificationFactory.kt | 21 +- .../anod/car/home/ExportedActivitiesTest.kt | 43 ++++ .../anod/car/home/incar/ModeDetectorTest.kt | 74 +++++++ .../content/BitmapLruCacheUnitTest.kt | 55 ++++++ .../content/db/ShortcutCorruptRowUnitTest.kt | 131 +++++++++++++ .../content/db/ShortcutOverwriteUnitTest.kt | 184 ++++++++++++++++++ .../carwidget/content/BitmapLruCache.kt | 7 +- .../carwidget/content/backup/BackupManager.kt | 14 +- .../carwidget/content/db/ShortcutsDatabase.kt | 65 +++---- gradle/libs.versions.toml | 2 + 16 files changed, 649 insertions(+), 236 deletions(-) create mode 100644 app/src/debug/AndroidManifest.xml delete mode 100644 app/src/main/java/com/anod/car/home/incar/AcceptCallActivity.kt create mode 100644 app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt create mode 100644 app/src/test/kotlin/com/anod/car/home/incar/ModeDetectorTest.kt create mode 100644 content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/BitmapLruCacheUnitTest.kt create mode 100644 content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutCorruptRowUnitTest.kt create mode 100644 content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index be5516b1..f358ace4 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -37,6 +37,11 @@ dependencies { compileOnly(libs.auto.service.annotations) ksp(libs.auto.service.ksp) ksp(libs.auto.service) + + testImplementation(libs.kotlin.test) + testImplementation(libs.junit4) + testImplementation(libs.robolectric) + testImplementation(libs.androidx.test.core) } kotlin { @@ -98,5 +103,12 @@ android { lint { warning.add("InvalidFragmentVersionForActivityResult") } + + testOptions { + unitTests { + isIncludeAndroidResources = true + isReturnDefaultValues = true + } + } namespace = "com.anod.car.home" } diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml new file mode 100644 index 00000000..773c1011 --- /dev/null +++ b/app/src/debug/AndroidManifest.xml @@ -0,0 +1,16 @@ + + + + + + + + diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index cd72a51b..41a04d58 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -74,7 +74,7 @@ @@ -83,7 +83,7 @@ android:name="com.anod.car.home.ShortcutActivity" android:clearTaskOnLaunch="true" android:excludeFromRecents="true" - android:exported="true" + android:exported="false" android:launchMode="singleInstance" android:noHistory="true" android:taskAffinity="com.anod.car.home.shortcut" @@ -128,7 +128,7 @@ android:name="com.anod.car.home.incar.SwitchInCarActivity" android:clearTaskOnLaunch="true" android:excludeFromRecents="true" - android:exported="true" + android:exported="false" android:label="@string/switch_in_car_activity" android:launchMode="singleInstance" android:noHistory="true" @@ -254,14 +254,6 @@ android:name="preloaded_fonts" android:resource="@array/preloaded_fonts" /> - - { @@ -54,7 +54,9 @@ object ModeDetector { } fun onRegister(context: Context) { - sEventState[FLAG_POWER] = Power.isConnected(context) + synchronized(sLock) { + sEventState[FLAG_POWER] = Power.isConnected(context) + } } fun updatePrefState(prefs: InCarInterface) { @@ -68,21 +70,23 @@ object ModeDetector { } fun forceState(prefs: InCarInterface, forceMode: Boolean) { - updatePrefState(prefs) - if (sPrefState[FLAG_POWER]) { - sEventState[FLAG_POWER] = forceMode - } - if (sPrefState[FLAG_HEADSET]) { - sEventState[FLAG_HEADSET] = forceMode - } - if (sPrefState[FLAG_BLUETOOTH]) { - sEventState[FLAG_BLUETOOTH] = forceMode - } - if (sPrefState[FLAG_ACTIVITY]) { - sEventState[FLAG_ACTIVITY] = forceMode - } - if (sPrefState[FLAG_CAR_DOCK]) { - sEventState[FLAG_CAR_DOCK] = forceMode + synchronized(sLock) { + updatePrefState(prefs) + if (sPrefState[FLAG_POWER]) { + sEventState[FLAG_POWER] = forceMode + } + if (sPrefState[FLAG_HEADSET]) { + sEventState[FLAG_HEADSET] = forceMode + } + if (sPrefState[FLAG_BLUETOOTH]) { + sEventState[FLAG_BLUETOOTH] = forceMode + } + if (sPrefState[FLAG_ACTIVITY]) { + sEventState[FLAG_ACTIVITY] = forceMode + } + if (sPrefState[FLAG_CAR_DOCK]) { + sEventState[FLAG_CAR_DOCK] = forceMode + } } } @@ -99,15 +103,16 @@ object ModeDetector { onPowerConnected(prefs, context) } - updatePrefState(prefs) - updateEventState(prefs, intent) - if (BuildConfig.DEBUG) { - for (i in sPrefState.indices) { - AppLog.d(sTitles[i] + ": pref - " + sPrefState[i] + ", event - " + sEventState[i]) + val newMode = synchronized(sLock) { + updatePrefState(prefs) + updateEventState(prefs, intent) + if (BuildConfig.DEBUG) { + for (i in sPrefState.indices) { + AppLog.d(sTitles[i] + ": pref - " + sPrefState[i] + ", event - " + sEventState[i]) + } } + detectNewMode() } - - val newMode = detectNewMode() AppLog.i("New mode: " + newMode + " Car Mode: " + ModeService.sInCarMode) if (!ModeService.sInCarMode && newMode) { val service = ModeService.createStartIntent(context, ModeService.MODE_SWITCH_ON) @@ -231,12 +236,16 @@ object ModeDetector { } fun switchOn(prefs: InCarInterface, modeHandler: ModeHandler) { - sMode = true + synchronized(sLock) { + sMode = true + } modeHandler.enable(prefs) } fun switchOff(prefs: InCarInterface, modeHandler: ModeHandler) { - sMode = false + synchronized(sLock) { + sMode = false + } modeHandler.disable(prefs) } } \ No newline at end of file diff --git a/app/src/main/java/com/anod/car/home/incar/ModeService.kt b/app/src/main/java/com/anod/car/home/incar/ModeService.kt index 9c9670b0..18c9db74 100644 --- a/app/src/main/java/com/anod/car/home/incar/ModeService.kt +++ b/app/src/main/java/com/anod/car/home/incar/ModeService.kt @@ -1,5 +1,6 @@ package com.anod.car.home.incar +import android.app.NotificationManager import android.app.Service import android.content.Context import android.content.Intent @@ -15,7 +16,12 @@ import info.anodsplace.applog.AppLog import info.anodsplace.carwidget.content.preferences.InCarSettings import info.anodsplace.permissions.AppPermission import info.anodsplace.permissions.AppPermissions -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.koin.core.component.KoinComponent import org.koin.core.component.get import org.koin.core.component.inject @@ -25,8 +31,19 @@ class ModeService : Service(), KoinComponent { private var phoneListener: ModePhoneStateListener? = null private val modeHandler: ModeHandler by inject() private var forceState: Boolean = false + private val serviceScope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) + + private val notificationFactory: InCarModeNotificationFactory by lazy { + InCarModeNotificationFactory( + context = this, + database = get(), + iconLoader = get(), + shortcutResources = get() + ) + } override fun onDestroy() { + serviceScope.cancel() ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE) val prefs = get() @@ -54,14 +71,10 @@ class ModeService : Service(), KoinComponent { AppLog.i("Start InCar Mode service, sInCarMode = " + sInCarMode + ", redelivered = " + redelivered) - val notificationFactory = InCarModeNotificationFactory( - context = this, - database = get(), - iconLoader = get(), - shortcutResources = get() - ) - val notification = runBlocking { notificationFactory.create() } - startForeground(InCarModeNotificationFactory.id, notification) + // Enter the foreground immediately with a lightweight notification. Building the rich + // notification touches the database, PackageManager and decodes icons, so it must not + // run on the main thread (ANR / foreground-service start-timeout risk). + startForeground(InCarModeNotificationFactory.id, notificationFactory.createBasic()) if (intent == null) { AppLog.e("ModeService started without intent") @@ -92,11 +105,27 @@ class ModeService : Service(), KoinComponent { initPhoneListener(prefs) requestWidgetsUpdate() + updateNotification() + // We want this service to continue running until it is explicitly // stopped, so return sticky. return START_REDELIVER_INTENT } + private fun updateNotification() { + serviceScope.launch { + try { + val notification = withContext(Dispatchers.Default) { notificationFactory.create() } + if (sInCarMode) { + getSystemService(NotificationManager::class.java) + ?.notify(InCarModeNotificationFactory.id, notification) + } + } catch (e: Exception) { + AppLog.e(e) + } + } + } + private fun initPhoneListener(prefs: info.anodsplace.carwidget.content.preferences.InCarInterface) { if (prefs.isAutoSpeaker || prefs.autoAnswer != info.anodsplace.carwidget.content.preferences.InCarInterface.AUTOANSWER_DISABLED) { if (phoneListener == null) { @@ -151,6 +180,7 @@ class ModeService : Service(), KoinComponent { const val MODE_SWITCH_OFF = 1 const val MODE_SWITCH_ON = 0 + @Volatile var sInCarMode: Boolean = false @Volatile diff --git a/app/src/main/java/com/anod/car/home/notifications/InCarModeNotificationFactory.kt b/app/src/main/java/com/anod/car/home/notifications/InCarModeNotificationFactory.kt index 1ff7cbe8..a7719673 100644 --- a/app/src/main/java/com/anod/car/home/notifications/InCarModeNotificationFactory.kt +++ b/app/src/main/java/com/anod/car/home/notifications/InCarModeNotificationFactory.kt @@ -26,19 +26,32 @@ class InCarModeNotificationFactory( private val buttonIds = intArrayOf(R.id.btn0, R.id.btn1, R.id.btn2, R.id.btn3) } - suspend fun create(): Notification { + private fun baseBuilder(): NotificationCompat.Builder { val notificationIntent = ModeService.createStartIntent(context, ModeService.MODE_SWITCH_OFF) notificationIntent.data = Deeplink.SwitchMode(false).toUri() - val r = context.resources val contentIntent = PendingIntent.getService(context, 0, notificationIntent, PendingIntent.FLAG_IMMUTABLE) - val notification = NotificationCompat.Builder(context, Channels.inCarMode) + return NotificationCompat.Builder(context, Channels.inCarMode) .setSmallIcon(info.anodsplace.carwidget.skin.R.drawable.ic_stat_incar) .setOngoing(true) .addAction(0, context.getString(info.anodsplace.carwidget.content.R.string.disable), contentIntent) .setPriority(NotificationCompat.PRIORITY_MAX) .setCategory(NotificationCompat.CATEGORY_SERVICE) + } + + /** + * Lightweight notification with no database or icon access, safe to build on the main thread. + * Used to satisfy the foreground-service start deadline before the rich notification is ready. + */ + fun createBasic(): Notification { + return baseBuilder() + .setContentTitle(context.getString(info.anodsplace.carwidget.content.R.string.incar_mode_enabled)) + .build() + } + + suspend fun create(): Notification { + val notification = baseBuilder() .setStyle(NotificationCompat.DecoratedCustomViewStyle()) val model = NotificationShortcutsModel.init(context, database) @@ -47,7 +60,7 @@ class InCarModeNotificationFactory( contentView.setTextViewText(android.R.id.text1, "") notification.setContent(contentView) } else { - notification.setContentTitle(r.getString(info.anodsplace.carwidget.content.R.string.incar_mode_enabled)) + notification.setContentTitle(context.getString(info.anodsplace.carwidget.content.R.string.incar_mode_enabled)) } return notification.build() diff --git a/app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt b/app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt new file mode 100644 index 00000000..7e7bc1da --- /dev/null +++ b/app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt @@ -0,0 +1,43 @@ +// Copyright (c) CarWidget contributors. Licensed under the project license. +package com.anod.car.home + +import android.content.Context +import android.content.pm.PackageManager +import androidx.test.core.app.ApplicationProvider +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Security regression guard: activities that are only launched via internal PendingIntents must + * stay android:exported="false" so other apps cannot start them. SwitchInCarActivity is the one + * exception in debug builds, where a manifest overlay re-exports it for the adb switch script. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [31], application = android.app.Application::class) +class ExportedActivitiesTest { + + private fun exportedByActivityName(): Map { + val context = ApplicationProvider.getApplicationContext() + val info = context.packageManager.getPackageInfo( + context.packageName, + PackageManager.GET_ACTIVITIES + ) + return info.activities.orEmpty().associate { it.name to it.exported } + } + + @Test + fun pendingIntentOnlyActivitiesAreNotExported() { + val exported = exportedByActivityName() + assertEquals(false, exported["com.anod.car.home.OverlayActivity"]) + assertEquals(false, exported["com.anod.car.home.ShortcutActivity"]) + } + + @Test + fun switchInCarActivityIsReExportedInDebugForSwitchScript() { + val exported = exportedByActivityName() + assertEquals(true, exported["com.anod.car.home.incar.SwitchInCarActivity"]) + } +} diff --git a/app/src/test/kotlin/com/anod/car/home/incar/ModeDetectorTest.kt b/app/src/test/kotlin/com/anod/car/home/incar/ModeDetectorTest.kt new file mode 100644 index 00000000..1ca39f4b --- /dev/null +++ b/app/src/test/kotlin/com/anod/car/home/incar/ModeDetectorTest.kt @@ -0,0 +1,74 @@ +// Copyright (c) CarWidget contributors. Licensed under the project license. +package com.anod.car.home.incar + +import info.anodsplace.carwidget.content.preferences.InCarInterface +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotSame +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Exercises the thread-safety hardening around ModeDetector's shared state: prefState must hand + * back an independent copy, and updatePrefState/forceState must map each preference flag to the + * correct slot without leaking across flags. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [31], application = android.app.Application::class) +class ModeDetectorTest { + + @Test + fun prefStateReturnsDefensiveCopy() { + ModeDetector.updatePrefState(InCarInterface.NoOp(isPowerRequired = true)) + val snapshot = ModeDetector.prefState + assertNotSame(snapshot, ModeDetector.prefState) + assertTrue(snapshot[FLAG_POWER]) + + snapshot[FLAG_POWER] = false + + assertTrue("mutating the returned array must not affect internal state", ModeDetector.prefState[FLAG_POWER]) + } + + @Test + fun updatePrefStateMapsEachPreferenceFlag() { + ModeDetector.updatePrefState( + InCarInterface.NoOp( + isPowerRequired = true, + isHeadsetRequired = false, + isBluetoothRequired = true, + isActivityRequired = true, + isCarDockRequired = false + ) + ) + val state = ModeDetector.prefState + assertTrue(state[FLAG_POWER]) + assertFalse(state[FLAG_HEADSET]) + assertTrue(state[FLAG_BLUETOOTH]) + assertTrue(state[FLAG_ACTIVITY]) + assertFalse(state[FLAG_CAR_DOCK]) + } + + @Test + fun forceStateActivatesOnlyEnabledFlags() { + ModeDetector.forceState( + InCarInterface.NoOp(isPowerRequired = true, isBluetoothRequired = false), + forceMode = true + ) + val byFlag = ModeDetector.eventsState().associateBy { it.id } + + assertTrue(byFlag.getValue(FLAG_POWER).enabled) + assertTrue(byFlag.getValue(FLAG_POWER).active) + assertFalse(byFlag.getValue(FLAG_BLUETOOTH).enabled) + assertFalse(byFlag.getValue(FLAG_BLUETOOTH).active) + } + + private companion object { + const val FLAG_POWER = 0 + const val FLAG_HEADSET = 1 + const val FLAG_BLUETOOTH = 2 + const val FLAG_ACTIVITY = 3 + const val FLAG_CAR_DOCK = 4 + } +} diff --git a/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/BitmapLruCacheUnitTest.kt b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/BitmapLruCacheUnitTest.kt new file mode 100644 index 00000000..0e8b3f1b --- /dev/null +++ b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/BitmapLruCacheUnitTest.kt @@ -0,0 +1,55 @@ +// Copyright (c) CarWidget contributors. Licensed under the project license. +package info.anodsplace.carwidget.content + +import android.graphics.Bitmap +import androidx.test.core.app.ApplicationProvider +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Guards the LruCache sizing fix: sizeOf() must never round down to 0 for sub-kilobyte + * bitmaps (which previously let entries accumulate without counting against the cache), + * and the cache budget must be a positive number of kilobytes. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [31]) +class BitmapLruCacheUnitTest { + + private fun cache() = BitmapLruCache(ApplicationProvider.getApplicationContext()) + + private fun bitmap(width: Int, height: Int) = + Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888) + + @Test + fun cacheBudgetIsPositiveKilobytes() { + assertTrue(cache().maxSize() > 0) + } + + @Test + fun subKilobyteBitmapCountsAsOneKilobyte() { + val cache = cache() + cache.put("tiny", bitmap(1, 1)) + assertEquals(1, cache.size()) + } + + @Test + fun largerBitmapIsMeasuredInKilobytes() { + val cache = cache() + // 64 x 64 x 4 bytes = 16384 bytes = 16 KB + cache.put("big", bitmap(64, 64)) + assertEquals(16, cache.size()) + } + + @Test + fun storedBitmapCanBeRetrieved() { + val cache = cache() + val bmp = bitmap(2, 2) + cache.put("k", bmp) + assertSame(bmp, cache.get("k")) + } +} diff --git a/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutCorruptRowUnitTest.kt b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutCorruptRowUnitTest.kt new file mode 100644 index 00000000..b48edbdd --- /dev/null +++ b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutCorruptRowUnitTest.kt @@ -0,0 +1,131 @@ +// Copyright (c) CarWidget contributors. Licensed under the project license. +package info.anodsplace.carwidget.content.db + +import android.content.ComponentName +import android.content.Intent +import androidx.sqlite.db.SupportSQLiteDatabase +import androidx.test.core.app.ApplicationProvider +import app.cash.sqldelight.adapter.primitive.IntColumnAdapter +import app.cash.sqldelight.driver.android.AndroidSqliteDriver +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Corrupt rows (empty/unparseable intent) must be treated as invalid so that reads suppress + * them: loadShortcut/loadTarget return null for the slot, and folder reads (loadFolderItems, + * observeFolder) drop the bad child while keeping the valid ones. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [31]) +class ShortcutCorruptRowUnitTest { + + private fun createDb(): Triple { + val context = ApplicationProvider.getApplicationContext() + val driver = AndroidSqliteDriver( + schema = Database.Schema, + context = context, + name = "shortcut-corrupt-unit.db", + callback = object : AndroidSqliteDriver.Callback(Database.Schema) { + override fun onConfigure(db: SupportSQLiteDatabase) { + db.setForeignKeyConstraintsEnabled(true) + } + } + ) + val database = Database( + driver = driver, + favoritesAdapter = Favorites.Adapter( + targetIdAdapter = IntColumnAdapter, + iconTypeAdapter = IntColumnAdapter, + itemTypeAdapter = IntColumnAdapter, + positionAdapter = IntColumnAdapter + ), + FolderItemAdapter = FolderItem.Adapter( + iconTypeAdapter = IntColumnAdapter, + itemTypeAdapter = IntColumnAdapter + ) + ) + return Triple(ShortcutsDatabase(database), database, driver) + } + + private fun activityIntentUri(cls: String): String = + Intent(Intent.ACTION_MAIN) + .addCategory(Intent.CATEGORY_LAUNCHER) + .setComponent(ComponentName("com.test", cls)) + .toUri(0) + + private fun Database.insertRawShortcut(targetId: Int, position: Int, title: String, intent: String) { + shortcutsQueries.insert( + targetId = targetId, + position = position, + itemType = 0, + title = title, + intent = intent, + iconType = 0, + icon = null, + iconPackage = null, + iconResource = null, + isCustomIcon = false + ) + } + + private fun Database.insertRawFolderItem(shortcutId: Long, itemId: String, title: String, intent: String) { + folderItemQueries.insertFolder( + shortcutId = shortcutId, + itemId = itemId, + itemType = 0, + title = title, + intent = intent, + iconType = 0, + icon = null, + iconPackage = null, + iconResource = null, + isCustomIcon = false + ) + } + + @Test + fun emptyIntentShortcut_isSuppressedButValidNeighbourRemains() = runBlocking { + val (shortcutsDb, database, driver) = createDb() + driver.use { + database.insertRawShortcut(TARGET, 0, "Corrupt", intent = "") + database.insertRawShortcut(TARGET, 1, "Valid", intent = activityIntentUri("ValidActivity")) + + assertNull(shortcutsDb.loadShortcut(TARGET, 0)) + assertNotNull(shortcutsDb.loadShortcut(TARGET, 1)) + + val target = shortcutsDb.loadTarget(TARGET) + assertNull(target[0]) + assertEquals("Valid", target[1]?.title) + } + } + + @Test + fun emptyIntentFolderItem_isFilteredFromFolderReads() = runBlocking { + val (shortcutsDb, database, driver) = createDb() + driver.use { + database.insertRawShortcut(TARGET, 0, "Folder", intent = activityIntentUri("FolderActivity")) + val folderId = database.shortcutsQueries.lastInsertId().executeAsOne() + database.insertRawFolderItem(folderId, itemId = "valid", title = "Child", intent = activityIntentUri("ChildActivity")) + database.insertRawFolderItem(folderId, itemId = "corrupt", title = "Broken", intent = "") + + val items = shortcutsDb.loadFolderItems(folderId) + assertEquals(1, items.size) + assertEquals("Child", items[0].title) + + val observed = shortcutsDb.observeFolder(folderId).first() + assertEquals(1, observed.size) + assertEquals("Child", observed[0].title) + } + } + + companion object { + private const val TARGET = 33 + } +} diff --git a/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt new file mode 100644 index 00000000..3e23c428 --- /dev/null +++ b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt @@ -0,0 +1,184 @@ +// Copyright (c) CarWidget contributors. Licensed under the project license. +package info.anodsplace.carwidget.content.db + +import android.content.ComponentName +import android.content.Intent +import android.graphics.Bitmap +import androidx.sqlite.db.SupportSQLiteDatabase +import androidx.test.core.app.ApplicationProvider +import app.cash.sqldelight.adapter.primitive.IntColumnAdapter +import app.cash.sqldelight.driver.android.AndroidSqliteDriver +import info.anodsplace.carwidget.content.shortcuts.ShortcutExtra +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Covers the delete-then-insert write path: overwriting an occupied (targetId, position) + * slot must attach folder children to the freshly inserted row, cascade-delete the previous + * row's children, and let copyShortcut replace an occupied slot without throwing. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [31]) +class ShortcutOverwriteUnitTest { + + private fun createDb(): Pair { + val context = ApplicationProvider.getApplicationContext() + val driver = AndroidSqliteDriver( + schema = Database.Schema, + context = context, + name = "shortcut-overwrite-unit.db", + callback = object : AndroidSqliteDriver.Callback(Database.Schema) { + override fun onConfigure(db: SupportSQLiteDatabase) { + db.setForeignKeyConstraintsEnabled(true) + } + } + ) + val database = Database( + driver = driver, + favoritesAdapter = Favorites.Adapter( + targetIdAdapter = IntColumnAdapter, + iconTypeAdapter = IntColumnAdapter, + itemTypeAdapter = IntColumnAdapter, + positionAdapter = IntColumnAdapter + ), + FolderItemAdapter = FolderItem.Adapter( + iconTypeAdapter = IntColumnAdapter, + itemTypeAdapter = IntColumnAdapter + ) + ) + return ShortcutsDatabase(database) to driver + } + + private fun icon() = ShortcutIcon.forActivity(0, Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888)) + + private fun activity(title: String, cls: String): Shortcut = + Shortcut.forActivity(0, 0, title, false, ComponentName("com.test", cls), 0) + + private fun folder(title: String): Shortcut = + Shortcut( + id = 0, + position = 0, + itemType = LauncherSettings.Favorites.ITEM_TYPE_APPLICATION, + title = title, + isCustomIcon = false, + intent = Intent(ShortcutExtra.ACTION_FOLDER) + ) + + @Test + fun saveFolder_overOccupiedPosition_attachesChildrenToNewFolder() = runBlocking { + val (db, driver) = createDb() + driver.use { + // Occupy the destination slot, then create another shortcut so last_insert_rowid() + // points at a different row than the slot being overwritten. + db.addItem(TARGET, 2, activity("Occupant", "OccupantActivity"), icon()) + val neighbourId = db.addItem(TARGET, 0, activity("Neighbour", "NeighbourActivity"), icon()) + + val children = listOf( + activity("Child1", "Child1Activity") to icon(), + activity("Child2", "Child2Activity") to icon() + ) + val folderId = db.saveFolder(TARGET, 2, folder("Folder"), icon(), children) + + val folderRow = db.loadShortcut(TARGET, 2) + assertNotNull(folderRow) + assertTrue(folderRow!!.isFolder) + assertEquals(folderRow.id, folderId) + + val folderItems = db.loadFolderItems(folderId) + assertEquals(2, folderItems.size) + assertEquals("Child1", folderItems[0].title) + assertEquals("Child2", folderItems[1].title) + + // Children must not be misattached to the unrelated neighbour row. + assertTrue(db.loadFolderItems(neighbourId).isEmpty()) + } + } + + @Test + fun saveFolder_overExistingFolder_cascadeDeletesOldChildren() = runBlocking { + val (db, driver) = createDb() + driver.use { + val oldId = db.saveFolder( + TARGET, 0, folder("Old"), icon(), + listOf( + activity("OldA", "OldAActivity") to icon(), + activity("OldB", "OldBActivity") to icon() + ) + ) + assertEquals(2, db.loadFolderItems(oldId).size) + + val newId = db.saveFolder( + TARGET, 0, folder("New"), icon(), + listOf(activity("NewA", "NewAActivity") to icon()) + ) + + assertNotEqualsId(oldId, newId) + assertTrue(db.loadFolderItems(oldId).isEmpty()) + val newItems = db.loadFolderItems(newId) + assertEquals(1, newItems.size) + assertEquals("NewA", newItems[0].title) + assertEquals(newId, db.loadShortcut(TARGET, 0)!!.id) + } + } + + @Test + fun copyShortcut_ontoOccupiedPosition_replacesWithoutThrowing() = runBlocking { + val (db, driver) = createDb() + driver.use { + val sourceId = db.addItem(TARGET, 0, activity("Source", "SourceActivity"), icon()) + db.addItem(TARGET, 1, activity("Occupant", "OccupantActivity"), icon()) + + val result = db.copyShortcut(TARGET, 1, sourceId) + + assertTrue(result) + val copied = db.loadShortcut(TARGET, 1) + assertNotNull(copied) + assertEquals("Source", copied!!.title) + assertEquals(2, db.loadTarget(TARGET).size) + } + } + + @Test + fun copyShortcut_folderOntoOccupiedPosition_copiesChildrenAndDropsOld() = runBlocking { + val (db, driver) = createDb() + driver.use { + val sourceId = db.saveFolder( + TARGET, 0, folder("Source"), icon(), + listOf( + activity("SrcA", "SrcAActivity") to icon(), + activity("SrcB", "SrcBActivity") to icon() + ) + ) + val destId = db.saveFolder( + TARGET, 1, folder("Dest"), icon(), + listOf(activity("OldChild", "OldChildActivity") to icon()) + ) + + val result = db.copyShortcut(TARGET, 1, sourceId) + + assertTrue(result) + val newDest = db.loadShortcut(TARGET, 1) + assertNotNull(newDest) + assertTrue(newDest!!.isFolder) + val copiedItems = db.loadFolderItems(newDest.id) + assertEquals(2, copiedItems.size) + assertEquals("SrcA", copiedItems[0].title) + assertEquals("SrcB", copiedItems[1].title) + assertTrue(db.loadFolderItems(destId).isEmpty()) + } + } + + private fun assertNotEqualsId(unexpected: Long, actual: Long) = + assertFalse("expected a new row id, got reused id $actual", unexpected == actual) + + companion object { + private const val TARGET = 21 + } +} diff --git a/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/BitmapLruCache.kt b/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/BitmapLruCache.kt index c70f85c6..84620108 100644 --- a/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/BitmapLruCache.kt +++ b/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/BitmapLruCache.kt @@ -5,6 +5,7 @@ import android.content.Context import android.content.pm.ApplicationInfo import android.graphics.Bitmap import android.util.LruCache +import kotlin.math.max /** * @author alex @@ -16,7 +17,7 @@ class BitmapLruCache(context: Context) : LruCache(calculateMemor // The cache size will be measured in kilobytes rather than // number of items. - return bitmap.byteCount / 1024 + return max(1, bitmap.byteCount / 1024) } companion object { @@ -24,8 +25,8 @@ class BitmapLruCache(context: Context) : LruCache(calculateMemor val am = context.getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager val largeHeap = context.applicationInfo?.flags?.and(ApplicationInfo.FLAG_LARGE_HEAP) val memoryClass = if (largeHeap != 0) am.largeMemoryClass else am.memoryClass - // Target ~15% of the available heap. - return 1024 * 1024 * memoryClass / 7 + // Target ~15% of the available heap, measured in kilobytes to match sizeOf(). + return memoryClass * 1024 / 7 } } } diff --git a/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/backup/BackupManager.kt b/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/backup/BackupManager.kt index 56b8449b..5392561b 100644 --- a/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/backup/BackupManager.kt +++ b/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/backup/BackupManager.kt @@ -15,9 +15,7 @@ import info.anodsplace.carwidget.content.preferences.InCarSettings import info.anodsplace.carwidget.content.preferences.WidgetSettings import info.anodsplace.carwidget.content.shortcuts.NotificationShortcutsModel import info.anodsplace.carwidget.content.shortcuts.WidgetShortcutsModel -import kotlinx.coroutines.DelicateCoroutinesApi -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.newSingleThreadContext +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import java.io.FileNotFoundException import java.io.IOException @@ -30,15 +28,13 @@ sealed interface BackupCheckResult { class Success(val hasInCar: Boolean) : BackupCheckResult } -@OptIn(ExperimentalCoroutinesApi::class) class BackupManager( private val context: Context, private val database: ShortcutsDatabase, private val inCarSettings: InCarSettings ) { - @OptIn(DelicateCoroutinesApi::class) - suspend fun backup(appWidgetIdScope: AppWidgetIdScope, uri: Uri): Int = withContext(newSingleThreadContext("BackupWidget")) { + suspend fun backup(appWidgetIdScope: AppWidgetIdScope, uri: Uri): Int = withContext(Dispatchers.IO) { return@withContext try { val outputStream = context.contentResolver.openOutputStream(uri) ?: return@withContext Backup.ERROR_UNEXPECTED writeBackup(outputStream, appWidgetIdScope) @@ -48,8 +44,7 @@ class BackupManager( } } - @OptIn(DelicateCoroutinesApi::class) - suspend fun checkSrcUri(srcUri: Uri): BackupCheckResult = withContext(newSingleThreadContext("RestoreWidget")) { + suspend fun checkSrcUri(srcUri: Uri): BackupCheckResult = withContext(Dispatchers.IO) { return@withContext try { val inputStream: InputStream = context.contentResolver.openInputStream(srcUri) ?: return@withContext BackupCheckResult.Error(errorCode = Backup.ERROR_FILE_READ) checkBackup(inputStream) @@ -59,8 +54,7 @@ class BackupManager( } } - @OptIn(DelicateCoroutinesApi::class) - suspend fun restore(appWidgetIdScope: AppWidgetIdScope, uri: Uri, restoreInCar: Boolean): Int = withContext(newSingleThreadContext("RestoreWidget")) { + suspend fun restore(appWidgetIdScope: AppWidgetIdScope, uri: Uri, restoreInCar: Boolean): Int = withContext(Dispatchers.IO) { return@withContext try { val inputStream = context.contentResolver.openInputStream(uri) ?: return@withContext Backup.ERROR_FILE_READ doRestoreWidget(inputStream, appWidgetIdScope, restoreInCar) diff --git a/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/db/ShortcutsDatabase.kt b/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/db/ShortcutsDatabase.kt index 9751d78c..7de0e573 100644 --- a/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/db/ShortcutsDatabase.kt +++ b/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/db/ShortcutsDatabase.kt @@ -93,6 +93,7 @@ class ShortcutsDatabase(private val db: Database) { return db.folderItemQueries.selectFolderShortcut(shortcutId, mapper = ::mapFolderItem) .asFlow() .mapToList(Dispatchers.IO) + .map { list -> list.filter { it.isValid } } } /** @@ -102,9 +103,11 @@ class ShortcutsDatabase(private val db: Database) { */ suspend fun addItem(targetId: Int, position: Int, item: Shortcut, icon: ShortcutIcon): Long = withContext(Dispatchers.IO) { - insert(targetId, position, item, icon) - return@withContext db.shortcutsQueries.lastInsertId().executeAsOneOrNull() - ?: Shortcut.ID_UNKNOWN + return@withContext db.transactionWithResult { + insert(targetId, position, item, icon) + db.shortcutsQueries.lastInsertId().executeAsOneOrNull() + ?: Shortcut.ID_UNKNOWN + } } suspend fun saveFolder( @@ -196,34 +199,23 @@ class ShortcutsDatabase(private val db: Database) { private fun insert(targetId: Int, position: Int, item: Shortcut, icon: ShortcutIcon) { val values = createShortcutContentValues(item, icon) - try { - db.shortcutsQueries.insert( - targetId = targetId, - position = position, - itemType = values.getAsInteger(LauncherSettings.Favorites.ITEM_TYPE), - title = values.getAsString(LauncherSettings.Favorites.TITLE), - intent = values.getAsString(LauncherSettings.Favorites.INTENT), - iconType = values.getAsInteger(LauncherSettings.Favorites.ICON_TYPE), - icon = values.getAsByteArray(LauncherSettings.Favorites.ICON), - iconPackage = values.getAsString(LauncherSettings.Favorites.ICON_PACKAGE), - iconResource = values.getAsString(LauncherSettings.Favorites.ICON_RESOURCE), - isCustomIcon = values.getAsBoolean(LauncherSettings.Favorites.IS_CUSTOM_ICON) - ) - } catch (e: SQLiteConstraintException) { - AppLog.e(e) - db.shortcutsQueries.update( - targetId = targetId, - position = position, - itemType = values.getAsInteger(LauncherSettings.Favorites.ITEM_TYPE), - title = values.getAsString(LauncherSettings.Favorites.TITLE), - intent = values.getAsString(LauncherSettings.Favorites.INTENT), - iconType = values.getAsInteger(LauncherSettings.Favorites.ICON_TYPE), - icon = values.getAsByteArray(LauncherSettings.Favorites.ICON), - iconPackage = values.getAsString(LauncherSettings.Favorites.ICON_PACKAGE), - iconResource = values.getAsString(LauncherSettings.Favorites.ICON_RESOURCE), - isCustomIcon = values.getAsBoolean(LauncherSettings.Favorites.IS_CUSTOM_ICON) - ) - } + // Replace any existing row at (targetId, position) so the INSERT always creates a new + // row. This keeps last_insert_rowid() valid for callers (folder children were being + // attached to a stale id when an UPDATE fallback was used) and lets the FK ON DELETE + // CASCADE drop the previous shortcut's folder items. + db.shortcutsQueries.deleteTargetPosition(targetId, position) + db.shortcutsQueries.insert( + targetId = targetId, + position = position, + itemType = values.getAsInteger(LauncherSettings.Favorites.ITEM_TYPE), + title = values.getAsString(LauncherSettings.Favorites.TITLE), + intent = values.getAsString(LauncherSettings.Favorites.INTENT), + iconType = values.getAsInteger(LauncherSettings.Favorites.ICON_TYPE), + icon = values.getAsByteArray(LauncherSettings.Favorites.ICON), + iconPackage = values.getAsString(LauncherSettings.Favorites.ICON_PACKAGE), + iconResource = values.getAsString(LauncherSettings.Favorites.ICON_RESOURCE), + isCustomIcon = values.getAsBoolean(LauncherSettings.Favorites.IS_CUSTOM_ICON) + ) } private fun insertFolderItem(shortcutId: Long, item: Shortcut, icon: ShortcutIcon) { @@ -286,6 +278,9 @@ class ShortcutsDatabase(private val db: Database) { withContext(Dispatchers.IO) { return@withContext db.transactionWithResult { if (sourceShortcutId != Shortcut.ID_UNKNOWN) { + // Free the destination slot first: duplicateShortcut does an INSERT that would + // otherwise violate UNIQUE(targetId, position) and throw when the slot is taken. + db.shortcutsQueries.deleteTargetPosition(targetId, position) val result = db.shortcutsQueries.duplicateShortcut(targetId, position, sourceShortcutId) if (result.value > 0) { val shortcutId = db.shortcutsQueries.lastInsertId().executeAsOneOrNull() @@ -325,14 +320,14 @@ class ShortcutsDatabase(private val db: Database) { ): Shortcut { if (intent.isEmpty()) { AppLog.e("Intent is empty for id:${shortcutId} title:${title}") - Shortcut(shortcutId, position, itemType, title, isCustomIcon, Intent()) + return Shortcut(Shortcut.ID_UNKNOWN, position, itemType, title, isCustomIcon, Intent()) } val parsedIntent: Intent = try { Intent.parseUri(intent, 0) } catch (e: URISyntaxException) { AppLog.e(e) - Intent() + return Shortcut(Shortcut.ID_UNKNOWN, position, itemType, title, isCustomIcon, Intent()) } return Shortcut(shortcutId, position, itemType, title, isCustomIcon, parsedIntent) @@ -349,14 +344,14 @@ class ShortcutsDatabase(private val db: Database) { ): Shortcut { if (intent.isEmpty()) { AppLog.e("Intent is empty for id:${itemId} title:${title}") - return Shortcut(id, -1, itemType, title, isCustomIcon, Intent()) + return Shortcut(Shortcut.ID_UNKNOWN, -1, itemType, title, isCustomIcon, Intent()) } val parsedIntent: Intent = try { Intent.parseUri(intent, 0) } catch (e: URISyntaxException) { AppLog.e(e) - Intent() + return Shortcut(Shortcut.ID_UNKNOWN, -1, itemType, title, isCustomIcon, Intent()) } return Shortcut(id, -1, itemType, title, isCustomIcon, parsedIntent) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 97262165..89a2704a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -28,6 +28,7 @@ play-services-location = "21.4.0" preference-ktx = "1.2.1" sqldelight = "2.3.2" junit = "1.3.0" +junit4 = "4.13.2" espresso-core = "3.7.0" uiautomator = "2.4.0" benchmark = "1.5.0-alpha07" @@ -83,6 +84,7 @@ sqldelight-coroutines-extensions-jvm = { module = "app.cash.sqldelight:coroutine sqldelight-primitive-adapters = { module = "app.cash.sqldelight:primitive-adapters", version.ref = "sqldelight" } sqldelight-driver-sqlite = { module = "app.cash.sqldelight:sqlite-driver", version.ref = "sqldelight" } androidx-junit = { group = "androidx.test.ext", name = "junit", version.ref = "junit" } +junit4 = { group = "junit", name = "junit", version.ref = "junit4" } androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espresso-core" } androidx-uiautomator = { group = "androidx.test.uiautomator", name = "uiautomator", version.ref = "uiautomator" } androidx-benchmark-macro-junit4 = { group = "androidx.benchmark", name = "benchmark-macro-junit4", version.ref = "benchmark" } From 721bc032eb329af5c34beb3cdba44a1db5db4db5 Mon Sep 17 00:00:00 2001 From: Alex Gavrishev <171704+anod@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:23:03 +0300 Subject: [PATCH 2/4] fix: guard copyShortcut self-copy and move observeFolder filter off main Address PR review: copyShortcut deleted the destination slot before duplicating, which for a self-copy (destination already holds sourceShortcutId) dropped the source row and made duplicateShortcut a silent no-op. Early-return when the slot already holds the source. Also flowOn(Dispatchers.IO) for observeFolder so the isValid filter runs off the UI thread. Adds self-copy regression tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bf666aec-f66e-4aa1-b001-2f43c2ed883d --- .../content/db/ShortcutOverwriteUnitTest.kt | 43 +++++++++++++++++++ .../carwidget/content/db/ShortcutsDatabase.kt | 10 +++++ 2 files changed, 53 insertions(+) diff --git a/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt index 3e23c428..58f1db27 100644 --- a/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt +++ b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt @@ -175,6 +175,49 @@ class ShortcutOverwriteUnitTest { } } + @Test + fun copyShortcut_ontoOwnPosition_isNoOpAndKeepsSource() = runBlocking { + val (db, driver) = createDb() + driver.use { + val sourceId = db.addItem(TARGET, 1, activity("Source", "SourceActivity"), icon()) + + // Copying a shortcut onto the slot it already occupies must not destroy it. + val result = db.copyShortcut(TARGET, 1, sourceId) + + assertTrue(result) + val kept = db.loadShortcut(TARGET, 1) + assertNotNull(kept) + assertEquals(sourceId, kept!!.id) + assertEquals("Source", kept.title) + assertEquals(1, db.loadTarget(TARGET).size) + } + } + + @Test + fun copyShortcut_folderOntoOwnPosition_keepsChildren() = runBlocking { + val (db, driver) = createDb() + driver.use { + val folderId = db.saveFolder( + TARGET, 1, folder("Source"), icon(), + listOf( + activity("SrcA", "SrcAActivity") to icon(), + activity("SrcB", "SrcBActivity") to icon() + ) + ) + + val result = db.copyShortcut(TARGET, 1, folderId) + + assertTrue(result) + val kept = db.loadShortcut(TARGET, 1) + assertNotNull(kept) + assertEquals(folderId, kept!!.id) + val items = db.loadFolderItems(folderId) + assertEquals(2, items.size) + assertEquals("SrcA", items[0].title) + assertEquals("SrcB", items[1].title) + } + } + private fun assertNotEqualsId(unexpected: Long, actual: Long) = assertFalse("expected a new row id, got reused id $actual", unexpected == actual) diff --git a/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/db/ShortcutsDatabase.kt b/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/db/ShortcutsDatabase.kt index 7de0e573..21c17bec 100644 --- a/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/db/ShortcutsDatabase.kt +++ b/content/src/androidMain/kotlin/info/anodsplace/carwidget/content/db/ShortcutsDatabase.kt @@ -15,6 +15,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.withContext import java.net.URISyntaxException @@ -94,6 +95,7 @@ class ShortcutsDatabase(private val db: Database) { .asFlow() .mapToList(Dispatchers.IO) .map { list -> list.filter { it.isValid } } + .flowOn(Dispatchers.IO) } /** @@ -278,6 +280,14 @@ class ShortcutsDatabase(private val db: Database) { withContext(Dispatchers.IO) { return@withContext db.transactionWithResult { if (sourceShortcutId != Shortcut.ID_UNKNOWN) { + // Copying a shortcut onto the slot it already occupies is a no-op. Skip the + // delete-then-insert below, which would otherwise drop the source row and turn + // duplicateShortcut (which copies FROM that same row) into a silent failure. + val occupantId = db.shortcutsQueries.selectTargetPosition(targetId, position) + .executeAsOneOrNull()?.shortcutId + if (occupantId == sourceShortcutId) { + return@transactionWithResult true + } // Free the destination slot first: duplicateShortcut does an INSERT that would // otherwise violate UNIQUE(targetId, position) and throw when the slot is taken. db.shortcutsQueries.deleteTargetPosition(targetId, position) From 5e2d65f900f24eb8897de50b6442cb134e305b7e Mon Sep 17 00:00:00 2001 From: Alex Gavrishev <171704+anod@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:32:20 +0300 Subject: [PATCH 3/4] perf: build in-car notification on Dispatchers.IO instead of Default Address PR review: rich notification build performs blocking work (PackageManager.getActivityIcon via ShortcutIconLoader), so run it on Dispatchers.IO rather than the CPU-bound Default pool to avoid starving it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bf666aec-f66e-4aa1-b001-2f43c2ed883d --- app/src/main/java/com/anod/car/home/incar/ModeService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/anod/car/home/incar/ModeService.kt b/app/src/main/java/com/anod/car/home/incar/ModeService.kt index 18c9db74..e43161ea 100644 --- a/app/src/main/java/com/anod/car/home/incar/ModeService.kt +++ b/app/src/main/java/com/anod/car/home/incar/ModeService.kt @@ -115,7 +115,7 @@ class ModeService : Service(), KoinComponent { private fun updateNotification() { serviceScope.launch { try { - val notification = withContext(Dispatchers.Default) { notificationFactory.create() } + val notification = withContext(Dispatchers.IO) { notificationFactory.create() } if (sInCarMode) { getSystemService(NotificationManager::class.java) ?.notify(InCarModeNotificationFactory.id, notification) From f419435e9946c8d9cf9bd47ef1d3de130c5b14b7 Mon Sep 17 00:00:00 2001 From: Alex Gavrishev <171704+anod@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:39:21 +0300 Subject: [PATCH 4/4] fix: preserve coroutine cancellation and make exported-activity test build-variant aware Address PR review: ModeService.updateNotification() now re-throws CancellationException so the service coroutine can still be cancelled cooperatively; only other exceptions are logged. ExportedActivitiesTest asserts SwitchInCarActivity's exported flag against BuildConfig.DEBUG instead of unconditionally true, so it stays correct under a release unit-test variant (release keeps it exported=false). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bf666aec-f66e-4aa1-b001-2f43c2ed883d --- app/src/main/java/com/anod/car/home/incar/ModeService.kt | 3 +++ .../test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/anod/car/home/incar/ModeService.kt b/app/src/main/java/com/anod/car/home/incar/ModeService.kt index e43161ea..e245d89a 100644 --- a/app/src/main/java/com/anod/car/home/incar/ModeService.kt +++ b/app/src/main/java/com/anod/car/home/incar/ModeService.kt @@ -16,6 +16,7 @@ import info.anodsplace.applog.AppLog import info.anodsplace.carwidget.content.preferences.InCarSettings import info.anodsplace.permissions.AppPermission import info.anodsplace.permissions.AppPermissions +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -120,6 +121,8 @@ class ModeService : Service(), KoinComponent { getSystemService(NotificationManager::class.java) ?.notify(InCarModeNotificationFactory.id, notification) } + } catch (e: CancellationException) { + throw e } catch (e: Exception) { AppLog.e(e) } diff --git a/app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt b/app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt index 7e7bc1da..a59c6faa 100644 --- a/app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt +++ b/app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt @@ -36,8 +36,9 @@ class ExportedActivitiesTest { } @Test - fun switchInCarActivityIsReExportedInDebugForSwitchScript() { + fun switchInCarActivityExportedOnlyInDebugBuilds() { val exported = exportedByActivityName() - assertEquals(true, exported["com.anod.car.home.incar.SwitchInCarActivity"]) + // Debug re-exports it for the adb switch script; release keeps it exported=false. + assertEquals(BuildConfig.DEBUG, exported["com.anod.car.home.incar.SwitchInCarActivity"]) } }