diff --git a/bitchat/Services/LocationStateManager.swift b/bitchat/Services/LocationStateManager.swift index 5f6ac61a..209fab6c 100644 --- a/bitchat/Services/LocationStateManager.swift +++ b/bitchat/Services/LocationStateManager.swift @@ -109,6 +109,8 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl private let teleportedStoreKey = "locationChannel.teleportedSet" private let bookmarksKey = "locationChannel.bookmarks" private let bookmarkNamesKey = "locationChannel.bookmarkNames" + private let bookmarkNamesSchemaVersionKey = "locationChannel.bookmarkNamesSchemaVersion" + private let bookmarkNamesCurrentSchemaVersion = 1 // MARK: - Published State (Channel) @@ -222,6 +224,26 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl let dict = try? JSONDecoder().decode([String: String].self, from: data) { bookmarkNames = dict } + + migrateBookmarkNamesIfNeeded() + } + + /// Drops names cached before low-precision geohashes became country-first. + /// Correctly resolved names written after this migration must survive + /// subsequent launches, so the migration is explicitly versioned. + private func migrateBookmarkNamesIfNeeded() { + guard storage.integer(forKey: bookmarkNamesSchemaVersionKey) < bookmarkNamesCurrentSchemaVersion else { + return + } + + let retainedNames = bookmarkNames.filter { + Self.normalizeGeohash($0.key).count > 2 + } + if retainedNames.count != bookmarkNames.count { + bookmarkNames = retainedNames + persistBookmarkNames() + } + storage.set(bookmarkNamesCurrentSchemaVersion, forKey: bookmarkNamesSchemaVersionKey) } private func initializePermissionState() { @@ -525,15 +547,15 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl } private func resolveCompositeAdminName(geohash gh: String, points: [CLLocation]) { - var uniqueAdmins: [String] = [] - var seenAdmins = Set() + var uniqueRegions: [String] = [] + var seenRegions = Set() var idx = 0 func step() { if idx >= points.count { let finalName: String? = { - if uniqueAdmins.count >= 2 { return uniqueAdmins[0] + " and " + uniqueAdmins[1] } - return uniqueAdmins.first + if uniqueRegions.count >= 2 { return uniqueRegions[0] + " and " + uniqueRegions[1] } + return uniqueRegions.first }() if let finalName = finalName, !finalName.isEmpty { DispatchQueue.main.async { @@ -549,12 +571,16 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl geocoder.reverseGeocodeLocation(loc) { [weak self] placemarks, _ in guard self != nil else { return } if let pm = placemarks?.first { - if let admin = pm.administrativeArea, !admin.isEmpty, !seenAdmins.contains(admin) { - seenAdmins.insert(admin) - uniqueAdmins.append(admin) - } else if let country = pm.country, !country.isEmpty, !seenAdmins.contains(country) { - seenAdmins.insert(country) - uniqueAdmins.append(country) + if let country = pm.country, !country.isEmpty { + if !seenRegions.contains(country) { + seenRegions.insert(country) + uniqueRegions.append(country) + } + } else if let admin = pm.administrativeArea, + !admin.isEmpty, + !seenRegions.contains(admin) { + seenRegions.insert(admin) + uniqueRegions.append(admin) } } step() @@ -566,7 +592,7 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl private static func nameForGeohashLength(_ len: Int, from pm: CLPlacemark) -> String? { switch len { case 0...2: - return pm.administrativeArea ?? pm.country + return pm.country ?? pm.administrativeArea case 3...4: return pm.administrativeArea ?? pm.subAdministrativeArea ?? pm.country case 5: diff --git a/bitchatTests/Services/LocationStateManagerTests.swift b/bitchatTests/Services/LocationStateManagerTests.swift index 46edcd6a..69515f76 100644 --- a/bitchatTests/Services/LocationStateManagerTests.swift +++ b/bitchatTests/Services/LocationStateManagerTests.swift @@ -207,7 +207,54 @@ final class LocationStateManagerTests: XCTestCase { XCTAssertFalse(reloaded.teleported) } - func test_addBookmark_lowPrecisionResolvesCompositeAdminName() async { + func test_loadPersistedState_migratesLowPrecisionBookmarkNamesOnce() throws { + let storage = makeStorage() + let staleNames = [ + "gc": "England", + "u3": "Île-de-France", + "u4pr": "Paris", + "u4pruy": "Le Marais" + ] + storage.set(try JSONEncoder().encode(staleNames), forKey: "locationChannel.bookmarkNames") + + let migrated = LocationStateManager( + storage: storage, + locationManager: MockLocationManager(authorizationStatus: .denied), + geocoder: MockLocationGeocoder(), + shouldInitializeCoreLocation: false + ) + + XCTAssertNil(migrated.bookmarkNames["gc"]) + XCTAssertNil(migrated.bookmarkNames["u3"]) + XCTAssertEqual(migrated.bookmarkNames["u4pr"], "Paris") + XCTAssertEqual(migrated.bookmarkNames["u4pruy"], "Le Marais") + XCTAssertEqual(storage.integer(forKey: "locationChannel.bookmarkNamesSchemaVersion"), 1) + + let persistedData = try XCTUnwrap(storage.data(forKey: "locationChannel.bookmarkNames")) + let persistedNames = try JSONDecoder().decode([String: String].self, from: persistedData) + XCTAssertEqual(persistedNames, [ + "u4pr": "Paris", + "u4pruy": "Le Marais" + ]) + + let correctedNames = [ + "gc": "United Kingdom", + "u4pr": "Paris" + ] + storage.set(try JSONEncoder().encode(correctedNames), forKey: "locationChannel.bookmarkNames") + + let reloaded = LocationStateManager( + storage: storage, + locationManager: MockLocationManager(authorizationStatus: .denied), + geocoder: MockLocationGeocoder(), + shouldInitializeCoreLocation: false + ) + + XCTAssertEqual(reloaded.bookmarkNames["gc"], "United Kingdom") + XCTAssertEqual(reloaded.bookmarkNames["u4pr"], "Paris") + } + + func test_addBookmark_lowPrecisionPrefersCountryOverAdministrativeAreas() async { let geocoder = MockLocationGeocoder() geocoder.enqueue(placemarks: [makePlacemark(country: "United States", administrativeArea: "California")]) geocoder.enqueue(placemarks: [makePlacemark(country: "United States", administrativeArea: "Nevada")]) @@ -223,12 +270,50 @@ final class LocationStateManagerTests: XCTestCase { manager.addBookmark("9q") - let bookmarkResolved = await waitUntil { manager.bookmarkNames["9q"] == "California and Nevada" } + let bookmarkResolved = await waitUntil { manager.bookmarkNames["9q"] == "United States" } XCTAssertTrue(bookmarkResolved) XCTAssertEqual(geocoder.reverseRequests.count, 5) XCTAssertEqual(manager.bookmarks, ["9q"]) } + func test_addBookmark_lowPrecisionFallsBackToDistinctAdministrativeAreas() async { + let geocoder = MockLocationGeocoder() + geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "California")]) + geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "Nevada")]) + geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "California")]) + geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "Arizona")]) + geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "Nevada")]) + let manager = LocationStateManager( + storage: makeStorage(), + locationManager: MockLocationManager(authorizationStatus: .denied), + geocoder: geocoder, + shouldInitializeCoreLocation: false + ) + + manager.addBookmark("9q") + + let bookmarkResolved = await waitUntil { manager.bookmarkNames["9q"] == "California and Nevada" } + XCTAssertTrue(bookmarkResolved) + XCTAssertEqual(geocoder.reverseRequests.count, 5) + } + + func test_addBookmark_higherPrecisionStillPrefersAdministrativeArea() async { + let geocoder = MockLocationGeocoder() + geocoder.enqueue(placemarks: [makePlacemark(country: "United States", administrativeArea: "California")]) + let manager = LocationStateManager( + storage: makeStorage(), + locationManager: MockLocationManager(authorizationStatus: .denied), + geocoder: geocoder, + shouldInitializeCoreLocation: false + ) + + manager.addBookmark("9q8") + + let bookmarkResolved = await waitUntil { manager.bookmarkNames["9q8"] == "California" } + XCTAssertTrue(bookmarkResolved) + XCTAssertEqual(geocoder.reverseRequests.count, 1) + } + private func makeStorage() -> UserDefaults { let suiteName = "LocationStateManagerTests-\(UUID().uuidString)" let storage = UserDefaults(suiteName: suiteName)!