Skip to content

Commit c33b3cb

Browse files
jamesarichclaude
andcommitted
fix(data): don't mark ignored empty link payloads fresh; single-flight stale link refreshes
- store() reports whether it persisted anything; lastRefreshMillis only advances on an applied payload, so an ignored empty response can't suppress retries for a whole expiration window. - refreshIfStale() double-checks freshness under a dedicated mutex so concurrent collectors don't start duplicate long-running fetches now that the fetch runs outside writeMutex. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a19e2d9 commit c33b3cb

1 file changed

Lines changed: 15 additions & 7 deletions

File tree

core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/DeviceLinkRepositoryImpl.kt

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ class DeviceLinkRepositoryImpl(
6363
/** Serializes seeding and network refreshes so concurrent collectors don't duplicate writes. */
6464
private val writeMutex = Mutex()
6565

66+
/** Single-flights stale-triggered refreshes so concurrent collectors don't start duplicate fetches. */
67+
private val refreshMutex = Mutex()
68+
6669
@Volatile private var lastRefreshMillis = 0L
6770

6871
override suspend fun ensureImported() {
@@ -78,8 +81,11 @@ class DeviceLinkRepositoryImpl(
7881
val remoteLinks = remoteDataSource.getDeviceLinks()
7982
writeMutex.withLock {
8083
withContext(NonCancellable + dispatchers.io) {
81-
store(remoteLinks)
82-
lastRefreshMillis = nowMillis
84+
// Only an applied payload counts as fresh — an ignored empty response must not suppress
85+
// retries for a whole expiration window while the cache stays stale.
86+
if (store(remoteLinks)) {
87+
lastRefreshMillis = nowMillis
88+
}
8389
}
8490
}
8591
}
@@ -128,25 +134,27 @@ class DeviceLinkRepositoryImpl(
128134
}
129135
}
130136

131-
/** Best-effort network refresh, gated by [CACHE_EXPIRATION_TIME_MS]. */
137+
/** Best-effort network refresh, gated by [CACHE_EXPIRATION_TIME_MS] and single-flighted via [refreshMutex]. */
132138
private suspend fun refreshIfStale() {
133-
if (nowMillis - lastRefreshMillis > CACHE_EXPIRATION_TIME_MS) reconcile()
139+
if (nowMillis - lastRefreshMillis <= CACHE_EXPIRATION_TIME_MS) return
140+
refreshMutex.withLock { if (nowMillis - lastRefreshMillis > CACHE_EXPIRATION_TIME_MS) reconcile() }
134141
}
135142

136143
/**
137144
* Maps resolved API links to the cached domain model, upserts them, and prunes short codes that no longer exist.
138145
* Internal links (GitHub, YouTube, …) are dropped — they never belong to a device's purchase section. An empty list
139-
* is ignored rather than wiping the cache on a bad response.
146+
* is ignored rather than wiping the cache on a bad response. Returns whether anything was stored.
140147
*/
141-
private suspend fun store(networkLinks: List<NetworkDeviceLink>) {
148+
private suspend fun store(networkLinks: List<NetworkDeviceLink>): Boolean {
142149
val links = networkLinks.filter { it.type != NetworkDeviceLink.TYPE_INTERNAL }.map { it.toDeviceLink() }
143150
if (links.isEmpty()) {
144151
Logger.w { "DeviceLinkRepository: no device links to store; leaving cache untouched" }
145-
return
152+
return false
146153
}
147154
localDataSource.upsertAll(links.map { it.asEntity() })
148155
localDataSource.deleteNotIn(links.map { it.shortCode })
149156
Logger.i { "DeviceLinkRepository: stored ${links.size} device links" }
157+
return true
150158
}
151159

152160
private companion object {

0 commit comments

Comments
 (0)