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 42b637b7..6ac5b62e 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 @@ -67,17 +67,33 @@ class DatabaseRepo( override suspend fun updateFromRemote() = withContext(dispatcher) { val dataSourcesSettings = settingsRepo.dataSourcesSettings.value - val tleUrls = buildMap { - putAll(Sources.satelliteDataUrls) - // Switch on + non-empty URL -> All uses the custom URL; otherwise the default URL (online-update default source) - put("All", if (dataSourcesSettings.useCustomTLE && dataSourcesSettings.tleUrl.isNotBlank()) - dataSourcesSettings.tleUrl else Sources.defaultTleUrl) - }.filterValues { it.isNotBlank() } - val radioUrls = buildMap { - putAll(Sources.transceiversDataUrls) - put("SatNOGS", if (dataSourcesSettings.useCustomTransceivers && dataSourcesSettings.transceiversUrl.isNotBlank()) - dataSourcesSettings.transceiversUrl else Sources.defaultTransceiversUrl) - }.filterValues { it.isNotBlank() } + // A custom URL REPLACES the built-in sources rather than joining them. The previous map + // overwrote only the "All" value and still fetched the other 26, so switching this on meant + // "my source AND yours" - which defeats the reasons for setting one: a mirror, a filtered + // subset, an offline server, or a network where Celestrak is unreachable. On a blocked link + // the real behaviour was 26 failing requests. + // + // Satellites already stored do not disappear: insertEntries is OnConflictStrategy.REPLACE + // and nothing is deleted before the insert, so rows the new source does not mention survive. + // 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. + val tleUrls = if (dataSourcesSettings.useCustomTLE && dataSourcesSettings.tleUrl.isNotBlank()) { + mapOf(customSourceType to dataSourcesSettings.tleUrl) + } else { + Sources.satelliteDataUrls.filterValues { it.isNotBlank() } + } + val radioUrls = if (dataSourcesSettings.useCustomTransceivers && + dataSourcesSettings.transceiversUrl.isNotBlank() + ) { + mapOf("SatNOGS" to dataSourcesSettings.transceiversUrl) + } else { + Sources.transceiversDataUrls.filterValues { it.isNotBlank() } + } // launch all network requests concurrently val tleJobs = tleUrls.values.map { url -> async { url to remoteSource.getNetworkStream(url) } } val radioJobs = radioUrls.values.map { url -> async { url to remoteSource.getNetworkStream(url) } } 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 58194d23..4e927707 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 @@ -18,6 +18,7 @@ package com.rtbishop.look4sat.core.data.repository import com.rtbishop.look4sat.core.domain.model.DataSourcesSettings +import com.rtbishop.look4sat.core.domain.source.Sources import com.rtbishop.look4sat.core.domain.model.DatabaseState import com.rtbishop.look4sat.core.domain.model.OtherSettings import com.rtbishop.look4sat.core.domain.model.PassesSettings @@ -102,8 +103,57 @@ class DatabaseRepoTest { repository.updateFromRemote() assertTrue(localSource.insertedEntries.any { it.catnum == 25544 }) - // New semantics: switch on + non-empty URL -> the All source uses the custom URL, data lands in the All type - assertEquals(listOf(25544), settingsRepo.satelliteTypeIdsByType["All"]) + // A custom URL replaces the built-in TLE sources: none of them is requested. The + // transceivers group is separate and its own switch is off here, so it still fetches. + val builtInTle = Sources.satelliteDataUrls.values.filter { it.isNotBlank() } + assertTrue( + "no built-in TLE source may be fetched, got " + remoteSource.requestedUrls, + builtInTle.none { it in remoteSource.requestedUrls } + ) + assertTrue( + "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"]) + assertEquals(null, settingsRepo.satelliteTypeIdsByType["All"]) + } + + /** The switch-off path must be untouched: all built-in sources, exactly as before. */ + @Test + fun `without a custom source every built-in source is fetched`() = runTest(dispatcher) { + val localSource = FakeLocalSource() + val remoteSource = FakeRemoteSource().apply { + // Every built-in TLE source has to answer, or updateFromRemote throws because all of + // them failed, which would mask what this test checks. The transceivers group is left + // unanswered on purpose: org.json is compileOnly in core:domain, so DataParser cannot + // parse a radio payload on the JVM anyway. + Sources.satelliteDataUrls.values.filter { it.isNotBlank() } + .forEach { networkStreams[it] = { validCsvStream() } } + } + val settingsRepo = FakeSettingsRepo( + dataSources = DataSourcesSettings( + useCustomTLE = false, + useCustomTransceivers = false, + tleUrl = "https://example.com/ignored.csv", + transceiversUrl = "" + ) + ) + val repository = DatabaseRepo(dispatcher, dataParser, localSource, remoteSource, settingsRepo) + + repository.updateFromRemote() + + val expected = Sources.satelliteDataUrls.values.filter { it.isNotBlank() } + assertTrue( + "expected all built-in sources, got " + remoteSource.requestedUrls.size, + expected.all { it in remoteSource.requestedUrls } + ) + assertTrue( + "the custom URL must not be fetched when the switch is off", + "https://example.com/ignored.csv" !in remoteSource.requestedUrls + ) } private fun validCsvStream(): InputStream = """ @@ -122,9 +172,15 @@ private class FakeRemoteSource : IRemoteSource { val fileStreams: MutableMap InputStream> = mutableMapOf() val networkStreams: MutableMap InputStream> = mutableMapOf() + /** Every URL asked for, so a test can assert WHICH sources were fetched, not just the result. */ + val requestedUrls = mutableListOf() + override suspend fun getFileStream(uri: String): InputStream? = fileStreams[uri]?.invoke() - override suspend fun getNetworkStream(url: String): InputStream? = networkStreams[url]?.invoke() + override suspend fun getNetworkStream(url: String): InputStream? { + requestedUrls += url + return networkStreams[url]?.invoke() + } override suspend fun getAmSatCatalog(): String? = null