From 41c4daa67b6cb25bca84f044993d34d8b4587355 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Thu, 26 Mar 2026 21:37:25 +0530 Subject: [PATCH 1/9] fix(favorites): removing places from the favorites - There was inconsistent place id --- .../java/earth/maps/cardinal/data/LocationRepository.kt | 9 +++++++++ .../java/earth/maps/cardinal/data/room/RecentSearch.kt | 2 +- .../java/earth/maps/cardinal/data/room/SavedPlace.kt | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt index 130fa44..de9178c 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt @@ -37,6 +37,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext +import java.security.MessageDigest import javax.inject.Inject import javax.inject.Singleton @@ -464,7 +465,10 @@ class LocationRepository @Inject constructor( fun createSearchResultPlace(result: GeocodeResult): Place { val openingHours = result.properties["opening_hours"] + val rawIdSource = "${result.displayName}_${"%.5f".format(result.latitude)}_${"%.5f".format(result.longitude)}" + val deterministicId = generateStableId(rawIdSource) return Place( + id = deterministicId, name = result.displayName, description = mapOsmTagsToDescription(result.properties), icon = "search", @@ -476,6 +480,11 @@ class LocationRepository @Inject constructor( openingHours = openingHours, ) } + private fun generateStableId(input: String): String { + return MessageDigest.getInstance("SHA-256") + .digest(input.toByteArray()) + .fold("") { str, it -> str + "%02x".format(it) } + } fun mapOsmTagsToDescription(properties: Map): String { Log.d("LocationRepository", "$properties") diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt index d48bfeb..645fc67 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt @@ -46,7 +46,7 @@ data class RecentSearch( val timestamp = System.currentTimeMillis() return RecentSearch( - id = UUID.randomUUID().toString(), + id = place.id ?: UUID.randomUUID().toString(), name = place.name, description = place.description, icon = place.icon, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt index 6663701..702aadb 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt @@ -55,7 +55,7 @@ data class SavedPlace( val timestamp = System.currentTimeMillis() return SavedPlace( - id = UUID.randomUUID().toString(), + id = place.id ?: UUID.randomUUID().toString(), placeId = 0, customName = null, customDescription = null, -- GitLab From 49367ce62829e351060b3a1b9230dbbe00288629 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Thu, 26 Mar 2026 23:16:26 +0530 Subject: [PATCH 2/9] fix(favorites): removing places from the favorites - AI code review changes --- .../maps/cardinal/data/LocationRepository.kt | 22 ++++++++++++++----- .../maps/cardinal/data/room/RecentSearch.kt | 2 +- .../maps/cardinal/data/room/SavedPlace.kt | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt index de9178c..d9598ea 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt @@ -37,6 +37,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext +import java.nio.ByteBuffer import java.security.MessageDigest import javax.inject.Inject import javax.inject.Singleton @@ -465,8 +466,18 @@ class LocationRepository @Inject constructor( fun createSearchResultPlace(result: GeocodeResult): Place { val openingHours = result.properties["opening_hours"] - val rawIdSource = "${result.displayName}_${"%.5f".format(result.latitude)}_${"%.5f".format(result.longitude)}" - val deterministicId = generateStableId(rawIdSource) + val osmId = result.properties["osm_id"] + val osmType = result.properties["osm_type"] + val deterministicId = if (osmId != null) { + "osm:$osmType:$osmId" + } else { + // 2. Fallback: Use a binary hash of coordinates (Locale Independent) + // We use Long bits to ensure 1.11.111 is different from 11.1.111 + val buffer = ByteBuffer.allocate(16) + buffer.putDouble(result.latitude) + buffer.putDouble(result.longitude) + generateStableIdFromBytes(buffer.array()) + } return Place( id = deterministicId, name = result.displayName, @@ -480,10 +491,11 @@ class LocationRepository @Inject constructor( openingHours = openingHours, ) } - private fun generateStableId(input: String): String { + + private fun generateStableIdFromBytes(bytes: ByteArray): String { return MessageDigest.getInstance("SHA-256") - .digest(input.toByteArray()) - .fold("") { str, it -> str + "%02x".format(it) } + .digest(bytes) + .joinToString("") { "%02x".format(it) } } fun mapOsmTagsToDescription(properties: Map): String { diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt index 645fc67..6317ff2 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt @@ -46,7 +46,7 @@ data class RecentSearch( val timestamp = System.currentTimeMillis() return RecentSearch( - id = place.id ?: UUID.randomUUID().toString(), + id = if (place.id.isNullOrBlank()) UUID.randomUUID().toString() else place.id, name = place.name, description = place.description, icon = place.icon, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt index 702aadb..d786699 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt @@ -55,7 +55,7 @@ data class SavedPlace( val timestamp = System.currentTimeMillis() return SavedPlace( - id = place.id ?: UUID.randomUUID().toString(), + id = if (place.id.isNullOrBlank()) UUID.randomUUID().toString() else place.id, placeId = 0, customName = null, customDescription = null, -- GitLab From 07bd46890dc19008f7346f9b88df8130ff9e4c9b Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Fri, 27 Mar 2026 15:50:27 +0530 Subject: [PATCH 3/9] fix(favorites): removing places from the favorites - AI code review changes 2 - Created new class for placeId generation with Unit Test --- .../maps/cardinal/data/LocationRepository.kt | 20 ++- .../maps/cardinal/data/PlaceIdGenerator.kt | 69 +++++++++++ .../maps/cardinal/data/room/RecentSearch.kt | 10 +- .../maps/cardinal/data/room/SavedPlace.kt | 10 +- .../cardinal/data/PlaceIdGeneratorTest.kt | 114 ++++++++++++++++++ 5 files changed, 205 insertions(+), 18 deletions(-) create mode 100644 cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt create mode 100644 cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt index d9598ea..ad67c02 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt @@ -37,7 +37,6 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext -import java.nio.ByteBuffer import java.security.MessageDigest import javax.inject.Inject import javax.inject.Singleton @@ -466,18 +465,13 @@ class LocationRepository @Inject constructor( fun createSearchResultPlace(result: GeocodeResult): Place { val openingHours = result.properties["opening_hours"] - val osmId = result.properties["osm_id"] - val osmType = result.properties["osm_type"] - val deterministicId = if (osmId != null) { - "osm:$osmType:$osmId" - } else { - // 2. Fallback: Use a binary hash of coordinates (Locale Independent) - // We use Long bits to ensure 1.11.111 is different from 11.1.111 - val buffer = ByteBuffer.allocate(16) - buffer.putDouble(result.latitude) - buffer.putDouble(result.longitude) - generateStableIdFromBytes(buffer.array()) - } + + val deterministicId = PlaceIdGenerator.generateId( + latitude = result.latitude, + longitude = result.longitude, + name = result.displayName + ) + return Place( id = deterministicId, name = result.displayName, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt new file mode 100644 index 0000000..2954951 --- /dev/null +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt @@ -0,0 +1,69 @@ +/* + * Cardinal Maps + * Copyright (C) 2026 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 java.nio.ByteBuffer +import java.security.MessageDigest/** + * A utility for generating deterministic, stable identifiers for geographical places. + * + * This generator uses a combination of coordinates and the place's display name to create + * a SHA-256 hash. Using raw byte buffers for coordinates ensures that the ID remains + * consistent across different device locales (avoiding decimal separator issues like '.' vs ','). + * + * Including the [name] in the generation process prevents ID collisions in scenarios where + * multiple distinct points of interest (POIs) exist at the exact same coordinates. + */ +object PlaceIdGenerator { + /** + * The number of bytes required to store two Double values (Latitude and Longitude). + */ + private const val COORD_BYTE_SIZE = 16 + + /** + * Generates a stable hex ID based on the provided location and name. + * + * @param latitude The latitude of the place. + * @param longitude The longitude of the place. + * @param name The display name or title of the place. + * @return A deterministic SHA-256 hash string in hexadecimal format. + */ + fun generateId(latitude: Double, longitude: Double, name: String): String { + val nameBytes = name.toByteArray(Charsets.UTF_8) + + // Allocate space for coordinates (16 bytes) + name bytes + val buffer = ByteBuffer.allocate(COORD_BYTE_SIZE + nameBytes.size) + buffer.putDouble(latitude) + buffer.putDouble(longitude) + buffer.put(nameBytes) + + return hashBytesToHex(buffer.array()) + } + + /** + * Hashes a byte array using SHA-256 and converts it to a hex string. + * + * Implementation note: Uses [joinToString] for linear-time string construction + * to avoid excessive memory allocations. + */ + private fun hashBytesToHex(bytes: ByteArray): String { + return MessageDigest.getInstance("SHA-256") + .digest(bytes) + .joinToString("") { "%02x".format(it) } + } +} \ No newline at end of file diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt index 6317ff2..ef1b508 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt @@ -21,7 +21,7 @@ package earth.maps.cardinal.data.room import androidx.room.Entity import androidx.room.PrimaryKey import earth.maps.cardinal.data.Place -import java.util.UUID +import earth.maps.cardinal.data.PlaceIdGenerator @Entity(tableName = "recent_searches") data class RecentSearch( @@ -45,8 +45,14 @@ data class RecentSearch( fun fromPlace(place: Place): RecentSearch { val timestamp = System.currentTimeMillis() + val deterministicId = if (place.id.isNullOrBlank()) PlaceIdGenerator.generateId( + latitude = place.latLng.latitude, + longitude = place.latLng.longitude, + name = place.name + ) else place.id + return RecentSearch( - id = if (place.id.isNullOrBlank()) UUID.randomUUID().toString() else place.id, + id = deterministicId, name = place.name, description = place.description, icon = place.icon, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt index d786699..2f273ce 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt @@ -21,7 +21,7 @@ package earth.maps.cardinal.data.room import androidx.room.Entity import androidx.room.PrimaryKey import earth.maps.cardinal.data.Place -import java.util.UUID +import earth.maps.cardinal.data.PlaceIdGenerator @Entity(tableName = "saved_places") data class SavedPlace( @@ -53,9 +53,13 @@ data class SavedPlace( companion object { fun fromPlace(place: Place): SavedPlace { val timestamp = System.currentTimeMillis() - + val deterministicId = if (place.id.isNullOrBlank()) PlaceIdGenerator.generateId( + latitude = place.latLng.latitude, + longitude = place.latLng.longitude, + name = place.name + ) else place.id return SavedPlace( - id = if (place.id.isNullOrBlank()) UUID.randomUUID().toString() else place.id, + id = deterministicId, placeId = 0, customName = null, customDescription = null, diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt new file mode 100644 index 0000000..7de72da --- /dev/null +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt @@ -0,0 +1,114 @@ +/* + * Cardinal Maps + * Copyright (C) 2026 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 org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Test +import java.util.Locale + +class PlaceIdGeneratorTest { + + @Test + fun `generateId is deterministic for the same input`() { + val lat = 19.1136 + val lon = 72.8697 + val name = "JB Nagar" + + val id1 = PlaceIdGenerator.generateId(lat, lon, name) + val id2 = PlaceIdGenerator.generateId(lat, lon, name) + + assertEquals("Same input must produce the same ID", id1, id2) + } + + @Test + fun `generateId produces different IDs for different names at same coordinates`() { + val lat = 19.1136 + val lon = 72.8697 + + val id1 = PlaceIdGenerator.generateId(lat, lon, "Coffee Shop") + val id2 = PlaceIdGenerator.generateId(lat, lon, "Law Firm") + + assertNotEquals("Different names at same coordinates must produce different IDs", id1, id2) + } + + @Test + fun `generateId produces different IDs for different coordinates with same name`() { + val name = "Starbucks" + + val id1 = PlaceIdGenerator.generateId(19.1136, 72.8697, name) + val id2 = PlaceIdGenerator.generateId(18.9218, 72.8347, name) + + assertNotEquals("Same name at different coordinates must produce different IDs", id1, id2) + } + + @Test + fun `generateId is locale independent`() { + val lat = 19.1136 + val lon = 72.8697 + val name = "JB Nagar" + + // Store original locale + val originalLocale = Locale.getDefault() + + try { + // Set locale to US (uses '.' as decimal separator) + Locale.setDefault(Locale.US) + val idUS = PlaceIdGenerator.generateId(lat, lon, name) + + // Set locale to GERMANY (uses ',' as decimal separator) + Locale.setDefault(Locale.GERMANY) + val idGermany = PlaceIdGenerator.generateId(lat, lon, name) + + assertEquals( + "ID must be identical regardless of system decimal separators", + idUS, + idGermany + ) + } finally { + // Restore locale to avoid affecting other tests + Locale.setDefault(originalLocale) + } + } + + @Test + fun `generateId handles special characters and emojis in name`() { + val lat = 0.0 + val lon = 0.0 + val name1 = "Müchen Café ☕" + val name2 = "Müchen Café ☕" + + val id1 = PlaceIdGenerator.generateId(lat, lon, name1) + val id2 = PlaceIdGenerator.generateId(lat, lon, name2) + + assertEquals("Special characters and emojis should be handled deterministically", id1, id2) + } + + @Test + fun `generateId produces a valid 64 character SHA-256 hex string`() { + val id = PlaceIdGenerator.generateId(1.0, 1.0, "Test") + + // SHA-256 produces 32 bytes, which is 64 hex characters + assertEquals("ID length should be 64 characters", 64, id.length) + + // Verify it only contains hex characters + val hexRegex = Regex("^[0-9a-fA-F]+$") + assert(id.matches(hexRegex)) { "ID should be a valid hexadecimal string" } + } +} \ No newline at end of file -- GitLab From 88ddd2817f321a9493f9c06bee237e80b43d3f83 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Fri, 27 Mar 2026 21:57:27 +0530 Subject: [PATCH 4/9] fix(favorites): removing places from the favorites - Ferrostar Update --- cardinal-android/gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cardinal-android/gradle/libs.versions.toml b/cardinal-android/gradle/libs.versions.toml index 0693e64..eb7e509 100644 --- a/cardinal-android/gradle/libs.versions.toml +++ b/cardinal-android/gradle/libs.versions.toml @@ -26,7 +26,7 @@ hiltNavigationCompose = "1.3.0" cargo-ndk = "0.3.4" valhallaMobile = "0.3.0" valhallaMobileConfig = "0.0.9" -ferrostar = "0.45.0" +ferrostar = "0.48.1" okhttp3 = "5.3.2" material3 = "1.5.0-alpha10" detekt = "2.0.0-alpha.1" -- GitLab From cb499a716ec80d9f0e3fe87f524e6ff586f99e28 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Mon, 30 Mar 2026 23:22:33 +0530 Subject: [PATCH 5/9] fix(favorites): removing places from the favorites - Using gId for online place search and using foll back option for offline --- .../earth/maps/cardinal/data/GeocodeResult.kt | 1 + .../maps/cardinal/data/LocationRepository.kt | 15 +-------------- .../cardinal/geocoding/OfflineGeocodingService.kt | 8 +++++++- .../cardinal/geocoding/PeliasGeocodingService.kt | 1 + cardinal-android/gradle/libs.versions.toml | 2 +- 5 files changed, 11 insertions(+), 16 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/GeocodeResult.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/GeocodeResult.kt index 8d2b188..5808677 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/GeocodeResult.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/GeocodeResult.kt @@ -21,6 +21,7 @@ package earth.maps.cardinal.data import kotlin.math.abs data class GeocodeResult( + val gId: String, val latitude: Double, val longitude: Double, val displayName: String, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt index ad67c02..5f0a006 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt @@ -37,7 +37,6 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext -import java.security.MessageDigest import javax.inject.Inject import javax.inject.Singleton @@ -466,14 +465,8 @@ class LocationRepository @Inject constructor( fun createSearchResultPlace(result: GeocodeResult): Place { val openingHours = result.properties["opening_hours"] - val deterministicId = PlaceIdGenerator.generateId( - latitude = result.latitude, - longitude = result.longitude, - name = result.displayName - ) - return Place( - id = deterministicId, + id = result.gId, name = result.displayName, description = mapOsmTagsToDescription(result.properties), icon = "search", @@ -486,12 +479,6 @@ class LocationRepository @Inject constructor( ) } - private fun generateStableIdFromBytes(bytes: ByteArray): String { - return MessageDigest.getInstance("SHA-256") - .digest(bytes) - .joinToString("") { "%02x".format(it) } - } - fun mapOsmTagsToDescription(properties: Map): String { Log.d("LocationRepository", "$properties") // Check for common amenity tags first diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt index b72b88b..680920e 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt @@ -25,6 +25,7 @@ import earth.maps.cardinal.data.Address import earth.maps.cardinal.data.GeocodeResult import earth.maps.cardinal.data.LatLng import earth.maps.cardinal.data.LocationRepository +import earth.maps.cardinal.data.PlaceIdGenerator import uniffi.cardinal_geocoder.newAirmailIndex import java.io.File @@ -98,8 +99,13 @@ class OfflineGeocodingService( // Populate address from available tags val address = buildAddress(tags) - + val id = PlaceIdGenerator.generateId( + latitude = latitude, + longitude = longitude, + name = displayName, + ) return GeocodeResult( + gId = id, displayName = displayName, latitude = latitude, longitude = longitude, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt index aa56172..2ef77e5 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt @@ -192,6 +192,7 @@ class PeliasGeocodingService( } GeocodeResult( + gId = properties?.get("gid")?.jsonPrimitive?.content ?: "", latitude = lat, longitude = lon, displayName = displayName, diff --git a/cardinal-android/gradle/libs.versions.toml b/cardinal-android/gradle/libs.versions.toml index eb7e509..0693e64 100644 --- a/cardinal-android/gradle/libs.versions.toml +++ b/cardinal-android/gradle/libs.versions.toml @@ -26,7 +26,7 @@ hiltNavigationCompose = "1.3.0" cargo-ndk = "0.3.4" valhallaMobile = "0.3.0" valhallaMobileConfig = "0.0.9" -ferrostar = "0.48.1" +ferrostar = "0.45.0" okhttp3 = "5.3.2" material3 = "1.5.0-alpha10" detekt = "2.0.0-alpha.1" -- GitLab From 0d5d8e2cb71ff0aa0ebad75d26b63e3c2f421722 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Tue, 31 Mar 2026 13:37:25 +0530 Subject: [PATCH 6/9] fix(favorites): removing places from the favorites - Using gId for online place search and using foll back option for offline - Code review changes --- .../main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt | 6 ++++-- .../maps/cardinal/geocoding/PeliasGeocodingService.kt | 7 ++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt index 2954951..e9d293b 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt @@ -19,8 +19,10 @@ package earth.maps.cardinal.data import java.nio.ByteBuffer -import java.security.MessageDigest/** - * A utility for generating deterministic, stable identifiers for geographical places. +import java.security.MessageDigest + +/** + * * A utility for generating deterministic, stable identifiers for geographical places. * * This generator uses a combination of coordinates and the place's display name to create * a SHA-256 hash. Using raw byte buffers for coordinates ensures that the ID remains diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt index 2ef77e5..4d62779 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt @@ -24,6 +24,7 @@ import earth.maps.cardinal.data.AppPreferenceRepository import earth.maps.cardinal.data.GeocodeResult import earth.maps.cardinal.data.LatLng import earth.maps.cardinal.data.LocationRepository +import earth.maps.cardinal.data.PlaceIdGenerator import io.ktor.client.HttpClient import io.ktor.client.call.body import io.ktor.client.engine.android.Android @@ -192,7 +193,11 @@ class PeliasGeocodingService( } GeocodeResult( - gId = properties?.get("gid")?.jsonPrimitive?.content ?: "", + gId = properties?.get("gid")?.jsonPrimitive?.content ?: PlaceIdGenerator.generateId( + latitude = lat, + longitude = lon, + name = displayName + ), latitude = lat, longitude = lon, displayName = displayName, -- GitLab From 78729699397a00c498681b24e1c7f17840d0a385 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Tue, 31 Mar 2026 17:12:43 +0530 Subject: [PATCH 7/9] fix(favorites): removing places from the favorites - Using geocodeId for online place search and using foll back option for offline - Code review changes --- .../earth/maps/cardinal/data/GeocodeResult.kt | 2 +- .../maps/cardinal/data/LocationRepository.kt | 2 +- .../maps/cardinal/data/PlaceIdGenerator.kt | 20 ++++- .../maps/cardinal/data/room/RecentSearch.kt | 8 +- .../maps/cardinal/data/room/SavedPlace.kt | 7 +- .../geocoding/OfflineGeocodingService.kt | 2 +- .../geocoding/PeliasGeocodingService.kt | 2 +- .../cardinal/data/PlaceIdGeneratorTest.kt | 85 +++++++++++++++++++ 8 files changed, 110 insertions(+), 18 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/GeocodeResult.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/GeocodeResult.kt index 5808677..77ae2da 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/GeocodeResult.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/GeocodeResult.kt @@ -21,7 +21,7 @@ package earth.maps.cardinal.data import kotlin.math.abs data class GeocodeResult( - val gId: String, + val geocodeId: String, val latitude: Double, val longitude: Double, val displayName: String, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt index 5f0a006..12a71bb 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt @@ -466,7 +466,7 @@ class LocationRepository @Inject constructor( val openingHours = result.properties["opening_hours"] return Place( - id = result.gId, + id = result.geocodeId, name = result.displayName, description = mapOsmTagsToDescription(result.properties), icon = "search", diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt index e9d293b..49143b3 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/PlaceIdGenerator.kt @@ -22,7 +22,7 @@ import java.nio.ByteBuffer import java.security.MessageDigest /** - * * A utility for generating deterministic, stable identifiers for geographical places. + * A utility for generating deterministic, stable identifiers for geographical places. * * This generator uses a combination of coordinates and the place's display name to create * a SHA-256 hash. Using raw byte buffers for coordinates ensures that the ID remains @@ -57,6 +57,24 @@ object PlaceIdGenerator { return hashBytesToHex(buffer.array()) } + /** + * Returns a stable identifier for the given [place]. + * + * If the [place] already contains a valid ID (e.g., from an online provider like Pelias), + * that ID is returned. If the ID is null or blank (common with offline geocoding results), + * a deterministic ID is generated based on the place's coordinates and name. + * + * @param place The place object to get or generate an ID for. + * @return The existing ID if present, otherwise a generated SHA-256 hex string. + */ + fun generateId(place: Place): String { + return if (place.id.isNullOrBlank()) generateId( + latitude = place.latLng.latitude, + longitude = place.latLng.longitude, + name = place.name + ) else place.id + } + /** * Hashes a byte array using SHA-256 and converts it to a hex string. * diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt index ef1b508..990aaf1 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/RecentSearch.kt @@ -45,14 +45,8 @@ data class RecentSearch( fun fromPlace(place: Place): RecentSearch { val timestamp = System.currentTimeMillis() - val deterministicId = if (place.id.isNullOrBlank()) PlaceIdGenerator.generateId( - latitude = place.latLng.latitude, - longitude = place.latLng.longitude, - name = place.name - ) else place.id - return RecentSearch( - id = deterministicId, + id = PlaceIdGenerator.generateId(place), name = place.name, description = place.description, icon = place.icon, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt index 2f273ce..d48ea6a 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt @@ -53,13 +53,8 @@ data class SavedPlace( companion object { fun fromPlace(place: Place): SavedPlace { val timestamp = System.currentTimeMillis() - val deterministicId = if (place.id.isNullOrBlank()) PlaceIdGenerator.generateId( - latitude = place.latLng.latitude, - longitude = place.latLng.longitude, - name = place.name - ) else place.id return SavedPlace( - id = deterministicId, + id = PlaceIdGenerator.generateId(place), placeId = 0, customName = null, customDescription = null, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt index 680920e..d4812ef 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/OfflineGeocodingService.kt @@ -105,7 +105,7 @@ class OfflineGeocodingService( name = displayName, ) return GeocodeResult( - gId = id, + geocodeId = id, displayName = displayName, latitude = latitude, longitude = longitude, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt index 4d62779..3fb11a5 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt @@ -193,7 +193,7 @@ class PeliasGeocodingService( } GeocodeResult( - gId = properties?.get("gid")?.jsonPrimitive?.content ?: PlaceIdGenerator.generateId( + geocodeId = properties?.get("gid")?.jsonPrimitive?.content ?: PlaceIdGenerator.generateId( latitude = lat, longitude = lon, name = displayName diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt index 7de72da..0ebdb7a 100644 --- a/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt @@ -18,6 +18,8 @@ package earth.maps.cardinal.data +import io.mockk.every +import io.mockk.mockk import org.junit.Assert.assertEquals import org.junit.Assert.assertNotEquals import org.junit.Test @@ -111,4 +113,87 @@ class PlaceIdGeneratorTest { val hexRegex = Regex("^[0-9a-fA-F]+$") assert(id.matches(hexRegex)) { "ID should be a valid hexadecimal string" } } + + @Test + fun `generateId with existing ID returns the original ID`() { + val existingId = "osm:way:12345" + val place = mockk { + every { id } returns existingId + } + + val result = PlaceIdGenerator.generateId(place) + + assertEquals("Should return the ID already present in the Place object", existingId, result) + } + + @Test + fun `generateId with null or blank ID generates a deterministic hash`() { + val latitude = 45.523062 + val longitude = -122.676482 + val placeName = "Pioneer Courthouse Square" + + val place = mockk { + every { id } returns "" // or null + every { latLng } returns LatLng(latitude, longitude) + every { name } returns placeName + } + + val result = PlaceIdGenerator.generateId(place) + + // Calculate expected hash using the existing primitive function to ensure parity + val expected = PlaceIdGenerator.generateId(latitude, longitude, placeName) + + assertEquals("Generated ID should match the hash of coordinates and name", expected, result) + assertNotEquals("Result should not be blank", "", result) + } + + @Test + fun `generateId is stable for identical offline places`() { + val lat = 52.5200 + val lon = 13.4050 + val placeName = "Berlin TV Tower" + + val place1 = mockk { + every { id } returns null + every { latLng } returns LatLng(lat, lon) + every { name } returns placeName + } + + val place2 = mockk { + every { id } returns "" + every { latLng } returns LatLng(lat, lon) + every { name } returns placeName + } + + assertEquals( + "Two different place objects with same data should produce the same ID", + PlaceIdGenerator.generateId(place1), + PlaceIdGenerator.generateId(place2) + ) + } + + @Test + fun `generateId differs when coordinates or name change`() { + val lat = 40.7128 + val lon = -74.0060 + + val placeA = mockk { + every { id } returns null + every { latLng } returns LatLng(lat, lon) + every { name } returns "Location A" + } + + val placeB = mockk { + every { id } returns null + every { latLng } returns LatLng(lat, lon) + every { name } returns "Location B" + } + + assertNotEquals( + "Places at the same coordinates but with different names should have different IDs", + PlaceIdGenerator.generateId(placeA), + PlaceIdGenerator.generateId(placeB) + ) + } + } \ No newline at end of file -- GitLab From 6116723483536dff47907f540b78f12fe20c900d Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Wed, 1 Apr 2026 14:11:26 +0530 Subject: [PATCH 8/9] fix(remove favourite): Code review update --- .../earth/maps/cardinal/geocoding/PeliasGeocodingService.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt index 3fb11a5..5b8030b 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/geocoding/PeliasGeocodingService.kt @@ -37,6 +37,7 @@ import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.contentOrNull import kotlinx.serialization.json.doubleOrNull import kotlinx.serialization.json.jsonArray import kotlinx.serialization.json.jsonObject @@ -193,7 +194,7 @@ class PeliasGeocodingService( } GeocodeResult( - geocodeId = properties?.get("gid")?.jsonPrimitive?.content ?: PlaceIdGenerator.generateId( + geocodeId = properties?.get("gid")?.jsonPrimitive?.contentOrNull ?: PlaceIdGenerator.generateId( latitude = lat, longitude = lon, name = displayName -- GitLab From 66fbd88c4440ec1e864ea33b9e2238789816d2b8 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Wed, 1 Apr 2026 14:32:37 +0530 Subject: [PATCH 9/9] fix(test): Code review update --- .../test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt index 0ebdb7a..f1a7ce4 100644 --- a/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/data/PlaceIdGeneratorTest.kt @@ -22,6 +22,7 @@ import io.mockk.every import io.mockk.mockk import org.junit.Assert.assertEquals import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue import org.junit.Test import java.util.Locale @@ -111,7 +112,7 @@ class PlaceIdGeneratorTest { // Verify it only contains hex characters val hexRegex = Regex("^[0-9a-fA-F]+$") - assert(id.matches(hexRegex)) { "ID should be a valid hexadecimal string" } + assertTrue(id.matches(hexRegex)) } @Test -- GitLab