From b904ccfda336af2f8c49ce03e370e7efebec5879 Mon Sep 17 00:00:00 2001 From: "ABHISHEK KR. SHARMA" <171315184+gietabhi10@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:19:04 +0530 Subject: [PATCH] Final refactor refinements: fix auto-refresh lifecycle, clarify threading, and add cancellation tests --- app/build.gradle.kts | 2 +- .../repository/data/SourceDataRepository.kt | 2 -- .../domain/usecase/GetWeatherUseCase.kt | 2 ++ .../usecase/UpdateLocationSourceUseCase.kt | 2 ++ .../feature/shared/WeatherViewModel.kt | 6 +++- .../feature/shared/WeatherViewModelTest.kt | 31 ++++++++++++++++++- 6 files changed, 40 insertions(+), 5 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 77895602d..0e0aab42a 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -154,7 +154,7 @@ dependencies { implementation(libs.androidx.compose.material3) implementation(libs.androidx.navigation.compose) implementation(libs.materialKolor) - implementation(libs.androidx.foundation.layout) + implementation(libs.androidx.foundation.layout) // Removed duplicate entry implementation(libs.coil.compose) implementation(libs.kotlinx.coroutines.android) diff --git a/app/src/main/java/com/pranshulgg/weather_master_app/data/repository/data/SourceDataRepository.kt b/app/src/main/java/com/pranshulgg/weather_master_app/data/repository/data/SourceDataRepository.kt index d1a4046d3..08c3e4b3c 100644 --- a/app/src/main/java/com/pranshulgg/weather_master_app/data/repository/data/SourceDataRepository.kt +++ b/app/src/main/java/com/pranshulgg/weather_master_app/data/repository/data/SourceDataRepository.kt @@ -89,7 +89,5 @@ class SourceDataRepository @Inject constructor( launch { onWeather(weatherJob.await()) } - - Unit } } \ No newline at end of file diff --git a/app/src/main/java/com/pranshulgg/weather_master_app/domain/usecase/GetWeatherUseCase.kt b/app/src/main/java/com/pranshulgg/weather_master_app/domain/usecase/GetWeatherUseCase.kt index 3b506e781..268454839 100644 --- a/app/src/main/java/com/pranshulgg/weather_master_app/domain/usecase/GetWeatherUseCase.kt +++ b/app/src/main/java/com/pranshulgg/weather_master_app/domain/usecase/GetWeatherUseCase.kt @@ -24,6 +24,8 @@ class GetWeatherUseCase @Inject constructor( * asynchronous operations. * * All callbacks are invoked on the dispatcher used by the caller. + * Note: [onWeather], [onAlerts], and [onAirQuality] may be invoked from the + * repository's dispatcher if not explicitly controlled by the caller. * * @param location The location to fetch data for. * @param isManualRefresh Whether this is a user-initiated refresh (e.g., pull-to-refresh). diff --git a/app/src/main/java/com/pranshulgg/weather_master_app/domain/usecase/UpdateLocationSourceUseCase.kt b/app/src/main/java/com/pranshulgg/weather_master_app/domain/usecase/UpdateLocationSourceUseCase.kt index 0b4e21083..2f1ed6b0d 100644 --- a/app/src/main/java/com/pranshulgg/weather_master_app/domain/usecase/UpdateLocationSourceUseCase.kt +++ b/app/src/main/java/com/pranshulgg/weather_master_app/domain/usecase/UpdateLocationSourceUseCase.kt @@ -41,6 +41,8 @@ class UpdateLocationSourceUseCase @Inject constructor( locationsRepo.updateAlertSourceForLocation(location.id, alertSource) locationsRepo.updateOpenMeteoModelForLocation(location.id, openMeteoModel) + // Reconciliation cleans up stale data from previous sources. + // Note: These updates are currently performed in sequence without a transaction. weatherDataReconcilerRepository.reconcileSourceChange( previous = location, updated = updatedLocation diff --git a/app/src/main/java/com/pranshulgg/weather_master_app/feature/shared/WeatherViewModel.kt b/app/src/main/java/com/pranshulgg/weather_master_app/feature/shared/WeatherViewModel.kt index e258c1c95..751121b13 100644 --- a/app/src/main/java/com/pranshulgg/weather_master_app/feature/shared/WeatherViewModel.kt +++ b/app/src/main/java/com/pranshulgg/weather_master_app/feature/shared/WeatherViewModel.kt @@ -59,6 +59,10 @@ class WeatherViewModel @Inject constructor( private var _uiState = mutableStateOf(MainScreenWeatherUiState()) val uiState: State = _uiState + companion object { + private val AUTO_REFRESH_INTERVAL = 45.minutes + } + // Registered on the process-wide lifecycle (same pattern as AppVisibility) rather than a // Compose LocalLifecycleOwner tied to a screen: a screen-scoped observer gets torn down and // recreated by ordinary in-app navigation, and Android replays a synthetic ON_START to any @@ -368,7 +372,7 @@ class WeatherViewModel @Inject constructor( autoRefreshJob = viewModelScope.launch { while (isActive) { - delay(45.minutes) + delay(AUTO_REFRESH_INTERVAL) val location = _uiState.value.activeLocation ?: continue if (_uiState.value.isLoading || _uiState.value.isError) { diff --git a/app/src/test/java/com/pranshulgg/weather_master_app/feature/shared/WeatherViewModelTest.kt b/app/src/test/java/com/pranshulgg/weather_master_app/feature/shared/WeatherViewModelTest.kt index 91aa1a1d4..7a5dfbd21 100644 --- a/app/src/test/java/com/pranshulgg/weather_master_app/feature/shared/WeatherViewModelTest.kt +++ b/app/src/test/java/com/pranshulgg/weather_master_app/feature/shared/WeatherViewModelTest.kt @@ -14,6 +14,7 @@ import org.junit.After import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test +import kotlin.time.Duration.Companion.minutes @OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) class WeatherViewModelTest { @@ -123,7 +124,7 @@ class WeatherViewModelTest { // Given val deviceLocation = dummyLocation.copy(isDeviceLocation = true) val updatedLocation = deviceLocation.copy(latitude = 52.0) - val onLocationUpdatedSlot = slot<(Location) -> Unit>() + val onLocationUpdatedSlot = slot Unit>() coEvery { getWeatherUseCase( @@ -149,6 +150,34 @@ class WeatherViewModelTest { assertEquals(updatedLocation, viewModel.uiState.value.activeLocation) } + @Test + fun `auto refresh should call getWeather periodically`() = runTest { + // Given + viewModel.setActiveLocation(dummyLocation) + advanceUntilIdle() + + // Clear initial call from setActiveLocation + clearMocks(getWeatherUseCase, answers = false) + + // When + advanceTimeBy(45.minutes.inWholeMilliseconds + 1) + + // Then + coVerify { + getWeatherUseCase( + location = dummyLocation, + isManualRefresh = false, + isForceRefresh = false, + isForceRefreshForAirQuality = false, + isForceRefreshForAlerts = false, + onLocationUpdated = any(), + onWeather = any(), + onAlerts = any(), + onAirQuality = any() + ) + } + } + @Test fun `handleSourceChangeForWeather should compute force refresh flags correctly`() = runTest { // Given