From 59179a5a17f9a09e558ae08f3891f19b21386b1b Mon Sep 17 00:00:00 2001 From: Ankush Date: Mon, 22 Jun 2026 00:56:01 +0530 Subject: [PATCH] Fix: Duplicate weather API call during startup and refresh --- .../data/repository/WeatherRepositoryImpl.kt | 32 ++++++++++++------- .../ankush/network/model/LocationModels.kt | 1 + .../network/repository/WeatherRepository.kt | 9 ++++-- .../repository/WeatherRepositoryImpl.kt | 31 ++++++++++++------ .../ankush/storage/impl/WeatherStorageImpl.kt | 9 ++++-- 5 files changed, 56 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/bose/ankush/weatherify/data/repository/WeatherRepositoryImpl.kt b/app/src/main/java/bose/ankush/weatherify/data/repository/WeatherRepositoryImpl.kt index d1490ef6..937cb541 100644 --- a/app/src/main/java/bose/ankush/weatherify/data/repository/WeatherRepositoryImpl.kt +++ b/app/src/main/java/bose/ankush/weatherify/data/repository/WeatherRepositoryImpl.kt @@ -46,18 +46,26 @@ constructor( val isDataStale = forceRefresh || (currentTime - lastUpdateTime) > ONE_HOUR_IN_MILLIS if (isDataStale) { - val weatherData = networkRepository.refreshWeatherData(coordinates) - - if (weatherData != null) { - val weatherStorageData = - NetworkToStorageMapper.mapWeatherToStorageEntity(weatherData) - val airQualityStorageData = - NetworkToStorageMapper.mapAirQualityToStorageEntity( - weatherData.data?.airQuality, - ) - weatherStorage.saveWeatherData(weatherStorageData, airQualityStorageData) - weatherStorage.saveLastWeatherUpdateTime(coordinates, currentTime) - } + networkRepository.refreshWeatherData(coordinates).fold( + onSuccess = { data -> + data?.let { + val weatherStorageData = + NetworkToStorageMapper.mapWeatherToStorageEntity(it) + val airQualityStorageData = + NetworkToStorageMapper.mapAirQualityToStorageEntity( + it.data?.airQuality, + ) + weatherStorage.saveWeatherData( + weatherStorageData, + airQualityStorageData + ) + weatherStorage.saveLastWeatherUpdateTime(coordinates, currentTime) + } + }, + onFailure = { + // Log error or pass it to UI + } + ) } } } diff --git a/network/src/commonMain/kotlin/bose/ankush/network/model/LocationModels.kt b/network/src/commonMain/kotlin/bose/ankush/network/model/LocationModels.kt index 44632d1a..4d0ef681 100644 --- a/network/src/commonMain/kotlin/bose/ankush/network/model/LocationModels.kt +++ b/network/src/commonMain/kotlin/bose/ankush/network/model/LocationModels.kt @@ -38,6 +38,7 @@ private object ObjectIdAsStringSerializer : KSerializer { */ @Serializable data class SavedLocation( + @SerialName("_id") @Serializable(with = ObjectIdAsStringSerializer::class) val id: String = "", val userEmail: String = "", diff --git a/network/src/commonMain/kotlin/bose/ankush/network/repository/WeatherRepository.kt b/network/src/commonMain/kotlin/bose/ankush/network/repository/WeatherRepository.kt index b2784a25..2f1a11b5 100644 --- a/network/src/commonMain/kotlin/bose/ankush/network/repository/WeatherRepository.kt +++ b/network/src/commonMain/kotlin/bose/ankush/network/repository/WeatherRepository.kt @@ -3,6 +3,11 @@ package bose.ankush.network.repository import bose.ankush.network.model.WeatherForecast interface WeatherRepository { - /** Fetches fresh weather data from the network. Returns null if offline. */ - suspend fun refreshWeatherData(coordinates: Pair): WeatherForecast? + /** + * Fetches fresh weather data from the network. + * + * Returns [Result.failure] with a [bose.ankush.network.common.NetworkException] when offline + * or the request fails, so callers can surface the actual cause instead of a silent null. + */ + suspend fun refreshWeatherData(coordinates: Pair): Result } diff --git a/network/src/commonMain/kotlin/bose/ankush/network/repository/WeatherRepositoryImpl.kt b/network/src/commonMain/kotlin/bose/ankush/network/repository/WeatherRepositoryImpl.kt index 659a08aa..d636d77c 100644 --- a/network/src/commonMain/kotlin/bose/ankush/network/repository/WeatherRepositoryImpl.kt +++ b/network/src/commonMain/kotlin/bose/ankush/network/repository/WeatherRepositoryImpl.kt @@ -2,6 +2,7 @@ package bose.ankush.network.repository import bose.ankush.network.api.WeatherApiService import bose.ankush.network.common.NetworkConnectivity +import bose.ankush.network.common.NetworkException import bose.ankush.network.model.WeatherForecast import bose.ankush.network.utils.NetworkUtils @@ -9,18 +10,30 @@ class WeatherRepositoryImpl( private val apiService: WeatherApiService, private val networkConnectivity: NetworkConnectivity, ) : WeatherRepository { - override suspend fun refreshWeatherData(coordinates: Pair): WeatherForecast? { - if (!networkConnectivity.isNetworkAvailable()) return null + override suspend fun refreshWeatherData( + coordinates: Pair, + ): Result { + if (!networkConnectivity.isNetworkAvailable()) { + return Result.failure( + NetworkException( + NetworkException.NETWORK_UNAVAILABLE, + "No internet connection available", + ), + ) + } return try { - NetworkUtils.retryWithExponentialBackoff { - apiService.getOneCallWeather( - coordinates.first.toString(), - coordinates.second.toString(), - ) - } + Result.success( + NetworkUtils.retryWithExponentialBackoff { + apiService.getOneCallWeather( + coordinates.first.toString(), + coordinates.second.toString(), + ) + }, + ) } catch (e: Exception) { - throw Exception("Network | Failed to refresh weather data: ${e.message}", e) + // retryWithExponentialBackoff already normalizes failures to NetworkException. + Result.failure(e) } } } diff --git a/storage/src/androidMain/kotlin/bose/ankush/storage/impl/WeatherStorageImpl.kt b/storage/src/androidMain/kotlin/bose/ankush/storage/impl/WeatherStorageImpl.kt index 576ceffc..21861cfb 100644 --- a/storage/src/androidMain/kotlin/bose/ankush/storage/impl/WeatherStorageImpl.kt +++ b/storage/src/androidMain/kotlin/bose/ankush/storage/impl/WeatherStorageImpl.kt @@ -11,6 +11,8 @@ import bose.ankush.storage.room.WeatherEntity import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext class WeatherStorageImpl( @@ -19,6 +21,7 @@ class WeatherStorageImpl( // In-memory per-location timestamp map. Keyed by "lat_lon" string. // Reset on process restart intentionally — fresh data should be fetched after a cold start. private val locationTimestamps = mutableMapOf() + private val timestampsMutex = Mutex() private fun locationKey(coordinates: Pair) = "${coordinates.first}_${coordinates.second}" @@ -30,13 +33,13 @@ class WeatherStorageImpl( weatherDatabase.weatherDao().getAirQuality().map { it?.toAirQualityData() } override suspend fun getLastWeatherUpdateTime(coordinates: Pair): Long = - locationTimestamps[locationKey(coordinates)] ?: 0L + timestampsMutex.withLock { locationTimestamps[locationKey(coordinates)] ?: 0L } override suspend fun saveLastWeatherUpdateTime( coordinates: Pair, time: Long, ) { - locationTimestamps[locationKey(coordinates)] = time + timestampsMutex.withLock { locationTimestamps[locationKey(coordinates)] = time } } override suspend fun saveWeatherData( @@ -55,7 +58,7 @@ class WeatherStorageImpl( withContext(Dispatchers.IO) { weatherDatabase.weatherDao().clearAll() } - locationTimestamps.clear() + timestampsMutex.withLock { locationTimestamps.clear() } } private fun List?.toWeatherConditions() =