From afe1a7932bb80ba6c2b252709e7a5bc0deed37cf Mon Sep 17 00:00:00 2001 From: Tymofiy Bortnyk Date: Mon, 9 Mar 2026 21:58:47 +0200 Subject: [PATCH] Fix SharedPreferences key/type mismatches and upgrade safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Read saved_* (Long, user intent) instead of cached_* (Float) in WeatherFetcher, with fallback to cached_* for upgrade compatibility - Decode home_widget doubles correctly: getLong() + Double.fromBits() - Persist GPS coordinates in _loadWeather for background refresh - Add race condition check: discard stale fetch if location changed - Migrate location_source → saved_location_source, remove use_gps - Clear stale city when saveLocation called without city - Skip redundant writes when coordinates unchanged - Remove dead getSavedLocationFromWidget() method - Document SharedPreferences keys and upgrade safety pattern Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 16 ++- .../org/bortnik/meteogram/WeatherFetcher.kt | 47 +++++-- .../bortnik/meteogram/WeatherFetcherTest.kt | 120 ++++++++++++++++-- docs/ai/architecture.md | 39 ++++++ docs/ai/widget.md | 33 ++--- lib/main.dart | 2 + lib/screens/home_screen.dart | 9 ++ lib/services/location_service.dart | 105 ++++++++------- test/location_service_test.dart | 83 +++++++++++- 9 files changed, 366 insertions(+), 88 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index dc8842c..9151270 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,8 +129,8 @@ Android widgets use RemoteViews which only support: **NOT supported:** View, Space, custom views, most Material widgets ### Data Flow -1. **In-app**: `home_screen.dart` gets location → calls `NativeSvgService.fetchWeather()` → Kotlin fetches from Open-Meteo → caches to SharedPreferences → Dart reads cache → Kotlin generates SVG → rendered via `NativeSvgChartView` -2. **Widget**: Native code reads cached weather from SharedPreferences → `SvgChartGenerator.kt` generates SVG → AndroidSVG renders to bitmap → ImageView +1. **In-app**: `home_screen.dart` gets location → saves coordinates to SharedPreferences via `home_widget` (`saved_latitude`/`saved_longitude` as Long-encoded doubles) → calls `NativeSvgService.fetchWeather()` → Kotlin fetches from Open-Meteo → caches weather JSON to SharedPreferences → Dart reads cache → Kotlin generates SVG → rendered via `NativeSvgChartView` +2. **Widget**: Native code reads `saved_latitude`/`saved_longitude` from SharedPreferences (Long-encoded doubles from home_widget) → `WeatherFetcher.kt` fetches from Open-Meteo → caches weather JSON → `SvgChartGenerator.kt` generates SVG → AndroidSVG renders to bitmap → ImageView ### Background Refresh (fully native) - **AlarmManager**: `WidgetAlarmScheduler.kt` schedules 15-min inexact alarm (catches up on wake) @@ -238,6 +238,18 @@ adb logcat | grep -i "Error inflating" | `lib/theme/app_theme.dart` | All colors and gradients | | `scripts/generate_version.sh` | Generates version.dart from git tag/commit | +## Upgrade Safety + +**Renaming SharedPreferences keys requires migration.** Users have data stored under old key names that must not be lost or misinterpreted on upgrade. When renaming a key: + +1. Add migration code in `LocationService.migrateIfNeeded()` (called from `main.dart` at startup) +2. Read old key → write new key → remove old key +3. Migrate both `SharedPreferences` and `HomeWidget` storage (they're separate stores) +4. Keep the legacy key constant in code for reference +5. Migration must be idempotent (safe to run multiple times) + +See `location_source` → `saved_location_source` migration as the reference pattern. + ## Gotchas 1. **RemoteViews errors** - Check logcat for "Class not allowed to be inflated" diff --git a/android/app/src/main/kotlin/org/bortnik/meteogram/WeatherFetcher.kt b/android/app/src/main/kotlin/org/bortnik/meteogram/WeatherFetcher.kt index 65bc8ea..3b74b35 100644 --- a/android/app/src/main/kotlin/org/bortnik/meteogram/WeatherFetcher.kt +++ b/android/app/src/main/kotlin/org/bortnik/meteogram/WeatherFetcher.kt @@ -1,6 +1,7 @@ package org.bortnik.meteogram import android.content.Context +import android.content.SharedPreferences import android.util.Log import org.json.JSONArray import org.json.JSONObject @@ -22,22 +23,45 @@ object WeatherFetcher { private const val TIMEOUT_MS = 10_000 private const val PAST_HOURS = 6 + /** + * Read user's saved location from SharedPreferences. + * Dart writes these via home_widget, which stores doubles as Long + * via Double.doubleToRawLongBits(). + * @return (latitude, longitude) pair, or null if no location is saved. + */ + fun getLocation(prefs: SharedPreferences): Pair? { + if (prefs.contains("saved_latitude") && prefs.contains("saved_longitude")) { + val latitude = Double.fromBits(prefs.getLong("saved_latitude", 0L)) + val longitude = Double.fromBits(prefs.getLong("saved_longitude", 0L)) + return Pair(latitude, longitude) + } + + // Upgrade fallback: pre-v1.0.9 wrote cached_* as Float from previous fetches. + // Bridges GPS users who haven't opened the app since upgrade. + if (prefs.contains("cached_latitude") && prefs.contains("cached_longitude")) { + val latitude = prefs.getFloat("cached_latitude", 0f).toDouble() + val longitude = prefs.getFloat("cached_longitude", 0f).toDouble() + Log.d(TAG, "Using cached_* fallback for upgrade compatibility") + return Pair(latitude, longitude) + } + + return null + } + /** * Fetch weather data synchronously and save to SharedPreferences. - * Uses cached location from SharedPreferences. + * Uses saved location from SharedPreferences. * Call from background thread only. */ fun fetchAndUpdateSync(context: Context) { val prefs = context.getSharedPreferences(WidgetUtils.PREFS_NAME, Context.MODE_PRIVATE) - // Get cached location - val latitude = prefs.getFloat("cached_latitude", 0f).toDouble() - val longitude = prefs.getFloat("cached_longitude", 0f).toDouble() - - if (latitude == 0.0 && longitude == 0.0) { - Log.w(TAG, "No cached location available") + val location = getLocation(prefs) + if (location == null) { + Log.w(TAG, "No saved location available") return } + val (latitude, longitude) = location if (fetchWeatherSync(context, latitude, longitude)) { // Trigger widget update @@ -67,9 +91,16 @@ object WeatherFetcher { return false } + // Check if user changed location while we were fetching + val prefs = context.getSharedPreferences(WidgetUtils.PREFS_NAME, Context.MODE_PRIVATE) + val currentLocation = getLocation(prefs) + if (currentLocation != null && (currentLocation.first != latitude || currentLocation.second != longitude)) { + Log.d(TAG, "Location changed during fetch, discarding stale result") + return false + } + // Save to SharedPreferences // Note: Values read by Dart via home_widget must be stored as strings - val prefs = context.getSharedPreferences(WidgetUtils.PREFS_NAME, Context.MODE_PRIVATE) val now = System.currentTimeMillis() // Extract current temperature using same logic as Dart's getNowIndex() diff --git a/android/app/src/test/kotlin/org/bortnik/meteogram/WeatherFetcherTest.kt b/android/app/src/test/kotlin/org/bortnik/meteogram/WeatherFetcherTest.kt index 6898efa..b26bf9a 100644 --- a/android/app/src/test/kotlin/org/bortnik/meteogram/WeatherFetcherTest.kt +++ b/android/app/src/test/kotlin/org/bortnik/meteogram/WeatherFetcherTest.kt @@ -249,12 +249,8 @@ class WeatherFetcherTest { // ==================== fetchAndUpdateSync Tests ==================== @Test - fun `fetchAndUpdateSync returns early without cached location`() { - // No location cached - should return without crashing + fun `fetchAndUpdateSync returns early without saved location`() { prefs.edit().clear().commit() - - // This will log "No cached location available" and return - // We can't easily verify the return since it's void, but it shouldn't crash try { WeatherFetcher.fetchAndUpdateSync(context) } catch (e: Exception) { @@ -263,13 +259,16 @@ class WeatherFetcherTest { } @Test - fun `fetchAndUpdateSync returns early with zero coordinates`() { - // 0,0 location is treated as "no location" + fun `fetchAndUpdateSync uses cached Float coordinates as upgrade fallback`() { + // Pre-v1.0.9 wrote cached_* as Float; getLocation() falls back to these + // when saved_* (Long) keys don't exist yet prefs.edit() - .putFloat("cached_latitude", 0f) - .putFloat("cached_longitude", 0f) + .putFloat("cached_latitude", 52.52f) + .putFloat("cached_longitude", 13.405f) .commit() + // Should not crash — getLocation() reads cached_* as fallback, + // then fetchWeatherSync attempts HTTP (fails silently in test) try { WeatherFetcher.fetchAndUpdateSync(context) } catch (e: Exception) { @@ -277,6 +276,109 @@ class WeatherFetcherTest { } } + // ==================== getLocation Tests ==================== + + @Test + fun `getLocation returns null when no location keys exist`() { + prefs.edit().clear().commit() + assertNull(WeatherFetcher.getLocation(prefs)) + } + + @Test + fun `getLocation reads saved coordinates from home_widget format`() { + // home_widget stores doubles as Long via Double.doubleToRawLongBits() + val lat = 52.52 + val lon = 13.405 + prefs.edit() + .putLong("saved_latitude", lat.toRawBits()) + .putLong("saved_longitude", lon.toRawBits()) + .commit() + + val location = WeatherFetcher.getLocation(prefs) + assertNotNull(location) + assertEquals(lat, location!!.first, 0.0001) + assertEquals(lon, location.second, 0.0001) + } + + @Test + fun `getLocation handles zero coordinates as valid location`() { + val lat = 0.0 + val lon = 0.0 + prefs.edit() + .putLong("saved_latitude", lat.toRawBits()) + .putLong("saved_longitude", lon.toRawBits()) + .commit() + + val location = WeatherFetcher.getLocation(prefs) + assertNotNull(location) + assertEquals(lat, location!!.first, 0.0001) + assertEquals(lon, location.second, 0.0001) + } + + @Test + fun `getLocation handles negative coordinates`() { + val lat = -33.87 + val lon = -151.21 + prefs.edit() + .putLong("saved_latitude", lat.toRawBits()) + .putLong("saved_longitude", lon.toRawBits()) + .commit() + + val location = WeatherFetcher.getLocation(prefs) + assertNotNull(location) + assertEquals(lat, location!!.first, 0.0001) + assertEquals(lon, location.second, 0.0001) + } + + @Test + fun `getLocation returns null when only latitude is saved`() { + prefs.edit() + .putLong("saved_latitude", 52.52.toRawBits()) + .commit() + + assertNull(WeatherFetcher.getLocation(prefs)) + } + + @Test + fun `getLocation returns null when only longitude is saved`() { + prefs.edit() + .putLong("saved_longitude", 13.405.toRawBits()) + .commit() + + assertNull(WeatherFetcher.getLocation(prefs)) + } + + @Test + fun `getLocation falls back to cached Float coordinates for upgrade`() { + // Pre-v1.0.9: fetchWeatherSync wrote cached_* as Float + prefs.edit() + .putFloat("cached_latitude", 52.52f) + .putFloat("cached_longitude", 13.405f) + .commit() + + val location = WeatherFetcher.getLocation(prefs) + assertNotNull(location) + assertEquals(52.52, location!!.first, 0.01) + assertEquals(13.405, location.second, 0.01) + } + + @Test + fun `getLocation prefers saved Long over cached Float`() { + val lat = 48.85 + val lon = 2.35 + prefs.edit() + .putLong("saved_latitude", lat.toRawBits()) + .putLong("saved_longitude", lon.toRawBits()) + .putFloat("cached_latitude", 52.52f) + .putFloat("cached_longitude", 13.405f) + .commit() + + val location = WeatherFetcher.getLocation(prefs) + assertNotNull(location) + assertEquals(lat, location!!.first, 0.0001) + assertEquals(lon, location.second, 0.0001) + } + // ==================== Helper Methods ==================== private fun createMockApiResponse(): JSONObject { diff --git a/docs/ai/architecture.md b/docs/ai/architecture.md index a65bad4..8211901 100644 --- a/docs/ai/architecture.md +++ b/docs/ai/architecture.md @@ -79,6 +79,45 @@ class _HomeScreenState extends State { } ``` +## SharedPreferences Keys + +All keys live in the `HomeWidgetPreferences` file (shared between Dart via `home_widget` and native Kotlin). + +### User Intent (`saved_*`) — written by Dart via `home_widget` + +| Key | Type | Written by | Read by | Purpose | +|-----|------|-----------|---------|---------| +| `saved_latitude` | Long (double bits) | `location_service.dart` | `WeatherFetcher.kt` | User's chosen latitude | +| `saved_longitude` | Long (double bits) | `location_service.dart` | `WeatherFetcher.kt` | User's chosen longitude | +| `saved_city` | String | `location_service.dart` | `location_service.dart` | User's chosen city name | +| `saved_location_source` | String | `location_service.dart` | `location_service.dart` | User's chosen mode (`gps` or `manual`) | + +**Note:** `home_widget` stores Dart `double` values as `Long` via `Double.doubleToRawLongBits()`. Native Kotlin must read with `getLong()` + `Double.fromBits()`, not `getFloat()`. + +### Cache (`cached_*`) — written by Kotlin after successful fetch + +| Key | Type | Written by | Read by | Purpose | +|-----|------|-----------|---------|---------| +| `cached_weather` | String (JSON) | `WeatherFetcher.kt` | `WeatherDataParser.kt`, Dart | Full weather response | +| `cached_latitude` | Float | `WeatherFetcher.kt` | — (reserved for future use) | Last successfully fetched latitude | +| `cached_longitude` | Float | `WeatherFetcher.kt` | — (reserved for future use) | Last successfully fetched longitude | +| `cached_city_name` | String | `home_screen.dart` | `home_screen.dart` | Last displayed city name | +| `cached_location_source` | String | `home_screen.dart` | `home_screen.dart` | Location source from last successful display (for UI restore) | +| `current_temperature_celsius` | String | `WeatherFetcher.kt` | `native_svg_service.dart` | Current temp for quick display | + +### Widget State — written/read by Kotlin + +| Key | Type | Written by | Read by | Purpose | +|-----|------|-----------|---------|---------| +| `last_weather_update` | String (millis) | `WeatherFetcher.kt` | `WidgetUtils.kt`, `native_svg_service.dart` | Timestamp of last successful fetch | +| `last_render_time` | Long (millis) | `WidgetUtils.kt` | `WidgetUtils.kt` | Timestamp of last widget render | +| `widget_width_px` | Int | `MeteogramWidgetProvider.kt` | `WidgetUtils.kt` | Widget width in pixels | +| `widget_height_px` | Int | `MeteogramWidgetProvider.kt` | `WidgetUtils.kt` | Widget height in pixels | +| `widget_ids` | String (CSV) | `MeteogramWidgetProvider.kt` | `WidgetUtils.kt` | Active widget IDs | +| `widget_resized` | Boolean | `MeteogramWidgetProvider.kt` | `home_screen.dart` | Flag to trigger in-app re-render | +| `svg_path_light` | String | `WidgetUtils.kt` | `MeteogramWidgetProvider.kt` | Path to light theme SVG | +| `svg_path_dark` | String | `WidgetUtils.kt` | `MeteogramWidgetProvider.kt` | Path to dark theme SVG | + ## Key Components ### SvgChartGenerator (`android/.../SvgChartGenerator.kt`) diff --git a/docs/ai/widget.md b/docs/ai/widget.md index e7eaa13..42e4d60 100644 --- a/docs/ai/widget.md +++ b/docs/ai/widget.md @@ -393,24 +393,16 @@ Future homeWidgetBackgroundCallback(Uri? uri) async { } } +// NOTE: Re-rendering is now handled natively in Kotlin. +// WidgetUtils.rerenderAllWidgetsNative() reads cached weather from +// SharedPreferences and generates SVGs via SvgChartGenerator.kt. +// The Dart-based _reRenderCharts flow below is historical reference only. + // Re-render from cached data (no network call) -// URI params: width, height, locale (all optional, fallback to cached/Platform values) Future _reRenderCharts([Uri? uri]) async { - // Extract params from URI (more reliable than Platform.localeName in cold-start) - final uriWidth = int.tryParse(uri?.queryParameters['width'] ?? ''); - final uriHeight = int.tryParse(uri?.queryParameters['height'] ?? ''); - final uriLocale = uri?.queryParameters['locale']; - final cachedJson = await HomeWidget.getWidgetData('cached_weather'); if (cachedJson == null) return; - - final weather = WeatherData.fromJson(jsonDecode(cachedJson)); - final latitude = await HomeWidget.getWidgetData('cached_latitude') ?? 0.0; - final longitude = await HomeWidget.getWidgetData('cached_longitude') ?? 0.0; - - await _generateSvgCharts(weather, latitude, longitude, - uriWidth: uriWidth, uriHeight: uriHeight, uriLocale: uriLocale); - await HomeWidget.updateWidget(androidName: 'MeteogramWidgetProvider'); + // Native Kotlin handles SVG generation and widget update } ``` @@ -530,13 +522,14 @@ void main() async { ## Data Flow -1. **App loads weather** from Open-Meteo API -2. **SVG generated** in Dart via `SvgChartGenerator` (both light AND dark themes) -3. **SVG files saved** to app documents folder (`meteogram_light.svg`, `meteogram_dark.svg`) -4. **Widget data saved** via HomeWidget.saveWidgetData → SharedPreferences (SVG paths) +1. **App saves location** via `HomeWidget.saveWidgetData` (`saved_latitude`/`saved_longitude` as Long-encoded doubles, `saved_city`) +2. **App loads weather** from Open-Meteo API (via native `WeatherFetcher.kt`) +3. **SVG generated** natively in Kotlin via `SvgChartGenerator.kt` (both light AND dark themes) +4. **Weather JSON cached** to SharedPreferences (`cached_weather`) 5. **Native provider** reads SVG, renders via AndroidSVG → Bitmap → ImageView -6. **In-app display** uses same pipeline via PlatformView (AndroidView) -7. **Theme switching** handled automatically by Android resource system +6. **Background refresh** reads `saved_latitude`/`saved_longitude` from SharedPreferences (user's chosen location) +7. **In-app display** uses same native SVG pipeline via PlatformView (AndroidView) +8. **Theme switching** handled automatically by Android resource system **Key benefit:** SVG generation works in background isolates (no Flutter UI required), enabling true background updates. diff --git a/lib/main.dart b/lib/main.dart index 636e2c8..458ff93 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -4,6 +4,7 @@ import 'l10n/app_localizations.dart'; import 'package:flutter_localizations/flutter_localizations.dart'; import 'screens/home_screen.dart'; +import 'services/location_service.dart'; import 'services/widget_service.dart'; import 'services/material_you_service.dart'; import 'theme/app_theme.dart'; @@ -19,6 +20,7 @@ void main() async { systemNavigationBarDividerColor: Colors.transparent, )); + await LocationService.migrateIfNeeded(); await WidgetService.initialize(); // Load Material You colors from native Android code diff --git a/lib/screens/home_screen.dart b/lib/screens/home_screen.dart index 71048d8..2c08214 100644 --- a/lib/screens/home_screen.dart +++ b/lib/screens/home_screen.dart @@ -284,6 +284,15 @@ class _HomeScreenState extends State with WidgetsBindingObserver { try { final location = await _locationService.getLocation(); + // Persist coordinates for native background refresh (AlarmManager, WorkManager, etc.) + // GPS coordinates change each fetch; background services need the latest. + await _locationService.saveLocation( + location.latitude, + location.longitude, + city: location.city, + source: location.source, + ); + // Fetch weather via native Kotlin HTTP client final success = await NativeSvgService.fetchWeather( latitude: location.latitude, diff --git a/lib/services/location_service.dart b/lib/services/location_service.dart index e58ed45..045da4b 100644 --- a/lib/services/location_service.dart +++ b/lib/services/location_service.dart @@ -15,8 +15,8 @@ class LocationService { static const String _latKey = 'saved_latitude'; static const String _lonKey = 'saved_longitude'; static const String _cityKey = 'saved_city'; - static const String _useGpsKey = 'use_gps'; - static const String _sourceKey = 'location_source'; + static const String _sourceKey = 'saved_location_source'; + static const String _legacySourceKey = 'location_source'; /// HTTP client for making requests. Defaults to standard client. final http.Client _client; @@ -24,15 +24,44 @@ class LocationService { /// Create a LocationService with optional custom HTTP client. LocationService({http.Client? client}) : _client = client ?? http.Client(); + /// Migrate legacy SharedPreferences keys to current names. + /// Call once at app startup. Safe to call multiple times. + static Future migrateIfNeeded() async { + final prefs = await SharedPreferences.getInstance(); + + // v1.0.8 → v1.0.9: location_source → saved_location_source, remove use_gps + // These legacy keys were always written together, so checking one suffices. + if (prefs.containsKey(_legacySourceKey)) { + if (!prefs.containsKey(_sourceKey)) { + final value = prefs.getString(_legacySourceKey); + if (value != null) { + await prefs.setString(_sourceKey, value); + } + } + await prefs.remove(_legacySourceKey); + await prefs.remove('use_gps'); + + // Also clean up HomeWidget storage + final legacyWidgetSource = await HomeWidget.getWidgetData(_legacySourceKey); + if (legacyWidgetSource != null) { + if (await HomeWidget.getWidgetData(_sourceKey) == null) { + await HomeWidget.saveWidgetData(_sourceKey, legacyWidgetSource); + } + await HomeWidget.saveWidgetData(_legacySourceKey, null); + } + await HomeWidget.saveWidgetData('use_gps', null); + } + } + /// Get the current location (GPS or saved). Future getLocation() async { final prefs = await SharedPreferences.getInstance(); - final useGps = prefs.getBool(_useGpsKey) ?? true; + final source = prefs.getString(_sourceKey); - if (useGps) { - return _getGpsLocation(); - } else { + if (source == LocationSource.manual.name) { return _getSavedLocation(prefs); + } else { + return _getGpsLocation(); } } @@ -189,70 +218,50 @@ class LocationService { LocationSource source = LocationSource.manual, }) async { final prefs = await SharedPreferences.getInstance(); + + // Skip writes if nothing changed (avoids redundant I/O on every refresh) + if (prefs.getDouble(_latKey) == latitude && + prefs.getDouble(_lonKey) == longitude && + prefs.getString(_sourceKey) == source.name && + prefs.getString(_cityKey) == city) { + return; + } + await prefs.setDouble(_latKey, latitude); await prefs.setDouble(_lonKey, longitude); if (city != null) { await prefs.setString(_cityKey, city); + } else { + await prefs.remove(_cityKey); } await prefs.setString(_sourceKey, source.name); - await prefs.setBool(_useGpsKey, false); // Also save to HomeWidget for background service access - await HomeWidget.saveWidgetData('saved_latitude', latitude); - await HomeWidget.saveWidgetData('saved_longitude', longitude); + await HomeWidget.saveWidgetData(_latKey, latitude); + await HomeWidget.saveWidgetData(_lonKey, longitude); if (city != null) { - await HomeWidget.saveWidgetData('saved_city', city); + await HomeWidget.saveWidgetData(_cityKey, city); + } else { + await HomeWidget.saveWidgetData(_cityKey, null); } - await HomeWidget.saveWidgetData('location_source', source.name); - await HomeWidget.saveWidgetData('use_gps', false); + await HomeWidget.saveWidgetData(_sourceKey, source.name); } /// Switch to using GPS location. /// Saves to both SharedPreferences (for app) and HomeWidget (for background service). Future useGpsLocation() async { final prefs = await SharedPreferences.getInstance(); - await prefs.setBool(_useGpsKey, true); + await prefs.setString(_sourceKey, LocationSource.gps.name); // Also save to HomeWidget for background service access - await HomeWidget.saveWidgetData('use_gps', true); - await HomeWidget.saveWidgetData('location_source', LocationSource.gps.name); + await HomeWidget.saveWidgetData(_sourceKey, LocationSource.gps.name); } - /// Check if using GPS. + /// Check if using GPS. Defaults to true (GPS) when no source is saved. Future isUsingGps() async { final prefs = await SharedPreferences.getInstance(); - return prefs.getBool(_useGpsKey) ?? true; - } - - /// Get saved location from HomeWidget storage (for background service). - /// Returns null if no location is saved or if using GPS. - /// This is more reliable than SharedPreferences in background isolates. - Future getSavedLocationFromWidget() async { - final useGps = await HomeWidget.getWidgetData('use_gps') ?? true; - if (useGps) { - return null; // GPS mode, no saved location - } - - final lat = await HomeWidget.getWidgetData('saved_latitude'); - final lon = await HomeWidget.getWidgetData('saved_longitude'); - final city = await HomeWidget.getWidgetData('saved_city'); - final sourceName = await HomeWidget.getWidgetData('location_source'); - - if (lat == null || lon == null) { - return null; - } - - final source = LocationSource.values.firstWhere( - (s) => s.name == sourceName, - orElse: () => LocationSource.manual, - ); - - return LocationData( - latitude: lat, - longitude: lon, - source: source, - city: city, - ); + final source = prefs.getString(_sourceKey); + return source == null || source == LocationSource.gps.name; } /// Request GPS permission explicitly. diff --git a/test/location_service_test.dart b/test/location_service_test.dart index ed01658..5ab9ad3 100644 --- a/test/location_service_test.dart +++ b/test/location_service_test.dart @@ -550,7 +550,7 @@ void main() { expect(prefs.getDouble('saved_latitude'), 48.85); expect(prefs.getDouble('saved_longitude'), 2.35); expect(prefs.getString('saved_city'), 'Paris'); - expect(prefs.getBool('use_gps'), false); + expect(prefs.getString('saved_location_source'), 'manual'); }); test('useGpsLocation sets GPS flag', () async { @@ -561,6 +561,31 @@ void main() { expect(await service.isUsingGps(), isTrue); }); + test('saveLocation clears stale city when switching to GPS', () async { + final service = LocationService(client: MockClient((r) async => http.Response('', 200))); + + await service.saveLocation(48.85, 2.35, city: 'Paris'); + final prefs = await SharedPreferences.getInstance(); + expect(prefs.getString('saved_city'), 'Paris'); + + await service.saveLocation(48.85, 2.35, source: LocationSource.gps); + expect(prefs.getString('saved_city'), isNull); + }); + + test('saveLocation skips writes when nothing changed', () async { + final service = LocationService(client: MockClient((r) async => http.Response('', 200))); + + await service.saveLocation(48.85, 2.35, city: 'Paris'); + final prefs = await SharedPreferences.getInstance(); + expect(prefs.getString('saved_city'), 'Paris'); + + // Same values — should be a no-op (we can't directly observe the skip, + // but we verify the values remain correct) + await service.saveLocation(48.85, 2.35, city: 'Paris'); + expect(prefs.getDouble('saved_latitude'), 48.85); + expect(prefs.getString('saved_city'), 'Paris'); + }); + test('saveLocation disables GPS', () async { final service = LocationService(client: MockClient((r) async => http.Response('', 200))); @@ -572,6 +597,62 @@ void main() { }); }); + group('migrateIfNeeded', () { + test('migrates legacy keys and cleans up', () async { + SharedPreferences.setMockInitialValues({ + 'location_source': 'manual', + 'use_gps': false, + }); + + await LocationService.migrateIfNeeded(); + + final prefs = await SharedPreferences.getInstance(); + expect(prefs.getString('saved_location_source'), 'manual'); + expect(prefs.containsKey('location_source'), isFalse); + expect(prefs.containsKey('use_gps'), isFalse); + }); + + test('cleans up legacy keys without overwriting existing value', () async { + SharedPreferences.setMockInitialValues({ + 'saved_location_source': 'gps', + 'location_source': 'manual', + 'use_gps': false, + }); + + await LocationService.migrateIfNeeded(); + + final prefs = await SharedPreferences.getInstance(); + expect(prefs.getString('saved_location_source'), 'gps'); + expect(prefs.containsKey('location_source'), isFalse); + expect(prefs.containsKey('use_gps'), isFalse); + }); + + test('is a no-op when no keys exist', () async { + SharedPreferences.setMockInitialValues({}); + + await LocationService.migrateIfNeeded(); + + final prefs = await SharedPreferences.getInstance(); + expect(prefs.containsKey('saved_location_source'), isFalse); + expect(prefs.containsKey('location_source'), isFalse); + }); + + test('is idempotent', () async { + SharedPreferences.setMockInitialValues({ + 'location_source': 'manual', + 'use_gps': false, + }); + + await LocationService.migrateIfNeeded(); + await LocationService.migrateIfNeeded(); + + final prefs = await SharedPreferences.getInstance(); + expect(prefs.getString('saved_location_source'), 'manual'); + expect(prefs.containsKey('location_source'), isFalse); + expect(prefs.containsKey('use_gps'), isFalse); + }); + }); + group('Default location constants', () { test('default latitude is Berlin', () { expect(kDefaultLatitude, closeTo(52.52, 0.01));