fix(sources): a custom URL now replaces the built-in sources

Switching "Custom TLE URL" on used to mean "my source AND yours". The map overwrote only
the value keyed "All" and the other 26 built-in sources were still fetched, so pointing
Look4Sat at a mirror, a filtered subset, an offline server or a URL reachable on a censored
network did not stop it hitting Celestrak 26 more times. On a blocked link the real
behaviour was 26 failing requests.

This was a fossil rather than a decision: upstream has satelliteDataUrls as a plain list of
six URLs with no keys and no custom-URL concept, all fetched unconditionally. The fork
turned the list into a keyed map and bolted the override onto one key.

Three things made replacing safe to choose, all checked rather than assumed. Stored
satellites do not disappear, because insertEntries is OnConflictStrategy.REPLACE and
updateFromRemote deletes nothing first, so rows the new source does not mention survive.
The selection is a plain id list and is untouched. What degrades is the type filter for the
skipped keys, which goes stale rather than empty - the last known membership, not a claim
about the current fetch.

The key is now customSourceType ("Other"), not "All". setSatelliteTypeIds early-returns on
"All", so indexing there was always a no-op - satellites from a custom URL were never
reachable by the type filter at all. "Other" is what manual file import already uses, which
is the same meaning: satellites from a source the operator supplied. The existing test
asserted the "All" index, which means it was asserting a no-op; it now checks that no
built-in source is fetched and that the entries land somewhere the filter can see.

A second test covers the switch-off path, which must fetch every built-in source exactly as
before.
This commit is contained in:
mckero committed 2026-08-26 03:44:59 +00:00
1 parent b1dce9c518
commit a25f0fe2cf
2 files changed
+86 -14

No files matched your search

@@ -67,17 +67,33 @@ class DatabaseRepo(
override suspend fun updateFromRemote() = withContext(dispatcher) { override suspend fun updateFromRemote() = withContext(dispatcher) {
val dataSourcesSettings = settingsRepo.dataSourcesSettings.value val dataSourcesSettings = settingsRepo.dataSourcesSettings.value
val tleUrls = buildMap { // A custom URL REPLACES the built-in sources rather than joining them. The previous map
putAll(Sources.satelliteDataUrls) // overwrote only the "All" value and still fetched the other 26, so switching this on meant
// Switch on + non-empty URL -> All uses the custom URL; otherwise the default URL (online-update default source) // "my source AND yours" - which defeats the reasons for setting one: a mirror, a filtered
put("All", if (dataSourcesSettings.useCustomTLE && dataSourcesSettings.tleUrl.isNotBlank()) // subset, an offline server, or a network where Celestrak is unreachable. On a blocked link
dataSourcesSettings.tleUrl else Sources.defaultTleUrl) // the real behaviour was 26 failing requests.
}.filterValues { it.isNotBlank() } //
val radioUrls = buildMap { // Satellites already stored do not disappear: insertEntries is OnConflictStrategy.REPLACE
putAll(Sources.transceiversDataUrls) // and nothing is deleted before the insert, so rows the new source does not mention survive.
put("SatNOGS", if (dataSourcesSettings.useCustomTransceivers && dataSourcesSettings.transceiversUrl.isNotBlank()) // The type index for the skipped keys goes stale rather than empty, which is the honest
dataSourcesSettings.transceiversUrl else Sources.defaultTransceiversUrl) // outcome - it is the last known membership, not a claim about this fetch.
}.filterValues { it.isNotBlank() } //
// 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 // launch all network requests concurrently
val tleJobs = tleUrls.values.map { url -> async { url to remoteSource.getNetworkStream(url) } } val tleJobs = tleUrls.values.map { url -> async { url to remoteSource.getNetworkStream(url) } }
val radioJobs = radioUrls.values.map { url -> async { url to remoteSource.getNetworkStream(url) } } val radioJobs = radioUrls.values.map { url -> async { url to remoteSource.getNetworkStream(url) } }
@@ -18,6 +18,7 @@
package com.rtbishop.look4sat.core.data.repository package com.rtbishop.look4sat.core.data.repository
import com.rtbishop.look4sat.core.domain.model.DataSourcesSettings 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.DatabaseState
import com.rtbishop.look4sat.core.domain.model.OtherSettings import com.rtbishop.look4sat.core.domain.model.OtherSettings
import com.rtbishop.look4sat.core.domain.model.PassesSettings import com.rtbishop.look4sat.core.domain.model.PassesSettings
@@ -102,8 +103,57 @@ class DatabaseRepoTest {
repository.updateFromRemote() repository.updateFromRemote()
assertTrue(localSource.insertedEntries.any { it.catnum == 25544 }) 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 // A custom URL replaces the built-in TLE sources: none of them is requested. The
assertEquals(listOf(25544), settingsRepo.satelliteTypeIdsByType["All"]) // 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 = """ private fun validCsvStream(): InputStream = """
@@ -122,9 +172,15 @@ private class FakeRemoteSource : IRemoteSource {
val fileStreams: MutableMap<String, () -> InputStream> = mutableMapOf() val fileStreams: MutableMap<String, () -> InputStream> = mutableMapOf()
val networkStreams: MutableMap<String, () -> InputStream> = mutableMapOf() val networkStreams: MutableMap<String, () -> InputStream> = mutableMapOf()
/** Every URL asked for, so a test can assert WHICH sources were fetched, not just the result. */
val requestedUrls = mutableListOf<String>()
override suspend fun getFileStream(uri: String): InputStream? = fileStreams[uri]?.invoke() 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 override suspend fun getAmSatCatalog(): String? = null