fix: Lade-Race in BoolPref/StringSetPref (Review-Befund)
StringSetPref.add/remove machten Read-Modify-Write auf möglicherweise noch ungeladenem State: ein add() direkt nach Erstellung persistierte nur den neuen Eintrag und LÖSCHTE alle bereits gespeicherten (per neuem Test am alten Code bewiesen). Jetzt warten add/remove auf _loaded. BoolPref bekommt einen _userSet-Guard, damit ein spät landender persistierter Wert nie eine schon getätigte Nutzeraktion zurücküberschreibt (Reihenfolge ist plattform- abhängig). In der Praxis kaum treffbar (Provider laden beim App-Start), aber die Fassade wird beim M2-Storage-Umbau Pflicht-Schnittstelle für neue, ggf. früh laufende Aufrufer. 2 neue Regressionstests, alle 24 grün. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1212,3 +1212,8 @@
|
|||||||
2026-07-03 22:52 [Write] /home/steggi/pyramid/test/settings_prefs_test.dart
|
2026-07-03 22:52 [Write] /home/steggi/pyramid/test/settings_prefs_test.dart
|
||||||
2026-07-03 22:53 [Edit] /home/steggi/pyramid/PROGRESS.md
|
2026-07-03 22:53 [Edit] /home/steggi/pyramid/PROGRESS.md
|
||||||
2026-07-03 22:53 [Edit] /home/steggi/pyramid/docs/PC_TESTPLAN.md
|
2026-07-03 22:53 [Edit] /home/steggi/pyramid/docs/PC_TESTPLAN.md
|
||||||
|
2026-07-03 22:54 [Edit] /home/steggi/.claude-pyramid/projects/-home-steggi-pyramid/memory/pyramid-arbeitsstand.md
|
||||||
|
2026-07-03 23:18 [Edit] /home/steggi/pyramid/test/settings_prefs_test.dart
|
||||||
|
2026-07-03 23:18 [Edit] /home/steggi/pyramid/test/settings_prefs_test.dart
|
||||||
|
2026-07-03 23:19 [Edit] /home/steggi/pyramid/lib/core/settings_prefs.dart
|
||||||
|
2026-07-03 23:20 [Edit] /home/steggi/pyramid/lib/core/settings_prefs.dart
|
||||||
|
|||||||
@@ -7,6 +7,9 @@ import 'package:shared_preferences/shared_preferences.dart';
|
|||||||
|
|
||||||
class BoolPref extends StateNotifier<bool> {
|
class BoolPref extends StateNotifier<bool> {
|
||||||
final String _key;
|
final String _key;
|
||||||
|
// Guards against the load race: if set()/toggle() ran before _load's read
|
||||||
|
// lands, the stale persisted value must not overwrite the user's choice.
|
||||||
|
bool _userSet = false;
|
||||||
|
|
||||||
BoolPref(this._key, bool defaultValue) : super(defaultValue) {
|
BoolPref(this._key, bool defaultValue) : super(defaultValue) {
|
||||||
_load(defaultValue);
|
_load(defaultValue);
|
||||||
@@ -14,10 +17,11 @@ class BoolPref extends StateNotifier<bool> {
|
|||||||
|
|
||||||
Future<void> _load(bool def) async {
|
Future<void> _load(bool def) async {
|
||||||
final p = await SharedPreferences.getInstance();
|
final p = await SharedPreferences.getInstance();
|
||||||
if (mounted) state = p.getBool(_key) ?? def;
|
if (mounted && !_userSet) state = p.getBool(_key) ?? def;
|
||||||
}
|
}
|
||||||
|
|
||||||
Future<void> set(bool v) async {
|
Future<void> set(bool v) async {
|
||||||
|
_userSet = true;
|
||||||
state = v;
|
state = v;
|
||||||
final p = await SharedPreferences.getInstance();
|
final p = await SharedPreferences.getInstance();
|
||||||
await p.setBool(_key, v);
|
await p.setBool(_key, v);
|
||||||
@@ -30,9 +34,10 @@ class BoolPref extends StateNotifier<bool> {
|
|||||||
|
|
||||||
class StringSetPref extends StateNotifier<Set<String>> {
|
class StringSetPref extends StateNotifier<Set<String>> {
|
||||||
final String _key;
|
final String _key;
|
||||||
|
late final Future<void> _loaded;
|
||||||
|
|
||||||
StringSetPref(this._key) : super({}) {
|
StringSetPref(this._key) : super({}) {
|
||||||
_load();
|
_loaded = _load();
|
||||||
}
|
}
|
||||||
|
|
||||||
Future<void> _load() async {
|
Future<void> _load() async {
|
||||||
@@ -42,14 +47,18 @@ class StringSetPref extends StateNotifier<Set<String>> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
Future<void> add(String v) async {
|
Future<void> add(String v) async {
|
||||||
if (state.contains(v)) return;
|
// Read-modify-write: ohne abgeschlossenes Laden würde hier der halbleere
|
||||||
|
// Start-State zurückgeschrieben und persistierte Einträge gingen verloren.
|
||||||
|
await _loaded;
|
||||||
|
if (!mounted || state.contains(v)) return;
|
||||||
state = {...state, v};
|
state = {...state, v};
|
||||||
final p = await SharedPreferences.getInstance();
|
final p = await SharedPreferences.getInstance();
|
||||||
await p.setStringList(_key, state.toList());
|
await p.setStringList(_key, state.toList());
|
||||||
}
|
}
|
||||||
|
|
||||||
Future<void> remove(String v) async {
|
Future<void> remove(String v) async {
|
||||||
if (!state.contains(v)) return;
|
await _loaded;
|
||||||
|
if (!mounted || !state.contains(v)) return;
|
||||||
state = {...state}..remove(v);
|
state = {...state}..remove(v);
|
||||||
final p = await SharedPreferences.getInstance();
|
final p = await SharedPreferences.getInstance();
|
||||||
await p.setStringList(_key, state.toList());
|
await p.setStringList(_key, state.toList());
|
||||||
|
|||||||
@@ -23,6 +23,21 @@ void main() {
|
|||||||
expect(pref.state, false, reason: 'persistierter Wert muss gewinnen');
|
expect(pref.state, false, reason: 'persistierter Wert muss gewinnen');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test(
|
||||||
|
'set() sofort nach Erstellung gewinnt gegen den später ladenden '
|
||||||
|
'persistierten Wert (Lade-Race)', () async {
|
||||||
|
SharedPreferences.setMockInitialValues({'notif_sound': false});
|
||||||
|
final pref = BoolPref('notif_sound', false);
|
||||||
|
final done = pref.set(true); // bevor _load fertig ist
|
||||||
|
await pumpEventQueue();
|
||||||
|
await done;
|
||||||
|
expect(pref.state, true,
|
||||||
|
reason: 'die Nutzeraktion darf nicht vom Lade-Ergebnis '
|
||||||
|
'zurücküberschrieben werden');
|
||||||
|
final p = await SharedPreferences.getInstance();
|
||||||
|
expect(p.getBool('notif_sound'), true);
|
||||||
|
});
|
||||||
|
|
||||||
test('set/toggle ändern Zustand und persistieren', () async {
|
test('set/toggle ändern Zustand und persistieren', () async {
|
||||||
final pref = BoolPref('notif_desktop', true);
|
final pref = BoolPref('notif_desktop', true);
|
||||||
await pumpEventQueue();
|
await pumpEventQueue();
|
||||||
@@ -49,6 +64,23 @@ void main() {
|
|||||||
expect(p.getStringList('muted_rooms')!.toSet(), {'!b:hs', '!c:hs'});
|
expect(p.getStringList('muted_rooms')!.toSet(), {'!b:hs', '!c:hs'});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test(
|
||||||
|
'add() sofort nach Erstellung verliert keine bereits persistierten '
|
||||||
|
'Einträge (Lade-Race)', () async {
|
||||||
|
SharedPreferences.setMockInitialValues({
|
||||||
|
'muted_rooms': ['!a:hs'],
|
||||||
|
});
|
||||||
|
final pref = StringSetPref('muted_rooms');
|
||||||
|
final done = pref.add('!b:hs'); // bevor _load fertig ist
|
||||||
|
await pumpEventQueue();
|
||||||
|
await done;
|
||||||
|
expect(pref.state, {'!a:hs', '!b:hs'});
|
||||||
|
final p = await SharedPreferences.getInstance();
|
||||||
|
expect(p.getStringList('muted_rooms')!.toSet(), {'!a:hs', '!b:hs'},
|
||||||
|
reason: 'Read-Modify-Write auf ungeladenem State darf keine '
|
||||||
|
'persistierten Einträge verlieren');
|
||||||
|
});
|
||||||
|
|
||||||
test('doppeltes add und remove von Fehlendem sind No-Ops', () async {
|
test('doppeltes add und remove von Fehlendem sind No-Ops', () async {
|
||||||
final pref = StringSetPref('muted_rooms');
|
final pref = StringSetPref('muted_rooms');
|
||||||
await pumpEventQueue();
|
await pumpEventQueue();
|
||||||
|
|||||||
Reference in New Issue
Block a user