Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1df97c86 authored by Ellen Poe's avatar Ellen Poe
Browse files

fix: tests and detekt

parent c993eedd
Loading
Loading
Loading
Loading
Loading
+125 −96
Original line number Diff line number Diff line
@@ -169,102 +169,17 @@ fun AppContent(
    LaunchedEffect(state.peekHeight) {
        mapViewModel.peekHeight = state.peekHeight
    }
    Box(
        modifier = Modifier
            .fillMaxSize()
            .onGloballyPositioned {
                state.screenHeightDp = with(state.density) { it.size.height.toDp() }
                state.screenWidthDp = with(state.density) { it.size.width.toDp() }
                // For very annoying reasons, this ViewModel needs to know the size of the screen.
                // Specifically, it is responsible for tracking the state of the "locate me" button across
                // a permission request lifecycle. When the permission request is done, it has zero
                // business calling back into the view to perform the animateTo operation, and in order
                // to perform the animateTo you need to calculate padding based on screen size and peek
                // height. :(
                mapViewModel.screenWidth = state.screenWidthDp
                mapViewModel.screenHeight = state.screenHeightDp
            },

        ) {
        if (port != null && port != -1) {
            MapView(
    MapViewContainer(
        port = port,
        mapViewModel = mapViewModel,
                onMapInteraction = {
                    if (topOfBackStack?.destination?.route?.startsWith("place_card") == true) {
                        navController.popBackStack()
                    }
                },
                onMapPoiClick = {
                    if (topOfBackStack?.destination?.route?.startsWith("directions") != true && topOfBackStack?.destination?.route?.startsWith(
                            "transit_itinerary_detail"
                        ) != true
                    ) {
                        NavigationUtils.navigate(navController, Screen.PlaceCard(it))
                    }
                },
                onDropPin = {
                    val place = Place(
                        name = droppedPinName,
                        description = "",
                        icon = "place",
                        latLng = it,
                        address = null,
                        isMyLocation = false
                    )
                    NavigationUtils.navigate(navController, Screen.PlaceCard(place))
                },
        state = state,
        navController = navController,
        topOfBackStack = topOfBackStack,
        droppedPinName = droppedPinName,
        onRequestLocationPermission = onRequestLocationPermission,
        hasLocationPermission = hasLocationPermission,
                fabInsets = PaddingValues(
                    start = 0.dp,
                    top = 0.dp,
                    end = 0.dp,
                    bottom = if (state.screenHeightDp > state.fabHeight) {
                        state.screenHeightDp - state.fabHeight
                    } else {
                        0.dp
                    }
                ),
                cameraState = state.cameraState,
                mapPins = state.mapPins,
                appPreferences = appPreferenceRepository,
                selectedOfflineArea = state.selectedOfflineArea,
                currentRoute = state.currentRoute,
                allRoutes = state.allRoutes,
                currentTransitItinerary = state.currentTransitItinerary,
                onRouteAnnotationClick = { routeIndex ->
                    // Handle route annotation click by updating the selected route index in AppContentState
                    // The DirectionsScreen will observe this change and update the DirectionsViewModel
                    if (state.allRoutes.isNotEmpty()) {
                        val actualIndex = if (routeIndex == -1) {
                            // If -1 is passed, it means the current selected route was tapped
                            // Keep the current selection
                            state.selectedRouteIndex ?: 0
                        } else {
                            // Convert the reversed index back to the correct index
                            // because routes are displayed in reverse order in the RouteLayer
                            state.allRoutes.size - 1 - routeIndex
                        }

                        if (actualIndex >= 0 && actualIndex < state.allRoutes.size) {
                            state.selectedRouteIndex = actualIndex
                        }
                    }
                }
        appPreferenceRepository = appPreferenceRepository
    )
        } else {
            LaunchedEffect(key1 = port) {
                Log.e("AppContent", "Tileserver port is $port, can't display a map!")
            }
        }

        Box(
            modifier = Modifier.windowInsetsPadding(WindowInsets.safeDrawing)
        ) {
            BirdSettingsFab(navController)
        }
    }

    NavHost(
        navController = navController, startDestination = Screen.HOME_SEARCH
@@ -465,6 +380,120 @@ fun AppContent(
    }
}

@Composable
@Suppress("CognitiveComplexMethod")
private fun MapViewContainer(
    port: Int?,
    mapViewModel: MapViewModel,
    state: AppContentState,
    navController: NavHostController,
    topOfBackStack: NavBackStackEntry?,
    droppedPinName: String,
    onRequestLocationPermission: () -> Unit,
    hasLocationPermission: Boolean,
    appPreferenceRepository: AppPreferenceRepository
) {
    Box(
        modifier = Modifier
            .fillMaxSize()
            .onGloballyPositioned {
                state.screenHeightDp = with(state.density) { it.size.height.toDp() }
                state.screenWidthDp = with(state.density) { it.size.width.toDp() }
                // For very annoying reasons, this ViewModel needs to know the size of the screen.
                // Specifically, it is responsible for tracking the state of the "locate me" button across
                // a permission request lifecycle. When the permission request is done, it has zero
                // business calling back into the view to perform the animateTo operation, and in order
                // to perform the animateTo you need to calculate padding based on screen size and peek
                // height. :(
                mapViewModel.screenWidth = state.screenWidthDp
                mapViewModel.screenHeight = state.screenHeightDp
            },
    ) {
        if (port != null && port != -1) {
            MapView(
                port = port,
                mapViewModel = mapViewModel,
                onMapInteraction = {
                    if (topOfBackStack?.destination?.route?.startsWith("place_card") == true) {
                        navController.popBackStack()
                    }
                },
                onMapPoiClick = {
                    if (topOfBackStack?.destination?.route?.startsWith("directions") != true && topOfBackStack?.destination?.route?.startsWith(
                            "transit_itinerary_detail"
                        ) != true
                    ) {
                        NavigationUtils.navigate(navController, Screen.PlaceCard(it))
                    }
                },
                onDropPin = {
                    val place = Place(
                        name = droppedPinName,
                        description = "",
                        icon = "place",
                        latLng = it,
                        address = null,
                        isMyLocation = false
                    )
                    NavigationUtils.navigate(navController, Screen.PlaceCard(place))
                },
                onRequestLocationPermission = onRequestLocationPermission,
                hasLocationPermission = hasLocationPermission,
                fabInsets = PaddingValues(
                    start = 0.dp,
                    top = 0.dp,
                    end = 0.dp,
                    bottom = if (state.screenHeightDp > state.fabHeight) {
                        state.screenHeightDp - state.fabHeight
                    } else {
                        0.dp
                    }
                ),
                cameraState = state.cameraState,
                mapPins = state.mapPins,
                appPreferences = appPreferenceRepository,
                selectedOfflineArea = state.selectedOfflineArea,
                currentRoute = state.currentRoute,
                allRoutes = state.allRoutes,
                currentTransitItinerary = state.currentTransitItinerary,
                onRouteAnnotationClick = { routeIndex ->
                    handleRouteAnnotationClick(routeIndex, state)
                }
            )
        } else {
            LaunchedEffect(key1 = port) {
                Log.e("AppContent", "Tileserver port is $port, can't display a map!")
            }
        }

        Box(
            modifier = Modifier.windowInsetsPadding(WindowInsets.safeDrawing)
        ) {
            BirdSettingsFab(navController)
        }
    }
}

private fun handleRouteAnnotationClick(routeIndex: Int, state: AppContentState) {
    // Handle route annotation click by updating the selected route index in AppContentState
    // The DirectionsScreen will observe this change and update the DirectionsViewModel
    if (state.allRoutes.isNotEmpty()) {
        val actualIndex = if (routeIndex == -1) {
            // If -1 is passed, it means the current selected route was tapped
            // Keep the current selection
            state.selectedRouteIndex ?: 0
        } else {
            // Convert the reversed index back to the correct index
            // because routes are displayed in reverse order in the RouteLayer
            state.allRoutes.size - 1 - routeIndex
        }

        if (actualIndex >= 0 && actualIndex < state.allRoutes.size) {
            state.selectedRouteIndex = actualIndex
        }
    }
}

@OptIn(ExperimentalMaterial3Api::class)
@Composable
private fun HomeRoute(
+110 −63
Original line number Diff line number Diff line
@@ -60,26 +60,80 @@ class AnnotationPlacer @Inject constructor() {
     */
    fun placeAnnotations(routes: List<Route>): Map<Route, LatLng> {
        Log.d(TAG, "calculating annotations for ${routes.size} routes")
        
        if (routes.isEmpty()) {
            return emptyMap()
        }
        
        val routesLatLng = convertRoutesToLatLng(routes)
        val routeOverlapCount = countOverlappingPoints(routesLatLng)
        
        return routesLatLng.indices.mapNotNull { index ->
            findOptimalPlacement(routes[index], routesLatLng[index], routeOverlapCount)
        }.toMap()
    }
    
    /**
     * Converts Route objects to lists of LatLng coordinates.
     */
    private fun convertRoutesToLatLng(routes: List<Route>): List<List<LatLng>> {
        return routes.map { route ->
            route.geometry.map { LatLng(it.lat, it.lng) }
        }
    }
    
    /**
     * Counts how many routes pass through each geographic point.
     */
    private fun countOverlappingPoints(routesLatLng: List<List<LatLng>>): Map<LatLng, Int> {
        val routeOverlapCount: MutableMap<LatLng, Int> = mutableMapOf()
        val routesLatLng = routes.map { route -> route.geometry.map { LatLng(it.lat, it.lng) } }
        val placements: MutableMap<Route, LatLng> = mutableMapOf()
        
        // Step 1: Count how many routes pass through each point
        for (route in routesLatLng) {
            for (point in route) {
                routeOverlapCount.merge(point, 1, Int::plus)
            }
        }
        
        // Step 2: For each route, find divergent segments and place annotations
        routesLatLng.forEachIndexed { index, route ->
        return routeOverlapCount
    }
    
    /**
     * Finds the optimal annotation placement for a single route.
     * Returns null if no suitable placement is found.
     */
    private fun findOptimalPlacement(
        route: Route,
        routeLatLng: List<LatLng>,
        routeOverlapCount: Map<LatLng, Int>
    ): Pair<Route, LatLng>? {
        val divergentSegments = identifyDivergentSegments(routeLatLng, routeOverlapCount)
        
        if (divergentSegments.isEmpty()) {
            Log.w(TAG, "Route with zero divergent segments found, skipping")
            return null
        }
        
        val bestSegment = selectLongestSegment(divergentSegments)
        val midpoint = findSegmentMidpoint(bestSegment)
        
        return Pair(route, midpoint)
    }
    
    /**
     * Identifies all contiguous divergent segments in a route.
     * A segment is divergent when only one route passes through its points.
     */
    private fun identifyDivergentSegments(
        route: List<LatLng>,
        routeOverlapCount: Map<LatLng, Int>
    ): List<List<Pair<LatLng, Double>>> {
        val segments: MutableList<List<Pair<LatLng, Double>>> = mutableListOf()
        var currentSegment: MutableList<Pair<LatLng, Double>>? = null
        var currentSegmentLengthMeters: Double? = null
        
            // Step 2a: Identify contiguous divergent segments
        for (point in route) {
            val isDiverged = routeOverlapCount[point] == 1
            
            if (isDiverged) {
                if (currentSegment != null) {
                    // Continue current segment
@@ -107,36 +161,29 @@ class AnnotationPlacer @Inject constructor() {
            segments.add(currentSegment)
        }
        
            // Step 2b: Select the longest divergent segment
            val bestSegment =
                segments.sortedByDescending { segment -> segment.lastOrNull()?.second }
                    .firstOrNull()
            
            if (bestSegment == null) {
                Log.w(TAG, "Route with zero divergent segments found, skipping")
                return@forEachIndexed
        return segments
    }
    
            val bestSegmentLength = bestSegment.lastOrNull()?.second
            if (bestSegmentLength == null) {
                Log.w(TAG, "Empty divergent segment found, skipping. This is a logic error.")
                return@forEachIndexed
    /**
     * Selects the longest divergent segment from all available segments.
     */
    private fun selectLongestSegment(segments: List<List<Pair<LatLng, Double>>>): List<Pair<LatLng, Double>> {
        return segments
            .maxByOrNull { segment -> segment.lastOrNull()?.second ?: 0.0 }
            ?: throw IllegalStateException("No segments available when selecting longest segment")
    }
    
            // Step 2c: Find the midpoint of the selected segment
            val segmentMidpoint =
                bestSegment.minByOrNull { abs(it.second - bestSegmentLength / 2.0) }
            
            if (segmentMidpoint == null) {
                Log.w(TAG, "Empty divergent segment found after sorting, skipping. This is a logic error.")
                return@forEachIndexed
            }
    /**
     * Finds the midpoint of a segment based on cumulative distance.
     */
    private fun findSegmentMidpoint(segment: List<Pair<LatLng, Double>>): LatLng {
        val segmentLength = segment.lastOrNull()?.second
            ?: throw IllegalStateException("Empty segment found when finding midpoint")
        
            // Step 2d: Store the placement for this route
            placements.put(routes[index], segmentMidpoint.first)
        }
        val midpoint = segment.minByOrNull { abs(it.second - segmentLength / 2.0) }
            ?: throw IllegalStateException("Empty segment found after sorting when finding midpoint")
        
        return placements
        return midpoint.first
    }

    companion object {
+12 −5
Original line number Diff line number Diff line
@@ -9,6 +9,7 @@ import earth.maps.cardinal.data.ViewportPreferences
import earth.maps.cardinal.data.ViewportRepository
import earth.maps.cardinal.data.room.SavedPlace
import earth.maps.cardinal.geocoding.OfflineGeocodingService
import earth.maps.cardinal.ui.util.AnnotationPlacer
import io.github.dellisd.spatialk.geojson.Position
import io.mockk.coEvery
import io.mockk.every
@@ -64,7 +65,8 @@ class MapViewModelTest {
            locationRepository = mockLocationRepository,
            orientationRepository = mockOrientationRepository,
            offlineGeocodingService = mockOfflineGeocodingService,
            placeDao = mockPlaceDao
            placeDao = mockPlaceDao,
            annotationPlacer = AnnotationPlacer()
        )

        // Initialize screen dimensions
@@ -138,7 +140,8 @@ class MapViewModelTest {
            locationRepository = mockLocationRepository,
            orientationRepository = mockOrientationRepository,
            offlineGeocodingService = mockOfflineGeocodingService,
            placeDao = mockPlaceDao
            placeDao = mockPlaceDao,
            annotationPlacer = AnnotationPlacer()
        )

        assertThat(viewModel.isLocating.first()).isFalse()
@@ -167,7 +170,9 @@ class MapViewModelTest {
            locationRepository = mockLocationRepository,
            orientationRepository = mockOrientationRepository,
            offlineGeocodingService = mockOfflineGeocodingService,
            placeDao = mockPlaceDao
            placeDao = mockPlaceDao,
            annotationPlacer = AnnotationPlacer()

        )

        assertThat(viewModel.locationFlow.first()).isEqualTo(expectedLocation)
@@ -194,7 +199,8 @@ class MapViewModelTest {
            locationRepository = mockLocationRepository,
            orientationRepository = mockOrientationRepository,
            offlineGeocodingService = mockOfflineGeocodingService,
            placeDao = mockPlaceDao
            placeDao = mockPlaceDao,
            annotationPlacer = AnnotationPlacer()
        )

        assertThat(viewModel.heading.first()).isEqualTo(expectedHeading)
@@ -228,7 +234,8 @@ class MapViewModelTest {
            locationRepository = mockLocationRepository,
            orientationRepository = mockOrientationRepository,
            offlineGeocodingService = mockOfflineGeocodingService,
            placeDao = mockPlaceDao
            placeDao = mockPlaceDao,
            annotationPlacer = AnnotationPlacer()
        )

        val featureCollection = viewModel.savedPlacesFlow.first()
+5 −5
Original line number Diff line number Diff line
@@ -84,7 +84,7 @@ class DirectionsViewModelTest {
        coEvery { mockPlanStateRepository.clear() } returns Unit
        coEvery { mockRouteStateRepository.setLoading(any()) } returns Unit
        coEvery { mockPlanStateRepository.setLoading(any()) } returns Unit
        coEvery { mockRouteStateRepository.setRoute(any()) } returns Unit
        coEvery { mockRouteStateRepository.setRoutes(any()) } returns Unit
        coEvery { mockPlanStateRepository.setPlanResponse(any()) } returns Unit
        coEvery { mockRouteStateRepository.setError(any()) } returns Unit
        coEvery { mockPlanStateRepository.setError(any()) } returns Unit
@@ -158,7 +158,7 @@ class DirectionsViewModelTest {
            val initialState = viewModel.routeState.value
            assertEquals(RouteState(), initialState)
            assertFalse(initialState.isLoading)
            assertNull(initialState.route)
            assertTrue(initialState.routes.isEmpty())

            // Trigger the state changes
            viewModel.updateFromPlace(fromPlace)
@@ -169,7 +169,7 @@ class DirectionsViewModelTest {

            // Verify repository interactions
            coVerify { mockRouteStateRepository.setLoading(true) }
            coVerify { mockRouteStateRepository.setRoute(mockRoute) }
            coVerify { mockRouteStateRepository.setRoutes(listOf(mockRoute)) }
        }

    @Test
@@ -287,7 +287,7 @@ class DirectionsViewModelTest {

        // 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.setRoutes(any()) }
        coVerify(exactly = 0) { mockRouteStateRepository.setError(any()) }
    }

@@ -306,7 +306,7 @@ class DirectionsViewModelTest {

        // 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.setRoutes(any()) }
        coVerify(exactly = 0) { mockRouteStateRepository.setError(any()) }
    }
}