refactor(repository)!: improve api caching and error handling (#2680)

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>

app/src/main/java/com/geeksville/mesh/model/MetricsViewModel.kt

@@ -264,7 +264,11 @@ constructor(
                     val deviceHardware =
                         actualNode.user.hwModel.number.let { deviceHardwareRepository.getDeviceHardwareByModel(it) }
                     _state.update { state ->
-                        state.copy(node = actualNode, isLocal = destNum == ourNode, deviceHardware = deviceHardware)
+                        state.copy(
+                            node = actualNode,
+                            isLocal = destNum == ourNode,
+                            deviceHardware = deviceHardware.getOrNull(),
+                        )
                     }
                 }
                 .launchIn(viewModelScope)

app/src/main/java/com/geeksville/mesh/model/UIState.kt

@@ -230,7 +230,9 @@ constructor(
     val deviceHardware: StateFlow<DeviceHardware?> =
         ourNodeInfo
             .mapNotNull { nodeInfo ->
-                nodeInfo?.user?.hwModel?.let { deviceHardwareRepository.getDeviceHardwareByModel(it.number) }
+                nodeInfo?.user?.hwModel?.let {
+                    deviceHardwareRepository.getDeviceHardwareByModel(it.number).getOrNull()
+                }
             }
             .stateIn(scope = viewModelScope, started = SharingStarted.WhileSubscribed(5_000), initialValue = null)
 

app/src/main/java/com/geeksville/mesh/repository/api/DeviceHardwareRepository.kt

@@ -19,65 +19,93 @@ package com.geeksville.mesh.repository.api
 
 import com.geeksville.mesh.android.BuildUtils.debug
 import com.geeksville.mesh.android.BuildUtils.warn
+import com.geeksville.mesh.database.entity.DeviceHardwareEntity
 import com.geeksville.mesh.database.entity.asExternalModel
 import com.geeksville.mesh.model.DeviceHardware
 import com.geeksville.mesh.network.DeviceHardwareRemoteDataSource
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import java.io.IOException
+import java.util.concurrent.TimeUnit
 import javax.inject.Inject
+import javax.inject.Singleton
 
+// Annotating with Singleton to ensure a single instance manages the cache
+@Singleton
 class DeviceHardwareRepository
 @Inject
 constructor(
-    private val apiDataSource: DeviceHardwareRemoteDataSource,
+    private val remoteDataSource: DeviceHardwareRemoteDataSource,
     private val localDataSource: DeviceHardwareLocalDataSource,
     private val jsonDataSource: DeviceHardwareJsonDataSource,
 ) {
 
-    companion object {
-        // 1 day
-        private const val CACHE_EXPIRATION_TIME_MS = 24 * 60 * 60 * 1000L
-    }
-
-    suspend fun getDeviceHardwareByModel(hwModel: Int, refresh: Boolean = false): DeviceHardware? {
-        return withContext(Dispatchers.IO) {
-            if (refresh) {
-                invalidateCache()
+    /**
+     * Retrieves device hardware information by its model ID.
+     *
+     * This function implements a cache-aside pattern with a fallback mechanism:
+     * 1. Check for a valid, non-expired local cache entry.
+     * 2. If not found or expired, fetch fresh data from the remote API.
+     * 3. If the remote fetch fails, attempt to use stale data from the cache.
+     * 4. If the cache is empty, fall back to loading data from a bundled JSON asset.
+     *
+     * @param hwModel The hardware model identifier.
+     * @param forceRefresh If true, the local cache will be invalidated and data will be fetched remotely.
+     * @return A [Result] containing the [DeviceHardware] on success (or null if not found), or an exception on failure.
+     */
+    suspend fun getDeviceHardwareByModel(hwModel: Int, forceRefresh: Boolean = false): Result<DeviceHardware?> =
+        withContext(Dispatchers.IO) {
+            if (forceRefresh) {
+                localDataSource.deleteAllDeviceHardware()
             } else {
-                val cachedHardware = localDataSource.getByHwModel(hwModel)
-                if (cachedHardware != null && !isCacheExpired(cachedHardware.lastUpdated)) {
-                    val externalModel = cachedHardware.asExternalModel()
-                    return@withContext externalModel
+                // 1. Attempt to retrieve from cache first
+                val cachedEntity = localDataSource.getByHwModel(hwModel)
+                if (cachedEntity != null && !cachedEntity.isStale()) {
+                    debug("Using fresh cached device hardware for model $hwModel")
+                    return@withContext Result.success(cachedEntity.asExternalModel())
                 }
             }
-            try {
-                val deviceHardware =
-                    apiDataSource.getAllDeviceHardware() ?: throw IOException("empty response from server")
-                localDataSource.insertAllDeviceHardware(deviceHardware)
-                val cachedHardware = localDataSource.getByHwModel(hwModel)
-                val externalModel = cachedHardware?.asExternalModel()
-                return@withContext externalModel
-            } catch (e: IOException) {
-                warn("Failed to fetch device hardware from server: ${e.message}")
-                var cachedHardware = localDataSource.getByHwModel(hwModel)
-                if (cachedHardware != null) {
-                    debug("Using stale cached device hardware")
-                    return@withContext cachedHardware.asExternalModel()
-                }
-                localDataSource.insertAllDeviceHardware(jsonDataSource.loadDeviceHardwareFromJsonAsset())
-                cachedHardware = localDataSource.getByHwModel(hwModel)
-                val externalModel = cachedHardware?.asExternalModel()
-                return@withContext externalModel
+
+            // 2. Fetch from remote API
+            runCatching {
+                debug("Fetching device hardware from remote API.")
+                val remoteHardware =
+                    remoteDataSource.getAllDeviceHardware() ?: throw IOException("Empty response from server")
+
+                localDataSource.insertAllDeviceHardware(remoteHardware)
+                localDataSource.getByHwModel(hwModel)?.asExternalModel()
             }
+                .onSuccess {
+                    // Successfully fetched and found the model
+                    return@withContext Result.success(it)
+                }
+                .onFailure { e ->
+                    warn("Failed to fetch device hardware from server: ${e.message}")
+
+                    // 3. Attempt to use stale cache as a fallback
+                    val staleEntity = localDataSource.getByHwModel(hwModel)
+                    if (staleEntity != null) {
+                        debug("Using stale cached device hardware for model $hwModel")
+                        return@withContext Result.success(staleEntity.asExternalModel())
+                    }
+
+                    // 4. Fallback to bundled JSON if cache is empty
+                    debug("Cache is empty, falling back to bundled JSON asset.")
+                    return@withContext loadFromBundledJson(hwModel)
+                }
         }
+
+    private suspend fun loadFromBundledJson(hwModel: Int): Result<DeviceHardware?> = runCatching {
+        val jsonHardware = jsonDataSource.loadDeviceHardwareFromJsonAsset()
+        localDataSource.insertAllDeviceHardware(jsonHardware)
+        localDataSource.getByHwModel(hwModel)?.asExternalModel()
     }
 
-    suspend fun invalidateCache() {
-        localDataSource.deleteAllDeviceHardware()
-    }
+    /** Extension function to check if the cached entity is stale. */
+    private fun DeviceHardwareEntity.isStale(): Boolean =
+        (System.currentTimeMillis() - this.lastUpdated) > CACHE_EXPIRATION_TIME_MS
 
-    /** Check if the cache is expired */
-    private fun isCacheExpired(lastUpdated: Long): Boolean =
-        System.currentTimeMillis() - lastUpdated > CACHE_EXPIRATION_TIME_MS
+    companion object {
+        private val CACHE_EXPIRATION_TIME_MS = TimeUnit.DAYS.toMillis(1)
+    }
 }

app/src/main/java/com/geeksville/mesh/repository/api/FirmwareReleaseRepository.kt

@@ -17,77 +17,118 @@
 
 package com.geeksville.mesh.repository.api
 
+import com.geeksville.mesh.android.BuildUtils.debug
 import com.geeksville.mesh.android.BuildUtils.warn
 import com.geeksville.mesh.database.entity.FirmwareRelease
+import com.geeksville.mesh.database.entity.FirmwareReleaseEntity
 import com.geeksville.mesh.database.entity.FirmwareReleaseType
 import com.geeksville.mesh.database.entity.asExternalModel
 import com.geeksville.mesh.network.FirmwareReleaseRemoteDataSource
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.flow
 import java.io.IOException
+import java.util.concurrent.TimeUnit
 import javax.inject.Inject
+import javax.inject.Singleton
 
+@Singleton
 class FirmwareReleaseRepository
 @Inject
 constructor(
-    private val apiDataSource: FirmwareReleaseRemoteDataSource,
+    private val remoteDataSource: FirmwareReleaseRemoteDataSource,
     private val localDataSource: FirmwareReleaseLocalDataSource,
     private val jsonDataSource: FirmwareReleaseJsonDataSource,
 ) {
 
-    companion object {
-        // 1 hour
-        private const val CACHE_EXPIRATION_TIME_MS = 60 * 60 * 1000L
-    }
-
+    /**
+     * A flow that provides the latest STABLE firmware release. It follows a "cache-then-network" strategy:
+     * 1. Immediately emits the cached version (if any).
+     * 2. If the cached version is stale, triggers a network fetch in the background.
+     * 3. Emits the updated version upon successful fetch. Collectors should use `.distinctUntilChanged()` to avoid
+     *    redundant UI updates.
+     */
     val stableRelease: Flow<FirmwareRelease?> = getLatestFirmware(FirmwareReleaseType.STABLE)
 
+    /**
+     * A flow that provides the latest ALPHA firmware release.
+     *
+     * @see stableRelease for behavior details.
+     */
     val alphaRelease: Flow<FirmwareRelease?> = getLatestFirmware(FirmwareReleaseType.ALPHA)
 
-    private fun getLatestFirmware(releaseType: FirmwareReleaseType, refresh: Boolean = false): Flow<FirmwareRelease?> =
-        flow {
-            if (refresh) {
-                invalidateCache()
-            } else {
-                val cachedRelease = localDataSource.getLatestRelease(releaseType)
-                if (cachedRelease != null && !isCacheExpired(cachedRelease.lastUpdated)) {
-                    val externalModel = cachedRelease.asExternalModel()
-                    emit(externalModel)
-                    return@flow
-                }
-            }
-            try {
-                val networkFirmwareReleases =
-                    apiDataSource.getFirmwareReleases() ?: throw IOException("empty response from server")
-                val releases =
-                    when (releaseType) {
-                        FirmwareReleaseType.STABLE -> networkFirmwareReleases.releases.stable
-                        FirmwareReleaseType.ALPHA -> networkFirmwareReleases.releases.alpha
-                    }
-                localDataSource.insertFirmwareReleases(releases, releaseType)
-                val cachedRelease = localDataSource.getLatestRelease(releaseType)
-                val externalModel = cachedRelease?.asExternalModel()
-                emit(externalModel)
-            } catch (e: IOException) {
-                warn("Failed to fetch firmware releases from server: ${e.message}")
-                val jsonFirmwareReleases = jsonDataSource.loadFirmwareReleaseFromJsonAsset()
-                val releases =
-                    when (releaseType) {
-                        FirmwareReleaseType.STABLE -> jsonFirmwareReleases.releases.stable
-                        FirmwareReleaseType.ALPHA -> jsonFirmwareReleases.releases.alpha
-                    }
-                localDataSource.insertFirmwareReleases(releases, releaseType)
-                val cachedRelease = localDataSource.getLatestRelease(releaseType)
-                val externalModel = cachedRelease?.asExternalModel()
-                emit(externalModel)
-            }
+    private fun getLatestFirmware(
+        releaseType: FirmwareReleaseType,
+        forceRefresh: Boolean = false,
+    ): Flow<FirmwareRelease?> = flow {
+        if (forceRefresh) {
+            invalidateCache()
         }
 
+        // 1. Emit cached data first, regardless of staleness.
+        // This gives the UI something to show immediately.
+        val cachedRelease = localDataSource.getLatestRelease(releaseType)
+        cachedRelease?.let {
+            debug("Emitting cached firmware for $releaseType (isStale=${it.isStale()})")
+            emit(it.asExternalModel())
+        }
+
+        // 2. If the cache was fresh and we are not forcing a refresh, we're done.
+        if (cachedRelease != null && !cachedRelease.isStale() && !forceRefresh) {
+            return@flow
+        }
+
+        // 3. Cache is stale, empty, or refresh is forced. Fetch new data.
+        updateCacheFromSources()
+
+        // 4. Emit the final, updated value from the cache.
+        // The `distinctUntilChanged()` operator on the collector side will prevent
+        // re-emitting the same data if the cache wasn't actually updated.
+        val finalRelease = localDataSource.getLatestRelease(releaseType)
+        debug("Emitting final firmware for $releaseType from cache.")
+        emit(finalRelease?.asExternalModel())
+    }
+
+    /**
+     * Updates the local cache by fetching from the remote API, with a fallback to a bundled JSON asset if the remote
+     * fetch fails.
+     *
+     * This method is efficient because it fetches and caches all release types (stable, alpha, etc.) in a single
+     * operation.
+     */
+    private suspend fun updateCacheFromSources() {
+        val remoteFetchSuccess =
+            runCatching {
+                debug("Fetching fresh firmware releases from remote API.")
+                val networkReleases =
+                    remoteDataSource.getFirmwareReleases() ?: throw IOException("Empty response from server")
+
+                // The API fetches all release types, so we cache them all at once.
+                localDataSource.insertFirmwareReleases(networkReleases.releases.stable, FirmwareReleaseType.STABLE)
+                localDataSource.insertFirmwareReleases(networkReleases.releases.alpha, FirmwareReleaseType.ALPHA)
+            }
+                .isSuccess
+
+        // If remote fetch failed, try the JSON fallback as a last resort.
+        if (!remoteFetchSuccess) {
+            warn("Remote fetch failed, attempting to cache from bundled JSON.")
+            runCatching {
+                val jsonReleases = jsonDataSource.loadFirmwareReleaseFromJsonAsset()
+                localDataSource.insertFirmwareReleases(jsonReleases.releases.stable, FirmwareReleaseType.STABLE)
+                localDataSource.insertFirmwareReleases(jsonReleases.releases.alpha, FirmwareReleaseType.ALPHA)
+            }
+                .onFailure { warn("Failed to cache from JSON: ${it.message}") }
+        }
+    }
+
     suspend fun invalidateCache() {
         localDataSource.deleteAllFirmwareReleases()
     }
 
-    /** Check if the cache is expired */
-    private fun isCacheExpired(lastUpdated: Long): Boolean =
-        System.currentTimeMillis() - lastUpdated > CACHE_EXPIRATION_TIME_MS
+    /** Extension function to check if the cached entity is stale. */
+    private fun FirmwareReleaseEntity.isStale(): Boolean =
+        (System.currentTimeMillis() - this.lastUpdated) > CACHE_EXPIRATION_TIME_MS
+
+    companion object {
+        private val CACHE_EXPIRATION_TIME_MS = TimeUnit.HOURS.toMillis(1)
+    }
 }