From c68d99688165cd72a4d996f1d90ea0dbfd4fd6e6 Mon Sep 17 00:00:00 2001 From: tapframe <85391825+tapframe@users.noreply.github.com> Date: Thu, 16 Jul 2026 22:40:38 +0530 Subject: [PATCH] fix: fix collection keys dupliation causing sigsegv --- .../collection/CollectionRepository.kt | 53 ++++++++++++++++--- .../home/HomeCatalogSettingsRepository.kt | 7 ++- .../com/nuvio/app/features/home/HomeScreen.kt | 11 ++-- .../app/core/ui/DuplicateSafeLazyKeysTest.kt | 14 +++++ .../collection/CollectionRepositoryTest.kt | 36 +++++++++++++ .../home/HomeCollectionDefinitionsTest.kt | 24 +++++++++ 6 files changed, 134 insertions(+), 11 deletions(-) create mode 100644 composeApp/src/commonTest/kotlin/com/nuvio/app/core/ui/DuplicateSafeLazyKeysTest.kt create mode 100644 composeApp/src/commonTest/kotlin/com/nuvio/app/features/collection/CollectionRepositoryTest.kt create mode 100644 composeApp/src/commonTest/kotlin/com/nuvio/app/features/home/HomeCollectionDefinitionsTest.kt diff --git a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/collection/CollectionRepository.kt b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/collection/CollectionRepository.kt index 5f801327..7bd6f211 100644 --- a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/collection/CollectionRepository.kt +++ b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/collection/CollectionRepository.kt @@ -56,7 +56,11 @@ object CollectionRepository { val parsed = json.parseToJsonElement(payload) rawCollectionsJson = parsed val decoded = json.decodeFromString>(payload) - _collections.value = CollectionMobileSettingsRepository.applyToCollections(decoded) + val normalized = normalizeCollections(decoded, source = "local storage") + _collections.value = CollectionMobileSettingsRepository.applyToCollections(normalized) + if (normalized.size != decoded.size) { + persist(sync = false) + } }.onFailure { e -> log.e(e) { "Failed to load collections from storage" } } @@ -79,14 +83,15 @@ object CollectionRepository { fun addCollection(collection: Collection) { ensureLoaded() - _collections.value = _collections.value + CollectionMobileSettingsRepository.applyToCollection(collection) + val decorated = CollectionMobileSettingsRepository.applyToCollection(collection) + _collections.value = _collections.value.upsertCollectionById(decorated) persist() } fun updateCollection(collection: Collection) { ensureLoaded() val decorated = CollectionMobileSettingsRepository.applyToCollection(collection) - _collections.value = _collections.value.map { + _collections.value = _collections.value.deduplicatedById().map { if (it.id == collection.id) decorated else it } persist() @@ -100,7 +105,8 @@ object CollectionRepository { fun setCollections(collections: List) { ensureLoaded() - _collections.value = CollectionMobileSettingsRepository.applyToCollections(collections) + val normalized = normalizeCollections(collections, source = "setCollections") + _collections.value = CollectionMobileSettingsRepository.applyToCollections(normalized) persist() } @@ -135,7 +141,7 @@ object CollectionRepository { throw IllegalArgumentException(validation.error.orEmpty()) } rawCollectionsJson = json.parseToJsonElement(jsonString) - val imported = json.decodeFromString>(jsonString) + val imported = json.decodeFromString>(jsonString).deduplicatedById() _collections.value = CollectionMobileSettingsRepository.applyToCollections(imported) persist() imported @@ -197,13 +203,26 @@ object CollectionRepository { internal fun applyFromRemote(collections: List, rawJson: JsonElement) { rawCollectionsJson = rawJson - _collections.value = CollectionMobileSettingsRepository.applyToCollections(collections) + val normalized = normalizeCollections(collections, source = "remote sync") + _collections.value = CollectionMobileSettingsRepository.applyToCollections(normalized) persist(sync = false) } internal fun onMobileSettingsChanged() { if (!hasLoaded) return - _collections.value = CollectionMobileSettingsRepository.applyToCollections(_collections.value) + _collections.value = CollectionMobileSettingsRepository.applyToCollections( + _collections.value.deduplicatedById(), + ) + } + + private fun normalizeCollections(collections: List, source: String): List { + val normalized = collections.deduplicatedById() + if (normalized.size != collections.size) { + log.w { + "Dropped ${collections.size - normalized.size} duplicate collection IDs from $source" + } + } + return normalized } private fun ensureLoaded() { @@ -227,6 +246,26 @@ object CollectionRepository { } } +internal fun List.deduplicatedById(): List { + if (size < 2) return this + + val collectionsById = linkedMapOf() + forEach { collection -> + collectionsById[collection.id] = collection + } + return if (collectionsById.size == size) this else collectionsById.values.toList() +} + +internal fun List.upsertCollectionById(collection: Collection): List { + val normalized = deduplicatedById() + val existingIndex = normalized.indexOfFirst { existing -> existing.id == collection.id } + if (existingIndex == -1) return normalized + collection + + return normalized.toMutableList().apply { + this[existingIndex] = collection + } +} + internal sealed interface CollectionImportModelError { data class BlankCollectionId(val collectionIndex: Int) : CollectionImportModelError data class DuplicateCollectionId(val collectionId: String) : CollectionImportModelError diff --git a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/home/HomeCatalogSettingsRepository.kt b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/home/HomeCatalogSettingsRepository.kt index de504b5e..0dde2af5 100644 --- a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/home/HomeCatalogSettingsRepository.kt +++ b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/home/HomeCatalogSettingsRepository.kt @@ -588,8 +588,13 @@ internal data class CollectionCatalogDefinition( val isPinnedToTop: Boolean, ) +internal fun visibleCollectionsWithUniqueIds(collections: List): List = + collections + .filter { collection -> collection.folders.isNotEmpty() } + .distinctBy(Collection::id) + internal fun buildCollectionDefinitions(collections: List): List = - collections.filter { it.folders.isNotEmpty() }.map { collection -> + visibleCollectionsWithUniqueIds(collections).map { collection -> CollectionCatalogDefinition( key = "collection_${collection.id}", collectionId = collection.id, diff --git a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/home/HomeScreen.kt b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/home/HomeScreen.kt index f502369f..31676eee 100644 --- a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/home/HomeScreen.kt +++ b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/home/HomeScreen.kt @@ -25,6 +25,7 @@ import com.nuvio.app.core.ui.NuvioNetworkOfflineCard import com.nuvio.app.core.ui.nuvioSafeBottomPadding import com.nuvio.app.core.ui.rememberHeroStretchState import com.nuvio.app.core.ui.rememberPosterCardStyleUiState +import com.nuvio.app.core.ui.withDuplicateSafeLazyKeys import com.nuvio.app.features.addons.AddonRepository import com.nuvio.app.features.addons.enabledAddons import com.nuvio.app.features.cloud.CloudLibraryContentType @@ -774,6 +775,9 @@ fun HomeScreen( val enabledHomeItems = remember(homeSettingsUiState.items) { homeSettingsUiState.items.filter { it.enabled } } + val keyedEnabledHomeItems = remember(enabledHomeItems) { + enabledHomeItems.withDuplicateSafeLazyKeys(HomeCatalogSettingsItem::key) + } val visibleSeriesPosterTargets = remember(enabledHomeItems, sectionsMap) { enabledHomeItems .filterNot { it.isCollection } @@ -970,11 +974,12 @@ fun HomeScreen( } } - enabledHomeItems.forEach { settingsItem -> + keyedEnabledHomeItems.forEach { keyedSettingsItem -> + val settingsItem = keyedSettingsItem.value if (settingsItem.isCollection) { val collection = collectionsMap[settingsItem.key] if (collection != null) { - item(key = settingsItem.key) { + item(key = keyedSettingsItem.lazyKey) { HomeCollectionRowSection( collection = collection, modifier = Modifier.padding(bottom = 12.dp), @@ -987,7 +992,7 @@ fun HomeScreen( } else { val section = sectionsMap[settingsItem.key] if (section != null && section.items.isNotEmpty()) { - item(key = settingsItem.key) { + item(key = keyedSettingsItem.lazyKey) { HomeCatalogRowSection( section = section, entries = section.items.take(HOME_CATALOG_PREVIEW_LIMIT), diff --git a/composeApp/src/commonTest/kotlin/com/nuvio/app/core/ui/DuplicateSafeLazyKeysTest.kt b/composeApp/src/commonTest/kotlin/com/nuvio/app/core/ui/DuplicateSafeLazyKeysTest.kt new file mode 100644 index 00000000..9d46669f --- /dev/null +++ b/composeApp/src/commonTest/kotlin/com/nuvio/app/core/ui/DuplicateSafeLazyKeysTest.kt @@ -0,0 +1,14 @@ +package com.nuvio.app.core.ui + +import kotlin.test.Test +import kotlin.test.assertEquals + +class DuplicateSafeLazyKeysTest { + @Test + fun `duplicate lazy keys receive stable occurrence suffixes`() { + val result = listOf("duplicate", "other", "duplicate") + .withDuplicateSafeLazyKeys { value -> value } + + assertEquals(listOf("duplicate#0", "other", "duplicate#1"), result.map { it.lazyKey }) + } +} diff --git a/composeApp/src/commonTest/kotlin/com/nuvio/app/features/collection/CollectionRepositoryTest.kt b/composeApp/src/commonTest/kotlin/com/nuvio/app/features/collection/CollectionRepositoryTest.kt new file mode 100644 index 00000000..81de684a --- /dev/null +++ b/composeApp/src/commonTest/kotlin/com/nuvio/app/features/collection/CollectionRepositoryTest.kt @@ -0,0 +1,36 @@ +package com.nuvio.app.features.collection + +import kotlin.test.Test +import kotlin.test.assertEquals + +class CollectionRepositoryTest { + @Test + fun `deduplication keeps stable order and latest collection value`() { + val collections = listOf( + Collection(id = "duplicate", title = "Old value"), + Collection(id = "other", title = "Other"), + Collection(id = "duplicate", title = "Latest value"), + ) + + val result = collections.deduplicatedById() + + assertEquals(listOf("duplicate", "other"), result.map(Collection::id)) + assertEquals("Latest value", result.first().title) + } + + @Test + fun `upsert replaces duplicate collection without changing its position`() { + val collections = listOf( + Collection(id = "duplicate", title = "Old value"), + Collection(id = "other", title = "Other"), + Collection(id = "duplicate", title = "Stale duplicate"), + ) + + val result = collections.upsertCollectionById( + Collection(id = "duplicate", title = "Replacement"), + ) + + assertEquals(listOf("duplicate", "other"), result.map(Collection::id)) + assertEquals("Replacement", result.first().title) + } +} diff --git a/composeApp/src/commonTest/kotlin/com/nuvio/app/features/home/HomeCollectionDefinitionsTest.kt b/composeApp/src/commonTest/kotlin/com/nuvio/app/features/home/HomeCollectionDefinitionsTest.kt new file mode 100644 index 00000000..d62c74e3 --- /dev/null +++ b/composeApp/src/commonTest/kotlin/com/nuvio/app/features/home/HomeCollectionDefinitionsTest.kt @@ -0,0 +1,24 @@ +package com.nuvio.app.features.home + +import com.nuvio.app.features.collection.Collection +import com.nuvio.app.features.collection.CollectionFolder +import kotlin.test.Test +import kotlin.test.assertEquals + +class HomeCollectionDefinitionsTest { + @Test + fun `visible home collections ignore duplicate IDs and empty collections`() { + val visibleFolder = CollectionFolder(id = "folder", title = "Folder") + val collections = listOf( + Collection(id = "duplicate", title = "First", folders = listOf(visibleFolder)), + Collection(id = "empty", title = "Empty"), + Collection(id = "duplicate", title = "Second", folders = listOf(visibleFolder)), + Collection(id = "other", title = "Other", folders = listOf(visibleFolder)), + ) + + val result = visibleCollectionsWithUniqueIds(collections) + + assertEquals(listOf("duplicate", "other"), result.map(Collection::id)) + assertEquals("First", result.first().title) + } +}