From 6741aada4acd6cba002efef2f8f147584440bff7 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Tue, 6 Oct 2026 22:00:48 +0700 Subject: [PATCH] fix(settings): retain pending preferences during asynchronous loading --- lib/providers/settings_provider.dart | 26 +++- lib/providers/theme_provider.dart | 67 +++++++--- test/settings_persistence_test.dart | 153 ++++++++++++++++++++++ test/theme_provider_persistence_test.dart | 131 ++++++++++++++++++ 4 files changed, 349 insertions(+), 28 deletions(-) create mode 100644 test/settings_persistence_test.dart create mode 100644 test/theme_provider_persistence_test.dart diff --git a/lib/providers/settings_provider.dart b/lib/providers/settings_provider.dart index 3deaab54..57bf92ff 100644 --- a/lib/providers/settings_provider.dart +++ b/lib/providers/settings_provider.dart @@ -92,6 +92,9 @@ Future resetRestoredInstallationSettings(SharedPreferences prefs) async { } class SettingsNotifier extends Notifier { + SettingsNotifier({Future Function()? preferences}) + : _loadPreferences = preferences ?? SharedPreferences.getInstance; + static final RegExp _isoRegionPattern = RegExp(r'^[A-Z]{2}$'); static const Set _searchTabValues = { 'all', @@ -119,9 +122,10 @@ class SettingsNotifier extends Notifier { 2000, }; - Future get _prefs => SharedPreferences.getInstance(); + final Future Function() _loadPreferences; + Future get _prefs => _loadPreferences(); final FlutterSecureStorage _secureStorage = const FlutterSecureStorage(); - bool _isSavingSettings = false; + Future? _saveSettingsFuture; bool _saveQueued = false; String? _pendingSettingsJson; Future? _loadSettingsFuture; @@ -302,15 +306,22 @@ class SettingsNotifier extends Notifier { } } - Future _saveSettings() async { + Future _saveSettings() { _pendingSettingsJson = jsonEncode(state.toJson()); - if (_isSavingSettings) { + final activeSave = _saveSettingsFuture; + if (activeSave != null) { _saveQueued = true; - return; + return activeSave; } - _isSavingSettings = true; + final saved = Completer(); + _saveSettingsFuture = saved.future; + unawaited(_drainSettingsSave(saved)); + return saved.future; + } + + Future _drainSettingsSave(Completer saved) async { try { final prefs = await _prefs; do { @@ -323,7 +334,8 @@ class SettingsNotifier extends Notifier { } catch (e) { _log.e('Failed to save settings: $e'); } finally { - _isSavingSettings = false; + _saveSettingsFuture = null; + saved.complete(); } } diff --git a/lib/providers/theme_provider.dart b/lib/providers/theme_provider.dart index b7280ab8..7938bec4 100644 --- a/lib/providers/theme_provider.dart +++ b/lib/providers/theme_provider.dart @@ -27,71 +27,96 @@ final themeProvider = NotifierProvider(() { }); class ThemeNotifier extends Notifier { - final Future _prefs = SharedPreferences.getInstance(); + ThemeNotifier({Future? preferences}) + : _prefs = preferences ?? SharedPreferences.getInstance(); + + final Future _prefs; + late Future _loadFuture; + Future _saveChain = Future.value(); + final _loadingEdits = []; + bool _loaded = false; @override ThemeSettings build() { - _loadFromStorage(); + _loadFuture = _loadFromStorage(); return ref.read(initialThemeSettingsProvider); } Future _loadFromStorage() async { try { final prefs = await _prefs; - state = loadBootstrapThemeSettings(prefs); + var loaded = loadBootstrapThemeSettings(prefs); + for (final edit in _loadingEdits) { + loaded = edit(loaded); + } + if (ref.mounted) state = loaded; } catch (e) { debugPrint('Error loading theme settings: $e'); + } finally { + _loaded = true; + _loadingEdits.clear(); } } - Future _saveToStorage() async { + void _editState(ThemeSettings Function(ThemeSettings) edit) { + if (!_loaded) _loadingEdits.add(edit); + state = edit(state); + } + + Future _saveToStorage() => _saveChain = _saveChain.then((_) async { try { + await _loadFuture; final prefs = await _prefs; - await prefs.setString(kThemeModeKey, state.themeMode.name); - await prefs.setBool(kUseDynamicColorKey, state.useDynamicColor); - await prefs.setInt(kSeedColorKey, state.seedColorValue); - await prefs.setBool(kUseAmoledKey, state.useAmoled); - await prefs.setString(kThemeStyleKey, state.style.name); - await prefs.setString(kMornyeAccentKey, state.mornyeAccent.name); - await prefs.setBool(kUseSystemFontKey, state.useSystemFont); - await prefs.setDouble(kMornyeGlassClarityKey, state.mornyeGlassClarity); + if (!ref.mounted) return; + final snapshot = state; + await prefs.setString(kThemeModeKey, snapshot.themeMode.name); + await prefs.setBool(kUseDynamicColorKey, snapshot.useDynamicColor); + await prefs.setInt(kSeedColorKey, snapshot.seedColorValue); + await prefs.setBool(kUseAmoledKey, snapshot.useAmoled); + await prefs.setString(kThemeStyleKey, snapshot.style.name); + await prefs.setString(kMornyeAccentKey, snapshot.mornyeAccent.name); + await prefs.setBool(kUseSystemFontKey, snapshot.useSystemFont); + await prefs.setDouble( + kMornyeGlassClarityKey, + snapshot.mornyeGlassClarity, + ); } catch (e) { debugPrint('Error saving theme settings: $e'); } - } + }); Future setThemeMode(ThemeMode mode) async { - state = state.copyWith(themeMode: mode); + _editState((current) => current.copyWith(themeMode: mode)); await _saveToStorage(); } Future setStyle(AppThemeStyle style) async { - state = state.copyWith(style: style); + _editState((current) => current.copyWith(style: style)); await _saveToStorage(); } Future setUseDynamicColor(bool value) async { - state = state.copyWith(useDynamicColor: value); + _editState((current) => current.copyWith(useDynamicColor: value)); await _saveToStorage(); } Future setSeedColor(Color color) async { - state = state.copyWith(seedColorValue: color.toARGB32()); + _editState((current) => current.copyWith(seedColorValue: color.toARGB32())); await _saveToStorage(); } Future setUseAmoled(bool value) async { - state = state.copyWith(useAmoled: value); + _editState((current) => current.copyWith(useAmoled: value)); await _saveToStorage(); } Future setMornyeAccent(MornyeAccent accent) async { - state = state.copyWith(mornyeAccent: accent); + _editState((current) => current.copyWith(mornyeAccent: accent)); await _saveToStorage(); } Future setUseSystemFont(bool value) async { - state = state.copyWith(useSystemFont: value); + _editState((current) => current.copyWith(useSystemFont: value)); await _saveToStorage(); } @@ -99,7 +124,7 @@ class ThemeNotifier extends Notifier { double value, { bool persist = true, }) async { - state = state.copyWith(mornyeGlassClarity: value); + _editState((current) => current.copyWith(mornyeGlassClarity: value)); if (persist) await _saveToStorage(); } } diff --git a/test/settings_persistence_test.dart b/test/settings_persistence_test.dart new file mode 100644 index 00000000..38341ec2 --- /dev/null +++ b/test/settings_persistence_test.dart @@ -0,0 +1,153 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:spotiflac_android/models/settings.dart'; +import 'package:spotiflac_android/providers/settings_provider.dart'; +import 'package:spotiflac_android/utils/logger.dart'; + +class _Preferences implements SharedPreferences { + final writes = >[]; + final _started = >{}; + final _released = >{}; + + Future whenStarted(int index) => + (_started[index] ??= Completer()).future; + + Completer release(int index) => _released[index] ??= Completer(); + + @override + Future setString(String key, String value) { + expect(key, 'app_settings'); + final index = writes.length; + writes.add(jsonDecode(value) as Map); + (_started[index] ??= Completer()).complete(); + return release(index).future; + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +class _Settings extends SettingsNotifier { + _Settings(_Preferences preferences) + : super(preferences: () async => preferences); + + _Settings.withLoader(Future Function() load) + : super(preferences: load); + + @override + AppSettings build() => const AppSettings(); +} + +void main() { + test('queued save callers await the complete coalescing drain', () async { + final preferences = _Preferences(); + final container = ProviderContainer( + overrides: [settingsProvider.overrideWith(() => _Settings(preferences))], + ); + addTearDown(container.dispose); + final notifier = container.read(settingsProvider.notifier); + final first = notifier.useAppFolderStorage('/output/first'); + await preferences.whenStarted(0); + final second = notifier.setNetworkDownloadFolder('remote:first', 'First'); + final third = notifier.setNetworkDownloadFolder('remote:latest', 'Latest'); + var firstCompleted = false; + var secondCompleted = false; + var thirdCompleted = false; + unawaited(first.then((_) => firstCompleted = true)); + unawaited(second.then((_) => secondCompleted = true)); + unawaited(third.then((_) => thirdCompleted = true)); + await Future.delayed(Duration.zero); + expect( + [firstCompleted, secondCompleted, thirdCompleted], + [false, false, false], + ); + + preferences.release(0).complete(true); + await preferences.whenStarted(1); + expect(preferences.writes.length, 2); + expect(preferences.writes.last['networkDownloadFolder'], 'remote:latest'); + expect(preferences.writes.last['networkDownloadLabel'], 'Latest'); + expect(preferences.writes.last['downloadDirectory'], '/output/first'); + expect( + [firstCompleted, secondCompleted, thirdCompleted], + [false, false, false], + ); + preferences.release(1).complete(true); + await Future.wait([first, second, third]); + expect( + [firstCompleted, secondCompleted, thirdCompleted], + [true, true, true], + ); + expect(preferences.writes.length, 2); + }); + + test( + 'failed active save keeps logged-error API and releases its owner', + () async { + final preferences = _Preferences(); + final container = ProviderContainer( + overrides: [ + settingsProvider.overrideWith(() => _Settings(preferences)), + ], + ); + addTearDown(container.dispose); + final notifier = container.read(settingsProvider.notifier); + final first = notifier.useAppFolderStorage('/output/first'); + await preferences.whenStarted(0); + final queued = notifier.setNetworkDownloadFolder( + 'remote:queued', + 'Queued', + ); + preferences.release(0).completeError(StateError('settings_save_failed')); + await Future.wait([first, queued]); + expect( + LogBuffer().entries.any( + (entry) => + entry.tag == 'SettingsProvider' && + entry.message.contains('settings_save_failed'), + ), + isTrue, + ); + + final retry = notifier.useAppFolderStorage('/output/retry'); + await preferences.whenStarted(1); + expect(preferences.writes.last['downloadDirectory'], '/output/retry'); + expect(preferences.writes.last['networkDownloadFolder'], 'remote:queued'); + preferences.release(1).complete(true); + await retry; + }, + ); + + test( + 'synchronous preference failure does not retain a finished drain', + () async { + final preferences = _Preferences(); + var loads = 0; + final container = ProviderContainer( + overrides: [ + settingsProvider.overrideWith( + () => _Settings.withLoader(() { + if (loads++ == 0) throw StateError('settings_preferences_failed'); + return Future.value(preferences); + }), + ), + ], + ); + addTearDown(container.dispose); + final notifier = container.read(settingsProvider.notifier); + await notifier.useAppFolderStorage('/output/first'); + expect(preferences.writes, isEmpty); + + final retry = notifier.useAppFolderStorage('/output/retry'); + await preferences.whenStarted(0); + expect(loads, 2); + expect(preferences.writes.single['downloadDirectory'], '/output/retry'); + preferences.release(0).complete(true); + await retry; + }, + ); +} diff --git a/test/theme_provider_persistence_test.dart b/test/theme_provider_persistence_test.dart new file mode 100644 index 00000000..ff902970 --- /dev/null +++ b/test/theme_provider_persistence_test.dart @@ -0,0 +1,131 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:spotiflac_android/models/theme_settings.dart'; +import 'package:spotiflac_android/providers/theme_provider.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + test( + 'early theme edits preserve untouched saved settings and await saving', + () async { + SharedPreferences.setMockInitialValues({ + kThemeModeKey: 'dark', + kThemeStyleKey: 'mornye', + kSeedColorKey: 0xff123456, + kUseDynamicColorKey: false, + kUseAmoledKey: true, + kUseSystemFontKey: true, + kMornyeAccentKey: 'blue', + kMornyeGlassClarityKey: 0.2, + }); + final prefs = await SharedPreferences.getInstance(); + final releaseLoad = Completer(); + final container = ProviderContainer( + overrides: [ + themeProvider.overrideWith( + () => ThemeNotifier(preferences: releaseLoad.future), + ), + ], + ); + addTearDown(container.dispose); + final notifier = container.read(themeProvider.notifier); + // An explicit edit equal to the bootstrap default still owns this field. + final savingMode = notifier.setThemeMode(ThemeMode.system); + final savingAccent = notifier.setMornyeAccent(MornyeAccent.red); + await notifier.setMornyeGlassClarity(0.8, persist: false); + var saved = false; + unawaited(savingAccent.then((_) => saved = true)); + await Future.delayed(Duration.zero); + expect(saved, isFalse); + expect(container.read(themeProvider).mornyeGlassClarity, 0.8); + expect(prefs.getString(kThemeModeKey), 'dark'); + + releaseLoad.complete(prefs); + await Future.wait([savingMode, savingAccent]); + final state = container.read(themeProvider); + expect(state.themeMode, ThemeMode.system); + expect(state.mornyeAccent, MornyeAccent.red); + expect(state.mornyeGlassClarity, 0.8); + expect(state.style, AppThemeStyle.mornye); + expect(state.seedColorValue, 0xff123456); + expect(state.useDynamicColor, isFalse); + expect(state.useAmoled, isTrue); + expect(state.useSystemFont, isTrue); + expect(loadBootstrapThemeSettings(prefs), state); + }, + ); + + test('early preview remains immediate without writing preferences', () async { + SharedPreferences.setMockInitialValues({ + kMornyeGlassClarityKey: 0.2, + kMornyeAccentKey: 'blue', + }); + final prefs = await SharedPreferences.getInstance(); + final releaseLoad = Completer(); + final loaded = Completer(); + final container = ProviderContainer( + overrides: [ + themeProvider.overrideWith( + () => ThemeNotifier(preferences: releaseLoad.future), + ), + ], + ); + addTearDown(container.dispose); + container.listen(themeProvider, (_, next) { + if (next.mornyeAccent == MornyeAccent.blue) loaded.complete(); + }); + await container + .read(themeProvider.notifier) + .setMornyeGlassClarity(0.8, persist: false); + expect(container.read(themeProvider).mornyeGlassClarity, 0.8); + releaseLoad.complete(prefs); + await loaded.future; + expect(container.read(themeProvider).mornyeGlassClarity, 0.8); + expect(prefs.getDouble(kMornyeGlassClarityKey), 0.2); + }); + + test( + 'failed preference loading keeps early edits without leaking errors', + () async { + final releaseLoad = Completer(); + final container = ProviderContainer( + overrides: [ + themeProvider.overrideWith( + () => ThemeNotifier(preferences: releaseLoad.future), + ), + ], + ); + addTearDown(container.dispose); + final saving = container.read(themeProvider.notifier).setUseAmoled(true); + releaseLoad.completeError(StateError('theme_load_failed')); + await saving; + expect(container.read(themeProvider).useAmoled, isTrue); + }, + ); + + test( + 'disposed theme does not read state or save after delayed loading', + () async { + SharedPreferences.setMockInitialValues({kUseAmoledKey: false}); + final prefs = await SharedPreferences.getInstance(); + final releaseLoad = Completer(); + final container = ProviderContainer( + overrides: [ + themeProvider.overrideWith( + () => ThemeNotifier(preferences: releaseLoad.future), + ), + ], + ); + final saving = container.read(themeProvider.notifier).setUseAmoled(true); + container.dispose(); + releaseLoad.complete(prefs); + await saving; + expect(prefs.getBool(kUseAmoledKey), isFalse); + }, + ); +}