From cd70e3654c798cbe5e96fc708d9c0f4b5b8aeeed Mon Sep 17 00:00:00 2001 From: QIU Date: Wed, 26 Aug 2026 03:51:03 +0000 Subject: [PATCH] fix(sources): a custom URL no longer wipes the manual-import type index The previous commit indexed custom-URL satellites under "Other", which is the key manual file import already writes. setSatelliteTypeIds overwrites rather than merges, so importing a file and then updating from a custom URL erased each other's type index - a probe confirmed it in both directions. The satellites were never at risk: database rows survive because insertEntries is REPLACE with no delete, and the selection is a separate id list. What was lost was their grouping in the type filter. Still worth a distinct key, since a URL and a hand-picked file are different things. Custom URLs now use "Custom". Manual import keeps "Other". The test asserts the new key and that "Other" stays untouched, so the collision cannot come back unnoticed. --- .../core/data/repository/DatabaseRepo.kt | 20 ++++++++++++++----- .../core/data/repository/DatabaseRepoTest.kt | 11 ++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/DatabaseRepo.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/DatabaseRepo.kt index 6ac5b62e..05a6891c 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/DatabaseRepo.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/DatabaseRepo.kt @@ -42,6 +42,17 @@ class DatabaseRepo( private val customSourceType = "Other" + /** + * Type key for satellites fetched from a custom URL. + * + * Separate from customSourceType because setSatelliteTypeIds overwrites rather than merges, so + * sharing "Other" with manual file import meant each wiped the other's type index. The + * satellites stayed in the database and stayed selectable either way - only their grouping in + * the type filter was lost - but the two sources are different things and deserve different + * keys. + */ + private val customUrlType = "Custom" + override suspend fun updateTLEFromFile(uri: String): Int = withContext(dispatcher) { var importedCount = 0 remoteSource.getFileStream(uri)?.let { stream -> @@ -78,12 +89,11 @@ class DatabaseRepo( // The type index for the skipped keys goes stale rather than empty, which is the honest // outcome - it is the last known membership, not a claim about this fetch. // - // The key is customSourceType, not "All": setSatelliteTypeIds early-returns on "All", so - // indexing under it was always a no-op, and "Other" is what manual file import already - // uses. Same meaning - satellites from a source the operator supplied - and it makes them - // reachable by the type filter, which they were not before. + // The key is customUrlType, not "All": setSatelliteTypeIds early-returns on "All", so + // indexing under it was always a no-op and satellites from a custom URL were never + // reachable by the type filter at all. They now are. val tleUrls = if (dataSourcesSettings.useCustomTLE && dataSourcesSettings.tleUrl.isNotBlank()) { - mapOf(customSourceType to dataSourcesSettings.tleUrl) + mapOf(customUrlType to dataSourcesSettings.tleUrl) } else { Sources.satelliteDataUrls.filterValues { it.isNotBlank() } } diff --git a/core/data/src/test/java/com/rtbishop/look4sat/core/data/repository/DatabaseRepoTest.kt b/core/data/src/test/java/com/rtbishop/look4sat/core/data/repository/DatabaseRepoTest.kt index 4e927707..46467c9b 100644 --- a/core/data/src/test/java/com/rtbishop/look4sat/core/data/repository/DatabaseRepoTest.kt +++ b/core/data/src/test/java/com/rtbishop/look4sat/core/data/repository/DatabaseRepoTest.kt @@ -114,11 +114,14 @@ class DatabaseRepoTest { "the operator's URL must be fetched", customCsvUrl in remoteSource.requestedUrls ) - // Indexed under "Other", the key manual file import uses. It used to be indexed under - // "All", where setSatelliteTypeIds early-returns - so the type filter never saw these - // satellites at all and this assertion was checking a no-op. - assertEquals(listOf(25544), settingsRepo.satelliteTypeIdsByType["Other"]) + // Indexed under "Custom". It used to go under "All", where setSatelliteTypeIds + // early-returns, so the type filter never saw these satellites and the old assertion here + // was checking a no-op. + assertEquals(listOf(25544), settingsRepo.satelliteTypeIdsByType["Custom"]) assertEquals(null, settingsRepo.satelliteTypeIdsByType["All"]) + // NOT "Other": that key belongs to manual file import, and setSatelliteTypeIds overwrites + // rather than merges, so sharing it would have each source wipe the other's index. + assertEquals(null, settingsRepo.satelliteTypeIdsByType["Other"]) } /** The switch-off path must be untouched: all built-in sources, exactly as before. */