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..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 @@ -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,13 @@ 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.CancellationException +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 +32,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 +72,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 +106,29 @@ 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.IO) { notificationFactory.create() } + if (sInCarMode) { + getSystemService(NotificationManager::class.java) + ?.notify(InCarModeNotificationFactory.id, notification) + } + } catch (e: CancellationException) { + throw e + } 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 +183,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..a59c6faa --- /dev/null +++ b/app/src/test/kotlin/com/anod/car/home/ExportedActivitiesTest.kt @@ -0,0 +1,44 @@ +// 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 switchInCarActivityExportedOnlyInDebugBuilds() { + val exported = exportedByActivityName() + // Debug re-exports it for the adb switch script; release keeps it exported=false. + assertEquals(BuildConfig.DEBUG, 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..58f1db27 --- /dev/null +++ b/content/src/androidHostTest/kotlin/info/anodsplace/carwidget/content/db/ShortcutOverwriteUnitTest.kt @@ -0,0 +1,227 @@ +// 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()) + } + } + + @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) + + 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..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 @@ -93,6 +94,8 @@ class ShortcutsDatabase(private val db: Database) { return db.folderItemQueries.selectFolderShortcut(shortcutId, mapper = ::mapFolderItem) .asFlow() .mapToList(Dispatchers.IO) + .map { list -> list.filter { it.isValid } } + .flowOn(Dispatchers.IO) } /** @@ -102,9 +105,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 +201,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 +280,17 @@ 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) val result = db.shortcutsQueries.duplicateShortcut(targetId, position, sourceShortcutId) if (result.value > 0) { val shortcutId = db.shortcutsQueries.lastInsertId().executeAsOneOrNull() @@ -325,14 +330,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 +354,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" }