mirror of
https://github.com/vide/matedroid.git
synced 2026-01-20 00:03:17 +08:00
fix: resolve race condition in dashboard counts display
The DashboardViewModel was using non-atomic state updates
(_uiState.value = _uiState.value.copy(...)) which caused a race
condition when two coroutines tried to update state concurrently.
When loading charges and drives counts in parallel:
1. Both coroutines read the state snapshot
2. Both do async work (API calls)
3. Each writes back with .copy() - second write overwrites first
With large datasets, API calls take longer, making this race window
much larger - explaining why users with many drives/charges saw
missing totals on the dashboard.
Fixed by using MutableStateFlow.update{} throughout the ViewModel,
which provides atomic state updates. Also updated tests to match
the current ViewModel API (CarStatusWithUnits, GeocodingRepository).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- **Software Versions**: Tap the external link icon next to any version to view release notes on NotATeslaApp
|
||||
|
||||
### Fixed
|
||||
- **Dashboard**: Fix race condition where drive/charge counts could fail to display for users with large datasets
|
||||
- **Software Versions**: Show all software updates instead of only the first 100
|
||||
|
||||
## [0.6.1] - 2025-12-22
|
||||
|
||||
@@ -15,6 +15,7 @@ import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.coroutines.isActive
|
||||
import kotlinx.coroutines.launch
|
||||
import javax.inject.Inject
|
||||
@@ -75,24 +76,28 @@ class DashboardViewModel @Inject constructor(
|
||||
|
||||
fun loadCars() {
|
||||
viewModelScope.launch {
|
||||
_uiState.value = _uiState.value.copy(isLoading = true, error = null)
|
||||
_uiState.update { it.copy(isLoading = true, error = null) }
|
||||
|
||||
when (val result = repository.getCars()) {
|
||||
is ApiResult.Success -> {
|
||||
val cars = result.data
|
||||
val selectedCarId = cars.firstOrNull()?.carId
|
||||
_uiState.value = _uiState.value.copy(
|
||||
isLoading = false,
|
||||
cars = cars,
|
||||
selectedCarId = selectedCarId
|
||||
)
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
isLoading = false,
|
||||
cars = cars,
|
||||
selectedCarId = selectedCarId
|
||||
)
|
||||
}
|
||||
selectedCarId?.let { loadCarStatus(it) }
|
||||
}
|
||||
is ApiResult.Error -> {
|
||||
_uiState.value = _uiState.value.copy(
|
||||
isLoading = false,
|
||||
error = result.message
|
||||
)
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
isLoading = false,
|
||||
error = result.message
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -101,40 +106,42 @@ class DashboardViewModel @Inject constructor(
|
||||
fun selectCar(carId: Int) {
|
||||
// Reset state when switching cars
|
||||
lastGeocodedLocation = null
|
||||
_uiState.value = _uiState.value.copy(
|
||||
selectedCarId = carId,
|
||||
carStatus = null,
|
||||
resolvedAddress = null,
|
||||
totalCharges = null,
|
||||
totalDrives = null
|
||||
)
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
selectedCarId = carId,
|
||||
carStatus = null,
|
||||
resolvedAddress = null,
|
||||
totalCharges = null,
|
||||
totalDrives = null
|
||||
)
|
||||
}
|
||||
loadCarStatus(carId)
|
||||
}
|
||||
|
||||
fun refresh() {
|
||||
val carId = _uiState.value.selectedCarId ?: return
|
||||
viewModelScope.launch {
|
||||
_uiState.value = _uiState.value.copy(isRefreshing = true, error = null)
|
||||
_uiState.update { it.copy(isRefreshing = true, error = null) }
|
||||
|
||||
// Fetch car status directly (not via loadCarStatus which launches separate coroutine)
|
||||
when (val result = repository.getCarStatus(carId)) {
|
||||
is ApiResult.Success -> {
|
||||
val status = result.data.status
|
||||
_uiState.value = _uiState.value.copy(
|
||||
carStatus = status,
|
||||
units = result.data.units,
|
||||
error = null
|
||||
)
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
carStatus = status,
|
||||
units = result.data.units,
|
||||
error = null
|
||||
)
|
||||
}
|
||||
fetchAddressIfNeeded(status)
|
||||
}
|
||||
is ApiResult.Error -> {
|
||||
_uiState.value = _uiState.value.copy(
|
||||
error = result.message
|
||||
)
|
||||
_uiState.update { it.copy(error = result.message) }
|
||||
}
|
||||
}
|
||||
|
||||
_uiState.value = _uiState.value.copy(isRefreshing = false)
|
||||
_uiState.update { it.copy(isRefreshing = false) }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -143,18 +150,18 @@ class DashboardViewModel @Inject constructor(
|
||||
when (val result = repository.getCarStatus(carId)) {
|
||||
is ApiResult.Success -> {
|
||||
val status = result.data.status
|
||||
_uiState.value = _uiState.value.copy(
|
||||
carStatus = status,
|
||||
units = result.data.units,
|
||||
error = null
|
||||
)
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
carStatus = status,
|
||||
units = result.data.units,
|
||||
error = null
|
||||
)
|
||||
}
|
||||
// Fetch address if no geofence but coordinates are available
|
||||
fetchAddressIfNeeded(status)
|
||||
}
|
||||
is ApiResult.Error -> {
|
||||
_uiState.value = _uiState.value.copy(
|
||||
error = result.message
|
||||
)
|
||||
_uiState.update { it.copy(error = result.message) }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -167,7 +174,7 @@ class DashboardViewModel @Inject constructor(
|
||||
// Load charges count
|
||||
when (val result = repository.getCharges(carId, null, null)) {
|
||||
is ApiResult.Success -> {
|
||||
_uiState.value = _uiState.value.copy(totalCharges = result.data.size)
|
||||
_uiState.update { it.copy(totalCharges = result.data.size) }
|
||||
}
|
||||
is ApiResult.Error -> { /* ignore */ }
|
||||
}
|
||||
@@ -176,7 +183,7 @@ class DashboardViewModel @Inject constructor(
|
||||
// Load drives count
|
||||
when (val result = repository.getDrives(carId, null, null)) {
|
||||
is ApiResult.Success -> {
|
||||
_uiState.value = _uiState.value.copy(totalDrives = result.data.size)
|
||||
_uiState.update { it.copy(totalDrives = result.data.size) }
|
||||
}
|
||||
is ApiResult.Error -> { /* ignore */ }
|
||||
}
|
||||
@@ -202,12 +209,12 @@ class DashboardViewModel @Inject constructor(
|
||||
viewModelScope.launch {
|
||||
val address = geocodingRepository.reverseGeocode(lat, lon)
|
||||
if (address != null) {
|
||||
_uiState.value = _uiState.value.copy(resolvedAddress = address)
|
||||
_uiState.update { it.copy(resolvedAddress = address) }
|
||||
}
|
||||
}
|
||||
} else if (hasGeofence) {
|
||||
// Clear resolved address if geofence is available
|
||||
_uiState.value = _uiState.value.copy(resolvedAddress = null)
|
||||
_uiState.update { it.copy(resolvedAddress = null) }
|
||||
lastGeocodedLocation = null
|
||||
}
|
||||
}
|
||||
@@ -220,10 +227,12 @@ class DashboardViewModel @Inject constructor(
|
||||
when (val result = repository.getCarStatus(carId)) {
|
||||
is ApiResult.Success -> {
|
||||
val status = result.data.status
|
||||
_uiState.value = _uiState.value.copy(
|
||||
carStatus = status,
|
||||
units = result.data.units
|
||||
)
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
carStatus = status,
|
||||
units = result.data.units
|
||||
)
|
||||
}
|
||||
// Update address if location changed
|
||||
fetchAddressIfNeeded(status)
|
||||
}
|
||||
@@ -236,6 +245,6 @@ class DashboardViewModel @Inject constructor(
|
||||
}
|
||||
|
||||
fun clearError() {
|
||||
_uiState.value = _uiState.value.copy(error = null)
|
||||
_uiState.update { it.copy(error = null) }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,7 +5,10 @@ import com.matedroid.data.api.models.CarData
|
||||
import com.matedroid.data.api.models.CarStatus
|
||||
import com.matedroid.data.api.models.CarStatusDetails
|
||||
import com.matedroid.data.api.models.ChargingDetails
|
||||
import com.matedroid.data.api.models.Units
|
||||
import com.matedroid.data.repository.ApiResult
|
||||
import com.matedroid.data.repository.CarStatusWithUnits
|
||||
import com.matedroid.data.repository.GeocodingRepository
|
||||
import com.matedroid.data.repository.TeslamateRepository
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
@@ -29,6 +32,7 @@ class DashboardViewModelTest {
|
||||
|
||||
private val testDispatcher = StandardTestDispatcher()
|
||||
private lateinit var repository: TeslamateRepository
|
||||
private lateinit var geocodingRepository: GeocodingRepository
|
||||
private lateinit var viewModel: DashboardViewModel
|
||||
|
||||
private val testCar = CarData(
|
||||
@@ -50,10 +54,16 @@ class DashboardViewModelTest {
|
||||
carStatus = CarStatusDetails(locked = true)
|
||||
)
|
||||
|
||||
private val testStatusWithUnits = CarStatusWithUnits(
|
||||
status = testStatus,
|
||||
units = Units()
|
||||
)
|
||||
|
||||
@Before
|
||||
fun setup() {
|
||||
Dispatchers.setMain(testDispatcher)
|
||||
repository = mockk()
|
||||
geocodingRepository = mockk()
|
||||
}
|
||||
|
||||
@After
|
||||
@@ -62,13 +72,15 @@ class DashboardViewModelTest {
|
||||
}
|
||||
|
||||
private fun createViewModel(): DashboardViewModel {
|
||||
return DashboardViewModel(repository)
|
||||
return DashboardViewModel(repository, geocodingRepository)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `loadCars fetches cars and selects first one`() = runTest {
|
||||
coEvery { repository.getCars() } returns ApiResult.Success(listOf(testCar))
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Success(testStatus)
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Success(testStatusWithUnits)
|
||||
coEvery { repository.getCharges(1, null, null) } returns ApiResult.Success(emptyList())
|
||||
coEvery { repository.getDrives(1, null, null) } returns ApiResult.Success(emptyList())
|
||||
|
||||
viewModel = createViewModel()
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
@@ -108,10 +120,13 @@ class DashboardViewModelTest {
|
||||
val car1 = CarData(carId = 1, name = "Car 1")
|
||||
val car2 = CarData(carId = 2, name = "Car 2")
|
||||
val status2 = testStatus.copy(displayName = "Car 2")
|
||||
val status2WithUnits = CarStatusWithUnits(status = status2, units = Units())
|
||||
|
||||
coEvery { repository.getCars() } returns ApiResult.Success(listOf(car1, car2))
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Success(testStatus)
|
||||
coEvery { repository.getCarStatus(2) } returns ApiResult.Success(status2)
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Success(testStatusWithUnits)
|
||||
coEvery { repository.getCarStatus(2) } returns ApiResult.Success(status2WithUnits)
|
||||
coEvery { repository.getCharges(any(), null, null) } returns ApiResult.Success(emptyList())
|
||||
coEvery { repository.getDrives(any(), null, null) } returns ApiResult.Success(emptyList())
|
||||
|
||||
viewModel = createViewModel()
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
@@ -128,7 +143,9 @@ class DashboardViewModelTest {
|
||||
@Test
|
||||
fun `refresh reloads car status`() = runTest {
|
||||
coEvery { repository.getCars() } returns ApiResult.Success(listOf(testCar))
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Success(testStatus)
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Success(testStatusWithUnits)
|
||||
coEvery { repository.getCharges(1, null, null) } returns ApiResult.Success(emptyList())
|
||||
coEvery { repository.getDrives(1, null, null) } returns ApiResult.Success(emptyList())
|
||||
|
||||
viewModel = createViewModel()
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
@@ -136,7 +153,8 @@ class DashboardViewModelTest {
|
||||
val updatedStatus = testStatus.copy(
|
||||
batteryDetails = BatteryDetails(batteryLevel = 80, ratedBatteryRange = 300.0)
|
||||
)
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Success(updatedStatus)
|
||||
val updatedStatusWithUnits = CarStatusWithUnits(status = updatedStatus, units = Units())
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Success(updatedStatusWithUnits)
|
||||
|
||||
viewModel.refresh()
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
@@ -178,6 +196,8 @@ class DashboardViewModelTest {
|
||||
fun `status error is shown when status fetch fails`() = runTest {
|
||||
coEvery { repository.getCars() } returns ApiResult.Success(listOf(testCar))
|
||||
coEvery { repository.getCarStatus(1) } returns ApiResult.Error("Status error")
|
||||
coEvery { repository.getCharges(1, null, null) } returns ApiResult.Success(emptyList())
|
||||
coEvery { repository.getDrives(1, null, null) } returns ApiResult.Success(emptyList())
|
||||
|
||||
viewModel = createViewModel()
|
||||
testDispatcher.scheduler.advanceUntilIdle()
|
||||
|
||||
Reference in New Issue
Block a user