From f2eddbaf5bc91d198e1afb612df4538b7f789ffb Mon Sep 17 00:00:00 2001 From: tapframe <85391825+tapframe@users.noreply.github.com> Date: Sat, 27 Jun 2026 15:17:44 +0530 Subject: [PATCH] ref: add validation for duplicate collection --- .../composeResources/values/strings.xml | 2 + .../collection/CollectionManagementScreen.kt | 9 +- .../collection/CollectionRepository.kt | 161 +++++++++--------- .../CollectionSourceSerializationTest.kt | 35 ++++ 4 files changed, 128 insertions(+), 79 deletions(-) diff --git a/composeApp/src/commonMain/composeResources/values/strings.xml b/composeApp/src/commonMain/composeResources/values/strings.xml index 105c796c..68ef9347 100644 --- a/composeApp/src/commonMain/composeResources/values/strings.xml +++ b/composeApp/src/commonMain/composeResources/values/strings.xml @@ -1466,8 +1466,10 @@ Resume %1$s JSON is empty. Collection %1$d has blank id. + Collection id '%1$s' is used more than once. Collection '%1$s' has blank title. Folder %1$d in '%2$s' has blank id. + Folder id '%1$s' is used more than once in '%2$s'. Folder '%1$s' in '%2$s' has blank title. Source %1$d in folder '%2$s' has blank fields. Source %1$d in folder '%2$s' is missing a Trakt list ID. diff --git a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/collection/CollectionManagementScreen.kt b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/collection/CollectionManagementScreen.kt index 74deba81..6e0c3e88 100644 --- a/composeApp/src/commonMain/kotlin/com/nuvio/app/features/collection/CollectionManagementScreen.kt +++ b/composeApp/src/commonMain/kotlin/com/nuvio/app/features/collection/CollectionManagementScreen.kt @@ -55,6 +55,7 @@ import com.nuvio.app.core.ui.NuvioScreenHeader import com.nuvio.app.core.ui.NuvioSectionLabel import com.nuvio.app.core.ui.NuvioStatusModal import com.nuvio.app.core.ui.NuvioSurfaceCard +import com.nuvio.app.core.ui.withDuplicateSafeLazyKeys import nuvio.composeapp.generated.resources.* import org.jetbrains.compose.resources.stringResource import sh.calvin.reorderable.ReorderableCollectionItemScope @@ -215,6 +216,9 @@ private fun CollectionReorderableList( ) { val hapticFeedback = LocalHapticFeedback.current val lazyListState = rememberLazyListState() + val keyedCollections = remember(collections) { + collections.withDuplicateSafeLazyKeys { collection -> collection.id } + } val reorderableLazyListState = rememberReorderableLazyListState( lazyListState = lazyListState, ) { from, to -> @@ -229,8 +233,9 @@ private fun CollectionReorderableList( state = lazyListState, verticalArrangement = Arrangement.spacedBy(12.dp), ) { - itemsIndexed(collections, key = { _, collection -> collection.id }) { _, collection -> - ReorderableItem(reorderableLazyListState, key = collection.id) { isDragging -> + itemsIndexed(keyedCollections, key = { _, collection -> collection.lazyKey }) { _, keyedCollection -> + val collection = keyedCollection.value + ReorderableItem(reorderableLazyListState, key = keyedCollection.lazyKey) { isDragging -> val elevation by animateDpAsState(if (isDragging) 4.dp else 0.dp) Surface( 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 b5baf26e..5f801327 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 @@ -19,9 +19,11 @@ import kotlinx.serialization.json.JsonElement import nuvio.composeapp.generated.resources.Res import nuvio.composeapp.generated.resources.collections_import_error_collection_blank_id import nuvio.composeapp.generated.resources.collections_import_error_collection_blank_title +import nuvio.composeapp.generated.resources.collections_import_error_collection_duplicate_id import nuvio.composeapp.generated.resources.collections_import_error_empty_json import nuvio.composeapp.generated.resources.collections_import_error_folder_blank_id import nuvio.composeapp.generated.resources.collections_import_error_folder_blank_title +import nuvio.composeapp.generated.resources.collections_import_error_folder_duplicate_id import nuvio.composeapp.generated.resources.collections_import_error_invalid_json import nuvio.composeapp.generated.resources.collections_import_error_source_blank_fields import nuvio.composeapp.generated.resources.collections_import_error_trakt_list_id @@ -128,6 +130,10 @@ object CollectionRepository { fun importFromJson(jsonString: String): Result> { return runCatching { + val validation = validateJson(jsonString) + if (!validation.valid) { + throw IllegalArgumentException(validation.error.orEmpty()) + } rawCollectionsJson = json.parseToJsonElement(jsonString) val imported = json.decodeFromString>(jsonString) _collections.value = CollectionMobileSettingsRepository.applyToCollections(imported) @@ -145,87 +151,13 @@ object CollectionRepository { } return try { val collections = json.decodeFromString>(jsonString) - var totalFolders = 0 - collections.forEachIndexed { ci, c -> - if (c.id.isBlank()) { - return ValidationResult( - valid = false, - error = runBlocking { - getString(Res.string.collections_import_error_collection_blank_id, ci + 1) - }, - ) - } - if (c.title.isBlank()) { - return ValidationResult( - valid = false, - error = runBlocking { - getString(Res.string.collections_import_error_collection_blank_title, c.id) - }, - ) - } - c.folders.forEachIndexed { fi, f -> - if (f.id.isBlank()) { - return ValidationResult( - valid = false, - error = runBlocking { - getString( - Res.string.collections_import_error_folder_blank_id, - fi + 1, - c.title, - ) - }, - ) - } - if (f.title.isBlank()) { - return ValidationResult( - valid = false, - error = runBlocking { - getString( - Res.string.collections_import_error_folder_blank_title, - f.id, - c.title, - ) - }, - ) - } - f.resolvedSources.forEachIndexed { si, s -> - if (s.hasInvalidTraktListId()) { - return ValidationResult( - valid = false, - error = runBlocking { - getString( - Res.string.collections_import_error_trakt_list_id, - si + 1, - f.title, - ) - }, - ) - } - - val invalidAddon = !s.isTmdb && !s.isTrakt && - (s.addonId.isNullOrBlank() || s.type.isNullOrBlank() || s.catalogId.isNullOrBlank()) - val invalidTmdb = s.isTmdb && - s.tmdbSourceType.isNullOrBlank() - if (invalidAddon || invalidTmdb) { - return ValidationResult( - valid = false, - error = runBlocking { - getString( - Res.string.collections_import_error_source_blank_fields, - si + 1, - f.title, - ) - }, - ) - } - } - totalFolders++ - } + validateImportModel(collections)?.let { error -> + return ValidationResult(valid = false, error = error.localizedMessage()) } ValidationResult( valid = true, collectionCount = collections.size, - folderCount = totalFolders, + folderCount = collections.sumOf { it.folders.size }, ) } catch (e: Exception) { ValidationResult( @@ -294,3 +226,78 @@ object CollectionRepository { rawCollectionsJson = it } } + +internal sealed interface CollectionImportModelError { + data class BlankCollectionId(val collectionIndex: Int) : CollectionImportModelError + data class DuplicateCollectionId(val collectionId: String) : CollectionImportModelError + data class BlankCollectionTitle(val collectionId: String) : CollectionImportModelError + data class BlankFolderId(val folderIndex: Int, val collectionTitle: String) : CollectionImportModelError + data class DuplicateFolderId(val folderId: String, val collectionTitle: String) : CollectionImportModelError + data class BlankFolderTitle(val folderId: String, val collectionTitle: String) : CollectionImportModelError + data class InvalidTraktListId(val sourceIndex: Int, val folderTitle: String) : CollectionImportModelError + data class BlankSourceFields(val sourceIndex: Int, val folderTitle: String) : CollectionImportModelError +} + +internal fun validateImportModel(collections: List): CollectionImportModelError? { + val collectionIds = mutableSetOf() + collections.forEachIndexed { ci, c -> + if (c.id.isBlank()) { + return CollectionImportModelError.BlankCollectionId(ci + 1) + } + if (!collectionIds.add(c.id)) { + return CollectionImportModelError.DuplicateCollectionId(c.id) + } + if (c.title.isBlank()) { + return CollectionImportModelError.BlankCollectionTitle(c.id) + } + + val folderIds = mutableSetOf() + c.folders.forEachIndexed { fi, f -> + if (f.id.isBlank()) { + return CollectionImportModelError.BlankFolderId(fi + 1, c.title) + } + if (!folderIds.add(f.id)) { + return CollectionImportModelError.DuplicateFolderId(f.id, c.title) + } + if (f.title.isBlank()) { + return CollectionImportModelError.BlankFolderTitle(f.id, c.title) + } + f.resolvedSources.forEachIndexed { si, s -> + if (s.hasInvalidTraktListId()) { + return CollectionImportModelError.InvalidTraktListId(si + 1, f.title) + } + + val invalidAddon = !s.isTmdb && !s.isTrakt && + (s.addonId.isNullOrBlank() || s.type.isNullOrBlank() || s.catalogId.isNullOrBlank()) + val invalidTmdb = s.isTmdb && + s.tmdbSourceType.isNullOrBlank() + if (invalidAddon || invalidTmdb) { + return CollectionImportModelError.BlankSourceFields(si + 1, f.title) + } + } + } + } + return null +} + +private fun CollectionImportModelError.localizedMessage(): String = + runBlocking { + when (this@localizedMessage) { + is CollectionImportModelError.BlankCollectionId -> + getString(Res.string.collections_import_error_collection_blank_id, collectionIndex) + is CollectionImportModelError.DuplicateCollectionId -> + getString(Res.string.collections_import_error_collection_duplicate_id, collectionId) + is CollectionImportModelError.BlankCollectionTitle -> + getString(Res.string.collections_import_error_collection_blank_title, collectionId) + is CollectionImportModelError.BlankFolderId -> + getString(Res.string.collections_import_error_folder_blank_id, folderIndex, collectionTitle) + is CollectionImportModelError.DuplicateFolderId -> + getString(Res.string.collections_import_error_folder_duplicate_id, folderId, collectionTitle) + is CollectionImportModelError.BlankFolderTitle -> + getString(Res.string.collections_import_error_folder_blank_title, folderId, collectionTitle) + is CollectionImportModelError.InvalidTraktListId -> + getString(Res.string.collections_import_error_trakt_list_id, sourceIndex, folderTitle) + is CollectionImportModelError.BlankSourceFields -> + getString(Res.string.collections_import_error_source_blank_fields, sourceIndex, folderTitle) + } + } diff --git a/composeApp/src/commonTest/kotlin/com/nuvio/app/features/collection/CollectionSourceSerializationTest.kt b/composeApp/src/commonTest/kotlin/com/nuvio/app/features/collection/CollectionSourceSerializationTest.kt index 5f83cd99..04836f29 100644 --- a/composeApp/src/commonTest/kotlin/com/nuvio/app/features/collection/CollectionSourceSerializationTest.kt +++ b/composeApp/src/commonTest/kotlin/com/nuvio/app/features/collection/CollectionSourceSerializationTest.kt @@ -93,6 +93,41 @@ class CollectionSourceSerializationTest { assertTrue(source.hasInvalidTraktListId()) } + @Test + fun importModelRejectsDuplicateCollectionIds() { + val collections = listOf( + Collection(id = "collection-1", title = "One"), + Collection(id = "collection-1", title = "Two"), + ) + + assertEquals( + CollectionImportModelError.DuplicateCollectionId("collection-1"), + validateImportModel(collections), + ) + } + + @Test + fun importModelRejectsDuplicateFolderIdsWithinCollection() { + val collections = listOf( + Collection( + id = "collection-1", + title = "Favorites", + folders = listOf( + CollectionFolder(id = "folder-1", title = "One"), + CollectionFolder(id = "folder-1", title = "Two"), + ), + ), + ) + + assertEquals( + CollectionImportModelError.DuplicateFolderId( + folderId = "folder-1", + collectionTitle = "Favorites", + ), + validateImportModel(collections), + ) + } + @Test fun legacyAddonCatalogSourcesRemainCompatible() { val payload = """