diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlanStateRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlanStateRepository.kt new file mode 100644 index 0000000000000000000000000000000000000000..a57c2fe27763294e5d74cbe4710b0b13d1d1717a --- /dev/null +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlanStateRepository.kt @@ -0,0 +1,57 @@ +/* + * Cardinal Maps + * Copyright (C) 2025 Cardinal Maps Authors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package earth.maps.cardinal.data + +import androidx.annotation.VisibleForTesting +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import earth.maps.cardinal.transit.PlanResponse + +data class TransitPlanState( + val planResponse: PlanResponse? = null, + val isLoading: Boolean = false, + val error: String? = null +) + +class PlanStateRepository { + private val _planState = MutableStateFlow(TransitPlanState()) + val planState: StateFlow = _planState.asStateFlow() + + fun setLoading(isLoading: Boolean) { + _planState.value = _planState.value.copy(isLoading = isLoading) + } + + fun setPlanResponse(planResponse: PlanResponse?) { + _planState.value = _planState.value.copy(planResponse = planResponse, isLoading = false, error = null) + } + + fun setError(error: String?) { + _planState.value = _planState.value.copy(isLoading = false, error = error) + } + + fun clear() { + _planState.value = TransitPlanState() + } + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal fun setStateForTest(state: TransitPlanState) { + _planState.value = state + } +} diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/RouteStateRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/RouteStateRepository.kt new file mode 100644 index 0000000000000000000000000000000000000000..78572bdc1ef19a23f4b1daa0fa6a9c2529a19cc3 --- /dev/null +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/RouteStateRepository.kt @@ -0,0 +1,57 @@ +/* + * Cardinal Maps + * Copyright (C) 2025 Cardinal Maps Authors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package earth.maps.cardinal.data + +import androidx.annotation.VisibleForTesting +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import uniffi.ferrostar.Route + +data class RouteState( + val route: Route? = null, + val isLoading: Boolean = false, + val error: String? = null +) + +class RouteStateRepository { + private val _routeState = MutableStateFlow(RouteState()) + val routeState: StateFlow = _routeState.asStateFlow() + + fun setLoading(isLoading: Boolean) { + _routeState.value = _routeState.value.copy(isLoading = isLoading) + } + + fun setRoute(route: Route?) { + _routeState.value = _routeState.value.copy(route = route, isLoading = false, error = null) + } + + fun setError(error: String?) { + _routeState.value = _routeState.value.copy(isLoading = false, error = error) + } + + fun clear() { + _routeState.value = RouteState() + } + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal fun setStateForTest(state: RouteState) { + _routeState.value = state + } +} diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/di/GeocodingModule.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/di/GeocodingModule.kt index bf2d2c36bbaba8334a52da8ed23bfd9a7c01ae5e..89c7ea7ac004ed22d8809a9ccb70db561709544c 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/di/GeocodingModule.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/di/GeocodingModule.kt @@ -37,8 +37,6 @@ import javax.inject.Singleton @InstallIn(SingletonComponent::class) object GeocodingModule { - var geocodingService: OfflineGeocodingService? = null - @Provides @Singleton fun provideGeocodingService( @@ -46,7 +44,8 @@ object GeocodingModule { locationRepository: LocationRepository, @ApplicationContext context: Context ): GeocodingService { - val onlineService = providePeliasGeocodingService(appPreferenceRepository, locationRepository) + val onlineService = + providePeliasGeocodingService(appPreferenceRepository, locationRepository) val offlineService = provideOfflineGeocodingService(context, locationRepository) return MultiplexedGeocodingService( appPreferenceRepository = appPreferenceRepository, @@ -71,14 +70,7 @@ object GeocodingModule { @ApplicationContext context: Context, locationRepository: LocationRepository ): OfflineGeocodingService { - val globalGeocodingService = geocodingService - if (globalGeocodingService != null) { - return globalGeocodingService - } else { - val newGeocoder = OfflineGeocodingService(context, locationRepository) - geocodingService = newGeocoder - return newGeocoder - } + return OfflineGeocodingService(context, locationRepository) } @Provides diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/di/RepositoryModule.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/di/RepositoryModule.kt index 3c52a62eacd7391ce4202f020b863bdf51c5ff6f..da9af4f6897226763583d3fe1ff9b69cf2bc51e8 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/di/RepositoryModule.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/di/RepositoryModule.kt @@ -24,6 +24,8 @@ import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent +import earth.maps.cardinal.data.PlanStateRepository +import earth.maps.cardinal.data.RouteStateRepository import earth.maps.cardinal.data.room.DownloadedTileDao import earth.maps.cardinal.data.room.OfflineAreaDao import earth.maps.cardinal.data.room.OfflineAreaRepository @@ -41,6 +43,18 @@ object RepositoryModule { return OfflineAreaRepository(offlineAreaDao) } + @Provides + @Singleton + fun provideRouteStateRepository(): RouteStateRepository { + return RouteStateRepository() + } + + @Provides + @Singleton + fun providePlanStateRepository(): PlanStateRepository { + return PlanStateRepository() + } + @Provides @Singleton fun provideTileDownloadManager( diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/core/AppContent.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/core/AppContent.kt index ee9c1cd647f6885914dc96b0f66e2c0ac0ec5808..2a62dc80c6118fdc016234f649ad97ee0f2ba97c 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/core/AppContent.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/core/AppContent.kt @@ -612,8 +612,8 @@ private fun PlaceCardRoute( val placeJson = backStackEntry.arguments?.getString("place") val place = placeJson?.let { Gson().fromJson(it, Place::class.java) } place?.let { place -> - viewModel.setPlace(place) LaunchedEffect(place) { + viewModel.setPlace(place) // Clear any existing pins and add the new one to ensure only one pin is shown at a time state.mapPins.clear() state.mapPins.add(place) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsScreen.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsScreen.kt index 6b3b914dc94ed3dc28360cf7febdb3fdadeee9a1..ec848f88ff6b5d55ece39935413798fe75361738 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsScreen.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsScreen.kt @@ -73,7 +73,9 @@ import earth.maps.cardinal.R.string import earth.maps.cardinal.data.AppPreferenceRepository import earth.maps.cardinal.data.GeoUtils import earth.maps.cardinal.data.Place +import earth.maps.cardinal.data.RouteState import earth.maps.cardinal.data.RoutingMode +import earth.maps.cardinal.data.TransitPlanState import earth.maps.cardinal.data.room.RoutingProfile import earth.maps.cardinal.ui.core.NavigationUtils import earth.maps.cardinal.ui.core.Screen @@ -114,12 +116,17 @@ fun DirectionsScreen( var pendingNavigationStart by remember { mutableStateOf(false) } // Get route result from ViewModel - val routeState = viewModel.routeState + val routeState by viewModel.routeState.collectAsState(initial = RouteState()) val savedPlaces by viewModel.savedPlaces.collectAsState(initial = emptyList()) val coroutineScope = rememberCoroutineScope() + LaunchedEffect(Unit) { + // Run this in our coroutine scope rather than viewModelScope. + viewModel.initializeRoutingMode() + } + // Auto-retry location request when permissions are granted AutoRetryMyLocation( hasLocationPermission = hasLocationPermission, @@ -141,8 +148,7 @@ fun DirectionsScreen( onRequestNotificationPermission = onRequestNotificationPermission, onPendingNavigationChange = { pendingNavigationStart = it - } - ) + }) Column( modifier = Modifier @@ -231,19 +237,17 @@ private fun AutoRetryNavigationStart( onPendingNavigationChange: (Boolean) -> Unit, ) { var showNotificationDialog by remember { mutableStateOf(false) } - NotificationRequestDialog( - showDialog = showNotificationDialog, - onDismiss = { - showNotificationDialog = false - onPendingNavigationChange(false) - }, - onConfirm = { - showNotificationDialog = false - onRequestNotificationPermission() - }) + val routeState by viewModel.routeState.collectAsState() + NotificationRequestDialog(showDialog = showNotificationDialog, onDismiss = { + showNotificationDialog = false + onPendingNavigationChange(false) + }, onConfirm = { + showNotificationDialog = false + onRequestNotificationPermission() + }) LaunchedEffect(hasNotificationPermission, hasLocationPermission, pendingNavigation) { if (pendingNavigation && hasNotificationPermission && hasLocationPermission) { - viewModel.startNavigation(navController) + viewModel.startNavigation(navController, routeState) } else if (pendingNavigation && !hasNotificationPermission) { showNotificationDialog = true } else if (pendingNavigation) { @@ -267,6 +271,8 @@ private fun DirectionsScreenFullUI( ) { val density = androidx.compose.ui.platform.LocalDensity.current + val coroutineScope = rememberCoroutineScope() + // Show full UI when no field is focused Column( modifier = Modifier @@ -346,7 +352,7 @@ private fun DirectionsScreenFullUI( RoutingModeSelector( availableModes = availableRoutingModes, selectedMode = viewModel.selectedRoutingMode, - onModeSelected = { viewModel.updateRoutingMode(it) }, + onModeSelected = { coroutineScope.launch { viewModel.updateRoutingMode(it) } }, modifier = Modifier .fillMaxWidth() .padding(bottom = dimensionResource(dimen.padding_minor)) @@ -386,7 +392,7 @@ private fun DirectionsRouteResults( appPreferences: AppPreferenceRepository, onPendingNavigationChange: (Boolean) -> Unit, ) { - val planState = viewModel.planState + val planState by viewModel.planState.collectAsState() if (viewModel.selectedRoutingMode == RoutingMode.PUBLIC_TRANSPORT) { TransitRouteResults( planState = planState, @@ -630,12 +636,10 @@ private fun SearchResultsContent( fieldFocusState: FieldFocusState ) { SearchResults( - places = viewModel.geocodeResults.value, - onPlaceSelected = { place -> + places = viewModel.geocodeResults.value, onPlaceSelected = { place -> updatePlaceForField(viewModel, fieldFocusState, place) onFieldFocusStateChange(FieldFocusState.NONE) - }, - modifier = Modifier.fillMaxWidth() + }, modifier = Modifier.fillMaxWidth() ) } @@ -685,7 +689,7 @@ fun RouteDisplayHandler( padding: PaddingValues, onRouteUpdate: (Route?) -> Unit ) { - val routeState = viewModel.routeState + val routeState by viewModel.routeState.collectAsState() val selectedMode = viewModel.selectedRoutingMode val coroutineScope = rememberCoroutineScope() diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt index bc332c7899a8643a8a6f7a0b17769ccd59ff04b5..907c1ffd1613f832a500acd5f704c7b5cf52d832 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt @@ -30,7 +30,11 @@ import earth.maps.cardinal.data.AppPreferenceRepository import earth.maps.cardinal.data.LatLng import earth.maps.cardinal.data.LocationRepository import earth.maps.cardinal.data.Place +import earth.maps.cardinal.data.PlanStateRepository +import earth.maps.cardinal.data.RouteState +import earth.maps.cardinal.data.RouteStateRepository import earth.maps.cardinal.data.RoutingMode +import earth.maps.cardinal.data.TransitPlanState import earth.maps.cardinal.data.ViewportRepository import earth.maps.cardinal.data.room.RecentSearchRepository import earth.maps.cardinal.data.room.RoutingProfile @@ -40,7 +44,6 @@ import earth.maps.cardinal.data.room.SavedPlaceRepository import earth.maps.cardinal.geocoding.GeocodingService import earth.maps.cardinal.routing.FerrostarWrapperRepository import earth.maps.cardinal.routing.RouteRepository -import earth.maps.cardinal.transit.PlanResponse import earth.maps.cardinal.transit.TransitousService import earth.maps.cardinal.ui.core.NavigationUtils import earth.maps.cardinal.ui.core.Screen @@ -58,7 +61,6 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import uniffi.ferrostar.GeographicCoordinate -import uniffi.ferrostar.Route import uniffi.ferrostar.UserLocation import uniffi.ferrostar.Waypoint import uniffi.ferrostar.WaypointKind @@ -66,20 +68,6 @@ import java.time.Instant import javax.inject.Inject import kotlin.time.ExperimentalTime -// Consolidated route state -data class RouteState( - val route: Route? = null, - val isLoading: Boolean = false, - val error: String? = null -) - -// Consolidated transit plan state -data class TransitPlanState( - val planResponse: PlanResponse? = null, - val isLoading: Boolean = false, - val error: String? = null -) - @OptIn(FlowPreview::class) @HiltViewModel class DirectionsViewModel @Inject constructor( @@ -94,6 +82,8 @@ class DirectionsViewModel @Inject constructor( private val appPreferenceRepository: AppPreferenceRepository, private val transitousService: TransitousService, private val recentSearchRepository: RecentSearchRepository, + private val routeStateRepository: RouteStateRepository, + private val planStateRepository: PlanStateRepository, ) : ViewModel() { // Search query flow for debouncing @@ -124,13 +114,9 @@ class DirectionsViewModel @Inject constructor( var selectedRoutingProfile by mutableStateOf(null) private set - // Consolidated route state - var routeState by mutableStateOf(RouteState()) - private set - - // Consolidated transit plan state - var planState by mutableStateOf(TransitPlanState()) - private set + // Expose state from repositories + val routeState: StateFlow = routeStateRepository.routeState + val planState: StateFlow = planStateRepository.planState // Saved places for quick suggestions val savedPlaces = placeDao.getAllPlacesAsFlow().map { list -> @@ -157,14 +143,15 @@ class DirectionsViewModel @Inject constructor( } } .launchIn(viewModelScope) + } - // Initialize with the default profile for the current routing mode - initializeDefaultProfileForMode(selectedRoutingMode) - + suspend fun initializeRoutingMode() { // Set initial routing mode from preferences selectedRoutingMode = appPreferenceRepository.lastRoutingMode.value.let { modeString -> RoutingMode.entries.find { it.value == modeString } ?: RoutingMode.AUTO } + // Initialize with the default profile for the current routing mode + initializeDefaultProfileForMode(selectedRoutingMode) } fun updateSearchQuery(query: String) { @@ -201,8 +188,8 @@ class DirectionsViewModel @Inject constructor( if (origin != null && destination != null) { fetchDirections(origin, destination) } else { - routeState = RouteState() - planState = TransitPlanState() + routeStateRepository.clear() + planStateRepository.clear() } } @@ -216,7 +203,7 @@ class DirectionsViewModel @Inject constructor( private fun fetchDrivingDirections(origin: Place, destination: Place) { viewModelScope.launch { - routeState = routeState.copy(isLoading = true, error = null) + routeStateRepository.setLoading(true) try { val ferrostarWrapper = getFerrostarWrapper() @@ -227,17 +214,11 @@ class DirectionsViewModel @Inject constructor( ferrostarWrapper.core.getRoutes(userLocation, waypoints) } - routeState = routeState.copy( - route = routes.firstOrNull(), - isLoading = false, - error = null - ) + routeStateRepository.setRoute(routes.firstOrNull()) } catch (e: Exception) { Log.e(TAG, "Error while fetching route", e) - routeState = routeState.copy( - route = null, - isLoading = false, - error = e.message ?: "An error occurred while fetching the route" + routeStateRepository.setError( + e.message ?: "An error occurred while fetching the route" ) } } @@ -246,7 +227,7 @@ class DirectionsViewModel @Inject constructor( @OptIn(ExperimentalTime::class) private fun fetchTransitDirections(origin: Place, destination: Place) { viewModelScope.launch { - planState = planState.copy(isLoading = true, error = null) + planStateRepository.setLoading(true) try { transitousService.getPlan( @@ -255,18 +236,12 @@ class DirectionsViewModel @Inject constructor( timetableView = false, withFares = true, ).collect { planResponse -> - planState = planState.copy( - planResponse = planResponse, - isLoading = false, - error = null - ) + planStateRepository.setPlanResponse(planResponse) } } catch (e: Exception) { Log.e(TAG, "Error while fetching transit plan", e) - planState = planState.copy( - planResponse = null, - isLoading = false, - error = e.message ?: "An error occurred while fetching transit directions" + planStateRepository.setError( + e.message ?: "An error occurred while fetching transit directions" ) } } @@ -297,7 +272,7 @@ class DirectionsViewModel @Inject constructor( speed = null ) - fun updateRoutingMode(mode: RoutingMode) { + suspend fun updateRoutingMode(mode: RoutingMode) { selectedRoutingMode = mode appPreferenceRepository.setLastRoutingMode(mode.value) // Load the default profile for the new mode @@ -309,7 +284,7 @@ class DirectionsViewModel @Inject constructor( * Ensures the selected routing mode is valid by checking if it's in the available modes list. * If not, falls back to AUTO mode. This should be called when available modes change. */ - private fun ensureSelectedModeIsValid(availableModes: List) { + private suspend fun ensureSelectedModeIsValid(availableModes: List) { if (selectedRoutingMode !in availableModes) { // Fall back to AUTO mode if current mode is no longer available updateRoutingMode(RoutingMode.AUTO) @@ -396,35 +371,33 @@ class DirectionsViewModel @Inject constructor( * This loads the saved default profile from the database and applies its options. * If no default profile exists, sets selectedRoutingProfile to null and uses built-in defaults. */ - private fun initializeDefaultProfileForMode(mode: RoutingMode) { - viewModelScope.launch { - routingProfileRepository.getDefaultProfile(mode).fold( - onSuccess = { profileWithOptions -> - if (profileWithOptions != null) { - val (profile, options) = profileWithOptions - options?.let { - selectedRoutingProfile = profile - ferrostarWrapperRepository.setOptionsForMode(mode, it) - } - } else { - // No default profile exists, use built-in defaults - selectedRoutingProfile = null - ferrostarWrapperRepository.resetOptionsToDefaultsForMode(mode) + private suspend fun initializeDefaultProfileForMode(mode: RoutingMode) { + routingProfileRepository.getDefaultProfile(mode).fold( + onSuccess = { profileWithOptions -> + if (profileWithOptions != null) { + val (profile, options) = profileWithOptions + options?.let { + selectedRoutingProfile = profile + ferrostarWrapperRepository.setOptionsForMode(mode, it) } - }, - onFailure = { error -> - Log.e(TAG, "Failed to load default profile for mode $mode", error) - // Fallback to built-in defaults + } else { + // No default profile exists, use built-in defaults selectedRoutingProfile = null ferrostarWrapperRepository.resetOptionsToDefaultsForMode(mode) } - ) - } + }, + onFailure = { error -> + Log.e(TAG, "Failed to load default profile for mode $mode", error) + // Fallback to built-in defaults + selectedRoutingProfile = null + ferrostarWrapperRepository.resetOptionsToDefaultsForMode(mode) + } + ) } - fun startNavigation(navController: NavController) { - routeState.route?.let { route -> + fun startNavigation(navController: NavController, state: RouteState) { + state.route?.let { route -> NavigationUtils.navigate( navController, Screen.TurnByTurnNavigation( diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/TransitDirectionsScreen.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/TransitDirectionsScreen.kt index 5fae7f7821230c14ee0f5f29b528c02afe87c9ac..d7bbb987f63be9a41a4679b32fe5026caed61c0a 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/TransitDirectionsScreen.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/TransitDirectionsScreen.kt @@ -61,7 +61,8 @@ fun TransitDirectionsScreen( ) { val use24HourFormat by appPreferences.use24HourFormat.collectAsState() val distanceUnit by appPreferences.distanceUnit.collectAsState() - val planState = viewModel.planState + val planStateState by viewModel.planState.collectAsState() + val planState = planStateState // Strip delegated property status. when { planState.isLoading -> { Text( diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt index 0f2bb5ad633fa6f7f35c8d02c7def7682e210270..f9650c9cff2fb243d625d9baf4819f56c7cf9496 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt @@ -41,6 +41,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -59,6 +60,7 @@ import earth.maps.cardinal.R.string import earth.maps.cardinal.data.AddressFormatter import earth.maps.cardinal.data.Place import earth.maps.cardinal.data.format +import kotlinx.coroutines.launch @Composable fun PlaceCardScreen( @@ -104,7 +106,12 @@ fun PlaceCardScreen( ) { PlaceHeader(displayedPlace) PlaceAddress(displayedPlace, addressFormatter) - PlaceActions(displayedPlace, viewModel, place, onGetDirections) { showUnsaveConfirmationDialog = true } + PlaceActions( + displayedPlace, + viewModel, + place, + onGetDirections + ) { showUnsaveConfirmationDialog = true } // Inset horizontal divider HorizontalDivider( modifier = Modifier @@ -121,7 +128,11 @@ fun PlaceCardScreen( }, onRouteClicked = {}) } - UnsaveConfirmationDialog(displayedPlace, viewModel, showUnsaveConfirmationDialog) { showUnsaveConfirmationDialog = false } + UnsaveConfirmationDialog( + displayedPlace, + viewModel, + showUnsaveConfirmationDialog + ) { showUnsaveConfirmationDialog = false } } } @@ -186,6 +197,8 @@ private fun PlaceActions( Text(stringResource(string.get_directions)) } + val coroutineScope = rememberCoroutineScope() + // Save/Unsave button Button( onClick = { @@ -193,7 +206,9 @@ private fun PlaceActions( // Show confirmation dialog for unsaving onShowUnsaveDialog() } else { - viewModel.savePlace(place) + coroutineScope.launch { + viewModel.savePlace(place) + } } }, modifier = Modifier.padding(start = dimensionResource(dimen.padding_minor)) ) { @@ -228,6 +243,8 @@ private fun UnsaveConfirmationDialog( show: Boolean, onDismiss: () -> Unit ) { + val coroutineScope = rememberCoroutineScope() + if (show) { AlertDialog( onDismissRequest = onDismiss, @@ -242,8 +259,10 @@ private fun UnsaveConfirmationDialog( confirmButton = { TextButton( onClick = { - viewModel.unsavePlace(displayedPlace) - onDismiss() + coroutineScope.launch { + viewModel.unsavePlace(displayedPlace) + onDismiss() + } }) { Text(stringResource(string.unsave_place)) } diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardViewModel.kt index c085708efa94ae3c79296597bff3425a320cc416..0633769f176f19ead915633df358fee77953a932 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardViewModel.kt @@ -20,11 +20,9 @@ package earth.maps.cardinal.ui.place import androidx.compose.runtime.mutableStateOf import androidx.lifecycle.ViewModel -import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import earth.maps.cardinal.data.Place import earth.maps.cardinal.data.room.SavedPlaceRepository -import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel @@ -35,33 +33,27 @@ class PlaceCardViewModel @Inject constructor( val isPlaceSaved = mutableStateOf(false) val place = mutableStateOf(null) - fun setPlace(place: Place) { + suspend fun setPlace(place: Place) { this.place.value = place checkIfPlaceIsSaved(place) } - fun checkIfPlaceIsSaved(place: Place) { - viewModelScope.launch { - if (place.id != null) { - val existingPlace = savedPlaceRepository.getPlaceById(place.id).getOrNull() - isPlaceSaved.value = existingPlace != null - } + suspend fun checkIfPlaceIsSaved(place: Place) { + if (place.id != null) { + val existingPlace = savedPlaceRepository.getPlaceById(place.id).getOrNull() + isPlaceSaved.value = existingPlace != null } } - fun savePlace(place: Place) { - viewModelScope.launch { - savedPlaceRepository.savePlace(place) - isPlaceSaved.value = true - } + suspend fun savePlace(place: Place) { + savedPlaceRepository.savePlace(place) + isPlaceSaved.value = true } - fun unsavePlace(place: Place) { - viewModelScope.launch { - place.id?.let { id -> - savedPlaceRepository.deletePlace(placeId = id) - } - isPlaceSaved.value = false + suspend fun unsavePlace(place: Place) { + place.id?.let { id -> + savedPlaceRepository.deletePlace(placeId = id) } + isPlaceSaved.value = false } } diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/MainCoroutineRule.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/MainCoroutineRule.kt new file mode 100644 index 0000000000000000000000000000000000000000..ab0885518360522e0781f56025acb5b6e3dd9731 --- /dev/null +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/MainCoroutineRule.kt @@ -0,0 +1,25 @@ +package earth.maps.cardinal + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import org.junit.rules.TestWatcher +import org.junit.runner.Description + +@ExperimentalCoroutinesApi +class MainCoroutineRule(private val dispatcher: TestDispatcher = StandardTestDispatcher()) : + TestWatcher() { + + override fun starting(description: Description?) { + super.starting(description) + Dispatchers.setMain(dispatcher) + } + + override fun finished(description: Description?) { + super.finished(description) + Dispatchers.resetMain() + } +} \ No newline at end of file diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/directions/DirectionsViewModelTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/directions/DirectionsViewModelTest.kt new file mode 100644 index 0000000000000000000000000000000000000000..66708f6714ae863c9bfbc6cba2f850ca9e764891 --- /dev/null +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/directions/DirectionsViewModelTest.kt @@ -0,0 +1,312 @@ +package earth.maps.cardinal.ui.directions + +import earth.maps.cardinal.MainCoroutineRule +import earth.maps.cardinal.data.AppPreferenceRepository +import earth.maps.cardinal.data.LatLng +import earth.maps.cardinal.data.LocationRepository +import earth.maps.cardinal.data.Place +import earth.maps.cardinal.data.PlanStateRepository +import earth.maps.cardinal.data.RouteState +import earth.maps.cardinal.data.RouteStateRepository +import earth.maps.cardinal.data.RoutingMode +import earth.maps.cardinal.data.TransitPlanState +import earth.maps.cardinal.data.ViewportRepository +import earth.maps.cardinal.data.room.RecentSearchRepository +import earth.maps.cardinal.data.room.RoutingProfileRepository +import earth.maps.cardinal.data.room.SavedPlaceDao +import earth.maps.cardinal.data.room.SavedPlaceRepository +import earth.maps.cardinal.geocoding.GeocodingService +import earth.maps.cardinal.routing.FerrostarWrapper +import earth.maps.cardinal.routing.FerrostarWrapperRepository +import earth.maps.cardinal.routing.RouteRepository +import earth.maps.cardinal.transit.TransitousService +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import uniffi.ferrostar.Route +import kotlin.time.ExperimentalTime + +@OptIn(ExperimentalTime::class, ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class DirectionsViewModelTest { + + @ExperimentalCoroutinesApi + @get:Rule + var mainCoroutineRule = MainCoroutineRule(UnconfinedTestDispatcher()) + + private lateinit var viewModel: DirectionsViewModel + + private val mockGeocodingService = mockk() + private val mockFerrostarWrapperRepository = mockk() + private val mockViewportRepository = mockk() + private val mockPlaceDao = mockk() + private val mockSavedPlaceRepository = mockk() + private val mockLocationRepository = mockk() + private val mockRoutingProfileRepository = mockk() + private val mockRouteRepository = mockk() + private val mockAppPreferenceRepository = mockk() + private val mockTransitousService = mockk() + private val mockRecentSearchRepository = mockk() + private val mockRouteStateRepository = mockk() + private val mockPlanStateRepository = mockk() + + @Before + fun setup() { + // Mock saved places flow + coEvery { mockPlaceDao.getAllPlacesAsFlow() } returns flowOf(emptyList()) + + // Mock app preferences + coEvery { mockAppPreferenceRepository.continuousLocationTracking } returns MutableStateFlow( + false + ) + coEvery { mockAppPreferenceRepository.lastRoutingMode } returns MutableStateFlow("auto") + coEvery { mockAppPreferenceRepository.setLastRoutingMode(any()) } returns Unit + + // Mock route state repository + coEvery { mockRouteStateRepository.routeState } returns MutableStateFlow(RouteState()) + coEvery { mockPlanStateRepository.planState } returns MutableStateFlow(TransitPlanState()) + coEvery { mockRouteStateRepository.clear() } returns Unit + coEvery { mockPlanStateRepository.clear() } returns Unit + coEvery { mockRouteStateRepository.setLoading(any()) } returns Unit + coEvery { mockPlanStateRepository.setLoading(any()) } returns Unit + coEvery { mockRouteStateRepository.setRoute(any()) } returns Unit + coEvery { mockPlanStateRepository.setPlanResponse(any()) } returns Unit + coEvery { mockRouteStateRepository.setError(any()) } returns Unit + coEvery { mockPlanStateRepository.setError(any()) } returns Unit + + // Mock routing profile repository + coEvery { mockRoutingProfileRepository.getDefaultProfile(any()) } returns Result.success( + null + ) + coEvery { mockRoutingProfileRepository.getProfilesForMode(any()) } returns flowOf(emptyList()) + + coEvery { mockFerrostarWrapperRepository.resetOptionsToDefaultsForMode(any()) } returns Unit + + viewModel = DirectionsViewModel( + geocodingService = mockGeocodingService, + ferrostarWrapperRepository = mockFerrostarWrapperRepository, + viewportRepository = mockViewportRepository, + placeDao = mockPlaceDao, + savedPlaceRepository = mockSavedPlaceRepository, + locationRepository = mockLocationRepository, + routingProfileRepository = mockRoutingProfileRepository, + routeRepository = mockRouteRepository, + appPreferenceRepository = mockAppPreferenceRepository, + transitousService = mockTransitousService, + recentSearchRepository = mockRecentSearchRepository, + routeStateRepository = mockRouteStateRepository, + planStateRepository = mockPlanStateRepository, + ) + } + + @Test + fun `updateSearchQuery should update search query`() = runTest { + val testQuery = "Test Search Query" + + viewModel.updateSearchQuery(testQuery) + + assertEquals(testQuery, viewModel.searchQuery) + } + + @Test + fun `updateSearchQuery with empty string should clear search query`() = runTest { + val testQuery = "Test Search Query" + viewModel.updateSearchQuery(testQuery) + assertEquals(testQuery, viewModel.searchQuery) + + viewModel.updateSearchQuery("") + assertEquals("", viewModel.searchQuery) + } + + @Test + fun `fetchDirectionsIfNeeded should set loading state for driving directions when both places are set`() = + runTest { + val fromPlace = Place( + id = "1", + name = "From Place", + latLng = LatLng(0.0, 0.0), + address = null + ) + val toPlace = Place( + id = "2", + name = "To Place", + latLng = LatLng(1.0, 1.0), + address = null + ) + + val mockFerrostarWrapper = mockk() + coEvery { mockFerrostarWrapperRepository.driving } returns mockFerrostarWrapper + val mockRoute = mockk() + coEvery { mockFerrostarWrapper.core.getRoutes(any(), any()) } returns listOf(mockRoute) + + // Get initial state + val initialState = viewModel.routeState.value + assertEquals(RouteState(), initialState) + assertFalse(initialState.isLoading) + assertNull(initialState.route) + + // Trigger the state changes + viewModel.updateFromPlace(fromPlace) + viewModel.updateToPlace(toPlace) + + // Allow all coroutines, including viewModelScope, to complete + advanceUntilIdle() + + // Verify repository interactions + coVerify { mockRouteStateRepository.setLoading(true) } + coVerify { mockRouteStateRepository.setRoute(mockRoute) } + } + + @Test + fun `updateRoutingMode should update selected mode and save to preferences`() = runTest { + val testMode = RoutingMode.PEDESTRIAN + + // Get initial mode + val initialMode = viewModel.selectedRoutingMode + assertEquals(RoutingMode.AUTO, initialMode) + + // Update the mode + viewModel.updateRoutingMode(testMode) + + // Verify the mode was updated + assertEquals(testMode, viewModel.selectedRoutingMode) + + // Verify it was saved to preferences + coVerify { mockAppPreferenceRepository.setLastRoutingMode(testMode.value) } + } + + @Test + fun `updateToPlace with null should clear route and plan state`() = runTest { + viewModel.updateToPlace(null) + coVerify { mockRouteStateRepository.clear() } + coVerify { mockPlanStateRepository.clear() } + } + + @Test + fun `updateFromPlace and updateToPlace should set places correctly`() = runTest { + val fromPlace = Place( + id = "from1", + name = "From Place", + latLng = LatLng(0.0, 0.0), + address = null + ) + val toPlace = Place( + id = "to1", + name = "To Place", + latLng = LatLng(1.0, 1.0), + address = null + ) + + // Initially, both places should be null + assertNull(viewModel.fromPlace) + assertNull(viewModel.toPlace) + + // Update from place + viewModel.updateFromPlace(fromPlace) + assertEquals(fromPlace, viewModel.fromPlace) + assertNull(viewModel.toPlace) // toPlace should still be null + + // Update to place + viewModel.updateToPlace(toPlace) + assertEquals(fromPlace, viewModel.fromPlace) + assertEquals(toPlace, viewModel.toPlace) + + // Setting fromPlace to null should not affect toPlace + viewModel.updateFromPlace(null) + assertNull(viewModel.fromPlace) + assertEquals(toPlace, viewModel.toPlace) + + // Setting toPlace to null should not affect fromPlace + viewModel.updateFromPlace(fromPlace) + viewModel.updateToPlace(null) + assertEquals(fromPlace, viewModel.fromPlace) + assertNull(viewModel.toPlace) + } + + @Test + fun `fetchDirectionsIfNeeded should handle route fetching error`() = runTest { + val fromPlace = Place( + id = "1", + name = "From Place", + latLng = LatLng(0.0, 0.0), + address = null + ) + val toPlace = Place( + id = "2", + name = "To Place", + latLng = LatLng(1.0, 1.0), + address = null + ) + + // Setup mock to throw an exception + val mockFerrostarWrapper = mockk() + coEvery { mockFerrostarWrapperRepository.driving } returns mockFerrostarWrapper + val errorMessage = "Failed to get route" + coEvery { mockFerrostarWrapper.core.getRoutes(any(), any()) } throws Exception(errorMessage) + + // Trigger the direction fetching + viewModel.updateFromPlace(fromPlace) + viewModel.updateToPlace(toPlace) + + // Allow coroutines to complete + advanceUntilIdle() + + // Verify repository interactions. + // The view model will call setLoading(true), then setError(errorMessage) in the catch block. + coVerify { mockRouteStateRepository.setLoading(true) } + coVerify { mockRouteStateRepository.setError(errorMessage) } + } + + @Test + fun `fetchDirectionsIfNeeded should not fetch if fromPlace is null`() = runTest { + val toPlace = Place( + id = "to1", + name = "To Place", + latLng = LatLng(1.0, 1.0), + address = null + ) + + viewModel.updateFromPlace(null) + viewModel.updateToPlace(toPlace) + advanceUntilIdle() + + // Verify that no loading or route setting was called + coVerify(exactly = 0) { mockRouteStateRepository.setLoading(any()) } + coVerify(exactly = 0) { mockRouteStateRepository.setRoute(any()) } + coVerify(exactly = 0) { mockRouteStateRepository.setError(any()) } + } + + @Test + fun `fetchDirectionsIfNeeded should not fetch if toPlace is null`() = runTest { + val fromPlace = Place( + id = "from1", + name = "From Place", + latLng = LatLng(0.0, 0.0), + address = null + ) + + viewModel.updateFromPlace(fromPlace) + viewModel.updateToPlace(null) + advanceUntilIdle() + + // Verify that no loading or route setting was called + coVerify(exactly = 0) { mockRouteStateRepository.setLoading(any()) } + coVerify(exactly = 0) { mockRouteStateRepository.setRoute(any()) } + coVerify(exactly = 0) { mockRouteStateRepository.setError(any()) } + } +} diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/place/PlaceCardViewModelTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/place/PlaceCardViewModelTest.kt new file mode 100644 index 0000000000000000000000000000000000000000..55d43dfd903712c43843713d240b53eaf0c66d88 --- /dev/null +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/place/PlaceCardViewModelTest.kt @@ -0,0 +1,148 @@ +package earth.maps.cardinal.ui.place + +import earth.maps.cardinal.data.Place +import earth.maps.cardinal.data.room.SavedPlace +import earth.maps.cardinal.data.room.SavedPlaceRepository +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class PlaceCardViewModelTest { + + private lateinit var viewModel: PlaceCardViewModel + + private val mockSavedPlaceRepository = mockk() + + @Before + fun setup() { + viewModel = PlaceCardViewModel( + savedPlaceRepository = mockSavedPlaceRepository + ) + } + + @Test + fun `setPlace should update place and check if place is saved`() = runTest { + val testPlace = Place( + id = "1", + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + val savedPlace = SavedPlace.fromPlace(testPlace) + + coEvery { mockSavedPlaceRepository.getPlaceById("1") } returns Result.success(savedPlace) + + viewModel.setPlace(testPlace) + + assertThat(viewModel.place.value).isEqualTo(testPlace) + assertThat(viewModel.isPlaceSaved.value).isTrue() + coVerify { mockSavedPlaceRepository.getPlaceById("1") } + } + + @Test + fun `checkIfPlaceIsSaved should set isPlaceSaved to true when place exists`() = runTest { + val testPlace = Place( + id = "1", + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + val savedPlace = SavedPlace.fromPlace(testPlace) + + coEvery { mockSavedPlaceRepository.getPlaceById("1") } returns Result.success(savedPlace) + + viewModel.checkIfPlaceIsSaved(testPlace) + + assertThat(viewModel.isPlaceSaved.value).isTrue() + } + + @Test + fun `checkIfPlaceIsSaved should set isPlaceSaved to false when place does not exist`() = runTest { + val testPlace = Place( + id = "1", + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + + coEvery { mockSavedPlaceRepository.getPlaceById("1") } returns Result.success(null) + + viewModel.checkIfPlaceIsSaved(testPlace) + + assertThat(viewModel.isPlaceSaved.value).isFalse() + } + + @Test + fun `checkIfPlaceIsSaved should not check when place id is null`() = runTest { + val testPlace = Place( + id = null, + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + + viewModel.checkIfPlaceIsSaved(testPlace) + + assertThat(viewModel.isPlaceSaved.value).isFalse() + coVerify(exactly = 0) { mockSavedPlaceRepository.getPlaceById(any()) } + } + + @Test + fun `savePlace should save place and set isPlaceSaved to true`() = runTest { + val testPlace = Place( + id = "1", + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + + coEvery { mockSavedPlaceRepository.savePlace(testPlace) } returns Result.success("1") + + viewModel.savePlace(testPlace) + + coVerify { mockSavedPlaceRepository.savePlace(testPlace) } + assertThat(viewModel.isPlaceSaved.value).isTrue() + } + + @Test + fun `unsavePlace should delete place and set isPlaceSaved to false`() = runTest { + val testPlace = Place( + id = "1", + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + + coEvery { mockSavedPlaceRepository.deletePlace("1") } returns Result.success(Unit) + + viewModel.unsavePlace(testPlace) + + coVerify { mockSavedPlaceRepository.deletePlace(placeId = "1") } + assertThat(viewModel.isPlaceSaved.value).isFalse() + } + + @Test + fun `unsavePlace should not delete when place id is null`() = runTest { + val testPlace = Place( + id = null, + name = "Test Place", + latLng = earth.maps.cardinal.data.LatLng(0.0, 0.0), + address = null + ) + + viewModel.unsavePlace(testPlace) + + coVerify(exactly = 0) { mockSavedPlaceRepository.deletePlace(any()) } + assertThat(viewModel.isPlaceSaved.value).isFalse() + } +} diff --git a/cardinal-android/gradle/libs.versions.toml b/cardinal-android/gradle/libs.versions.toml index 5473b5539f663cf66529c6a0fef9bb7216ca1fda..10d20c0355d1ef4e171a522bd9f762d1c0857412 100644 --- a/cardinal-android/gradle/libs.versions.toml +++ b/cardinal-android/gradle/libs.versions.toml @@ -30,7 +30,7 @@ ferrostar = "0.41.0" okhttp3 = "5.1.0" material3 = "1.5.0-alpha04" detekt = "2.0.0-alpha.0" -mockk = "1.13.16" +mockk = "1.14.6" kotlinxCoroutinesTest = "1.10.2" hiltAndroidTesting = "2.57.1"